diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index e55bf5c..500e41b 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -1254,6 +1254,17 @@ func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.Conf } sources := make([]models.ConfiguredSource, len(sourcesWrap.Sources)) + defaults := ds.getDefaultSources() + + // Pre-claim IDs already explicitly set in the file so the canonical fill + // below doesn't reuse them when multiple entries share a SourceKey.Type. + claimedIDs := make(map[string]bool, len(sourcesWrap.Sources)) + for i := range sourcesWrap.Sources { + if id := sourcesWrap.Sources[i].ID; id != "" { + claimedIDs[id] = true + } + } + for i := range sourcesWrap.Sources { ps := &sourcesWrap.Sources[i] s := &sources[i] @@ -1293,11 +1304,9 @@ func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.Conf s.SourceKeyAccount = s.SourceKey.Account } - // Ensure Type is populated from SourceKey if missing - if s.Type == "" && s.SourceKey.Type != "" { - s.Type = s.SourceKey.Type - } + applyCanonicalDefaults(s, defaults, claimedIDs) + // Last-resort ID for unknown providers. if s.ID == "" { s.ID = strconv.Itoa(2000001 + i) } @@ -1306,6 +1315,51 @@ func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.Conf return sources, nil } +// applyCanonicalDefaults fills missing canonical ID/Type/SourceProviderID for +// known providers and repairs Type that was previously synthesized from +// SourceKey.Type (e.g. "AUX") rather than the canonical value (e.g. "Audio"). +// Without this, the on-device Sources.xml — which carries only displayName + +// sourceKey — would round-trip as id="2000001+i" type="" and +// be rejected by the speaker as INVALID_SOURCE after migration. +// +// claimedIDs tracks which canonical IDs are already in use so that multiple +// entries with the same SourceKey.Type don't collide on the same ID. +func applyCanonicalDefaults(s *models.ConfiguredSource, defaults []models.ConfiguredSource, claimedIDs map[string]bool) { + def := findCanonicalSource(defaults, s.SourceKey.Type) + if def == nil { + return + } + + if s.ID == "" && !claimedIDs[def.ID] { + s.ID = def.ID + claimedIDs[def.ID] = true + } + + if s.Type == "" || s.Type == s.SourceKey.Type { + s.Type = def.Type + } + + if s.SourceProviderID == "" { + s.SourceProviderID = def.SourceProviderID + } +} + +// findCanonicalSource returns the default source matching the given +// SourceKey.Type, or nil if it's not one of our known providers. +func findCanonicalSource(defaults []models.ConfiguredSource, sourceKeyType string) *models.ConfiguredSource { + if sourceKeyType == "" { + return nil + } + + for i := range defaults { + if defaults[i].SourceKey.Type == sourceKeyType { + return &defaults[i] + } + } + + return nil +} + // SaveConfiguredSources saves the configured sources list for the specified account and device. func (ds *DataStore) SaveConfiguredSources(account, device string, sources []models.ConfiguredSource) error { ds.fileMutex.Lock() diff --git a/pkg/service/datastore/sources_regression_test.go b/pkg/service/datastore/sources_regression_test.go index f451d45..5692b4e 100644 --- a/pkg/service/datastore/sources_regression_test.go +++ b/pkg/service/datastore/sources_regression_test.go @@ -98,3 +98,155 @@ func TestSaveSources_Format(t *testing.T) { t.Errorf("Sources.xml should not contain tag") } } + +// TestGetConfiguredSources_MinimalAuxEntryNormalized covers the migration case from +// issue #195: the device's on-disk Sources.xml carries only displayName + sourceKey +// for AUX (no id, no type). When read back, the AUX entry must surface as the +// canonical id="10001" type="Audio" sourceproviderid="9", not synthesized values. +func TestGetConfiguredSources_MinimalAuxEntryNormalized(t *testing.T) { + tempDir, err := os.MkdirTemp("", "st-sources-min-aux-*") + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.RemoveAll(tempDir) }() + + ds := NewDataStore(tempDir) + account := "1234567" + device := "001122334455" + + deviceDir := ds.AccountDeviceDir(account, device) + if err := os.MkdirAll(deviceDir, 0755); err != nil { + t.Fatal(err) + } + + minimalSourcesXML := ` + + + +` + if err := os.WriteFile(filepath.Join(deviceDir, "Sources.xml"), []byte(minimalSourcesXML), 0644); err != nil { + t.Fatal(err) + } + + sources, err := ds.GetConfiguredSources(account, device) + if err != nil { + t.Fatalf("GetConfiguredSources failed: %v", err) + } + + if len(sources) != 1 { + t.Fatalf("expected 1 source, got %d", len(sources)) + } + + s := sources[0] + if s.ID != "10001" { + t.Errorf("expected canonical AUX id 10001, got %q", s.ID) + } + if s.Type != "Audio" { + t.Errorf("expected canonical AUX type 'Audio', got %q", s.Type) + } + if s.SourceKey.Type != "AUX" || s.SourceKey.Account != "AUX" { + t.Errorf("expected sourceKey type/account AUX/AUX, got %q/%q", s.SourceKey.Type, s.SourceKey.Account) + } +} + +// TestGetConfiguredSources_DuplicateProviderUniqueIDs ensures that when a file +// contains multiple entries for the same SourceKey.Type (e.g. two AUX entries), +// only one gets the canonical ID; the rest fall back to synthesized IDs so they +// don't collide. +func TestGetConfiguredSources_DuplicateProviderUniqueIDs(t *testing.T) { + tempDir, err := os.MkdirTemp("", "st-sources-dup-*") + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.RemoveAll(tempDir) }() + + ds := NewDataStore(tempDir) + account := "1234567" + device := "001122334455" + + deviceDir := ds.AccountDeviceDir(account, device) + if err := os.MkdirAll(deviceDir, 0755); err != nil { + t.Fatal(err) + } + + dupXML := ` + + + + + + +` + if err := os.WriteFile(filepath.Join(deviceDir, "Sources.xml"), []byte(dupXML), 0644); err != nil { + t.Fatal(err) + } + + sources, err := ds.GetConfiguredSources(account, device) + if err != nil { + t.Fatalf("GetConfiguredSources failed: %v", err) + } + + if len(sources) != 2 { + t.Fatalf("expected 2 sources, got %d", len(sources)) + } + + if sources[0].ID == sources[1].ID { + t.Errorf("duplicate AUX entries must not share an ID, got %q for both", sources[0].ID) + } + + // Both should still have Type repaired to the canonical "Audio". + for i, s := range sources { + if s.Type != "Audio" { + t.Errorf("source %d: expected Type 'Audio', got %q", i, s.Type) + } + } +} + +// TestGetConfiguredSources_PoisonedAuxEntryRepaired covers the case where a previous +// version of the datastore already persisted bad synthesized values (type="AUX", +// id="2000001"). On read, those values must be repaired to the canonical defaults. +func TestGetConfiguredSources_PoisonedAuxEntryRepaired(t *testing.T) { + tempDir, err := os.MkdirTemp("", "st-sources-poisoned-aux-*") + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.RemoveAll(tempDir) }() + + ds := NewDataStore(tempDir) + account := "1234567" + device := "001122334455" + + deviceDir := ds.AccountDeviceDir(account, device) + if err := os.MkdirAll(deviceDir, 0755); err != nil { + t.Fatal(err) + } + + poisonedXML := ` + + + + +` + if err := os.WriteFile(filepath.Join(deviceDir, "Sources.xml"), []byte(poisonedXML), 0644); err != nil { + t.Fatal(err) + } + + sources, err := ds.GetConfiguredSources(account, device) + if err != nil { + t.Fatalf("GetConfiguredSources failed: %v", err) + } + + if len(sources) != 1 { + t.Fatalf("expected 1 source, got %d", len(sources)) + } + + s := sources[0] + if s.Type != "Audio" { + t.Errorf("expected Type to be repaired to 'Audio', got %q", s.Type) + } + // ID repair is intentionally not aggressive — only empty IDs are filled + // from canonical defaults to avoid breaking references in recents/presets. + if s.ID != "2000001" { + t.Errorf("expected ID preserved as 2000001, got %q", s.ID) + } +} diff --git a/pkg/service/marge/marge.go b/pkg/service/marge/marge.go index 7663ea1..0271faf 100644 --- a/pkg/service/marge/marge.go +++ b/pkg/service/marge/marge.go @@ -105,7 +105,10 @@ func ensureTimestamps(s *models.ConfiguredSource) { } func ensureSourceType(s *models.ConfiguredSource) { - if s.Type == "" || (s.SourceKey.Type != "" && s.SourceKey.Type != constants.ProviderAux && s.SourceKey.Type != constants.ProviderBluetooth) { + // AUX must be normalized to Type="Audio" — the speaker rejects type="AUX" + // (which the datastore previously synthesized from SourceKey.Type). + // Bluetooth is left alone since its canonical Type isn't "Audio". + if s.Type == "" || (s.SourceKey.Type != "" && s.SourceKey.Type != constants.ProviderBluetooth) { if s.SourceKey.Type == constants.ProviderAmazon { s.Type = constants.ProviderAmazon } else {