refactor(handlers): seed placeholder sources at device discovery, not priming

EnsurePlaceholderSources was previously called from
registerSpotifySourceForDevice (inside PrimeDeviceWithSpotify), which is
gated on a Spotify service being configured AND an account being linked.
That meant a user who only ever uses Spotify Connect (push from the
Spotify app, no OAuth-managed account in AfterTouch) never got the
SPOTIFY/SpotifyConnectUserName placeholder, so Connect-initiated presets
still collapsed back to "invalid SourceID".

Move the call to handleDiscoveredDevice (and its fallback) where it
belongs semantically: the placeholder represents a firmware-managed
slot on the speaker, so it should exist for every known device, period —
no music-service config required. Idempotent, so re-discovery is safe.

Remove the duplicate call from registerSpotifySourceForDevice; the
discovery hook is now the single source of truth.

New test in placeholder_discovery_test.go asserts that
handleDiscoveredDevice seeds the placeholder for a fresh device when
NO Spotify service is configured (srv.spotifyService == nil).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-05-16 22:49:28 +02:00
co-authored by Claude Opus 4.7
parent 8a7f8010c9
commit c817b4902f
2 changed files with 118 additions and 8 deletions
@@ -0,0 +1,102 @@
package handlers
import (
"fmt"
"net/http"
"net/http/httptest"
"os"
"testing"
"github.com/gesellix/bose-soundtouch/pkg/models"
"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
"github.com/gesellix/bose-soundtouch/pkg/service/marge"
"github.com/gesellix/bose-soundtouch/pkg/service/setup"
)
// TestHandleDiscoveredDevice_SeedsPlaceholdersWithoutSpotify guards the
// "Spotify-agnostic" property of placeholder seeding: a brand-new device
// being discovered must get its SPOTIFY/SpotifyConnectUserName placeholder
// even when no Spotify service is configured and no OAuth account is linked.
// That's the use case where the operator hasn't set anything up yet but the
// user pushes Spotify Connect from their phone and wants to preset it.
func TestHandleDiscoveredDevice_SeedsPlaceholdersWithoutSpotify(t *testing.T) {
tempDir, err := os.MkdirTemp("", "placeholder-discovery-*")
if err != nil {
t.Fatalf("MkdirTemp: %v", err)
}
defer func() { _ = os.RemoveAll(tempDir) }()
const (
deviceID = "AABBCCDDEEFF"
accountID = "7654321"
)
deviceInfoXML := `<info deviceID="` + deviceID + `">
<name>Bare Speaker</name>
<type>SoundTouch 20</type>
<margeAccountUUID>` + accountID + `</margeAccountUUID>
<components>
<component>
<componentCategory>SCM</componentCategory>
<softwareVersion>27.0.6</softwareVersion>
<serialNumber>SN-PLACEHOLDER-TEST</serialNumber>
</component>
</components>
<networkInfo type="SCM">
<macAddress>` + deviceID + `</macAddress>
<ipAddress>127.0.0.1</ipAddress>
</networkInfo>
</info>`
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/info" {
w.Header().Set("Content-Type", "application/xml")
fmt.Fprint(w, deviceInfoXML)
return
}
http.NotFound(w, r)
}))
defer server.Close()
deviceIP := server.URL[len("http://"):]
ds := datastore.NewDataStore(tempDir)
sm := setup.NewManager(server.URL, ds, nil)
srv := NewServer(ds, sm, "http://localhost", false, false, false)
// Deliberately NOT calling srv.SetSpotifyService — the whole point is
// that placeholder seeding must work without any music-service config.
if srv.spotifyService != nil {
t.Fatalf("test precondition: spotifyService should be nil for this scenario")
}
srv.handleDiscoveredDevice(models.DiscoveredDevice{
Host: deviceIP,
Name: "Bare Speaker",
DiscoveryMethod: "UPnP",
})
sources, err := ds.GetConfiguredSources(accountID, deviceID)
if err != nil {
t.Fatalf("GetConfiguredSources: %v", err)
}
found := false
for i := range sources {
if sources[i].SourceKey.Type == "SPOTIFY" && sources[i].SourceKey.Account == marge.PlaceholderSpotifyConnectAccount {
found = true
break
}
}
if !found {
t.Errorf("expected SPOTIFY/%s placeholder after discovery (without Spotify config), got %d sources",
marge.PlaceholderSpotifyConnectAccount, len(sources))
for i := range sources {
t.Logf(" source[%d]: %s/%s (id=%s)", i, sources[i].SourceKey.Type, sources[i].SourceKey.Account, sources[i].ID)
}
}
}
+16 -8
View File
@@ -719,14 +719,9 @@ func (s *Server) registerSpotifySourceForDevice(deviceIP string, accounts []spot
registered = true
}
// Seed firmware-internal placeholder sources (e.g. SPOTIFY/SpotifyConnectUserName)
// so storePreset payloads originating from Spotify-Connect playback bind to the
// Connect placeholder rather than the OAuth-brokered entry above. Idempotent.
if registered && deviceID != "" {
if err := marge.EnsurePlaceholderSources(s.ds, accountID, deviceID); err != nil {
log.Printf("[Spotify Watchdog] Failed to seed placeholder sources for account %s device %s: %v", accountID, deviceID, err)
}
}
// Note: firmware-internal placeholder sources (SPOTIFY/SpotifyConnectUserName, …)
// are seeded at device-discovery time by handleDiscoveredDevice — they live
// independently of whether any music service is OAuth-linked.
// Tell the speaker its sources list changed so it re-fetches from marge.
// Without this its on-device Sources.xml stays stale until something else
@@ -925,6 +920,14 @@ func (s *Server) handleDiscoveredDevice(d models.DiscoveredDevice) {
}
}
// 9. Seed firmware-internal placeholder sources (e.g. SPOTIFY/SpotifyConnectUserName)
// so storePreset payloads carrying speaker-managed sourceAccounts bind to the
// right placeholder. Spotify-agnostic — runs even when no music service is
// linked yet, since Spotify Connect from a phone doesn't need our OAuth setup.
if err := marge.EnsurePlaceholderSources(s.ds, accountID, deviceID); err != nil {
log.Printf("Failed to seed placeholder sources for %s/%s: %v", accountID, deviceID, err)
}
log.Printf("Successfully saved device %s (%s) with MAC-based deviceID: %s", info.Name, d.Host, deviceID)
}
@@ -978,6 +981,11 @@ func (s *Server) handleDiscoveredDeviceFallback(d models.DiscoveredDevice) {
}
}
// Seed firmware-internal placeholder sources — same as the live-info path.
if err := marge.EnsurePlaceholderSources(s.ds, accountID, deviceID); err != nil {
log.Printf("Failed to seed placeholder sources for %s/%s: %v", accountID, deviceID, err)
}
log.Printf("Successfully saved device %s (%s) with fallback deviceID: %s", info.Name, d.Host, deviceID)
}