diff --git a/pkg/service/soundtouchweb/handler.go b/pkg/service/soundtouchweb/handler.go index 9ba89437..e8524d27 100644 --- a/pkg/service/soundtouchweb/handler.go +++ b/pkg/service/soundtouchweb/handler.go @@ -170,22 +170,50 @@ func NewWebApp() *WebApp { CheckOrigin: checkWebSocketOrigin, }, } - app.StereoPairs = stereopair.NewWithGenerationLifecyclePersistence( - app.stereoPairClient, - func(ref stereopair.GenerationRef) error { - return stereopair.DeleteMargeGroupGeneration(app.stereoPairPersistenceClient(), ref) - }, - func(refs []stereopair.GenerationRef) error { - return stereopair.EnsureMargeNoGroupGenerations(app.stereoPairPersistenceClient(), refs) - }, - func(ref stereopair.GenerationRef, name string) error { - return stereopair.RenameMargeGroupGeneration(app.stereoPairPersistenceClient(), ref, name) - }, - ) + cleanup, preflight, rename := playerStereoPairGenerationPersistence(app.stereoPairPersistenceClient) + app.StereoPairs = stereopair.NewWithGenerationLifecyclePersistence(app.stereoPairClient, cleanup, preflight, rename) return app } +// playerStereoPairGenerationPersistence wires generation-lifecycle hooks for +// contexts with no local datastore of their own (the standalone player, and +// the player component embedded in -service before SetStereoPairGenerationPersistence +// overrides it): cleanup and rename are no-ops, and preflight's read-only +// dangling-generation check is advisory. +// +// A speaker self-reports its own group create/rename/teardown to whatever +// Marge backend it's configured with -- that's the entire reason +// HandleMargeAddGroup/HandleMargeModifyGroup/HandleMargeDeleteGroup exist, +// they're only ever called by speakers, never by us. Proactively pushing the +// same update ourselves would duplicate that against a backend we generally +// can't authenticate to anyway (real Bose cloud, another AfterTouch/SoundCork +// instance, ...). The one part with a distinct purpose -- checking for a +// dangling stale generation before a new Create -- is still attempted, but +// its failure must not block Create: it's a best-effort safety net on top of +// the coordinator's own physical preflight, not the primary guard. +func playerStereoPairGenerationPersistence( + persistenceClient func() *http.Client, +) (stereopair.GenerationCleanup, stereopair.GenerationPreflight, stereopair.GenerationRename) { + cleanup := func(stereopair.GenerationRef) error { + return nil + } + + preflight := func(refs []stereopair.GenerationRef) error { + if err := stereopair.EnsureMargeNoGroupGenerations(persistenceClient(), refs); err != nil { + log.Printf("Stereo-pair external generation preflight inconclusive, proceeding: %v", err) + } + + return nil + } + + rename := func(stereopair.GenerationRef, string) error { + return nil + } + + return cleanup, preflight, rename +} + func (app *WebApp) stereoPairPersistenceClient() *http.Client { base := app.serviceHTTPClient() if base.Timeout >= stereopair.RequestTimeout { diff --git a/pkg/service/soundtouchweb/stereo_pair_persistence_test.go b/pkg/service/soundtouchweb/stereo_pair_persistence_test.go new file mode 100644 index 00000000..a23d205d --- /dev/null +++ b/pkg/service/soundtouchweb/stereo_pair_persistence_test.go @@ -0,0 +1,56 @@ +package soundtouchweb + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/stereopair" +) + +// TestPlayerStereoPairGenerationPersistenceSkipsWritesButAttemptsRead covers +// the standalone player's (and embedded player's default) generation +// lifecycle wiring: cleanup and rename must never push to a Marge backend -- +// the speaker itself self-reports its own group teardown/rename to whatever +// backend it's configured with (see HandleMargeDeleteGroup/HandleMargeModifyGroup, +// only ever called by speakers). Preflight still attempts its read-only +// dangling-generation check, but a failure there (network error, wrong +// credentials, real Bose cloud rejecting us, ...) must not block Create. +func TestPlayerStereoPairGenerationPersistenceSkipsWritesButAttemptsRead(t *testing.T) { + getCalls := 0 + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/device/LEFT-ID/group") { + getCalls++ + http.Error(w, "unauthorized", http.StatusUnauthorized) + + return + } + + t.Fatalf("unexpected external %s %s: cleanup/rename must not write to a backend the player doesn't own", r.Method, r.URL.Path) + })) + defer server.Close() + + cleanup, preflight, rename := playerStereoPairGenerationPersistence(func() *http.Client { + return server.Client() + }) + + ref := stereopair.GenerationRef{ + DeviceID: "LEFT-ID", AccountID: "ACCOUNT1", MargeURL: server.URL + "/marge", + GroupID: "7654321", + } + + if err := rename(ref, "Renamed living room"); err != nil { + t.Fatalf("rename = %v, want nil (speaker self-reports its own rename)", err) + } + if err := cleanup(ref); err != nil { + t.Fatalf("cleanup = %v, want nil (speaker self-reports its own teardown)", err) + } + if err := preflight([]stereopair.GenerationRef{ref}); err != nil { + t.Fatalf("preflight = %v, want nil: an unauthenticated external check must not block Create", err) + } + if getCalls != 1 { + t.Fatalf("external GET calls = %d, want exactly 1 (preflight must still attempt the read)", getCalls) + } +}