diff --git a/pkg/service/marge/lastplayedat_test.go b/pkg/service/marge/lastplayedat_test.go index cebe762..0d4791d 100644 --- a/pkg/service/marge/lastplayedat_test.go +++ b/pkg/service/marge/lastplayedat_test.go @@ -15,18 +15,30 @@ func TestLastPlayedAtParity(t *testing.T) { utcTimeStr := strconv.FormatInt(now, 10) expectedLastPlayedAt := time.Unix(now, 0).UTC().Format("2006-01-02T15:04:05.000+00:00") + // The recent needs a resolvable source so mapRecentsToFullResponse + // emits it — empty blocks are now filtered to avoid + // protobuf-required-field aborts on the speaker (GH-269). recents := []models.ServiceRecent{ { ServiceContentItem: models.ServiceContentItem{ - ID: "1", - Name: "Recent 1", + ID: "1", + Name: "Recent 1", + Source: "TUNEIN", + SourceID: "10004", }, UtcTime: utcTimeStr, LastPlayedAt: "", // Empty in datastore }, } - sources := []models.ConfiguredSource{} + sources := []models.ConfiguredSource{ + { + ID: "10004", + Type: "Audio", + SourceKeyType: "TUNEIN", + SourceProviderID: "25", + }, + } fullRecents := mapRecentsToFullResponse(recents, sources) diff --git a/pkg/service/marge/marge.go b/pkg/service/marge/marge.go index 8bccc23..62c2ee3 100644 --- a/pkg/service/marge/marge.go +++ b/pkg/service/marge/marge.go @@ -920,7 +920,21 @@ func mapPresetsToFullResponse(presets []models.ServicePreset, sources []models.C // provider type isn't one we can safely synthesise (e.g. Spotify, Amazon, // where the ID and credential are account-bound). func synthesiseDefaultSourceForPreset(p *models.ServicePreset) (models.FullResponseSource, bool) { - id, providerID := canonicalDefaultsByType(p.Source) + return synthesiseDefaultSourceByType(p.Source) +} + +// synthesiseDefaultSourceForRecent is the recent-side twin of +// synthesiseDefaultSourceForPreset. Same protobuf-required-field invariants +// apply to recents>recent>source. +func synthesiseDefaultSourceForRecent(r *models.ServiceRecent) (models.FullResponseSource, bool) { + return synthesiseDefaultSourceByType(r.Source) +} + +// synthesiseDefaultSourceByType is the shared helper behind the preset/recent +// synthesisers. Keep them as separate wrappers so callers stay readable and +// the call site signals which container we're patching up. +func synthesiseDefaultSourceByType(sourceKeyType string) (models.FullResponseSource, bool) { + id, providerID := canonicalDefaultsByType(sourceKeyType) if id == "" || providerID == "" { return models.FullResponseSource{}, false } @@ -931,13 +945,41 @@ func synthesiseDefaultSourceForPreset(p *models.ServicePreset) (models.FullRespo CreatedOn: constants.DateStr, UpdatedOn: constants.DateStr, SourceProviderID: providerID, - Name: p.Source, + Name: sourceKeyType, } synth.Credential.Type = constants.CredentialTypeToken return synth, true } +// findMatchingSourceForRecent resolves the configured source a recent +// references. Tries exact SourceID match first, then SourceKeyType+account. +// Returns nil when nothing matches — caller falls back to synthesise/skip. +func findMatchingSourceForRecent(r *models.ServiceRecent, sources []models.ConfiguredSource) *models.ConfiguredSource { + if r.SourceID != "" { + for j := range sources { + if sources[j].ID == r.SourceID { + copySource := sources[j] + PrepareConfiguredSource(©Source) + + return ©Source + } + } + } + + for j := range sources { + s := sources[j] + if s.SourceKeyType == r.Source && (r.SourceAccount == "" || s.SourceKeyAccount == r.SourceAccount) { + copySource := s + PrepareConfiguredSource(©Source) + + return ©Source + } + } + + return nil +} + func mapRecentsToFullResponse(recents []models.ServiceRecent, sources []models.ConfiguredSource) []models.FullResponseRecent { var fullRecents []models.FullResponseRecent @@ -951,33 +993,7 @@ func mapRecentsToFullResponse(recents []models.ServiceRecent, sources []models.C r.UpdatedOn = constants.DateStr } - var matchedSource *models.ConfiguredSource - // 1. Try exact ID match first - if r.SourceID != "" { - for j := range sources { - if sources[j].ID == r.SourceID { - copySource := sources[j] - PrepareConfiguredSource(©Source) - matchedSource = ©Source - - break - } - } - } - - // 2. Fallback to type/account match if ID didn't match or was empty - if matchedSource == nil { - for j := range sources { - s := sources[j] - if s.SourceKeyType == r.Source && (r.SourceAccount == "" || s.SourceKeyAccount == r.SourceAccount) { - copySource := s - PrepareConfiguredSource(©Source) - matchedSource = ©Source - - break - } - } - } + matchedSource := findMatchingSourceForRecent(r, sources) fullRecent := models.FullResponseRecent{ ID: r.ID, @@ -1009,9 +1025,34 @@ func mapRecentsToFullResponse(recents []models.ServiceRecent, sources []models.C if fullRecent.SourceID == "" { fullRecent.SourceID = fullRecent.Source.ID } + + fullRecents = append(fullRecents, fullRecent) + + continue } - fullRecents = append(fullRecents, fullRecent) + // Same protobuf-required-field hazard as presets: emitting a + // recent with an empty block makes the speaker abort + // the whole /full sync. Mirror the preset behaviour — synthesise + // for well-known radio providers, skip otherwise. Recents are + // less load-bearing than presets, but a single broken recent can + // still take the sync down. + if synth, ok := synthesiseDefaultSourceForRecent(r); ok { + log.Printf("[Marge] /full: synthesising canonical %s source (id=%s, providerid=%s) for recent id=%s — original source %q not in configured sources", + r.Source, synth.ID, synth.SourceProviderID, r.ID, r.SourceID) + + fullRecent.Source = synth + if fullRecent.SourceID == "" { + fullRecent.SourceID = synth.ID + } + + fullRecents = append(fullRecents, fullRecent) + + continue + } + + log.Printf("[Marge] /full: skipping recent id=%s — source %q (id=%q, account=%q) not in configured sources and not synthesisable", + r.ID, r.Source, r.SourceID, r.SourceAccount) } return fullRecents diff --git a/pkg/service/marge/presets_unresolved_source_regression_test.go b/pkg/service/marge/presets_unresolved_source_regression_test.go index 239164f..49eb0fb 100644 --- a/pkg/service/marge/presets_unresolved_source_regression_test.go +++ b/pkg/service/marge/presets_unresolved_source_regression_test.go @@ -175,6 +175,89 @@ func requireNonEmptySourceBlock(t *testing.T, label string, p models.FullRespons } } +// TestMapRecentsToFullResponse_UnresolvedSource is the recent-side twin of +// the preset regression above. Same protobuf invariant applies to the +// inner of ; an empty block aborts the whole /full sync +// — the older AccountFullToXML_RecentWithPoisonedSourceProviderID case +// caught one form of this via post-marshal stripping. The skip/synthesise +// path here closes the other form: orphan recent whose source is no +// longer configured at all. +func TestMapRecentsToFullResponse_UnresolvedSource(t *testing.T) { + configured := []models.ConfiguredSource{ + { + ID: "14774275", + Type: "Audio", + SourceKeyType: constants.ProviderTunein, + SourceProviderID: "25", + CreatedOn: "2017-07-20T16:43:48.000+00:00", + UpdatedOn: "2017-07-20T16:43:48.000+00:00", + }, + } + + recents := []models.ServiceRecent{ + // 1. Resolved by exact SourceID match. + { + ServiceContentItem: models.ServiceContentItem{ + ID: "rec-1", + Name: "Resolved TuneIn", + Source: constants.ProviderTunein, + SourceID: "14774275", + Location: "/v1/playback/station/s166521", + }, + LastPlayedAt: "2026-04-04T21:25:33.000+00:00", + }, + // 2. Synthesise: LOCAL_INTERNET_RADIO no longer configured. + { + ServiceContentItem: models.ServiceContentItem{ + ID: "rec-2", + Name: "Orphaned laut.fm", + Source: constants.ProviderLocalInternetRadio, + SourceID: "77777777", + Location: "http://example.invalid/custom/v1/playback/abc", + }, + LastPlayedAt: "2026-04-04T21:25:33.000+00:00", + }, + // 3. Skip: Spotify is account-bound. + { + ServiceContentItem: models.ServiceContentItem{ + ID: "rec-3", + Name: "Orphaned Spotify", + Source: constants.ProviderSpotify, + SourceID: "100004", + Location: "/playback/container/abc", + }, + LastPlayedAt: "2026-04-04T21:25:33.000+00:00", + }, + } + + got := mapRecentsToFullResponse(recents, configured) + + if len(got) != 2 { + t.Fatalf("expected 2 emitted recents (1 resolved + 1 synthesised, Spotify skipped), got %d", len(got)) + } + + byID := map[string]models.FullResponseRecent{} + for _, r := range got { + byID[r.ID] = r + } + + if _, ok := byID["rec-3"]; ok { + t.Errorf("expected rec-3 (orphaned Spotify) to be skipped, but it was emitted") + } + + if byID["rec-1"].Source.SourceProviderID != "25" { + t.Errorf("rec-1: expected sourceproviderid=25 (TuneIn), got %q", byID["rec-1"].Source.SourceProviderID) + } + + if byID["rec-2"].Source.ID != "10003" { + t.Errorf("rec-2: expected synthesised LocalInternetRadio id=10003, got %q", byID["rec-2"].Source.ID) + } + + if byID["rec-2"].Source.SourceProviderID != "11" { + t.Errorf("rec-2: expected synthesised LocalInternetRadio providerid=11, got %q", byID["rec-2"].Source.SourceProviderID) + } +} + // TestCanonicalDefaultsByType pins the type → (id, providerid) mapping // against the canonical IDs the speaker firmware ships with. // canonicalProviderIDByID (the inverse) is already tested implicitly via