From 6ad75657e85f04ce7c016ac90a84ffd319a8928c Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 14 Jun 2026 19:59:50 +0200 Subject: [PATCH] fix(datastore): dedup recents by ID in SaveRecents (stop same-recent pile-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A speaker<->marge recents sync could re-store the same recent (same ID) multiple times — observed live as one STORED_MUSIC track appearing 4x in the speaker's /recents, the service's stored Recents.xml, and /full. The duplicates crowd the capped (10) recents list and evict other sources (e.g. a freshly played Spotify track never appears). SaveConfiguredSources already dedups by ID; SaveRecents did not, so dupes introduced by any path (AddRecent move-to-front, syncRecents from the speaker's /full, setup/health) persisted and fed back through the sync loop. SaveRecents now dedups by ID (first occurrence wins) at the single chokepoint all callers share, so the list self-heals on the next write. Regression test added. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/service/datastore/datastore.go | 21 +++++++ pkg/service/datastore/recents_dedup_test.go | 67 +++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 pkg/service/datastore/recents_dedup_test.go diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index c7b0103..ba471aa 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -1436,6 +1436,27 @@ func (ds *DataStore) SaveRecents(account, device string, recents []models.Servic Recents []RecentXML `xml:"recent"` } + // Deduplicate by ID before saving; first occurrence wins. A speaker<->marge + // recents sync can otherwise re-store the same recent (same ID) multiple + // times — it then crowds the capped list and evicts other sources from the + // speaker's recents. Mirrors SaveConfiguredSources. + seen := make(map[string]bool) + deduped := make([]models.ServiceRecent, 0, len(recents)) + + for i := range recents { + if id := recents[i].ID; id != "" { + if seen[id] { + continue + } + + seen[id] = true + } + + deduped = append(deduped, recents[i]) + } + + recents = deduped + wrap := RecentsXML{ Recents: make([]RecentXML, 0, len(recents)), } diff --git a/pkg/service/datastore/recents_dedup_test.go b/pkg/service/datastore/recents_dedup_test.go new file mode 100644 index 0000000..5972870 --- /dev/null +++ b/pkg/service/datastore/recents_dedup_test.go @@ -0,0 +1,67 @@ +package datastore + +import ( + "os" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +// TestSaveRecents_DeduplicatesByID is a regression test for the recents +// duplication bug: a speaker<->marge sync could re-store the same recent (same +// ID) multiple times, crowding the capped list and evicting other sources from +// the speaker's recents. SaveRecents must dedup by ID (first occurrence wins). +func TestSaveRecents_DeduplicatesByID(t *testing.T) { + tmp, err := os.MkdirTemp("", "recents-dedup-*") + if err != nil { + t.Fatalf("temp dir: %v", err) + } + + defer func() { _ = os.RemoveAll(tmp) }() + + ds := NewDataStore(tmp) + account, device := "6919733", "A81B6A536A98" + + mk := func(id, name string) models.ServiceRecent { + var r models.ServiceRecent + r.ID = id + r.Name = name + r.Source = "7" + r.SourceAccount = "4d696e69-444c-164e-9d41-72ecda78e4c1/0" + r.Location = "1$4$2 TRACK" + + return r + } + + // Same ID four times (the observed live state), plus two distinct recents. + in := []models.ServiceRecent{ + mk("260614006", "03 - Salvation"), + mk("260614006", "03 - Salvation"), + mk("260614006", "03 - Salvation"), + mk("260614006", "03 - Salvation"), + mk("260614004", "06 - Back Burner"), + mk("260613001", "Artifact"), + } + + if err := ds.SaveRecents(account, device, in); err != nil { + t.Fatalf("SaveRecents: %v", err) + } + + out, err := ds.GetRecents(account, device) + if err != nil { + t.Fatalf("GetRecents: %v", err) + } + + counts := map[string]int{} + for _, r := range out { + counts[r.ID]++ + } + + if counts["260614006"] != 1 { + t.Errorf("duplicate recent not deduped: id 260614006 appears %d times (want 1)", counts["260614006"]) + } + + if len(out) != 3 { + t.Errorf("expected 3 distinct recents, got %d: %+v", len(out), counts) + } +}