diff --git a/pkg/service/soundtouchweb/webtypes/status_test.go b/pkg/service/soundtouchweb/webtypes/status_test.go index 7c77db79..c3da39e7 100644 --- a/pkg/service/soundtouchweb/webtypes/status_test.go +++ b/pkg/service/soundtouchweb/webtypes/status_test.go @@ -163,6 +163,45 @@ func TestGroupEventSupersedesInFlightPoll(t *testing.T) { } } +// TestPolledGroupAppliesAfterNewerRefreshStartFailsToApply guards against +// invalidating by start order: a second poll starting (and never applying, +// e.g. its own GetGroup failed) must not discard an earlier poll's +// still-arriving successful result. +func TestPolledGroupAppliesAfterNewerRefreshStartFailsToApply(t *testing.T) { + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) + genA := conn.BeginGroupRefresh() + _ = conn.BeginGroupRefresh() // a second poll starts but never calls ApplyPolledGroup + + if !conn.ApplyPolledGroup(genA, &models.Group{ID: "pair-1", MasterDeviceID: "master"}) { + t.Fatal("an older poll's successful result must still apply when nothing newer ever actually applied") + } + + if got := conn.Status().Group; got == nil || got.ID != "pair-1" { + t.Fatalf("Group = %+v, want pair-1 applied", got) + } +} + +// TestOlderPolledGroupRejectedAfterNewerPollApplies guards the original +// protection this mechanism exists for: a genuinely newer successful poll +// must not be clobbered by an older one arriving late. +func TestOlderPolledGroupRejectedAfterNewerPollApplies(t *testing.T) { + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) + genA := conn.BeginGroupRefresh() + genB := conn.BeginGroupRefresh() + + if !conn.ApplyPolledGroup(genB, &models.Group{ID: "pair-new", MasterDeviceID: "master"}) { + t.Fatal("newer poll result should apply") + } + + if conn.ApplyPolledGroup(genA, &models.Group{ID: "pair-stale", MasterDeviceID: "master"}) { + t.Fatal("an older poll's result arriving after a newer one already applied must be rejected") + } + + if got := conn.Status().Group; got == nil || got.ID != "pair-new" { + t.Fatalf("Group = %+v, want pair-new preserved", got) + } +} + func TestEmptyGroupClearsCurrentClaim(t *testing.T) { conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) conn.SetStatus(&DeviceStatus{Group: &models.Group{ID: "pair-1"}}) diff --git a/pkg/service/soundtouchweb/webtypes/types.go b/pkg/service/soundtouchweb/webtypes/types.go index 9eaaac99..5f71b983 100644 --- a/pkg/service/soundtouchweb/webtypes/types.go +++ b/pkg/service/soundtouchweb/webtypes/types.go @@ -48,10 +48,17 @@ type DeviceConnection struct { status atomic.Pointer[DeviceStatus] // groupMu orders polled /getGroup responses against real-time - // groupUpdated events. Starting a newer refresh or receiving an event - // invalidates any older in-flight poll. - groupMu sync.Mutex - groupGeneration uint64 + // groupUpdated events. groupGeneration is the highest generation + // issued (by BeginGroupRefresh or ApplyGroupEvent); + // groupAppliedGeneration is the highest generation whose result was + // actually applied. Invalidation keys off completion order via + // groupAppliedGeneration, not merely "a newer refresh has started" — + // otherwise a later poll that starts but never applies (e.g. its own + // GetGroup fails) would still discard an earlier poll's still-arriving + // successful result. + groupMu sync.Mutex + groupGeneration uint64 + groupAppliedGeneration uint64 // done is closed by Close when the device is removed from the // registry, signalling its background goroutines (the status poller @@ -164,26 +171,31 @@ func (c *DeviceConnection) BeginGroupRefresh() uint64 { return c.groupGeneration } -// ApplyPolledGroup stores a /getGroup result only when no newer poll or -// groupUpdated event superseded it. Empty groups clear the current claim. +// ApplyPolledGroup stores a /getGroup result only if no strictly newer +// result (poll or event) has already applied. Empty groups clear the +// current claim. func (c *DeviceConnection) ApplyPolledGroup(generation uint64, group *models.Group) bool { c.groupMu.Lock() defer c.groupMu.Unlock() - if generation != c.groupGeneration { + if generation <= c.groupAppliedGeneration { return false } + c.groupAppliedGeneration = generation + return c.replaceGroup(normalizeGroup(group), time.Time{}) } // ApplyGroupEvent stores the newest groupUpdated event and invalidates all -// in-flight /getGroup requests. Empty teardown events clear the current claim. +// in-flight /getGroup requests, including ones that have not started yet. +// Empty teardown events clear the current claim. func (c *DeviceConnection) ApplyGroupEvent(group *models.Group, activity time.Time) bool { c.groupMu.Lock() defer c.groupMu.Unlock() c.groupGeneration++ + c.groupAppliedGeneration = c.groupGeneration return c.replaceGroup(normalizeGroup(group), activity) }