mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-09-07 15:07:17 +00:00
fix(player): stop pushing to Marge, make its preflight read advisory
Same reasoning as the -service fix: NewWebApp's default generation- lifecycle wiring (used by the standalone player, and by embedded -service until SetStereoPairGenerationPersistence overrides it) proactively pushed group cleanup/rename to an external Marge backend the player doesn't own. A speaker's own firmware already self-reports that 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. Extracted the wiring into playerStereoPairGenerationPersistence (mirroring cmd/soundtouch-service's own testable helper): cleanup and rename are now no-ops, and preflight's read-only dangling-generation check stays but its failure no longer blocks Create. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
66eedeefd3
commit
cb87d86a0b
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user