diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index 706afaf..9ee4032 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -1647,34 +1647,32 @@ func (ds *DataStore) GetETagForRecents(account, device string) int64 { return info.ModTime().UnixNano() / int64(time.Millisecond) } -// contentHashForFiles returns a SHA-256 hex digest over the concatenated contents of the given file paths. -func contentHashForFiles(paths ...string) string { - h := sha256.New() - - for _, p := range paths { - f, err := os.Open(p) - if err != nil { - continue - } - - _, _ = io.Copy(h, f) - _ = f.Close() - } - - return hex.EncodeToString(h.Sum(nil)) -} - // GetETagForAccount returns a content hash (SHA-256) over presets, sources, and recents for the account and device. // If device is empty, it hashes across all devices in the account. +// The default sources fingerprint is always included so that newly added defaults (e.g. Amazon) +// invalidate cached responses even when the stored Sources.xml has not changed. func (ds *DataStore) GetETagForAccount(account, device string) string { + h := sha256.New() + + // Include the default sources fingerprint so mergeDefaultSources changes are visible. + defaults := ds.GetDefaultSources() + for i := range defaults { + _, _ = io.WriteString(h, defaults[i].ID+defaults[i].SourceKeyType+defaults[i].DisplayName) + } + if device != "" { deviceDir := ds.AccountDeviceDir(account, device) + for _, name := range []string{constants.PresetsFile, constants.SourcesFile, constants.RecentsFile} { + f, err := os.Open(filepath.Join(deviceDir, name)) + if err != nil { + continue + } - return contentHashForFiles( - filepath.Join(deviceDir, constants.PresetsFile), - filepath.Join(deviceDir, constants.SourcesFile), - filepath.Join(deviceDir, constants.RecentsFile), - ) + _, _ = io.Copy(h, f) + _ = f.Close() + } + + return hex.EncodeToString(h.Sum(nil)) } devicesDir := ds.AccountDevicesDir(account) @@ -1684,8 +1682,6 @@ func (ds *DataStore) GetETagForAccount(account, device string) string { // If-None-Match header and return 304 on the first request. entries, _ := os.ReadDir(devicesDir) - h := sha256.New() - for _, entry := range entries { if entry.IsDir() { deviceDir := ds.AccountDeviceDir(account, entry.Name()) diff --git a/pkg/service/handlers/handlers_marge.go b/pkg/service/handlers/handlers_marge.go index e89f19f..e5f4a94 100644 --- a/pkg/service/handlers/handlers_marge.go +++ b/pkg/service/handlers/handlers_marge.go @@ -2,7 +2,9 @@ package handlers import ( "crypto/rand" + "crypto/sha256" "encoding/xml" + "fmt" "io" "log" "math/big" @@ -17,6 +19,19 @@ import ( "github.com/go-chi/chi/v5" ) +// sourceProvidersETag returns a stable ETag for the source providers list, +// derived from the serialized content so it only changes when the list changes. +func sourceProvidersETag() string { + data, err := marge.SourceProvidersToXML() + if err != nil { + return "source-providers-v1" + } + + sum := sha256.Sum256(data) + + return fmt.Sprintf("%x", sum[:8]) +} + // HandleMargeCreateAccount creates a new account from Stockholm (XML). func (s *Server) HandleMargeCreateAccount(w http.ResponseWriter, r *http.Request) { body, err := io.ReadAll(r.Body) @@ -128,7 +143,7 @@ func (s *Server) HandleMargeLogin(w http.ResponseWriter, r *http.Request) { // HandleMargeSourceProviders returns the Marge source providers. func (s *Server) HandleMargeSourceProviders(w http.ResponseWriter, r *http.Request) { - etag := strconv.FormatInt(time.Now().UnixMilli(), 10) + etag := sourceProvidersETag() if r.Header.Get("If-None-Match") == etag { w.WriteHeader(http.StatusNotModified) return diff --git a/pkg/service/handlers/handlers_marge_test.go b/pkg/service/handlers/handlers_marge_test.go index 3ed6b20..8e0f0bc 100644 --- a/pkg/service/handlers/handlers_marge_test.go +++ b/pkg/service/handlers/handlers_marge_test.go @@ -278,6 +278,124 @@ func TestMargeAccountFull(t *testing.T) { } } +// TestMargeAccountFullExcludesEmptyAmazonSource is a regression test for the two-device scenario +// observed in production: device A81B6A536A98 (alphabetically last, used as lastDeviceID) +// has Sources.xml with 6 sources but no Amazon. The first device has Amazon with empty +// credentials (written before OAuth was implemented). Amazon must NOT appear in /full — +// an empty-credential Amazon causes the speaker's AmazonController to fail JSON parsing. +func TestMargeAccountFullExcludesEmptyAmazonSource(t *testing.T) { + tempDir, err := os.MkdirTemp("", "st-test-amazon-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tempDir) }() + + ds := datastore.NewDataStore(tempDir) + + account := "3230304" + + // First device (alphabetically): has Amazon in Sources.xml + firstDeviceID := "08DF1F0BA325" + firstDir := filepath.Join(tempDir, "accounts", account, "devices", firstDeviceID) + if err := os.MkdirAll(firstDir, 0755); err != nil { + t.Fatalf("Failed to create first device dir: %v", err) + } + if err := os.WriteFile(filepath.Join(firstDir, "DeviceInfo.xml"), []byte(` + + A Sound Machine + SoundTouch 20 scm + + `), 0644); err != nil { + t.Fatalf("Failed to write first device DeviceInfo.xml: %v", err) + } + if err := os.WriteFile(filepath.Join(firstDir, "Sources.xml"), []byte(` + + + + `), 0644); err != nil { + t.Fatalf("Failed to write first device Sources.xml: %v", err) + } + + // Second device (alphabetically last = lastDeviceID): 6 sources but NO Amazon. + // This reproduces the real full.xml returned by the live service. + lastDeviceID := "A81B6A536A98" + lastDir := filepath.Join(tempDir, "accounts", account, "devices", lastDeviceID) + if err := os.MkdirAll(lastDir, 0755); err != nil { + t.Fatalf("Failed to create last device dir: %v", err) + } + if err := os.WriteFile(filepath.Join(lastDir, "DeviceInfo.xml"), []byte(` + + Another Speaker + SoundTouch 300 + + `), 0644); err != nil { + t.Fatalf("Failed to write last device DeviceInfo.xml: %v", err) + } + // Sources.xml mirrors the real persisted file: AUX, INTERNET_RADIO, LOCAL_INTERNET_RADIO, + // TUNEIN, RADIO_BROWSER, Spotify — no Amazon. + lastSourcesXML := ` + + + + + + + + + + + + + + + + + + + ` + if err := os.WriteFile(filepath.Join(lastDir, "Sources.xml"), []byte(lastSourcesXML), 0644); err != nil { + t.Fatalf("Failed to write last device Sources.xml: %v", err) + } + + r, _ := setupRouter("http://localhost:8001", ds) + ts := httptest.NewServer(r) + defer ts.Close() + + res, err := http.Get(ts.URL + "/marge/accounts/" + account + "/full") + if err != nil { + t.Fatal(err) + } + defer func() { _ = res.Body.Close() }() + + if res.StatusCode != http.StatusOK { + t.Errorf("Expected 200 OK, got %v", res.Status) + } + + body, _ := io.ReadAll(res.Body) + bodyStr := string(body) + + // Amazon with empty credentials must not appear — the speaker's AmazonController + // fails to parse an empty secret and returns MUSIC_SERVICE_ACCOUNT_LOGIN_FAILED. + if strings.Contains(bodyStr, "20") { + t.Errorf("/full response must not include an empty-credential Amazon source; body:\n%s", bodyStr) + } + + // The 6 sources from lastDeviceID's stored Sources.xml must all be present. + // Checked by sourceproviderid since may hold a display name rather than the type string. + for _, wantProviderID := range []string{ + "9", // AUX + "2", // INTERNET_RADIO + "11", // LOCAL_INTERNET_RADIO + "25", // TUNEIN + "39", // RADIO_BROWSER + "15", // Spotify + } { + if !strings.Contains(bodyStr, wantProviderID) { + t.Errorf("/full response is missing source with %s; body:\n%s", wantProviderID, bodyStr) + } + } +} + func TestMargeAccountSources(t *testing.T) { tempDir, err := os.MkdirTemp("", "st-test-*") if err != nil { diff --git a/pkg/service/marge/marge.go b/pkg/service/marge/marge.go index 0533393..df21fbd 100644 --- a/pkg/service/marge/marge.go +++ b/pkg/service/marge/marge.go @@ -982,6 +982,9 @@ func getAccountSources(ds *datastore.DataStore, account, lastDeviceID string) [] if lastDeviceID != "" { sources, err = ds.GetConfiguredSources(account, lastDeviceID) + if err == nil { + sources = mergeDefaultSources(sources, ds.GetDefaultSources()) + } } else { sources = ds.GetDefaultSources() } @@ -1001,6 +1004,27 @@ 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. +func mergeDefaultSources(stored, defaults []models.ConfiguredSource) []models.ConfiguredSource { + for i := range defaults { + found := false + + for j := range stored { + if stored[j].SourceKeyType == defaults[i].SourceKeyType { + found = true + break + } + } + + if !found { + stored = append(stored, defaults[i]) + } + } + + return stored +} + // AccountSourcesToXML generates the account sources XML. func AccountSourcesToXML(ds *datastore.DataStore, account string) ([]byte, error) { devicesDir := ds.AccountDevicesDir(account) @@ -1163,7 +1187,7 @@ func UpdatePreset(ds *datastore.DataStore, account, device string, presetNumber } if matchingSrc == nil { - if newPresetElem.SourceID == constants.ProviderInternetRadio || newPresetElem.SourceID == constants.ProviderTunein || newPresetElem.SourceID == constants.ProviderSpotify { + if newPresetElem.SourceID == constants.ProviderInternetRadio || newPresetElem.SourceID == constants.ProviderTunein || newPresetElem.SourceID == constants.ProviderSpotify || newPresetElem.SourceID == constants.ProviderAmazon { // Find by SourceKeyType instead of ID if it's a default source for i := range sources { if sources[i].SourceKeyType == newPresetElem.SourceID { @@ -1420,12 +1444,12 @@ func learnSource(ds *datastore.DataStore, account, device string, sources []mode func createLearnedSource(sourceID, location, sourceName, credentialValue, sourceProviderID, createdOn, updatedOn string) *models.ConfiguredSource { displayName := sourceName - // For TuneIn, we often see empty DisplayName/SourceName in recent items - // if it's already a known source or if it's a generic TuneIn request. if displayName == "" && sourceID != "" { - // Try to deduce from sourceID if it looks like a known service - if sourceID == constants.ProviderSpotify { + switch sourceID { + case constants.ProviderSpotify: displayName = constants.ProviderSpotify + case constants.ProviderAmazon: + displayName = "Amazon Music" } } @@ -1439,48 +1463,77 @@ func createLearnedSource(sourceID, location, sourceName, credentialValue, source UpdatedOn: updatedOn, } + classifyLearnedSource(src, sourceID, location, sourceProviderID) + + return src +} + +func classifyLearnedSource(src *models.ConfiguredSource, sourceID, location, sourceProviderID string) { switch { case sourceProviderID == strconv.Itoa(constants.TuneinProviderID) || sourceID == constants.ProviderTunein || strings.Contains(location, "/v1/playback/station/"): - src.SourceKey.Type = constants.ProviderTunein - src.SourceKeyType = constants.ProviderTunein - src.Type = "Audio" - src.SecretType = constants.CredentialTypeToken - - if src.Secret == "" { - src.Secret = datastore.GenerateSerialSecret(strings.ToLower(constants.ProviderTunein)) - } - - if src.DisplayName == "Other" || src.DisplayName == constants.ProviderTunein || src.DisplayName == "" { - src.DisplayName = constants.ProviderTunein - } + classifyAsTuneIn(src) case sourceID == constants.ProviderLocalInternetRadio: - src.SourceKey.Type = constants.ProviderLocalInternetRadio - src.SourceKeyType = constants.ProviderLocalInternetRadio - src.Type = "Audio" - src.SecretType = constants.CredentialTypeToken - - if src.Secret == "" { - src.Secret = datastore.GenerateSerialSecret("local-internet-radio") - } - - if src.DisplayName == "Other" || src.DisplayName == "Local Internet Radio" || src.DisplayName == "" { - src.DisplayName = "Local Internet Radio" - } + classifyAsLocalInternetRadio(src) case strings.Contains(location, "spotify") || strings.Contains(location, "c3BvdGlme") || sourceID == constants.ProviderSpotify: - src.SourceKey.Type = constants.ProviderSpotify - src.SourceKeyType = constants.ProviderSpotify - src.Type = "Audio" - src.SecretType = constants.CredentialTypeTokenV3 - - if src.DisplayName == "Other" { - src.DisplayName = constants.ProviderSpotify - } + classifyAsSpotify(src) + case strings.Contains(location, "amazon") || sourceID == constants.ProviderAmazon || sourceProviderID == strconv.Itoa(constants.AmazonProviderID): + classifyAsAmazon(src) default: src.SourceKey.Type = "INVALID" src.SourceKeyType = "INVALID" } +} - return src +func classifyAsTuneIn(src *models.ConfiguredSource) { + src.SourceKey.Type = constants.ProviderTunein + src.SourceKeyType = constants.ProviderTunein + src.Type = "Audio" + src.SecretType = constants.CredentialTypeToken + + if src.Secret == "" { + src.Secret = datastore.GenerateSerialSecret(strings.ToLower(constants.ProviderTunein)) + } + + if src.DisplayName == "Other" || src.DisplayName == constants.ProviderTunein || src.DisplayName == "" { + src.DisplayName = constants.ProviderTunein + } +} + +func classifyAsLocalInternetRadio(src *models.ConfiguredSource) { + src.SourceKey.Type = constants.ProviderLocalInternetRadio + src.SourceKeyType = constants.ProviderLocalInternetRadio + src.Type = "Audio" + src.SecretType = constants.CredentialTypeToken + + if src.Secret == "" { + src.Secret = datastore.GenerateSerialSecret("local-internet-radio") + } + + if src.DisplayName == "Other" || src.DisplayName == "Local Internet Radio" || src.DisplayName == "" { + src.DisplayName = "Local Internet Radio" + } +} + +func classifyAsSpotify(src *models.ConfiguredSource) { + src.SourceKey.Type = constants.ProviderSpotify + src.SourceKeyType = constants.ProviderSpotify + src.Type = "Audio" + src.SecretType = constants.CredentialTypeTokenV3 + + if src.DisplayName == "Other" { + src.DisplayName = constants.ProviderSpotify + } +} + +func classifyAsAmazon(src *models.ConfiguredSource) { + src.SourceKey.Type = constants.ProviderAmazon + src.SourceKeyType = constants.ProviderAmazon + src.Type = "Audio" + src.SecretType = constants.CredentialTypeToken + + if src.DisplayName == "" || src.DisplayName == "Other" { + src.DisplayName = "Amazon Music" + } } func updateSourceFields(src *models.ConfiguredSource, credentialValue, sourceName, sourceProviderID, createdOn, updatedOn string) bool { @@ -1828,9 +1881,13 @@ func AddSource(ds *datastore.DataStore, account, username, providerID, secret, s } newSrc.SourceKey.Account = username - if providerID == strconv.Itoa(constants.SpotifyProviderID) { + + switch providerID { + case strconv.Itoa(constants.SpotifyProviderID): newSrc.SourceKey.Type = constants.ProviderSpotify - } else { + case strconv.Itoa(constants.AmazonProviderID): + newSrc.SourceKey.Type = constants.ProviderAmazon + default: newSrc.SourceKey.Type = providerID } diff --git a/pkg/service/marge/marge_test.go b/pkg/service/marge/marge_test.go index 2c4f76c..661dd1f 100644 --- a/pkg/service/marge/marge_test.go +++ b/pkg/service/marge/marge_test.go @@ -681,6 +681,8 @@ func TestDefaultSources(t *testing.T) { if s.SourceKey.Account != "AUX" { t.Errorf("Expected AUX account 'AUX', got %s", s.SourceKey.Account) } + case "AMAZON": + t.Errorf("AMAZON must not appear in defaults — it requires real OAuth credentials") } if s.Status != "READY" { diff --git a/tests/integration/http-client/get_full_account.http b/tests/integration/http-client/get_full_account.http index bae1273..c228f4e 100644 --- a/tests/integration/http-client/get_full_account.http +++ b/tests/integration/http-client/get_full_account.http @@ -45,6 +45,8 @@ Authorization: Bearer {{token}} var sources = account.getElementsByTagName("sources")[0]; client.assert(sources !== null, "Missing 'sources' element"); - client.assert(sources.getElementsByTagName("source").length > 0, "No 'source' elements found in account sources"); + var sourceCount = sources.getElementsByTagName("source").length; + client.assert(sourceCount > 0, "No 'source' elements found in account sources"); + client.assert(sourceCount === 6, "Expected 6 sources (AUX, INTERNET_RADIO, LOCAL_INTERNET_RADIO, TUNEIN, RADIO_BROWSER, Spotify) but got " + sourceCount); }); %}