fix(marge): keep all DLNA media servers registered (don't evict on second add)

A speaker registers each DLNA media server as a STORED_MUSIC source whose
account is "<UDN>/0", and reconciles its source list against marge (/full +
/sources). AddSource deduped STORED_MUSIC by provider ID alone, so registering
a second media server overwrote the first in the datastore; the first then
disappeared from /full + /sources and the speaker dropped it. Only one media
server could ever stay registered.

- STORED_MUSIC now replaces only when the account (SourceKey.Account) matches,
  so distinct servers coexist and re-adding the same server updates in place.
  Other (singleton) providers keep replace-by-provider.
- Generate source IDs from crypto/rand instead of a per-second timestamp.
  SaveConfiguredSources dedups by ID, so two sources created in the same instant
  would otherwise collide and one would be silently dropped; a timestamp (even
  nanosecond) is fragile on coarse clocks, so use 64 bits of randomness with a
  timestamp fallback only if the RNG fails.
- Add a regression test for two coexisting media servers + same-account update.

Diagnosed from speaker + service logs: setMusicServiceAccount succeeds locally,
the speaker pushes AddSource to marge (streaming.bose.com, DNS-intercepted to
AfterTouch), then re-fetches /full + /sources; that list returned only the
latest STORED_MUSIC source, so the speaker pruned the previously-added one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-06-14 18:23:29 +02:00
co-authored by Claude Opus 4.8
parent 836e985c58
commit d862666fb7
2 changed files with 131 additions and 3 deletions
@@ -0,0 +1,97 @@
package marge
import (
"os"
"strconv"
"testing"
"github.com/gesellix/bose-soundtouch/pkg/service/constants"
"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
)
// TestAddSource_MultipleStoredMusicServersCoexist is a regression test for the
// media-server eviction bug: AddSource deduped STORED_MUSIC by provider ID, so
// registering a second DLNA media server overwrote the first. The first then
// vanished from /full + /sources and the speaker dropped it, meaning only one
// media server could ever stay registered.
//
// Each media server is a separate account (username = "<UDN>/0"), so two
// distinct servers must coexist, while re-adding the same server (same account)
// updates in place.
func TestAddSource_MultipleStoredMusicServersCoexist(t *testing.T) {
tempDir, err := os.MkdirTemp("", "addsource-storedmusic-*")
if err != nil {
t.Fatalf("temp dir: %v", err)
}
defer func() { _ = os.RemoveAll(tempDir) }()
ds := datastore.NewDataStore(tempDir)
account := "6919733"
device := "A81B6A536A98"
if mkErr := os.MkdirAll(ds.AccountDeviceDir(account, device), 0o755); mkErr != nil {
t.Fatalf("mkdir device dir: %v", mkErr)
}
sm := strconv.Itoa(constants.StoredMusicProviderID)
const (
fritzAcct = "fa095ecc-e13e-40e7-8e6c-e0286d5bc000/0"
testAcct = "4d696e69-444c-164e-9d41-72ecda78e4c1/0"
)
// Register two different media servers.
if _, err := AddSource(ds, account, fritzAcct, sm, "", "", "fritz"); err != nil {
t.Fatalf("add server 1: %v", err)
}
if _, err := AddSource(ds, account, testAcct, sm, "", "", "AfterTouch Test Library"); err != nil {
t.Fatalf("add server 2: %v", err)
}
// storedMusicAccounts returns the set of STORED_MUSIC source accounts the
// datastore would serve via /full + /sources. SourceKey.Account is the
// persisted identity (the display name lives on the speaker, set via
// setMusicServiceAccount, and does not round-trip here).
storedMusicAccounts := func() map[string]bool {
sources, gerr := ds.GetConfiguredSources(account, device)
if gerr != nil {
t.Fatalf("get sources: %v", gerr)
}
out := map[string]bool{}
for _, s := range sources {
if s.SourceProviderID == sm {
out[s.SourceKey.Account] = true
}
}
return out
}
got := storedMusicAccounts()
if len(got) != 2 {
t.Fatalf("expected 2 STORED_MUSIC sources, got %d: %+v", len(got), got)
}
if !got[fritzAcct] {
t.Errorf("first media server was evicted (account %q missing)", fritzAcct)
}
if !got[testAcct] {
t.Errorf("second media server not registered (account %q missing)", testAcct)
}
// Re-adding the SAME server (same account) updates in place; it must not
// create a duplicate or drop the other server.
if _, err := AddSource(ds, account, fritzAcct, sm, "", "", "fritz (renamed)"); err != nil {
t.Fatalf("re-add server 1: %v", err)
}
got = storedMusicAccounts()
if len(got) != 2 || !got[fritzAcct] || !got[testAcct] {
t.Fatalf("re-adding the same server should keep exactly both accounts; got %+v", got)
}
}
+34 -3
View File
@@ -4,6 +4,8 @@ package marge
import (
"bytes"
"crypto/rand"
"encoding/hex"
"encoding/xml"
"fmt"
"log"
@@ -2322,11 +2324,27 @@ func RemoveSourceFromAccount(ds *datastore.DataStore, account, sourceID string)
return nil
}
// newSourceID returns a unique opaque source ID. SaveConfiguredSources dedups
// by ID, so it must be collision-free even for sources created in the same
// instant; it uses crypto/rand (64 bits) and falls back to a nanosecond
// timestamp only if the RNG ever fails.
func newSourceID() string {
var b [8]byte
if _, err := rand.Read(b[:]); err != nil {
return "SRC_" + strconv.FormatInt(time.Now().UnixNano(), 10)
}
return "SRC_" + hex.EncodeToString(b[:])
}
// AddSource adds a new music source to the account and returns the generated source ID.
func AddSource(ds *datastore.DataStore, account, username, providerID, secret, secretType, sourceName string) (string, error) {
now := time.Now()
createdOn := FormatTime(now)
sourceID := "SRC_" + strconv.FormatInt(now.Unix(), 10)
// SaveConfiguredSources dedups by ID, so IDs must be unique even when two
// sources are added in the same instant. Use a random ID rather than a
// timestamp (which can collide on coarse clocks or rapid calls).
sourceID := newSourceID()
// List accounts directly from the account directory to be sure we find them.
devicesDir := ds.AccountDevicesDir(account)
@@ -2368,11 +2386,24 @@ func AddSource(ds *datastore.DataStore, account, username, providerID, secret, s
PrepareConfiguredSource(&newSrc)
// Update or append. If it's the same provider, we replace it.
// Update or append. Most providers are singletons (one account each), so
// the same provider replaces the existing entry. STORED_MUSIC is the
// exception: each DLNA media server is a separate account (username =
// "<UDN>/0"), so it must only replace when the account also matches.
// Otherwise registering a second media server overwrites the first, which
// then vanishes from /full + /sources and the speaker drops it (only one
// media server could ever stay registered).
replaced := false
for i := range sources {
if sources[i].SourceProviderID == providerID ||
sameProvider := sources[i].SourceProviderID == providerID
if providerID == strconv.Itoa(constants.StoredMusicProviderID) {
// Match on the persisted account identity (SourceKey.Account),
// not Username, which does not round-trip through the datastore.
sameProvider = sameProvider && sources[i].SourceKey.Account == username
}
if sameProvider ||
(providerID == strconv.Itoa(constants.SpotifyProviderID) && sources[i].SourceKey.Type == constants.ProviderSpotify) {
sources[i] = newSrc
replaced = true