mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-18 08:36:13 +00:00
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>
270 lines
6.3 KiB
Go
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)")
|
|
}
|
|
}
|