fix(marge): mirror skip-or-synthesise into mapRecentsToFullResponse

Recents had the same protobuf-required-field hazard as presets — an
empty <source/> block inside <recent> would also abort the speaker's
/full sync (the recents poisoned-sourceproviderid regression
documented this once for a related sub-symptom). Apply the same
skip-or-synthesise filter so an orphaned recent can never take the
whole account sync down.

The synthesise/skip code paths log at info level; same visibility
posture as the preset side.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-05-20 22:30:53 +02:00
co-authored by Claude Sonnet 4.6
parent 22f60459ba
commit 9f8c1cf536
3 changed files with 169 additions and 33 deletions
+15 -3
View File
@@ -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 <source/> 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)
+71 -30
View File
@@ -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(&copySource)
return &copySource
}
}
}
for j := range sources {
s := sources[j]
if s.SourceKeyType == r.Source && (r.SourceAccount == "" || s.SourceKeyAccount == r.SourceAccount) {
copySource := s
PrepareConfiguredSource(&copySource)
return &copySource
}
}
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(&copySource)
matchedSource = &copySource
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(&copySource)
matchedSource = &copySource
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 <source/> 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
@@ -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 <source> of <recent>; 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