diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go
index b7fe8625..eda819d3 100644
--- a/cmd/soundtouch-service/main.go
+++ b/cmd/soundtouch-service/main.go
@@ -1598,7 +1598,12 @@ func embeddedStereoPairGenerationPersistence(
return err
}
- return stereopair.DeleteMargeGroupGeneration(httpClient, ref)
+ // The speaker itself self-reports its own group teardown to
+ // whatever Marge backend it's configured with -- that's the entire
+ // reason HandleMargeDeleteGroup/HandleMargeDeleteAccountGroups
+ // exist, they're only ever called by speakers, never by us.
+ // Nothing for us to push to a backend we don't own.
+ return nil
}
preflight := func(refs []stereopair.GenerationRef) error {
@@ -1621,7 +1626,14 @@ func embeddedStereoPairGenerationPersistence(
}
if len(externalRefs) > 0 {
- return stereopair.EnsureMargeNoGroupGenerations(httpClient, externalRefs)
+ // Best-effort dangling-generation check against a backend we
+ // don't own (real Bose cloud, another AfterTouch/SoundCork
+ // instance, ...): attempt it, but never let it being
+ // unreachable or unauthenticated block a Create the
+ // coordinator's own physical preflight already verified safe.
+ if err := stereopair.EnsureMargeNoGroupGenerations(httpClient, externalRefs); err != nil {
+ log.Printf("Stereo-pair external generation preflight inconclusive, proceeding: %v", err)
+ }
}
return nil
@@ -1637,7 +1649,9 @@ func embeddedStereoPairGenerationPersistence(
return err
}
- return stereopair.RenameMargeGroupGeneration(httpClient, ref, name)
+ // See cleanup's comment above: the speaker self-reports its own
+ // rename to whatever Marge backend it's configured with.
+ return nil
}
return cleanup, preflight, rename
diff --git a/cmd/soundtouch-service/stereo_pair_persistence_test.go b/cmd/soundtouch-service/stereo_pair_persistence_test.go
index 2650a53a..2099515a 100644
--- a/cmd/soundtouch-service/stereo_pair_persistence_test.go
+++ b/cmd/soundtouch-service/stereo_pair_persistence_test.go
@@ -1,9 +1,7 @@
package main
import (
- "encoding/xml"
"errors"
- "fmt"
"net/http"
"net/http/httptest"
"strings"
@@ -19,9 +17,18 @@ import (
// TestEmbeddedStereoPairPersistenceTreatsDNSHijackedBoseHostAsLocal).
func neverDNSHijacked(string) bool { return false }
-type rejectingRoundTripper struct{}
+// rejectingRoundTripper errors on every request and counts how many it saw.
+// A preflight failure is now logged and swallowed rather than propagated
+// (see TestEmbeddedStereoPairPersistenceSkipsExternalWritesButAttemptsRead),
+// so tests that need to prove an external dispatch actually happened check
+// calls rather than the returned error.
+type rejectingRoundTripper struct {
+ calls int
+}
+
+func (r *rejectingRoundTripper) RoundTrip(*http.Request) (*http.Response, error) {
+ r.calls++
-func (rejectingRoundTripper) RoundTrip(*http.Request) (*http.Response, error) {
return nil, errors.New("unexpected HTTP persistence request")
}
@@ -50,7 +57,7 @@ func TestEmbeddedStereoPairPersistenceUsesLocalDatastoreAcrossAccounts(t *testin
ds,
func() []string { return []string{localURL} },
neverDNSHijacked,
- &http.Client{Transport: rejectingRoundTripper{}},
+ &http.Client{Transport: &rejectingRoundTripper{}},
)
err = preflight([]stereopair.GenerationRef{{
@@ -93,7 +100,7 @@ func TestEmbeddedStereoPairCleanupMapsAmbiguousGenerationToConflict(t *testing.T
ds,
func() []string { return []string{localURL} },
neverDNSHijacked,
- &http.Client{Transport: rejectingRoundTripper{}},
+ &http.Client{Transport: &rejectingRoundTripper{}},
)
wrongTopology := persistenceTestGroup(groupID)
wrongTopology.Roles.Roles[1].DeviceID = "SUBSTITUTE-RIGHT-ID"
@@ -107,64 +114,54 @@ func TestEmbeddedStereoPairCleanupMapsAmbiguousGenerationToConflict(t *testing.T
}
}
-func TestEmbeddedStereoPairPersistenceUsesExternalMargeBackend(t *testing.T) {
- active := true
- deleteCalls := 0
- postCalls := 0
- expected := persistenceTestGroup("7654321")
+// TestEmbeddedStereoPairPersistenceSkipsExternalWritesButAttemptsRead covers
+// an external (non-local) MargeURL: cleanup and rename must never push to a
+// backend we don't own -- the speaker itself self-reports its own group
+// teardown/rename to whatever Marge backend it's configured with, which is
+// the entire reason HandleMargeDeleteGroup/HandleMargeModifyGroup exist
+// (they're 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, since it's a best-effort check on top of the coordinator's own
+// physical preflight, not the primary guard.
+func TestEmbeddedStereoPairPersistenceSkipsExternalWritesButAttemptsRead(t *testing.T) {
+ getCalls := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- switch {
- case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/device/LEFT-ID/group"):
- if !active {
- _, _ = w.Write([]byte(``))
- return
- }
- _, _ = fmt.Fprintf(w, `%s%sLEFT-IDLEFT192.0.2.10RIGHT-IDRIGHT192.0.2.11`, expected.ID, expected.Name, expected.MasterDeviceID)
- case r.Method == http.MethodDelete && strings.HasSuffix(r.URL.Path, "/group/"+expected.ID):
- deleteCalls++
- active = false
- w.WriteHeader(http.StatusOK)
- case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/group/"+expected.ID):
- postCalls++
- var update models.Group
- if err := xml.NewDecoder(r.Body).Decode(&update); err != nil {
- http.Error(w, err.Error(), http.StatusBadRequest)
- return
- }
- expected = &update
- w.WriteHeader(http.StatusOK)
- default:
- http.NotFound(w, r)
+ 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 we don't own", r.Method, r.URL.Path)
}))
defer server.Close()
- cleanup, _, rename := embeddedStereoPairGenerationPersistence(
+ cleanup, preflight, rename := embeddedStereoPairGenerationPersistence(
datastore.NewDataStore(t.TempDir()),
func() []string { return []string{"http://aftertouch.invalid:18000"} },
neverDNSHijacked,
server.Client(),
)
- if err := rename(stereopair.GenerationRef{
+
+ ref := stereopair.GenerationRef{
DeviceID: "LEFT-ID", AccountID: "ACCOUNT1", MargeURL: server.URL + "/marge",
- GroupID: expected.ID, ExpectedGroup: persistenceTestGroup(expected.ID),
- }, "Renamed living room"); err != nil {
- t.Fatalf("rename: %v", err)
- }
- if postCalls != 1 || expected.Name != "Renamed living room" {
- t.Fatalf("external POST calls = %d, name = %q; want 1, Renamed living room", postCalls, expected.Name)
+ GroupID: "7654321", ExpectedGroup: persistenceTestGroup("7654321"),
}
- if err := cleanup(stereopair.GenerationRef{
- DeviceID: "LEFT-ID", AccountID: "ACCOUNT1", MargeURL: server.URL + "/marge",
- GroupID: expected.ID, ExpectedGroup: expected,
- }); err != nil {
- t.Fatalf("cleanup: %v", err)
+ if err := rename(ref, "Renamed living room"); err != nil {
+ t.Fatalf("rename = %v, want nil (speaker self-reports its own rename)", err)
}
-
- if deleteCalls != 1 || active {
- t.Fatalf("external DELETE calls = %d, active = %v; want 1, false", deleteCalls, active)
+ 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)
}
}
@@ -178,6 +175,7 @@ func TestEmbeddedStereoPairPersistenceReadsOneCurrentURLSnapshot(t *testing.T) {
currentURL := "http://old.invalid:18000"
providerCalls := 0
+ transport := &rejectingRoundTripper{}
_, preflight, _ := embeddedStereoPairGenerationPersistence(
ds,
func() []string {
@@ -185,7 +183,7 @@ func TestEmbeddedStereoPairPersistenceReadsOneCurrentURLSnapshot(t *testing.T) {
return []string{currentURL}
},
neverDNSHijacked,
- &http.Client{Transport: rejectingRoundTripper{}},
+ &http.Client{Transport: transport},
)
currentURL = "http://new.invalid:18000"
@@ -200,11 +198,18 @@ func TestEmbeddedStereoPairPersistenceReadsOneCurrentURLSnapshot(t *testing.T) {
t.Fatalf("URL provider calls = %d, want one coherent snapshot", providerCalls)
}
+ // The old URL no longer matches localMargeURLs()'s current snapshot, so
+ // this ref is external. Preflight still attempts the read (proven by the
+ // transport call count) but no longer propagates its failure -- an
+ // external check failing must not block Create.
err = preflight([]stereopair.GenerationRef{{
DeviceID: "LEFT-ID", AccountID: "OLD-ACCOUNT", MargeURL: "http://old.invalid:18000",
}})
- if err == nil || !strings.Contains(err.Error(), "unexpected HTTP persistence request") {
- t.Fatalf("old URL preflight error = %v, want external HTTP dispatch", err)
+ if err != nil {
+ t.Fatalf("old URL preflight error = %v, want nil (external check failure must not block)", err)
+ }
+ if transport.calls != 1 {
+ t.Fatalf("external HTTP dispatch calls = %d, want exactly 1", transport.calls)
}
}
@@ -228,7 +233,7 @@ func TestEmbeddedStereoPairPersistenceTreatsDNSHijackedBoseHostAsLocal(t *testin
ds,
func() []string { return []string{"https://aftertouch.invalid:18443"} },
func(margeURL string) bool { return strings.Contains(margeURL, "streaming.bose.com") },
- &http.Client{Transport: rejectingRoundTripper{}},
+ &http.Client{Transport: &rejectingRoundTripper{}},
)
err = preflight([]stereopair.GenerationRef{{