From a0d1fa7a04c3d352f2fd6119665303be0021e2d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lipinsk=C3=BD?= <6032558+Mr-Tao@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:38:22 +0200 Subject: [PATCH] feat: manage stereo pair lifecycle --- cmd/soundtouch-cli/cmd_group.go | 418 ++--- cmd/soundtouch-cli/cmd_group_test.go | 303 +-- cmd/soundtouch-player/README.md | 3 + cmd/soundtouch-service/main.go | 80 + .../stereo_pair_persistence_test.go | 175 ++ .../testdata/router_routes.txt | 4 + .../appendix/soundtouch-player-roadmap.md | 59 +- docs/content/docs/guides/CLI-REFERENCE.md | 42 + pkg/client/client.go | 49 +- pkg/client/group_test.go | 72 + pkg/service/datastore/datastore.go | 713 +++++++- pkg/service/datastore/group_lifecycle_test.go | 745 ++++++++ pkg/service/handlers/handlers_marge.go | 46 +- .../handlers_marge_group_lifecycle_test.go | 267 +++ pkg/service/soundtouchweb/handler.go | 69 +- .../soundtouchweb/handler_stereopair.go | 404 ++++ .../soundtouchweb/handler_stereopair_test.go | 507 ++++++ pkg/service/soundtouchweb/handler_test.go | 17 + pkg/service/soundtouchweb/mount.go | 7 + pkg/service/soundtouchweb/mount_test.go | 1 + pkg/service/soundtouchweb/static/css/app.css | 40 + pkg/service/soundtouchweb/static/js/api.js | 16 + pkg/service/soundtouchweb/static/js/app.js | 17 +- .../static/js/components/StereoPair.js | 296 +++ pkg/service/soundtouchweb/websocket.go | 148 +- pkg/service/soundtouchweb/websocket_test.go | 67 + pkg/stereopair/coordinator.go | 1617 +++++++++++++++++ pkg/stereopair/coordinator_test.go | 1278 +++++++++++++ pkg/stereopair/marge_cleanup.go | 301 +++ pkg/stereopair/marge_cleanup_test.go | 377 ++++ .../integration/http-client/delete_group.http | 5 +- 31 files changed, 7636 insertions(+), 507 deletions(-) create mode 100644 cmd/soundtouch-service/stereo_pair_persistence_test.go create mode 100644 pkg/service/datastore/group_lifecycle_test.go create mode 100644 pkg/service/handlers/handlers_marge_group_lifecycle_test.go create mode 100644 pkg/service/soundtouchweb/handler_stereopair.go create mode 100644 pkg/service/soundtouchweb/handler_stereopair_test.go create mode 100644 pkg/service/soundtouchweb/static/js/components/StereoPair.js create mode 100644 pkg/stereopair/coordinator.go create mode 100644 pkg/stereopair/coordinator_test.go create mode 100644 pkg/stereopair/marge_cleanup.go create mode 100644 pkg/stereopair/marge_cleanup_test.go diff --git a/cmd/soundtouch-cli/cmd_group.go b/cmd/soundtouch-cli/cmd_group.go index 507ef1e7..51c1973a 100644 --- a/cmd/soundtouch-cli/cmd_group.go +++ b/cmd/soundtouch-cli/cmd_group.go @@ -3,11 +3,10 @@ package main import ( "fmt" "net" - "sync" + "net/http" - "github.com/gesellix/bose-soundtouch/pkg/client" "github.com/gesellix/bose-soundtouch/pkg/models" - "github.com/gesellix/bose-soundtouch/pkg/speaker" + "github.com/gesellix/bose-soundtouch/pkg/stereopair" "github.com/urfave/cli/v2" ) @@ -16,32 +15,26 @@ func getGroupStatus(c *cli.Context) error { clientConfig := GetClientConfig(c) PrintDeviceHeader("Getting group information", clientConfig.Host, clientConfig.Port) - client, err := CreateSoundTouchClient(clientConfig) - if err != nil { - PrintError(fmt.Sprintf("Failed to create client: %v", err)) - return err - } - - group, err := client.GetGroup() + result, err := newGroupCoordinator(clientConfig).Inspect(clientConfig.Host) if err != nil { PrintError(fmt.Sprintf("Failed to get group: %v", err)) + printGroupResultDetails(result) + return err } - if group.IsEmpty() { + if result.Group == nil || result.Group.IsEmpty() { fmt.Println("Device is not in a stereo pair") + return nil } - printGroup(group) + printGroup(result.Group) return nil } -// createGroup forms a stereo pair by POSTing /addGroup to both speakers in -// parallel. LEFT is the master. Addressing each speaker directly (instead of -// only the master and letting it propagate via marge) sidesteps the -// inter-device round-trip that surfaced as client timeouts in #252. +// createGroup forms and verifies a stereo pair. LEFT is always the master. func createGroup(c *cli.Context) error { leftIP := c.String("left") rightIP := c.String("right") @@ -49,283 +42,117 @@ func createGroup(c *cli.Context) error { if net.ParseIP(leftIP) == nil { PrintError(fmt.Sprintf("Invalid left IP address: %s", leftIP)) + return fmt.Errorf("invalid left IP: %s", leftIP) } if net.ParseIP(rightIP) == nil { PrintError(fmt.Sprintf("Invalid right IP address: %s", rightIP)) + return fmt.Errorf("invalid right IP: %s", rightIP) } - PrintDeviceHeader(fmt.Sprintf("Creating stereo pair: LEFT=%s RIGHT=%s", leftIP, rightIP), leftIP, speaker.HTTPPort) + clientConfig := GetClientConfig(c) + PrintDeviceHeader(fmt.Sprintf("Creating stereo pair: LEFT=%s RIGHT=%s", leftIP, rightIP), leftIP, clientConfig.Port) - leftInfo, err := fetchDeviceInfo(c, leftIP) - if err != nil { - PrintError(fmt.Sprintf("Failed to read LEFT device info: %v", err)) - return err - } - - rightInfo, err := fetchDeviceInfo(c, rightIP) - if err != nil { - PrintError(fmt.Sprintf("Failed to read RIGHT device info: %v", err)) - return err - } - - if name == "" { - name = fmt.Sprintf("%s + %s", leftInfo.Name, rightInfo.Name) - } - - req := &models.Group{ + result, err := newGroupCoordinator(clientConfig).Create(stereopair.CreateRequest{ + LeftIPAddress: leftIP, + RightIPAddress: rightIP, Name: name, - MasterDeviceID: leftInfo.DeviceID, - Roles: models.GroupRoles{ - Roles: []models.GroupRole{ - {DeviceID: leftInfo.DeviceID, Role: "LEFT", IPAddress: leftIP}, - {DeviceID: rightInfo.DeviceID, Role: "RIGHT", IPAddress: rightIP}, - }, - }, - // SenderIPAddress is intentionally omitted on the base request. - // propagateAddGroup adds it to the slave's copy only — see comment there. - } - - leftClient, err := clientForHost(c, leftIP) + }) if err != nil { - PrintError(fmt.Sprintf("Failed to create client for LEFT: %v", err)) + PrintError(fmt.Sprintf("Failed to create stereo pair: %v", err)) + printGroupResultDetails(result) + return err } - rightClient, err := clientForHost(c, rightIP) - if err != nil { - PrintError(fmt.Sprintf("Failed to create client for RIGHT: %v", err)) - return err - } - - leftOut, rightOut := propagateAddGroup(leftClient, rightClient, leftIP, rightIP, req) - - if leftOut.err != nil { - PrintError(fmt.Sprintf("LEFT (%s) /addGroup failed: %v", leftIP, leftOut.err)) - } - - if rightOut.err != nil { - PrintError(fmt.Sprintf("RIGHT (%s) /addGroup failed: %v", rightIP, rightOut.err)) - } - - if leftOut.err != nil || rightOut.err != nil { - if (leftOut.err == nil) != (rightOut.err == nil) { - succeeded := leftIP - if leftOut.err != nil { - succeeded = rightIP - } - - PrintError(fmt.Sprintf("Partial group state on %s — clean up with `soundtouch-cli --host %s group remove`", succeeded, succeeded)) - } - - return fmt.Errorf("/addGroup propagation failed") - } - - // The LEFT (master) response carries the assigned group ID; use it for display. - PrintSuccess(fmt.Sprintf("Stereo pair created (id=%s)", leftOut.group.ID)) - printGroup(leftOut.group) + PrintSuccess(fmt.Sprintf("Stereo pair created (id=%s)", result.Group.ID)) + printGroup(result.Group) return nil } -// addGroupOutcome is the per-speaker result of a parallel /addGroup call. -type addGroupOutcome struct { - host string - group *models.Group - err error -} - -// propagateAddGroup POSTs /addGroup to both speakers concurrently and returns -// the (LEFT, RIGHT) outcomes. A non-GROUP_OK Status in the response is -// reported as an error so callers don't have to re-inspect the body. -// -// The two POSTs carry different payloads: the master (LEFT) receives the base -// request with no senderIPAddress so its state machine forms the group as the -// master, while the slave (RIGHT) receives a copy with senderIPAddress set to -// the master's IP so its state machine joins as the slave. Sending the same -// payload to both makes both speakers think they're the slave — they enter -// AddingSlave, wait for a master that never confirms, time out after 5 s, and -// revert (issue #252). -func propagateAddGroup(left, right *client.Client, leftIP, rightIP string, req *models.Group) (addGroupOutcome, addGroupOutcome) { - masterReq := *req - masterReq.SenderIPAddress = "" - - slaveReq := *req - slaveReq.SenderIPAddress = leftIP - - var ( - wg sync.WaitGroup - leftOut, rightOut addGroupOutcome - ) - - wg.Add(2) - - go func() { - defer wg.Done() - - leftOut = postAddGroup(left, leftIP, &masterReq) - }() - - go func() { - defer wg.Done() - - rightOut = postAddGroup(right, rightIP, &slaveReq) - }() - - wg.Wait() - - return leftOut, rightOut -} - -func postAddGroup(cli *client.Client, host string, req *models.Group) addGroupOutcome { - out := addGroupOutcome{host: host} - - g, err := cli.AddGroup(req) - if err != nil { - out.err = err - return out - } - - out.group = g - - if g != nil && g.Status != "" && g.Status != "GROUP_OK" { - out.err = fmt.Errorf("device returned status %q (want GROUP_OK)", g.Status) - } - - return out -} - -// renameGroup updates the name of the existing stereo pair. The device -// requires the full structure on every update, so we fetch the current -// state first. +// renameGroup updates and verifies the name on both stereo-pair members. func renameGroup(c *cli.Context) error { clientConfig := GetClientConfig(c) newName := c.String("name") if newName == "" { PrintError("--name is required") + return fmt.Errorf("name is required") } PrintDeviceHeader(fmt.Sprintf("Renaming stereo pair to %q", newName), clientConfig.Host, clientConfig.Port) - stClient, err := CreateSoundTouchClient(clientConfig) + coordinator := newGroupCoordinator(clientConfig) + + current, err := coordinator.Inspect(clientConfig.Host) if err != nil { - PrintError(fmt.Sprintf("Failed to create client: %v", err)) + PrintError(fmt.Sprintf("Failed to inspect stereo pair before rename: %v", err)) + printGroupResultDetails(current) + return err } - current, err := stClient.GetGroup() - if err != nil { - PrintError(fmt.Sprintf("Failed to read current group: %v", err)) - return err + if current.Group == nil || current.Group.IsEmpty() { + return fmt.Errorf("device is not in a stereo pair") } - if current.IsEmpty() { - PrintError("Device is not in a stereo pair — nothing to rename") - return fmt.Errorf("no group configured") - } - - // Status is read-only on the device side; don't echo it back. - current.Status = "" - current.Name = newName - - result, err := stClient.UpdateGroup(current) + result, err := coordinator.Rename(stereopair.RenameRequest{ + MemberIPAddress: clientConfig.Host, + ExpectedGroupID: current.Group.ID, + Name: newName, + }) if err != nil { PrintError(fmt.Sprintf("Failed to rename group: %v", err)) + printGroupResultDetails(result) + return err } - PrintSuccess(fmt.Sprintf("Stereo pair renamed to %q", result.Name)) - printGroup(result) + PrintSuccess(fmt.Sprintf("Stereo pair renamed to %q", result.Group.Name)) + printGroup(result.Group) return nil } -// removeGroup tears down the device's stereo pair by sending /removeGroup to -// every member in parallel. Sending it only to the master (as the old code -// did) leaves the slave stuck in GroupSlave state indefinitely — mirrors the -// same symmetry as createGroup (see issue #252 comment there). +// removeGroup dissolves and verifies the stereo pair on every member. func removeGroup(c *cli.Context) error { clientConfig := GetClientConfig(c) PrintDeviceHeader("Removing stereo pair", clientConfig.Host, clientConfig.Port) - stClient, err := CreateSoundTouchClient(clientConfig) + coordinator := newGroupCoordinator(clientConfig) + + current, err := coordinator.Inspect(clientConfig.Host) if err != nil { - PrintError(fmt.Sprintf("Failed to create client: %v", err)) - return err + PrintWarning(fmt.Sprintf("Stereo pair is degraded before removal: %v", err)) + printGroupResultDetails(current) + + if current.Group == nil || current.Group.IsEmpty() { + return err + } } - // Fetch current group to learn every member's IP before tearing down. - group, err := stClient.GetGroup() - if err != nil { - PrintError(fmt.Sprintf("Failed to read current group: %v", err)) - return err - } - - if group.IsEmpty() { + if current.Group == nil || current.Group.IsEmpty() { fmt.Println("Device is not in a stereo pair — nothing to remove") + return nil } - // Collect the unique set of member IPs. The master is always reachable - // via clientConfig.Host; the roles carry all members including slaves. - type memberResult struct { - ip string - err error - } + dissolveHost := dissolveRecoveryHost(current, clientConfig.Host) - members := make([]string, 0, len(group.Roles.Roles)) - seen := map[string]bool{} + result, err := coordinator.Dissolve(stereopair.DissolveRequest{ + MemberIPAddress: dissolveHost, + ExpectedGroupID: current.Group.ID, + ExpectedGroup: current.Group, + }) + if err != nil { + PrintError(fmt.Sprintf("Failed to remove stereo pair: %v", err)) + printGroupResultDetails(result) - for _, role := range group.Roles.Roles { - if role.IPAddress != "" && !seen[role.IPAddress] { - seen[role.IPAddress] = true - members = append(members, role.IPAddress) - } - } - - // Always include the addressed host even if the group response omitted IPs. - if !seen[clientConfig.Host] { - members = append(members, clientConfig.Host) - } - - results := make([]memberResult, len(members)) - - var wg sync.WaitGroup - - for i, ip := range members { - wg.Add(1) - - go func(idx int, host string) { - defer wg.Done() - - mc, mcErr := clientForHost(c, host) - if mcErr != nil { - results[idx] = memberResult{ip: host, err: mcErr} - return - } - - results[idx] = memberResult{ip: host, err: mc.RemoveGroup()} - }(i, ip) - } - - wg.Wait() - - anyErr := false - - for _, r := range results { - if r.err != nil { - PrintError(fmt.Sprintf("%s /removeGroup failed: %v", r.ip, r.err)) - - anyErr = true - } - } - - if anyErr { - return fmt.Errorf("/removeGroup propagation failed") + return err } PrintSuccess("Stereo pair removed") @@ -333,32 +160,111 @@ func removeGroup(c *cli.Context) error { return nil } -// fetchDeviceInfo builds a one-off client for the given IP and reads /info. -// Reused for both halves of a `create` invocation so the caller doesn't have -// to babysit two host/port pairs. -func fetchDeviceInfo(c *cli.Context, host string) (*models.DeviceInfo, error) { - stClient, err := clientForHost(c, host) - if err != nil { - return nil, err +func dissolveRecoveryHost(result stereopair.Result, fallback string) string { + if result.Group == nil || result.Group.ID == "" { + return fallback } - return stClient.GetDeviceInfo() + for i := range result.Members { + member := &result.Members[i] + if member.Group != nil && member.Group.ID == result.Group.ID && net.ParseIP(member.IPAddress) != nil { + return member.IPAddress + } + } + + return fallback } -// clientForHost mirrors CreateSoundTouchClient but overrides the host so we -// can talk to a speaker other than the one named in --host. -func clientForHost(c *cli.Context, host string) (*client.Client, error) { - cfg, err := loadConfig(c.Duration("timeout")) - if err != nil { - return nil, fmt.Errorf("failed to load config: %w", err) +func newGroupCoordinator(config *ClientConfig) *stereopair.Coordinator { + lifecycleConfig := *config + if lifecycleConfig.Timeout < stereopair.RequestTimeout { + lifecycleConfig.Timeout = stereopair.RequestTimeout } - return client.NewClient(&client.Config{ - Host: host, - Port: speaker.HTTPPort, - Timeout: cfg.HTTPTimeout, - UserAgent: cfg.UserAgent, - }), nil + cleanupClient := &http.Client{Timeout: lifecycleConfig.Timeout} + + return stereopair.NewWithGenerationLifecyclePersistence( + groupClientFactory(&lifecycleConfig), + func(ref stereopair.GenerationRef) error { + return stereopair.DeleteMargeGroupGeneration(cleanupClient, ref) + }, + func(refs []stereopair.GenerationRef) error { + return stereopair.EnsureMargeNoGroupGenerations(cleanupClient, refs) + }, + func(ref stereopair.GenerationRef, name string) error { + return stereopair.RenameMargeGroupGeneration(cleanupClient, ref, name) + }, + ) +} + +// groupClientFactory addresses every member directly while retaining the +// effective CLI port and timeout. +func groupClientFactory(config *ClientConfig) stereopair.ClientFactory { + baseConfig := *config + + return func(ipAddress string) (stereopair.Client, error) { + memberConfig := baseConfig + memberConfig.Host = ipAddress + + return CreateSoundTouchClient(&memberConfig) + } +} + +func printGroupResultDetails(result stereopair.Result) { + if result.Status == stereopair.StatusDegraded { + PrintWarning(fmt.Sprintf("Stereo-pair %s result is degraded", result.Operation)) + } + + for i := range result.Members { + member := &result.Members[i] + label := groupMemberLabel(i, member) + + if member.PreflightError != nil { + PrintError(fmt.Sprintf("%s preflight failed: %v", label, member.PreflightError)) + } + + if member.MutationError != nil { + PrintError(fmt.Sprintf("%s mutation failed: %v", label, member.MutationError)) + } + + if member.VerificationError != nil { + PrintError(fmt.Sprintf("%s verification failed: %v", label, member.VerificationError)) + } + + if member.CompensationError != nil { + PrintError(fmt.Sprintf("%s cleanup failed: %v", label, member.CompensationError)) + } else if member.CompensationAttempted && !member.CompensationVerified { + PrintWarning(fmt.Sprintf("%s cleanup could not be verified", label)) + } + } + + if result.CompensationAttempted { + if result.CompensationComplete { + PrintWarning("Partial stereo-pair state was cleaned up and verified") + } else { + PrintError("Partial stereo-pair state cleanup is incomplete") + } + } + + if result.PersistenceError != nil { + PrintError(fmt.Sprintf("Persistent group generation update failed: %v", result.PersistenceError)) + } +} + +func groupMemberLabel(index int, member *stereopair.MemberResult) string { + if member.IPAddress != "" && member.DeviceID != "" { + return fmt.Sprintf("%s (%s)", member.IPAddress, member.DeviceID) + } + + if member.IPAddress != "" { + return member.IPAddress + } + + if member.DeviceID != "" { + return member.DeviceID + } + + return fmt.Sprintf("member %d", index+1) } func printGroup(g *models.Group) { diff --git a/cmd/soundtouch-cli/cmd_group_test.go b/cmd/soundtouch-cli/cmd_group_test.go index 8eb5c96f..70ef8699 100644 --- a/cmd/soundtouch-cli/cmd_group_test.go +++ b/cmd/soundtouch-cli/cmd_group_test.go @@ -1,184 +1,211 @@ package main import ( - "encoding/xml" - "io" + "errors" + "net" "net/http" "net/http/httptest" + "net/url" + "strconv" "strings" "testing" + "time" - "github.com/gesellix/bose-soundtouch/pkg/client" "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/stereopair" ) -// happyAddGroupServer fakes a speaker's /addGroup that echoes the request -// with an assigned ID and GROUP_OK status, matching real hardware behaviour. -func happyAddGroupServer(t *testing.T, assignedID string) (*httptest.Server, *[]string) { - t.Helper() - - bodies := make([]string, 0) - +func TestGroupClientFactoryUsesMemberHostAndConfiguredPort(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/addGroup" || r.Method != http.MethodPost { - t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) - http.NotFound(w, r) - - return + if r.URL.Path != "/getGroup" { + t.Errorf("path = %q, want /getGroup", r.URL.Path) } - body, _ := io.ReadAll(r.Body) - bodies = append(bodies, string(body)) - - var got models.Group - if err := xml.Unmarshal(body, &got); err != nil { - t.Fatalf("decode request body: %v", err) - } - - got.ID = assignedID - got.Status = "GROUP_OK" - w.Header().Set("Content-Type", "application/xml") - - enc, _ := xml.Marshal(&got) - _, _ = w.Write(enc) + _, _ = w.Write([]byte(`Pair`)) })) + defer srv.Close() - return srv, &bodies -} + host, port := testServerHostPort(t, srv.URL) + factory := groupClientFactory(&ClientConfig{ + Host: "192.0.2.200", + Port: port, + Timeout: time.Second, + }) -func newTestGroupClient(serverURL string) *client.Client { - return client.NewClientFromHost(serverURL) -} + memberClient, err := factory(host) + if err != nil { + t.Fatalf("factory: %v", err) + } -func sampleGroupRequest(leftIP, rightIP string) *models.Group { - return &models.Group{ - Name: "Living Room", - MasterDeviceID: "9070658C9D4A", - Roles: models.GroupRoles{ - Roles: []models.GroupRole{ - {DeviceID: "9070658C9D4A", Role: "LEFT", IPAddress: leftIP}, - {DeviceID: "F45EAB3115DA", Role: "RIGHT", IPAddress: rightIP}, - }, - }, - // senderIPAddress is intentionally not set here; propagateAddGroup - // adds it to the slave's copy only. + group, err := memberClient.GetGroup() + if err != nil { + t.Fatalf("GetGroup: %v", err) + } + + if group.ID != "pair-id" || group.Name != "Pair" { + t.Fatalf("group = %+v, want test server response", group) } } -func TestPropagateAddGroup_BothSucceed(t *testing.T) { - leftSrv, leftBodies := happyAddGroupServer(t, "9999999") - defer leftSrv.Close() +func TestGroupClientFactoryUsesConfiguredTimeout(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + time.Sleep(100 * time.Millisecond) + _, _ = w.Write([]byte(``)) + })) + defer srv.Close() - rightSrv, rightBodies := happyAddGroupServer(t, "9999999") - defer rightSrv.Close() - - leftClient := newTestGroupClient(leftSrv.URL) - rightClient := newTestGroupClient(rightSrv.URL) - - req := sampleGroupRequest("192.0.2.131", "192.0.2.134") - - leftOut, rightOut := propagateAddGroup(leftClient, rightClient, "192.0.2.131", "192.0.2.134", req) - - if leftOut.err != nil { - t.Errorf("LEFT err = %v, want nil", leftOut.err) + host, port := testServerHostPort(t, srv.URL) + factory := groupClientFactory(&ClientConfig{Port: port, Timeout: 5 * time.Millisecond}) + memberClient, err := factory(host) + if err != nil { + t.Fatalf("factory: %v", err) } - if rightOut.err != nil { - t.Errorf("RIGHT err = %v, want nil", rightOut.err) + if _, err := memberClient.GetGroup(); err == nil { + t.Fatal("GetGroup succeeded, want configured timeout") + } +} + +func TestMargeGroupGenerationURL(t *testing.T) { + tests := []struct { + base string + want string + }{ + {base: "http://aftertouch.example:8000", want: "http://aftertouch.example:8000/streaming/account/ACCOUNT1/group/PAIR1"}, + {base: "http://unifi:8001/marge", want: "http://unifi:8001/marge/streaming/account/ACCOUNT1/group/PAIR1"}, + {base: "https://proxy.example/prefix/streaming/", want: "https://proxy.example/prefix/streaming/account/ACCOUNT1/group/PAIR1"}, } - if leftOut.group == nil || leftOut.group.ID != "9999999" || leftOut.group.Status != "GROUP_OK" { - t.Errorf("LEFT group = %+v, want id=9999999 status=GROUP_OK", leftOut.group) - } - - if rightOut.group == nil || rightOut.group.Status != "GROUP_OK" { - t.Errorf("RIGHT group = %+v, want status=GROUP_OK", rightOut.group) - } - - // Both speakers must have received the roles, but only the slave's payload - // carries senderIPAddress — see propagateAddGroup for the why. - for label, bodies := range map[string]*[]string{"LEFT": leftBodies, "RIGHT": rightBodies} { - if len(*bodies) != 1 { - t.Fatalf("%s: expected exactly one POST, got %d", label, len(*bodies)) + for _, test := range tests { + got, err := stereopair.MargeGroupGenerationURL(stereopair.GenerationRef{ + MargeURL: test.base, AccountID: "ACCOUNT1", GroupID: "PAIR1", + }) + if err != nil { + t.Fatalf("margeGroupGenerationURL(%q): %v", test.base, err) } + if got != test.want { + t.Errorf("margeGroupGenerationURL(%q) = %q, want %q", test.base, got, test.want) + } + } +} - body := (*bodies)[0] - for _, want := range []string{"LEFT", "RIGHT"} { - if !strings.Contains(body, want) { - t.Errorf("%s body missing %q\nbody:\n%s", label, want, body) +func TestDeleteMargeGroupGenerationUsesExactEndpoint(t *testing.T) { + deleteSeen := false + getSeen := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodDelete: + deleteSeen = true + if r.URL.Path != "/streaming/account/ACCOUNT1/group/PAIR1" { + t.Errorf("DELETE path = %s", r.URL.Path) + } + w.WriteHeader(http.StatusOK) + case http.MethodGet: + getSeen = true + if r.URL.Path != "/streaming/account/ACCOUNT1/device/LEFT-ID/group" { + t.Errorf("GET path = %s", r.URL.Path) + } + if deleteSeen { + _, _ = w.Write([]byte(``)) + } else { + _, _ = w.Write([]byte(`LEFT-IDLEFT-IDLEFT192.0.2.10RIGHT-IDRIGHT192.0.2.11`)) } } - } - - leftBody := (*leftBodies)[0] - if strings.Contains(leftBody, "") { - t.Errorf("LEFT (master) body must NOT carry , otherwise the master flips into slave mode (issue #252)\nbody:\n%s", leftBody) - } - - rightBody := (*rightBodies)[0] - if !strings.Contains(rightBody, "192.0.2.131") { - t.Errorf("RIGHT (slave) body must carry 192.0.2.131\nbody:\n%s", rightBody) - } -} - -func TestPropagateAddGroup_RightFails(t *testing.T) { - leftSrv, _ := happyAddGroupServer(t, "9999999") - defer leftSrv.Close() - - rightSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - http.Error(w, "boom", http.StatusInternalServerError) })) - defer rightSrv.Close() + defer server.Close() - leftClient := newTestGroupClient(leftSrv.URL) - rightClient := newTestGroupClient(rightSrv.URL) - - req := sampleGroupRequest("192.0.2.131", "192.0.2.134") - - leftOut, rightOut := propagateAddGroup(leftClient, rightClient, "192.0.2.131", "192.0.2.134", req) - - if leftOut.err != nil { - t.Errorf("LEFT err = %v, want nil", leftOut.err) + err := stereopair.DeleteMargeGroupGeneration(server.Client(), stereopair.GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: &models.Group{ + ID: "PAIR1", + MasterDeviceID: "LEFT-ID", + Roles: models.GroupRoles{Roles: []models.GroupRole{ + {DeviceID: "LEFT-ID", Role: "LEFT", IPAddress: "192.0.2.10"}, + {DeviceID: "RIGHT-ID", Role: "RIGHT", IPAddress: "192.0.2.11"}, + }}, + }, + }) + if err != nil { + t.Fatalf("deleteMargeGroupGeneration: %v", err) } - - if rightOut.err == nil { - t.Error("RIGHT err = nil, want non-nil") + if !deleteSeen || !getSeen { + t.Fatalf("Marge cleanup requests DELETE=%t GET=%t, want both", deleteSeen, getSeen) } } -func TestPostAddGroup_StatusOtherThanGroupOKIsError(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "application/xml") - _, _ = w.Write([]byte(`GROUP_NOT_READY`)) - })) - defer srv.Close() - - out := postAddGroup(newTestGroupClient(srv.URL), "test", sampleGroupRequest("1.1.1.1", "2.2.2.2")) - - if out.err == nil { - t.Fatal("expected error for non-GROUP_OK status") +func TestPrintGroupResultDetailsReportsMemberFailuresAndCleanup(t *testing.T) { + result := stereopair.Result{ + Operation: stereopair.OperationCreate, + Status: stereopair.StatusDegraded, + CompensationAttempted: true, + PersistenceError: errors.New("datastore unavailable"), + Members: []stereopair.MemberResult{ + { + IPAddress: "192.0.2.10", + DeviceID: "LEFT-ID", + PreflightError: errors.New("offline"), + }, + { + IPAddress: "192.0.2.11", + MutationError: errors.New("add failed"), + VerificationError: errors.New("unexpected group"), + CompensationAttempted: true, + CompensationError: errors.New("remove failed"), + }, + }, } - if !strings.Contains(out.err.Error(), "GROUP_NOT_READY") { - t.Errorf("error %q does not mention returned status", out.err) + output := captureStdout(t, func() { + printGroupResultDetails(result) + }) + + for _, expected := range []string{ + "Stereo-pair create result is degraded", + "192.0.2.10 (LEFT-ID) preflight failed: offline", + "192.0.2.11 mutation failed: add failed", + "192.0.2.11 verification failed: unexpected group", + "192.0.2.11 cleanup failed: remove failed", + "Partial stereo-pair state cleanup is incomplete", + "Persistent group generation update failed: datastore unavailable", + } { + if !strings.Contains(output, expected) { + t.Errorf("output missing %q:\n%s", expected, output) + } } } -func TestPostAddGroup_EmptyStatusIsAccepted(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "application/xml") - _, _ = w.Write([]byte(`n`)) - })) - defer srv.Close() - - out := postAddGroup(newTestGroupClient(srv.URL), "test", sampleGroupRequest("1.1.1.1", "2.2.2.2")) - - if out.err != nil { - t.Errorf("err = %v, want nil for empty status (some firmware omits it)", out.err) +func TestDissolveRecoveryHostSelectsStillGroupedMember(t *testing.T) { + result := stereopair.Result{ + Group: &models.Group{ID: "PAIR-ID"}, + Members: []stereopair.MemberResult{ + {IPAddress: "192.0.2.10", Group: &models.Group{}}, + {IPAddress: "192.0.2.11", Group: &models.Group{ID: "PAIR-ID"}}, + }, } - if out.group == nil || out.group.ID != "42" { - t.Errorf("group = %+v, want id=42", out.group) + if got := dissolveRecoveryHost(result, "192.0.2.10"); got != "192.0.2.11" { + t.Fatalf("recovery host = %q, want surviving member", got) } } + +func testServerHostPort(t *testing.T, serverURL string) (string, int) { + t.Helper() + + parsed, err := url.Parse(serverURL) + if err != nil { + t.Fatalf("parse server URL: %v", err) + } + + host, portText, err := net.SplitHostPort(parsed.Host) + if err != nil { + t.Fatalf("split server host: %v", err) + } + + port, err := strconv.Atoi(portText) + if err != nil { + t.Fatalf("parse server port: %v", err) + } + + return host, port +} diff --git a/cmd/soundtouch-player/README.md b/cmd/soundtouch-player/README.md index b2f0a947..fa4150c7 100644 --- a/cmd/soundtouch-player/README.md +++ b/cmd/soundtouch-player/README.md @@ -25,6 +25,9 @@ Based on captured WebSocket interactions and device API capabilities, this web U - **Real-time status monitoring** via WebSocket connections - **Multi-device support** with centralized control - **Connection status** indicators and health monitoring +- **SoundTouch 10 stereo pairs** shown as one target, with verified create, + rename, and dissolve operations across both physical speakers and their + exact persisted group generation ### Playback Control - **Play/Pause/Stop/Next/Previous** controls diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go index 08a796c5..493e9a4b 100644 --- a/cmd/soundtouch-service/main.go +++ b/cmd/soundtouch-service/main.go @@ -6,6 +6,7 @@ import ( "context" "crypto/tls" "encoding/json" + "errors" "fmt" "io" "log" @@ -34,6 +35,7 @@ import ( "github.com/gesellix/bose-soundtouch/pkg/service/spotify" "github.com/gesellix/bose-soundtouch/pkg/service/stockholm" "github.com/gesellix/bose-soundtouch/pkg/service/updatecheck" + "github.com/gesellix/bose-soundtouch/pkg/stereopair" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" "github.com/urfave/cli/v2" @@ -1504,6 +1506,19 @@ func newEmbeddedWebApp(server *handlers.Server, serverURL, internalURL string, d return err } + // Preserve the datastore's atomic, all-account guarantees for speakers that + // point at this service, while following fresh /info to an external Marge + // backend for speakers still managed by SoundCork or another service. + cleanup, preflight, rename := embeddedStereoPairGenerationPersistence( + ds, + func() []string { + localServerURL, localHTTPSServerURL := server.GetSettings() + return []string{localServerURL, localHTTPSServerURL} + }, + &http.Client{Timeout: stereopair.RequestTimeout}, + ) + webApp.SetStereoPairGenerationPersistence(cleanup, preflight, rename) + // Keep the UI registry live as the service discovers or devices are added. server.SetDevicesChangedHook(func() { webApp.SeedExtraDevices() @@ -1531,6 +1546,71 @@ func newEmbeddedWebApp(server *handlers.Server, serverURL, internalURL string, d return webApp } +func embeddedStereoPairGenerationPersistence( + ds *datastore.DataStore, + localMargeURLs func() []string, + httpClient *http.Client, +) (stereopair.GenerationCleanup, stereopair.GenerationPreflight, stereopair.GenerationRename) { + isLocal := func(margeURL string, localURLs []string) bool { + for _, localURL := range localURLs { + if stereopair.SameMargeBackend(margeURL, localURL) { + return true + } + } + + return false + } + + cleanup := func(ref stereopair.GenerationRef) error { + if isLocal(ref.MargeURL, localMargeURLs()) { + return ds.DeleteGroupGenerationForDevice(ref.DeviceID, ref.GroupID, ref.ExpectedGroup) + } + + return stereopair.DeleteMargeGroupGeneration(httpClient, ref) + } + + preflight := func(refs []stereopair.GenerationRef) error { + localURLs := localMargeURLs() + localDeviceIDs := make([]string, 0, len(refs)) + externalRefs := make([]stereopair.GenerationRef, 0, len(refs)) + + for i := range refs { + if isLocal(refs[i].MargeURL, localURLs) { + localDeviceIDs = append(localDeviceIDs, refs[i].DeviceID) + } else { + externalRefs = append(externalRefs, refs[i]) + } + } + + if len(localDeviceIDs) > 0 { + if err := ds.EnsureNoGroupsForDevices(localDeviceIDs); err != nil { + return err + } + } + + if len(externalRefs) > 0 { + return stereopair.EnsureMargeNoGroupGenerations(httpClient, externalRefs) + } + + return nil + } + + rename := func(ref stereopair.GenerationRef, name string) error { + if isLocal(ref.MargeURL, localMargeURLs()) { + _, err := ds.RenameGroupGenerationForDevice(ref.DeviceID, ref.GroupID, ref.ExpectedGroup, name) + if errors.Is(err, datastore.ErrGroupNotFound) || errors.Is(err, datastore.ErrGroupDeleteAmbiguous) { + return fmt.Errorf("%w: %w", stereopair.ErrConflict, err) + } + + return err + } + + return stereopair.RenameMargeGroupGeneration(httpClient, ref, name) + } + + return cleanup, preflight, rename +} + func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler, webApp *soundtouchweb.WebApp) *chi.Mux { r := chi.NewRouter() diff --git a/cmd/soundtouch-service/stereo_pair_persistence_test.go b/cmd/soundtouch-service/stereo_pair_persistence_test.go new file mode 100644 index 00000000..41f8c22b --- /dev/null +++ b/cmd/soundtouch-service/stereo_pair_persistence_test.go @@ -0,0 +1,175 @@ +package main + +import ( + "encoding/xml" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/datastore" + "github.com/gesellix/bose-soundtouch/pkg/stereopair" +) + +type rejectingRoundTripper struct{} + +func (rejectingRoundTripper) RoundTrip(*http.Request) (*http.Response, error) { + return nil, errors.New("unexpected HTTP persistence request") +} + +func persistenceTestGroup(id string) *models.Group { + return &models.Group{ + ID: id, + Name: "Living room", + MasterDeviceID: "LEFT-ID", + Roles: models.GroupRoles{Roles: []models.GroupRole{ + {DeviceID: "LEFT-ID", Role: "LEFT", IPAddress: "192.0.2.10"}, + {DeviceID: "RIGHT-ID", Role: "RIGHT", IPAddress: "192.0.2.11"}, + }}, + } +} + +func TestEmbeddedStereoPairPersistenceUsesLocalDatastoreAcrossAccounts(t *testing.T) { + ds := datastore.NewDataStore(t.TempDir()) + group := persistenceTestGroup("") + groupID, err := ds.AddGroup("OLD-ACCOUNT", group) + if err != nil { + t.Fatalf("AddGroup: %v", err) + } + + localURL := "https://aftertouch.invalid:18443" + cleanup, preflight, rename := embeddedStereoPairGenerationPersistence( + ds, + func() []string { return []string{localURL} }, + &http.Client{Transport: rejectingRoundTripper{}}, + ) + + err = preflight([]stereopair.GenerationRef{{ + DeviceID: "LEFT-ID", AccountID: "NEW-ACCOUNT", MargeURL: localURL, + }}) + if err == nil || !strings.Contains(err.Error(), groupID) { + t.Fatalf("preflight error = %v, want cross-account generation %s", err, groupID) + } + if err := rename(stereopair.GenerationRef{ + DeviceID: "LEFT-ID", AccountID: "NEW-ACCOUNT", MargeURL: localURL, + GroupID: groupID, ExpectedGroup: group, + }, "Renamed living room"); err != nil { + t.Fatalf("rename: %v", err) + } + group.Name = "Renamed living room" + + if err := cleanup(stereopair.GenerationRef{ + DeviceID: "LEFT-ID", AccountID: "NEW-ACCOUNT", MargeURL: localURL, + GroupID: groupID, ExpectedGroup: group, + }); err != nil { + t.Fatalf("cleanup: %v", err) + } + + if err := preflight([]stereopair.GenerationRef{{ + DeviceID: "LEFT-ID", AccountID: "NEW-ACCOUNT", MargeURL: localURL, + }}); err != nil { + t.Fatalf("preflight after exact cleanup: %v", err) + } +} + +func TestEmbeddedStereoPairPersistenceUsesExternalMargeBackend(t *testing.T) { + active := true + deleteCalls := 0 + postCalls := 0 + expected := persistenceTestGroup("7654321") + + 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) + } + })) + defer server.Close() + + cleanup, _, rename := embeddedStereoPairGenerationPersistence( + datastore.NewDataStore(t.TempDir()), + func() []string { return []string{"http://aftertouch.invalid:18000"} }, + server.Client(), + ) + if err := rename(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) + } + + 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 deleteCalls != 1 || active { + t.Fatalf("external DELETE calls = %d, active = %v; want 1, false", deleteCalls, active) + } +} + +func TestEmbeddedStereoPairPersistenceReadsOneCurrentURLSnapshot(t *testing.T) { + ds := datastore.NewDataStore(t.TempDir()) + group := persistenceTestGroup("") + groupID, err := ds.AddGroup("OLD-ACCOUNT", group) + if err != nil { + t.Fatalf("AddGroup: %v", err) + } + + currentURL := "http://old.invalid:18000" + providerCalls := 0 + _, preflight, _ := embeddedStereoPairGenerationPersistence( + ds, + func() []string { + providerCalls++ + return []string{currentURL} + }, + &http.Client{Transport: rejectingRoundTripper{}}, + ) + + currentURL = "http://new.invalid:18000" + err = preflight([]stereopair.GenerationRef{ + {DeviceID: "LEFT-ID", AccountID: "NEW-ACCOUNT", MargeURL: currentURL}, + {DeviceID: "RIGHT-ID", AccountID: "NEW-ACCOUNT", MargeURL: currentURL}, + }) + if err == nil || !strings.Contains(err.Error(), groupID) { + t.Fatalf("preflight error = %v, want current local generation %s", err, groupID) + } + if providerCalls != 1 { + t.Fatalf("URL provider calls = %d, want one coherent snapshot", providerCalls) + } + + 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) + } +} diff --git a/cmd/soundtouch-service/testdata/router_routes.txt b/cmd/soundtouch-service/testdata/router_routes.txt index 9f8d0b3f..1b0260a3 100644 --- a/cmd/soundtouch-service/testdata/router_routes.txt +++ b/cmd/soundtouch-service/testdata/router_routes.txt @@ -6,6 +6,7 @@ DELETE /accounts/{account}/group/ handlers.( DELETE /accounts/{account}/group/{groupId} handlers.(*Server).HandleUnsupported-fm DELETE /api/control/devices/{id}/ soundtouchweb.(*WebApp).HandleDeleteDevice-fm DELETE /api/control/devices/{id}/library/servers/{account} soundtouchweb.(*WebApp).HandleRemoveLibraryServer-fm +DELETE /api/control/devices/{id}/stereo-pair/ soundtouchweb.(*WebApp).HandleDissolveStereoPair-fm DELETE /api/setup/devices/{deviceId} handlers.(*Server).HandleRemoveDevice-fm DELETE /api/setup/dns-discoveries handlers.(*Server).HandleClearDNSDiscoveries-fm DELETE /api/setup/interactions/sessions handlers.(*Server).HandleCleanupSessions-fm @@ -44,6 +45,7 @@ GET /api/control/devices/{id}/library/browse soundtouch GET /api/control/devices/{id}/library/servers soundtouchweb.(*WebApp).HandleDeviceLibraryServers-fm GET /api/control/devices/{id}/power-status soundtouchweb.(*WebApp).HandleDevicePowerStatus-fm GET /api/control/devices/{id}/recents soundtouchweb.(*WebApp).HandleDeviceRecents-fm +GET /api/control/devices/{id}/stereo-pair/ soundtouchweb.(*WebApp).HandleGetStereoPair-fm GET /api/control/devices/{id}/ws soundtouchweb.(*WebApp).HandleDeviceWebSocket-fm GET /api/control/devices/{id}/zone/ soundtouchweb.(*WebApp).HandleGetZone-fm GET /api/control/devices/{id}/zone/candidates soundtouchweb.(*WebApp).HandleGetZoneCandidates-fm @@ -175,6 +177,7 @@ HEAD /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter handler HEAD /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter/* handlers.(*Server).HandleSiriusXMLiveAdapterSubpath-fm OPTIONS /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter handlers.(*Server).HandleSiriusXMLiveAdapter-fm OPTIONS /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter/* handlers.(*Server).HandleSiriusXMLiveAdapterSubpath-fm +PATCH /api/control/devices/{id}/stereo-pair/ soundtouchweb.(*WebApp).HandleRenameStereoPair-fm PATCH /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter handlers.(*Server).HandleSiriusXMLiveAdapter-fm PATCH /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter/* handlers.(*Server).HandleSiriusXMLiveAdapterSubpath-fm POST /accounts/{account}/devices handlers.(*Server).HandleUnsupported-fm @@ -195,6 +198,7 @@ POST /api/control/devices/{id}/providers/radiobrowser/play soundtouch POST /api/control/devices/{id}/providers/tts/play soundtouchweb.(*WebApp).HandleAPISpeakText-fm POST /api/control/devices/{id}/providers/tunein/play soundtouchweb.(*WebApp).HandlePlayTuneIn-fm POST /api/control/devices/{id}/providers/url/play soundtouchweb.(*WebApp).HandlePlayURL-fm +POST /api/control/devices/{id}/stereo-pair/ soundtouchweb.(*WebApp).HandleCreateStereoPair-fm POST /api/control/devices/{id}/volume/{volume} soundtouchweb.(*WebApp).HandleDirectVolumeControl-fm POST /api/control/devices/{id}/zone/add/{slaveId} soundtouchweb.(*WebApp).HandleZoneAdd-fm POST /api/control/devices/{id}/zone/dissolve soundtouchweb.(*WebApp).HandleZoneDissolve-fm diff --git a/docs/content/docs/appendix/soundtouch-player-roadmap.md b/docs/content/docs/appendix/soundtouch-player-roadmap.md index 4c069a72..955bd3c9 100644 --- a/docs/content/docs/appendix/soundtouch-player-roadmap.md +++ b/docs/content/docs/appendix/soundtouch-player-roadmap.md @@ -101,7 +101,7 @@ rename and network/firmware info. --- -## 4. Render stereo pairs as a single device (shipped) +## 4. Stereo-pair presentation and lifecycle (shipped) soundtouch-player projects a valid two-speaker stereo pair (formed via `/addGroup` - see [issue #252](https://github.com/gesellix/Bose-SoundTouch/issues/252)) @@ -136,12 +136,59 @@ registry. a member is unavailable or the group reports a non-OK state. - Hide the single-device remove action on a projected pair. Standalone speakers continue to render as before. +- For standalone stereo-capable SoundTouch 10 speakers, offer pair creation + with an explicit LEFT/master and RIGHT member. +- For an existing pair, offer rename and a separately confirmed dissolve + action. A dissolve changes speaker group state; it does not delete either + physical speaker from the player registry. -**Note:** Pair lifecycle (create / rename / remove) remains available through -the existing client and CLI group operations. The player intentionally does -not expose a "Dissolve pair" action yet: its current remove operation deletes -one physical registry record rather than performing an atomic pair lifecycle -operation. +**Lifecycle safety:** +- A shared coordinator backs both the CLI and player. It freshly checks both + speakers, their L/R capability, current group, and temporary-zone state + before a mutation. Pair creation also requires one shared Marge account and + backend. +- After both create candidates are freshly verified as physically standalone, + a fail-closed, read-only persistence barrier checks for a stored group before + either speaker is mutated. The embedded service searches every account by + device ID; standalone player and CLI query the speakers' current Marge + backend. Creation stops and reports the exact stale generation when any + record remains; pre-create checks never delete it. +- Create sends the asymmetric master/slave payloads required by the speaker + state machines, then freshly verifies that both members agree. A partial + create is compensated only where the exact group generation returned by + that speaker can be proven. +- Rename and dissolve update both physical speakers and report a degraded + result, including per-member detail, instead of claiming success after a + partial transition. +- Rename and dissolve carry the group ID displayed to the user and reject a + stale request if either speaker now belongs to another generation. +- A degraded dissolve retains the last exact L/R topology for a bounded retry. + The retry freshly verifies both physical identities and states, and stored + persistence must match that full topology before it can be retired. +- Legacy Marge teardown callbacks without a group ID are acknowledged without + deleting persistent state. After a verified physical dissolve, the embedded + player retires the exact generation directly in its datastore; standalone + player and CLI use the generation-aware endpoint derived from fresh speaker + info. Physical verification and exact persistence cleanup share one + coordinator lock, and a cleanup failure is returned as degraded. +- Retired group IDs leave their small XML snapshot in the datastore and + active/retired IDs are reserved across all accounts, so an account move or + stale request cannot match a later physical generation. +- Pair mutations are rejected while either member belongs to a temporary + multi-room zone. The zone must be dissolved first. + +!!! warning "Run lifecycle operations site-locally" + Marge hostnames such as `unifi` are resolved from the caller's site, not + from the speaker's site. Create, rename, and dissolve a pair through the + Player/service deployment co-located with both speakers and their Marge + backend; a cross-site registry entry is not a backend-routing mechanism. + +!!! warning "Datastore downgrade boundary" + Once this lifecycle has written a `Group_.retired` snapshot, do not run an + older service binary against the same datastore. Older allocators do not + reserve these generation IDs globally and can reuse one. Restore both the + binary and its pre-lifecycle datastore snapshot for a rollback, or upgrade + forward. --- diff --git a/docs/content/docs/guides/CLI-REFERENCE.md b/docs/content/docs/guides/CLI-REFERENCE.md index 6d1ea63a..d5944059 100644 --- a/docs/content/docs/guides/CLI-REFERENCE.md +++ b/docs/content/docs/guides/CLI-REFERENCE.md @@ -855,6 +855,48 @@ soundtouch-cli --host 192.0.2.10 zone remove --member 192.0.2.12 soundtouch-cli --host 192.0.2.10 zone dissolve ``` +### Stereo Pair Management + +Create and manage a persistent LEFT/RIGHT pair of two SoundTouch 10 speakers. +This is distinct from a temporary multi-room zone. Both speakers must be +online, stereo-capable, standalone, and outside any zone before a lifecycle +operation. Pair creation also requires both speakers to use the same Marge +account and backend. Run lifecycle commands from the site containing both +speakers; site-relative Marge names such as `unifi` do not identify a remote +site when resolved by the CLI host. + +```bash +# Inspect a standalone speaker or either member of a pair +soundtouch-cli --host 192.0.2.10 group status + +# Create a pair; the LEFT speaker becomes the master +soundtouch-cli group create \ + --left 192.0.2.10 \ + --right 192.0.2.11 \ + --name "Living Room" + +# Rename through either member +soundtouch-cli --host 192.0.2.10 group rename --name "Living Room Pair" + +# Dissolve the pair without removing either speaker from AfterTouch +soundtouch-cli --host 192.0.2.10 group remove +``` + +Create, rename, and remove verify fresh state on both speakers. Rename and +remove first inspect the current group and carry its ID as a generation guard; +if the pair changes before mutation, the operation fails without touching the +newer pair. A partial transition is reported as degraded with per-speaker +details rather than as a successful operation. A remove attempt carries the +last exact L/R topology, freshly verifies both speakers, and retires +persistence only if the stored generation still matches it. Before create, +the CLI verifies +both speakers as standalone, queries their current Marge backend for stale +group records, and refuses to mutate either speaker while any record remains. +After verified physical cleanup, the CLI removes the exact group ID through the +Marge URL and account freshly read from the speaker. A backend cleanup failure +is therefore visible as a degraded result instead of leaving an apparently +successful stale generation. + ### Browse and Navigation Browse and navigate content sources on your device. diff --git a/pkg/client/client.go b/pkg/client/client.go index fe8f2318..e27b7993 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -144,6 +144,7 @@ package client import ( "bytes" "encoding/xml" + "errors" "fmt" "io" "net" @@ -1105,6 +1106,10 @@ func (c *Client) Host() string { // get performs a GET request and unmarshals the XML response func (c *Client) get(endpoint string, result interface{}) error { + return c.getWithHTTPClient(c.httpClient, endpoint, result) +} + +func (c *Client) getWithHTTPClient(httpClient *http.Client, endpoint string, result interface{}) error { url := c.baseURL + endpoint req, err := http.NewRequest("GET", url, nil) @@ -1115,7 +1120,7 @@ func (c *Client) get(endpoint string, result interface{}) error { req.Header.Set("User-Agent", c.userAgent) req.Header.Set("Accept", "application/xml") - resp, err := c.httpClient.Do(req) + resp, err := httpClient.Do(req) if err != nil { return fmt.Errorf("failed to execute request: %w", err) } @@ -1151,6 +1156,37 @@ func (c *Client) get(endpoint string, result interface{}) error { return nil } +// mutatingGet performs a firmware-required state-changing GET exactly once at +// the HTTP transport layer. A fresh connection prevents net/http from +// automatically replaying the request after an ambiguous failure on a reused +// connection. +func (c *Client) mutatingGet(endpoint string, result interface{}) error { + baseTransport := c.httpClient.Transport + if baseTransport == nil { + baseTransport = http.DefaultTransport + } + + transport, ok := baseTransport.(*http.Transport) + if !ok { + return errors.New("state-changing GET requires a cloneable HTTP transport") + } + + oneShotTransport := transport.Clone() + + oneShotTransport.DisableKeepAlives = true + defer oneShotTransport.CloseIdleConnections() + + oneShotClient := &http.Client{ + Transport: oneShotTransport, + Timeout: c.httpClient.Timeout, + CheckRedirect: func(_ *http.Request, _ []*http.Request) error { + return http.ErrUseLastResponse + }, + } + + return c.getWithHTTPClient(oneShotClient, endpoint, result) +} + // post performs a POST request with XML body func (c *Client) post(endpoint string, payload interface{}) error { url := c.baseURL + endpoint @@ -1448,10 +1484,11 @@ func (c *Client) GetGroup() (*models.Group, error) { return &g, err } -// AddGroup creates a new stereo pair on the device addressed by this client, -// which becomes the master. The supplied group must contain both LEFT and -// RIGHT roles; the device assigns the group ID and echoes the full state -// in the response. +// AddGroup applies one side of stereo-pair creation to the addressed device. +// The supplied group must contain both LEFT and RIGHT roles. A master-bound +// request omits SenderIPAddress; a slave-bound request sets it to the master's +// IP address. Firmware may acknowledge the request without returning the +// assigned group ID, so callers must verify the resulting state with GetGroup. func (c *Client) AddGroup(group *models.Group) (*models.Group, error) { var result models.Group if err := c.postWithResponse("/addGroup", group, &result); err != nil { @@ -1481,7 +1518,7 @@ func (c *Client) UpdateGroup(group *models.Group) (*models.Group, error) { func (c *Client) RemoveGroup() error { var g models.Group - return c.get("/removeGroup", &g) + return c.mutatingGet("/removeGroup", &g) } // SetName sets the device name diff --git a/pkg/client/group_test.go b/pkg/client/group_test.go index ac02ca8d..89c76d70 100644 --- a/pkg/client/group_test.go +++ b/pkg/client/group_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync/atomic" "testing" "github.com/gesellix/bose-soundtouch/pkg/models" @@ -232,3 +233,74 @@ func TestClient_RemoveGroup(t *testing.T) { t.Fatalf("RemoveGroup: %v", err) } } + +func TestClientRemoveGroupDoesNotReplayDroppedResponse(t *testing.T) { + var calls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/getGroup" { + _, _ = w.Write([]byte(``)) + return + } + + if r.URL.Path != "/removeGroup" { + http.NotFound(w, r) + return + } + + calls.Add(1) + connection, _, err := w.(http.Hijacker).Hijack() + if err != nil { + t.Errorf("hijack response: %v", err) + return + } + + _ = connection.Close() + })) + defer server.Close() + + client := createTestClient(server.URL) + if _, err := client.GetGroup(); err != nil { + t.Fatalf("prime ordinary client connection: %v", err) + } + + err := client.RemoveGroup() + if err == nil { + t.Fatal("RemoveGroup succeeded after the response was dropped") + } + if got := calls.Load(); got != 1 { + t.Fatalf("/removeGroup requests = %d, want exactly 1", got) + } +} + +func TestClientRemoveGroupDoesNotFollowRedirect(t *testing.T) { + var calls atomic.Int32 + var redirectedCalls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + + switch r.URL.Path { + case "/removeGroup": + http.Redirect(w, r, "/redirected", http.StatusTemporaryRedirect) + case "/redirected": + redirectedCalls.Add(1) + _, _ = w.Write([]byte(``)) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + err := createTestClient(server.URL).RemoveGroup() + if err == nil { + t.Fatal("RemoveGroup followed a redirect") + } + if !strings.Contains(err.Error(), "status 307") { + t.Fatalf("RemoveGroup error = %q, want redirect status", err) + } + if got := calls.Load(); got != 1 { + t.Fatalf("HTTP requests = %d, want exactly 1", got) + } + if got := redirectedCalls.Load(); got != 0 { + t.Fatalf("redirect target requests = %d, want 0", got) + } +} diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index 63e41a94..1a96eaae 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -25,8 +25,42 @@ import ( "github.com/gesellix/bose-soundtouch/pkg/service/constants" ) -// ErrGroupNotFound is returned when no group is found for a given device. -var ErrGroupNotFound = errors.New("group not found") +var ( + // ErrGroupNotFound is returned when no group is found for a given device. + ErrGroupNotFound = errors.New("group not found") + // ErrGroupMembershipConflict is returned when a device already belongs to + // another stored group. + ErrGroupMembershipConflict = errors.New("group membership conflict") + // ErrGroupDeleteAmbiguous is returned when a deletion cannot identify + // exactly one group generation without risking unrelated group state. + ErrGroupDeleteAmbiguous = errors.New("group deletion is ambiguous") +) + +// GroupGeneration identifies one active stored group generation. +type GroupGeneration struct { + Account string + ID string +} + +// GroupMembershipConflictError reports the exact active generations which +// contain devices that callers have freshly verified as standalone. +type GroupMembershipConflictError struct { + Generations []GroupGeneration +} + +func (err *GroupMembershipConflictError) Error() string { + generations := make([]string, 0, len(err.Generations)) + for _, generation := range err.Generations { + generations = append(generations, generation.Account+"/"+generation.ID) + } + + return fmt.Sprintf("%s: active group generations %s", ErrGroupMembershipConflict, strings.Join(generations, ", ")) +} + +// Unwrap allows errors.Is to classify this as ErrGroupMembershipConflict. +func (err *GroupMembershipConflictError) Unwrap() error { + return ErrGroupMembershipConflict +} func exists(path string) bool { _, err := os.Stat(path) @@ -3153,14 +3187,310 @@ func (ds *DataStore) groupFilePath(account, groupID string) string { return filepath.Join(ds.AccountDevicesDir(account), "Group_"+groupID+".xml") } -// generateGroupID returns a unique 7-digit group ID that has no existing file. -func (ds *DataStore) generateGroupID(account string) string { - for { - id := fmt.Sprintf("%07d", rand.Int63n(10_000_000)) //nolint:gosec - if !ds.rootExists(ds.groupFilePath(account, id)) { - return id +func (ds *DataStore) retiredGroupFilePath(account, groupID string) string { + return filepath.Join(ds.AccountDevicesDir(account), "Group_"+groupID+".retired") +} + +type groupFileLocation struct { + account string + path string +} + +type groupGenerationLocations struct { + active []groupFileLocation + retired []groupFileLocation +} + +func groupIDFromFilename(name, suffix string) (string, bool) { + if !strings.HasPrefix(name, "Group_") || !strings.HasSuffix(name, suffix) { + return "", false + } + + id := strings.TrimSuffix(strings.TrimPrefix(name, "Group_"), suffix) + + return id, id != "" +} + +func (ds *DataStore) accountNamesNoLock() ([]string, error) { + entries, err := ds.rootReadDir(filepath.Join(ds.baseDir, "accounts")) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + + return nil, err + } + + accounts := make([]string, 0, len(entries)) + for _, entry := range entries { + if entry.IsDir() { + accounts = append(accounts, entry.Name()) } } + + return accounts, nil +} + +func (ds *DataStore) loadGroupGenerationLocationsNoLock() (map[string]groupGenerationLocations, error) { + accounts, err := ds.accountNamesNoLock() + if err != nil { + return nil, err + } + + locations := make(map[string]groupGenerationLocations) + + for _, account := range accounts { + dir := ds.AccountDevicesDir(account) + + entries, readErr := ds.rootReadDir(dir) + if readErr != nil { + if os.IsNotExist(readErr) { + continue + } + + return nil, readErr + } + + for _, entry := range entries { + if entry.IsDir() { + continue + } + + name := entry.Name() + location := groupFileLocation{account: account, path: filepath.Join(dir, name)} + + if id, ok := groupIDFromFilename(name, ".xml"); ok { + generation := locations[id] + generation.active = append(generation.active, location) + locations[id] = generation + } else if id, ok := groupIDFromFilename(name, ".retired"); ok { + generation := locations[id] + generation.retired = append(generation.retired, location) + locations[id] = generation + } + } + } + + return locations, nil +} + +// generateGroupID returns a globally unique 7-digit group ID. Active and +// retired generations reserve their IDs across every account. +func (ds *DataStore) generateGroupID() (string, error) { + locations, err := ds.loadGroupGenerationLocationsNoLock() + if err != nil { + return "", err + } + + for attempts := 0; attempts < 128; attempts++ { + id := fmt.Sprintf("%07d", rand.Int63n(10_000_000)) //nolint:gosec + if _, reserved := locations[id]; !reserved { + return id, nil + } + } + + // Near exhaustion, random selection may repeatedly collide. A deterministic + // fallback guarantees progress whenever any ID remains available. + for candidate := 0; candidate < 10_000_000; candidate++ { + id := fmt.Sprintf("%07d", candidate) + if _, reserved := locations[id]; !reserved { + return id, nil + } + } + + return "", errors.New("all stereo group generation IDs are reserved") +} + +type storedGroup struct { + account string + id string + path string + group models.Group +} + +func (ds *DataStore) loadStoredGroupsNoLock(account string) ([]storedGroup, error) { + dir := ds.AccountDevicesDir(account) + + entries, err := ds.rootReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + + return nil, err + } + + groups := make([]storedGroup, 0) + + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || !strings.HasPrefix(name, "Group_") || !strings.HasSuffix(name, ".xml") { + continue + } + + path := filepath.Join(dir, name) + + data, readErr := ds.rootReadFile(path) + if readErr != nil { + return nil, fmt.Errorf("read stored group %s: %w", name, readErr) + } + + var group models.Group + if unmarshalErr := xml.Unmarshal(data, &group); unmarshalErr != nil { + return nil, fmt.Errorf("parse stored group %s: %w", name, unmarshalErr) + } + + groups = append(groups, storedGroup{ + account: account, + id: strings.TrimSuffix(strings.TrimPrefix(name, "Group_"), ".xml"), + path: path, + group: group, + }) + } + + return groups, nil +} + +func (ds *DataStore) loadAllStoredGroupsNoLock() ([]storedGroup, error) { + accounts, err := ds.accountNamesNoLock() + if err != nil { + return nil, err + } + + groups := make([]storedGroup, 0) + + for _, account := range accounts { + accountGroups, loadErr := ds.loadStoredGroupsNoLock(account) + if loadErr != nil { + return nil, loadErr + } + + groups = append(groups, accountGroups...) + } + + return groups, nil +} + +func uniqueStoredGroupForGeneration(groups []storedGroup, groupID string) (*storedGroup, error) { + matches := make([]storedGroup, 0, 1) + + for i := range groups { + if groups[i].id == groupID { + matches = append(matches, groups[i]) + } + } + + if len(matches) != 1 { + return nil, fmt.Errorf("%w: generation %s has %d matches", + ErrGroupDeleteAmbiguous, groupID, len(matches)) + } + + return &matches[0], nil +} + +func validateExpectedGroupGeneration(expected *models.Group, groupID, deviceID string) error { + if expected == nil || expected.ID != groupID || expected.MasterDeviceID != deviceID { + return fmt.Errorf("%w: generation %s has no exact expected topology", ErrGroupDeleteAmbiguous, groupID) + } + + if _, containsDevice := groupDeviceIDs(expected)[deviceID]; !containsDevice { + return fmt.Errorf("%w: generation %s expected topology does not contain device %s", + ErrGroupDeleteAmbiguous, groupID, deviceID) + } + + return nil +} + +func stereoRoleDevices(group *models.Group) (left, right string, ok bool) { + for _, role := range group.Roles.Roles { + switch role.Role { + case "LEFT": + if left != "" || role.DeviceID == "" { + return "", "", false + } + + left = role.DeviceID + case "RIGHT": + if right != "" || role.DeviceID == "" { + return "", "", false + } + + right = role.DeviceID + } + } + + return left, right, left != "" && right != "" +} + +func sameStereoPair(a, b *models.Group) bool { + if len(a.Roles.Roles) != 2 || len(b.Roles.Roles) != 2 || a.MasterDeviceID != b.MasterDeviceID { + return false + } + + aLeft, aRight, aOK := stereoRoleDevices(a) + bLeft, bRight, bOK := stereoRoleDevices(b) + + return aOK && bOK && aLeft == bLeft && aRight == bRight +} + +func sameGroupGeneration(a, b *models.Group) bool { + if a == nil || b == nil || a.ID != b.ID || a.Name != b.Name || + a.MasterDeviceID != b.MasterDeviceID || len(a.Roles.Roles) != len(b.Roles.Roles) { + return false + } + + roles := make(map[string]models.GroupRole, len(a.Roles.Roles)) + for i := range a.Roles.Roles { + role := a.Roles.Roles[i] + if _, duplicate := roles[role.Role]; duplicate { + return false + } + + roles[role.Role] = role + } + + seen := make(map[string]struct{}, len(b.Roles.Roles)) + for i := range b.Roles.Roles { + role := b.Roles.Roles[i] + if _, duplicate := seen[role.Role]; duplicate { + return false + } + + seen[role.Role] = struct{}{} + + other, found := roles[role.Role] + if !found || other.DeviceID != role.DeviceID || other.IPAddress != role.IPAddress { + return false + } + } + + return true +} + +func groupDeviceIDs(group *models.Group) map[string]struct{} { + deviceIDs := make(map[string]struct{}, len(group.Roles.Roles)+1) + if group.MasterDeviceID != "" { + deviceIDs[group.MasterDeviceID] = struct{}{} + } + + for _, role := range group.Roles.Roles { + if role.DeviceID != "" { + deviceIDs[role.DeviceID] = struct{}{} + } + } + + return deviceIDs +} + +func groupsShareDevice(a, b *models.Group) bool { + aDevices := groupDeviceIDs(a) + for deviceID := range groupDeviceIDs(b) { + if _, found := aDevices[deviceID]; found { + return true + } + } + + return false } // GetGroupForDevice returns the group containing the given device, or nil if ungrouped. @@ -3168,43 +3498,37 @@ func (ds *DataStore) GetGroupForDevice(account, deviceID string) (*models.Group, ds.fileMutex.RLock() defer ds.fileMutex.RUnlock() - dir := ds.AccountDevicesDir(account) - - entries, err := ds.rootReadDir(dir) + groups, err := ds.loadStoredGroupsNoLock(account) if err != nil { - if os.IsNotExist(err) { - return nil, ErrGroupNotFound - } - return nil, err } - for _, e := range entries { - if e.IsDir() || !strings.HasPrefix(e.Name(), "Group_") || !strings.HasSuffix(e.Name(), ".xml") { + var match *models.Group + + for i := range groups { + if _, found := groupDeviceIDs(&groups[i].group)[deviceID]; !found { continue } - data, readErr := ds.rootReadFile(filepath.Join(dir, e.Name())) - if readErr != nil { - continue + if match != nil { + return nil, fmt.Errorf("%w: device %s belongs to multiple active groups", + ErrGroupMembershipConflict, deviceID) } - var g models.Group - if unmarshalErr := xml.Unmarshal(data, &g); unmarshalErr != nil { - continue - } + match = &groups[i].group + } - for _, role := range g.Roles.Roles { - if role.DeviceID == deviceID { - return &g, nil - } - } + if match != nil { + return match, nil } return nil, ErrGroupNotFound } -// AddGroup saves a new group to disk and returns its generated ID. +// AddGroup saves a new group to disk and returns its generated ID. If the same +// master and LEFT/RIGHT assignments are already stored in the requested +// account, it returns that group unchanged. A device assigned to any other +// stored group in any account is a conflict. func (ds *DataStore) AddGroup(account string, group *models.Group) (string, error) { ds.fileMutex.Lock() defer ds.fileMutex.Unlock() @@ -3214,7 +3538,42 @@ func (ds *DataStore) AddGroup(account string, group *models.Group) (string, erro return "", err } - id := ds.generateGroupID(account) + storedGroups, err := ds.loadAllStoredGroupsNoLock() + if err != nil { + return "", err + } + + var existing *storedGroup + + for i := range storedGroups { + stored := &storedGroups[i] + if stored.account == account && sameStereoPair(group, &stored.group) { + if existing != nil { + return "", fmt.Errorf("%w: stereo pair is stored more than once", ErrGroupMembershipConflict) + } + + existing = stored + + continue + } + + if groupsShareDevice(group, &stored.group) { + return "", fmt.Errorf("%w: a requested device belongs to group %s in account %s", + ErrGroupMembershipConflict, stored.id, stored.account) + } + } + + if existing != nil { + *group = existing.group + + return existing.id, nil + } + + id, err := ds.generateGroupID() + if err != nil { + return "", err + } + group.ID = id data, err := xml.MarshalIndent(group, "", " ") @@ -3260,24 +3619,296 @@ func (ds *DataStore) ModifyGroup(account, groupID, newName string) (*models.Grou return &g, nil } -// DeleteGroup removes a group from disk. +// DeleteGroup retires a group generation. The renamed XML tombstone prevents a +// stale client request from ever matching a later physical generation with the +// same seven-digit ID. func (ds *DataStore) DeleteGroup(account, groupID string) error { ds.fileMutex.Lock() defer ds.fileMutex.Unlock() - err := ds.rootRemove(ds.groupFilePath(account, groupID)) - if os.IsNotExist(err) { - return fmt.Errorf("group %s not found", groupID) + locations, err := ds.loadGroupGenerationLocationsNoLock() + if err != nil { + return err } - return err + generation, found := locations[groupID] + if !found { + return fmt.Errorf("%w: group %s", ErrGroupNotFound, groupID) + } + + if len(generation.active) > 0 && len(generation.retired) > 0 { + return fmt.Errorf("%w: generation %s is both active and retired", ErrGroupDeleteAmbiguous, groupID) + } + + if len(generation.active) == 0 { + return nil + } + + if len(generation.active) != 1 || generation.active[0].account != account { + return fmt.Errorf("%w: generation %s is not uniquely active in account %s", + ErrGroupDeleteAmbiguous, groupID, account) + } + + return ds.retireGroupNoLock(account, groupID, generation.active[0].path) } -// DeleteAllGroupsForAccount removes every Group_*.xml file stored under -// account. Speakers send DELETE /streaming/account/{id}/group/ (no group -// ID) during stereo-pair teardown; since master and slave may live in -// different accounts each speaker deletes its own copy. Returns nil if no -// group files are found — idempotent by design. +// DeleteGroupGenerationForDevice retires exactly one stored group generation +// containing deviceID, regardless of which account currently owns that device. +// The active XML must match expected before the atomic rename. A missing or +// already-retired generation is an idempotent success; every ambiguity fails +// closed. +func (ds *DataStore) DeleteGroupGenerationForDevice( + deviceID string, + groupID string, + expected *models.Group, +) error { + ds.fileMutex.Lock() + defer ds.fileMutex.Unlock() + + if deviceID == "" || groupID == "" { + return fmt.Errorf("%w: device ID and group ID are required", ErrGroupDeleteAmbiguous) + } + + locations, err := ds.loadGroupGenerationLocationsNoLock() + if err != nil { + return err + } + + generation, found := locations[groupID] + if !found { + return nil + } + + if len(generation.active) > 0 && len(generation.retired) > 0 { + return fmt.Errorf("%w: generation %s is both active and retired", ErrGroupDeleteAmbiguous, groupID) + } + + if len(generation.active) == 0 { + return nil + } + + if len(generation.active) != 1 { + return fmt.Errorf("%w: generation %s has %d active files", + ErrGroupDeleteAmbiguous, groupID, len(generation.active)) + } + + if expected == nil || expected.ID != groupID { + return fmt.Errorf("%w: generation %s has no exact expected topology", ErrGroupDeleteAmbiguous, groupID) + } + + if _, expectedContainsDevice := groupDeviceIDs(expected)[deviceID]; !expectedContainsDevice { + return fmt.Errorf("%w: generation %s expected topology does not contain device %s", + ErrGroupDeleteAmbiguous, groupID, deviceID) + } + + groups, err := ds.loadAllStoredGroupsNoLock() + if err != nil { + return err + } + + matches := make([]storedGroup, 0, 1) + + for i := range groups { + if groups[i].id == groupID { + matches = append(matches, groups[i]) + } + } + + switch len(matches) { + case 0: + return fmt.Errorf("%w: active generation %s has no readable group", ErrGroupDeleteAmbiguous, groupID) + case 1: + if matches[0].path != generation.active[0].path { + return fmt.Errorf("%w: active generation %s path does not match", ErrGroupDeleteAmbiguous, groupID) + } + + if _, containsDevice := groupDeviceIDs(&matches[0].group)[deviceID]; !containsDevice { + return fmt.Errorf("%w: generation %s does not contain device %s", + ErrGroupDeleteAmbiguous, groupID, deviceID) + } + + if !sameGroupGeneration(&matches[0].group, expected) { + return fmt.Errorf("%w: generation %s topology does not match", + ErrGroupDeleteAmbiguous, groupID) + } + + return ds.retireGroupNoLock(matches[0].account, groupID, matches[0].path) + default: + return fmt.Errorf("%w: generation %s has %d matches", + ErrGroupDeleteAmbiguous, groupID, len(matches)) + } +} + +// RenameGroupGenerationForDevice renames exactly one active stored group +// generation mastered by deviceID, regardless of which account currently owns +// that device. The active XML topology must match expected, but its current +// name may differ so retries can repair name drift. +func (ds *DataStore) RenameGroupGenerationForDevice( + deviceID string, + groupID string, + expected *models.Group, + newName string, +) (*models.Group, error) { + ds.fileMutex.Lock() + defer ds.fileMutex.Unlock() + + newName = strings.TrimSpace(newName) + if deviceID == "" || groupID == "" || newName == "" { + return nil, fmt.Errorf("%w: device ID, group ID, and new name are required", ErrGroupDeleteAmbiguous) + } + + locations, err := ds.loadGroupGenerationLocationsNoLock() + if err != nil { + return nil, err + } + + generation, found := locations[groupID] + if !found || len(generation.active) == 0 { + return nil, fmt.Errorf("%w: group %s", ErrGroupNotFound, groupID) + } + + if len(generation.active) > 0 && len(generation.retired) > 0 { + return nil, fmt.Errorf("%w: generation %s is both active and retired", ErrGroupDeleteAmbiguous, groupID) + } + + if len(generation.active) != 1 { + return nil, fmt.Errorf("%w: generation %s has %d active files", + ErrGroupDeleteAmbiguous, groupID, len(generation.active)) + } + + if validationErr := validateExpectedGroupGeneration(expected, groupID, deviceID); validationErr != nil { + return nil, validationErr + } + + groups, err := ds.loadAllStoredGroupsNoLock() + if err != nil { + return nil, err + } + + match, err := uniqueStoredGroupForGeneration(groups, groupID) + if err != nil { + return nil, err + } + + if match.path != generation.active[0].path { + return nil, fmt.Errorf("%w: active generation %s path does not match", ErrGroupDeleteAmbiguous, groupID) + } + + if match.group.MasterDeviceID != deviceID { + return nil, fmt.Errorf("%w: generation %s is not mastered by device %s", + ErrGroupDeleteAmbiguous, groupID, deviceID) + } + + expectedWithCurrentName := *expected + + expectedWithCurrentName.Name = match.group.Name + if !sameGroupGeneration(&match.group, &expectedWithCurrentName) { + return nil, fmt.Errorf("%w: generation %s topology does not match", + ErrGroupDeleteAmbiguous, groupID) + } + + match.group.Name = newName + + updated, err := xml.MarshalIndent(&match.group, "", " ") + if err != nil { + return nil, err + } + + if err := ds.atomicWriteFile(match.path, append([]byte(xml.Header), updated...)); err != nil { + return nil, err + } + + return &match.group, nil +} + +// EnsureNoGroupsForDevices verifies that no active stored group contains any +// supplied device ID across all accounts. Callers must supply only device IDs +// freshly verified as physically standalone. This check never mutates storage. +func (ds *DataStore) EnsureNoGroupsForDevices(deviceIDs []string) error { + ds.fileMutex.RLock() + defer ds.fileMutex.RUnlock() + + verified := make(map[string]struct{}, len(deviceIDs)) + for _, deviceID := range deviceIDs { + if deviceID == "" { + return fmt.Errorf("%w: verified device IDs must not be empty", ErrGroupDeleteAmbiguous) + } + + verified[deviceID] = struct{}{} + } + + if len(verified) == 0 { + return fmt.Errorf("%w: at least one verified device ID is required", ErrGroupDeleteAmbiguous) + } + + locations, err := ds.loadGroupGenerationLocationsNoLock() + if err != nil { + return err + } + + for id, generation := range locations { + if len(generation.active) > 1 || len(generation.active) > 0 && len(generation.retired) > 0 { + return fmt.Errorf("%w: generation %s has %d active and %d retired files", + ErrGroupDeleteAmbiguous, id, len(generation.active), len(generation.retired)) + } + } + + groups, err := ds.loadAllStoredGroupsNoLock() + if err != nil { + return err + } + + generations := make([]GroupGeneration, 0) + + for i := range groups { + for deviceID := range groupDeviceIDs(&groups[i].group) { + if _, found := verified[deviceID]; found { + generations = append(generations, GroupGeneration{ + Account: groups[i].account, + ID: groups[i].id, + }) + + break + } + } + } + + if len(generations) == 0 { + return nil + } + + sort.Slice(generations, func(i, j int) bool { + if generations[i].Account == generations[j].Account { + return generations[i].ID < generations[j].ID + } + + return generations[i].Account < generations[j].Account + }) + + return &GroupMembershipConflictError{Generations: generations} +} + +func (ds *DataStore) retireGroupNoLock(account, groupID, activePath string) error { + retiredPath := ds.retiredGroupFilePath(account, groupID) + if _, err := ds.rootStat(retiredPath); err == nil { + return fmt.Errorf("%w: generation %s is already retired", ErrGroupDeleteAmbiguous, groupID) + } else if !os.IsNotExist(err) { + return err + } + + if err := ds.rootRename(activePath, retiredPath); err != nil { + return fmt.Errorf("retire group %s: %w", groupID, err) + } + + ds.rootSyncDir(filepath.Dir(activePath)) + + return nil +} + +// DeleteAllGroupsForAccount removes every Group_*.xml file stored under the +// account. It is reserved for explicit internal or factory-reset workflows, +// not speaker teardown requests without a group ID. Returns nil if no group +// files are found. func (ds *DataStore) DeleteAllGroupsForAccount(account string) error { ds.fileMutex.Lock() defer ds.fileMutex.Unlock() diff --git a/pkg/service/datastore/group_lifecycle_test.go b/pkg/service/datastore/group_lifecycle_test.go new file mode 100644 index 00000000..82d66dc6 --- /dev/null +++ b/pkg/service/datastore/group_lifecycle_test.go @@ -0,0 +1,745 @@ +package datastore + +import ( + "bytes" + "encoding/xml" + "errors" + "os" + "reflect" + "strings" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +func lifecycleTestGroup(master, left, right, name string) models.Group { + return models.Group{ + Name: name, + MasterDeviceID: master, + Roles: models.GroupRoles{Roles: []models.GroupRole{ + {DeviceID: left, Role: "LEFT"}, + {DeviceID: right, Role: "RIGHT"}, + }}, + } +} + +func countLifecycleGroupFiles(t *testing.T, ds *DataStore, account string) int { + t.Helper() + + entries, err := os.ReadDir(ds.AccountDevicesDir(account)) + if err != nil { + if os.IsNotExist(err) { + return 0 + } + + t.Fatalf("read account devices directory: %v", err) + } + + count := 0 + for _, entry := range entries { + if !entry.IsDir() && strings.HasPrefix(entry.Name(), "Group_") && strings.HasSuffix(entry.Name(), ".xml") { + count++ + } + } + + return count +} + +func writeLifecycleGroup(t *testing.T, ds *DataStore, account, groupID string, group models.Group) { + t.Helper() + + group.ID = groupID + + data, err := xml.MarshalIndent(&group, "", " ") + if err != nil { + t.Fatalf("marshal group %s: %v", groupID, err) + } + + if err := ds.rootMkdirAll(ds.AccountDevicesDir(account), 0755); err != nil { + t.Fatalf("create account %s devices directory: %v", account, err) + } + + if err := ds.atomicWriteFile(ds.groupFilePath(account, groupID), append([]byte(xml.Header), data...)); err != nil { + t.Fatalf("write group %s: %v", groupID, err) + } +} + +func TestGroupGenerationReservationsAreGlobal(t *testing.T) { + ds := NewDataStore(t.TempDir()) + + writeLifecycleGroup(t, ds, "ACCOUNT1", "1234567", + lifecycleTestGroup("MASTER1", "MASTER1", "SLAVE1", "Active pair")) + + if err := ds.rootMkdirAll(ds.AccountDevicesDir("ACCOUNT2"), 0755); err != nil { + t.Fatalf("create tombstone account: %v", err) + } + if err := ds.atomicWriteFile(ds.retiredGroupFilePath("ACCOUNT2", "7654321"), []byte("retired\n")); err != nil { + t.Fatalf("write cross-account tombstone: %v", err) + } + + locations, err := ds.loadGroupGenerationLocationsNoLock() + if err != nil { + t.Fatalf("load generation reservations: %v", err) + } + + if got := locations["1234567"]; len(got.active) != 1 || got.active[0].account != "ACCOUNT1" { + t.Fatalf("active reservation = %#v, want ACCOUNT1", got) + } + if got := locations["7654321"]; len(got.retired) != 1 || got.retired[0].account != "ACCOUNT2" { + t.Fatalf("retired reservation = %#v, want ACCOUNT2", got) + } +} + +func TestAddGroupReusesStoredStereoPair(t *testing.T) { + ds := NewDataStore(t.TempDir()) + const account = "ACCOUNT1" + + original := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Original name") + original.SenderIPAddress = "192.0.2.10" + + firstID, err := ds.AddGroup(account, &original) + if err != nil { + t.Fatalf("add original group: %v", err) + } + + retry := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Retry name") + retry.Roles.Roles[0].IPAddress = "198.51.100.10" + + retryID, err := ds.AddGroup(account, &retry) + if err != nil { + t.Fatalf("retry group creation: %v", err) + } + + if retryID != firstID { + t.Fatalf("retry ID = %q, want stored ID %q", retryID, firstID) + } + + if retry.ID != firstID || retry.Name != original.Name || retry.SenderIPAddress != original.SenderIPAddress { + t.Fatalf("retry returned %#v, want unchanged stored group %#v", retry, original) + } + + if got := countLifecycleGroupFiles(t, ds, account); got != 1 { + t.Fatalf("stored group files = %d, want 1", got) + } +} + +func TestAddGroupRejectsExistingDeviceMembership(t *testing.T) { + ds := NewDataStore(t.TempDir()) + const account = "ACCOUNT1" + + original := lifecycleTestGroup("MASTER1", "MASTER1", "SHARED", "First pair") + if _, err := ds.AddGroup(account, &original); err != nil { + t.Fatalf("add original group: %v", err) + } + + conflicting := lifecycleTestGroup("MASTER2", "MASTER2", "SHARED", "Conflicting pair") + _, err := ds.AddGroup(account, &conflicting) + if !errors.Is(err, ErrGroupMembershipConflict) { + t.Fatalf("conflicting add error = %v, want ErrGroupMembershipConflict", err) + } + + if got := countLifecycleGroupFiles(t, ds, account); got != 1 { + t.Fatalf("stored group files = %d after conflict, want 1", got) + } + + if group, getErr := ds.GetGroupForDevice(account, "SHARED"); getErr != nil || group.ID != original.ID { + t.Fatalf("stored group changed after conflict: group=%#v err=%v", group, getErr) + } +} + +func TestAddGroupRejectsCrossAccountMembership(t *testing.T) { + tests := []struct { + name string + requested models.Group + }{ + { + name: "same stereo pair", + requested: lifecycleTestGroup("MASTER1", "MASTER1", "SLAVE1", "Same pair in another account"), + }, + { + name: "shared member", + requested: lifecycleTestGroup("MASTER2", "MASTER2", "SLAVE1", "Conflicting pair in another account"), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + stored := lifecycleTestGroup("MASTER1", "MASTER1", "SLAVE1", "Stored pair") + writeLifecycleGroup(t, ds, "ACCOUNT1", "1234567", stored) + + _, err := ds.AddGroup("ACCOUNT2", &test.requested) + if !errors.Is(err, ErrGroupMembershipConflict) { + t.Fatalf("cross-account add error = %v, want ErrGroupMembershipConflict", err) + } + if got := countLifecycleGroupFiles(t, ds, "ACCOUNT1"); got != 1 { + t.Fatalf("source account group files = %d after conflict, want 1", got) + } + if got := countLifecycleGroupFiles(t, ds, "ACCOUNT2"); got != 0 { + t.Fatalf("requested account group files = %d after conflict, want 0", got) + } + }) + } +} + +func TestAddGroupDoesNotReuseMalformedSuperset(t *testing.T) { + ds := NewDataStore(t.TempDir()) + const account = "ACCOUNT1" + + original := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Pair") + if _, err := ds.AddGroup(account, &original); err != nil { + t.Fatalf("add original group: %v", err) + } + + malformed := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Pair") + malformed.Roles.Roles = append(malformed.Roles.Roles, models.GroupRole{DeviceID: "EXTRA", Role: "CENTER"}) + if _, err := ds.AddGroup(account, &malformed); !errors.Is(err, ErrGroupMembershipConflict) { + t.Fatalf("malformed superset error = %v, want ErrGroupMembershipConflict", err) + } +} + +func TestDeleteGroupGenerationForDevice(t *testing.T) { + t.Run("removes only the exact generation containing the device", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + const ( + account = "ACCOUNT1" + deviceID = "SLAVE1" + ) + + first := lifecycleTestGroup("MASTER1", "MASTER1", "SLAVE1", "First pair") + second := lifecycleTestGroup("MASTER2", "MASTER2", "SLAVE2", "Second pair") + if _, err := ds.AddGroup(account, &first); err != nil { + t.Fatalf("add first group: %v", err) + } + if _, err := ds.AddGroup(account, &second); err != nil { + t.Fatalf("add second group: %v", err) + } + + if err := ds.DeleteGroupGenerationForDevice(deviceID, first.ID, &first); err != nil { + t.Fatalf("delete exact generation: %v", err) + } + + if _, err := ds.GetGroupForDevice(account, deviceID); !errors.Is(err, ErrGroupNotFound) { + t.Fatalf("deleted device lookup error = %v, want ErrGroupNotFound", err) + } + if group, err := ds.GetGroupForDevice(account, "SLAVE2"); err != nil || group.ID != second.ID { + t.Fatalf("unrelated group was not preserved: group=%#v err=%v", group, err) + } + }) + + t.Run("stale generation is an idempotent no-op", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + const account = "ACCOUNT1" + + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Current pair") + if _, err := ds.AddGroup(account, &group); err != nil { + t.Fatalf("add group: %v", err) + } + + if err := ds.DeleteGroupGenerationForDevice("MASTER", "OLDER-ID", nil); err != nil { + t.Fatalf("delete stale generation: %v", err) + } + + if current, err := ds.GetGroupForDevice(account, "MASTER"); err != nil || current.ID != group.ID { + t.Fatalf("current generation changed: group=%#v err=%v", current, err) + } + }) + + t.Run("missing generation is idempotent", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + if err := ds.DeleteGroupGenerationForDevice("MASTER", "PAIR-ID", nil); err != nil { + t.Fatalf("delete missing generation: %v", err) + } + }) + + t.Run("ambiguous duplicate generation fails closed", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + const deviceID = "MASTER" + + first := lifecycleTestGroup(deviceID, deviceID, "SLAVE", "First pair") + if _, err := ds.AddGroup("ACCOUNT1", &first); err != nil { + t.Fatalf("add first group: %v", err) + } + + second := lifecycleTestGroup(deviceID, deviceID, "SLAVE", "Second pair") + writeLifecycleGroup(t, ds, "ACCOUNT2", first.ID, second) + + err := ds.DeleteGroupGenerationForDevice(deviceID, first.ID, &first) + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("ambiguous delete error = %v, want ErrGroupDeleteAmbiguous", err) + } + if got := countLifecycleGroupFiles(t, ds, "ACCOUNT1") + countLifecycleGroupFiles(t, ds, "ACCOUNT2"); got != 2 { + t.Fatalf("stored group files = %d after ambiguity, want 2", got) + } + }) + + t.Run("same ID for another device fails closed", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Current pair") + if _, err := ds.AddGroup("ACCOUNT1", &group); err != nil { + t.Fatalf("add group: %v", err) + } + + err := ds.DeleteGroupGenerationForDevice("OTHER", group.ID, &group) + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("wrong-device delete error = %v, want ErrGroupDeleteAmbiguous", err) + } + if !ds.rootExists(ds.groupFilePath("ACCOUNT1", group.ID)) { + t.Fatal("wrong-device delete retired the active group") + } + }) + + t.Run("submitted topology must match the stored generation", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + stored := lifecycleTestGroup("MASTER", "MASTER", "REAL-SLAVE", "Current pair") + if _, err := ds.AddGroup("ACCOUNT1", &stored); err != nil { + t.Fatalf("add group: %v", err) + } + + submitted := stored + submitted.Roles.Roles = append([]models.GroupRole(nil), stored.Roles.Roles...) + submitted.Roles.Roles[1].DeviceID = "SUBSTITUTE-SLAVE" + err := ds.DeleteGroupGenerationForDevice("MASTER", stored.ID, &submitted) + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("topology mismatch error = %v, want ErrGroupDeleteAmbiguous", err) + } + if !ds.rootExists(ds.groupFilePath("ACCOUNT1", stored.ID)) { + t.Fatal("topology mismatch retired the active group") + } + }) +} + +func TestRenameGroupGenerationForDevice(t *testing.T) { + t.Run("renames an exact generation across accounts", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + + unrelated := lifecycleTestGroup("OTHER-MASTER", "OTHER-MASTER", "OTHER-SLAVE", "Unrelated pair") + if _, err := ds.AddGroup("ACCOUNT1", &unrelated); err != nil { + t.Fatalf("add unrelated group: %v", err) + } + + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Original name") + group.Roles.Roles[0].IPAddress = "192.0.2.10" + group.Roles.Roles[1].IPAddress = "192.0.2.11" + if _, err := ds.AddGroup("ACCOUNT2", &group); err != nil { + t.Fatalf("add group: %v", err) + } + + updated, err := ds.RenameGroupGenerationForDevice("MASTER", group.ID, &group, "Renamed pair") + if err != nil { + t.Fatalf("rename exact generation: %v", err) + } + if updated.ID != group.ID || updated.Name != "Renamed pair" { + t.Fatalf("updated group = %#v, want ID %q and renamed name", updated, group.ID) + } + + stored, err := ds.GetGroupForDevice("ACCOUNT2", "MASTER") + if err != nil || !reflect.DeepEqual(stored, updated) { + t.Fatalf("stored renamed group = %#v err=%v, want %#v", stored, err, updated) + } + if current, err := ds.GetGroupForDevice("ACCOUNT1", "OTHER-MASTER"); err != nil || current.Name != unrelated.Name { + t.Fatalf("unrelated group changed: group=%#v err=%v", current, err) + } + }) + + t.Run("retry allows the stored name to differ from expected", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Original name") + if _, err := ds.AddGroup("ACCOUNT", &group); err != nil { + t.Fatalf("add group: %v", err) + } + + if _, err := ds.RenameGroupGenerationForDevice("MASTER", group.ID, &group, "Renamed pair"); err != nil { + t.Fatalf("first rename: %v", err) + } + + updated, err := ds.RenameGroupGenerationForDevice("MASTER", group.ID, &group, "Renamed pair") + if err != nil { + t.Fatalf("idempotent rename retry: %v", err) + } + if updated.Name != "Renamed pair" { + t.Fatalf("retry returned name %q, want Renamed pair", updated.Name) + } + }) + + t.Run("topology mismatch does not write", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + stored := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Original name") + stored.Roles.Roles[0].IPAddress = "192.0.2.10" + stored.Roles.Roles[1].IPAddress = "192.0.2.11" + if _, err := ds.AddGroup("ACCOUNT", &stored); err != nil { + t.Fatalf("add group: %v", err) + } + + before, err := ds.rootReadFile(ds.groupFilePath("ACCOUNT", stored.ID)) + if err != nil { + t.Fatalf("read group before rename: %v", err) + } + + expected := stored + expected.Roles.Roles = append([]models.GroupRole(nil), stored.Roles.Roles...) + expected.Roles.Roles[1].IPAddress = "198.51.100.11" + _, err = ds.RenameGroupGenerationForDevice("MASTER", stored.ID, &expected, "Renamed pair") + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("topology mismatch error = %v, want ErrGroupDeleteAmbiguous", err) + } + + after, err := ds.rootReadFile(ds.groupFilePath("ACCOUNT", stored.ID)) + if err != nil { + t.Fatalf("read group after rename: %v", err) + } + if !bytes.Equal(after, before) { + t.Fatal("topology mismatch rewrote the active group") + } + }) + + t.Run("unrelated device does not write", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Original name") + if _, err := ds.AddGroup("ACCOUNT", &group); err != nil { + t.Fatalf("add group: %v", err) + } + + before, err := ds.rootReadFile(ds.groupFilePath("ACCOUNT", group.ID)) + if err != nil { + t.Fatalf("read group before rename: %v", err) + } + + _, err = ds.RenameGroupGenerationForDevice("OTHER", group.ID, &group, "Renamed pair") + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("unrelated-device error = %v, want ErrGroupDeleteAmbiguous", err) + } + + after, err := ds.rootReadFile(ds.groupFilePath("ACCOUNT", group.ID)) + if err != nil { + t.Fatalf("read group after rename: %v", err) + } + if !bytes.Equal(after, before) { + t.Fatal("unrelated-device rename rewrote the active group") + } + }) + + t.Run("ambiguous duplicate generation does not write", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Original name") + writeLifecycleGroup(t, ds, "ACCOUNT1", "1234567", group) + writeLifecycleGroup(t, ds, "ACCOUNT2", "1234567", group) + group.ID = "1234567" + + _, err := ds.RenameGroupGenerationForDevice("MASTER", group.ID, &group, "Renamed pair") + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("ambiguous rename error = %v, want ErrGroupDeleteAmbiguous", err) + } + + for _, account := range []string{"ACCOUNT1", "ACCOUNT2"} { + stored, getErr := ds.GetGroupForDevice(account, "MASTER") + if getErr != nil || stored.Name != "Original name" { + t.Fatalf("group in %s changed after ambiguity: group=%#v err=%v", account, stored, getErr) + } + } + }) + + t.Run("empty name is rejected", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Original name") + if _, err := ds.AddGroup("ACCOUNT", &group); err != nil { + t.Fatalf("add group: %v", err) + } + + _, err := ds.RenameGroupGenerationForDevice("MASTER", group.ID, &group, "") + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("empty-name error = %v, want ErrGroupDeleteAmbiguous", err) + } + if current, getErr := ds.GetGroupForDevice("ACCOUNT", "MASTER"); getErr != nil || current.Name != group.Name { + t.Fatalf("group changed after empty name: group=%#v err=%v", current, getErr) + } + }) + + t.Run("non-master device is rejected", func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Original name") + if _, err := ds.AddGroup("ACCOUNT", &group); err != nil { + t.Fatalf("add group: %v", err) + } + + _, err := ds.RenameGroupGenerationForDevice("SLAVE", group.ID, &group, "Renamed pair") + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("non-master error = %v, want ErrGroupDeleteAmbiguous", err) + } + if current, getErr := ds.GetGroupForDevice("ACCOUNT", "MASTER"); getErr != nil || current.Name != group.Name { + t.Fatalf("group changed after non-master rename: group=%#v err=%v", current, getErr) + } + }) +} + +func TestEnsureNoGroupsForDevicesReportsStaleGroupsAcrossAccountsWithoutMutation(t *testing.T) { + ds := NewDataStore(t.TempDir()) + const ( + firstID = "1234567" + secondID = "7654321" + ) + + first := lifecycleTestGroup("MOVED", "MOVED", "OLD-SLAVE-1", "First stale pair") + writeLifecycleGroup(t, ds, "ACCOUNT1", firstID, first) + + second := lifecycleTestGroup("MOVED", "MOVED", "OLD-SLAVE-2", "Second stale pair") + writeLifecycleGroup(t, ds, "ACCOUNT2", secondID, second) + + unrelated := lifecycleTestGroup("OTHER-MASTER", "OTHER-MASTER", "OTHER-SLAVE", "Unrelated pair") + if _, err := ds.AddGroup("ACCOUNT3", &unrelated); err != nil { + t.Fatalf("add unrelated group: %v", err) + } + + if firstID == unrelated.ID || secondID == unrelated.ID { + t.Fatalf("active generation IDs are not globally unique: %q %q %q", firstID, secondID, unrelated.ID) + } + + firstBefore, err := ds.rootReadFile(ds.groupFilePath("ACCOUNT1", firstID)) + if err != nil { + t.Fatalf("read first group before check: %v", err) + } + secondBefore, err := ds.rootReadFile(ds.groupFilePath("ACCOUNT2", secondID)) + if err != nil { + t.Fatalf("read second group before check: %v", err) + } + + err = ds.EnsureNoGroupsForDevices([]string{"MOVED"}) + if !errors.Is(err, ErrGroupMembershipConflict) { + t.Fatalf("cross-account check error = %v, want ErrGroupMembershipConflict", err) + } + + var conflict *GroupMembershipConflictError + if !errors.As(err, &conflict) { + t.Fatalf("cross-account check error type = %T, want *GroupMembershipConflictError", err) + } + wantGenerations := []GroupGeneration{ + {Account: "ACCOUNT1", ID: firstID}, + {Account: "ACCOUNT2", ID: secondID}, + } + if !reflect.DeepEqual(conflict.Generations, wantGenerations) { + t.Fatalf("conflicting generations = %#v, want %#v", conflict.Generations, wantGenerations) + } + + firstAfter, err := ds.rootReadFile(ds.groupFilePath("ACCOUNT1", firstID)) + if err != nil { + t.Fatalf("read first group after check: %v", err) + } + secondAfter, err := ds.rootReadFile(ds.groupFilePath("ACCOUNT2", secondID)) + if err != nil { + t.Fatalf("read second group after check: %v", err) + } + if !bytes.Equal(firstAfter, firstBefore) || !bytes.Equal(secondAfter, secondBefore) { + t.Fatal("read-only group check changed active group data") + } + if ds.rootExists(ds.retiredGroupFilePath("ACCOUNT1", firstID)) || + ds.rootExists(ds.retiredGroupFilePath("ACCOUNT2", secondID)) { + t.Fatal("read-only group check created a tombstone") + } + if got := countLifecycleGroupFiles(t, ds, "ACCOUNT3"); got != 1 { + t.Fatalf("unrelated active group files = %d, want 1", got) + } +} + +func TestRetireGroupAtomicallyRenamesActiveXML(t *testing.T) { + ds := NewDataStore(t.TempDir()) + const ( + account = "ACCOUNT1" + groupID = "1234567" + ) + + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Pair") + writeLifecycleGroup(t, ds, account, groupID, group) + + activePath := ds.groupFilePath(account, groupID) + retiredPath := ds.retiredGroupFilePath(account, groupID) + activeInfo, err := os.Stat(activePath) + if err != nil { + t.Fatalf("stat active group: %v", err) + } + activeXML, err := os.ReadFile(activePath) + if err != nil { + t.Fatalf("read active group: %v", err) + } + + if err := ds.DeleteGroup(account, groupID); err != nil { + t.Fatalf("retire group: %v", err) + } + + if _, err := os.Stat(activePath); !os.IsNotExist(err) { + t.Fatalf("active path stat error = %v, want not exist", err) + } + retiredInfo, err := os.Stat(retiredPath) + if err != nil { + t.Fatalf("stat retired group: %v", err) + } + if !os.SameFile(activeInfo, retiredInfo) { + t.Fatal("retired group is not the renamed active file") + } + retiredXML, err := os.ReadFile(retiredPath) + if err != nil { + t.Fatalf("read retired group: %v", err) + } + if !bytes.Equal(retiredXML, activeXML) { + t.Fatal("retired group did not preserve the active XML contents") + } +} + +func TestEnsureNoGroupsForDevicesRejectsActiveTombstoneAmbiguity(t *testing.T) { + ds := NewDataStore(t.TempDir()) + + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Ambiguous pair") + if _, err := ds.AddGroup("ACCOUNT1", &group); err != nil { + t.Fatalf("add active group: %v", err) + } + + if err := ds.rootMkdirAll(ds.AccountDevicesDir("ACCOUNT2"), 0755); err != nil { + t.Fatalf("create tombstone account: %v", err) + } + if err := ds.atomicWriteFile(ds.retiredGroupFilePath("ACCOUNT2", group.ID), []byte("retired\n")); err != nil { + t.Fatalf("write conflicting tombstone: %v", err) + } + + err := ds.EnsureNoGroupsForDevices([]string{"MASTER"}) + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("ambiguous check error = %v, want ErrGroupDeleteAmbiguous", err) + } + if !ds.rootExists(ds.groupFilePath("ACCOUNT1", group.ID)) { + t.Fatal("ambiguous check removed the active group") + } + if _, readErr := ds.rootReadFile(ds.retiredGroupFilePath("ACCOUNT2", group.ID)); readErr != nil { + t.Fatalf("ambiguous check changed the tombstone: %v", readErr) + } +} + +func TestGroupReadsFailClosedOnMalformedOrUnreadableData(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T, ds *DataStore) string + }{ + { + name: "malformed XML", + setup: func(t *testing.T, ds *DataStore) string { + t.Helper() + + path := ds.groupFilePath("ACCOUNT1", "1234567") + if err := ds.rootMkdirAll(ds.AccountDevicesDir("ACCOUNT1"), 0755); err != nil { + t.Fatalf("create account directory: %v", err) + } + if err := ds.atomicWriteFile(path, []byte("")); err != nil { + t.Fatalf("write malformed group: %v", err) + } + + return path + }, + }, + { + name: "unreadable group", + setup: func(t *testing.T, ds *DataStore) string { + t.Helper() + + if err := ds.rootMkdirAll(ds.AccountDevicesDir("ACCOUNT1"), 0755); err != nil { + t.Fatalf("create account directory: %v", err) + } + path := ds.groupFilePath("ACCOUNT1", "1234567") + if err := os.Symlink("missing-group-target", path); err != nil { + t.Fatalf("create unreadable group symlink: %v", err) + } + + return path + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ds := NewDataStore(t.TempDir()) + path := test.setup(t, ds) + + if err := ds.EnsureNoGroupsForDevices([]string{"MASTER"}); err == nil { + t.Fatal("EnsureNoGroupsForDevices error = nil, want datastore error") + } + if _, err := ds.GetGroupForDevice("ACCOUNT1", "MASTER"); err == nil || errors.Is(err, ErrGroupNotFound) { + t.Fatalf("GetGroupForDevice error = %v, want datastore error", err) + } + if _, err := os.Lstat(path); err != nil { + t.Fatalf("fail-closed reads mutated group path: %v", err) + } + }) + } +} + +func TestGetGroupForDeviceFailsClosedOnDuplicateMembership(t *testing.T) { + ds := NewDataStore(t.TempDir()) + first := lifecycleTestGroup("MASTER1", "MASTER1", "SHARED", "First pair") + second := lifecycleTestGroup("MASTER2", "MASTER2", "SHARED", "Second pair") + + writeLifecycleGroup(t, ds, "ACCOUNT1", "1234567", first) + writeLifecycleGroup(t, ds, "ACCOUNT1", "7654321", second) + + group, err := ds.GetGroupForDevice("ACCOUNT1", "SHARED") + if group != nil || !errors.Is(err, ErrGroupMembershipConflict) { + t.Fatalf("group=%#v error=%v, want membership conflict", group, err) + } +} + +func TestDeleteGroupClassifiesMissingAndAmbiguousGenerations(t *testing.T) { + ds := NewDataStore(t.TempDir()) + + if err := ds.DeleteGroup("ACCOUNT1", "1234567"); !errors.Is(err, ErrGroupNotFound) { + t.Fatalf("missing delete error = %v, want ErrGroupNotFound", err) + } + + group := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Ambiguous pair") + writeLifecycleGroup(t, ds, "ACCOUNT1", "7654321", group) + if err := ds.atomicWriteFile(ds.retiredGroupFilePath("ACCOUNT1", "7654321"), []byte("retired\n")); err != nil { + t.Fatalf("write conflicting tombstone: %v", err) + } + + err := ds.DeleteGroup("ACCOUNT1", "7654321") + if !errors.Is(err, ErrGroupDeleteAmbiguous) { + t.Fatalf("ambiguous delete error = %v, want ErrGroupDeleteAmbiguous", err) + } + if !ds.rootExists(ds.groupFilePath("ACCOUNT1", "7654321")) { + t.Fatal("ambiguous delete removed the active group") + } +} + +func TestRetiredStereoPairGetsFreshGeneration(t *testing.T) { + ds := NewDataStore(t.TempDir()) + const account = "ACCOUNT1" + + first := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Pair") + firstID, err := ds.AddGroup(account, &first) + if err != nil { + t.Fatalf("add first generation: %v", err) + } + if err := ds.DeleteGroupGenerationForDevice("MASTER", firstID, &first); err != nil { + t.Fatalf("retire first generation: %v", err) + } + if !ds.rootExists(ds.retiredGroupFilePath(account, firstID)) { + t.Fatalf("retired generation %q has no tombstone", firstID) + } + tombstone, err := ds.rootReadFile(ds.retiredGroupFilePath(account, firstID)) + if err != nil { + t.Fatalf("read retired generation: %v", err) + } + var retired models.Group + if err := xml.Unmarshal(tombstone, &retired); err != nil { + t.Fatalf("retired generation does not contain group XML: %v", err) + } + if retired.ID != firstID { + t.Fatalf("retired generation ID = %q, want %q", retired.ID, firstID) + } + if err := ds.DeleteGroup(account, firstID); err != nil { + t.Fatalf("repeat exact generation delete should be idempotent: %v", err) + } + + second := lifecycleTestGroup("MASTER", "MASTER", "SLAVE", "Pair") + secondID, err := ds.AddGroup(account, &second) + if err != nil { + t.Fatalf("add second generation: %v", err) + } + if secondID == firstID { + t.Fatalf("new physical generation reused retired ID %q", firstID) + } +} diff --git a/pkg/service/handlers/handlers_marge.go b/pkg/service/handlers/handlers_marge.go index 1f763ce1..15413ab3 100644 --- a/pkg/service/handlers/handlers_marge.go +++ b/pkg/service/handlers/handlers_marge.go @@ -4,6 +4,7 @@ import ( "crypto/rand" "crypto/sha256" "encoding/xml" + "errors" "fmt" "io" "log" @@ -819,17 +820,21 @@ func (s *Server) HandleMargeDeviceGroup(w http.ResponseWriter, r *http.Request) group, err := s.ds.GetGroupForDevice(account, device) if err != nil { - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(constants.XMLHeader + ``)) + if errors.Is(err, datastore.ErrGroupNotFound) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(constants.XMLHeader + ``)) + + return + } + + http.Error(w, err.Error(), http.StatusInternalServerError) return } data, err := xml.Marshal(group) if err != nil { - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(constants.XMLHeader + ``)) - + http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -870,7 +875,13 @@ func (s *Server) HandleMargeAddGroup(w http.ResponseWriter, r *http.Request) { id, err := s.ds.AddGroup(account, &group) if err != nil { + if errors.Is(err, datastore.ErrGroupMembershipConflict) { + http.Error(w, err.Error(), http.StatusConflict) + return + } + http.Error(w, err.Error(), http.StatusInternalServerError) + return } @@ -937,7 +948,15 @@ func (s *Server) HandleMargeDeleteGroup(w http.ResponseWriter, r *http.Request) } if err := s.ds.DeleteGroup(account, groupID); err != nil { - http.Error(w, err.Error(), http.StatusNotFound) + switch { + case errors.Is(err, datastore.ErrGroupNotFound): + http.Error(w, err.Error(), http.StatusNotFound) + case errors.Is(err, datastore.ErrGroupDeleteAmbiguous): + http.Error(w, err.Error(), http.StatusConflict) + default: + http.Error(w, err.Error(), http.StatusInternalServerError) + } + return } @@ -946,10 +965,10 @@ func (s *Server) HandleMargeDeleteGroup(w http.ResponseWriter, r *http.Request) _, _ = w.Write([]byte(constants.XMLHeader + `Group deleted successfully`)) } -// HandleMargeDeleteAccountGroups removes all stereo groups stored for an -// account. Speakers send DELETE /streaming/account/{id}/group/ (trailing -// slash, no group ID) during stereo-pair teardown. Master and slave often -// live in different accounts, so each speaker deletes its own copy here. +// 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. func (s *Server) HandleMargeDeleteAccountGroups(w http.ResponseWriter, r *http.Request) { account := chi.URLParam(r, "account") @@ -958,14 +977,9 @@ 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 deleted successfully`)) + _, _ = w.Write([]byte(constants.XMLHeader + `Group teardown acknowledged`)) } // HandleMusicProviderIsEligible returns the music provider eligibility. diff --git a/pkg/service/handlers/handlers_marge_group_lifecycle_test.go b/pkg/service/handlers/handlers_marge_group_lifecycle_test.go new file mode 100644 index 00000000..08c9e512 --- /dev/null +++ b/pkg/service/handlers/handlers_marge_group_lifecycle_test.go @@ -0,0 +1,267 @@ +package handlers + +import ( + "encoding/xml" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/constants" + "github.com/gesellix/bose-soundtouch/pkg/service/datastore" + "github.com/go-chi/chi/v5" +) + +func margeLifecycleRouter(ds *datastore.DataStore) http.Handler { + server := NewServer(ds, nil, "http://localhost:8001", false, false, false) + router := chi.NewRouter() + router.Use(clientIPMiddleware(false, nil, nil)) + router.Get("/streaming/account/{account}/device/{device}/group", server.HandleMargeDeviceGroup) + router.Post("/streaming/account/{account}/group/", server.HandleMargeAddGroup) + router.Delete("/streaming/account/{account}/group/", server.HandleMargeDeleteAccountGroups) + router.Delete("/streaming/account/{account}/group/{groupId}", server.HandleMargeDeleteGroup) + + return router +} + +func margeLifecycleGroupXML(master, left, right, name string) string { + return fmt.Sprintf(`%s%s`+ + `%sLEFT`+ + `%sRIGHT`+ + ``, name, master, left, right) +} + +func margeLifecycleRequest(t *testing.T, handler http.Handler, method, path, remoteAddr, body string) *httptest.ResponseRecorder { + t.Helper() + + request := httptest.NewRequest(method, path, strings.NewReader(body)) + request.RemoteAddr = remoteAddr + request.Header.Set("Content-Type", "application/vnd.bose.streaming-v1.2+xml") + recorder := httptest.NewRecorder() + handler.ServeHTTP(recorder, request) + + return recorder +} + +func margeLifecycleGroup(master, left, right, name string) models.Group { + return models.Group{ + Name: name, + MasterDeviceID: master, + Roles: models.GroupRoles{Roles: []models.GroupRole{ + {DeviceID: left, Role: "LEFT"}, + {DeviceID: right, Role: "RIGHT"}, + }}, + } +} + +func countMargeLifecycleGroupFiles(t *testing.T, ds *datastore.DataStore, account string) int { + t.Helper() + + entries, err := os.ReadDir(ds.AccountDevicesDir(account)) + if err != nil { + if os.IsNotExist(err) { + return 0 + } + + t.Fatalf("read account devices directory: %v", err) + } + + count := 0 + for _, entry := range entries { + if !entry.IsDir() && strings.HasPrefix(entry.Name(), "Group_") && strings.HasSuffix(entry.Name(), ".xml") { + count++ + } + } + + return count +} + +func TestMargeAddGroupRetryReusesStoredGroup(t *testing.T) { + ds := datastore.NewDataStore(t.TempDir()) + handler := margeLifecycleRouter(ds) + const ( + account = "ACCOUNT1" + path = "/streaming/account/" + account + "/group/" + ) + + first := margeLifecycleRequest(t, handler, http.MethodPost, path, "192.0.2.10:1234", + margeLifecycleGroupXML("MASTER", "MASTER", "SLAVE", "Original name")) + if first.Code != http.StatusCreated { + t.Fatalf("first POST status = %d, want 201; body=%s", first.Code, first.Body.String()) + } + + var firstGroup models.Group + if err := xml.Unmarshal(first.Body.Bytes(), &firstGroup); err != nil { + t.Fatalf("decode first response: %v; body=%s", err, first.Body.String()) + } + firstLocation := first.Header().Get("Location") + if firstGroup.ID == "" || !strings.HasSuffix(firstLocation, "/group/"+firstGroup.ID) { + t.Fatalf("first response ID=%q Location=%q", firstGroup.ID, firstLocation) + } + + retry := margeLifecycleRequest(t, handler, http.MethodPost, path, "192.0.2.10:1234", + margeLifecycleGroupXML("MASTER", "MASTER", "SLAVE", "Retry name")) + if retry.Code != http.StatusCreated { + t.Fatalf("retry POST status = %d, want 201; body=%s", retry.Code, retry.Body.String()) + } + + var retryGroup models.Group + if err := xml.Unmarshal(retry.Body.Bytes(), &retryGroup); err != nil { + t.Fatalf("decode retry response: %v; body=%s", err, retry.Body.String()) + } + if retryGroup.ID != firstGroup.ID || retryGroup.Name != firstGroup.Name { + t.Fatalf("retry group = %#v, want stored group %#v", retryGroup, firstGroup) + } + if got := retry.Header().Get("Location"); got != firstLocation { + t.Fatalf("retry Location = %q, want %q", got, firstLocation) + } + if got := retry.Header().Get("Content-Type"); got != "application/vnd.bose.streaming-v1.2+xml" { + t.Fatalf("retry Content-Type = %q", got) + } + if got := countMargeLifecycleGroupFiles(t, ds, account); got != 1 { + t.Fatalf("stored group files = %d, want 1", got) + } +} + +func TestMargeAddGroupMembershipConflictReturns409(t *testing.T) { + ds := datastore.NewDataStore(t.TempDir()) + handler := margeLifecycleRouter(ds) + const ( + account = "ACCOUNT1" + path = "/streaming/account/" + account + "/group/" + ) + + first := margeLifecycleRequest(t, handler, http.MethodPost, path, "192.0.2.10:1234", + margeLifecycleGroupXML("MASTER1", "MASTER1", "SHARED", "First pair")) + if first.Code != http.StatusCreated { + t.Fatalf("first POST status = %d, want 201; body=%s", first.Code, first.Body.String()) + } + + conflict := margeLifecycleRequest(t, handler, http.MethodPost, path, "192.0.2.20:1234", + margeLifecycleGroupXML("MASTER2", "MASTER2", "SHARED", "Conflicting pair")) + if conflict.Code != http.StatusConflict { + t.Fatalf("conflicting POST status = %d, want 409; body=%s", conflict.Code, conflict.Body.String()) + } + if got := countMargeLifecycleGroupFiles(t, ds, account); got != 1 { + t.Fatalf("stored group files = %d after conflict, want 1", got) + } +} + +func TestMargeDeviceGroupReturnsEmptyGroupOnlyWhenNotFound(t *testing.T) { + ds := datastore.NewDataStore(t.TempDir()) + response := margeLifecycleRequest(t, margeLifecycleRouter(ds), http.MethodGet, + "/streaming/account/ACCOUNT1/device/MASTER/group", "192.0.2.10:1234", "") + if response.Code != http.StatusOK { + t.Fatalf("missing group GET status = %d, want 200; body=%s", response.Code, response.Body.String()) + } + if got := response.Body.String(); got != constants.XMLHeader+`` { + t.Fatalf("missing group GET body = %q, want empty group", got) + } +} + +func TestMargeDeviceGroupReturns500ForMalformedOrUnreadableData(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T, path string) + }{ + { + name: "malformed XML", + setup: func(t *testing.T, path string) { + t.Helper() + + if err := os.WriteFile(path, []byte(""), 0600); err != nil { + t.Fatalf("write malformed group: %v", err) + } + }, + }, + { + name: "unreadable group", + setup: func(t *testing.T, path string) { + t.Helper() + + if err := os.Symlink("missing-group-target", path); err != nil { + t.Fatalf("create unreadable group symlink: %v", err) + } + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ds := datastore.NewDataStore(t.TempDir()) + if err := os.MkdirAll(ds.AccountDevicesDir("ACCOUNT1"), 0755); err != nil { + t.Fatalf("create account directory: %v", err) + } + test.setup(t, filepath.Join(ds.AccountDevicesDir("ACCOUNT1"), "Group_1234567.xml")) + + response := margeLifecycleRequest(t, margeLifecycleRouter(ds), http.MethodGet, + "/streaming/account/ACCOUNT1/device/MASTER/group", "192.0.2.10:1234", "") + if response.Code != http.StatusInternalServerError { + t.Fatalf("invalid group GET status = %d, want 500; body=%s", response.Code, response.Body.String()) + } + if strings.Contains(response.Body.String(), "") { + t.Fatalf("invalid group GET returned empty-group success: %s", response.Body.String()) + } + }) + } +} + +func TestMargeDeleteAccountGroupsAcknowledgesWithoutDeleting(t *testing.T) { + ds := datastore.NewDataStore(t.TempDir()) + handler := margeLifecycleRouter(ds) + const account = "ACCOUNT1" + + group := margeLifecycleGroup("MASTER", "MASTER", "SLAVE", "Current pair") + if _, err := ds.AddGroup(account, &group); err != nil { + t.Fatalf("add group: %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) + } +} + +func TestMargeDeleteGroupDoesNotHideAmbiguousActiveGeneration(t *testing.T) { + baseDir := t.TempDir() + ds := datastore.NewDataStore(baseDir) + handler := margeLifecycleRouter(ds) + const account = "ACCOUNT1" + + group := margeLifecycleGroup("MASTER", "MASTER", "SLAVE", "Current pair") + if _, err := ds.AddGroup(account, &group); err != nil { + t.Fatalf("add group: %v", err) + } + + retiredPath := filepath.Join(ds.AccountDevicesDir(account), "Group_"+group.ID+".retired") + if err := os.WriteFile(retiredPath, []byte("retired\n"), 0600); err != nil { + t.Fatalf("create conflicting tombstone: %v", err) + } + + response := margeLifecycleRequest(t, handler, http.MethodDelete, + "/streaming/account/"+account+"/group/"+group.ID, "192.0.2.10:1234", "") + if response.Code != http.StatusConflict { + t.Fatalf("ambiguous DELETE status = %d, want 409; body=%s", response.Code, response.Body.String()) + } + if current, err := ds.GetGroupForDevice(account, "MASTER"); err != nil || current.ID != group.ID { + t.Fatalf("ambiguous DELETE changed active generation: group=%#v err=%v", current, err) + } +} + +func TestMargeDeleteMissingGroupReturns404(t *testing.T) { + ds := datastore.NewDataStore(t.TempDir()) + response := margeLifecycleRequest(t, margeLifecycleRouter(ds), http.MethodDelete, + "/streaming/account/ACCOUNT1/group/MISSING", "192.0.2.10:1234", "") + if response.Code != http.StatusNotFound { + t.Fatalf("missing DELETE status = %d, want 404; body=%s", response.Code, response.Body.String()) + } +} diff --git a/pkg/service/soundtouchweb/handler.go b/pkg/service/soundtouchweb/handler.go index e027b07b..6cab50de 100644 --- a/pkg/service/soundtouchweb/handler.go +++ b/pkg/service/soundtouchweb/handler.go @@ -14,10 +14,12 @@ import ( "sync/atomic" "time" + "github.com/gesellix/bose-soundtouch/pkg/client" "github.com/gesellix/bose-soundtouch/pkg/models" bmxpkg "github.com/gesellix/bose-soundtouch/pkg/service/bmx" "github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes" "github.com/gesellix/bose-soundtouch/pkg/service/stations" + "github.com/gesellix/bose-soundtouch/pkg/stereopair" "github.com/go-chi/chi/v5" "github.com/gorilla/websocket" ) @@ -42,6 +44,14 @@ type WebApp struct { // connection, unlike a single application-wide write lock would. WSClients map[*websocket.Conn]*sync.Mutex WSMutex sync.RWMutex + // DeviceWSClients mirrors WSClients but for HandleDeviceWebSocket's + // per-device status connections (see withDeviceConnWrite). Keyed by the + // webSocketWriter interface rather than *websocket.Conn so tests can + // register a mock writer the same way production code registers a real + // connection. awaitPriorGlobalWebSocketWrites barriers against both + // pools uniformly. + DeviceWSClients map[webSocketWriter]*sync.Mutex + DeviceWSMutex sync.RWMutex // discoveryPublishMu serializes discoveryStatus publications against // each other only (Store + client snapshot + fan-out stay ordered across // concurrent BroadcastDiscoveryStatus calls). It is deliberately NOT @@ -111,6 +121,11 @@ type WebApp struct { // SeedExtraDevices never probe the same still-offline host concurrently. seedMu sync.Mutex + // StereoPairs coordinates persistent ST10 stereo-pair mutations across + // both physical speakers. It is shared for the lifetime of WebApp so its + // mutation lock covers concurrent CLI-like requests from every browser. + StereoPairs StereoPairLifecycle + discoveryStatus atomic.Value // stores *webtypes.DiscoveryStatus } @@ -147,13 +162,61 @@ type DeviceEntry struct { // NewWebApp creates a new WebApp instance for SPA mode func NewWebApp() *WebApp { - return &WebApp{ - devices: make(map[string]*webtypes.DeviceConnection), - WSClients: make(map[*websocket.Conn]*sync.Mutex), + app := &WebApp{ + devices: make(map[string]*webtypes.DeviceConnection), + WSClients: make(map[*websocket.Conn]*sync.Mutex), + DeviceWSClients: make(map[webSocketWriter]*sync.Mutex), Upgrader: websocket.Upgrader{ 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) + }, + ) + + return app +} + +func (app *WebApp) stereoPairPersistenceClient() *http.Client { + base := app.serviceHTTPClient() + if base.Timeout >= stereopair.RequestTimeout { + return base + } + + configured := *base + configured.Timeout = stereopair.RequestTimeout + + return &configured +} + +// SetStereoPairGenerationPersistence overrides both exact post-teardown +// retirement and the read-only pre-create generation barrier. +func (app *WebApp) SetStereoPairGenerationPersistence( + cleanup stereopair.GenerationCleanup, + preflight stereopair.GenerationPreflight, + rename stereopair.GenerationRename, +) { + app.StereoPairs = stereopair.NewWithGenerationLifecyclePersistence(app.stereoPairClient, cleanup, preflight, rename) +} + +// stereoPairClient uses a dedicated long-timeout client. Pair creation can +// legitimately span multiple 15-second speaker/Marge retry cycles, while the +// ordinary status clients intentionally use a shorter timeout. +func (app *WebApp) stereoPairClient(host string) (stereopair.Client, error) { + if strings.TrimSpace(host) == "" { + return nil, fmt.Errorf("speaker host is empty") + } + + return client.NewClient(&client.Config{Host: host, Timeout: stereopair.RequestTimeout}), nil } // GetDevice returns the device for id and whether it exists. diff --git a/pkg/service/soundtouchweb/handler_stereopair.go b/pkg/service/soundtouchweb/handler_stereopair.go new file mode 100644 index 00000000..a048f841 --- /dev/null +++ b/pkg/service/soundtouchweb/handler_stereopair.go @@ -0,0 +1,404 @@ +package soundtouchweb + +import ( + "encoding/json" + "errors" + "log" + "net" + "net/http" + "strings" + "time" + + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes" + "github.com/gesellix/bose-soundtouch/pkg/stereopair" + "github.com/go-chi/chi/v5" +) + +// StereoPairLifecycle is the shared mutation boundary used by the web player. +// The concrete coordinator serializes operations and verifies both speakers; +// the interface keeps HTTP tests independent from physical devices. +type StereoPairLifecycle interface { + Inspect(memberIPAddress string) (stereopair.Result, error) + Create(req stereopair.CreateRequest) (stereopair.Result, error) + Rename(req stereopair.RenameRequest) (stereopair.Result, error) + Dissolve(req stereopair.DissolveRequest) (stereopair.Result, error) +} + +type stereoPairRequest struct { + RightID string `json:"rightId"` + GroupID string `json:"groupId"` + Name string `json:"name"` + Group *models.Group `json:"group,omitempty"` +} + +type stereoPairResponse struct { + Operation string `json:"operation"` + Status string `json:"status"` + Capable bool `json:"capable"` + Paired bool `json:"paired"` + Group *models.Group `json:"group,omitempty"` + Members []stereoPairMemberResponse `json:"members,omitempty"` + PersistenceAttempted bool `json:"persistenceAttempted,omitempty"` + PersistenceComplete bool `json:"persistenceComplete,omitempty"` + PersistenceError string `json:"persistenceError,omitempty"` +} + +type stereoPairMemberResponse struct { + IPAddress string `json:"ipAddress"` + DeviceID string `json:"deviceId,omitempty"` + Reachable bool `json:"reachable"` + Verified bool `json:"verified"` + Group *models.Group `json:"group,omitempty"` + PreflightError string `json:"preflightError,omitempty"` + MutationError string `json:"mutationError,omitempty"` + VerificationError string `json:"verificationError,omitempty"` + CompensationError string `json:"compensationError,omitempty"` + CompensationVerified bool `json:"compensationVerified,omitempty"` +} + +// HandleGetStereoPair returns a fresh speaker-backed view of one pair or +// standalone ST10. Cached player projection is deliberately not consulted. +func (app *WebApp) HandleGetStereoPair(w http.ResponseWriter, r *http.Request) { + host, conn, ok := app.stereoPairDevice(w, r) + if !ok { + return + } + + result, err := app.StereoPairs.Inspect(host) + app.writeStereoPairResult(w, conn.DeviceInfo, result, err) +} + +// HandleCreateStereoPair makes {id} the LEFT/master and rightId the +// RIGHT/member. The coordinator repeats every precondition against both +// physical speakers immediately before mutation. +func (app *WebApp) HandleCreateStereoPair(w http.ResponseWriter, r *http.Request) { + leftHost, left, ok := app.stereoPairDevice(w, r) + if !ok { + return + } + + var req stereoPairRequest + if err := decodeStereoPairRequest(r, &req); err != nil { + app.sendError(w, err.Error(), http.StatusBadRequest) + return + } + + if req.RightID == "" { + app.sendError(w, "rightId is required", http.StatusBadRequest) + return + } + + right, exists := app.GetDevice(req.RightID) + if !exists || right.DeviceInfo == nil { + app.sendError(w, "Right speaker not found", http.StatusNotFound) + return + } + + rightHost, err := stereoPairIPAddress(req.RightID, right) + if err != nil { + app.sendError(w, err.Error(), http.StatusConflict) + return + } + + result, err := app.StereoPairs.Create(stereopair.CreateRequest{ + LeftIPAddress: leftHost, + RightIPAddress: rightHost, + Name: req.Name, + }) + app.completeStereoPairMutation(w, left.DeviceInfo, result, err) +} + +// HandleRenameStereoPair updates the full pair on both physical speakers and +// succeeds only after both fresh reads agree on the new name. +func (app *WebApp) HandleRenameStereoPair(w http.ResponseWriter, r *http.Request) { + host, conn, ok := app.stereoPairDevice(w, r) + if !ok { + return + } + + var req stereoPairRequest + if err := decodeStereoPairRequest(r, &req); err != nil { + app.sendError(w, err.Error(), http.StatusBadRequest) + return + } + + if req.Name == "" { + app.sendError(w, "name is required", http.StatusBadRequest) + return + } + + if req.GroupID == "" { + app.sendError(w, "groupId is required", http.StatusBadRequest) + return + } + + result, err := app.StereoPairs.Rename(stereopair.RenameRequest{ + MemberIPAddress: host, + ExpectedGroupID: req.GroupID, + Name: req.Name, + }) + app.completeStereoPairMutation(w, conn.DeviceInfo, result, err) +} + +// HandleDissolveStereoPair removes the pair from both preflighted members and +// reports a degraded outcome rather than hiding a partial teardown. +func (app *WebApp) HandleDissolveStereoPair(w http.ResponseWriter, r *http.Request) { + host, conn, ok := app.stereoPairDevice(w, r) + if !ok { + return + } + + var req stereoPairRequest + if err := decodeStereoPairRequest(r, &req); err != nil { + app.sendError(w, err.Error(), http.StatusBadRequest) + return + } + + if req.GroupID == "" { + app.sendError(w, "groupId is required", http.StatusBadRequest) + return + } + + if err := app.validateStereoPairRecoverySnapshot(req.Group); err != nil { + app.sendError(w, err.Error(), http.StatusConflict) + return + } + + result, err := app.StereoPairs.Dissolve(stereopair.DissolveRequest{ + MemberIPAddress: host, + ExpectedGroupID: req.GroupID, + ExpectedGroup: req.Group, + }) + app.completeStereoPairMutation(w, conn.DeviceInfo, result, err) +} + +func (app *WebApp) validateStereoPairRecoverySnapshot(group *models.Group) error { + if group == nil { + return nil + } + + for i := range group.Roles.Roles { + role := &group.Roles.Roles[i] + if net.ParseIP(role.IPAddress) == nil { + return errors.New("recovery snapshot has an invalid member IP address") + } + + conn, found := app.deviceByStereoPairIPAddress(role.IPAddress) + if !found || conn == nil || conn.DeviceInfo == nil || conn.DeviceInfo.DeviceID != role.DeviceID { + return errors.New("recovery snapshot does not match the registered speakers") + } + } + + return nil +} + +func (app *WebApp) stereoPairDevice(w http.ResponseWriter, r *http.Request) (string, *webtypes.DeviceConnection, bool) { + deviceID := strings.TrimSpace(chi.URLParam(r, "id")) + if deviceID == "" { + app.sendError(w, "Device ID required", http.StatusBadRequest) + return "", nil, false + } + + if app.StereoPairs == nil { + app.sendError(w, "Stereo-pair lifecycle is unavailable", http.StatusServiceUnavailable) + return "", nil, false + } + + conn, ok := app.GetDevice(deviceID) + if !ok || conn.DeviceInfo == nil { + app.sendError(w, "Device not found", http.StatusNotFound) + return "", nil, false + } + + host, err := stereoPairIPAddress(deviceID, conn) + if err != nil { + app.sendError(w, err.Error(), http.StatusConflict) + return "", nil, false + } + + return host, conn, true +} + +func stereoPairIPAddress(deviceID string, conn *webtypes.DeviceConnection) (string, error) { + if conn != nil && conn.DeviceInfo != nil { + if ip := strings.TrimSpace(conn.DeviceInfo.IPAddress); net.ParseIP(ip) != nil { + return ip, nil + } + } + + if ip := strings.TrimSpace(deviceID); net.ParseIP(ip) != nil { + return ip, nil + } + + return "", errors.New("device has no valid IP address for stereo pairing") +} + +func decodeStereoPairRequest(r *http.Request, req *stereoPairRequest) error { + if err := json.NewDecoder(r.Body).Decode(req); err != nil { + return errors.New("invalid request body") + } + + req.RightID = strings.TrimSpace(req.RightID) + req.GroupID = strings.TrimSpace(req.GroupID) + + req.Name = strings.TrimSpace(req.Name) + if len(req.Name) > 64 { + return errors.New("name must not exceed 64 characters") + } + + return nil +} + +func (app *WebApp) completeStereoPairMutation( + w http.ResponseWriter, + info *models.DeviceInfo, + result stereopair.Result, + operationErr error, +) { + app.applyStereoPairProjection(result) + app.awaitPriorGlobalWebSocketWrites() + app.writeStereoPairResult(w, info, result, operationErr) + app.refreshStereoPairMembersAsync(result) +} + +// applyStereoPairProjection publishes the coordinator's final fresh group +// reads locally before any follow-up poll starts. ApplyGroupEvent invalidates +// older in-flight /getGroup generations, so stale status cannot replace this +// newer lifecycle observation. +func (app *WebApp) applyStereoPairProjection(result stereopair.Result) { + activity := time.Now() + + for i := range result.Members { + member := &result.Members[i] + if member.Group == nil { + continue + } + + if conn, ok := app.deviceByStereoPairIPAddress(member.IPAddress); ok && conn != nil { + conn.ApplyGroupEvent(member.Group, activity) + } + } +} + +func (app *WebApp) refreshStereoPairMembersAsync(result stereopair.Result) { + go func() { + defer func() { + if recovered := recover(); recovered != nil { + log.Printf("Stereo-pair follow-up refresh failed: %v", recovered) + } + }() + + app.refreshStereoPairMembers(result) + }() +} + +func (app *WebApp) refreshStereoPairMembers(result stereopair.Result) { + for i := range result.Members { + if conn, ok := app.deviceByStereoPairIPAddress(result.Members[i].IPAddress); ok && conn != nil { + app.UpdateDeviceStatus(result.Members[i].IPAddress, conn) + } + } + + app.BroadcastDeviceList() +} + +func (app *WebApp) deviceByStereoPairIPAddress(ipAddress string) (*webtypes.DeviceConnection, bool) { + if conn, ok := app.GetDevice(ipAddress); ok { + return conn, true + } + + for _, entry := range app.DeviceSnapshot() { + if entry.Device != nil && entry.Device.DeviceInfo != nil && entry.Device.DeviceInfo.IPAddress == ipAddress { + return entry.Device, true + } + } + + return nil, false +} + +func (app *WebApp) writeStereoPairResult(w http.ResponseWriter, info *models.DeviceInfo, result stereopair.Result, operationErr error) { + data := stereoPairResponse{ + Operation: string(result.Operation), + Status: string(result.Status), + Capable: stereoPairCapable(info), + Paired: result.Group != nil && !result.Group.IsEmpty(), + Group: result.Group, + Members: make([]stereoPairMemberResponse, 0, len(result.Members)), + PersistenceAttempted: result.PersistenceAttempted, + PersistenceComplete: result.PersistenceComplete, + PersistenceError: errorString(result.PersistenceError), + } + for i := range result.Members { + member := &result.Members[i] + data.Members = append(data.Members, stereoPairMemberResponse{ + IPAddress: member.IPAddress, + DeviceID: member.DeviceID, + Reachable: member.Reachable, + Verified: member.Verified, + Group: member.Group, + PreflightError: errorString(member.PreflightError), + MutationError: errorString(member.MutationError), + VerificationError: errorString(member.VerificationError), + CompensationError: errorString(member.CompensationError), + CompensationVerified: member.CompensationVerified, + }) + } + + status := http.StatusOK + + response := webtypes.APIResponse{Success: operationErr == nil, Data: data} + if operationErr != nil { + status = stereoPairHTTPStatus(result) + response.Error = operationErr.Error() + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + + if err := json.NewEncoder(w).Encode(response); err != nil { + log.Printf("Failed to encode stereo-pair response: %v", err) + } +} + +func stereoPairHTTPStatus(result stereopair.Result) int { + if result.Status == stereopair.StatusDegraded || resultHasStereoPairError(result, stereopair.ErrUnavailable) { + return http.StatusBadGateway + } + + if resultHasStereoPairError(result, stereopair.ErrInvalidRequest) { + return http.StatusBadRequest + } + + return http.StatusConflict +} + +func resultHasStereoPairError(result stereopair.Result, target error) bool { + if errors.Is(result.PersistenceError, target) { + return true + } + + for i := range result.Members { + member := &result.Members[i] + for _, candidate := range []error{ + member.PreflightError, + member.MutationError, + member.VerificationError, + member.CompensationError, + } { + if errors.Is(candidate, target) { + return true + } + } + } + + return false +} + +func errorString(err error) string { + if err == nil { + return "" + } + + return err.Error() +} diff --git a/pkg/service/soundtouchweb/handler_stereopair_test.go b/pkg/service/soundtouchweb/handler_stereopair_test.go new file mode 100644 index 00000000..a773df07 --- /dev/null +++ b/pkg/service/soundtouchweb/handler_stereopair_test.go @@ -0,0 +1,507 @@ +package soundtouchweb + +import ( + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gesellix/bose-soundtouch/pkg/client" + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes" + "github.com/gesellix/bose-soundtouch/pkg/stereopair" +) + +type fakeStereoPairLifecycle struct { + inspectResult stereopair.Result + inspectErr error + createResult stereopair.Result + createErr error + renameResult stereopair.Result + renameErr error + dissolveResult stereopair.Result + dissolveErr error + + createRequest stereopair.CreateRequest + renameRequest stereopair.RenameRequest + dissolveRequest stereopair.DissolveRequest +} + +func (f *fakeStereoPairLifecycle) Inspect(string) (stereopair.Result, error) { + return f.inspectResult, f.inspectErr +} + +func (f *fakeStereoPairLifecycle) Create(req stereopair.CreateRequest) (stereopair.Result, error) { + f.createRequest = req + return f.createResult, f.createErr +} + +func (f *fakeStereoPairLifecycle) Rename(req stereopair.RenameRequest) (stereopair.Result, error) { + f.renameRequest = req + return f.renameResult, f.renameErr +} + +func (f *fakeStereoPairLifecycle) Dissolve(req stereopair.DissolveRequest) (stereopair.Result, error) { + f.dissolveRequest = req + + return f.dissolveResult, f.dissolveErr +} + +func stereoPairTestApp(lifecycle StereoPairLifecycle) *WebApp { + app := NewWebApp() + app.StereoPairs = lifecycle + for _, speaker := range []struct { + host, id, name string + }{ + {"192.0.2.10", "left-id", "Left"}, + {"192.0.2.11", "right-id", "Right"}, + } { + conn := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{ + DeviceID: speaker.id, + Name: speaker.name, + Type: "SoundTouch 10", + IPAddress: speaker.host, + }) + app.AddDevice(speaker.host, conn) + } + + return app +} + +func decodeStereoPairAPIResponse(t *testing.T, response *httptest.ResponseRecorder) struct { + Success bool `json:"success"` + Data stereoPairResponse `json:"data"` + Error string `json:"error"` +} { + t.Helper() + + var payload struct { + Success bool `json:"success"` + Data stereoPairResponse `json:"data"` + Error string `json:"error"` + } + if err := json.NewDecoder(response.Body).Decode(&payload); err != nil { + t.Fatalf("decode response: %v", err) + } + + return payload +} + +func TestHandleGetStereoPairReportsStandaloneCapableSpeaker(t *testing.T) { + fake := &fakeStereoPairLifecycle{inspectResult: stereopair.Result{ + Operation: stereopair.OperationInspect, + Status: stereopair.StatusSucceeded, + Group: &models.Group{}, + }} + app := stereoPairTestApp(fake) + + request := withChiParams(httptest.NewRequest(http.MethodGet, "/api/control/devices/192.0.2.10/stereo-pair", nil), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleGetStereoPair(response, request) + + payload := decodeStereoPairAPIResponse(t, response) + if response.Code != http.StatusOK || !payload.Success || !payload.Data.Capable || payload.Data.Paired { + t.Fatalf("unexpected standalone response: status=%d payload=%+v", response.Code, payload) + } +} + +func TestHandleGetStereoPairMapsUnavailableSpeakerToBadGateway(t *testing.T) { + fake := &fakeStereoPairLifecycle{ + inspectResult: stereopair.Result{ + Operation: stereopair.OperationInspect, + Status: stereopair.StatusFailed, + Members: []stereopair.MemberResult{{ + IPAddress: "192.0.2.10", + PreflightError: fmt.Errorf("%w: timeout", stereopair.ErrUnavailable), + }}, + }, + inspectErr: &stereopair.Error{Operation: stereopair.OperationInspect, Status: stereopair.StatusFailed}, + } + app := stereoPairTestApp(fake) + + request := withChiParams(httptest.NewRequest(http.MethodGet, + "/api/control/devices/192.0.2.10/stereo-pair", nil), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleGetStereoPair(response, request) + + if response.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want 502; body=%s", response.Code, response.Body.String()) + } +} + +func TestHandleGetStereoPairPreservesDegradedRecoveryGeneration(t *testing.T) { + group := &models.Group{ID: "PAIR-ID", Name: "Living Room", MasterDeviceID: "left-id"} + fake := &fakeStereoPairLifecycle{ + inspectResult: stereopair.Result{ + Operation: stereopair.OperationInspect, + Status: stereopair.StatusDegraded, + Group: group, + Members: []stereopair.MemberResult{ + {IPAddress: "192.0.2.10", DeviceID: "left-id", Group: group, Verified: true}, + {IPAddress: "192.0.2.11", DeviceID: "right-id", VerificationError: errors.New("group is empty")}, + }, + }, + inspectErr: &stereopair.Error{Operation: stereopair.OperationInspect, Status: stereopair.StatusDegraded}, + } + app := stereoPairTestApp(fake) + + request := withChiParams(httptest.NewRequest(http.MethodGet, + "/api/control/devices/192.0.2.10/stereo-pair", nil), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleGetStereoPair(response, request) + + payload := decodeStereoPairAPIResponse(t, response) + if response.Code != http.StatusBadGateway || payload.Success || !payload.Data.Paired || + payload.Data.Group == nil || payload.Data.Group.ID != "PAIR-ID" { + t.Fatalf("degraded recovery data was lost: status=%d payload=%+v", response.Code, payload) + } +} + +func TestHandleCreateStereoPairPassesPhysicalHostsAndName(t *testing.T) { + group := &models.Group{ID: "1234567", Name: "Office", MasterDeviceID: "left-id"} + fake := &fakeStereoPairLifecycle{createResult: stereopair.Result{ + Operation: stereopair.OperationCreate, + Status: stereopair.StatusSucceeded, + Group: group, + }} + app := stereoPairTestApp(fake) + + request := withChiParams(httptest.NewRequest(http.MethodPost, + "/api/control/devices/192.0.2.10/stereo-pair", + strings.NewReader(`{"rightId":"192.0.2.11","name":" Office "}`)), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleCreateStereoPair(response, request) + + if fake.createRequest.LeftIPAddress != "192.0.2.10" || fake.createRequest.RightIPAddress != "192.0.2.11" || fake.createRequest.Name != "Office" { + t.Fatalf("unexpected coordinator request: %+v", fake.createRequest) + } + payload := decodeStereoPairAPIResponse(t, response) + if response.Code != http.StatusOK || !payload.Success || !payload.Data.Paired || payload.Data.Group.ID != "1234567" { + t.Fatalf("unexpected create response: status=%d payload=%+v", response.Code, payload) + } +} + +func TestHandleCreateStereoPairRespondsBeforeStatusRefresh(t *testing.T) { + refreshStarted := make(chan struct{}) + releaseRefresh := make(chan struct{}) + speaker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/now_playing" { + close(refreshStarted) + <-releaseRefresh + } + + http.Error(w, "status unavailable", http.StatusServiceUnavailable) + })) + defer speaker.Close() + defer close(releaseRefresh) + + group := &models.Group{ID: "1234567", Name: "Office", MasterDeviceID: "left-id"} + fake := &fakeStereoPairLifecycle{createResult: stereopair.Result{ + Operation: stereopair.OperationCreate, + Status: stereopair.StatusSucceeded, + Group: group, + Members: []stereopair.MemberResult{{ + IPAddress: "192.0.2.10", + DeviceID: "left-id", + Verified: true, + Group: group, + }}, + }} + app := stereoPairTestApp(fake) + left, _ := app.GetDevice("192.0.2.10") + left.Client = client.NewClientFromHost(speaker.URL) + + request := withChiParams(httptest.NewRequest(http.MethodPost, + "/api/control/devices/192.0.2.10/stereo-pair", + strings.NewReader(`{"rightId":"192.0.2.11","name":"Office"}`)), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + handlerDone := make(chan struct{}) + go func() { + app.HandleCreateStereoPair(response, request) + close(handlerDone) + }() + + select { + case <-refreshStarted: + case <-time.After(time.Second): + t.Fatal("follow-up status refresh did not start") + } + + select { + case <-handlerDone: + case <-time.After(time.Second): + t.Fatal("lifecycle response waited for the blocked status refresh") + } + + payload := decodeStereoPairAPIResponse(t, response) + if response.Code != http.StatusOK || !payload.Success || payload.Data.Group == nil || payload.Data.Group.ID != group.ID { + t.Fatalf("unexpected create response: status=%d payload=%+v", response.Code, payload) + } + if projected := left.Status().Group; projected == nil || projected.ID != group.ID { + t.Fatalf("coordinator projection was not applied before refresh: %+v", projected) + } +} + +func TestApplyStereoPairProjectionSupersedesInFlightGroupRefresh(t *testing.T) { + app := stereoPairTestApp(&fakeStereoPairLifecycle{}) + left, _ := app.GetDevice("192.0.2.10") + staleGeneration := left.BeginGroupRefresh() + newGroup := &models.Group{ID: "pair-new", MasterDeviceID: "left-id"} + + app.applyStereoPairProjection(stereopair.Result{Members: []stereopair.MemberResult{{ + IPAddress: "192.0.2.10", + Group: newGroup, + }}}) + + if left.ApplyPolledGroup(staleGeneration, &models.Group{ID: "pair-old"}) { + t.Fatal("older status refresh replaced the lifecycle projection") + } + if projected := left.Status().Group; projected == nil || projected.ID != newGroup.ID { + t.Fatalf("projected group = %+v, want %q", projected, newGroup.ID) + } +} + +func TestStereoPairResponseWaitsForPreMutationWebSocketWriter(t *testing.T) { + app := stereoPairTestApp(&fakeStereoPairLifecycle{}) + app.webSocketWriteTimeout = 50 * time.Millisecond + blockedWriter := &deadlineBlockingWebSocketWriter{started: make(chan struct{})} + app.registerDeviceWebSocketClient(blockedWriter) + defer app.removeDeviceWebSocketClient(blockedWriter) + + writerDone := make(chan struct{}) + go func() { + _ = app.withDeviceConnWrite(blockedWriter, func(batch webSocketWriteBatch) error { + return batch.writeJSON(blockedWriter, struct{}{}) + }) + close(writerDone) + }() + <-blockedWriter.started + + response := httptest.NewRecorder() + responseDone := make(chan struct{}) + group := &models.Group{ID: "PAIR-ID", MasterDeviceID: "left-id"} + left, _ := app.GetDevice("192.0.2.10") + go func() { + app.completeStereoPairMutation(response, left.DeviceInfo, stereopair.Result{ + Operation: stereopair.OperationCreate, + Status: stereopair.StatusSucceeded, + Group: group, + Members: []stereopair.MemberResult{{ + IPAddress: "192.0.2.10", + DeviceID: "left-id", + Group: group, + }}, + }, nil) + close(responseDone) + }() + + select { + case <-responseDone: + t.Fatal("lifecycle response overtook a pre-mutation WebSocket writer") + case <-time.After(10 * time.Millisecond): + } + + select { + case <-writerDone: + case <-time.After(500 * time.Millisecond): + t.Fatal("stalled pre-mutation WebSocket writer ignored its batch deadline") + } + select { + case <-responseDone: + case <-time.After(500 * time.Millisecond): + t.Fatal("lifecycle response did not pass the WebSocket ordering barrier") + } +} + +func TestHandleCreateStereoPairUsesDeviceInfoIPsForHostnameRegistryKeys(t *testing.T) { + fake := &fakeStereoPairLifecycle{createResult: stereopair.Result{ + Operation: stereopair.OperationCreate, + Status: stereopair.StatusSucceeded, + Group: &models.Group{ID: "1234567"}, + }} + app := NewWebApp() + app.StereoPairs = fake + for _, speaker := range []struct { + key, ip, id string + }{ + {"left.example.test", "192.0.2.10", "left-id"}, + {"right.example.test", "192.0.2.11", "right-id"}, + } { + app.AddDevice(speaker.key, webtypes.NewDeviceConnection(nil, &models.DeviceInfo{ + DeviceID: speaker.id, Type: "ST10", IPAddress: speaker.ip, + })) + } + + request := withChiParams(httptest.NewRequest(http.MethodPost, + "/api/control/devices/left.example.test/stereo-pair", + strings.NewReader(`{"rightId":"right.example.test","name":"Office"}`)), map[string]string{"id": "left.example.test"}) + response := httptest.NewRecorder() + app.HandleCreateStereoPair(response, request) + + if response.Code != http.StatusOK { + t.Fatalf("status = %d; body=%s", response.Code, response.Body.String()) + } + payload := decodeStereoPairAPIResponse(t, response) + if !payload.Data.Capable { + t.Fatal("ST10 model spelling was not reported as stereo capable") + } + if fake.createRequest.LeftIPAddress != "192.0.2.10" || fake.createRequest.RightIPAddress != "192.0.2.11" { + t.Fatalf("coordinator addresses = %+v", fake.createRequest) + } +} + +func TestHandleRenameStereoPairPassesExpectedGeneration(t *testing.T) { + fake := &fakeStereoPairLifecycle{renameResult: stereopair.Result{ + Operation: stereopair.OperationRename, + Status: stereopair.StatusSucceeded, + Group: &models.Group{ID: "PAIR-ID", Name: "Office"}, + }} + app := stereoPairTestApp(fake) + + request := withChiParams(httptest.NewRequest(http.MethodPatch, + "/api/control/devices/192.0.2.10/stereo-pair", + strings.NewReader(`{"groupId":" PAIR-ID ","name":" Office "}`)), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleRenameStereoPair(response, request) + + if fake.renameRequest.ExpectedGroupID != "PAIR-ID" || fake.renameRequest.Name != "Office" { + t.Fatalf("rename request = %+v", fake.renameRequest) + } + if response.Code != http.StatusOK { + t.Fatalf("status = %d; body=%s", response.Code, response.Body.String()) + } +} + +func TestHandleRenameStereoPairRequiresExpectedGeneration(t *testing.T) { + fake := &fakeStereoPairLifecycle{} + app := stereoPairTestApp(fake) + + request := withChiParams(httptest.NewRequest(http.MethodPatch, + "/api/control/devices/192.0.2.10/stereo-pair", + strings.NewReader(`{"name":"Office"}`)), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleRenameStereoPair(response, request) + + if response.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body=%s", response.Code, response.Body.String()) + } +} + +func TestHandleDissolveStereoPairPreservesDegradedDetails(t *testing.T) { + verifyErr := errors.New("right speaker still reports the pair") + fake := &fakeStereoPairLifecycle{dissolveResult: stereopair.Result{ + Operation: stereopair.OperationDissolve, + Status: stereopair.StatusDegraded, + Members: []stereopair.MemberResult{{ + IPAddress: "192.0.2.11", + DeviceID: "right-id", + VerificationError: verifyErr, + }}, + }, dissolveErr: &stereopair.Error{Operation: stereopair.OperationDissolve, Status: stereopair.StatusDegraded}} + app := stereoPairTestApp(fake) + + request := withChiParams(httptest.NewRequest(http.MethodDelete, + "/api/control/devices/192.0.2.10/stereo-pair", + strings.NewReader(`{"groupId":"PAIR-ID"}`)), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleDissolveStereoPair(response, request) + + if fake.dissolveRequest.ExpectedGroupID != "PAIR-ID" { + t.Fatalf("dissolve request = %+v", fake.dissolveRequest) + } + + payload := decodeStereoPairAPIResponse(t, response) + if response.Code != http.StatusBadGateway || payload.Success || len(payload.Data.Members) != 1 || + payload.Data.Members[0].VerificationError != verifyErr.Error() { + t.Fatalf("unexpected degraded response: status=%d payload=%+v", response.Code, payload) + } +} + +func TestHandleDissolveStereoPairPassesExactRecoverySnapshot(t *testing.T) { + fake := &fakeStereoPairLifecycle{dissolveResult: stereopair.Result{ + Operation: stereopair.OperationDissolve, + Status: stereopair.StatusSucceeded, + Group: &models.Group{}, + }} + app := stereoPairTestApp(fake) + body := `{"groupId":"PAIR-ID","group":{"ID":"PAIR-ID","Name":"Office","MasterDeviceID":"left-id","Roles":{"Roles":[{"DeviceID":"left-id","Role":"LEFT","IPAddress":"192.0.2.10"},{"DeviceID":"right-id","Role":"RIGHT","IPAddress":"192.0.2.11"}]}}}` + + request := withChiParams(httptest.NewRequest(http.MethodDelete, + "/api/control/devices/192.0.2.10/stereo-pair", strings.NewReader(body)), + map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleDissolveStereoPair(response, request) + + group := fake.dissolveRequest.ExpectedGroup + if response.Code != http.StatusOK || group == nil || group.ID != "PAIR-ID" || + group.MasterDeviceID != "left-id" || len(group.Roles.Roles) != 2 { + t.Fatalf("status=%d dissolve request=%+v", response.Code, fake.dissolveRequest) + } +} + +func TestHandleDissolveStereoPairRejectsUnregisteredSnapshotMember(t *testing.T) { + fake := &fakeStereoPairLifecycle{} + app := stereoPairTestApp(fake) + body := `{"groupId":"PAIR-ID","group":{"ID":"PAIR-ID","MasterDeviceID":"left-id","Roles":{"Roles":[{"DeviceID":"left-id","Role":"LEFT","IPAddress":"192.0.2.10"},{"DeviceID":"substitute-id","Role":"RIGHT","IPAddress":"192.0.2.99"}]}}}` + + request := withChiParams(httptest.NewRequest(http.MethodDelete, + "/api/control/devices/192.0.2.10/stereo-pair", strings.NewReader(body)), + map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleDissolveStereoPair(response, request) + + if response.Code != http.StatusConflict || fake.dissolveRequest.ExpectedGroup != nil { + t.Fatalf("status=%d coordinator request=%+v", response.Code, fake.dissolveRequest) + } +} + +func TestHandleDissolveStereoPairReportsCompletedPersistence(t *testing.T) { + fake := &fakeStereoPairLifecycle{dissolveResult: stereopair.Result{ + Operation: stereopair.OperationDissolve, + Status: stereopair.StatusSucceeded, + Group: &models.Group{}, + PersistenceAttempted: true, + PersistenceComplete: true, + }} + app := stereoPairTestApp(fake) + + request := withChiParams(httptest.NewRequest(http.MethodDelete, + "/api/control/devices/192.0.2.10/stereo-pair", + strings.NewReader(`{"groupId":"PAIR-ID"}`)), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleDissolveStereoPair(response, request) + + payload := decodeStereoPairAPIResponse(t, response) + if response.Code != http.StatusOK || !payload.Success || + !payload.Data.PersistenceAttempted || !payload.Data.PersistenceComplete { + t.Fatalf("status=%d payload=%+v", response.Code, payload) + } +} + +func TestHandleDissolveStereoPairReportsPersistenceFailure(t *testing.T) { + fake := &fakeStereoPairLifecycle{dissolveResult: stereopair.Result{ + Operation: stereopair.OperationDissolve, + Status: stereopair.StatusDegraded, + Group: &models.Group{}, + PersistenceAttempted: true, + PersistenceError: errors.New("datastore unavailable"), + }, dissolveErr: &stereopair.Error{Operation: stereopair.OperationDissolve, Status: stereopair.StatusDegraded}} + app := stereoPairTestApp(fake) + + request := withChiParams(httptest.NewRequest(http.MethodDelete, + "/api/control/devices/192.0.2.10/stereo-pair", + strings.NewReader(`{"groupId":"PAIR-ID"}`)), map[string]string{"id": "192.0.2.10"}) + response := httptest.NewRecorder() + app.HandleDissolveStereoPair(response, request) + + payload := decodeStereoPairAPIResponse(t, response) + if response.Code != http.StatusBadGateway || payload.Success || + payload.Data.Status != string(stereopair.StatusDegraded) || + payload.Data.PersistenceError != "datastore unavailable" { + t.Fatalf("unexpected persistence response: status=%d payload=%+v", response.Code, payload) + } +} diff --git a/pkg/service/soundtouchweb/handler_test.go b/pkg/service/soundtouchweb/handler_test.go index f6143832..96609a1b 100644 --- a/pkg/service/soundtouchweb/handler_test.go +++ b/pkg/service/soundtouchweb/handler_test.go @@ -62,6 +62,23 @@ func TestNewWebApp(t *testing.T) { } } +func TestStereoPairPersistenceClientHasBoundedLifecycleTimeout(t *testing.T) { + app := NewWebApp() + transport := &http.Transport{} + app.ServiceClient = &http.Client{Transport: transport, Timeout: 10 * time.Second} + + configured := app.stereoPairPersistenceClient() + if configured.Timeout != 45*time.Second { + t.Fatalf("timeout = %s, want 45s", configured.Timeout) + } + if configured.Transport != transport { + t.Fatal("custom service transport was not preserved") + } + if app.ServiceClient.Timeout != 10*time.Second { + t.Fatalf("source client timeout was mutated to %s", app.ServiceClient.Timeout) + } +} + func TestHandleAPIDevices(t *testing.T) { app := createTestApp() diff --git a/pkg/service/soundtouchweb/mount.go b/pkg/service/soundtouchweb/mount.go index 19b020fd..9508d6f4 100644 --- a/pkg/service/soundtouchweb/mount.go +++ b/pkg/service/soundtouchweb/mount.go @@ -93,6 +93,13 @@ func (app *WebApp) MountWeb(r chi.Router, discoveryService *discovery.UnifiedDis r.Post("/leave", app.HandleZoneLeave) }) + r.Route("/stereo-pair", func(r chi.Router) { + r.Get("/", app.HandleGetStereoPair) + r.Post("/", app.HandleCreateStereoPair) + r.Patch("/", app.HandleRenameStereoPair) + r.Delete("/", app.HandleDissolveStereoPair) + }) + // Play a result from a content provider on this device. // Browsable providers (tunein, radiobrowser) take a catalog item; // input providers (url, tts) take the raw input. diff --git a/pkg/service/soundtouchweb/mount_test.go b/pkg/service/soundtouchweb/mount_test.go index 365ab0b0..4ec172b9 100644 --- a/pkg/service/soundtouchweb/mount_test.go +++ b/pkg/service/soundtouchweb/mount_test.go @@ -72,6 +72,7 @@ func TestMountWebControlAPIShape(t *testing.T) { "/api/control/devices/{id}/providers/radiobrowser/play", "/api/control/devices/{id}/providers/url/play", "/api/control/devices/{id}/providers/tts/play", + "/api/control/devices/{id}/stereo-pair/", } for _, want := range mustExist { if !registered[want] { diff --git a/pkg/service/soundtouchweb/static/css/app.css b/pkg/service/soundtouchweb/static/css/app.css index dc179a2e..7847235f 100644 --- a/pkg/service/soundtouchweb/static/css/app.css +++ b/pkg/service/soundtouchweb/static/css/app.css @@ -701,6 +701,46 @@ img { display: block; max-width: 100%; } .zone-actions { display: flex; gap: .5rem; margin-top: .25rem; flex-wrap: wrap; } .zone-btn { font-size: .8rem; padding: .3rem .7rem; } +/* ── Stereo pairs ─────────────────────────────────────────────────────────── */ +.stereo-pair-section { margin-top: 1.25rem; } +.stereo-pair-members { display: flex; flex-direction: column; gap: .3rem; } +.stereo-pair-member { + display: flex; align-items: center; gap: .6rem; + min-height: 2.25rem; padding: .4rem .6rem; + background: var(--surface); border: 1px solid var(--border); + border-radius: var(--radius); +} +.stereo-role { + min-width: 3rem; padding: .15rem .4rem; border-radius: 3px; + background: var(--border); color: var(--text-dim); + font-size: .65rem; font-weight: 700; text-align: center; letter-spacing: 0; +} +.stereo-member-name { flex: 1; min-width: 0; overflow-wrap: anywhere; font-size: .875rem; } +.stereo-pair-actions { display: flex; align-items: end; gap: .5rem; margin-top: .6rem; flex-wrap: wrap; } +.stereo-name-field { display: flex; flex: 1 1 14rem; flex-direction: column; gap: .2rem; } +.stereo-name-field span, .picker-label { font-size: .75rem; color: var(--text-dim); letter-spacing: 0; } +.stereo-name-field input { + min-width: 0; padding: .4rem .55rem; border: 1px solid var(--border); + border-radius: var(--radius); background: var(--surface); color: var(--text); + font: inherit; font-size: .875rem; +} +.stereo-name-field input:focus { outline: 2px solid var(--accent); outline-offset: 1px; } +.stereo-action { min-height: 2.2rem; font-size: .8rem; padding: .35rem .7rem; } +.stereo-action.danger { color: #b42318; border-color: #d92d20; } +.stereo-pair-standalone { display: flex; align-items: center; gap: .75rem; flex-wrap: wrap; color: var(--text-dim); } +.stereo-pair-error { margin-bottom: .6rem; color: #b42318; font-size: .8rem; } +.stereo-picker { width: min(22rem, calc(100vw - 2rem)); max-width: 22rem; } +.picker-name-field { margin: .75rem 0; } +.picker-label { margin-bottom: .4rem; } +.picker-device-btn.selected { border-color: var(--accent); background: var(--bg); } +.stereo-picker-actions { display: flex; justify-content: flex-end; gap: .5rem; } + +@media (max-width: 520px) { + .stereo-pair-actions { align-items: stretch; } + .stereo-name-field { flex-basis: 100%; } + .stereo-action { flex: 1; } +} + /* ── Recents ─────────────────────────────────────────────────────────────── */ .recents-section { margin-top: 1.25rem; } diff --git a/pkg/service/soundtouchweb/static/js/api.js b/pkg/service/soundtouchweb/static/js/api.js index f929663a..bb0f320c 100644 --- a/pkg/service/soundtouchweb/static/js/api.js +++ b/pkg/service/soundtouchweb/static/js/api.js @@ -25,6 +25,22 @@ export const api = { zoneRemove: (masterId, slaveId) => req(`/api/control/devices/${masterId}/zone/remove/${slaveId}`, { method: 'POST' }), zoneDissolve: (id) => req(`/api/control/devices/${id}/zone/dissolve`, { method: 'POST' }), zoneLeave: (id) => req(`/api/control/devices/${id}/zone/leave`, { method: 'POST' }), + stereoPair: (id) => req(`/api/control/devices/${id}/stereo-pair/`), + stereoPairCreate: (leftId, rightId, name) => req(`/api/control/devices/${leftId}/stereo-pair/`, { + method: 'POST', + headers: JSON_HEADERS, + body: JSON.stringify({ rightId, name }), + }), + stereoPairRename: (id, groupId, name) => req(`/api/control/devices/${id}/stereo-pair/`, { + method: 'PATCH', + headers: JSON_HEADERS, + body: JSON.stringify({ groupId, name }), + }), + stereoPairDissolve: (id, groupId, group) => req(`/api/control/devices/${id}/stereo-pair/`, { + method: 'DELETE', + headers: JSON_HEADERS, + body: JSON.stringify({ groupId, group }), + }), play: (id, item) => req(`/api/control/devices/${id}/play`, { method: 'POST', headers: JSON_HEADERS, diff --git a/pkg/service/soundtouchweb/static/js/app.js b/pkg/service/soundtouchweb/static/js/app.js index 2e31cd6f..ca8f9461 100644 --- a/pkg/service/soundtouchweb/static/js/app.js +++ b/pkg/service/soundtouchweb/static/js/app.js @@ -7,6 +7,7 @@ import { Controls } from './components/Controls.js'; import { Presets } from './components/Presets.js'; import { Sources } from './components/Sources.js'; import { Zone } from './components/Zone.js'; +import { StereoPair } from './components/StereoPair.js'; import { Recents } from './components/Recents.js'; import { TuneInBrowser } from './components/TuneInBrowser.js'; import { RadioBrowser } from './components/RadioBrowser.js'; @@ -18,7 +19,7 @@ import { api } from './api.js'; const html = htm.bind(h); -function DeviceDetail({ deviceId, devices, onBack }) { +function DeviceDetail({ deviceId, devices, onBack, onDevicesChanged, notify }) { const device = devices[deviceId]; if (!device) { @@ -45,6 +46,13 @@ function DeviceDetail({ deviceId, devices, onBack }) { <${Controls} deviceId=${deviceId} status=${device.status} /> <${Presets} deviceId=${deviceId} status=${device.status} /> <${Sources} deviceId=${deviceId} status=${device.status} /> + <${StereoPair} + deviceId=${deviceId} + device=${device} + devices=${devices} + onChanged=${onDevicesChanged} + notify=${notify} + /> <${Zone} deviceId=${deviceId} devices=${devices} /> <${Recents} deviceId=${deviceId} /> @@ -161,6 +169,11 @@ function App() { await api.discover(); } + async function refreshDevices() { + const resp = await api.devices(); + if (resp?.success) setDevices(resp.data || {}); + } + async function removeDevice(id) { const name = devices[id]?.info?.name || id; if (!confirm(`Remove "${name}"?\n\nThis clears it from AfterTouch. A device still online may reappear after the next discovery scan.`)) { @@ -262,6 +275,8 @@ function App() { deviceId=${selectedId} devices=${devices} onBack=${() => navigate('devices')} + onDevicesChanged=${refreshDevices} + notify=${showToast} /> ` : page === 'tunein' ? html` <${TuneInBrowser} key="tunein-browser" devices=${devices} /> diff --git a/pkg/service/soundtouchweb/static/js/components/StereoPair.js b/pkg/service/soundtouchweb/static/js/components/StereoPair.js new file mode 100644 index 00000000..0853446a --- /dev/null +++ b/pkg/service/soundtouchweb/static/js/components/StereoPair.js @@ -0,0 +1,296 @@ +import { h } from 'preact'; +import { useEffect, useRef, useState } from 'preact/hooks'; +import htm from 'htm'; +import { api } from '../api.js'; + +const html = htm.bind(h); + +function isStereoCapable(device) { + const type = (device?.info?.type || '').trim().toLowerCase(); + return type === 'st10' || type === 'soundtouch 10'; +} + +function responseError(resp, fallback) { + const details = (resp?.data?.members || []).flatMap(member => [ + member.preflightError, + member.mutationError, + member.verificationError, + member.compensationError, + ].filter(Boolean).map(message => `${member.ipAddress || member.deviceId || 'speaker'}: ${message}`)); + + return [resp?.error || fallback, resp?.data?.persistenceError, ...details].filter(Boolean).join('; '); +} + +function groupId(group) { + return group?.ID || group?.id || ''; +} + +function groupRoles(group) { + return group?.Roles?.Roles || group?.roles?.roles || []; +} + +function hasConfiguredGroup(group) { + return Boolean(group && (groupId(group) || group?.MasterDeviceID || group?.masterDeviceId || groupRoles(group).length)); +} + +function snapshotFromProjection(pair) { + if (!pair?.id) return null; + return { + ID: pair.id, + Name: pair.name || '', + MasterDeviceID: pair.masterDeviceId, + Roles: { + Roles: (pair.members || []).map(member => ({ + DeviceID: member.deviceId, + Role: member.role, + IPAddress: member.ipAddress, + })), + }, + }; +} + +export function StereoPair({ deviceId, device, devices, onChanged, notify }) { + const pair = device?.stereoPair; + const [state, setState] = useState(null); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(''); + const [showPicker, setShowPicker] = useState(false); + const [rightId, setRightId] = useState(''); + const [name, setName] = useState(pair?.name || device?.info?.name || ''); + const [savedRecovery, setSavedRecovery] = useState(null); + const refreshGeneration = useRef(0); + const nameEdited = useRef(false); + const mutationError = useRef(''); + const previousDeviceId = useRef(deviceId); + const mounted = useRef(true); + const currentSelection = useRef({ deviceId }); + if (currentSelection.current.deviceId !== deviceId) { + currentSelection.current = { deviceId }; + } + const refreshKey = JSON.stringify([deviceId, pair?.id || null, pair?.name || null]); + const currentRefreshKey = useRef(refreshKey); + currentRefreshKey.current = refreshKey; + + useEffect(() => { + mounted.current = true; + return () => { + mounted.current = false; + }; + }, []); + + async function refresh({ preserveError = false } = {}) { + if (refreshKey !== currentRefreshKey.current) return; + const generation = ++refreshGeneration.current; + try { + const resp = await api.stereoPair(deviceId); + if (generation !== refreshGeneration.current || refreshKey !== currentRefreshKey.current) return; + if (resp?.data) { + setState(resp.data); + const currentName = resp.data.group?.Name || resp.data.group?.name; + if (currentName && !nameEdited.current) setName(currentName); + } + if (resp?.success) { + if (!preserveError && !mutationError.current) setError(''); + } else if (!preserveError && !mutationError.current) { + setError(responseError(resp, 'Unable to read stereo-pair state')); + } + } catch (_) { + if (generation === refreshGeneration.current && refreshKey === currentRefreshKey.current && + !preserveError && !mutationError.current) { + setError('Unable to read stereo-pair state'); + } + } + } + + useEffect(() => { + if (previousDeviceId.current !== deviceId) { + previousDeviceId.current = deviceId; + nameEdited.current = false; + mutationError.current = ''; + setState(null); + setError(''); + setBusy(false); + setSavedRecovery(null); + } + if (!nameEdited.current) setName(pair?.name || device?.info?.name || ''); + setRightId(''); + setShowPicker(false); + refresh(); + return () => { + refreshGeneration.current++; + }; + }, [deviceId, pair?.id, pair?.name]); + + const stateGroup = state?.group; + const stateSnapshot = hasConfiguredGroup(stateGroup) && (!pair?.id || groupId(stateGroup) === pair.id) + ? stateGroup : null; + const observedSnapshot = stateSnapshot || snapshotFromProjection(pair); + const savedSnapshot = savedRecovery?.deviceId === deviceId ? savedRecovery.group : null; + const recoverySnapshot = observedSnapshot || savedSnapshot; + const recoveryGroupRoles = groupRoles(recoverySnapshot); + const recoveryGroupId = groupId(recoverySnapshot); + const recoveryGroup = !pair && hasConfiguredGroup(recoverySnapshot); + const expectedGroupId = pair?.id || recoveryGroupId; + + const candidates = Object.entries(devices || {}).filter(([id, candidate]) => + id !== deviceId && !candidate.stereoPair && !(candidate.status?.group?.ID || candidate.status?.group?.id) && + candidate.status?.isConnected && isStereoCapable(candidate)); + + async function recoverFromFailure(operationError) { + mutationError.current = operationError; + setError(operationError); + await Promise.allSettled([ + Promise.resolve().then(() => onChanged?.()), + refresh({ preserveError: true }), + ]); + } + + async function run(action, successMessage, onSuccess) { + const selection = currentSelection.current; + const isCurrentMutation = () => mounted.current && selection === currentSelection.current; + setBusy(true); + mutationError.current = ''; + setError(''); + try { + const resp = await action(); + if (!isCurrentMutation()) return; + refreshGeneration.current++; + if (!resp?.success) { + await recoverFromFailure(responseError(resp, 'Stereo-pair operation failed')); + return; + } + nameEdited.current = false; + setShowPicker(false); + onSuccess?.(); + notify?.(successMessage); + await onChanged?.(); + await refresh(); + } catch (_) { + if (isCurrentMutation()) { + await recoverFromFailure('Stereo-pair operation failed'); + } + } finally { + if (isCurrentMutation()) setBusy(false); + } + } + + function editName(event) { + nameEdited.current = true; + setName(event.currentTarget.value); + } + + function createPair() { + if (!rightId) return; + run(() => api.stereoPairCreate(deviceId, rightId, name.trim()), 'Stereo pair created'); + } + + function renamePair(event) { + event.preventDefault(); + const nextName = name.trim(); + if (!expectedGroupId || !nextName || nextName === pair?.name) return; + run(() => api.stereoPairRename(deviceId, expectedGroupId, nextName), 'Stereo pair renamed'); + } + + function dissolvePair() { + if (!expectedGroupId || !recoverySnapshot) return; + const pairName = pair?.name || recoverySnapshot?.Name || recoverySnapshot?.name || + device?.info?.name || 'this stereo pair'; + if (!confirm(`Dissolve "${pairName}"?\n\nBoth speakers will become standalone devices.`)) return; + setSavedRecovery({ deviceId, group: recoverySnapshot }); + run( + () => api.stereoPairDissolve(deviceId, expectedGroupId, recoverySnapshot), + 'Stereo pair dissolved', + () => setSavedRecovery(null), + ); + } + + if (!pair && !recoveryGroup && state && !state.capable) return null; + if (!pair && !state && !isStereoCapable(device)) return null; + + return html` +
+
Stereo pair
+ ${error ? html`` : null} + + ${pair ? html` +
+ ${(pair.members || []).map(member => html` +
+ ${member.role} + ${member.name || member.ipAddress || member.deviceId} + +
+ `)} +
+
+ + + +
+ ` : recoveryGroup ? html` +
+
+ Degraded + ${recoverySnapshot.Name || recoverySnapshot.name || 'Unnamed stereo pair'} +
+
+ Generation ID + ${recoveryGroupId} +
+ ${recoveryGroupRoles.map(member => html` +
+ ${member.Role || member.role || 'Member'} + ${member.IPAddress || member.ipAddress || member.DeviceID || member.deviceId || 'Unknown member'} +
+ `)} +
+
+ +
+ ` : html` +
+ Standalone + +
+ `} + + ${showPicker ? html` +
setShowPicker(false)}> +
event.stopPropagation()}> +
Create stereo pair
+ +
Right speaker
+
+ ${candidates.map(([id, candidate]) => html` + + `)} +
+
+ + +
+
+
+ ` : null} +
+ `; +} diff --git a/pkg/service/soundtouchweb/websocket.go b/pkg/service/soundtouchweb/websocket.go index c5259e33..b80507a5 100644 --- a/pkg/service/soundtouchweb/websocket.go +++ b/pkg/service/soundtouchweb/websocket.go @@ -306,6 +306,70 @@ func (app *WebApp) applyBassEvent( }) } +// registerDeviceWebSocketClient gives conn its own write-serialization lock, +// mirroring registerGlobalWebSocket's role for the browser-wide pool. Unlike +// registerGlobalWebSocket, there are no initial frames to send under it -- +// callers install the lock before their first write. +func (app *WebApp) registerDeviceWebSocketClient(conn webSocketWriter) { + app.DeviceWSMutex.Lock() + app.DeviceWSClients[conn] = &sync.Mutex{} + app.DeviceWSMutex.Unlock() +} + +// removeDeviceWebSocketClient unregisters conn. Unlike +// removeGlobalWebSocketClient, callers close the underlying connection +// themselves (HandleDeviceWebSocket already does via its own defer), so this +// only needs to drop the registry entry. +func (app *WebApp) removeDeviceWebSocketClient(conn webSocketWriter) { + app.DeviceWSMutex.Lock() + delete(app.DeviceWSClients, conn) + app.DeviceWSMutex.Unlock() +} + +// withDeviceConnWrite is withConnWrite for the per-device status pool. +func (app *WebApp) withDeviceConnWrite(conn webSocketWriter, write func(webSocketWriteBatch) error) error { + app.DeviceWSMutex.RLock() + mu := app.DeviceWSClients[conn] + app.DeviceWSMutex.RUnlock() + + if mu == nil { + return errConnUnregistered + } + + mu.Lock() + defer mu.Unlock() + + return write(webSocketWriteBatch{timeout: app.writeTimeout()}) +} + +func (app *WebApp) deviceWebSocketClients() []webSocketWriter { + app.DeviceWSMutex.RLock() + defer app.DeviceWSMutex.RUnlock() + + clients := make([]webSocketWriter, 0, len(app.DeviceWSClients)) + for client := range app.DeviceWSClients { + clients = append(clients, client) + } + + return clients +} + +// awaitPriorGlobalWebSocketWrites is an ordering barrier across both browser +// WebSocket connection pools (the global device-list feed and per-device +// status feeds). Once it returns, any write already in flight on any +// currently-registered connection in either pool has completed, so a caller +// that just applied a fresh projection is guaranteed a later write captures +// it rather than racing a stale one still being sent. +func (app *WebApp) awaitPriorGlobalWebSocketWrites() { + for _, client := range app.globalWebSocketClients() { + _ = app.withConnWrite(client, func(webSocketWriteBatch) error { return nil }) + } + + for _, client := range app.deviceWebSocketClients() { + _ = app.withDeviceConnWrite(client, func(webSocketWriteBatch) error { return nil }) + } +} + // HandleWebSocket handles WebSocket connections for real-time updates func (app *WebApp) HandleWebSocket(w http.ResponseWriter, r *http.Request) { conn, err := app.Upgrader.Upgrade(w, r, nil) @@ -703,19 +767,22 @@ func (app *WebApp) HandleDeviceWebSocket(w http.ResponseWriter, r *http.Request) } defer conn.Close() + app.registerDeviceWebSocketClient(conn) + defer app.removeDeviceWebSocketClient(conn) + log.Printf("Device WebSocket connected for %s", sanitizeLog(deviceID)) - // Send initial device status - initialMessage := webtypes.WebSocketMessage{ - Type: "device_status", - DeviceID: deviceID, - Data: map[string]interface{}{ - "info": device.DeviceInfo, - "status": device.Status(), - }, - } - - if err := conn.WriteJSON(initialMessage); err != nil { + // Capture and send under the same ordering seam used by lifecycle responses. + if err := app.withDeviceConnWrite(conn, func(batch webSocketWriteBatch) error { + return batch.writeJSON(conn, webtypes.WebSocketMessage{ + Type: "device_status", + DeviceID: deviceID, + Data: map[string]interface{}{ + "info": device.DeviceInfo, + "status": device.Status(), + }, + }) + }); err != nil { log.Printf("Failed to send initial device status: %v", err) return } @@ -746,45 +813,50 @@ func (app *WebApp) HandleDeviceWebSocket(w http.ResponseWriter, r *http.Request) defer ticker.Stop() for range ticker.C { - // Send ping to check if client is still connected - if err := conn.WriteMessage(websocket.PingMessage, []byte{}); err != nil { - log.Printf("Failed to send ping to device WebSocket %s: %v", sanitizeLog(deviceID), err) + if err := app.writeDeviceWebSocketUpdate(conn, deviceID, device); err != nil { + log.Printf("Failed to send device WebSocket update for %s: %v", sanitizeLog(deviceID), err) return } + } +} - // Send device status update +func (app *WebApp) writeDeviceWebSocketUpdate( + conn webSocketWriter, + deviceID string, + device *webtypes.DeviceConnection, +) error { + return app.withDeviceConnWrite(conn, func(batch webSocketWriteBatch) error { + // Capture after taking the lifecycle ordering lock. A status frame + // captured before a pair mutation therefore cannot follow its response. status := device.Status() - statusMessage := webtypes.WebSocketMessage{ + + if err := batch.writeMessage(conn, websocket.PingMessage, []byte{}); err != nil { + return err + } + + if err := batch.writeJSON(conn, webtypes.WebSocketMessage{ Type: "device_status", DeviceID: deviceID, Data: map[string]interface{}{ "info": device.DeviceInfo, "status": status, }, + }); err != nil { + return err } - if err := conn.WriteJSON(statusMessage); err != nil { - log.Printf("Failed to send device status update for %s: %v", sanitizeLog(deviceID), err) - return + if device.WebSocket == nil || !status.IsConnected { + return nil } - // If device has active WebSocket connection to SoundTouch device, - // also send any real-time updates from that connection - if device.WebSocket != nil && status.IsConnected { - realtimeMessage := webtypes.WebSocketMessage{ - Type: "device_realtime", - DeviceID: deviceID, - Data: map[string]interface{}{ - "nowPlaying": status.NowPlaying, - "volume": status.Volume, - "timestamp": time.Now(), - }, - } - - if err := conn.WriteJSON(realtimeMessage); err != nil { - log.Printf("Failed to send realtime update for %s: %v", sanitizeLog(deviceID), err) - return - } - } - } + return batch.writeJSON(conn, webtypes.WebSocketMessage{ + Type: "device_realtime", + DeviceID: deviceID, + Data: map[string]interface{}{ + "nowPlaying": status.NowPlaying, + "volume": status.Volume, + "timestamp": time.Now(), + }, + }) + }) } diff --git a/pkg/service/soundtouchweb/websocket_test.go b/pkg/service/soundtouchweb/websocket_test.go index 8dd4e7bd..ab464923 100644 --- a/pkg/service/soundtouchweb/websocket_test.go +++ b/pkg/service/soundtouchweb/websocket_test.go @@ -217,6 +217,61 @@ func TestPeriodicPlayerMessagesPreserveStatusUpdateStream(t *testing.T) { } } +// TestDeviceWebSocketUpdateCapturesStatusAfterLifecycleOrderingBarrier proves +// writeDeviceWebSocketUpdate captures status only after taking this specific +// connection's own write lock (registerDeviceWebSocketClient), not before -- +// the same "capture after taking the lock" guarantee awaitPriorGlobalWebSocketWrites +// relies on for the ordering barrier in completeStereoPairMutation. Holding +// the lock from before the update goroutine starts, across the mutation, +// makes this deterministic via happens-before rather than timing: the update +// can only proceed once we release the lock, by which point ApplyGroupEvent +// has already run in this (the only other) goroutine. +func TestDeviceWebSocketUpdateCapturesStatusAfterLifecycleOrderingBarrier(t *testing.T) { + app := NewWebApp() + device := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{DeviceID: "left-id"}) + device.SetStatus(&webtypes.DeviceStatus{Group: &models.Group{ID: "pair-old"}}) + + recorder := &recordingWebSocketWriter{} + app.registerDeviceWebSocketClient(recorder) + defer app.removeDeviceWebSocketClient(recorder) + + app.DeviceWSMutex.RLock() + connMu := app.DeviceWSClients[recorder] + app.DeviceWSMutex.RUnlock() + + connMu.Lock() + + updateDone := make(chan error, 1) + go func() { + updateDone <- app.writeDeviceWebSocketUpdate(recorder, "left-id", device) + }() + + device.ApplyGroupEvent(&models.Group{ID: "pair-new"}, time.Now()) + connMu.Unlock() + + select { + case err := <-updateDone: + if err != nil { + t.Fatalf("writeDeviceWebSocketUpdate: %v", err) + } + case <-time.After(time.Second): + t.Fatal("device WebSocket update remained blocked") + } + + message, ok := recorder.firstMessage() + if !ok || message.Type != "device_status" { + t.Fatalf("first device message = %#v", message) + } + data, ok := message.Data.(map[string]interface{}) + if !ok { + t.Fatalf("device message data = %#v", message.Data) + } + status, ok := data["status"].(*webtypes.DeviceStatus) + if !ok || status.Group == nil || status.Group.ID != "pair-new" { + t.Fatalf("device frame captured stale group: %#v", data["status"]) + } +} + func newStatusTestServer(t *testing.T, groupStatus int, groupBody string) *httptest.Server { t.Helper() @@ -298,6 +353,18 @@ func (writer *recordingWebSocketWriter) messageSnapshot() []interface{} { return append([]interface{}(nil), writer.messages...) } +func (writer *recordingWebSocketWriter) firstMessage() (webtypes.WebSocketMessage, bool) { + writer.mu.Lock() + defer writer.mu.Unlock() + if len(writer.messages) == 0 { + return webtypes.WebSocketMessage{}, false + } + + message, ok := writer.messages[0].(webtypes.WebSocketMessage) + + return message, ok +} + type deadlineBlockingWebSocketWriter struct { mu sync.Mutex deadline time.Time diff --git a/pkg/stereopair/coordinator.go b/pkg/stereopair/coordinator.go new file mode 100644 index 00000000..1d2faf7b --- /dev/null +++ b/pkg/stereopair/coordinator.go @@ -0,0 +1,1617 @@ +// Package stereopair coordinates the persistent stereo-pair lifecycle across +// both SoundTouch 10 speakers in a pair. +package stereopair + +import ( + "errors" + "fmt" + "net" + "net/url" + "strings" + "sync" + "time" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +// RequestTimeout covers the speaker's observed 15-second Marge retry cycle +// with enough room for multiple attempts before an AddGroup outcome is treated +// as uncertain. +const RequestTimeout = 45 * time.Second + +// Client is the subset of the SoundTouch client needed for stereo-pair +// lifecycle operations. +type Client interface { + GetDeviceInfo() (*models.DeviceInfo, error) + GetCapabilities() (*models.Capabilities, error) + GetZone() (*models.ZoneInfo, error) + GetGroup() (*models.Group, error) + AddGroup(*models.Group) (*models.Group, error) + UpdateGroup(*models.Group) (*models.Group, error) + RemoveGroup() error +} + +// ClientFactory returns a client addressed directly to one speaker IP. +type ClientFactory func(ipAddress string) (Client, error) + +// GenerationRef identifies one exact persisted stereo-pair generation. +// DeviceInfo values come from the freshly verified physical master, rather +// than from a player registry cache. +type GenerationRef struct { + GroupID string + DeviceID string + AccountID string + MargeURL string + ExpectedGroup *models.Group +} + +// GenerationCleanup removes one exact persisted stereo-pair generation. +// Implementations must be idempotent. It runs while the coordinator's +// mutation lock is held, after physical teardown has been verified. +type GenerationCleanup func(GenerationRef) error + +// GenerationPreflight verifies that no persisted group record remains for +// speakers whose fresh physical state is standalone. It must be read-only and +// fail closed; exact generation retirement is a separate operation. +type GenerationPreflight func([]GenerationRef) error + +// GenerationRename updates the name of one exact persisted generation. It +// must be idempotent so a degraded physical/backend rename can be retried. +type GenerationRename func(GenerationRef, string) error + +var ( + // ErrInvalidRequest marks malformed lifecycle input. + ErrInvalidRequest = errors.New("invalid stereo-pair request") + // ErrConflict marks a fresh speaker state that no longer matches the request. + ErrConflict = errors.New("stereo-pair state conflict") + // ErrUnavailable marks a speaker or transport operation that could not complete. + ErrUnavailable = errors.New("stereo-pair speaker unavailable") + errNoConfiguredPair = errors.New("no stereo pair configured") + errUncertainMutation = errors.New("stereo-pair mutation outcome uncertain") +) + +// Status describes the verified result of a lifecycle operation. +type Status string + +const ( + // StatusSucceeded means every requested postcondition was freshly verified. + StatusSucceeded Status = "succeeded" + // StatusFailed means no unverified partial mutation remains. + StatusFailed Status = "failed" + // StatusDegraded means the distributed state could not be fully verified or restored. + StatusDegraded Status = "degraded" +) + +// Operation identifies a lifecycle operation. +type Operation string + +const ( + // OperationInspect reads stereo-pair state without mutation. + OperationInspect Operation = "inspect" + // OperationCreate forms a stereo pair. + OperationCreate Operation = "create" + // OperationRename renames an existing stereo pair. + OperationRename Operation = "rename" + // OperationDissolve removes an existing stereo pair. + OperationDissolve Operation = "dissolve" +) + +// MemberResult records what happened on one speaker. Group is always from the +// final fresh verification read, never from a mutation response. +type MemberResult struct { + IPAddress string + DeviceID string + Reachable bool + + PreflightError error + + MutationAttempted bool + MutationError error + + Verified bool + VerificationError error + Group *models.Group + + CompensationAttempted bool + CompensationError error + CompensationVerified bool +} + +// Result describes the verified distributed outcome of an operation. +type Result struct { + Operation Operation + Status Status + Group *models.Group + Members []MemberResult + + CompensationAttempted bool + CompensationComplete bool + + PersistenceAttempted bool + PersistenceComplete bool + PersistenceError error +} + +// Error reports a lifecycle operation that did not reach its requested +// verified postcondition. Result carries the per-member details. +type Error struct { + Operation Operation + Status Status +} + +func (e *Error) Error() string { + return fmt.Sprintf("stereo-pair %s %s", e.Operation, e.Status) +} + +// CreateRequest describes a pair. LEFT is always the master. +type CreateRequest struct { + LeftIPAddress string + RightIPAddress string + Name string +} + +// RenameRequest identifies any current member and the new pair name. +type RenameRequest struct { + MemberIPAddress string + ExpectedGroupID string + Name string +} + +// DissolveRequest identifies one current member and the expected pair +// generation. The generation guard prevents a stale UI or CLI request from +// removing a newer pair. +type DissolveRequest struct { + MemberIPAddress string + ExpectedGroupID string + // ExpectedGroup is the last exact generation observed by the caller. It is + // used only when the addressed member is already standalone, so a retry can + // safely verify both known members and finish an interrupted teardown. + ExpectedGroup *models.Group +} + +// Coordinator serializes lifecycle mutations performed through one instance. +type Coordinator struct { + factory ClientFactory + generationCleanup GenerationCleanup + generationPreflight GenerationPreflight + generationRename GenerationRename + uncertainOutcomeDelays []time.Duration + mu sync.Mutex +} + +// New creates a stereo-pair lifecycle coordinator. +func New(factory ClientFactory) *Coordinator { + return NewWithGenerationCleanup(factory, nil) +} + +// NewWithGenerationCleanup creates a coordinator that also retires the exact +// persisted generation after verified physical teardown. +func NewWithGenerationCleanup(factory ClientFactory, cleanup GenerationCleanup) *Coordinator { + return NewWithGenerationPersistence(factory, cleanup, nil) +} + +// NewWithGenerationPersistence creates a coordinator with exact post-teardown +// cleanup and a read-only pre-create generation barrier. +func NewWithGenerationPersistence( + factory ClientFactory, + cleanup GenerationCleanup, + preflight GenerationPreflight, +) *Coordinator { + return NewWithGenerationLifecyclePersistence(factory, cleanup, preflight, nil) +} + +// NewWithGenerationLifecyclePersistence creates a coordinator with exact +// cleanup, pre-create checks, and persistent rename support. +func NewWithGenerationLifecyclePersistence( + factory ClientFactory, + cleanup GenerationCleanup, + preflight GenerationPreflight, + rename GenerationRename, +) *Coordinator { + return &Coordinator{ + factory: factory, + generationCleanup: cleanup, + generationPreflight: preflight, + generationRename: rename, + uncertainOutcomeDelays: []time.Duration{time.Second, 4 * time.Second, 10 * time.Second}, + } +} + +type memberState struct { + result MemberResult + client Client + info *models.DeviceInfo + group *models.Group + mutationGroupID string + mutationGroup *models.Group + + compensationVerificationError error +} + +// Inspect reads the addressed speaker and, for a configured pair, freshly +// reads both members. Group is populated only when those member reads agree. +func (c *Coordinator) Inspect(memberIPAddress string) (Result, error) { + result := Result{Operation: OperationInspect, Status: StatusFailed} + if c.factory == nil { + states := []memberState{{result: MemberResult{ + IPAddress: memberIPAddress, + PreflightError: fmt.Errorf("%w: client factory is nil", ErrUnavailable), + }}} + + return finish(result, states) + } + + entry, err := c.factory(memberIPAddress) + if err != nil { + states := []memberState{{result: MemberResult{ + IPAddress: memberIPAddress, + PreflightError: wrapUnavailable("create client", err), + }}} + + return finish(result, states) + } + + current, err := entry.GetGroup() + if err != nil { + states := []memberState{{result: MemberResult{ + IPAddress: memberIPAddress, + PreflightError: wrapUnavailable("get current group", err), + }}} + + return finish(result, states) + } + + if current == nil { + states := []memberState{{result: MemberResult{ + IPAddress: memberIPAddress, + Reachable: true, + PreflightError: wrapUnavailable("get current group", errors.New("nil response")), + }}} + + return finish(result, states) + } + + if current.IsEmpty() { + states := []memberState{{result: MemberResult{IPAddress: memberIPAddress, Reachable: true, Verified: true, Group: cloneGroup(current)}}} + result.Status = StatusSucceeded + result.Group = cloneGroup(current) + + return finish(result, states) + } + + members, err := pairMembers(current) + if err != nil { + states := []memberState{{result: MemberResult{ + IPAddress: memberIPAddress, + Reachable: true, + PreflightError: fmt.Errorf("%w: %w", ErrConflict, err), + Group: cloneGroup(current), + }}} + + return finish(result, states) + } + + // Preserve the initiating member's exact generation even when the peer is + // already empty or unreachable. Clients can then retry a partial dissolve + // through the surviving grouped member instead of losing the recovery key. + result.Group = cloneGroup(current) + + states := make([]memberState, len(members)) + for i, member := range members { + states[i].result.IPAddress = member.IPAddress + states[i].result.DeviceID = member.DeviceID + + states[i].client, err = c.factory(member.IPAddress) + if err != nil { + states[i].result.PreflightError = wrapUnavailable("create client", err) + continue + } + + states[i].group, err = states[i].client.GetGroup() + + states[i].result.Group = cloneGroup(states[i].group) + if err != nil { + states[i].result.VerificationError = wrapUnavailable("get group", err) + continue + } + + states[i].result.Reachable = true + if err := verifyConfiguredGroup(states[i].group); err != nil { + states[i].result.VerificationError = err + continue + } + + if !sameGroupConfiguration(states[i].group, current) { + states[i].result.VerificationError = errors.New("members do not agree on the current group") + continue + } + + states[i].result.Verified = true + } + + if pairVerified(states) { + result.Status = StatusSucceeded + result.Group = cloneGroup(states[0].group) + } else { + result.Status = StatusDegraded + } + + return finish(result, states) +} + +func (c *Coordinator) renameGeneration(result *Result, ref GenerationRef, name string) error { + if c.generationRename == nil { + return nil + } + + result.PersistenceAttempted = true + result.PersistenceComplete = false + + if ref.GroupID == "" || ref.DeviceID == "" || ref.ExpectedGroup == nil || + ref.ExpectedGroup.ID != ref.GroupID || ref.ExpectedGroup.MasterDeviceID != ref.DeviceID { + result.PersistenceError = fmt.Errorf("%w: persisted generation has no exact group topology", ErrConflict) + + return result.PersistenceError + } + + if err := c.generationRename(ref, name); err != nil { + if errors.Is(err, ErrInvalidRequest) || errors.Is(err, ErrConflict) || errors.Is(err, ErrUnavailable) { + result.PersistenceError = fmt.Errorf("rename persisted group generation: %w", err) + } else { + result.PersistenceError = wrapUnavailable("rename persisted group generation", err) + } + + return result.PersistenceError + } + + result.PersistenceComplete = true + result.PersistenceError = nil + + return nil +} + +// Create forms a persistent stereo pair and verifies both speakers with fresh +// GetGroup calls. If only a subset adopts the requested pair, Create removes +// only freshly observed exact matches and reports whether cleanup completed. +func (c *Coordinator) Create(req CreateRequest) (Result, error) { + c.mu.Lock() + defer c.mu.Unlock() + + states := []memberState{ + {result: MemberResult{IPAddress: req.LeftIPAddress}}, + {result: MemberResult{IPAddress: req.RightIPAddress}}, + } + result := Result{Operation: OperationCreate, Status: StatusFailed} + + expected := c.prepareCreate(req, states, &result) + if expected == nil { + return finish(result, states) + } + + c.applyCreate(states, expected, req.LeftIPAddress) + + verifyPair(states, func(group *models.Group) error { + return verifyCreatedGroup(group, expected) + }) + + if !pairVerified(states) && hasUncertainMutationOutcome(states) { + for _, delay := range c.uncertainOutcomeDelays { + time.Sleep(delay) + verifyPair(states, func(group *models.Group) error { + return verifyCreatedGroup(group, expected) + }) + + if pairVerified(states) { + break + } + } + } + + captureVerifiedGenerationIDs(states, expected) + + if pairVerified(states) { + result.Status = StatusSucceeded + result.Group = cloneGroup(states[0].group) + + return finish(result, states) + } + + c.compensateCreate(&result, states, expected) + + return finish(result, states) +} + +func (c *Coordinator) prepareCreate(req CreateRequest, states []memberState, result *Result) *models.Group { + if c.factory == nil { + states[0].result.PreflightError = fmt.Errorf("%w: client factory is nil", ErrUnavailable) + + return nil + } + + for i := range states { + c.preflightCreate(&states[i]) + } + + validateCreateCandidates(states) + + if hasPreflightError(states) { + return nil + } + + if c.generationPreflight != nil { + result.PersistenceAttempted = true + if err := c.generationPreflight(standaloneGenerationRefs(states)); err != nil { + result.PersistenceError = wrapUnavailable("check persisted group generations", err) + setPreflightError(&states[0], result.PersistenceError) + + return nil + } + + result.PersistenceComplete = true + + c.revalidateCreateCandidates(states) + validateCreateCandidates(states) + + if hasPreflightError(states) { + return nil + } + } + + name := req.Name + if name == "" { + name = states[0].info.Name + " + " + states[1].info.Name + } + + return &models.Group{ + Name: name, + MasterDeviceID: states[0].info.DeviceID, + Roles: models.GroupRoles{Roles: []models.GroupRole{ + {DeviceID: states[0].info.DeviceID, Role: "LEFT", IPAddress: req.LeftIPAddress}, + {DeviceID: states[1].info.DeviceID, Role: "RIGHT", IPAddress: req.RightIPAddress}, + }}, + } +} + +func validateCreateCandidates(states []memberState) { + if len(states) != 2 { + return + } + + if sameIP(states[0].result.IPAddress, states[1].result.IPAddress) { + setPreflightError(&states[1], fmt.Errorf("%w: LEFT and RIGHT IP addresses must be distinct", ErrInvalidRequest)) + } + + if states[0].info == nil || states[1].info == nil { + return + } + + if states[0].info.DeviceID == states[1].info.DeviceID { + setPreflightError(&states[1], fmt.Errorf("%w: LEFT and RIGHT device IDs must be distinct", ErrInvalidRequest)) + } + + if strings.TrimSpace(states[0].info.MargeAccountUUID) == "" || + states[0].info.MargeAccountUUID != states[1].info.MargeAccountUUID { + setPreflightError(&states[1], fmt.Errorf("%w: LEFT and RIGHT speakers must use the same Marge account", ErrConflict)) + } + + if !SameMargeBackend(states[0].info.MargeURL, states[1].info.MargeURL) { + setPreflightError(&states[1], fmt.Errorf("%w: LEFT and RIGHT speakers must use the same Marge backend", ErrConflict)) + } +} + +func (c *Coordinator) revalidateCreateCandidates(states []memberState) { + for i := range states { + previous := states[i].info + fresh := memberState{result: MemberResult{IPAddress: states[i].result.IPAddress}} + c.preflightCreate(&fresh) + + if fresh.result.PreflightError == nil && !sameCreateIdentity(previous, fresh.info) { + fresh.result.PreflightError = fmt.Errorf("%w: speaker identity or Marge ownership changed during preflight", ErrConflict) + } + + states[i] = fresh + } +} + +func sameCreateIdentity(previous, current *models.DeviceInfo) bool { + return previous != nil && current != nil && + previous.DeviceID == current.DeviceID && + previous.MargeAccountUUID == current.MargeAccountUUID && + SameMargeBackend(previous.MargeURL, current.MargeURL) +} + +func (c *Coordinator) applyCreate(states []memberState, expected *models.Group, masterIPAddress string) { + if len(states) != 2 { + return + } + + masterRequest := cloneGroup(expected) + applyCreateMember(&states[0], masterRequest) + + if states[0].result.MutationError != nil && + (!isUncertainMutationOutcome(states[0].result.MutationError) || + !c.resolveCreateGeneration(&states[0], expected)) { + return + } + + // The master persists the group through Marge and receives its generated + // ID. Give that exact generation to the slave so it cannot race a Marge + // lookup against the master's create request. + slaveRequest := cloneGroup(expected) + slaveRequest.ID = states[0].mutationGroupID + slaveRequest.SenderIPAddress = masterIPAddress + applyCreateMember(&states[1], slaveRequest) +} + +func (c *Coordinator) resolveCreateGeneration(state *memberState, expected *models.Group) bool { + readGeneration := func() bool { + group, err := state.client.GetGroup() + state.group = group + state.result.Group = cloneGroup(group) + + if err != nil || verifyCreatedGroup(group, expected) != nil { + return false + } + + state.mutationGroupID = group.ID + state.mutationGroup = cloneGroup(group) + state.result.MutationError = nil + + return true + } + + if readGeneration() { + return true + } + + for _, delay := range c.uncertainOutcomeDelays { + time.Sleep(delay) + + if readGeneration() { + return true + } + } + + return false +} + +func applyCreateMember(state *memberState, request *models.Group) { + state.result.MutationAttempted = true + response, err := state.client.AddGroup(request) + state.result.MutationError = validateMutationResponse("addGroup", response, err) + + if state.result.MutationError == nil { + state.mutationGroupID = response.ID + } +} + +func (c *Coordinator) compensateCreate(result *Result, states []memberState, expected *models.Group) { + result.Status = StatusDegraded + + for i := range states { + if states[i].mutationGroupID == "" || + !matchesRequestedGeneration(states[i].group, expected, states[i].mutationGroupID) { + continue + } + + latest, err := states[i].client.GetGroup() + states[i].group = latest + + states[i].result.Group = cloneGroup(latest) + if err != nil { + states[i].result.CompensationError = wrapUnavailable("recheck group before compensation", err) + + continue + } + + if !matchesRequestedGeneration(latest, expected, states[i].mutationGroupID) { + states[i].result.CompensationError = fmt.Errorf("%w: group changed before compensation", ErrConflict) + + continue + } + + states[i].mutationGroup = cloneGroup(latest) + + result.CompensationAttempted = true + states[i].result.CompensationAttempted = true + states[i].result.CompensationError = wrapUnavailable("remove group during compensation", states[i].client.RemoveGroup()) + } + + if !result.CompensationAttempted { + return + } + + verifyCompensation(states) + + if compensationReverificationPending(states) { + for _, delay := range c.uncertainOutcomeDelays { + time.Sleep(delay) + verifyCompensation(states) + + if !compensationReverificationPending(states) { + break + } + } + } + + result.CompensationComplete = compensationVerified(states) + if !result.CompensationComplete { + for i := range states { + if !states[i].result.CompensationVerified && states[i].result.CompensationError == nil { + states[i].result.CompensationError = states[i].compensationVerificationError + } + } + + return + } + + for _, ref := range compensationGenerations(states) { + if err := c.cleanupGeneration(result, ref); err != nil { + result.CompensationComplete = false + result.Status = StatusDegraded + + return + } + } + + result.Status = StatusFailed +} + +// Rename updates the full current group on both members, then verifies that +// both speakers expose the same renamed pair. +func (c *Coordinator) Rename(req RenameRequest) (Result, error) { + c.mu.Lock() + defer c.mu.Unlock() + + result := Result{Operation: OperationRename, Status: StatusFailed} + + name := strings.TrimSpace(req.Name) + if name == "" || strings.TrimSpace(req.ExpectedGroupID) == "" { + states := []memberState{{result: MemberResult{ + IPAddress: req.MemberIPAddress, + PreflightError: fmt.Errorf("%w: pair name and expected group ID are required", + ErrInvalidRequest), + }}} + + return finish(result, states) + } + + states, current, err := c.loadPair(req.MemberIPAddress, req.ExpectedGroupID, false) + if err != nil { + if len(states) == 0 { + states = []memberState{{result: MemberResult{IPAddress: req.MemberIPAddress, PreflightError: err}}} + } + + return finish(result, states) + } + + applyRename(states, current, name) + + expected := cloneGroup(current) + expected.Name = name + + verifyRenamedPair := func(group *models.Group) error { + if err := verifyConfiguredGroup(group); err != nil { + return err + } + + if !sameGroupConfiguration(group, expected) { + return errors.New("group does not match the renamed pair") + } + + return nil + } + verifyPair(states, verifyRenamedPair) + + if pairVerified(states) { + result.Group = cloneGroup(states[0].group) + + renameErr := c.renameGeneration(&result, generationRef(states, current), name) + if c.generationRename != nil { + // Persistence can cross a process boundary, so verify the physical + // postcondition once more before reporting distributed success. + verifyPair(states, verifyRenamedPair) + + if pairVerified(states) { + result.Group = cloneGroup(states[0].group) + } else { + result.Group = nil + } + } + + if renameErr == nil && pairVerified(states) { + result.Status = StatusSucceeded + } else { + result.Status = StatusDegraded + } + } else { + result.Status = StatusDegraded + } + + return finish(result, states) +} + +func applyRename(states []memberState, current *models.Group, name string) { + var wg sync.WaitGroup + + for i := range states { + wg.Add(1) + + go func(index int) { + defer wg.Done() + + states[index].result.MutationAttempted = true + + latest, err := states[index].client.GetGroup() + if err != nil { + states[index].result.MutationError = wrapUnavailable("recheck group before rename", err) + + return + } + + if !sameGroupConfiguration(latest, current) { + states[index].result.MutationError = fmt.Errorf("%w: group changed before rename", ErrConflict) + + return + } + + update := cloneGroup(latest) + update.Name = name + update.Status = "" + response, updateErr := states[index].client.UpdateGroup(update) + states[index].result.MutationError = validateMutationResponse("updateGroup", response, updateErr) + }(i) + } + + wg.Wait() +} + +// Dissolve removes the current pair from every member that passes a fresh +// group read, then verifies empty groups on both speakers. +func (c *Coordinator) Dissolve(req DissolveRequest) (Result, error) { + c.mu.Lock() + defer c.mu.Unlock() + + result := Result{Operation: OperationDissolve, Status: StatusFailed} + if strings.TrimSpace(req.ExpectedGroupID) == "" { + states := []memberState{{result: MemberResult{ + IPAddress: req.MemberIPAddress, + PreflightError: fmt.Errorf("%w: expected group ID is required", ErrInvalidRequest), + }}} + + return finish(result, states) + } + + states, current, err := c.loadPair(req.MemberIPAddress, req.ExpectedGroupID, true) + if err != nil && errors.Is(err, errNoConfiguredPair) && req.ExpectedGroup != nil { + states, current, err = c.loadPairFromSnapshot(req.MemberIPAddress, req.ExpectedGroupID, req.ExpectedGroup) + } + + if err != nil { + if len(states) == 0 { + states = []memberState{{result: MemberResult{IPAddress: req.MemberIPAddress, PreflightError: err}}} + } + + return finish(result, states) + } + + result.Group = cloneGroup(current) + + applyDissolve(states, current) + verifyDissolved(states) + + if removalReverificationPending(states) { + for _, delay := range c.uncertainOutcomeDelays { + time.Sleep(delay) + verifyDissolved(states) + + if !removalReverificationPending(states) { + break + } + } + } + + if allVerified(states) { + result.Group = &models.Group{} + if cleanupErr := c.cleanupGeneration(&result, generationRef(states, current)); cleanupErr == nil { + result.Status = StatusSucceeded + } else { + result.Status = StatusDegraded + } + } else { + result.Status = StatusDegraded + } + + return finish(result, states) +} + +func applyDissolve(states []memberState, current *models.Group) { + var wg sync.WaitGroup + + for i := range states { + wg.Add(1) + + go func(index int) { + defer wg.Done() + + latest, err := states[index].client.GetGroup() + if err != nil { + states[index].result.MutationError = wrapUnavailable("recheck group before dissolve", err) + + return + } + + if latest == nil { + states[index].result.MutationError = fmt.Errorf("%w: group changed before dissolve", ErrConflict) + + return + } + + if latest.IsEmpty() { + return + } + + if !sameGroupConfiguration(latest, current) { + states[index].result.MutationError = fmt.Errorf("%w: group changed before dissolve", ErrConflict) + + return + } + + states[index].result.MutationAttempted = true + states[index].result.MutationError = wrapUnavailable("remove group", states[index].client.RemoveGroup()) + }(i) + } + + wg.Wait() +} + +func verifyDissolved(states []memberState) { + for i := range states { + states[i].result.Verified = false + states[i].result.VerificationError = nil + + group, err := states[i].client.GetGroup() + states[i].group = group + + states[i].result.Group = cloneGroup(group) + + switch { + case err != nil: + states[i].result.VerificationError = wrapUnavailable("get group after dissolve", err) + case group == nil || !group.IsEmpty(): + states[i].result.VerificationError = errors.New("group is not empty after dissolve") + default: + states[i].result.Verified = true + } + } +} + +func removalReverificationPending(states []memberState) bool { + for i := range states { + if states[i].result.MutationAttempted && !states[i].result.Verified { + return true + } + } + + return false +} + +func verifyCompensation(states []memberState) { + for i := range states { + states[i].result.CompensationVerified = false + states[i].compensationVerificationError = nil + + group, err := states[i].client.GetGroup() + states[i].group = group + states[i].result.Group = cloneGroup(group) + + switch { + case err != nil: + states[i].compensationVerificationError = wrapUnavailable("verify compensation", err) + case group == nil || !group.IsEmpty(): + states[i].compensationVerificationError = errors.New("compensation did not leave an empty group") + default: + states[i].result.CompensationVerified = true + } + } +} + +func compensationVerified(states []memberState) bool { + if len(states) == 0 { + return false + } + + for i := range states { + if !states[i].result.CompensationVerified { + return false + } + } + + return true +} + +func compensationReverificationPending(states []memberState) bool { + for i := range states { + if states[i].result.CompensationAttempted && !states[i].result.CompensationVerified { + return true + } + } + + return false +} + +func (c *Coordinator) cleanupGeneration(result *Result, ref GenerationRef) error { + if c.generationCleanup == nil { + return nil + } + + result.PersistenceAttempted = true + result.PersistenceComplete = false + + if ref.GroupID == "" || ref.DeviceID == "" || ref.ExpectedGroup == nil || + ref.ExpectedGroup.ID != ref.GroupID || ref.ExpectedGroup.MasterDeviceID != ref.DeviceID { + result.PersistenceError = fmt.Errorf("%w: persisted generation has no exact group topology", ErrConflict) + + return result.PersistenceError + } + + if err := c.generationCleanup(ref); err != nil { + result.PersistenceError = wrapUnavailable("remove persisted group generation", err) + + return result.PersistenceError + } + + result.PersistenceComplete = true + result.PersistenceError = nil + + return nil +} + +func generationRef(states []memberState, group *models.Group) GenerationRef { + if group == nil { + return GenerationRef{} + } + + for i := range states { + if states[i].info == nil || states[i].info.DeviceID != group.MasterDeviceID { + continue + } + + ref := generationRefForState(&states[i], group.ID) + ref.ExpectedGroup = cloneGroup(group) + + return ref + } + + return GenerationRef{GroupID: group.ID, ExpectedGroup: cloneGroup(group)} +} + +func compensationGenerations(states []memberState) []GenerationRef { + seen := make(map[string]bool) + refs := make([]GenerationRef, 0, len(states)) + + for i := range states { + groupID := states[i].mutationGroupID + if groupID == "" || seen[groupID] { + continue + } + + seen[groupID] = true + ref := generationRef(states, states[i].mutationGroup) + refs = append(refs, ref) + } + + return refs +} + +func generationRefForState(state *memberState, groupID string) GenerationRef { + ref := GenerationRef{GroupID: groupID, DeviceID: state.result.DeviceID} + if state.info == nil { + return ref + } + + ref.DeviceID = state.info.DeviceID + ref.AccountID = state.info.MargeAccountUUID + ref.MargeURL = state.info.MargeURL + + return ref +} + +func standaloneGenerationRefs(states []memberState) []GenerationRef { + refs := make([]GenerationRef, 0, len(states)) + for i := range states { + refs = append(refs, generationRefForState(&states[i], "")) + } + + return refs +} + +func (c *Coordinator) preflightCreate(state *memberState) { + if net.ParseIP(state.result.IPAddress) == nil { + state.result.PreflightError = fmt.Errorf("%w: invalid speaker IP address", ErrInvalidRequest) + return + } + + client, err := c.factory(state.result.IPAddress) + if err != nil { + state.result.PreflightError = wrapUnavailable("create client", err) + return + } + + state.client = client + + state.info, err = client.GetDeviceInfo() + if err != nil { + state.result.PreflightError = wrapUnavailable("get device info", err) + return + } + + state.result.Reachable = true + if state.info == nil { + state.result.PreflightError = wrapUnavailable("get device info", errors.New("nil response")) + return + } + + state.result.DeviceID = state.info.DeviceID + if state.info.DeviceID == "" { + state.result.PreflightError = fmt.Errorf("%w: device info has no device ID", ErrConflict) + return + } + + capabilities, err := client.GetCapabilities() + if err != nil { + state.result.PreflightError = wrapUnavailable("get capabilities", err) + return + } + + if !isST10(state.info) || capabilities == nil || !capabilities.HasLRStereoCapability() { + state.result.PreflightError = fmt.Errorf("%w: speaker is not an ST10 with L/R stereo capability", ErrConflict) + return + } + + if capabilities.DeviceID != "" && capabilities.DeviceID != state.info.DeviceID { + state.result.PreflightError = fmt.Errorf("%w: capabilities do not match the physical speaker", ErrConflict) + return + } + + zone, err := client.GetZone() + if err != nil { + state.result.PreflightError = wrapUnavailable("get zone", err) + return + } + + if zone == nil || !zone.IsStandalone() { + state.result.PreflightError = fmt.Errorf("%w: speaker is currently in a zone", ErrConflict) + return + } + + state.group, err = client.GetGroup() + if err != nil { + state.result.PreflightError = wrapUnavailable("get group", err) + return + } + + if state.group == nil || !state.group.IsEmpty() { + state.result.PreflightError = fmt.Errorf("%w: speaker is currently grouped", ErrConflict) + } +} + +func (c *Coordinator) preflightStandaloneZone(state *memberState) { + zone, err := state.client.GetZone() + if err != nil { + setPreflightError(state, wrapUnavailable("get zone", err)) + return + } + + state.result.Reachable = true + if zone == nil { + setPreflightError(state, wrapUnavailable("get zone", errors.New("nil response"))) + } else if !zone.IsStandalone() { + setPreflightError(state, fmt.Errorf("%w: speaker is currently in a zone", ErrConflict)) + } +} + +func (c *Coordinator) loadPair(memberIPAddress, expectedGroupID string, allowEmptyMembers bool) ([]memberState, *models.Group, error) { + if c.factory == nil { + return nil, nil, fmt.Errorf("%w: client factory is nil", ErrUnavailable) + } + + entry, err := c.factory(memberIPAddress) + if err != nil { + return nil, nil, wrapUnavailable("create client", err) + } + + current, err := entry.GetGroup() + if err != nil { + return nil, nil, wrapUnavailable("get current group", err) + } + + if current == nil { + return nil, nil, wrapUnavailable("get current group", errors.New("nil response")) + } + + if current.IsEmpty() { + return nil, nil, fmt.Errorf("%w: %w", ErrConflict, errNoConfiguredPair) + } + + if current.ID != expectedGroupID { + return nil, nil, fmt.Errorf("%w: expected group %q, found %q", ErrConflict, expectedGroupID, current.ID) + } + + if verifyErr := verifyConfiguredGroup(current); verifyErr != nil { + return nil, nil, fmt.Errorf("%w: invalid current group: %w", ErrConflict, verifyErr) + } + + members, err := pairMembers(current) + if err != nil { + return nil, nil, err + } + + entryInfo, err := entry.GetDeviceInfo() + if err != nil { + return nil, nil, wrapUnavailable("get initiating device info", err) + } + + if entryInfo == nil || !groupContainsDevice(current, entryInfo.DeviceID) { + return nil, nil, fmt.Errorf("%w: initiating endpoint is not a member of the expected pair", ErrConflict) + } + + states := make([]memberState, len(members)) + for i, member := range members { + c.preflightExistingMember(&states[i], member, current, allowEmptyMembers) + } + + validateCreateCandidates(states) + + if hasPreflightError(states) { + return states, nil, errors.New("pair preflight failed") + } + + return states, current, nil +} + +// loadPairFromSnapshot recovers only an exact generation supplied by a caller +// that observed it before an interrupted dissolve. Every identity, capability, +// zone, and group state is read again from both physical role addresses. +func (c *Coordinator) loadPairFromSnapshot( + memberIPAddress string, + expectedGroupID string, + snapshot *models.Group, +) ([]memberState, *models.Group, error) { + current := cloneGroup(snapshot) + if current == nil || current.ID != expectedGroupID { + return nil, nil, fmt.Errorf("%w: recovery snapshot does not match expected group", ErrConflict) + } + + if err := verifyConfiguredGroup(current); err != nil { + return nil, nil, fmt.Errorf("%w: invalid recovery snapshot: %w", ErrConflict, err) + } + + members, err := pairMembers(current) + if err != nil { + return nil, nil, fmt.Errorf("%w: invalid recovery snapshot: %w", ErrConflict, err) + } + + entry, err := c.factory(memberIPAddress) + if err != nil { + return nil, nil, wrapUnavailable("create recovery client", err) + } + + entryInfo, err := entry.GetDeviceInfo() + if err != nil { + return nil, nil, wrapUnavailable("get initiating device info", err) + } + + if entryInfo == nil || !groupContainsDevice(current, entryInfo.DeviceID) { + return nil, nil, fmt.Errorf("%w: initiating endpoint is not a member of the recovery snapshot", ErrConflict) + } + + states := make([]memberState, len(members)) + for i, member := range members { + c.preflightExistingMember(&states[i], member, current, true) + } + + validateCreateCandidates(states) + + if hasPreflightError(states) { + return states, nil, errors.New("recovery snapshot preflight failed") + } + + return states, current, nil +} + +func (c *Coordinator) preflightExistingMember( + state *memberState, + member models.GroupRole, + current *models.Group, + allowEmpty bool, +) { + state.result.IPAddress = member.IPAddress + state.result.DeviceID = member.DeviceID + + client, err := c.factory(member.IPAddress) + if err != nil { + state.result.PreflightError = wrapUnavailable("create client", err) + + return + } + + state.client = client + + state.info, err = client.GetDeviceInfo() + if err != nil { + state.result.PreflightError = wrapUnavailable("get device info", err) + + return + } + + state.result.Reachable = true + if state.info == nil || state.info.DeviceID != member.DeviceID { + state.result.PreflightError = fmt.Errorf("%w: group role does not match the physical speaker", ErrConflict) + + return + } + + capabilities, err := client.GetCapabilities() + if err != nil { + state.result.PreflightError = wrapUnavailable("get capabilities", err) + + return + } + + if !isST10(state.info) || capabilities == nil || !capabilities.HasLRStereoCapability() { + state.result.PreflightError = fmt.Errorf("%w: speaker is not an ST10 with L/R stereo capability", ErrConflict) + + return + } + + if capabilities.DeviceID != "" && capabilities.DeviceID != state.info.DeviceID { + state.result.PreflightError = fmt.Errorf("%w: capabilities do not match the physical speaker", ErrConflict) + + return + } + + state.group, err = client.GetGroup() + switch { + case err != nil: + state.result.PreflightError = wrapUnavailable("get current group", err) + case state.group == nil: + state.result.PreflightError = wrapUnavailable("get current group", errors.New("nil response")) + case state.group.IsEmpty() && !allowEmpty: + state.result.PreflightError = fmt.Errorf("%w: member is no longer in the expected group", ErrConflict) + case !state.group.IsEmpty() && !sameGroupConfiguration(state.group, current): + state.result.PreflightError = fmt.Errorf("%w: members do not agree on the current group", ErrConflict) + } + + c.preflightStandaloneZone(state) +} + +func verifyPair(states []memberState, verify func(*models.Group) error) { + for i := range states { + states[i].result.Verified = false + states[i].result.VerificationError = nil + + group, err := states[i].client.GetGroup() + states[i].group = group + + states[i].result.Group = cloneGroup(group) + if err != nil { + states[i].result.VerificationError = fmt.Errorf("get group after mutation: %w", err) + continue + } + + if err := verify(group); err != nil { + states[i].result.VerificationError = err + continue + } + + states[i].result.Verified = true + } + + if len(states) == 2 && states[0].result.Verified && states[1].result.Verified && + !sameGroupConfiguration(states[0].group, states[1].group) { + err := errors.New("members do not agree on the resulting group") + states[0].result.Verified = false + states[1].result.Verified = false + states[0].result.VerificationError = err + states[1].result.VerificationError = err + } +} + +func hasUncertainMutationOutcome(states []memberState) bool { + for i := range states { + if isUncertainMutationOutcome(states[i].result.MutationError) { + return true + } + } + + return false +} + +func isUncertainMutationOutcome(err error) bool { + if errors.Is(err, errUncertainMutation) { + return true + } + + var timeout interface{ Timeout() bool } + + return errors.As(err, &timeout) && timeout.Timeout() +} + +func captureVerifiedGenerationIDs(states []memberState, expected *models.Group) { + for i := range states { + if !states[i].result.Verified || !matchesRequestedPair(states[i].group, expected) { + continue + } + + if states[i].mutationGroupID == "" { + states[i].mutationGroupID = states[i].group.ID + } + + if states[i].group.ID == states[i].mutationGroupID { + states[i].mutationGroup = cloneGroup(states[i].group) + } + } +} + +func verifyCreatedGroup(group, expected *models.Group) error { + if err := verifyConfiguredGroup(group); err != nil { + return err + } + + if !matchesRequestedPair(group, expected) { + return errors.New("group does not match the requested pair") + } + + return nil +} + +func validateMutationResponse(operation string, response *models.Group, err error) error { + if err != nil { + return wrapUnavailable(operation, err) + } + + if response == nil { + return wrapUnavailable(operation, errors.New("nil response")) + } + + if response.Status != "" && response.Status != "GROUP_OK" { + return fmt.Errorf("%w: %s returned status %q", ErrConflict, operation, response.Status) + } + + if response.ID == "" { + return fmt.Errorf("%w: %s response has no group ID: %w", + ErrUnavailable, operation, errUncertainMutation) + } + + return nil +} + +func verifyConfiguredGroup(group *models.Group) error { + if group == nil || group.IsEmpty() { + return errors.New("group is empty") + } + + if group.ID == "" { + return errors.New("group has no ID") + } + + if group.Status != "" && group.Status != "GROUP_OK" { + return fmt.Errorf("group status is %q", group.Status) + } + + _, err := pairMembers(group) + + return err +} + +func pairMembers(group *models.Group) ([]models.GroupRole, error) { + if group == nil || len(group.Roles.Roles) != 2 { + return nil, errors.New("stereo pair must contain exactly two roles") + } + + roles := make(map[string]models.GroupRole, 2) + + for _, role := range group.Roles.Roles { + if role.DeviceID == "" || net.ParseIP(role.IPAddress) == nil { + return nil, errors.New("stereo-pair roles require device IDs and IP addresses") + } + + if role.Role != "LEFT" && role.Role != "RIGHT" { + return nil, fmt.Errorf("invalid stereo-pair role %q", role.Role) + } + + if _, exists := roles[role.Role]; exists { + return nil, fmt.Errorf("duplicate stereo-pair role %q", role.Role) + } + + roles[role.Role] = role + } + + left, leftOK := roles["LEFT"] + + right, rightOK := roles["RIGHT"] + + if !leftOK || !rightOK || left.DeviceID == right.DeviceID || sameIP(left.IPAddress, right.IPAddress) { + return nil, errors.New("stereo pair requires distinct LEFT and RIGHT members") + } + + if group.MasterDeviceID != left.DeviceID { + return nil, errors.New("LEFT member must be the group master") + } + + return []models.GroupRole{left, right}, nil +} + +func matchesRequestedPair(group, expected *models.Group) bool { + if group == nil || expected == nil || group.Name != expected.Name || group.MasterDeviceID != expected.MasterDeviceID { + return false + } + + return sameRoles(group.Roles.Roles, expected.Roles.Roles) +} + +func matchesRequestedGeneration(group, expected *models.Group, groupID string) bool { + return groupID != "" && group != nil && group.ID == groupID && matchesRequestedPair(group, expected) +} + +func sameGroupConfiguration(a, b *models.Group) bool { + if a == nil || b == nil { + return a == b + } + + return a.ID == b.ID && a.Name == b.Name && a.MasterDeviceID == b.MasterDeviceID && sameRoles(a.Roles.Roles, b.Roles.Roles) +} + +func sameGroupTopology(a, b *models.Group) bool { + return a != nil && b != nil && a.ID == b.ID && a.MasterDeviceID == b.MasterDeviceID && + sameRoles(a.Roles.Roles, b.Roles.Roles) +} + +func sameRoles(a, b []models.GroupRole) bool { + if len(a) != len(b) { + return false + } + + byRole := make(map[string]models.GroupRole, len(a)) + for i := range a { + byRole[a[i].Role] = a[i] + } + + for i := range b { + other, ok := byRole[b[i].Role] + if !ok || other.DeviceID != b[i].DeviceID || !sameIP(other.IPAddress, b[i].IPAddress) { + return false + } + } + + return true +} + +func groupContainsDevice(group *models.Group, deviceID string) bool { + if group == nil || deviceID == "" { + return false + } + + for i := range group.Roles.Roles { + if group.Roles.Roles[i].DeviceID == deviceID { + return true + } + } + + return false +} + +func cloneGroup(group *models.Group) *models.Group { + if group == nil { + return nil + } + + clone := *group + clone.Roles.Roles = append([]models.GroupRole(nil), group.Roles.Roles...) + + return &clone +} + +func isST10(info *models.DeviceInfo) bool { + typeName := strings.TrimSpace(strings.ToLower(info.Type)) + return typeName == "st10" || typeName == "soundtouch 10" +} + +func sameIP(a, b string) bool { + left, right := net.ParseIP(a), net.ParseIP(b) + return left != nil && right != nil && left.Equal(right) +} + +// SameMargeBackend reports whether two Marge base URLs resolve to the same +// normalized streaming endpoint. +func SameMargeBackend(a, b string) bool { + left, leftErr := margeStreamingURL(a) + + right, rightErr := margeStreamingURL(b) + if leftErr != nil || rightErr != nil { + return false + } + + leftURL, leftErr := url.Parse(left) + + rightURL, rightErr := url.Parse(right) + if leftErr != nil || rightErr != nil { + return false + } + + return strings.EqualFold(leftURL.Scheme, rightURL.Scheme) && + strings.EqualFold(leftURL.Hostname(), rightURL.Hostname()) && + effectiveURLPort(leftURL) == effectiveURLPort(rightURL) && + leftURL.EscapedPath() == rightURL.EscapedPath() +} + +func effectiveURLPort(endpoint *url.URL) string { + if port := endpoint.Port(); port != "" { + return port + } + + switch strings.ToLower(endpoint.Scheme) { + case "http": + return "80" + case "https": + return "443" + default: + return "" + } +} + +func setPreflightError(state *memberState, err error) { + if state.result.PreflightError == nil { + state.result.PreflightError = err + } +} + +func wrapUnavailable(operation string, err error) error { + if err == nil { + return nil + } + + return fmt.Errorf("%w: %s: %w", ErrUnavailable, operation, err) +} + +func hasPreflightError(states []memberState) bool { + for i := range states { + if states[i].result.PreflightError != nil { + return true + } + } + + return false +} + +func allVerified(states []memberState) bool { + if len(states) == 0 { + return false + } + + for i := range states { + if !states[i].result.Verified { + return false + } + } + + return true +} + +func pairVerified(states []memberState) bool { + return len(states) == 2 && allVerified(states) +} + +func finish(result Result, states []memberState) (Result, error) { + result.Members = make([]MemberResult, len(states)) + for i := range states { + result.Members[i] = states[i].result + } + + if result.Status == StatusSucceeded { + return result, nil + } + + return result, &Error{Operation: result.Operation, Status: result.Status} +} diff --git a/pkg/stereopair/coordinator_test.go b/pkg/stereopair/coordinator_test.go new file mode 100644 index 00000000..66c2cd3d --- /dev/null +++ b/pkg/stereopair/coordinator_test.go @@ -0,0 +1,1278 @@ +package stereopair + +import ( + "errors" + "sync" + "testing" + "time" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +const ( + leftIP = "192.0.2.10" + rightIP = "192.0.2.11" + thirdIP = "192.0.2.12" + leftID = "LEFT-ID" + rightID = "RIGHT-ID" +) + +type fakeClient struct { + mu sync.Mutex + + info *models.DeviceInfo + capabilities *models.Capabilities + zone *models.ZoneInfo + group *models.Group + + infoErr error + capabilityErr error + zoneErr error + getGroupErr error + addErr error + updateErr error + removeErr error + + addRequest *models.Group + updateRequest *models.Group + removeCalls int + zoneCalls int + getGroupCalls int + addGroup func(*models.Group) *models.Group + addResponse func(*models.Group) *models.Group + getGroup func(int, *models.Group) *models.Group +} + +type fakeTimeoutError struct{} + +func (fakeTimeoutError) Error() string { return "timeout" } + +func (fakeTimeoutError) Timeout() bool { return true } + +func (f *fakeClient) GetDeviceInfo() (*models.DeviceInfo, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.infoErr != nil { + return nil, f.infoErr + } + result := *f.info + return &result, nil +} + +func (f *fakeClient) GetCapabilities() (*models.Capabilities, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.capabilityErr != nil { + return nil, f.capabilityErr + } + result := *f.capabilities + return &result, nil +} + +func (f *fakeClient) GetZone() (*models.ZoneInfo, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.zoneCalls++ + if f.zoneErr != nil { + return nil, f.zoneErr + } + result := *f.zone + result.Members = append([]models.Member(nil), f.zone.Members...) + return &result, nil +} + +func (f *fakeClient) GetGroup() (*models.Group, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.getGroupErr != nil { + return nil, f.getGroupErr + } + f.getGroupCalls++ + if f.getGroup != nil { + f.group = f.getGroup(f.getGroupCalls, f.group) + } + + return cloneGroup(f.group), nil +} + +func (f *fakeClient) AddGroup(group *models.Group) (*models.Group, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.addRequest = cloneGroup(group) + if f.addErr != nil { + if f.addGroup != nil { + f.group = f.addGroup(group) + } + + return nil, f.addErr + } + if f.addGroup != nil { + f.group = f.addGroup(group) + } else { + f.group = configuredGroup(group.Name) + } + if f.addResponse != nil { + return cloneGroup(f.addResponse(group)), nil + } + return cloneGroup(f.group), nil +} + +func (f *fakeClient) UpdateGroup(group *models.Group) (*models.Group, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.updateRequest = cloneGroup(group) + if f.updateErr != nil { + return nil, f.updateErr + } + f.group = cloneGroup(group) + f.group.Status = "GROUP_OK" + return cloneGroup(f.group), nil +} + +func (f *fakeClient) RemoveGroup() error { + f.mu.Lock() + defer f.mu.Unlock() + f.removeCalls++ + if f.removeErr != nil { + return f.removeErr + } + f.group = &models.Group{} + return nil +} + +func TestCreateSuccessPreservesAsymmetricPayloadAndVerifies(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + + result, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Living Room Pair", + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + if result.Status != StatusSucceeded || len(result.Members) != 2 { + t.Fatalf("result = %+v", result) + } + if left.addRequest.SenderIPAddress != "" { + t.Errorf("LEFT senderIPAddress = %q, want empty", left.addRequest.SenderIPAddress) + } + if right.addRequest.SenderIPAddress != leftIP { + t.Errorf("RIGHT senderIPAddress = %q, want %q", right.addRequest.SenderIPAddress, leftIP) + } + if right.addRequest.ID != "PAIR-ID" { + t.Errorf("RIGHT group ID = %q, want master-assigned PAIR-ID", right.addRequest.ID) + } + for _, request := range []*models.Group{left.addRequest, right.addRequest} { + if request.MasterDeviceID != leftID || len(request.Roles.Roles) != 2 { + t.Errorf("incomplete addGroup request: %+v", request) + } + } + for _, member := range result.Members { + if !member.Verified || member.Group == nil || member.Group.ID != "PAIR-ID" { + t.Errorf("member not verified from fresh group state: %+v", member) + } + } +} + +func TestCreateRejectsInvalidPreflightWithoutMutation(t *testing.T) { + tests := []struct { + name string + mutate func(left, right *fakeClient) + }{ + { + name: "not stereo capable", + mutate: func(_ *fakeClient, right *fakeClient) { + right.capabilities.LRStereo = false + }, + }, + { + name: "capabilities belong to another speaker", + mutate: func(_ *fakeClient, right *fakeClient) { + right.capabilities.DeviceID = "OTHER-ID" + }, + }, + { + name: "different Marge accounts", + mutate: func(_ *fakeClient, right *fakeClient) { + right.info.MargeAccountUUID = "ACCOUNT2" + }, + }, + { + name: "different Marge backends", + mutate: func(_ *fakeClient, right *fakeClient) { + right.info.MargeURL = "http://other-aftertouch.example" + }, + }, + { + name: "in temporary zone", + mutate: func(_ *fakeClient, right *fakeClient) { + right.zone.Members = []models.Member{{DeviceID: leftID, IP: leftIP}} + }, + }, + { + name: "already grouped", + mutate: func(_ *fakeClient, right *fakeClient) { + right.group = configuredGroup("Existing Pair") + }, + }, + { + name: "unreachable", + mutate: func(_ *fakeClient, right *fakeClient) { + right.infoErr = errors.New("offline") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + test.mutate(left, right) + result, err := coordinator.Create(CreateRequest{LeftIPAddress: leftIP, RightIPAddress: rightIP}) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.addRequest != nil || right.addRequest != nil { + t.Fatal("addGroup called after failed preflight") + } + }) + } +} + +func TestCreatePartialFailureIsCompensatedAndReported(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + right.addErr = errors.New("right add failed") + + result, err := coordinator.Create(CreateRequest{LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair"}) + if err == nil { + t.Fatal("Create unexpectedly succeeded") + } + if result.Status != StatusFailed || !result.CompensationAttempted || !result.CompensationComplete { + t.Fatalf("compensation result = %+v", result) + } + if left.removeCalls != 1 || right.removeCalls != 0 { + t.Fatalf("remove calls LEFT=%d RIGHT=%d, want 1/0", left.removeCalls, right.removeCalls) + } + if !result.Members[0].CompensationVerified || !result.Members[1].CompensationVerified { + t.Fatalf("cleanup was not verified empty: %+v", result.Members) + } +} + +func TestCreateReverifiesUncertainTimeout(t *testing.T) { + _, right, coordinator := newCreateCoordinator() + right.addErr = fakeTimeoutError{} + right.addGroup = func(group *models.Group) *models.Group { + return configuredGroup(group.Name) + } + right.getGroup = func(call int, current *models.Group) *models.Group { + if call <= 2 { + return &models.Group{} + } + + return configuredGroup("Pair") + } + coordinator.uncertainOutcomeDelays = []time.Duration{0} + + result, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair", + }) + if err != nil || result.Status != StatusSucceeded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if right.getGroupCalls != 3 { + t.Fatalf("RIGHT getGroup calls = %d, want 3", right.getGroupCalls) + } + if result.Members[1].Group == nil || result.Members[1].Group.ID != "PAIR-ID" { + t.Fatalf("late physical generation was not verified: %+v", result.Members[1]) + } +} + +func TestCreateReverifiesSuccessfulAddGroupResponseWithoutID(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + right.addResponse = func(*models.Group) *models.Group { + return &models.Group{} + } + right.getGroup = func(call int, current *models.Group) *models.Group { + if call <= 2 { + return &models.Group{} + } + + return configuredGroup("Pair") + } + coordinator.uncertainOutcomeDelays = []time.Duration{0} + + result, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair", + }) + if err != nil || result.Status != StatusSucceeded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.getGroupCalls != 3 || right.getGroupCalls != 3 { + t.Fatalf("getGroup calls LEFT=%d RIGHT=%d, want 3/3", + left.getGroupCalls, right.getGroupCalls) + } + if right.addRequest == nil || right.addRequest.ID != "PAIR-ID" { + t.Fatalf("RIGHT addGroup request = %+v, want master-assigned PAIR-ID", right.addRequest) + } + if left.removeCalls != 0 || right.removeCalls != 0 { + t.Fatalf("remove calls LEFT=%d RIGHT=%d, want 0/0", left.removeCalls, right.removeCalls) + } + for _, member := range result.Members { + if !member.Verified || member.Group == nil || member.Group.ID != "PAIR-ID" { + t.Fatalf("late physical generation was not verified: %+v", member) + } + } +} + +func TestCreateResolvesMasterResponseWithoutIDBeforeMutatingSlave(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + left.addResponse = func(*models.Group) *models.Group { + return &models.Group{} + } + left.getGroup = func(call int, current *models.Group) *models.Group { + if call <= 2 { + return &models.Group{} + } + + return configuredGroup("Pair") + } + coordinator.uncertainOutcomeDelays = []time.Duration{0} + + result, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair", + }) + if err != nil || result.Status != StatusSucceeded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.getGroupCalls != 4 || right.getGroupCalls != 2 { + t.Fatalf("getGroup calls LEFT=%d RIGHT=%d, want 4/2", + left.getGroupCalls, right.getGroupCalls) + } + if right.addRequest == nil || right.addRequest.ID != "PAIR-ID" { + t.Fatalf("RIGHT addGroup request = %+v, want resolved master PAIR-ID", right.addRequest) + } + if left.removeCalls != 0 || right.removeCalls != 0 { + t.Fatalf("remove calls LEFT=%d RIGHT=%d, want 0/0", left.removeCalls, right.removeCalls) + } +} + +func TestCreateCompensatesGenerationObservedAfterTimeout(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + right.addErr = fakeTimeoutError{} + right.addGroup = func(*models.Group) *models.Group { + return &models.Group{} + } + coordinator.uncertainOutcomeDelays = nil + + result, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair", + }) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if !result.CompensationAttempted || !result.CompensationComplete || left.removeCalls != 1 { + t.Fatalf("verified timed-out generation was not compensated: result=%+v LEFT removes=%d", + result, left.removeCalls) + } + if right.removeCalls != 0 { + t.Fatalf("RIGHT remove calls = %d, want 0", right.removeCalls) + } +} + +func TestCreateCompensationReverifiesLateRemovalWithoutReplay(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + right.addErr = errors.New("right add failed") + left.removeErr = fakeTimeoutError{} + left.getGroup = func(call int, current *models.Group) *models.Group { + if call >= 5 { + return &models.Group{} + } + + return current + } + coordinator.uncertainOutcomeDelays = []time.Duration{0} + + result, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair", + }) + if err == nil || result.Status != StatusFailed || !result.CompensationComplete { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.removeCalls != 1 { + t.Fatalf("LEFT remove calls = %d, want exactly 1", left.removeCalls) + } + if !result.Members[0].CompensationVerified { + t.Fatalf("late compensation was not verified: %+v", result.Members[0]) + } +} + +func TestCreateCompensationRetiresExactPersistedGeneration(t *testing.T) { + left, right, _ := newCreateCoordinator() + right.addErr = errors.New("right add failed") + + var cleaned GenerationRef + coordinator := NewWithGenerationCleanup(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + func(ref GenerationRef) error { + cleaned = ref + + return nil + }) + + result, err := coordinator.Create(CreateRequest{LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair"}) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if !result.PersistenceAttempted || !result.PersistenceComplete || result.PersistenceError != nil { + t.Fatalf("persistence result = %+v", result) + } + if cleaned.GroupID != "PAIR-ID" || cleaned.DeviceID != leftID || cleaned.AccountID != "ACCOUNT1" { + t.Fatalf("cleaned generation = %+v", cleaned) + } +} + +func TestCreateMasterFailureDoesNotMutateSlave(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + left.addErr = errors.New("left add failed") + + result, err := coordinator.Create(CreateRequest{LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair"}) + if err == nil || result.Status != StatusDegraded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if right.addRequest != nil || result.Members[1].MutationAttempted { + t.Fatalf("slave was mutated after master failure: request=%+v member=%+v", + right.addRequest, result.Members[1]) + } + if left.removeCalls != 0 || right.removeCalls != 0 || result.CompensationAttempted { + t.Fatalf("unexpected compensation after unobserved master failure: result=%+v", result) + } +} + +func TestCreateFailsClosedWhenGenerationPreflightFindsPersistedState(t *testing.T) { + group := configuredGroup("Old Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + + preflightCalls := 0 + coordinator := NewWithGenerationPersistence( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + func(GenerationRef) error { return errors.New("datastore unavailable") }, + func([]GenerationRef) error { + preflightCalls++ + + return errors.New("stale generation still active") + }, + ) + + dissolved, err := coordinator.Dissolve(DissolveRequest{ + MemberIPAddress: leftIP, + ExpectedGroupID: "PAIR-ID", + }) + if err == nil || dissolved.Status != StatusDegraded { + t.Fatalf("dissolve result = %+v, err = %v", dissolved, err) + } + + created, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "New Pair", + }) + if err == nil || created.Status != StatusFailed || preflightCalls != 1 { + t.Fatalf("create result = %+v, preflight calls = %d, err = %v", created, preflightCalls, err) + } + if !errors.Is(created.PersistenceError, ErrUnavailable) { + t.Fatalf("persistence error = %v, want ErrUnavailable", created.PersistenceError) + } + if left.addRequest != nil || right.addRequest != nil { + t.Fatal("addGroup called while an earlier persisted generation remained active") + } +} + +func TestCreateRevalidatesPhysicalStateAfterGenerationPreflight(t *testing.T) { + left, right, _ := newCreateCoordinator() + coordinator := NewWithGenerationPersistence( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + nil, + func([]GenerationRef) error { + right.mu.Lock() + right.group = configuredGroup("Concurrent Pair") + right.mu.Unlock() + + return nil + }, + ) + + result, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair", + }) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.addRequest != nil || right.addRequest != nil { + t.Fatal("addGroup called after physical state changed during persistence preflight") + } +} + +func TestCreatePostconditionMismatchIsDegraded(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + right.addGroup = func(group *models.Group) *models.Group { + mismatch := configuredGroup(group.Name) + mismatch.Name = "Unrelated Pair" + return mismatch + } + + result, err := coordinator.Create(CreateRequest{LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair"}) + if err == nil { + t.Fatal("Create unexpectedly succeeded") + } + if result.Status != StatusDegraded || !result.CompensationAttempted || result.CompensationComplete { + t.Fatalf("result = %+v", result) + } + if left.removeCalls != 1 || right.removeCalls != 0 { + t.Fatalf("remove calls LEFT=%d RIGHT=%d, want only exact requested state removed", left.removeCalls, right.removeCalls) + } +} + +func TestCreateCompensationDoesNotRemoveDifferentGeneration(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + right.addErr = errors.New("right add failed") + left.getGroup = func(call int, current *models.Group) *models.Group { + if call != 2 || current == nil || current.IsEmpty() { + return current + } + + replacement := cloneGroup(current) + replacement.ID = "NEWER-PAIR-ID" + + return replacement + } + + result, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair", + }) + if err == nil || result.Status != StatusDegraded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if result.CompensationAttempted || left.removeCalls != 0 { + t.Fatalf("different generation was selected for compensation: result=%+v removeCalls=%d", + result, left.removeCalls) + } +} + +func TestCreateCompensationRechecksGenerationImmediatelyBeforeRemoval(t *testing.T) { + left, right, coordinator := newCreateCoordinator() + right.addErr = errors.New("right add failed") + left.getGroup = func(call int, current *models.Group) *models.Group { + if call != 3 || current == nil || current.IsEmpty() { + return current + } + + replacement := cloneGroup(current) + replacement.ID = "NEWER-PAIR-ID" + + return replacement + } + + result, err := coordinator.Create(CreateRequest{ + LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair", + }) + if err == nil || result.Status != StatusDegraded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if result.CompensationAttempted || left.removeCalls != 0 { + t.Fatalf("replacement generation was removed: result=%+v removeCalls=%d", result, left.removeCalls) + } + if !errors.Is(result.Members[0].CompensationError, ErrConflict) { + t.Fatalf("compensation error = %v, want ErrConflict", result.Members[0].CompensationError) + } +} + +func TestInspectReturnsCanonicalFreshPair(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Inspect(leftIP) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + if result.Status != StatusSucceeded || result.Group == nil || result.Group.ID != "PAIR-ID" { + t.Fatalf("result = %+v", result) + } + if len(result.Members) != 2 || !result.Members[0].Verified || !result.Members[1].Verified { + t.Fatalf("member reads were not verified: %+v", result.Members) + } +} + +func TestInspectPreservesGenerationForPartialDissolveRecovery(t *testing.T) { + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = configuredGroup("Pair") + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Inspect(leftIP) + if err == nil || result.Status != StatusDegraded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if result.Group == nil || result.Group.ID != "PAIR-ID" { + t.Fatalf("degraded inspection lost recovery generation: %+v", result) + } +} + +func TestRenameUpdatesFullGroupOnBothAndVerifiesAgreement(t *testing.T) { + group := configuredGroup("Old Name") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: leftIP, + ExpectedGroupID: "PAIR-ID", + Name: "New Name", + }) + if err != nil { + t.Fatalf("Rename: %v", err) + } + if result.Status != StatusSucceeded { + t.Fatalf("result = %+v", result) + } + for _, client := range []*fakeClient{left, right} { + if client.updateRequest == nil || client.updateRequest.ID != "PAIR-ID" || client.updateRequest.Name != "New Name" || len(client.updateRequest.Roles.Roles) != 2 { + t.Errorf("full group was not updated: %+v", client.updateRequest) + } + if client.updateRequest.Status != "" { + t.Errorf("read-only status sent in update: %q", client.updateRequest.Status) + } + } +} + +func TestRenamePersistsVerifiedGeneration(t *testing.T) { + group := configuredGroup("Old Name") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + var persistedRef GenerationRef + var persistedName string + coordinator := NewWithGenerationLifecyclePersistence( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + nil, + nil, + func(ref GenerationRef, name string) error { + persistedRef = ref + persistedName = name + return nil + }, + ) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID", Name: "New Name", + }) + if err != nil { + t.Fatalf("Rename: %v", err) + } + if result.Status != StatusSucceeded || !result.PersistenceAttempted || !result.PersistenceComplete { + t.Fatalf("result = %+v", result) + } + if persistedRef.GroupID != "PAIR-ID" || persistedRef.DeviceID != leftID || + persistedRef.ExpectedGroup == nil || persistedRef.ExpectedGroup.Name != "Old Name" || persistedName != "New Name" { + t.Fatalf("persisted ref = %+v, name = %q", persistedRef, persistedName) + } +} + +func TestRenameRejectsSplitMargeOwnershipWithoutMutation(t *testing.T) { + for _, test := range []struct { + name string + mutate func(*models.DeviceInfo) + }{ + { + name: "account", + mutate: func(info *models.DeviceInfo) { + info.MargeAccountUUID = "OTHER-ACCOUNT" + }, + }, + { + name: "backend", + mutate: func(info *models.DeviceInfo) { + info.MargeURL = "http://other-aftertouch.example" + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + group := configuredGroup("Old Name") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + test.mutate(right.info) + persistenceCalls := 0 + coordinator := NewWithGenerationLifecyclePersistence( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + nil, + nil, + func(_ GenerationRef, _ string) error { + persistenceCalls++ + return nil + }, + ) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID", Name: "New Name", + }) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v; want failed ownership preflight", result, err) + } + if left.updateRequest != nil || right.updateRequest != nil || persistenceCalls != 0 { + t.Fatalf("ownership mismatch mutated pair: left=%+v right=%+v persistence=%d", + left.updateRequest, right.updateRequest, persistenceCalls) + } + if len(result.Members) != 2 || !errors.Is(result.Members[1].PreflightError, ErrConflict) { + t.Fatalf("members = %+v; want peer ownership conflict", result.Members) + } + }) + } +} + +func TestRenamePersistenceFailureIsDegradedAndRetryable(t *testing.T) { + group := configuredGroup("Old Name") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + persistenceCalls := 0 + coordinator := NewWithGenerationLifecyclePersistence( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + nil, + nil, + func(_ GenerationRef, _ string) error { + persistenceCalls++ + if persistenceCalls == 1 { + return errors.New("backend unavailable") + } + return nil + }, + ) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID", Name: "New Name", + }) + if err == nil || result.Status != StatusDegraded || !result.PersistenceAttempted || + result.PersistenceComplete || result.PersistenceError == nil { + t.Fatalf("first result = %+v, err = %v", result, err) + } + + result, err = coordinator.Rename(RenameRequest{ + MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID", Name: "New Name", + }) + if err != nil || result.Status != StatusSucceeded || !result.PersistenceComplete || persistenceCalls != 2 { + t.Fatalf("retry result = %+v, err = %v, calls = %d", result, err, persistenceCalls) + } +} + +func TestRenameDetectsPhysicalChangeDuringPersistence(t *testing.T) { + group := configuredGroup("Old Name") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + coordinator := NewWithGenerationLifecyclePersistence( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + nil, + nil, + func(_ GenerationRef, _ string) error { + left.group = configuredGroup("Concurrent Name") + right.group = configuredGroup("Concurrent Name") + return nil + }, + ) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID", Name: "New Name", + }) + if err == nil || result.Status != StatusDegraded || !result.PersistenceComplete || result.Group != nil { + t.Fatalf("result = %+v, err = %v; want degraded physical divergence", result, err) + } + for _, member := range result.Members { + if member.Verified || member.VerificationError == nil { + t.Fatalf("member = %+v; want fresh verification failure", member) + } + } +} + +func TestRenamePostconditionMismatchIsDegraded(t *testing.T) { + group := configuredGroup("Old Name") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + right.updateErr = errors.New("update failed") + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: rightIP, + ExpectedGroupID: "PAIR-ID", + Name: "New Name", + }) + if err == nil || result.Status != StatusDegraded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if result.Members[1].Verified || result.Members[1].VerificationError == nil { + t.Fatalf("RIGHT mismatch not reported: %+v", result.Members[1]) + } +} + +func TestRenameRejectsActiveZoneOnEitherMember(t *testing.T) { + group := configuredGroup("Old Name") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + right.zone.Members = []models.Member{{DeviceID: leftID, IP: leftIP}} + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: leftIP, + ExpectedGroupID: "PAIR-ID", + Name: "New Name", + }) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.zoneCalls != 1 || right.zoneCalls != 1 { + t.Fatalf("fresh zone reads LEFT=%d RIGHT=%d, want 1/1", left.zoneCalls, right.zoneCalls) + } + if left.updateRequest != nil || right.updateRequest != nil { + t.Fatal("updateGroup called despite active-zone preflight rejection") + } +} + +func TestRenameRejectsStaleGroupIDWithoutMutation(t *testing.T) { + group := configuredGroup("Current Name") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: leftIP, + ExpectedGroupID: "OLDER-PAIR-ID", + Name: "Stale Rename", + }) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.updateRequest != nil || right.updateRequest != nil { + t.Fatal("updateGroup called for a stale group generation") + } +} + +func TestRenameRejectsMismatchedPhysicalDeviceIdentity(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient("DIFFERENT-ID", "Replacement") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: leftIP, + ExpectedGroupID: "PAIR-ID", + Name: "New Name", + }) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.updateRequest != nil || right.updateRequest != nil { + t.Fatal("updateGroup called after physical identity mismatch") + } +} + +func TestRenameRejectsNonMemberInitiatingEndpoint(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + third := readyClient("THIRD-ID", "Unrelated") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + third.group = cloneGroup(group) + coordinator := New(factoryFor(map[string]*fakeClient{ + leftIP: left, rightIP: right, thirdIP: third, + })) + + result, err := coordinator.Rename(RenameRequest{ + MemberIPAddress: thirdIP, + ExpectedGroupID: "PAIR-ID", + Name: "Unauthorized Rename", + }) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.updateRequest != nil || right.updateRequest != nil || third.updateRequest != nil { + t.Fatal("updateGroup called for a non-member initiating endpoint") + } +} + +func TestDissolveRemovesAndVerifiesEveryMember(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Dissolve(DissolveRequest{MemberIPAddress: rightIP, ExpectedGroupID: "PAIR-ID"}) + if err != nil { + t.Fatalf("Dissolve: %v", err) + } + if result.Status != StatusSucceeded || left.removeCalls != 1 || right.removeCalls != 1 { + t.Fatalf("result = %+v; remove calls LEFT=%d RIGHT=%d", result, left.removeCalls, right.removeCalls) + } + for _, member := range result.Members { + if !member.Verified || member.Group == nil || !member.Group.IsEmpty() { + t.Errorf("member not verified empty: %+v", member) + } + } +} + +func TestDissolveReverifiesLateRemovalWithoutReplay(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + left.removeErr = fakeTimeoutError{} + left.getGroup = func(call int, current *models.Group) *models.Group { + if call >= 5 { + return &models.Group{} + } + + return current + } + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + coordinator.uncertainOutcomeDelays = []time.Duration{0} + + result, err := coordinator.Dissolve(DissolveRequest{ + MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID", + }) + if err != nil || result.Status != StatusSucceeded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.removeCalls != 1 { + t.Fatalf("LEFT remove calls = %d, want exactly 1", left.removeCalls) + } + if !result.Members[0].Verified { + t.Fatalf("late removal was not verified: %+v", result.Members[0]) + } +} + +func TestDissolveSkipsRemoveOnMemberAlreadyReportingEmpty(t *testing.T) { + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = configuredGroup("Pair") + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Dissolve(DissolveRequest{MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID"}) + if err != nil { + t.Fatalf("Dissolve: %v", err) + } + if result.Status != StatusSucceeded || left.removeCalls != 1 || right.removeCalls != 0 { + t.Fatalf("result = %+v; remove calls LEFT=%d RIGHT=%d, want 1/0", result, left.removeCalls, right.removeCalls) + } +} + +func TestDissolveRejectsRetryThroughAlreadyEmptyAddressedMember(t *testing.T) { + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + right.group = configuredGroup("Pair") + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Dissolve(DissolveRequest{MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID"}) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.removeCalls != 0 || right.removeCalls != 0 { + t.Fatalf("removeGroup called through an empty addressed member: LEFT=%d RIGHT=%d", + left.removeCalls, right.removeCalls) + } +} + +func TestDissolveRecoversExactSnapshotThroughAlreadyEmptyMember(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + right.group = cloneGroup(group) + + var cleaned GenerationRef + coordinator := NewWithGenerationCleanup( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + func(ref GenerationRef) error { + cleaned = ref + return nil + }, + ) + + result, err := coordinator.Dissolve(DissolveRequest{ + MemberIPAddress: leftIP, + ExpectedGroupID: group.ID, + ExpectedGroup: group, + }) + if err != nil || result.Status != StatusSucceeded { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.removeCalls != 0 || right.removeCalls != 1 { + t.Fatalf("remove calls LEFT=%d RIGHT=%d, want 0/1", left.removeCalls, right.removeCalls) + } + if cleaned.GroupID != group.ID || cleaned.DeviceID != leftID || cleaned.AccountID != "ACCOUNT1" { + t.Fatalf("cleanup ref = %+v", cleaned) + } +} + +func TestDissolveRetiresSnapshotAfterBothMembersAreFreshlyVerifiedEmpty(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + + cleanupCalls := 0 + coordinator := NewWithGenerationCleanup( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + func(ref GenerationRef) error { + cleanupCalls++ + if ref.GroupID != group.ID || ref.DeviceID != leftID { + t.Fatalf("cleanup ref = %+v", ref) + } + return nil + }, + ) + + result, err := coordinator.Dissolve(DissolveRequest{ + MemberIPAddress: rightIP, + ExpectedGroupID: group.ID, + ExpectedGroup: group, + }) + if err != nil || result.Status != StatusSucceeded || !result.PersistenceComplete { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.removeCalls != 0 || right.removeCalls != 0 || cleanupCalls != 1 { + t.Fatalf("remove calls LEFT=%d RIGHT=%d; cleanup=%d, want 0/0/1", + left.removeCalls, right.removeCalls, cleanupCalls) + } +} + +func TestDissolveRejectsSnapshotFromNonMemberEndpoint(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + third := readyClient("THIRD-ID", "Unrelated") + cleanupCalls := 0 + coordinator := NewWithGenerationCleanup( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right, thirdIP: third}), + func(GenerationRef) error { + cleanupCalls++ + return nil + }, + ) + + result, err := coordinator.Dissolve(DissolveRequest{ + MemberIPAddress: thirdIP, + ExpectedGroupID: group.ID, + ExpectedGroup: group, + }) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.removeCalls != 0 || right.removeCalls != 0 || third.removeCalls != 0 || cleanupCalls != 0 { + t.Fatalf("unsafe mutation LEFT=%d RIGHT=%d THIRD=%d cleanup=%d", + left.removeCalls, right.removeCalls, third.removeCalls, cleanupCalls) + } +} + +func TestDissolveSubstitutedSnapshotCannotRetireRealPair(t *testing.T) { + persisted := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + third := readyClient("THIRD-ID", "Substitute") + right.group = cloneGroup(persisted) + + submitted := cloneGroup(persisted) + submitted.Roles.Roles[1] = models.GroupRole{ + DeviceID: "THIRD-ID", Role: "RIGHT", IPAddress: thirdIP, + } + retireCalls := 0 + coordinator := NewWithGenerationCleanup( + factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right, thirdIP: third}), + func(ref GenerationRef) error { + if !sameGroupConfiguration(ref.ExpectedGroup, persisted) { + return errors.New("stored topology does not match submitted snapshot") + } + + retireCalls++ + return nil + }, + ) + + result, err := coordinator.Dissolve(DissolveRequest{ + MemberIPAddress: leftIP, + ExpectedGroupID: submitted.ID, + ExpectedGroup: submitted, + }) + if err == nil || result.Status != StatusDegraded || result.PersistenceComplete { + t.Fatalf("result = %+v, err = %v", result, err) + } + if retireCalls != 0 || right.removeCalls != 0 || right.getGroupCalls != 0 { + t.Fatalf("real pair was touched: retire=%d right removes=%d right reads=%d", + retireCalls, right.removeCalls, right.getGroupCalls) + } +} + +func TestDissolveRetiresExactGenerationInsideMutationBoundary(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + + cleanupStarted := make(chan GenerationRef, 1) + releaseCleanup := make(chan struct{}) + coordinator := NewWithGenerationCleanup(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + func(ref GenerationRef) error { + cleanupStarted <- ref + <-releaseCleanup + + return nil + }) + + dissolveDone := make(chan Result, 1) + go func() { + result, _ := coordinator.Dissolve(DissolveRequest{MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID"}) + dissolveDone <- result + }() + + cleaned := <-cleanupStarted + if cleaned.GroupID != "PAIR-ID" || cleaned.DeviceID != leftID || cleaned.AccountID != "ACCOUNT1" { + t.Fatalf("cleanup ref = %+v", cleaned) + } + + createDone := make(chan struct{}) + go func() { + _, _ = coordinator.Create(CreateRequest{LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "New Pair"}) + close(createDone) + }() + + select { + case <-createDone: + t.Fatal("concurrent create passed the coordinator lock before persistence cleanup completed") + case <-time.After(25 * time.Millisecond): + } + + close(releaseCleanup) + result := <-dissolveDone + if result.Status != StatusSucceeded || !result.PersistenceAttempted || !result.PersistenceComplete { + t.Fatalf("dissolve result = %+v", result) + } + <-createDone +} + +func TestDissolveReportsPersistenceFailureAsDegraded(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + coordinator := NewWithGenerationCleanup(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}), + func(GenerationRef) error { return errors.New("datastore unavailable") }) + + result, err := coordinator.Dissolve(DissolveRequest{MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID"}) + if err == nil || result.Status != StatusDegraded || !result.PersistenceAttempted || result.PersistenceComplete { + t.Fatalf("result = %+v, err = %v", result, err) + } + if !errors.Is(result.PersistenceError, ErrUnavailable) { + t.Fatalf("persistence error = %v, want ErrUnavailable", result.PersistenceError) + } +} + +func TestDissolveRejectsActiveZoneOnEitherMember(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + left.zone.Members = []models.Member{{DeviceID: rightID, IP: rightIP}} + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Dissolve(DissolveRequest{MemberIPAddress: rightIP, ExpectedGroupID: "PAIR-ID"}) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.zoneCalls != 1 || right.zoneCalls != 1 { + t.Fatalf("fresh zone reads LEFT=%d RIGHT=%d, want 1/1", left.zoneCalls, right.zoneCalls) + } + if left.removeCalls != 0 || right.removeCalls != 0 { + t.Fatalf("removeGroup called despite active zone: LEFT=%d RIGHT=%d", left.removeCalls, right.removeCalls) + } +} + +func TestDissolveRejectsStaleGroupIDWithoutMutation(t *testing.T) { + group := configuredGroup("Pair") + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + left.group = cloneGroup(group) + right.group = cloneGroup(group) + coordinator := New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) + + result, err := coordinator.Dissolve(DissolveRequest{ + MemberIPAddress: leftIP, + ExpectedGroupID: "OLDER-PAIR-ID", + }) + if err == nil || result.Status != StatusFailed { + t.Fatalf("result = %+v, err = %v", result, err) + } + if left.removeCalls != 0 || right.removeCalls != 0 { + t.Fatalf("removeGroup called for stale generation: LEFT=%d RIGHT=%d", + left.removeCalls, right.removeCalls) + } +} + +func newCreateCoordinator() (*fakeClient, *fakeClient, *Coordinator) { + left := readyClient(leftID, "Left") + right := readyClient(rightID, "Right") + return left, right, New(factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right})) +} + +func readyClient(deviceID, name string) *fakeClient { + return &fakeClient{ + info: &models.DeviceInfo{ + DeviceID: deviceID, Name: name, Type: "SoundTouch 10", + MargeAccountUUID: "ACCOUNT1", MargeURL: "http://aftertouch.example", + }, + capabilities: &models.Capabilities{DeviceID: deviceID, LRStereo: true}, + zone: &models.ZoneInfo{Master: deviceID}, + group: &models.Group{}, + } +} + +func configuredGroup(name string) *models.Group { + return &models.Group{ + ID: "PAIR-ID", + Name: name, + MasterDeviceID: leftID, + Roles: models.GroupRoles{Roles: []models.GroupRole{ + {DeviceID: leftID, Role: "LEFT", IPAddress: leftIP}, + {DeviceID: rightID, Role: "RIGHT", IPAddress: rightIP}, + }}, + SenderIPAddress: leftIP, + Status: "GROUP_OK", + } +} + +func factoryFor(clients map[string]*fakeClient) ClientFactory { + return func(ipAddress string) (Client, error) { + client, ok := clients[ipAddress] + if !ok { + return nil, errors.New("unknown speaker") + } + return client, nil + } +} diff --git a/pkg/stereopair/marge_cleanup.go b/pkg/stereopair/marge_cleanup.go new file mode 100644 index 00000000..fba4c034 --- /dev/null +++ b/pkg/stereopair/marge_cleanup.go @@ -0,0 +1,301 @@ +package stereopair + +import ( + "encoding/xml" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "path" + "strings" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +// DeleteMargeGroupGeneration removes one exact group generation from the +// backend configured by the freshly read speaker /info response. +func DeleteMargeGroupGeneration(httpClient *http.Client, ref GenerationRef) error { + if httpClient == nil { + httpClient = http.DefaultClient + } + + current, err := getMargeDeviceGroup(httpClient, ref) + if err != nil { + return fmt.Errorf("verify Marge group generation before deletion: %w", err) + } + + if current.IsEmpty() { + return nil + } + + if ref.ExpectedGroup == nil || !sameGroupConfiguration(current, ref.ExpectedGroup) || + current.ID != ref.GroupID || current.MasterDeviceID != ref.DeviceID || + !groupContainsDevice(current, ref.DeviceID) { + return fmt.Errorf("delete Marge group generation: device is associated with unrelated generation or topology %q", + current.ID) + } + + endpoint, err := MargeGroupGenerationURL(ref) + if err != nil { + return err + } + + request, err := http.NewRequest(http.MethodDelete, endpoint, nil) + if err != nil { + return fmt.Errorf("create Marge generation cleanup request: %w", err) + } + + response, err := httpClient.Do(request) + if err != nil { + return fmt.Errorf("delete Marge group generation: %w", err) + } + + if response.StatusCode == http.StatusNotFound || + (response.StatusCode >= http.StatusOK && response.StatusCode < http.StatusMultipleChoices) { + _, copyErr := io.Copy(io.Discard, response.Body) + closeErr := response.Body.Close() + + if copyErr != nil { + return fmt.Errorf("read Marge generation cleanup response: %w", copyErr) + } + + if closeErr != nil { + return fmt.Errorf("close Marge generation cleanup response: %w", closeErr) + } + + group, verifyErr := getMargeDeviceGroup(httpClient, ref) + if verifyErr != nil { + return fmt.Errorf("verify Marge group generation deletion: %w", verifyErr) + } + + if group.IsEmpty() || group.ID != ref.GroupID { + return nil + } + + return fmt.Errorf("delete Marge group generation: generation %s is still active", ref.GroupID) + } + + body, _ := io.ReadAll(io.LimitReader(response.Body, 1024)) + _ = response.Body.Close() + + return fmt.Errorf("delete Marge group generation: HTTP %d: %s", + response.StatusCode, strings.TrimSpace(string(body))) +} + +// RenameMargeGroupGeneration updates and verifies the name of one persisted +// generation at the backend configured by fresh speaker info. Topology, rather +// than the old name, is the retry guard so a degraded rename can converge. +func RenameMargeGroupGeneration(httpClient *http.Client, ref GenerationRef, name string) error { + if httpClient == nil { + httpClient = http.DefaultClient + } + + name = strings.TrimSpace(name) + + if name == "" { + return fmt.Errorf("%w: persisted group name must not be empty", ErrInvalidRequest) + } + + current, err := getMargeDeviceGroup(httpClient, ref) + if err != nil { + return fmt.Errorf("verify Marge group generation before rename: %w", err) + } + + if current.IsEmpty() || ref.ExpectedGroup == nil || current.ID != ref.GroupID || + current.MasterDeviceID != ref.DeviceID || !groupContainsDevice(current, ref.DeviceID) || + !sameGroupTopology(current, ref.ExpectedGroup) { + return fmt.Errorf("%w: rename Marge group generation: device is associated with unrelated generation or topology %q", + ErrConflict, current.ID) + } + + if current.Name == name { + return nil + } + + updated := cloneGroup(current) + updated.Name = name + updated.Status = "" + updated.SenderIPAddress = "" + + body, err := xml.Marshal(updated) + if err != nil { + return fmt.Errorf("encode Marge group rename: %w", err) + } + + mutationErr, err := postMargeGroupRename(httpClient, ref, body) + if err != nil { + return err + } + + verified, verifyErr := getMargeDeviceGroup(httpClient, ref) + if verifyErr == nil && verified.Name == name && sameGroupTopology(verified, updated) { + return nil + } + + if mutationErr != nil { + return fmt.Errorf("rename Marge group generation: %w", mutationErr) + } + + if verifyErr != nil { + return fmt.Errorf("verify Marge group generation rename: %w", verifyErr) + } + + return fmt.Errorf("%w: rename Marge group generation: generation %s did not retain name %q", + ErrConflict, ref.GroupID, name) +} + +func postMargeGroupRename(httpClient *http.Client, ref GenerationRef, body []byte) (error, error) { + endpoint, err := MargeGroupGenerationURL(ref) + if err != nil { + return nil, err + } + + request, err := http.NewRequest(http.MethodPost, endpoint, strings.NewReader(xml.Header+string(body))) + if err != nil { + return nil, fmt.Errorf("create Marge group rename request: %w", err) + } + + request.Header.Set("Content-Type", "application/vnd.bose.streaming-v1.2+xml") + + response, requestErr := httpClient.Do(request) + if requestErr != nil { + return requestErr, nil + } + + return margeGroupRenameResponseError(response), nil +} + +func margeGroupRenameResponseError(response *http.Response) error { + responseBody, readErr := io.ReadAll(io.LimitReader(response.Body, 1024)) + + closeErr := response.Body.Close() + + switch { + case response.StatusCode < http.StatusOK || response.StatusCode >= http.StatusMultipleChoices: + return fmt.Errorf("HTTP %d: %s", response.StatusCode, strings.TrimSpace(string(responseBody))) + case readErr != nil: + return fmt.Errorf("read response: %w", readErr) + case closeErr != nil: + return fmt.Errorf("close response: %w", closeErr) + default: + return nil + } +} + +// MargeGroupGenerationURL returns the standard generation-aware group endpoint +// below a speaker's configured Marge base URL. +func MargeGroupGenerationURL(ref GenerationRef) (string, error) { + if !safeMargePathSegment(ref.AccountID) || !safeMargePathSegment(ref.GroupID) { + return "", errors.New("speaker info has no safe Marge account or group ID") + } + + return margeStreamingURL(ref.MargeURL, "account", ref.AccountID, "group", ref.GroupID) +} + +// EnsureMargeNoGroupGenerations checks that the backend has no persisted group +// for speakers already proven physically standalone by the coordinator. The +// check is deliberately read-only so it cannot retire a concurrently created +// physical generation. +func EnsureMargeNoGroupGenerations(httpClient *http.Client, refs []GenerationRef) error { + if httpClient == nil { + httpClient = http.DefaultClient + } + + for i := range refs { + ref := refs[i] + + group, err := getMargeDeviceGroup(httpClient, ref) + if err != nil { + return err + } + + if group.IsEmpty() { + continue + } + + if !safeMargePathSegment(group.ID) || !groupContainsDevice(group, ref.DeviceID) { + return errors.New("marge returned an unsafe or unrelated stale group generation") + } + + return fmt.Errorf("persisted group generation %s still contains standalone device %s", + group.ID, ref.DeviceID) + } + + return nil +} + +func getMargeDeviceGroup(httpClient *http.Client, ref GenerationRef) (*models.Group, error) { + if !safeMargePathSegment(ref.AccountID) || !safeMargePathSegment(ref.DeviceID) { + return nil, errors.New("speaker info has no safe Marge account or device ID") + } + + endpoint, err := margeStreamingURL(ref.MargeURL, "account", ref.AccountID, "device", ref.DeviceID, "group") + if err != nil { + return nil, err + } + + request, err := http.NewRequest(http.MethodGet, endpoint, nil) + if err != nil { + return nil, fmt.Errorf("create Marge group query: %w", err) + } + + response, err := httpClient.Do(request) + if err != nil { + return nil, fmt.Errorf("query Marge group generation: %w", err) + } + + body, readErr := io.ReadAll(io.LimitReader(response.Body, 1<<20)) + closeErr := response.Body.Close() + + if readErr != nil { + return nil, fmt.Errorf("read Marge group generation: %w", readErr) + } + + if closeErr != nil { + return nil, fmt.Errorf("close Marge group response: %w", closeErr) + } + + if response.StatusCode < http.StatusOK || response.StatusCode >= http.StatusMultipleChoices { + return nil, fmt.Errorf("query Marge group generation: HTTP %d: %s", + response.StatusCode, strings.TrimSpace(string(body))) + } + + var group models.Group + if err := xml.Unmarshal(body, &group); err != nil { + return nil, fmt.Errorf("decode Marge group generation: %w", err) + } + + return &group, nil +} + +func margeStreamingURL(margeURL string, segments ...string) (string, error) { + for _, segment := range segments { + if !safeMargePathSegment(segment) { + return "", errors.New("unsafe Marge URL path segment") + } + } + + endpoint, err := url.Parse(strings.TrimSpace(margeURL)) + if err != nil || endpoint.Scheme == "" || endpoint.Host == "" { + return "", fmt.Errorf("speaker info has no usable Marge URL %q", margeURL) + } + + basePath := strings.TrimRight(endpoint.Path, "/") + if !strings.HasSuffix(basePath, "/streaming") { + basePath = path.Join(basePath, "streaming") + } + + allSegments := append([]string{basePath}, segments...) + endpoint.Path = path.Join(allSegments...) + endpoint.RawPath = "" + endpoint.RawQuery = "" + endpoint.Fragment = "" + + return endpoint.String(), nil +} + +func safeMargePathSegment(value string) bool { + return value != "" && value == strings.TrimSpace(value) && value != "." && value != ".." && + !strings.ContainsAny(value, "/?#\\\x00\r\n") +} diff --git a/pkg/stereopair/marge_cleanup_test.go b/pkg/stereopair/marge_cleanup_test.go new file mode 100644 index 00000000..9730d710 --- /dev/null +++ b/pkg/stereopair/marge_cleanup_test.go @@ -0,0 +1,377 @@ +package stereopair + +import ( + "encoding/xml" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +func margeTestGroup(id string) *models.Group { + return &models.Group{ + ID: id, + MasterDeviceID: "LEFT-ID", + Roles: models.GroupRoles{Roles: []models.GroupRole{ + {DeviceID: "LEFT-ID", Role: "LEFT", IPAddress: "192.0.2.10"}, + {DeviceID: "RIGHT-ID", Role: "RIGHT", IPAddress: "192.0.2.11"}, + }}, + } +} + +func TestEnsureMargeNoGroupGenerationsFailsClosedOnDiscoveredGeneration(t *testing.T) { + getCalls := 0 + deleteCalls := 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"): + getCalls++ + w.Header().Set("Content-Type", "application/xml") + _, _ = w.Write([]byte(`LEFT-IDOld PairLEFT-IDLEFTRIGHT-IDRIGHT`)) + case r.Method == http.MethodDelete && strings.HasSuffix(r.URL.Path, "/group/STALE1"): + deleteCalls++ + w.WriteHeader(http.StatusOK) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + err := EnsureMargeNoGroupGenerations(server.Client(), []GenerationRef{{ + DeviceID: "LEFT-ID", AccountID: "ACCOUNT1", MargeURL: server.URL + "/marge", + }}) + if err == nil || !strings.Contains(err.Error(), "STALE1") { + t.Fatalf("error = %v, want stale generation rejection", err) + } + if getCalls != 1 || deleteCalls != 0 { + t.Fatalf("requests GET=%d DELETE=%d, want 1/0", getCalls, deleteCalls) + } +} + +func TestEnsureMargeNoGroupGenerationsAcceptsEmptyGroup(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(``)) + })) + defer server.Close() + + err := EnsureMargeNoGroupGenerations(server.Client(), []GenerationRef{{ + DeviceID: "LEFT-ID", AccountID: "ACCOUNT1", MargeURL: server.URL, + }}) + if err != nil { + t.Fatalf("EnsureMargeNoGroupGenerations: %v", err) + } +} + +func TestEnsureMargeNoGroupGenerationsRejectsUnrelatedGroup(t *testing.T) { + deleteCalls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deleteCalls++ + } + _, _ = w.Write([]byte(`OTHER-LEFTOTHER-LEFTLEFTOTHER-RIGHTRIGHT`)) + })) + defer server.Close() + + err := EnsureMargeNoGroupGenerations(server.Client(), []GenerationRef{{ + DeviceID: "LEFT-ID", AccountID: "ACCOUNT1", MargeURL: server.URL, + }}) + if err == nil || !strings.Contains(err.Error(), "unrelated") { + t.Fatalf("error = %v, want unrelated-group rejection", err) + } + if deleteCalls != 0 { + t.Fatalf("unsafe DELETE calls = %d, want 0", deleteCalls) + } +} + +func TestEnsureMargeNoGroupGenerationsRejectsMissingEndpoint(t *testing.T) { + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + + err := EnsureMargeNoGroupGenerations(server.Client(), []GenerationRef{{ + DeviceID: "LEFT-ID", AccountID: "ACCOUNT1", MargeURL: server.URL, + }}) + if err == nil || !strings.Contains(err.Error(), "HTTP 404") { + t.Fatalf("error = %v, want fail-closed HTTP 404", err) + } +} + +func TestDeleteMargeGroupGenerationDoesNotHideConflict(t *testing.T) { + deleteCalls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deleteCalls++ + http.Error(w, "active generation conflicts with tombstone", http.StatusConflict) + return + } + + _, _ = w.Write([]byte(`LEFT-IDLEFT-IDLEFT192.0.2.10RIGHT-IDRIGHT192.0.2.11`)) + })) + defer server.Close() + + err := DeleteMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }) + if err == nil || !strings.Contains(err.Error(), "HTTP 409") { + t.Fatalf("error = %v, want propagated HTTP 409", err) + } + if deleteCalls != 1 { + t.Fatalf("DELETE calls = %d, want 1", deleteCalls) + } +} + +func TestDeleteMargeGroupGenerationVerifiesExactGenerationIsGone(t *testing.T) { + deleteCalls := 0 + getCalls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodDelete: + deleteCalls++ + w.WriteHeader(http.StatusOK) + case http.MethodGet: + getCalls++ + _, _ = w.Write([]byte(`LEFT-IDLEFT-IDLEFT192.0.2.10RIGHT-IDRIGHT192.0.2.11`)) + } + })) + defer server.Close() + + err := DeleteMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }) + if err == nil || !strings.Contains(err.Error(), "still active") { + t.Fatalf("error = %v, want failed postcondition", err) + } + if deleteCalls != 1 || getCalls != 2 { + t.Fatalf("requests DELETE=%d GET=%d, want 1/2", deleteCalls, getCalls) + } +} + +func TestDeleteMargeGroupGenerationAcceptsVerifiedAbsenceAfter404(t *testing.T) { + deleteSeen := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deleteSeen = true + http.NotFound(w, r) + return + } + + if deleteSeen { + _, _ = w.Write([]byte(``)) + return + } + + _, _ = w.Write([]byte(`LEFT-IDLEFT-IDLEFT192.0.2.10RIGHT-IDRIGHT192.0.2.11`)) + })) + defer server.Close() + + err := DeleteMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }) + if err != nil { + t.Fatalf("DeleteMargeGroupGeneration: %v", err) + } +} + +func TestDeleteMargeGroupGenerationRejectsUnrelatedGenerationBeforeDelete(t *testing.T) { + deleteCalls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deleteCalls++ + } + _, _ = w.Write([]byte(`LEFT-IDLEFT-IDLEFTRIGHT-IDRIGHT`)) + })) + defer server.Close() + + err := DeleteMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }) + if err == nil || !strings.Contains(err.Error(), "unrelated generation") { + t.Fatalf("error = %v, want unrelated-generation rejection", err) + } + if deleteCalls != 0 { + t.Fatalf("DELETE calls = %d, want 0", deleteCalls) + } +} + +func TestDeleteMargeGroupGenerationRejectsSubstitutedMemberBeforeDelete(t *testing.T) { + deleteCalls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deleteCalls++ + } + _, _ = w.Write([]byte(`LEFT-IDLEFT-IDLEFT192.0.2.10REAL-RIGHT-IDRIGHT192.0.2.11`)) + })) + defer server.Close() + + submitted := margeTestGroup("PAIR1") + submitted.Roles.Roles[1].DeviceID = "SUBSTITUTE-RIGHT-ID" + err := DeleteMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: submitted, + }) + if err == nil || !strings.Contains(err.Error(), "topology") { + t.Fatalf("error = %v, want topology rejection", err) + } + if deleteCalls != 0 { + t.Fatalf("DELETE calls = %d, want 0", deleteCalls) + } +} + +func TestDeleteMargeGroupGenerationTreatsVerifiedAbsenceAsIdempotent(t *testing.T) { + deleteCalls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deleteCalls++ + } + _, _ = w.Write([]byte(``)) + })) + defer server.Close() + + err := DeleteMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }) + if err != nil { + t.Fatalf("DeleteMargeGroupGeneration: %v", err) + } + if deleteCalls != 0 { + t.Fatalf("DELETE calls = %d, want 0", deleteCalls) + } +} + +func TestRenameMargeGroupGenerationUpdatesAndVerifiesExactGeneration(t *testing.T) { + current := margeTestGroup("PAIR1") + current.Name = "Old name" + postCalls := 0 + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + data, _ := xml.Marshal(current) + _, _ = w.Write(data) + case http.MethodPost: + postCalls++ + var update models.Group + if err := xml.NewDecoder(r.Body).Decode(&update); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + current = &update + data, _ := xml.Marshal(current) + _, _ = w.Write(data) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + ref := GenerationRef{ + MargeURL: server.URL + "/marge", AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + } + if err := RenameMargeGroupGeneration(server.Client(), ref, "New name"); err != nil { + t.Fatalf("RenameMargeGroupGeneration: %v", err) + } + if current.Name != "New name" || postCalls != 1 { + t.Fatalf("current name = %q, POST calls = %d; want New name, 1", current.Name, postCalls) + } + if err := RenameMargeGroupGeneration(server.Client(), ref, "New name"); err != nil { + t.Fatalf("idempotent RenameMargeGroupGeneration: %v", err) + } + if postCalls != 1 { + t.Fatalf("idempotent retry POST calls = %d, want 1", postCalls) + } +} + +func TestRenameMargeGroupGenerationAcceptsVerifiedStateAfterErrorResponse(t *testing.T) { + current := margeTestGroup("PAIR1") + current.Name = "Old name" + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + data, _ := xml.Marshal(current) + _, _ = w.Write(data) + case http.MethodPost: + var update models.Group + if err := xml.NewDecoder(r.Body).Decode(&update); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + current = &update + http.Error(w, "response lost after commit", http.StatusInternalServerError) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + err := RenameMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }, "New name") + if err != nil { + t.Fatalf("verified state after error response: %v", err) + } + if current.Name != "New name" { + t.Fatalf("current name = %q, want New name", current.Name) + } +} + +func TestRenameMargeGroupGenerationRejectsUnrelatedTopology(t *testing.T) { + postCalls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + postCalls++ + } + _, _ = w.Write([]byte(`OldLEFT-IDLEFT-IDLEFT192.0.2.10OTHER-RIGHTRIGHT192.0.2.11`)) + })) + defer server.Close() + + err := RenameMargeGroupGeneration(server.Client(), GenerationRef{ + MargeURL: server.URL, AccountID: "ACCOUNT1", GroupID: "PAIR1", DeviceID: "LEFT-ID", + ExpectedGroup: margeTestGroup("PAIR1"), + }, "New name") + if err == nil || !strings.Contains(err.Error(), "unrelated") { + t.Fatalf("error = %v, want unrelated-topology rejection", err) + } + if postCalls != 0 { + t.Fatalf("POST calls = %d, want 0", postCalls) + } +} + +func TestMargeGroupGenerationURLRejectsDotSegments(t *testing.T) { + for _, ref := range []GenerationRef{ + {MargeURL: "http://example.test", AccountID: "..", GroupID: "PAIR1"}, + {MargeURL: "http://example.test", AccountID: "ACCOUNT1", GroupID: "."}, + } { + if endpoint, err := MargeGroupGenerationURL(ref); err == nil { + t.Fatalf("MargeGroupGenerationURL(%+v) = %q, want error", ref, endpoint) + } + } +} + +func TestSameMargeBackendNormalizesDefaultPorts(t *testing.T) { + for _, test := range []struct { + name string + left string + right string + same bool + }{ + {name: "http default", left: "http://aftertouch.test", right: "http://aftertouch.test:80/streaming", same: true}, + {name: "https default", left: "https://aftertouch.test/streaming", right: "https://aftertouch.test:443", same: true}, + {name: "non-default port", left: "https://aftertouch.test", right: "https://aftertouch.test:18443", same: false}, + {name: "different prefix", left: "http://aftertouch.test/marge", right: "http://aftertouch.test", same: false}, + } { + t.Run(test.name, func(t *testing.T) { + if got := SameMargeBackend(test.left, test.right); got != test.same { + t.Fatalf("SameMargeBackend(%q, %q) = %v, want %v", test.left, test.right, got, test.same) + } + }) + } +} diff --git a/tests/integration/http-client/delete_group.http b/tests/integration/http-client/delete_group.http index 93617840..c0d68067 100644 --- a/tests/integration/http-client/delete_group.http +++ b/tests/integration/http-client/delete_group.http @@ -25,8 +25,9 @@ Accept: application/vnd.bose.streaming-v1.2+xml ### DELETE /streaming/account/{accountId}/group/ (account-level teardown, no id) ### -### The no-id, trailing-slash form a speaker sends to clear all of an account's -### groups (e.g. during a factory reset). HandleMargeDeleteAccountGroups -> 200. +### The no-id, trailing-slash form is emitted by legacy speakers but cannot +### identify a group generation safely. The server acknowledges it without +### deleting persistent state; generation-aware callers use /group/{groupId}. DELETE {{host}}/streaming/account/{{accountId}}/group/ Content-Type: application/vnd.bose.streaming-v1.2+xml Authorization: Bearer {{token}}