From c305d22de099cdca592291e498216b0a04e59a22 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 23 May 2026 21:08:07 +0200 Subject: [PATCH] refactor(datastore): drop INTERNET_RADIO from initial Sources.xml Add getInitialSources() that excludes the legacy INTERNET_RADIO (10002) provider from newly-created device Sources.xml files. GetDefaultSources() retains the entry for backward-compatible canonicalisation of existing devices and cloud-level account responses. Fix mergeDefaultSources() to rebuild the merged list in canonical ID order (defaults first, using stored credentials when present, then custom sources such as Spotify). This prevents INTERNET_RADIO from landing at the end of the cloud /sources response when a device's Sources.xml was created without it. Drop the two verbose search-loop log lines from resolvePresetSource. Co-Authored-By: Claude Sonnet 4.6 --- pkg/service/datastore/datastore.go | 23 ++++++++++- .../datastore/sources_deduction_test.go | 30 ++++++-------- pkg/service/marge/marge.go | 41 ++++++++++++++----- pkg/service/marge/marge_test.go | 13 +++--- 4 files changed, 71 insertions(+), 36 deletions(-) diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index 21d603e..93c03ef 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -1779,7 +1779,7 @@ func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.Conf data, err := ds.rootReadFile(path) if err != nil { if os.IsNotExist(err) { - sources := ds.getDefaultSources() + sources := ds.getInitialSources() ds.DeduceSourceIDs(account, device, sources) return sources, nil @@ -2189,6 +2189,27 @@ func (ds *DataStore) getDefaultSources() []models.ConfiguredSource { return sources } +// getInitialSources returns the default sources for a brand-new device. +// INTERNET_RADIO (ID 10002) is excluded — it is a legacy provider no longer +// actively served by AfterTouch; omitting it prevents speakers from +// receiving a stale entry in their initial Sources.xml. The full list +// (including 10002) is kept in getDefaultSources() for canonicalisation +// of existing devices that already reference INTERNET_RADIO. +func (ds *DataStore) getInitialSources() []models.ConfiguredSource { + all := ds.getDefaultSources() + + out := make([]models.ConfiguredSource, 0, len(all)) + for i := range all { + if all[i].SourceKeyType == constants.ProviderInternetRadio { + continue + } + + out = append(out, all[i]) + } + + return out +} + // isMACAddressFormat checks if a string looks like a MAC address func isMACAddressFormat(s string) bool { // AABBCCDDEEFF format diff --git a/pkg/service/datastore/sources_deduction_test.go b/pkg/service/datastore/sources_deduction_test.go index b295db1..f95fc46 100644 --- a/pkg/service/datastore/sources_deduction_test.go +++ b/pkg/service/datastore/sources_deduction_test.go @@ -24,6 +24,7 @@ func TestGetConfiguredSources_DeduceIDs(t *testing.T) { t.Fatalf("Failed to create device dir: %v", err) } + // Provider 11 = LOCAL_INTERNET_RADIO (active, present in initial sources) recentsXML := ` @@ -33,13 +34,13 @@ func TestGetConfiguredSources_DeduceIDs(t *testing.T) { 52349 Lounge FM Digital - 2015-03-11T19:12:38.000+00:00 + 2019-01-24T08:18:37.000+00:00 9330201 - 2 + 11 - 2015-03-11T19:12:38.000+00:00 + 2019-02-03T18:35:45.000+00:00 9330201 @@ -52,7 +53,7 @@ func TestGetConfiguredSources_DeduceIDs(t *testing.T) { t.Fatalf("Failed to write Recents.xml: %v", err) } - // Now call GetConfiguredSources and expect it to have "9330201" for provider ID "2" + // Now call GetConfiguredSources and expect it to have "9330201" for provider ID "11" sources, err := ds.GetConfiguredSources(account, device) if err != nil { t.Fatalf("GetConfiguredSources failed: %v", err) @@ -60,17 +61,17 @@ func TestGetConfiguredSources_DeduceIDs(t *testing.T) { foundDeducted := false for _, s := range sources { - if s.SourceProviderID == "2" { + if s.SourceProviderID == "11" { if s.ID == "9330201" { foundDeducted = true } else { - t.Errorf("Expected source ID 9330201 for provider 2, got %s", s.ID) + t.Errorf("Expected source ID 9330201 for provider 11, got %s", s.ID) } } } if !foundDeducted { - t.Errorf("Did not find source with provider ID 2 and deducted ID 9330201") + t.Errorf("Did not find source with provider ID 11 and deducted ID 9330201") } } @@ -90,34 +91,28 @@ func TestGetConfiguredSources_DeduceIDs_AllProviders(t *testing.T) { t.Fatalf("Failed to create device dir: %v", err) } - // 2: INTERNET_RADIO // 9: AUX // 11: LOCAL_INTERNET_RADIO // 25: TUNEIN + // INTERNET_RADIO (provider 2) is intentionally excluded from initial sources + // and therefore not expected in the deduction result. presetsXML := ` - - http://example.com/art2.png - - - ID2 - - http://example.com/art9.png ID9 - + http://example.com/art11.png ID11 - + http://example.com/art25.png @@ -136,7 +131,6 @@ func TestGetConfiguredSources_DeduceIDs_AllProviders(t *testing.T) { } expected := map[string]string{ - "2": "ID2", "9": "ID9", "11": "ID11", "25": "ID25", diff --git a/pkg/service/marge/marge.go b/pkg/service/marge/marge.go index b75a118..796a496 100644 --- a/pkg/service/marge/marge.go +++ b/pkg/service/marge/marge.go @@ -1259,25 +1259,48 @@ func getAccountSources(ds *datastore.DataStore, account, lastDeviceID string) [] return fullSources } -// mergeDefaultSources adds any default sources missing from stored that are not already present -// (matched by SourceKeyType). It does not persist — initializeDefaultSources handles persistence at startup. +// mergeDefaultSources returns sources in canonical order: defaults first (using +// the stored entry's credentials when a match is found), followed by any stored +// sources that have no matching default (e.g. Spotify, Amazon). This ensures +// that default sources always appear in a predictable order regardless of what +// is present in the device's on-disk Sources.xml. +// It does not persist — initializeDefaultSources handles persistence at startup. func mergeDefaultSources(stored, defaults []models.ConfiguredSource) []models.ConfiguredSource { + result := make([]models.ConfiguredSource, 0, len(stored)+len(defaults)) + for i := range defaults { - found := false + found := -1 for j := range stored { if stored[j].SourceKeyType == defaults[i].SourceKeyType { - found = true + found = j break } } - if !found { - stored = append(stored, defaults[i]) + if found >= 0 { + result = append(result, stored[found]) + } else { + result = append(result, defaults[i]) } } - return stored + for i := range stored { + isDefault := false + + for j := range defaults { + if stored[i].SourceKeyType == defaults[j].SourceKeyType { + isDefault = true + break + } + } + + if !isDefault { + result = append(result, stored[i]) + } + } + + return result } // AccountSourcesToXML generates the account sources XML. @@ -1415,11 +1438,7 @@ func RemovePreset(ds *datastore.DataStore, account, device string, presetNumber // (since auto-add appends). Returns (nil, sources) when no match could be // resolved — UpdatePreset turns that into a 500 with a diagnostic log line. func resolvePresetSource(ds *datastore.DataStore, account, device string, sources []models.ConfiguredSource, sourceID string, presetNumber int) (*models.ConfiguredSource, []models.ConfiguredSource) { - log.Printf("[Marge] Searching for source matching ID=%s in %d sources", sourceID, len(sources)) - for i := range sources { - log.Printf("[Marge] Source[%d]: ID=%s, Type=%s, SourceKeyType=%s, SourceKeyAccount=%s", i, sources[i].ID, sources[i].Type, sources[i].SourceKeyType, sources[i].SourceKeyAccount) - if sources[i].ID == sourceID { return &sources[i], sources } diff --git a/pkg/service/marge/marge_test.go b/pkg/service/marge/marge_test.go index 0b5ab64..095ca8e 100644 --- a/pkg/service/marge/marge_test.go +++ b/pkg/service/marge/marge_test.go @@ -638,7 +638,11 @@ func TestDefaultSources(t *testing.T) { t.Fatalf("Failed to get sources: %v", err) } - expectedCount := 5 + // INTERNET_RADIO (ID 10002) is excluded from initial sources — it is a legacy + // provider no longer served by AfterTouch. The cloud-level account sources + // (getAccountSources via GetDefaultSources) still include it for backward + // compatibility with existing speaker firmware. + expectedCount := 4 if len(sources) != expectedCount { t.Errorf("Expected %d sources, got %d", expectedCount, len(sources)) } @@ -664,10 +668,7 @@ func TestDefaultSources(t *testing.T) { t.Error("LOCAL_INTERNET_RADIO should have a secret") } case "INTERNET_RADIO": - foundIR = true - if s.SecretType != "token" { - t.Errorf("Expected INTERNET_RADIO secretType token, got %s", s.SecretType) - } + t.Errorf("INTERNET_RADIO must not appear in initial sources — it is excluded from getInitialSources()") case "RADIO_BROWSER": foundIR = true if s.SecretType != "token" { @@ -695,7 +696,7 @@ func TestDefaultSources(t *testing.T) { } if !foundTuneIn || !foundLocalIR || !foundIR || !foundAux { - t.Errorf("Missing expected sources: TuneIn=%v, LocalIR=%v, IR=%v, Aux=%v", foundTuneIn, foundLocalIR, foundIR, foundAux) + t.Errorf("Missing expected sources: TuneIn=%v, LocalIR=%v, RadioBrowser=%v, Aux=%v", foundTuneIn, foundLocalIR, foundIR, foundAux) } }