From 2014c95ac42aca347b4cce22021b1abdd32de2ca Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 5 Sep 2026 11:02:38 +0200 Subject: [PATCH] fix(marge): delete stored groups on account teardown HandleMargeDeleteAccountGroups (DELETE /streaming/account/{id}/group/, no group ID) had become a pure acknowledgement, leaving DeleteAllGroupsForAccount dead code. This is the exact request real firmware sends on factory reset/teardown; skipping the delete leaves a stale Group_*.xml behind, permanently rejecting the next legitimate Create for those devices via EnsureNoGroupsForDevices with no operator remedy. Co-Authored-By: Claude Sonnet 5 --- pkg/service/handlers/handlers_marge.go | 13 +++++++++---- .../handlers_marge_group_lifecycle_test.go | 17 ++++++++++++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/pkg/service/handlers/handlers_marge.go b/pkg/service/handlers/handlers_marge.go index 15413ab3..535617fd 100644 --- a/pkg/service/handlers/handlers_marge.go +++ b/pkg/service/handlers/handlers_marge.go @@ -965,10 +965,10 @@ func (s *Server) HandleMargeDeleteGroup(w http.ResponseWriter, r *http.Request) _, _ = w.Write([]byte(constants.XMLHeader + `Group deleted successfully`)) } -// HandleMargeDeleteAccountGroups acknowledges legacy speaker teardown -// callbacks that carry no group ID. Such a request cannot identify a group -// generation safely, so it is deliberately non-mutating. Generation-aware -// callers use HandleMargeDeleteGroup instead. +// HandleMargeDeleteAccountGroups handles legacy speaker teardown callbacks +// that carry no group ID (e.g. factory reset). It deletes every stored group +// for the account, mirroring the real firmware expectation that this call +// clears all group state so a later Create isn't blocked by a stale record. func (s *Server) HandleMargeDeleteAccountGroups(w http.ResponseWriter, r *http.Request) { account := chi.URLParam(r, "account") @@ -977,6 +977,11 @@ func (s *Server) HandleMargeDeleteAccountGroups(w http.ResponseWriter, r *http.R return } + if err := s.ds.DeleteAllGroupsForAccount(account); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/vnd.bose.streaming-v1.2+xml") w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(constants.XMLHeader + `Group teardown acknowledged`)) diff --git a/pkg/service/handlers/handlers_marge_group_lifecycle_test.go b/pkg/service/handlers/handlers_marge_group_lifecycle_test.go index 08c9e512..3bbc6e27 100644 --- a/pkg/service/handlers/handlers_marge_group_lifecycle_test.go +++ b/pkg/service/handlers/handlers_marge_group_lifecycle_test.go @@ -2,6 +2,7 @@ package handlers import ( "encoding/xml" + "errors" "fmt" "net/http" "net/http/httptest" @@ -210,24 +211,34 @@ func TestMargeDeviceGroupReturns500ForMalformedOrUnreadableData(t *testing.T) { } } -func TestMargeDeleteAccountGroupsAcknowledgesWithoutDeleting(t *testing.T) { +func TestMargeDeleteAccountGroupsDeletesAllGroupsForAccount(t *testing.T) { ds := datastore.NewDataStore(t.TempDir()) handler := margeLifecycleRouter(ds) const account = "ACCOUNT1" + const otherAccount = "ACCOUNT2" group := margeLifecycleGroup("MASTER", "MASTER", "SLAVE", "Current pair") if _, err := ds.AddGroup(account, &group); err != nil { t.Fatalf("add group: %v", err) } + otherGroup := margeLifecycleGroup("OTHER-MASTER", "OTHER-MASTER", "OTHER-SLAVE", "Other account pair") + if _, err := ds.AddGroup(otherAccount, &otherGroup); err != nil { + t.Fatalf("add group in other account: %v", err) + } + response := margeLifecycleRequest(t, handler, http.MethodDelete, "/streaming/account/"+account+"/group/", "192.0.2.10:1234", "") if response.Code != http.StatusOK { t.Fatalf("DELETE status = %d, want 200; body=%s", response.Code, response.Body.String()) } - if current, err := ds.GetGroupForDevice(account, "MASTER"); err != nil || current.ID != group.ID { - t.Fatalf("generation-less teardown changed stored group: group=%#v err=%v", current, err) + if _, err := ds.GetGroupForDevice(account, "MASTER"); !errors.Is(err, datastore.ErrGroupNotFound) { + t.Fatalf("generation-less teardown did not delete stored group: err=%v", err) + } + + if current, err := ds.GetGroupForDevice(otherAccount, "OTHER-MASTER"); err != nil || current.ID != otherGroup.ID { + t.Fatalf("teardown affected a different account's group: group=%#v err=%v", current, err) } }