mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-18 08:36:13 +00:00
fix(marge): synthesise or skip presets with unresolvable sources in /full
When a preset on disk referenced a source no longer in the configured- sources list, mapPresetsToFullResponse appended it with an empty <source/> block. The speaker decodes /full as protobuf and treats the inner source fields (id, type, sourceproviderid, credential) as required, so the malformed block aborted the whole account sync and wiped the speaker's locally stored presets — the GH-269 symptom of "/presets empty within seconds of AfterTouch coming online". For well-known radio providers (TuneIn, InternetRadio, LocalInternetRadio, RadioBrowser) the preset now gets a synthesised source block built from canonical defaults; account-bound providers (Spotify, Amazon) are skipped with a log line so other presets in the response survive the sync. Also folds RADIO_BROWSER into resolveSourceName's fallback switch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
e643495287
commit
09ec332375
@@ -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 <source> 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 <source/> 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
|
||||
|
||||
|
||||
@@ -748,15 +748,20 @@ func TestAccountFullToXML_WithBackupStructure(t *testing.T) {
|
||||
t.Errorf("Expected <name>Living Room SoundTouch</name> 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 := `<?xml version="1.0" encoding="UTF-8"?>
|
||||
<presets>
|
||||
<preset id="1" createdOn="1719128436" updatedOn="1728740382">
|
||||
<contentItem source="SPOTIFY" type="tracklisturl" location="/playback/container/c3BvdGlmeTpwbGF5bGlzdDo1Mm5QaVJrbWVmSkZPeHh1M1ZTd1hh" itemName="test-playlist" isPresetable="true" contentItemType="tracklisturl">
|
||||
<containerArt>https://i.scdn.co/image/art</containerArt>
|
||||
<contentItem source="TUNEIN" type="stationurl" location="/v1/playback/station/s166521" itemName="SMOOTH JAZZ" isPresetable="true" contentItemType="stationurl">
|
||||
<containerArt>https://cdn-profiles.tunein.com/s166521/images/logod.png</containerArt>
|
||||
</contentItem>
|
||||
<sourceid>10004</sourceid>
|
||||
</preset>
|
||||
</presets>`
|
||||
_ = 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, `<contentItemType>tracklisturl</contentItemType>`) {
|
||||
t.Errorf("Expected <contentItemType>tracklisturl</contentItemType>, got %s", xmlStr2)
|
||||
if !strings.Contains(xmlStr2, `<contentItemType>stationurl</contentItemType>`) {
|
||||
t.Errorf("Expected <contentItemType>stationurl</contentItemType>, got %s", xmlStr2)
|
||||
}
|
||||
|
||||
// 3. Test with empty name
|
||||
|
||||
@@ -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 <source/> 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 <sourceproviderid>.
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user