Files
Tobias GesellchenandClaude Opus 4.8 b861c11d37 feat(web): remove devices from the player UI (refs #451)
The merge of soundtouch-web into soundtouch-service was asymmetric:
manual device *adds* propagated to the player UI (HandleAddManualDevice
notifies, the hook re-seeds + broadcasts), but *removals* did not. The
datastore-removal handler never notified, and the web registry's sync
only ever added entries — its map was append-only, so a removed device
lingered in the player UI until restart.

This adds the missing removal path:

- DELETE /api/control/devices/{id} (HandleDeleteDevice). The registry is
  keyed by host/IP; the datastore by device ID (MAC), so the handler
  resolves one to the other via the connection's DeviceInfo, cascades to
  the datastore through a new RemoveDeviceHook (embedded build only),
  prunes the in-memory entry, and broadcasts the updated list.
- WebApp.RemoveDevice prunes the registry and stops the per-device
  goroutines (status poller + WebSocket reconnect loop) via a new
  done-channel + Close() on DeviceConnection — previously both ran for
  the life of the process.
- Server.RemoveDeviceByID extracts the cross-account lookup + remove from
  HandleRemoveDevice and now fires notifyDevicesChanged, so the admin
  Devices tab removal also propagates to the player UI.
- Player UI: a quiet per-card Remove control (visible on hover), a
  confirm dialog, optimistic prune, and a note that a still-online
  device may reappear after the next discovery scan (honest v1 — no
  ignore-list).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-07 15:08:26 +02:00

270 lines
6.3 KiB
Go

// Package handlers contains tests for the device registry API on
// WebApp (GetDevice, AddDevice, TouchDevice, DeviceSnapshot,
// DeviceCount).
package soundtouchweb
import (
"fmt"
"sync"
"testing"
"github.com/gesellix/bose-soundtouch/pkg/models"
"github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes"
)
func newRegistryDevice(name string) *webtypes.DeviceConnection {
conn := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{Name: name})
conn.SetStatus(&webtypes.DeviceStatus{IsConnected: true})
return conn
}
func TestAddDevice_Inserts(t *testing.T) {
app := NewWebApp()
conn := newRegistryDevice("first")
if !app.AddDevice("host-1", conn) {
t.Fatal("AddDevice returned false on first insert")
}
got, ok := app.GetDevice("host-1")
if !ok {
t.Fatal("GetDevice did not find the device after AddDevice")
}
if got != conn {
t.Errorf("GetDevice returned a different pointer than inserted")
}
}
func TestAddDevice_RejectsDuplicateAndBumpsLastSeen(t *testing.T) {
app := NewWebApp()
original := newRegistryDevice("first")
app.AddDevice("host-1", original)
originalSeen := original.LastSeen
replacement := newRegistryDevice("second")
if app.AddDevice("host-1", replacement) {
t.Fatal("AddDevice returned true on duplicate; expected false")
}
got, _ := app.GetDevice("host-1")
if got != original {
t.Error("Duplicate AddDevice replaced the existing device pointer")
}
if !got.LastSeen.After(originalSeen) {
t.Error("Duplicate AddDevice did not bump LastSeen on existing device")
}
}
func TestTouchDevice(t *testing.T) {
app := NewWebApp()
if app.TouchDevice("missing") {
t.Error("TouchDevice returned true for unknown id")
}
conn := newRegistryDevice("first")
app.AddDevice("host-1", conn)
seenBefore := conn.LastSeen
if !app.TouchDevice("host-1") {
t.Fatal("TouchDevice returned false for known id")
}
if !conn.LastSeen.After(seenBefore) {
t.Error("TouchDevice did not bump LastSeen")
}
}
func TestDeviceSnapshotAndCount(t *testing.T) {
app := NewWebApp()
if got := app.DeviceCount(); got != 0 {
t.Errorf("DeviceCount on empty app = %d; want 0", got)
}
if snap := app.DeviceSnapshot(); len(snap) != 0 {
t.Errorf("DeviceSnapshot on empty app = %v; want []", snap)
}
for i := 0; i < 5; i++ {
app.AddDevice(fmt.Sprintf("host-%d", i), newRegistryDevice(fmt.Sprintf("n%d", i)))
}
if got := app.DeviceCount(); got != 5 {
t.Errorf("DeviceCount after 5 adds = %d; want 5", got)
}
snap := app.DeviceSnapshot()
if len(snap) != 5 {
t.Errorf("DeviceSnapshot len = %d; want 5", len(snap))
}
// Spot-check that the snapshot ids match what we inserted.
seen := map[string]bool{}
for _, entry := range snap {
seen[entry.ID] = true
}
for i := 0; i < 5; i++ {
id := fmt.Sprintf("host-%d", i)
if !seen[id] {
t.Errorf("DeviceSnapshot missing %s", id)
}
}
}
func TestRemoveDevice(t *testing.T) {
app := NewWebApp()
if app.RemoveDevice("missing") {
t.Error("RemoveDevice returned true for unknown id")
}
conn := newRegistryDevice("first")
app.AddDevice("host-1", conn)
if !app.RemoveDevice("host-1") {
t.Fatal("RemoveDevice returned false for known id")
}
if _, ok := app.GetDevice("host-1"); ok {
t.Error("device still present after RemoveDevice")
}
if got := app.DeviceCount(); got != 0 {
t.Errorf("DeviceCount after removal = %d; want 0", got)
}
// Removing the same id again is a no-op.
if app.RemoveDevice("host-1") {
t.Error("RemoveDevice returned true on second removal")
}
// The connection's done channel must be closed so its background
// goroutines (status poll, WebSocket reconnect) stop.
select {
case <-conn.Done():
default:
t.Error("RemoveDevice did not close the connection's Done channel")
}
}
// TestRemoveDeviceConcurrent runs adds and removes of the same ids from
// many goroutines under `go test -race` to confirm the registry stays
// race-free when removal is in the mix.
func TestRemoveDeviceConcurrent(t *testing.T) {
app := NewWebApp()
const (
workers = 16
opsPerWorker = 200
)
var wg sync.WaitGroup
wg.Add(workers * 2)
for w := 0; w < workers; w++ {
go func(worker int) {
defer wg.Done()
for i := 0; i < opsPerWorker; i++ {
id := fmt.Sprintf("w%d-%d", worker, i)
app.AddDevice(id, newRegistryDevice(id))
}
}(w)
}
for w := 0; w < workers; w++ {
go func(worker int) {
defer wg.Done()
for i := 0; i < opsPerWorker; i++ {
app.RemoveDevice(fmt.Sprintf("w%d-%d", worker, i))
}
}(w)
}
wg.Wait()
}
// TestRegistryConcurrent exercises the registry from many goroutines
// at once. Before the introduction of devicesMu this would either
// panic with "fatal error: concurrent map read and map write" or be
// flagged by the race detector. The test runs under `go test -race`
// in CI so a future regression that re-exposes the underlying map
// without locking would be caught here.
func TestRegistryConcurrent(t *testing.T) {
app := NewWebApp()
const workers = 16
const opsPerWorker = 200
var wg sync.WaitGroup
wg.Add(workers * 4)
// Writers: insert distinct ids across workers.
for w := 0; w < workers; w++ {
go func(worker int) {
defer wg.Done()
for i := 0; i < opsPerWorker; i++ {
id := fmt.Sprintf("w%d-%d", worker, i)
app.AddDevice(id, newRegistryDevice(id))
}
}(w)
}
// Touchers: bump LastSeen on a shared id (which may or may not
// exist yet — both branches are exercised).
for w := 0; w < workers; w++ {
go func() {
defer wg.Done()
for i := 0; i < opsPerWorker; i++ {
app.TouchDevice("shared")
}
}()
}
// Readers via snapshot.
for w := 0; w < workers; w++ {
go func() {
defer wg.Done()
for i := 0; i < opsPerWorker; i++ {
_ = app.DeviceSnapshot()
}
}()
}
// Readers via direct lookup.
for w := 0; w < workers; w++ {
go func() {
defer wg.Done()
for i := 0; i < opsPerWorker; i++ {
_, _ = app.GetDevice("shared")
_ = app.DeviceCount()
}
}()
}
wg.Wait()
// Sanity check: every writer inserted opsPerWorker devices, plus
// the "shared" entry was never AddDevice'd so should be absent.
if got, want := app.DeviceCount(), workers*opsPerWorker; got != want {
t.Errorf("DeviceCount after concurrent inserts = %d; want %d", got, want)
}
if _, ok := app.GetDevice("shared"); ok {
t.Error("shared device should not exist (only TouchDevice was called for it)")
}
}