diff --git a/pkg/service/marge/marge.go b/pkg/service/marge/marge.go index 743024d..124e911 100644 --- a/pkg/service/marge/marge.go +++ b/pkg/service/marge/marge.go @@ -700,6 +700,8 @@ func resolveSourceName(s models.ConfiguredSource) string { name = constants.ProviderLocalInternetRadio case constants.ProviderTunein: name = constants.ProviderTunein + case constants.ProviderRadioBrowser: + name = constants.ProviderRadioBrowser case constants.ProviderAux: name = constants.ProviderAux } @@ -791,6 +793,30 @@ func canonicalProviderIDByID(id string) string { return "" } +// canonicalDefaultsByType returns the canonical (built-in) source ID and +// SourceProviderID for a well-known provider key type. Used to synthesise a +// minimum-viable block when a preset references a source we no +// longer have in the configured-sources list (e.g. after a factory reset +// that hasn't repopulated the device's sources yet). +// +// Returns ("", "") for provider types whose IDs are account-specific +// (Spotify, Amazon) — those cannot be synthesised without losing +// per-account state, so callers must skip the preset instead. +func canonicalDefaultsByType(sourceKeyType string) (id, providerID string) { + switch sourceKeyType { + case constants.ProviderInternetRadio: + return "10002", strconv.Itoa(constants.InternetRadioProviderID) + case constants.ProviderLocalInternetRadio: + return "10003", strconv.Itoa(constants.LocalInternetRadioProviderID) + case constants.ProviderTunein: + return "10004", strconv.Itoa(constants.TuneinProviderID) + case constants.ProviderRadioBrowser: + return "10005", strconv.Itoa(constants.RadioBrowserProviderID) + } + + return "", "" +} + func mapPresetsToFullResponse(presets []models.ServicePreset, sources []models.ConfiguredSource) []models.FullResponsePreset { var fullPresets []models.FullResponsePreset @@ -853,14 +879,63 @@ func mapPresetsToFullResponse(presets []models.ServicePreset, sources []models.C if matchedSource != nil { fullPreset.Source = mapToFullResponseSource(*matchedSource) + fullPresets = append(fullPresets, fullPreset) + + continue } - fullPresets = append(fullPresets, fullPreset) + // No configured source matches this preset. Emitting the preset + // with an empty block produces a /full response whose + // inner protobuf is missing required fields (sourceproviderid, + // id, type, credential), which makes the speaker abort the whole + // account sync and wipe its local presets. See GH-269. + // + // For the well-known built-in providers we can synthesise a + // minimum-viable source block from canonical defaults; the + // credential will be empty so play-time will fail loudly, but + // the sync survives and other presets stay intact. For + // account-bound providers (Spotify, Amazon) we can't synthesise + // without losing state, so we skip the preset entirely — its + // slot reverts to "Select a preset" until the source is + // repopulated, which is recoverable; a wiped /presets is not. + if synth, ok := synthesiseDefaultSourceForPreset(p); ok { + fullPreset.Source = synth + fullPresets = append(fullPresets, fullPreset) + + continue + } + + log.Printf("[Marge] /full: skipping preset %s — source %q (id=%q, account=%q) not in configured sources", + p.ButtonNumber, p.Source, p.SourceID, p.SourceAccount) } return fullPresets } +// synthesiseDefaultSourceForPreset builds a minimum-viable FullResponseSource +// for a preset whose source is no longer in the configured-sources list, +// using canonical built-in defaults. Returns (zero, false) when the preset's +// 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) + if id == "" || providerID == "" { + return models.FullResponseSource{}, false + } + + synth := models.FullResponseSource{ + ID: id, + Type: "Audio", + CreatedOn: constants.DateStr, + UpdatedOn: constants.DateStr, + SourceProviderID: providerID, + Name: p.Source, + } + synth.Credential.Type = constants.CredentialTypeToken + + return synth, true +} + func mapRecentsToFullResponse(recents []models.ServiceRecent, sources []models.ConfiguredSource) []models.FullResponseRecent { var fullRecents []models.FullResponseRecent diff --git a/pkg/service/marge/marge_test.go b/pkg/service/marge/marge_test.go index 9bc2fe3..0b5ab64 100644 --- a/pkg/service/marge/marge_test.go +++ b/pkg/service/marge/marge_test.go @@ -748,15 +748,20 @@ func TestAccountFullToXML_WithBackupStructure(t *testing.T) { t.Errorf("Expected Living Room SoundTouch under device, got %s", xmlStr) } - // 2. Verify ButtonNumber and ContentItemType mapping + // 2. Verify ButtonNumber and ContentItemType mapping. Uses TUNEIN + // because that's one of the default configured sources every device + // gets at pair time — a SPOTIFY preset with no matching configured + // source would now correctly be skipped per the GH-269 fix, which + // would defeat this test's structural assertion. presetsDir := filepath.Join(deviceDir) _ = os.MkdirAll(presetsDir, 0755) presetsXML := ` - - https://i.scdn.co/image/art + + https://cdn-profiles.tunein.com/s166521/images/logod.png + 10004 ` _ = os.WriteFile(filepath.Join(presetsDir, "Presets.xml"), []byte(presetsXML), 0644) @@ -767,8 +772,8 @@ func TestAccountFullToXML_WithBackupStructure(t *testing.T) { if !strings.Contains(xmlStr2, `buttonNumber="1"`) { t.Errorf("Expected buttonNumber=\"1\", got %s", xmlStr2) } - if !strings.Contains(xmlStr2, `tracklisturl`) { - t.Errorf("Expected tracklisturl, got %s", xmlStr2) + if !strings.Contains(xmlStr2, `stationurl`) { + t.Errorf("Expected stationurl, got %s", xmlStr2) } // 3. Test with empty name diff --git a/pkg/service/marge/presets_unresolved_source_regression_test.go b/pkg/service/marge/presets_unresolved_source_regression_test.go new file mode 100644 index 0000000..239164f --- /dev/null +++ b/pkg/service/marge/presets_unresolved_source_regression_test.go @@ -0,0 +1,215 @@ +package marge + +import ( + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/constants" +) + +// TestMapPresetsToFullResponse_UnresolvedSource is a regression test for +// GH-269: when a preset on disk references a source that is no longer in +// the device's configured-sources list (e.g. after the user restored +// Presets.xml from a backup but Sources.xml was reset, or sources were +// renumbered after a factory reset), AfterTouch used to emit the preset +// in /full with a completely empty inner block. +// +// The speaker decodes /full as protobuf, where the inner source block +// has required fields (id, type, sourceproviderid, credential). An empty +// block fails IsInitialized() and the speaker aborts the whole account +// sync — wiping its locally stored presets in the process. Users saw +// "/presets is empty within seconds of AfterTouch coming online" even +// though the browser-readable /full XML looked right. +// +// The fix: +// +// 1. Presets that resolve against the configured-sources list are +// emitted unchanged (happy path, no regression). +// 2. Presets whose SourceKeyType matches a built-in radio provider +// (TuneIn, InternetRadio, LocalInternetRadio, RadioBrowser) get a +// synthesised source block carrying the canonical default ID and +// sourceproviderid. The credential is empty so play-time will fail +// visibly, but the sync survives and other presets stay intact. +// 3. Presets whose SourceKeyType is account-bound (Spotify, Amazon) +// and unresolvable are dropped from the response. The preset stays +// on disk and returns once the source is repopulated; the speaker's +// slot reverts to "Select a preset" until then. +func TestMapPresetsToFullResponse_UnresolvedSource(t *testing.T) { + // Configured sources contain only TuneIn — RADIO_BROWSER, INTERNET_RADIO + // and SPOTIFY are deliberately absent so unresolved presets exercise + // the synthesise / skip branches. + 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", + }, + } + + presets := []models.ServicePreset{ + // 1. Happy path: SourceID resolves directly against configured. + { + ServiceContentItem: models.ServiceContentItem{ + Name: "Resolved TuneIn", + Source: constants.ProviderTunein, + SourceID: "14774275", + Location: "/v1/playback/station/s166521", + ContentItemType: "stationurl", + }, + ButtonNumber: "1", + CreatedOn: "2026-04-04T21:25:33.000+00:00", + UpdatedOn: "2026-04-04T21:25:33.000+00:00", + }, + // 2. Synthesise: INTERNET_RADIO type with no configured match. + { + ServiceContentItem: models.ServiceContentItem{ + Name: "Orphaned Internet Radio", + Source: constants.ProviderInternetRadio, + SourceID: "88888888", + Location: "http://example.invalid/stream.mp3", + ContentItemType: "stationurl", + }, + ButtonNumber: "2", + CreatedOn: "2026-04-04T21:25:33.000+00:00", + UpdatedOn: "2026-04-04T21:25:33.000+00:00", + }, + // 3. Skip: Spotify is account-bound; no canonical default. + { + ServiceContentItem: models.ServiceContentItem{ + Name: "Orphaned Spotify", + Source: constants.ProviderSpotify, + SourceID: "100004", + Location: "/playback/container/abc", + ContentItemType: "tracklisturl", + }, + ButtonNumber: "3", + CreatedOn: "2026-04-04T21:25:33.000+00:00", + UpdatedOn: "2026-04-04T21:25:33.000+00:00", + }, + // 4. Synthesise: RADIO_BROWSER with no configured match. + { + ServiceContentItem: models.ServiceContentItem{ + Name: "Orphaned RadioBrowser", + Source: constants.ProviderRadioBrowser, + SourceID: "99999999", + Location: "/something/radio-browser", + ContentItemType: "stationurl", + }, + ButtonNumber: "4", + CreatedOn: "2026-04-04T21:25:33.000+00:00", + UpdatedOn: "2026-04-04T21:25:33.000+00:00", + }, + } + + got := mapPresetsToFullResponse(presets, configured) + + if len(got) != 3 { + t.Fatalf("expected 3 emitted presets (1 resolved + 2 synthesised, Spotify skipped), got %d: %+v", len(got), got) + } + + byButton := map[string]models.FullResponsePreset{} + for _, p := range got { + byButton[p.ButtonNumber] = p + } + + if _, ok := byButton["3"]; ok { + t.Errorf("expected preset 3 (orphaned Spotify) to be skipped, but it was emitted") + } + + requireNonEmptySourceBlock(t, "preset 1 (resolved TuneIn)", byButton["1"]) + if byButton["1"].Source.ID != "14774275" { + t.Errorf("preset 1: expected configured source id 14774275, got %q", byButton["1"].Source.ID) + } + + if byButton["1"].Source.SourceProviderID != "25" { + t.Errorf("preset 1: expected sourceproviderid 25 (TuneIn), got %q", byButton["1"].Source.SourceProviderID) + } + + requireNonEmptySourceBlock(t, "preset 2 (synthesised InternetRadio)", byButton["2"]) + if byButton["2"].Source.ID != "10002" { + t.Errorf("preset 2: expected canonical InternetRadio id 10002, got %q", byButton["2"].Source.ID) + } + + if byButton["2"].Source.SourceProviderID != "2" { + t.Errorf("preset 2: expected sourceproviderid 2 (InternetRadio), got %q", byButton["2"].Source.SourceProviderID) + } + + requireNonEmptySourceBlock(t, "preset 4 (synthesised RadioBrowser)", byButton["4"]) + if byButton["4"].Source.ID != "10005" { + t.Errorf("preset 4: expected canonical RadioBrowser id 10005, got %q", byButton["4"].Source.ID) + } + + if byButton["4"].Source.SourceProviderID != "39" { + t.Errorf("preset 4: expected sourceproviderid 39 (RadioBrowser), got %q", byButton["4"].Source.SourceProviderID) + } +} + +// requireNonEmptySourceBlock asserts the FullResponseSource has every +// protobuf-required leaf field populated. This is the structural invariant +// the speaker enforces when decoding /full — see the comment near +// AccountFullToXML about not stripping empty . +func requireNonEmptySourceBlock(t *testing.T, label string, p models.FullResponsePreset) { + t.Helper() + + if p.Source.ID == "" { + t.Errorf("%s: source.id is empty", label) + } + + if p.Source.Type == "" { + t.Errorf("%s: source.type is empty", label) + } + + if p.Source.SourceProviderID == "" { + t.Errorf("%s: sourceproviderid is empty", label) + } + + if p.Source.CreatedOn == "" { + t.Errorf("%s: source.createdOn is empty", label) + } + + if p.Source.UpdatedOn == "" { + t.Errorf("%s: source.updatedOn is empty", label) + } +} + +// TestCanonicalDefaultsByType pins the type → (id, providerid) mapping +// against the canonical IDs the speaker firmware ships with. +// canonicalProviderIDByID (the inverse) is already tested implicitly via +// the recents regression test; we want a direct check here too. +func TestCanonicalDefaultsByType(t *testing.T) { + cases := []struct { + sourceKeyType string + wantID string + wantProviderID string + wantSynthesisable bool + }{ + {constants.ProviderInternetRadio, "10002", "2", true}, + {constants.ProviderLocalInternetRadio, "10003", "11", true}, + {constants.ProviderTunein, "10004", "25", true}, + {constants.ProviderRadioBrowser, "10005", "39", true}, + {constants.ProviderSpotify, "", "", false}, + {constants.ProviderAmazon, "", "", false}, + {"COMPLETELY_UNKNOWN", "", "", false}, + } + + for _, tc := range cases { + t.Run(tc.sourceKeyType, func(t *testing.T) { + id, providerID := canonicalDefaultsByType(tc.sourceKeyType) + if id != tc.wantID { + t.Errorf("id: want %q, got %q", tc.wantID, id) + } + + if providerID != tc.wantProviderID { + t.Errorf("providerID: want %q, got %q", tc.wantProviderID, providerID) + } + + synthesisable := id != "" && providerID != "" + if synthesisable != tc.wantSynthesisable { + t.Errorf("synthesisable: want %v, got %v", tc.wantSynthesisable, synthesisable) + } + }) + } +}