From 0e2f05e6e56afcb3070813480b296509c105c062 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Fri, 17 Apr 2026 18:50:51 +0200 Subject: [PATCH] Improve source sync by adding deduction of known source IDs (#167) --- cmd/soundtouch-cli/common_test.go | 36 ++-- pkg/service/datastore/datastore.go | 127 ++++++++++++- .../datastore/sources_deduction_test.go | 168 ++++++++++++++++++ .../handlers/handlers_account_mgmt_test.go | 31 ++-- pkg/service/marge/sync.go | 13 +- pkg/service/marge/sync_deduction_test.go | 97 ++++++++++ 6 files changed, 430 insertions(+), 42 deletions(-) create mode 100644 pkg/service/datastore/sources_deduction_test.go create mode 100644 pkg/service/marge/sync_deduction_test.go diff --git a/cmd/soundtouch-cli/common_test.go b/cmd/soundtouch-cli/common_test.go index b23577c..7752a80 100644 --- a/cmd/soundtouch-cli/common_test.go +++ b/cmd/soundtouch-cli/common_test.go @@ -37,16 +37,16 @@ func TestFetchTuneInMetadata(t *testing.T) { if metadata == nil { t.Fatal("fetchTuneInMetadata() returned nil metadata") - } + } else { + expectedName := "WDR 2 Rheinland" + if metadata.Name != expectedName { + t.Errorf("metadata.Name = %v, want %v", metadata.Name, expectedName) + } - expectedName := "WDR 2 Rheinland" - if metadata.Name != expectedName { - t.Errorf("metadata.Name = %v, want %v", metadata.Name, expectedName) - } - - expectedArtwork := "https://cdn-radiotime-logos.tunein.com/s213886g.png" - if metadata.Artwork != expectedArtwork { - t.Errorf("metadata.Artwork = %v, want %v", metadata.Artwork, expectedArtwork) + expectedArtwork := "https://cdn-radiotime-logos.tunein.com/s213886g.png" + if metadata.Artwork != expectedArtwork { + t.Errorf("metadata.Artwork = %v, want %v", metadata.Artwork, expectedArtwork) + } } } @@ -192,15 +192,15 @@ func TestFetchSpotifyMetadata(t *testing.T) { if metadata == nil { t.Fatal("fetchSpotifyMetadata() returned nil metadata") - } + } else { + expectedName := "Terminal Caribe - Album by Santi & Tuğçe" + if metadata.Name != expectedName { + t.Errorf("metadata.Name = %v, want %v", metadata.Name, expectedName) + } - expectedName := "Terminal Caribe - Album by Santi & Tuğçe" - if metadata.Name != expectedName { - t.Errorf("metadata.Name = %v, want %v", metadata.Name, expectedName) - } - - expectedArtwork := "https://i.scdn.co/image/ab67616d0000b273f0e55478f4a15182405bcb47" - if metadata.Artwork != expectedArtwork { - t.Errorf("metadata.Artwork = %v, want %v", metadata.Artwork, expectedArtwork) + expectedArtwork := "https://i.scdn.co/image/ab67616d0000b273f0e55478f4a15182405bcb47" + if metadata.Artwork != expectedArtwork { + t.Errorf("metadata.Artwork = %v, want %v", metadata.Artwork, expectedArtwork) + } } } diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index cb38eba..377f708 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -2,6 +2,7 @@ package datastore import ( + "bytes" "encoding/base64" "encoding/json" "encoding/xml" @@ -1076,6 +1077,124 @@ func (ds *DataStore) RemoveDeviceDir(account, device string) error { return ds.RemoveDevice(account, device) } +// DeduceSourceIDs updates the source IDs in the given slice by deducing them from recents and presets. +func (ds *DataStore) DeduceSourceIDs(account, device string, sources []models.ConfiguredSource) { + // Deduce source IDs from recents and presets + deducedIDs := ds.collectDeducedIDs(account, device) + + for i := range sources { + if id, ok := deducedIDs[sources[i].SourceProviderID]; ok { + sources[i].ID = id + } else if sources[i].SourceKeyType == "AUX" { + if id, ok := deducedIDs["9"]; ok { + sources[i].ID = id + sources[i].SourceProviderID = "9" + } + } + } +} + +func (ds *DataStore) collectDeducedIDs(account, device string) map[string]string { + deducedIDs := make(map[string]string) + + // Check recents and presets to find source IDs for provider IDs 2, 9, 11, 25 + for _, filename := range []string{constants.RecentsFile, constants.PresetsFile} { + fileContent, err := os.ReadFile(filepath.Join(ds.AccountDeviceDir(account, device), filename)) + if err != nil { + continue + } + + ds.parseIDsFromFile(fileContent, deducedIDs) + } + + return deducedIDs +} + +func (ds *DataStore) parseIDsFromFile(fileContent []byte, deducedIDs map[string]string) { + decoder := xml.NewDecoder(bytes.NewReader(fileContent)) + for { + token, _ := decoder.Token() + if token == nil { + break + } + + if se, ok := token.(xml.StartElement); ok { + switch se.Name.Local { + case "source": + ds.parseSourceElement(decoder, &se, deducedIDs) + case "recent", "preset": + ds.parseRecentPresetElement(decoder, &se, deducedIDs) + } + } + } +} + +func (ds *DataStore) parseSourceElement(decoder *xml.Decoder, se *xml.StartElement, deducedIDs map[string]string) { + var s struct { + ID string `xml:"id,attr"` + SourceProviderID string `xml:"sourceproviderid"` + // Also check for sourceproviderid as attribute just in case + SourceProviderIDAttr string `xml:"sourceproviderid,attr"` + } + if err := decoder.DecodeElement(&s, se); err == nil { + pid := s.SourceProviderID + if pid == "" { + pid = s.SourceProviderIDAttr + } + + ds.extractIDs(pid, s.ID, deducedIDs) + } +} + +func (ds *DataStore) parseRecentPresetElement(decoder *xml.Decoder, se *xml.StartElement, deducedIDs map[string]string) { + var s struct { + SourceID string `xml:"sourceid"` + SourceProviderID string `xml:"sourceproviderid"` + ContentItem struct { + Source string `xml:"source,attr"` + Type string `xml:"type,attr"` + } `xml:"contentItem"` + Source struct { + SourceProviderID string `xml:"sourceproviderid"` + } `xml:"source"` + } + if err := decoder.DecodeElement(&s, se); err == nil { + pid := s.SourceProviderID + if pid == "" { + pid = s.Source.SourceProviderID + } + + if pid == "" { + // For AUX, we often don't have provider ID 9 but we know its name/source + switch s.ContentItem.Source { + case "AUX": + pid = "9" + case "INTERNET_RADIO": + pid = "2" + case "LOCAL_INTERNET_RADIO": + pid = "11" + case "TUNEIN": + pid = "25" + } + } + + ds.extractIDs(pid, s.SourceID, deducedIDs) + } +} + +func (ds *DataStore) extractIDs(providerID, sourceID string, deducedIDs map[string]string) { + if sourceID == "" || providerID == "" { + return + } + // Stick to the provider ids mentioned: 2, 9, 11, 25 + switch providerID { + case "2", "9", "11", "25": + if _, exists := deducedIDs[providerID]; !exists { + deducedIDs[providerID] = sourceID + } + } +} + // GetConfiguredSources retrieves all configured sources for the specified account and device. func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.ConfiguredSource, error) { ds.fileMutex.RLock() @@ -1086,7 +1205,10 @@ func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.Conf data, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { - return ds.getDefaultSources(), nil + sources := ds.getDefaultSources() + ds.DeduceSourceIDs(account, device, sources) + + return sources, nil } return nil, err @@ -1139,9 +1261,6 @@ func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.Conf if ps.Credential.Value != "" { s.Secret = ps.Credential.Value s.SecretType = ps.Credential.Type - } else { - s.Secret = ps.Secret - s.SecretType = ps.SecretType } // Ensure Secret/SecretType values are prioritized from legacy fields if still missing diff --git a/pkg/service/datastore/sources_deduction_test.go b/pkg/service/datastore/sources_deduction_test.go new file mode 100644 index 0000000..b295db1 --- /dev/null +++ b/pkg/service/datastore/sources_deduction_test.go @@ -0,0 +1,168 @@ +package datastore + +import ( + "os" + "path/filepath" + "testing" +) + +func TestGetConfiguredSources_DeduceIDs(t *testing.T) { + tempDir, err := os.MkdirTemp("", "datastore-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + ds := NewDataStore(tempDir) + account := "test-account" + device := "test-device" + + // Create recents with specific source IDs for provider IDs + // Let's create a manual Recents.xml and Presets.xml in the temp directory to simulate the state. + deviceDir := ds.AccountDeviceDir(account, device) + if err := os.MkdirAll(deviceDir, 0755); err != nil { + t.Fatalf("Failed to create device dir: %v", err) + } + + recentsXML := ` + + + + 2017-02-07T11:22:00.000+00:00 + 2017-05-17T13:18:57.000+00:00 + 52349 + Lounge FM Digital + + 2015-03-11T19:12:38.000+00:00 + + 9330201 + 2 + + + 2015-03-11T19:12:38.000+00:00 + + + 9330201 + 2017-05-17T17:18:58.000+00:00 + Lounge FM Digital + +` + + if err := os.WriteFile(filepath.Join(deviceDir, "Recents.xml"), []byte(recentsXML), 0644); err != nil { + t.Fatalf("Failed to write Recents.xml: %v", err) + } + + // Now call GetConfiguredSources and expect it to have "9330201" for provider ID "2" + sources, err := ds.GetConfiguredSources(account, device) + if err != nil { + t.Fatalf("GetConfiguredSources failed: %v", err) + } + + foundDeducted := false + for _, s := range sources { + if s.SourceProviderID == "2" { + if s.ID == "9330201" { + foundDeducted = true + } else { + t.Errorf("Expected source ID 9330201 for provider 2, got %s", s.ID) + } + } + } + + if !foundDeducted { + t.Errorf("Did not find source with provider ID 2 and deducted ID 9330201") + } +} + +func TestGetConfiguredSources_DeduceIDs_AllProviders(t *testing.T) { + tempDir, err := os.MkdirTemp("", "datastore-test-all-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + ds := NewDataStore(tempDir) + account := "test-account" + device := "test-device" + + deviceDir := ds.AccountDeviceDir(account, device) + if err := os.MkdirAll(deviceDir, 0755); err != nil { + t.Fatalf("Failed to create device dir: %v", err) + } + + // 2: INTERNET_RADIO + // 9: AUX + // 11: LOCAL_INTERNET_RADIO + // 25: TUNEIN + presetsXML := ` + + + + http://example.com/art2.png + + + ID2 + + + + http://example.com/art9.png + + + ID9 + + + + http://example.com/art11.png + + + ID11 + + + + http://example.com/art25.png + + + ID25 + +` + + if err := os.WriteFile(filepath.Join(deviceDir, "Presets.xml"), []byte(presetsXML), 0644); err != nil { + t.Fatalf("Failed to write Presets.xml: %v", err) + } + + sources, err := ds.GetConfiguredSources(account, device) + if err != nil { + t.Fatalf("GetConfiguredSources failed: %v", err) + } + + expected := map[string]string{ + "2": "ID2", + "9": "ID9", + "11": "ID11", + "25": "ID25", + } + + found := make(map[string]bool) + for _, s := range sources { + if expID, ok := expected[s.SourceProviderID]; ok { + if s.ID != expID { + t.Errorf("Expected source ID %s for provider %s, got %s", expID, s.SourceProviderID, s.ID) + } + found[s.SourceProviderID] = true + } else if s.SourceKeyType == "AUX" && s.SourceProviderID == "" { + // Special case for AUX if it doesn't have provider ID 9 by default + if expID, ok := expected["9"]; ok { + if s.ID != expID { + t.Errorf("Expected source ID %s for AUX, got %s", expID, s.ID) + } + found["9"] = true + } + } + } + + for pid := range expected { + if !found[pid] { + t.Errorf("Did not find source with provider ID %s", pid) + } + } +} diff --git a/pkg/service/handlers/handlers_account_mgmt_test.go b/pkg/service/handlers/handlers_account_mgmt_test.go index 049f181..4d43485 100644 --- a/pkg/service/handlers/handlers_account_mgmt_test.go +++ b/pkg/service/handlers/handlers_account_mgmt_test.go @@ -394,17 +394,17 @@ func TestHandleMgmtAccountDetails_Sources(t *testing.T) { if gesellixSource == nil { t.Fatal("gesellix source not found") - } - - // It should have fallen back to Account name "gesellix" because DisplayName was generic "Audio" - if gesellixSource.DisplayName != "gesellix" { - t.Errorf("Expected display_name 'gesellix', got '%s'", gesellixSource.DisplayName) - } - if gesellixSource.Name != "gesellix" { - t.Errorf("Expected name 'gesellix', got '%s'", gesellixSource.Name) - } - if gesellixSource.Type != "Audio" { - t.Errorf("Expected type 'Audio', got '%s'", gesellixSource.Type) + } else { + // It should have fallen back to Account name "gesellix" because DisplayName was generic "Audio" + if gesellixSource.DisplayName != "gesellix" { + t.Errorf("Expected display_name 'gesellix', got '%s'", gesellixSource.DisplayName) + } + if gesellixSource.Name != "gesellix" { + t.Errorf("Expected name 'gesellix', got '%s'", gesellixSource.Name) + } + if gesellixSource.Type != "Audio" { + t.Errorf("Expected type 'Audio', got '%s'", gesellixSource.Type) + } } // Find the generic audio source @@ -417,9 +417,10 @@ func TestHandleMgmtAccountDetails_Sources(t *testing.T) { } if audioSource == nil { t.Fatal("audio source not found") - } - // It should still be "Audio" as there is no account fallback - if audioSource.DisplayName != "Audio" { - t.Errorf("Expected display_name 'Audio', got '%s'", audioSource.DisplayName) + } else { + // It should still be "Audio" as there is no account fallback + if audioSource.DisplayName != "Audio" { + t.Errorf("Expected display_name 'Audio', got '%s'", audioSource.DisplayName) + } } } diff --git a/pkg/service/marge/sync.go b/pkg/service/marge/sync.go index 1b2fe2c..77e676c 100644 --- a/pkg/service/marge/sync.go +++ b/pkg/service/marge/sync.go @@ -33,14 +33,14 @@ func SyncFromAccountFull(ds *datastore.DataStore, resp *models.AccountFullRespon // 1. Update Device Info syncDeviceInfo(ds, accountID, dev) - // 2. Update Configured Sources for this device - syncConfiguredSources(ds, accountID, deviceID, resp.Sources, dev) - - // 3. Update Presets + // 2. Update Presets syncPresets(ds, accountID, deviceID, dev.Presets) - // 4. Update Recents + // 3. Update Recents syncRecents(ds, accountID, deviceID, dev.Recents) + + // 4. Update Configured Sources for this device (requires presets and recents to be on disk for deduction) + syncConfiguredSources(ds, accountID, deviceID, resp.Sources, dev) } log.Printf("[SYNC] Synchronization completed for account %s", accountID) @@ -166,6 +166,9 @@ func syncConfiguredSources(ds *datastore.DataStore, accountID, deviceID string, } } + // 4. Add deduction based on local presets/recents + ds.DeduceSourceIDs(accountID, deviceID, deviceSources) + if err := ds.SaveConfiguredSources(accountID, deviceID, deviceSources); err != nil { log.Printf("[SYNC_ERR] Failed to save sources for %s: %v", deviceID, err) } diff --git a/pkg/service/marge/sync_deduction_test.go b/pkg/service/marge/sync_deduction_test.go new file mode 100644 index 0000000..c552e5f --- /dev/null +++ b/pkg/service/marge/sync_deduction_test.go @@ -0,0 +1,97 @@ +package marge + +import ( + "encoding/xml" + "os" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/datastore" +) + +func TestSyncFromAccountFull_DeduceIDs(t *testing.T) { + // Setup a temporary datastore + tmpDir, err := os.MkdirTemp("", "sync_deduce_test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + ds := datastore.NewDataStore(tmpDir) + accountID := "USER_123" + deviceID := "DEVICE_ABC" + + // Mock AccountFullResponse with generic source IDs (e.g., from a fresh sync or default mapping) + // and specific source IDs in presets/recents that we want to "deduce" and use. + xmlData := ` + + + + + + + + 2 + + 9330201 + + + + + + + 25 + + DEDUCED_TUNEIN + + + + + + + + +` + + var resp models.AccountFullResponse + if err := xml.Unmarshal([]byte(xmlData), &resp); err != nil { + t.Fatalf("Failed to unmarshal mock data: %v", err) + } + + // Run Sync + if err := SyncFromAccountFull(ds, &resp); err != nil { + t.Fatalf("SyncFromAccountFull failed: %v", err) + } + + // Verify Sources + sources, err := ds.GetConfiguredSources(accountID, deviceID) + if err != nil { + t.Errorf("Failed to get sources: %v", err) + } + + found2 := false + found25 := false + for _, s := range sources { + if s.SourceProviderID == "2" { + if s.ID == "9330201" { + found2 = true + } else { + t.Errorf("Expected source ID 9330201 for provider 2, got %s", s.ID) + } + } + if s.SourceProviderID == "25" { + if s.ID == "DEDUCED_TUNEIN" { + found25 = true + } else { + t.Errorf("Expected source ID DEDUCED_TUNEIN for provider 25, got %s", s.ID) + } + } + } + + if !found2 { + t.Errorf("Did not find source with provider ID 2 and deducted ID 9330201") + } + if !found25 { + t.Errorf("Did not find source with provider ID 25 and deducted ID DEDUCED_TUNEIN") + } +}