diff --git a/pkg/service/soundtouchweb/handler.go b/pkg/service/soundtouchweb/handler.go index aa4eacfe..3de7df5e 100644 --- a/pkg/service/soundtouchweb/handler.go +++ b/pkg/service/soundtouchweb/handler.go @@ -365,6 +365,36 @@ func (app *WebApp) removeDeviceIfMatch(id string, expected *webtypes.DeviceConne return ok } +// removeDeviceIfMatchOrAbsent is the postcondition check for an explicit +// removal request. The embedded datastore hook can synchronously reseed the +// registry and remove expected before it returns; absence is therefore already +// success, while a different current connection remains protected. +func (app *WebApp) removeDeviceIfMatchOrAbsent( + id string, + expected *webtypes.DeviceConnection, +) bool { + app.devicesMu.Lock() + + current, ok := app.devices[id] + if !ok { + app.devicesMu.Unlock() + + return true + } + + if current != expected { + app.devicesMu.Unlock() + + return false + } + + delete(app.devices, id) + app.devicesMu.Unlock() + expected.Close() + + return true +} + // HandleAPIDevices returns all devices as JSON func (app *WebApp) HandleAPIDevices(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -460,7 +490,7 @@ func (app *WebApp) HandleDeleteDevice(w http.ResponseWriter, r *http.Request) { } } - if !app.removeDeviceIfMatch(host, conn) { + if !app.removeDeviceIfMatchOrAbsent(host, conn) { app.sendError(w, "Device changed during removal", http.StatusConflict) return diff --git a/pkg/service/soundtouchweb/handler_test.go b/pkg/service/soundtouchweb/handler_test.go index a4103ab2..e961e90c 100644 --- a/pkg/service/soundtouchweb/handler_test.go +++ b/pkg/service/soundtouchweb/handler_test.go @@ -150,6 +150,43 @@ func TestHandleDeleteDevicePreservesConcurrentReplacement(t *testing.T) { app.RemoveDevice(host) } +func TestHandleDeleteDeviceAcceptsHookReseedRemoval(t *testing.T) { + app := NewWebApp() + host := "speaker.local" + original := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{DeviceID: "ORIGINAL"}) + app.AddDevice(host, original) + app.RemoveDeviceHook = func(deviceID string) error { + if deviceID != "ORIGINAL" { + t.Fatalf("RemoveDeviceHook deviceID = %q, want ORIGINAL", deviceID) + } + if !app.RemoveDevice(host) { + t.Fatal("synchronous reseed did not remove original connection") + } + + return nil + } + + req := httptest.NewRequest(http.MethodDelete, "/api/control/devices/"+host, nil) + req = withChiParams(req, map[string]string{"id": host}) + w := httptest.NewRecorder() + + app.HandleDeleteDevice(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want %d; body=%s", w.Code, http.StatusOK, w.Body.String()) + } + var response webtypes.APIResponse + if err := json.NewDecoder(w.Body).Decode(&response); err != nil { + t.Fatalf("decode response: %v", err) + } + if !response.Success { + t.Fatalf("response = %+v, want success", response) + } + if _, ok := app.GetDevice(host); ok { + t.Fatal("removed device remained in registry") + } +} + func TestHandleAPIDevice(t *testing.T) { app := createTestApp()