diff --git a/Makefile b/Makefile index 38814782..0b576639 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,7 @@ GOTEST=$(GOCMD) test GOGET=$(GOCMD) get GOMOD=$(GOCMD) mod GOFMT=gofmt +NODE?=node # Build parameters BINARY_NAME=soundtouch-cli @@ -155,6 +156,7 @@ test-coverage: test-browser: @echo "Running browser-level compatibility tests..." $(GOTEST) -tags browsertest -v ./pkg/service/soundtouchweb/... + $(NODE) --test pkg/service/soundtouchweb/static/js/api.test.mjs # Unit tests for the embedded player's static JS modules (see # pkg/service/soundtouchweb/frontend_test/), run via Node's built-in test diff --git a/pkg/service/soundtouchweb/browser_compatibility_test.go b/pkg/service/soundtouchweb/browser_compatibility_test.go index 860b78fc..0d6e38dc 100644 --- a/pkg/service/soundtouchweb/browser_compatibility_test.go +++ b/pkg/service/soundtouchweb/browser_compatibility_test.go @@ -11,15 +11,69 @@ package soundtouchweb import ( "context" + "encoding/json" + "fmt" + "io/fs" "net/http" "net/http/httptest" + "strings" + "sync" "testing" "time" "github.com/chromedp/chromedp" + "github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes" "github.com/go-chi/chi/v5" ) +func newPlayerFixtureServer(t *testing.T, moduleScript string, configure func(chi.Router)) *httptest.Server { + t.Helper() + + r := chi.NewRouter() + staticFS, err := fs.Sub(StaticFS, "static") + if err != nil { + t.Fatalf("open static fixture filesystem: %v", err) + } + r.Get("/app/static/*", http.StripPrefix("/app/static", http.FileServer(http.FS(staticFS))).ServeHTTP) + configure(r) + r.Get("/fixture", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/html") + _, _ = fmt.Fprintf(w, ` + + + + + +
+`, moduleScript) + }) + + server := httptest.NewServer(r) + t.Cleanup(server.Close) + + return server +} + +const sourceFixtureScript = ` +import { h, render } from 'preact'; +import { Sources } from '/app/static/js/components/Sources.js'; +const sources = [ + { Source: 'AUX', SourceAccount: 'AUX1', DisplayName: 'Aux 1', Status: 'READY' }, + { Source: 'PRODUCT', SourceAccount: '', DisplayName: 'Product', Status: 'READY' }, + { Source: 'SPOTIFY', SourceAccount: 'spotify-user', DisplayName: 'Spotify', Status: 'READY' }, +]; +let revision = 0; +window.renderStatus = (source = 'STANDBY', account = '', sourcesStale = false, deviceId = 'speaker', revisionOverride = null, sourceItems = sources) => { + const nextRevision = revisionOverride ?? ++revision; + return render(h(Sources, { + deviceId, + status: { revision: nextRevision, nowPlayingRevision: nextRevision, sources: { SourceItem: sourceItems }, sourcesStale, nowPlaying: { Source: source, SourceAccount: account } }, + readbackDelays: [100, 250, 500], + }), document.getElementById('fixture')); +}; +window.renderStatus(); +` + // newHeadlessChromeContext returns a context bound to a fresh headless // Chrome instance, torn down automatically at the end of the test. func newHeadlessChromeContext(t *testing.T) context.Context { @@ -141,3 +195,776 @@ func TestPlayerRendersUnderForcedShimMode(t *testing.T) { t.Error("app did not render under forced es-module-shims shim mode") } } + +func TestFrontendDeviceStateRejectsNonNewerStatusRevisions(t *testing.T) { + const revisionFixture = ` +import { mergeDevicesSnapshot, mergeStatusUpdate } from '/app/static/js/app.js'; +const current = { + speaker: { info: { name: 'Current' }, status: { revision: 5, sourcesStale: false, nowPlaying: { Track: 'new' } } }, +}; +const snapshot = mergeDevicesSnapshot(current, { + speaker: { info: { name: 'Renamed' }, status: { revision: 4, nowPlaying: { Track: 'old' } } }, + added: { info: { name: 'Added' }, status: { revision: 1 } }, +}); +const equal = mergeStatusUpdate(snapshot, 'speaker', { revision: 5, nowPlaying: { Track: 'equal' } }); +const older = mergeStatusUpdate(equal, 'speaker', { revision: 3, nowPlaying: { Track: 'older' } }); +const newer = mergeStatusUpdate(older, 'speaker', { revision: 6, nowPlaying: { Track: 'newest' } }); +const stale = mergeStatusUpdate(newer, 'speaker', { + revision: 6, + sourcesStale: true, + nowPlaying: { Track: 'must not replace canonical state' }, +}); +const adversarial = Object.fromEntries([ + ['__proto__', { status: { revision: 1, nowPlaying: { Track: 'old proto' } } }], + ['constructor', { status: { revision: 1, nowPlaying: { Track: 'old constructor' } } }], +]); +const protoUpdated = mergeStatusUpdate(adversarial, '__proto__', { + revision: 2, + nowPlaying: { Track: 'new proto' }, +}); +const constructorUpdated = mergeStatusUpdate(protoUpdated, 'constructor', { + revision: 2, + nowPlaying: { Track: 'new constructor' }, +}); +window.revisionChecks = { + snapshotKeptStatus: snapshot.speaker.status.revision === 5 && snapshot.speaker.status.nowPlaying.Track === 'new', + snapshotUpdatedInfo: snapshot.speaker.info.name === 'Renamed' && snapshot.added.status.revision === 1, + equalRejected: equal === snapshot, + olderRejected: older === equal, + newerAccepted: newer !== older && newer.speaker.status.revision === 6 && newer.speaker.status.nowPlaying.Track === 'newest', + derivedStaleAccepted: stale !== newer && stale.speaker.status.sourcesStale === true && + stale.speaker.status.nowPlaying.Track === 'newest', + staleCannotClearAtEqualRevision: mergeStatusUpdate(stale, 'speaker', { + revision: 6, + sourcesStale: false, + }) === stale, + unknownRejected: mergeStatusUpdate(newer, 'unknown', { revision: 99 }) === newer, + adversarialIDsAreOwnDataProperties: + Object.prototype.hasOwnProperty.call(constructorUpdated, '__proto__') && + Object.prototype.hasOwnProperty.call(constructorUpdated, 'constructor'), + adversarialIDsKeepObjectPrototype: Object.getPrototypeOf(constructorUpdated) === Object.prototype, + adversarialIDsUpdateOnlyTheirRecords: + constructorUpdated.__proto__.status.nowPlaying.Track === 'new proto' && + constructorUpdated.constructor.status.nowPlaying.Track === 'new constructor' && + Object.prototype.polluted === undefined, +}; +` + server := newPlayerFixtureServer(t, revisionFixture, func(chi.Router) {}) + ctx := newHeadlessChromeContext(t) + + var checks map[string]bool + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.Poll(`window.revisionChecks !== undefined`, nil), + chromedp.Evaluate(`window.revisionChecks`, &checks), + ); err != nil { + t.Fatalf("exercise frontend revision state owner: %v", err) + } + for name, passed := range checks { + if !passed { + t.Errorf("revision check %s failed", name) + } + } +} + +func TestDerivedSourceExpiryDisablesCommandsAtEqualRevision(t *testing.T) { + const sourceExpiryFixture = ` +import { h, render } from 'preact'; +import { mergeStatusUpdate } from '/app/static/js/app.js'; +import { Sources } from '/app/static/js/components/Sources.js'; +const ready = [{ Source: 'AUX', SourceAccount: 'AUX1', DisplayName: 'Aux 1', Status: 'READY' }]; +let devices = { + speaker: { + status: { + revision: 5, + nowPlayingRevision: 5, + sourcesStale: false, + sources: { SourceItem: ready }, + nowPlaying: { Source: 'STANDBY', SourceAccount: '' }, + }, + }, +}; +function redraw() { + render(h(Sources, { + deviceId: 'speaker', + status: devices.speaker.status, + readbackDelays: [100], + }), document.getElementById('fixture')); +} +window.expireSources = () => { + devices = mergeStatusUpdate(devices, 'speaker', { + revision: 5, + nowPlayingRevision: 5, + sourcesStale: true, + sources: { SourceItem: ready }, + nowPlaying: { Source: 'AUX', SourceAccount: 'AUX1' }, + }); + redraw(); +}; +redraw(); +` + + var mu sync.Mutex + writes := 0 + server := newPlayerFixtureServer(t, sourceExpiryFixture, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + writes++ + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + }) + + ctx := newHeadlessChromeContext(t) + var trackSource string + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Evaluate(`window.expireSources()`, nil), + chromedp.Poll(`document.querySelector('.source-btn')?.disabled === true`, nil), + chromedp.Evaluate(`document.querySelector('.source-btn').click()`, nil), + chromedp.Sleep(50*time.Millisecond), + chromedp.Evaluate(`document.querySelector('.source-btn').classList.contains('active') ? 'AUX' : 'STANDBY'`, &trackSource), + ); err != nil { + t.Fatalf("apply derived source expiry: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if writes != 0 || trackSource != "STANDBY" { + t.Fatalf("derived expiry writes=%d projected source=%q, want 0 and STANDBY", writes, trackSource) + } +} + +func TestSourceSelectionUsesOneWriteAndAbsoluteReadbacks(t *testing.T) { + type sourceRun struct { + body webtypes.SourceRequest + startedAt time.Time + readTimes []time.Duration + } + + var mu sync.Mutex + var runs []sourceRun + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, req *http.Request) { + var body webtypes.SourceRequest + if err := json.NewDecoder(req.Body).Decode(&body); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + mu.Lock() + runs = append(runs, sourceRun{body: body, startedAt: time.Now()}) + mu.Unlock() + + w.Header().Set("Content-Type", "application/json") + if body.Source == "SPOTIFY" { + _ = json.NewEncoder(w).Encode(webtypes.APIResponse{Success: false, Error: "source rejected"}) + return + } + _ = json.NewEncoder(w).Encode(webtypes.APIResponse{Success: true}) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + run := &runs[len(runs)-1] + run.readTimes = append(run.readTimes, time.Since(run.startedAt)) + readCount := len(run.readTimes) + body := run.body + mu.Unlock() + + nowPlaying := map[string]string{"Source": "STANDBY", "SourceAccount": ""} + if body.Source == "AUX" && readCount >= 2 { + nowPlaying = map[string]string{"Source": body.Source, "SourceAccount": body.Account} + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(webtypes.APIResponse{Success: true, Data: map[string]any{ + "status": map[string]any{"revision": readCount + 1, "nowPlayingRevision": readCount + 1, "nowPlaying": nowPlaying}, + }}) + }) + }) + + ctx := newHeadlessChromeContext(t) + var immediatelyPending bool + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Evaluate(`document.querySelector('.source-btn:nth-child(1)').getAttribute('aria-busy') === 'true'`, &immediatelyPending), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected'`, nil), + chromedp.Click(`.source-btn:nth-child(2)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection unverified'`, nil), + chromedp.Click(`.source-btn:nth-child(3)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-btn:nth-child(3)').classList.contains('unverified')`, nil), + ); err != nil { + t.Fatalf("exercise source commands: %v", err) + } + if !immediatelyPending { + t.Error("source command did not expose pending state immediately") + } + + mu.Lock() + defer mu.Unlock() + if len(runs) != 3 { + t.Fatalf("source writes = %d, want exactly one for each of 3 commands", len(runs)) + } + wantBodies := []webtypes.SourceRequest{ + {Source: "AUX", Account: "AUX1"}, + {Source: "PRODUCT", Account: ""}, + {Source: "SPOTIFY", Account: "spotify-user"}, + } + for i, want := range wantBodies { + if runs[i].body != want { + t.Errorf("write %d body = %+v, want %+v", i, runs[i].body, want) + } + } + if got := len(runs[0].readTimes); got != 3 { + t.Errorf("confirmed command readbacks = %d, want 3", got) + } + if got := len(runs[1].readTimes); got != 3 { + t.Errorf("unverified command readbacks = %d, want 3", got) + } + if got := len(runs[2].readTimes); got != 3 { + t.Errorf("transport-uncertain write readbacks = %d, want 3", got) + } + for i, want := range []time.Duration{100 * time.Millisecond, 250 * time.Millisecond, 500 * time.Millisecond} { + got := runs[0].readTimes[i] + if got < want-40*time.Millisecond || got > want+150*time.Millisecond { + t.Errorf("confirmed readback %d at %s, want absolute deadline near %s", i, got, want) + } + } + for i, want := range []time.Duration{100 * time.Millisecond, 250 * time.Millisecond, 500 * time.Millisecond} { + got := runs[1].readTimes[i] + if got < want-40*time.Millisecond || got > want+150*time.Millisecond { + t.Errorf("unverified readback %d at %s, want absolute deadline near %s", i, got, want) + } + } +} + +func TestSourceSelectionTreatsSelfAccountAsOmitted(t *testing.T) { + const fixture = ` +import { h, render } from 'preact'; +import { Sources } from '/app/static/js/components/Sources.js'; +const sourceItems = [{ Source: 'AUX', SourceAccount: 'AUX', DisplayName: 'AUX IN', Status: 'READY' }]; +render(h(Sources, { + deviceId: 'speaker', + status: { + revision: 1, + nowPlayingRevision: 1, + nowPlaying: { Source: 'STANDBY', SourceAccount: '' }, + sources: { SourceItem: sourceItems }, + }, + readbackDelays: [100, 250, 500], +}), document.getElementById('fixture')); +` + + var mu sync.Mutex + writes := 0 + var request webtypes.SourceRequest + server := newPlayerFixtureServer(t, fixture, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, req *http.Request) { + mu.Lock() + writes++ + _ = json.NewDecoder(req.Body).Decode(&request) + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"data":{"status":{"revision":2,"nowPlayingRevision":2,"nowPlaying":{"Source":"AUX","SourceAccount":""}}}}`)) + }) + }) + + ctx := newHeadlessChromeContext(t) + var active bool + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected'`, nil), + chromedp.Evaluate(`document.querySelector('.source-btn').classList.contains('active')`, &active), + ); err != nil { + t.Fatalf("select source whose self-account is omitted by now_playing: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if writes != 1 || request != (webtypes.SourceRequest{Source: "AUX", Account: "AUX"}) { + t.Fatalf("writes=%d request=%+v, want one AUX/AUX write", writes, request) + } + if !active { + t.Fatal("source with omitted self-account was not projected active") + } +} + +func TestSourceSelectionReadbacksDoNotWaitForSlowWriteResponse(t *testing.T) { + var mu sync.Mutex + startedAt := time.Time{} + var readTimes []time.Duration + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + startedAt = time.Now() + mu.Unlock() + time.Sleep(700 * time.Millisecond) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + readTimes = append(readTimes, time.Since(startedAt)) + readCount := len(readTimes) + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"success":true,"data":{"status":{"revision":%d,"nowPlayingRevision":%d,"nowPlaying":{"Source":"STANDBY","SourceAccount":""}}}}`, readCount+1, readCount+1) + }) + }) + + ctx := newHeadlessChromeContext(t) + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection unverified'`, nil), + ); err != nil { + t.Fatalf("exercise slow source response: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if len(readTimes) != 3 { + t.Fatalf("readbacks = %d, want 3", len(readTimes)) + } + for i, want := range []time.Duration{100 * time.Millisecond, 250 * time.Millisecond, 500 * time.Millisecond} { + if got := readTimes[i]; got < want-40*time.Millisecond || got > want+150*time.Millisecond { + t.Errorf("readback %d at %s, want absolute deadline near %s", i, got, want) + } + } +} + +func TestSourceSelectionLaterFirmwareErrorOverridesProvisionalConfirmation(t *testing.T) { + var mu sync.Mutex + reads := 0 + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + reads++ + read := reads + mu.Unlock() + + source := "AUX" + account := "AUX1" + if read == 2 { + source = "INVALID_SOURCE" + account = "" + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"success":true,"data":{"status":{"revision":%d,"nowPlayingRevision":%d,"nowPlaying":{"Source":%q,"SourceAccount":%q}}}}`, read+1, read+1, source, account) + }) + }) + + ctx := newHeadlessChromeContext(t) + var provisional bool + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected, confirming'`, nil), + chromedp.Evaluate(`document.querySelector('.source-btn:nth-child(1)').classList.contains('provisional-confirmed')`, &provisional), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection failed'`, nil), + ); err != nil { + t.Fatalf("exercise provisional source rejection: %v", err) + } + if !provisional { + t.Error("early matching readback did not expose provisional confirmation") + } + + mu.Lock() + defer mu.Unlock() + if reads != 2 { + t.Errorf("readbacks = %d, want provisional match followed by authoritative rejection", reads) + } +} + +func TestSourceSelectionRejectsReadbackWithOnlyNewerAggregateRevision(t *testing.T) { + var mu sync.Mutex + reads := 0 + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + reads++ + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"data":{"status":{"revision":99,"nowPlayingRevision":1,"nowPlaying":{"Source":"AUX","SourceAccount":"AUX1"}}}}`)) + }) + }) + + ctx := newHeadlessChromeContext(t) + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection unverified'`, nil), + ); err != nil { + t.Fatalf("exercise stale source readback: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if reads != 3 { + t.Errorf("readbacks = %d, want all 3 after stale matching responses", reads) + } +} + +func TestSourceSelectionTreatsFirmwareErrorSourceAsFailed(t *testing.T) { + var mu sync.Mutex + reads := 0 + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + reads++ + read := reads + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"success":true,"data":{"status":{"revision":%d,"nowPlayingRevision":%d,"nowPlaying":{"Source":"INVALID_SOURCE","SourceAccount":""}}}}`, read+1, read+1) + }) + }) + + ctx := newHeadlessChromeContext(t) + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection failed'`, nil), + ); err != nil { + t.Fatalf("exercise firmware source rejection: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if reads != 1 { + t.Errorf("readbacks = %d, want 1 after authoritative firmware rejection", reads) + } +} + +func TestSourceSelectionLaterAuthoritativeSourceClearsFinalProjection(t *testing.T) { + transitions := []struct { + name, source, account string + activeButtons int + }{ + {name: "airplay", source: "AIRPLAY"}, + {name: "spotify", source: "SPOTIFY", account: "spotify-user", activeButtons: 1}, + {name: "standby", source: "STANDBY"}, + {name: "invalid source", source: "INVALID_SOURCE"}, + } + + for _, transition := range transitions { + t.Run(transition.name, func(t *testing.T) { + var mu sync.Mutex + reads := 0 + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + reads++ + revision := reads + 1 + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"success":true,"data":{"status":{"revision":%d,"nowPlayingRevision":%d,"nowPlaying":{"Source":"AUX","SourceAccount":"AUX1"}}}}`, revision, revision) + }) + }) + + ctx := newHeadlessChromeContext(t) + var commandCleared, auxInactive bool + var activeButtons int + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected'`, nil), + chromedp.Evaluate(fmt.Sprintf(`window.renderStatus(%q, %q, false, 'speaker', 100)`, transition.source, transition.account), nil), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === ''`, nil), + chromedp.Evaluate(`document.querySelector('.source-command-status').textContent === ''`, &commandCleared), + chromedp.Evaluate(`!document.querySelector('.source-btn:nth-child(1)').classList.contains('active')`, &auxInactive), + chromedp.Evaluate(`document.querySelectorAll('.source-btn.active').length`, &activeButtons), + ); err != nil { + t.Fatalf("apply later authoritative source: %v", err) + } + if !commandCleared || !auxInactive || activeButtons != transition.activeButtons { + t.Errorf("projection after %s: cleared=%v auxInactive=%v activeButtons=%d want=%d", + transition.source, commandCleared, auxInactive, activeButtons, transition.activeButtons) + } + }) + } +} + +func TestSourceReadbackPublishesThroughAppDeviceState(t *testing.T) { + const fixture = ` +import { h, render } from 'preact'; +import { useState } from 'preact/hooks'; +import { mergeStatusUpdate } from '/app/static/js/app.js'; +import { NowPlaying } from '/app/static/js/components/NowPlaying.js'; +import { Sources } from '/app/static/js/components/Sources.js'; +const sourceItems = [{ Source: 'AUX', SourceAccount: 'AUX1', DisplayName: 'Aux 1', Status: 'READY' }]; +function Fixture() { + const [devices, setDevices] = useState({ speaker: { status: { + revision: 1, + nowPlayingRevision: 1, + nowPlaying: { Source: 'STANDBY' }, + sources: { SourceItem: sourceItems }, + } } }); + const status = devices.speaker.status; + return h('div', {}, + h(NowPlaying, { nowPlaying: status.nowPlaying }), + h(Sources, { + deviceId: 'speaker', + status, + readbackDelays: [100, 250, 500], + onStatusReadback: next => setDevices(previous => mergeStatusUpdate(previous, 'speaker', next)), + }), + ); +} +render(h(Fixture), document.getElementById('fixture')); +` + + var mu sync.Mutex + reads := 0 + server := newPlayerFixtureServer(t, fixture, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + reads++ + revision := reads + 1 + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(webtypes.APIResponse{Success: true, Data: map[string]any{ + "status": map[string]any{ + "revision": revision, + "nowPlayingRevision": revision, + "nowPlaying": map[string]string{ + "Source": "AUX", "SourceAccount": "AUX1", "Track": "Confirmed track", + }, + "sources": map[string]any{"SourceItem": []map[string]string{ + {"Source": "AUX", "SourceAccount": "AUX1", "DisplayName": "Aux 1", "Status": "READY"}, + }}, + }, + }}) + }) + }) + + ctx := newHeadlessChromeContext(t) + var title string + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.track-title')?.textContent === 'Confirmed track'`, nil), + chromedp.Text(`.track-title`, &title, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected'`, nil), + ); err != nil { + t.Fatalf("publish source readback through app state: %v", err) + } + if title != "Confirmed track" { + t.Fatalf("NowPlaying title = %q, want confirmed readback", title) + } +} + +func TestSourceSelectionResetsWhenDeviceChanges(t *testing.T) { + var mu sync.Mutex + writes := 0 + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + writes++ + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + }) + + ctx := newHeadlessChromeContext(t) + var statusText string + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Evaluate(`window.renderStatus('STANDBY', '', false, 'other-speaker')`, nil), + chromedp.Sleep(600*time.Millisecond), + chromedp.Text(`.source-command-status`, &statusText, chromedp.ByQuery), + ); err != nil { + t.Fatalf("switch source component device: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if statusText != "" || writes != 1 { + t.Errorf("device switch left command state=%q or writes=%d, want cleared state and one original write", statusText, writes) + } +} + +func TestSourceSelectionFencesOlderReadbackAndStatus(t *testing.T) { + var mu sync.Mutex + writes := map[string]int{} + currentSource := "" + readRevision := 10 + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, req *http.Request) { + var body webtypes.SourceRequest + _ = json.NewDecoder(req.Body).Decode(&body) + mu.Lock() + writes[body.Source]++ + currentSource = body.Source + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(webtypes.APIResponse{Success: true}) + }) + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + source := currentSource + readRevision++ + revision := readRevision + mu.Unlock() + if source == "AUX" { + time.Sleep(400 * time.Millisecond) + } + w.Header().Set("Content-Type", "application/json") + responseSource := "STANDBY" + responseAccount := "" + if source == "AUX" { + responseSource = source + responseAccount = "AUX1" + } + _ = json.NewEncoder(w).Encode(webtypes.APIResponse{Success: true, Data: map[string]any{ + "status": map[string]any{ + "revision": revision, + "nowPlayingRevision": revision, + "nowPlaying": map[string]string{ + "Source": responseSource, "SourceAccount": responseAccount, + }, + }, + }}) + }) + }) + + ctx := newHeadlessChromeContext(t) + var outcome, productClass string + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Sleep(140*time.Millisecond), + chromedp.Click(`.source-btn:nth-child(2)`, chromedp.ByQuery), + chromedp.Evaluate(`window.renderStatus('AUX', 'AUX1')`, nil), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection unverified'`, nil), + chromedp.Text(`.source-command-status`, &outcome, chromedp.ByQuery), + chromedp.AttributeValue(`.source-btn:nth-child(2)`, "class", &productClass, nil, chromedp.ByQuery), + ); err != nil { + t.Fatalf("exercise source generation fence: %v", err) + } + + if outcome != "Source selection unverified" || !strings.Contains(productClass, "unverified") { + t.Errorf("newer outcome overwritten: status=%q class=%q", outcome, productClass) + } + mu.Lock() + defer mu.Unlock() + if writes["AUX"] != 1 || writes["PRODUCT"] != 1 { + t.Errorf("writes = %#v, want one AUX and one PRODUCT write", writes) + } +} + +func TestStaleSourcesRemainVisibleButCannotBeSelected(t *testing.T) { + var mu sync.Mutex + writes := 0 + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + writes++ + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + }) + + ctx := newHeadlessChromeContext(t) + var sourceCount, disabledCount int + var staleText, staleRole string + var described bool + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Evaluate(`window.renderStatus('STANDBY', '', true)`, nil), + chromedp.WaitVisible(`#source-stale-status`, chromedp.ByQuery), + chromedp.Evaluate(`document.querySelectorAll('.source-btn').length`, &sourceCount), + chromedp.Evaluate(`document.querySelectorAll('.source-btn:disabled').length`, &disabledCount), + chromedp.Text(`#source-stale-status`, &staleText, chromedp.ByQuery), + chromedp.AttributeValue(`#source-stale-status`, "role", &staleRole, nil, chromedp.ByQuery), + chromedp.Evaluate(`[...document.querySelectorAll('.source-btn')].every(button => button.getAttribute('aria-describedby') === 'source-stale-status')`, &described), + chromedp.Evaluate(`document.querySelector('.source-btn').click()`, nil), + chromedp.Sleep(50*time.Millisecond), + ); err != nil { + t.Fatalf("render stale source cache: %v", err) + } + + mu.Lock() + staleWrites := writes + mu.Unlock() + if sourceCount != 3 || disabledCount != sourceCount { + t.Errorf("stale sources: rendered=%d disabled=%d, want all 3 retained and disabled", sourceCount, disabledCount) + } + if staleText != "Source list out of date" || staleRole != "status" || !described { + t.Errorf("stale indication: text=%q role=%q described=%v", staleText, staleRole, described) + } + if staleWrites != 0 { + t.Errorf("stale source selection issued %d writes, want 0", staleWrites) + } + + var enabledCount int + var staleIndicatorMissing bool + if err := chromedp.Run(ctx, + chromedp.Evaluate(`window.renderStatus('STANDBY', '', false)`, nil), + chromedp.Poll(`document.querySelectorAll('.source-btn:disabled').length === 0`, nil), + chromedp.Evaluate(`document.querySelectorAll('.source-btn:not(:disabled)').length`, &enabledCount), + chromedp.Evaluate(`document.querySelector('#source-stale-status') === null`, &staleIndicatorMissing), + ); err != nil { + t.Fatalf("render refreshed source cache: %v", err) + } + if enabledCount != sourceCount || !staleIndicatorMissing { + t.Errorf("fresh sources: enabled=%d want=%d staleIndicatorMissing=%v", enabledCount, sourceCount, staleIndicatorMissing) + } + + var noInventoryText, noInventoryRole string + if err := chromedp.Run(ctx, + chromedp.Evaluate(`window.renderStatus('STANDBY', '', false, 'speaker', null, [])`, nil), + chromedp.WaitVisible(`#source-inventory-status`, chromedp.ByQuery), + chromedp.Text(`#source-inventory-status`, &noInventoryText, chromedp.ByQuery), + chromedp.AttributeValue(`#source-inventory-status`, "role", &noInventoryRole, nil, chromedp.ByQuery), + ); err != nil { + t.Fatalf("render missing source inventory: %v", err) + } + if noInventoryText != "Source list unavailable" || noInventoryRole != "status" { + t.Errorf("missing source inventory: text=%q role=%q", noInventoryText, noInventoryRole) + } +} diff --git a/pkg/service/soundtouchweb/handler.go b/pkg/service/soundtouchweb/handler.go index b60d914b..53ceae77 100644 --- a/pkg/service/soundtouchweb/handler.go +++ b/pkg/service/soundtouchweb/handler.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "log" "net/http" "strconv" @@ -697,19 +698,50 @@ func (app *WebApp) handleBassControl(w http.ResponseWriter, r *http.Request, dev app.sendControlResponse(w, err, fmt.Sprintf("Bass set to %d", bassReq.Level)) } -// handleSourceControl processes source control requests +// handleSourceControl processes source control requests. POST with an exact +// {source, account} JSON body is canonical. GET query parameters remain as a +// temporary compatibility surface and are explicitly marked deprecated. func (app *WebApp) handleSourceControl(w http.ResponseWriter, r *http.Request, device *webtypes.DeviceConnection) { - sourceParam := r.URL.Query().Get("name") + var sourceParam, accountParam string + + switch r.Method { + case http.MethodPost: + r.Body = http.MaxBytesReader(w, r.Body, 8<<10) + decoder := json.NewDecoder(r.Body) + decoder.DisallowUnknownFields() + + var sourceReq webtypes.SourceRequest + if err := decoder.Decode(&sourceReq); err != nil { + app.sendError(w, "Invalid source data", http.StatusBadRequest) + return + } + + if err := decoder.Decode(&struct{}{}); err != io.EOF { + app.sendError(w, "Invalid source data", http.StatusBadRequest) + return + } + + sourceParam = sourceReq.Source + accountParam = sourceReq.Account + case http.MethodGet: + w.Header().Set("Deprecation", "true") + w.Header().Set("Warning", `299 - "GET source control is deprecated; use POST with a JSON body"`) + + sourceParam = r.URL.Query().Get("name") + accountParam = r.URL.Query().Get("account") + default: + app.sendError(w, "POST required for source control", http.StatusMethodNotAllowed) + return + } + if sourceParam == "" { app.sendError(w, "Source name required", http.StatusBadRequest) return } - // Forward the optional account parameter as sourceAccount. Devices with - // multiple jacks that share source="AUX" (e.g. ST-5 CD/Aux inputs) - // disambiguate them via distinct sourceAccount values (AUX, AUX1, …). - accountParam := r.URL.Query().Get("account") - + // Forward account verbatim as sourceAccount. Devices with multiple jacks + // that share source="AUX" (e.g. ST-5 CD/Aux inputs) disambiguate them via + // distinct sourceAccount values (AUX, AUX1, …). if device.Client == nil { app.sendError(w, "Device client not available", http.StatusInternalServerError) return diff --git a/pkg/service/soundtouchweb/handler_test.go b/pkg/service/soundtouchweb/handler_test.go index e961e90c..d6ea30c5 100644 --- a/pkg/service/soundtouchweb/handler_test.go +++ b/pkg/service/soundtouchweb/handler_test.go @@ -850,12 +850,10 @@ func TestHandleDevicePlay_SourceAccountFiltering(t *testing.T) { } } -// TestHandleSourceControl_ForwardsAccount verifies that the account query -// parameter is forwarded as sourceAccount in the /select XML. Devices like -// the ST-5 expose multiple AUX jacks that share source="AUX" and are only -// disambiguated by distinct sourceAccount values (AUX, AUX1, …). Regression -// test for issue #444, where the handler dropped the account parameter. -func TestHandleSourceControl_ForwardsAccount(t *testing.T) { +// TestHandleSourceControl_LegacyGETForwardsAccount verifies that the temporary +// GET compatibility route still forwards sourceAccount while clearly marking +// the response deprecated. +func TestHandleSourceControl_LegacyGETForwardsAccount(t *testing.T) { tests := []struct { name string query string @@ -904,6 +902,12 @@ func TestHandleSourceControl_ForwardsAccount(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) } + if got := w.Header().Get("Deprecation"); got != "true" { + t.Errorf("Deprecation header = %q, want true", got) + } + if got := w.Header().Get("Warning"); !strings.Contains(got, "use POST") { + t.Errorf("Warning header = %q, want POST migration guidance", got) + } if want := `source="AUX"`; !strings.Contains(capturedBody, want) { t.Errorf("XML should contain %q, got: %s", want, capturedBody) @@ -1060,6 +1064,63 @@ func TestHandleZoneAddRejectsStandbyMaster(t *testing.T) { } } +func TestHandleSourceControl_CanonicalPOSTForwardsExactBody(t *testing.T) { + var capturedBody string + speaker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/select" { + b, _ := io.ReadAll(r.Body) + capturedBody = string(b) + } + w.WriteHeader(http.StatusOK) + })) + defer speaker.Close() + + app := NewWebApp() + conn := webtypes.NewDeviceConnection( + client.NewClient(&client.Config{Host: speaker.URL}), + &models.DeviceInfo{Name: "Test Speaker"}, + ) + conn.SetStatus(&webtypes.DeviceStatus{IsConnected: true, LastActivity: time.Now()}) + app.AddDevice("source-device", conn) + + body := strings.NewReader(`{"source":"AUX","account":"AUX1"}`) + req := httptest.NewRequest(http.MethodPost, "/api/control/devices/source-device/action/source", body) + req.Header.Set("Content-Type", "application/json") + req = withChiParams(req, map[string]string{"id": "source-device", "action": "source"}) + w := httptest.NewRecorder() + + app.HandleAPIControl(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + if got := w.Header().Get("Deprecation"); got != "" { + t.Errorf("canonical POST unexpectedly marked deprecated: %q", got) + } + for _, want := range []string{`source="AUX"`, `sourceAccount="AUX1"`} { + if !strings.Contains(capturedBody, want) { + t.Errorf("XML should contain %q, got: %s", want, capturedBody) + } + } +} + +func TestHandleSourceControl_CanonicalPOSTRejectsUnknownFields(t *testing.T) { + app := NewWebApp() + conn := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{Name: "Test Speaker"}) + app.AddDevice("source-device", conn) + + body := strings.NewReader(`{"source":"AUX","account":"AUX1","name":"legacy"}`) + req := httptest.NewRequest(http.MethodPost, "/api/control/devices/source-device/action/source", body) + req = withChiParams(req, map[string]string{"id": "source-device", "action": "source"}) + w := httptest.NewRecorder() + + app.HandleAPIControl(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + // TestHandleZoneRemove_UsesRemoveZoneSlave is the #511 regression: removing one // member from a multi-member zone must target that member via /removeZoneSlave. // The previous implementation rebuilt the zone with /setZone and the remaining diff --git a/pkg/service/soundtouchweb/mount.go b/pkg/service/soundtouchweb/mount.go index 9508d6f4..7cc185f2 100644 --- a/pkg/service/soundtouchweb/mount.go +++ b/pkg/service/soundtouchweb/mount.go @@ -71,7 +71,9 @@ func (app *WebApp) MountWeb(r chi.Router, discoveryService *discovery.UnifiedDis r.Get("/recents", app.HandleDeviceRecents) // Low-level "play this ContentItem" primitive (not a provider). r.Post("/play", app.HandleDevicePlay) - // Generic key / preset / source / bass actions. + // Generic key / preset / source / bass actions. Source selection is + // canonically POSTed as JSON; its GET form remains temporarily for + // compatibility and marks every response as deprecated. r.Get("/action/{action}", app.HandleAPIControl) r.Post("/action/{action}", app.HandleAPIControl) r.Get("/ws", app.HandleDeviceWebSocket) diff --git a/pkg/service/soundtouchweb/source_cache_test.go b/pkg/service/soundtouchweb/source_cache_test.go new file mode 100644 index 00000000..a2ab86aa --- /dev/null +++ b/pkg/service/soundtouchweb/source_cache_test.go @@ -0,0 +1,145 @@ +package soundtouchweb + +import ( + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "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" +) + +// TestUpdateDeviceStatusDoesNotRefreshNowPlayingRevisionOnFailure: a poll round +// where /now_playing failed but /volume succeeded must merge the volume and +// leave now-playing authority untouched. A source selection waiting for +// confirmation reads NowPlayingRevision, so advancing it on a failed read +// would confirm a selection nothing actually verified. +func TestUpdateDeviceStatusDoesNotRefreshNowPlayingRevisionOnFailure(t *testing.T) { + speaker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/volume" { + _, _ = w.Write([]byte(`3535false`)) + + return + } + http.Error(w, "unavailable", http.StatusServiceUnavailable) + })) + defer speaker.Close() + + conn := webtypes.NewDeviceConnection(client.NewClient(&client.Config{Host: speaker.URL}), nil) + conn.SetStatus(&webtypes.DeviceStatus{NowPlaying: &models.NowPlaying{Source: "SPOTIFY"}}) + baseline := conn.Status() + + NewWebApp().UpdateDeviceStatus("speaker", conn) + updated := conn.Status() + + if updated.Revision <= baseline.Revision || updated.Volume == nil || updated.Volume.ActualVolume != 35 { + t.Fatalf("unrelated successful field was not merged: %+v", updated) + } + if updated.NowPlaying.Source != "SPOTIFY" || updated.NowPlayingRevision != baseline.NowPlayingRevision { + t.Fatalf("failed now-playing read advanced source authority: %+v", updated) + } +} + +// TestUpdateSourcesCacheRetainsFailureAndRefreshesSuccess: a failed refresh +// keeps the previous inventory and its read time but marks it stale; the next +// success replaces both and restores actionability. +func TestUpdateSourcesCacheRetainsFailureAndRefreshesSuccess(t *testing.T) { + conn := webtypes.NewDeviceConnection(nil, nil) + // Read times must sit inside sourceCacheTTL of now: Status() derives + // staleness against the wall clock, so fixed calendar dates would read + // back as expired no matter what this test does. + firstRead := time.Now() + oldSources := &models.Sources{SourceItem: []models.SourceItem{{Source: "AUX"}}} + + updateSourcesCache(conn, conn.BeginFieldPoll(webtypes.FieldSources), oldSources, nil, firstRead) + + if !updateSourcesCache(conn, conn.BeginFieldPoll(webtypes.FieldSources), nil, + errors.New("temporary read failure"), firstRead.Add(time.Second)) { + t.Fatal("failed source readback was not recorded") + } + + status := conn.Status() + if status.Sources != oldSources || !status.SourcesReadAt.Equal(firstRead) || !status.SourcesStale { + t.Fatalf("failed source readback changed the cache: %+v", status) + } + + secondRead := time.Now() + newSources := &models.Sources{SourceItem: []models.SourceItem{{Source: "PRODUCT"}}} + if !updateSourcesCache(conn, conn.BeginFieldPoll(webtypes.FieldSources), newSources, nil, secondRead) { + t.Fatal("successful source readback was not reported as an update") + } + + status = conn.Status() + if status.Sources != newSources || !status.SourcesReadAt.Equal(secondRead) || status.SourcesStale { + t.Fatalf("successful source readback did not refresh the cache: %+v", status) + } +} + +// TestHandleAPIDevicePublishesCanonicalReadback: the player's bounded readback +// after a source selection reads GET /devices/{id}, so that response must +// carry the same revisions the connection now holds -- not a snapshot taken +// before its own refresh. +func TestHandleAPIDevicePublishesCanonicalReadback(t *testing.T) { + speaker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + responses := map[string]string{ + "/now_playing": `Confirmed trackPLAY_STATE`, + "/volume": `3535false`, + "/presets": ``, + "/sources": `Aux 1`, + "/bass": `00`, + } + body, ok := responses[r.URL.Path] + if !ok { + http.NotFound(w, r) + + return + } + w.Header().Set("Content-Type", "application/xml") + _, _ = w.Write([]byte(body)) + })) + defer speaker.Close() + + app := NewWebApp() + conn := webtypes.NewDeviceConnection( + client.NewClient(&client.Config{Host: speaker.URL}), + &models.DeviceInfo{Name: "Speaker"}, + ) + conn.SetStatus(&webtypes.DeviceStatus{NowPlaying: &models.NowPlaying{Source: "STANDBY"}}) + baselineRevision := conn.Status().NowPlayingRevision + conn.WebSocket = &client.WebSocketClient{} + app.AddDevice("speaker", conn) + + req := httptest.NewRequest(http.MethodGet, "/api/control/devices/speaker", nil) + req = withChiParams(req, map[string]string{"id": "speaker"}) + w := httptest.NewRecorder() + app.HandleAPIDevice(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("GET device status = %d: %s", w.Code, w.Body.String()) + } + + var response struct { + Success bool `json:"success"` + Data struct { + Status webtypes.DeviceStatus `json:"status"` + } `json:"data"` + } + if err := json.NewDecoder(w.Body).Decode(&response); err != nil { + t.Fatalf("decode device readback: %v", err) + } + + canonical := conn.Status() + if !response.Success || canonical.NowPlaying.Source != "AUX" || canonical.NowPlaying.Track != "Confirmed track" { + t.Fatalf("canonical readback not merged: response=%+v status=%+v", response, canonical) + } + if response.Data.Status.NowPlaying.Source != canonical.NowPlaying.Source || + response.Data.Status.Revision != canonical.Revision || + response.Data.Status.NowPlayingRevision != canonical.NowPlayingRevision || + canonical.NowPlayingRevision <= baselineRevision { + t.Fatalf("response did not publish canonical status: response=%+v status=%+v", response.Data.Status, canonical) + } +} diff --git a/pkg/service/soundtouchweb/static/css/app.css b/pkg/service/soundtouchweb/static/css/app.css index a1dc4748..c426cb51 100644 --- a/pkg/service/soundtouchweb/static/css/app.css +++ b/pkg/service/soundtouchweb/static/css/app.css @@ -700,11 +700,23 @@ img { display: block; max-width: 100%; } font-size: .8rem; transition: background .1s, border-color .1s; } -.source-btn:hover { background: var(--bg); } +.source-btn:hover:not(:disabled) { background: var(--bg); } +.source-btn:disabled { cursor: not-allowed; opacity: .55; } .source-btn.active { border-color: var(--accent); background: var(--accent); color: var(--accent-fg); } +.source-btn.pending { cursor: wait; } +.source-btn.provisional-confirmed { border-style: dashed; cursor: progress; } +.source-btn.unverified { border-color: #a76b00; } +.source-btn.failed { border-color: var(--offline); } .source-btn.local { border-style: dashed; } .source-icon { font-size: .9rem; line-height: 1; } .source-name { font-weight: 500; } +.source-command-status { + min-height: 1.2em; + margin-top: .35rem; + color: var(--text-dim); + font-size: .75rem; +} +.source-command-status.availability { color: #8a5a00; } .stereo-pair-note { display: flex; diff --git a/pkg/service/soundtouchweb/static/js/api.js b/pkg/service/soundtouchweb/static/js/api.js index bb0f320c..747b075c 100644 --- a/pkg/service/soundtouchweb/static/js/api.js +++ b/pkg/service/soundtouchweb/static/js/api.js @@ -5,6 +5,20 @@ async function req(url, opts = {}) { return r.json(); } +async function checkedReq(url, opts = {}) { + const r = await fetch(url, opts); + let response; + try { + response = await r.json(); + } catch (_) { + throw new Error(`Request failed (${r.status})`); + } + if (!r.ok || response?.success === false) { + throw new Error(response?.error || `Request failed (${r.status})`); + } + return response; +} + export const api = { devices: () => req('/api/control/devices'), device: (id) => req(`/api/control/devices/${id}`), @@ -51,7 +65,11 @@ export const api = { tuneInSearchNext: (cursor) => req(`/api/control/providers/tunein/search/next?cursor=${encodeURIComponent(cursor)}`), control: (id, action, presetId) => req(`/api/control/devices/${id}/action/${action}?id=${presetId}`), storePreset: (id, slotId) => req(`/api/control/devices/${id}/action/storepreset?id=${slotId}`), - selectSource: (id, source, account) => req(`/api/control/devices/${id}/action/source?name=${encodeURIComponent(source)}&account=${encodeURIComponent(account || '')}`), + selectSource: (id, source, account) => checkedReq(`/api/control/devices/${id}/action/source`, { + method: 'POST', + headers: JSON_HEADERS, + body: JSON.stringify({ source, account: account ?? '' }), + }), tuneInPlay: (deviceId, item) => req(`/api/control/devices/${deviceId}/providers/tunein/play`, { method: 'POST', headers: JSON_HEADERS, diff --git a/pkg/service/soundtouchweb/static/js/api.test.mjs b/pkg/service/soundtouchweb/static/js/api.test.mjs new file mode 100644 index 00000000..2753f835 --- /dev/null +++ b/pkg/service/soundtouchweb/static/js/api.test.mjs @@ -0,0 +1,51 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { api } from './api.js'; + +test('selectSource posts the exact source and account body', async () => { + const requests = []; + globalThis.fetch = async (url, options) => { + requests.push({ url, options }); + return new Response('{"success":true}', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }; + + await api.selectSource('speaker', 'AUX', 'AUX1'); + await api.selectSource('speaker', 'PRODUCT'); + + const request = requests[0]; + assert.equal(request.url, '/api/control/devices/speaker/action/source'); + assert.equal(request.options.method, 'POST'); + assert.deepEqual(JSON.parse(request.options.body), { source: 'AUX', account: 'AUX1' }); + assert.deepEqual(JSON.parse(requests[1].options.body), { source: 'PRODUCT', account: '' }); +}); + +test('source selection rejects non-2xx responses', async () => { + globalThis.fetch = async () => new Response('{"success":false,"error":"offline"}', { + status: 503, + headers: { 'Content-Type': 'application/json' }, + }); + + await assert.rejects(api.selectSource('speaker', 'AUX', 'AUX1'), /offline/); +}); + +test('source selection rejects application failures on 2xx responses', async () => { + globalThis.fetch = async () => new Response('{"success":false,"error":"rejected"}', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + + await assert.rejects(api.selectSource('speaker', 'AUX', 'AUX1'), /rejected/); +}); + +test('legacy API consumers retain response-level error handling', async () => { + globalThis.fetch = async () => new Response('{"success":false,"error":"offline"}', { + status: 503, + headers: { 'Content-Type': 'application/json' }, + }); + + assert.deepEqual(await api.devices(), { success: false, error: 'offline' }); +}); diff --git a/pkg/service/soundtouchweb/static/js/app.js b/pkg/service/soundtouchweb/static/js/app.js index ff453b8d..3308ab8b 100644 --- a/pkg/service/soundtouchweb/static/js/app.js +++ b/pkg/service/soundtouchweb/static/js/app.js @@ -21,7 +21,79 @@ import { removeDeviceAndRefresh } from './deviceRemoval.js'; const html = htm.bind(h); -function DeviceDetail({ deviceId, devices, onBack, onDevicesChanged, notify, onRemove }) { +function statusRevision(status) { + const revision = status?.revision; + return Number.isSafeInteger(revision) && revision >= 0 ? revision : null; +} + +// The server advances DeviceStatus.revision on every projection, so a frame +// carrying a revision no newer than what we already hold is stale -- a slow +// `devices` snapshot overtaken by a `status_update` delta, or a REST refresh +// overtaken by either. A device we have never seen is always accepted. +function acceptsNewerStatus(current, incoming) { + const currentRevision = statusRevision(current); + const incomingRevision = statusRevision(incoming); + if (currentRevision === null) return true; + return incomingRevision !== null && incomingRevision > currentRevision; +} + +// The server derives sourcesStale at read time, so two reads at the same +// revision can disagree about it. Carry that one derived bit forward without +// letting an otherwise-stale frame replace the canonical state. +function mergeDerivedStatus(current, incoming) { + const currentRevision = statusRevision(current); + const incomingRevision = statusRevision(incoming); + if (currentRevision === null || incomingRevision !== currentRevision || + current?.sourcesStale === true || incoming?.sourcesStale !== true) { + return current; + } + + return { ...current, sourcesStale: true }; +} + +export function mergeDevicesSnapshot(previous, snapshot) { + return Object.fromEntries(Object.entries(snapshot || {}).map(([deviceId, incoming]) => { + const current = Object.prototype.hasOwnProperty.call(previous, deviceId) + ? previous[deviceId] : null; + if (!current || acceptsNewerStatus(current.status, incoming?.status)) { + return [deviceId, incoming]; + } + return [deviceId, { + ...incoming, + status: mergeDerivedStatus(current.status, incoming?.status), + }]; + })); +} + +function replaceDevice(previous, deviceId, device) { + return Object.fromEntries([ + ...Object.entries(previous), + [deviceId, device], + ]); +} + +export function mergeStatusUpdate(previous, deviceId, status) { + // Object.prototype.hasOwnProperty, not a plain previous[deviceId] truthy + // check: a deviceId of "__proto__" or "constructor" would otherwise + // resolve through the prototype chain to a truthy value and pass the + // check despite not being a real, known device. + if (!Object.prototype.hasOwnProperty.call(previous, deviceId) || + !acceptsNewerStatus(previous[deviceId]?.status, status)) { + const current = previous[deviceId]?.status; + const merged = mergeDerivedStatus(current, status); + if (merged === current) return previous; + return replaceDevice(previous, deviceId, { + ...previous[deviceId], + status: merged, + }); + } + return replaceDevice(previous, deviceId, { + ...previous[deviceId], + status, + }); +} + +function DeviceDetail({ deviceId, devices, onBack, onDevicesChanged, notify, onRemove, onStatusReadback }) { const device = devices[deviceId]; if (!device) { @@ -47,7 +119,11 @@ function DeviceDetail({ deviceId, devices, onBack, onDevicesChanged, notify, onR <${NowPlaying} nowPlaying=${device.status?.nowPlaying} deviceId=${deviceId} presets=${device.status?.presets} /> <${Controls} deviceId=${deviceId} status=${device.status} /> <${Presets} deviceId=${deviceId} status=${device.status} /> - <${Sources} deviceId=${deviceId} status=${device.status} /> + <${Sources} + deviceId=${deviceId} + status=${device.status} + onStatusReadback=${status => onStatusReadback(deviceId, status)} + /> <${StereoPair} deviceId=${deviceId} device=${device} @@ -136,7 +212,7 @@ function App() { ws.onmessage = (event) => { const msg = JSON.parse(event.data); if (msg.type === 'devices') { - setDevices(msg.data || {}); + setDevices(previous => mergeDevicesSnapshot(previous, msg.data)); } else if (msg.type === 'discovery_status') { if (msg.data?.isDiscovering !== undefined) { setIsDiscovering(msg.data.isDiscovering); @@ -150,17 +226,7 @@ function App() { showToast(`Found ${msg.data.deviceCount} device(s)`); } } else if (msg.type === 'status_update' && msg.deviceId) { - setDevices(prev => { - // Object.prototype.hasOwnProperty, not a plain prev[msg.deviceId] - // truthy check: a deviceId of "__proto__" or "constructor" would - // otherwise resolve through the prototype chain to a truthy value - // and pass the check despite not being a real, known device. - if (!Object.prototype.hasOwnProperty.call(prev, msg.deviceId)) return prev; - return { - ...prev, - [msg.deviceId]: { ...prev[msg.deviceId], status: msg.data }, - }; - }); + setDevices(previous => mergeStatusUpdate(previous, msg.deviceId, msg.data)); } }; @@ -203,6 +269,10 @@ function App() { setDevices(resp.data || {}); } + function mergeDeviceReadback(deviceId, status) { + setDevices(previous => mergeStatusUpdate(previous, deviceId, status)); + } + async function removeDevice(id) { const name = devices[id]?.info?.name || id; if (!confirm(`Remove "${name}" from AfterTouch?\n\nThis does not reset the speaker. A device still online may reappear after the next discovery scan.`)) { @@ -302,6 +372,7 @@ function App() { onDevicesChanged=${refreshDevices} notify=${showToast} onRemove=${removeDevice} + onStatusReadback=${mergeDeviceReadback} /> ` : page === 'tunein' ? html` <${TuneInBrowser} key="tunein-browser" devices=${devices} /> @@ -334,4 +405,5 @@ function App() { `; } -render(html`<${App} />`, document.getElementById('app')); +const appRoot = document.getElementById('app'); +if (appRoot) render(html`<${App} />`, appRoot); diff --git a/pkg/service/soundtouchweb/static/js/components/Sources.js b/pkg/service/soundtouchweb/static/js/components/Sources.js index 864cfe45..2e4ccbc1 100644 --- a/pkg/service/soundtouchweb/static/js/components/Sources.js +++ b/pkg/service/soundtouchweb/static/js/components/Sources.js @@ -1,4 +1,5 @@ import { h } from 'preact'; +import { useEffect, useRef, useState } from 'preact/hooks'; import htm from 'htm'; import { api } from '../api.js'; @@ -11,31 +12,219 @@ const SOURCE_ICONS = { AIRPLAY: '📡', PRODUCT: '🔊', }; -export function Sources({ deviceId, status }) { +const SOURCE_READBACK_DELAYS_MS = [2000, 5000, 10000]; + +function isErrorSource(source) { + return source === 'INVALID_SOURCE' || source?.endsWith('_ERROR'); +} + +function sourceAccountIdentity(source, account) { + return account && account !== source ? account : ''; +} + +function sourceAccountsMatch(source, left, right) { + return sourceAccountIdentity(source, left) === sourceAccountIdentity(source, right); +} + +export function Sources({ + deviceId, + status, + onStatusReadback, + readbackDelays = SOURCE_READBACK_DELAYS_MS, +}) { + const [command, setCommand] = useState(null); + const commandRef = useRef({ generation: 0, active: null, timers: [] }); + const mountedRef = useRef(false); const items = status?.sources?.SourceItem ?? []; const currentSource = status?.nowPlaying?.Source; const currentAccount = status?.nowPlaying?.SourceAccount; + const nowPlayingRevision = Number.isSafeInteger(status?.nowPlayingRevision) + ? status.nowPlayingRevision : null; + const sourcesStale = status?.sourcesStale === true; + + function clearReadbacks() { + commandRef.current.timers.forEach(clearTimeout); + commandRef.current.timers = []; + } + + useEffect(() => { + if (!mountedRef.current) { + mountedRef.current = true; + return; + } + + commandRef.current.generation += 1; + commandRef.current.active = null; + clearReadbacks(); + setCommand(null); + }, [deviceId]); + + useEffect(() => { + return () => { + commandRef.current.generation += 1; + commandRef.current.active = null; + clearReadbacks(); + }; + }, []); + + useEffect(() => { + if (!command || nowPlayingRevision === null || command.startNowPlayingRevision === null || + nowPlayingRevision <= command.startNowPlayingRevision || + command.outcome === 'failed' || command.outcome === 'unverified') return; + + const matches = currentSource === command.source && + sourceAccountsMatch(command.source, currentAccount, command.account); + if (command.outcome === 'final-confirmed') { + if (nowPlayingRevision > command.confirmedRevision && !matches) { + setCommand(previous => previous?.generation === command.generation + ? null : previous); + } + return; + } + if (isErrorSource(currentSource)) { + clearReadbacks(); + commandRef.current.active = null; + setCommand(previous => previous?.generation === command.generation + ? { ...previous, outcome: 'failed', error: currentSource } + : previous); + } else if (matches && command.outcome === 'pending') { + setCommand(previous => previous?.generation === command.generation + ? { ...previous, outcome: 'provisional-confirmed' } + : previous); + } + }, [command, currentSource, currentAccount, nowPlayingRevision]); const ready = items.filter(s => s.Status === 'READY'); - if (ready.length === 0) return null; + const availabilityMessage = sourcesStale + ? 'Source list out of date' + : (ready.length === 0 ? 'Source list unavailable' : ''); + const availabilityId = sourcesStale + ? 'source-stale-status' + : (ready.length === 0 ? 'source-inventory-status' : null); - function select(src) { - api.selectSource(deviceId, src.Source, src.SourceAccount ?? ''); + async function select(src) { + clearReadbacks(); + const generation = commandRef.current.generation + 1; + const target = { source: src.Source, account: src.SourceAccount ?? '' }; + commandRef.current.generation = generation; + const active = { generation, latestReadback: -1, writeError: null }; + commandRef.current.active = active; + setCommand({ + ...target, + generation, + outcome: 'pending', + startNowPlayingRevision: nowPlayingRevision, + }); + const startedAt = Date.now(); + + readbackDelays.forEach((delay, index) => { + const timer = setTimeout(async () => { + if (commandRef.current.active !== active) return; + active.latestReadback = index; + + try { + const response = await api.device(deviceId); + if (commandRef.current.active !== active || active.latestReadback !== index) return; + + const readbackStatus = response?.data?.status; + const nowPlaying = readbackStatus?.nowPlaying; + const readbackRevision = readbackStatus?.nowPlayingRevision; + const revisionIsNewer = nowPlayingRevision !== null && + Number.isSafeInteger(readbackRevision) && + readbackRevision > nowPlayingRevision; + onStatusReadback?.(readbackStatus); + if (revisionIsNewer && isErrorSource(nowPlaying?.Source)) { + clearReadbacks(); + commandRef.current.active = null; + setCommand({ + ...target, + generation, + outcome: 'failed', + error: nowPlaying.Source, + startNowPlayingRevision: nowPlayingRevision, + }); + } else if (revisionIsNewer && nowPlaying?.Source === target.source && + sourceAccountsMatch(target.source, nowPlaying?.SourceAccount, target.account)) { + const isFinalReadback = index === readbackDelays.length - 1; + setCommand({ + ...target, + generation, + outcome: isFinalReadback ? 'final-confirmed' : 'provisional-confirmed', + startNowPlayingRevision: nowPlayingRevision, + confirmedRevision: readbackRevision, + }); + if (isFinalReadback) { + clearReadbacks(); + commandRef.current.active = null; + } + } else if (index === readbackDelays.length - 1) { + commandRef.current.active = null; + setCommand({ + ...target, + generation, + outcome: 'unverified', + error: active.writeError?.message, + startNowPlayingRevision: nowPlayingRevision, + }); + } + } catch (_) { + if (commandRef.current.active === active && active.latestReadback === index && + index === readbackDelays.length - 1) { + commandRef.current.active = null; + setCommand({ + ...target, + generation, + outcome: 'unverified', + error: active.writeError?.message, + startNowPlayingRevision: nowPlayingRevision, + }); + } + } + }, Math.max(0, delay - (Date.now() - startedAt))); + commandRef.current.timers.push(timer); + }); + + try { + await api.selectSource(deviceId, target.source, target.account); + } catch (error) { + if (commandRef.current.active === active) active.writeError = error; + } } + const projectsTarget = command && (command.outcome === 'pending' || + command.outcome === 'provisional-confirmed' || command.outcome === 'final-confirmed'); + const projectedSource = projectsTarget + ? command.source : currentSource; + const projectedAccount = projectsTarget + ? command.account : (currentAccount ?? ''); + + const outcomeText = { + pending: 'Selecting source', + 'provisional-confirmed': 'Source selected, confirming', + 'final-confirmed': 'Source selected', + unverified: 'Source selection unverified', + failed: 'Source selection failed', + }; + return html`

Sources

${ready.map(src => { - const isActive = src.Source === currentSource && - (!src.SourceAccount || src.SourceAccount === currentAccount); + const account = src.SourceAccount ?? ''; + const isTarget = command?.source === src.Source && command?.account === account; + const isActive = src.Source === projectedSource && + sourceAccountsMatch(src.Source, account, projectedAccount); + const outcome = isTarget ? command.outcome : ''; return html`
+
+ ${availabilityMessage || (command ? outcomeText[command.outcome] : '')} +
`; -} \ No newline at end of file +} diff --git a/pkg/service/soundtouchweb/websocket.go b/pkg/service/soundtouchweb/websocket.go index b0ad54aa..50dcba96 100644 --- a/pkg/service/soundtouchweb/websocket.go +++ b/pkg/service/soundtouchweb/websocket.go @@ -752,6 +752,7 @@ func (app *WebApp) updateDeviceStatus(_ string, conn *webtypes.DeviceConnection, volume, volumeErr := conn.Client.GetVolume() presets, presetsErr := conn.Client.GetPresets() sources, sourcesErr := conn.Client.GetSources() + sourcesReadAt := time.Now() bass, bassErr := conn.Client.GetBass() var ( @@ -797,13 +798,14 @@ func (app *WebApp) updateDeviceStatus(_ string, conn *webtypes.DeviceConnection, if sourcesErr == nil { anyFetchSucceeded = true - - conn.CompleteFieldPoll(webtypes.FieldSources, sourcesGen, func(s *webtypes.DeviceStatus) { - s.Sources = sources - s.LastActivity = time.Now() - }) } + // Unlike the other fields, a FAILED /sources read is merged too: the last + // known inventory stays visible but is marked stale so it cannot be acted + // on. Running through CompleteFieldPoll is what makes a newer failure + // fence an older, still-in-flight success. + updateSourcesCache(conn, sourcesGen, sources, sourcesErr, sourcesReadAt) + if bassErr == nil { anyFetchSucceeded = true @@ -849,6 +851,30 @@ func (app *WebApp) updateDeviceStatus(_ string, conn *webtypes.DeviceConnection, } } +// updateSourcesCache records a source refresh, successful or not. A failure +// keeps the previous inventory and its read time, but marks it stale so the +// player stops offering it until a read succeeds again. +func updateSourcesCache( + conn *webtypes.DeviceConnection, + generation uint64, + sources *models.Sources, + err error, + readAt time.Time, +) bool { + return conn.CompleteFieldPoll(webtypes.FieldSources, generation, func(s *webtypes.DeviceStatus) { + if err != nil { + s.SourcesStale = true + + return + } + + s.Sources = sources + s.SourcesReadAt = readAt + s.SourcesStale = false + s.LastActivity = time.Now() + }) +} + func (app *WebApp) applyGroupUpdatedEvent( conn *webtypes.DeviceConnection, event *models.GroupUpdatedEvent, diff --git a/pkg/service/soundtouchweb/webtypes/revision_test.go b/pkg/service/soundtouchweb/webtypes/revision_test.go new file mode 100644 index 00000000..4b5d4f52 --- /dev/null +++ b/pkg/service/soundtouchweb/webtypes/revision_test.go @@ -0,0 +1,120 @@ +package webtypes + +import ( + "encoding/json" + "strings" + "sync" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +// TestNowPlayingRejectsPollOlderThanEvent is the core ordering property the +// player's source confirmation relies on: a /now_playing poll that started +// before a nowPlayingUpdated event must not overwrite the event's result when +// it finishes late. +func TestNowPlayingRejectsPollOlderThanEvent(t *testing.T) { + conn := NewDeviceConnection(nil, nil) + pollGeneration := conn.BeginFieldPoll(FieldNowPlaying) + eventNowPlaying := &models.NowPlaying{Track: "new event"} + + conn.ApplyFieldEvent(FieldNowPlaying, func(status *DeviceStatus) { + status.NowPlaying = eventNowPlaying + }) + + if conn.CompleteFieldPoll(FieldNowPlaying, pollGeneration, func(status *DeviceStatus) { + status.NowPlaying = &models.NowPlaying{Track: "old poll"} + }) { + t.Fatal("older NowPlaying poll was accepted after the event") + } + + if got := conn.Status().NowPlaying; got != eventNowPlaying { + t.Fatalf("older poll replaced NowPlaying event: %+v", got) + } +} + +func TestDeviceStatusRevisionAdvancesForEveryProjection(t *testing.T) { + conn := NewDeviceConnection(nil, nil) + if got := conn.Status().Revision; got != 0 { + t.Fatalf("initial revision = %d, want 0", got) + } + + conn.UpdateStatus(func(*DeviceStatus) {}) + if got := conn.Status().Revision; got != 1 { + t.Fatalf("revision after UpdateStatus = %d, want 1", got) + } + + // A caller-supplied Revision is never trusted: resetting it would make a + // browser holding a higher revision reject every later update. + conn.SetStatus(&DeviceStatus{Revision: 99}) + if got := conn.Status().Revision; got != 2 { + t.Fatalf("revision after SetStatus = %d, want 2", got) + } +} + +func TestUnrelatedProjectionDoesNotAdvanceNowPlayingRevision(t *testing.T) { + conn := NewDeviceConnection(nil, nil) + conn.SetStatus(&DeviceStatus{NowPlaying: &models.NowPlaying{Source: "SPOTIFY"}}) + baseline := conn.Status() + + volumeGeneration := conn.BeginFieldPoll(FieldVolume) + conn.CompleteFieldPoll(FieldVolume, volumeGeneration, func(status *DeviceStatus) { + status.Volume = &models.Volume{ActualVolume: 35} + }) + updated := conn.Status() + + if updated.Revision <= baseline.Revision { + t.Fatalf("aggregate revision = %d, want newer than %d", updated.Revision, baseline.Revision) + } + if updated.NowPlayingRevision != baseline.NowPlayingRevision { + t.Fatalf("now-playing revision = %d, want unchanged %d after volume update", + updated.NowPlayingRevision, baseline.NowPlayingRevision) + } +} + +func TestDeviceStatusRevisionIsMonotonicWithConcurrentProjections(t *testing.T) { + conn := NewDeviceConnection(nil, nil) + const projections = 64 + + var wg sync.WaitGroup + wg.Add(projections) + for i := range projections { + go func() { + defer wg.Done() + if i%2 == 0 { + conn.UpdateStatus(func(*DeviceStatus) {}) + + return + } + conn.SetStatus(&DeviceStatus{}) + }() + } + wg.Wait() + + if got := conn.Status().Revision; got != projections { + t.Fatalf("final revision = %d, want %d", got, projections) + } +} + +func TestDeviceStatusJSONExposesPublicRevisionsOnly(t *testing.T) { + conn := NewDeviceConnection(nil, nil) + conn.ApplyFieldEvent(FieldNowPlaying, func(status *DeviceStatus) { + status.NowPlaying = &models.NowPlaying{Track: "test"} + }) + + encoded, err := json.Marshal(conn.Status()) + if err != nil { + t.Fatalf("marshal DeviceStatus: %v", err) + } + + jsonStatus := string(encoded) + if !strings.Contains(jsonStatus, `"revision":1`) { + t.Fatalf("public revision missing from JSON: %s", jsonStatus) + } + if !strings.Contains(jsonStatus, `"nowPlayingRevision":1`) { + t.Fatalf("now-playing revision missing from JSON: %s", jsonStatus) + } + if strings.Contains(jsonStatus, "fieldGen") { + t.Fatalf("internal field generation leaked into JSON: %s", jsonStatus) + } +} diff --git a/pkg/service/soundtouchweb/webtypes/source_cache_test.go b/pkg/service/soundtouchweb/webtypes/source_cache_test.go new file mode 100644 index 00000000..39a1a569 --- /dev/null +++ b/pkg/service/soundtouchweb/webtypes/source_cache_test.go @@ -0,0 +1,113 @@ +package webtypes + +import ( + "encoding/json" + "strings" + "testing" + "time" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +func TestSourceCacheStatusAtTTLBoundary(t *testing.T) { + readAt := time.Date(2026, time.August, 30, 12, 0, 0, 0, time.UTC) + sources := &models.Sources{SourceItem: []models.SourceItem{{Source: "AUX"}}} + status := &DeviceStatus{ + Sources: sources, + SourcesReadAt: readAt, + } + + fresh := sourceCacheStatusAt(status, readAt.Add(sourceCacheTTL-time.Nanosecond), sourceCacheTTL) + if fresh.SourcesStale { + t.Fatal("source cache became stale before its TTL elapsed") + } + + stale := sourceCacheStatusAt(status, readAt.Add(sourceCacheTTL), sourceCacheTTL) + if !stale.SourcesStale { + t.Fatal("source cache was not stale at its TTL boundary") + } + if stale.Sources != sources { + t.Fatal("stale projection did not retain the last successful source list") + } + + refreshed := *stale + refreshed.SourcesReadAt = readAt.Add(sourceCacheTTL + time.Second) + refreshed.SourcesStale = false + + got := sourceCacheStatusAt(&refreshed, refreshed.SourcesReadAt, sourceCacheTTL) + if got.SourcesStale { + t.Fatal("successful source readback did not clear staleness immediately") + } +} + +func TestSourceCacheWithoutSuccessfulReadIsNotStale(t *testing.T) { + status := &DeviceStatus{Sources: &models.Sources{}} + + got := sourceCacheStatusAt(status, time.Now().Add(time.Hour), time.Nanosecond) + if got.SourcesStale { + t.Fatal("source cache without a recorded successful read was marked stale") + } +} + +func TestSourceCacheFailureWithoutInventoryIsExplicit(t *testing.T) { + conn := NewDeviceConnection(nil, nil) + conn.CompleteFieldPoll(FieldSources, conn.BeginFieldPoll(FieldSources), func(status *DeviceStatus) { + status.SourcesStale = true + }) + + status := conn.Status() + if status.Sources != nil || !status.SourcesStale { + t.Fatalf("initial source failure was not represented without inventory: %+v", status) + } + + encoded, err := json.Marshal(status) + if err != nil { + t.Fatalf("marshal initial source failure: %v", err) + } + if got := string(encoded); !strings.Contains(got, `"sourcesStale":true`) { + t.Fatalf("initial source failure omitted stale state: %s", got) + } +} + +// TestSourceCacheFailureFencesOlderOverlappingSuccess: a /sources read that +// FAILED must not be undone by an older, still-in-flight read that happens to +// succeed after it. Both go through CompleteFieldPoll(FieldSources, ...), so +// the failure's newer generation wins. +func TestSourceCacheFailureFencesOlderOverlappingSuccess(t *testing.T) { + conn := NewDeviceConnection(nil, nil) + readAt := time.Date(2026, time.August, 30, 12, 0, 0, 0, time.UTC) + retained := &models.Sources{SourceItem: []models.SourceItem{{Source: "AUX"}}} + + firstGeneration := conn.BeginFieldPoll(FieldSources) + secondGeneration := conn.BeginFieldPoll(FieldSources) + + conn.CompleteFieldPoll(FieldSources, secondGeneration, func(status *DeviceStatus) { + status.Sources = retained + status.SourcesReadAt = readAt + status.SourcesStale = true + }) + + older := &models.Sources{SourceItem: []models.SourceItem{{Source: "PRODUCT"}}} + if conn.CompleteFieldPoll(FieldSources, firstGeneration, func(status *DeviceStatus) { + status.Sources = older + status.SourcesStale = false + }) { + t.Fatal("older success was accepted after newer failure") + } + + if status := conn.Status(); !status.SourcesStale || status.Sources != retained { + t.Fatalf("older success cleared newer failure: %+v", status) + } + + newer := &models.Sources{SourceItem: []models.SourceItem{{Source: "BLUETOOTH"}}} + newerRead := time.Now() + conn.CompleteFieldPoll(FieldSources, conn.BeginFieldPoll(FieldSources), func(status *DeviceStatus) { + status.Sources = newer + status.SourcesReadAt = newerRead + status.SourcesStale = false + }) + + if status := conn.Status(); status.SourcesStale || status.Sources != newer { + t.Fatalf("newer source success did not restore actionability: %+v", status) + } +} diff --git a/pkg/service/soundtouchweb/webtypes/types.go b/pkg/service/soundtouchweb/webtypes/types.go index 90df2a8a..eef637d8 100644 --- a/pkg/service/soundtouchweb/webtypes/types.go +++ b/pkg/service/soundtouchweb/webtypes/types.go @@ -111,6 +111,8 @@ type DeviceStatus struct { Volume *models.Volume `json:"volume,omitempty"` Presets *models.Presets `json:"presets,omitempty"` Sources *models.Sources `json:"sources,omitempty"` + SourcesStale bool `json:"sourcesStale,omitempty"` + SourcesReadAt time.Time `json:"-"` Bass *models.Bass `json:"bass,omitempty"` Group *models.Group `json:"group,omitempty"` Connectivity Connectivity `json:"connectivity"` @@ -119,6 +121,18 @@ type DeviceStatus struct { SpeakerConnectionState *SpeakerConnectionState `json:"speakerConnectionState,omitempty"` IsConnected bool `json:"isConnected"` LastActivity time.Time `json:"lastActivity"` + + // Revision is a per-connection monotonic counter advanced by every + // successful UpdateStatus. It lets the browser order a full `devices` + // snapshot against a `status_update` delta -- without it the two frames + // carry no sequence at all and a slow snapshot can clobber a newer delta. + Revision uint64 `json:"revision"` + // NowPlayingRevision is the FieldNowPlaying generation that last wrote + // NowPlaying. Revision alone cannot answer "did now-playing actually + // change?", because an unrelated field's merge advances it too; a source + // selection waiting for authoritative confirmation needs exactly that + // distinction. + NowPlayingRevision uint64 `json:"nowPlayingRevision"` } // Connectivity is the player's aggregate view of HTTP and event-stream @@ -138,6 +152,10 @@ const ( offlineGracePeriod = 60 * time.Second ) +// sourceCacheTTL bounds how long a successfully-read source inventory stays +// actionable without a fresh confirmation from the speaker. +const sourceCacheTTL = 30 * time.Second + // SpeakerConnectionState is the network state reported by the speaker. type SpeakerConnectionState struct { State string `json:"state"` @@ -247,7 +265,20 @@ func (c *DeviceConnection) Info() *models.DeviceInfo { // mutated. Use UpdateStatus or SetStatus to apply changes. Never returns // nil for connections built via NewDeviceConnection. func (c *DeviceConnection) Status() *DeviceStatus { - return c.status.Load() + return sourceCacheStatusAt(c.status.Load(), time.Now(), sourceCacheTTL) +} + +func sourceCacheStatusAt(status *DeviceStatus, now time.Time, ttl time.Duration) *DeviceStatus { + stale := status.SourcesStale || status.Sources != nil && !status.SourcesReadAt.IsZero() && + !now.Before(status.SourcesReadAt.Add(ttl)) + if stale == status.SourcesStale { + return status + } + + next := *status + next.SourcesStale = stale + + return &next } // Done returns a channel that is closed when the connection is removed @@ -334,8 +365,40 @@ func (c *DeviceConnection) FinishWebSocketLoop() { // SetStatus atomically replaces the entire status. Use sparingly — // UpdateStatus is the preferred entry point because it preserves // concurrent changes from other goroutines. +// +// Revision is derived from the currently stored status rather than trusted +// from the caller: a replacement that reset it to the caller's zero value +// would make every browser holding a higher revision reject this device's +// subsequent updates outright. Replacing the whole status also supersedes +// every field, so each StatusField generation is advanced past any poll +// still in flight. func (c *DeviceConnection) SetStatus(s *DeviceStatus) { - c.status.Store(s) + nowPlayingGeneration := c.supersedeAllFields() + + for { + old := c.status.Load() + next := *s + next.Revision = old.Revision + 1 + next.NowPlayingRevision = nowPlayingGeneration + + if c.status.CompareAndSwap(old, &next) { + return + } + } +} + +// supersedeAllFields advances every StatusField generation past whatever is +// currently in flight and returns FieldNowPlaying's new generation. +func (c *DeviceConnection) supersedeAllFields() uint64 { + c.fieldGenMu.Lock() + defer c.fieldGenMu.Unlock() + + for field := range c.fieldGen { + c.fieldGen[field].issued++ + c.fieldGen[field].applied = c.fieldGen[field].issued + } + + return c.fieldGen[FieldNowPlaying].applied } // BeginFieldPoll reserves a generation for an asynchronous fetch of field, @@ -367,7 +430,10 @@ func (c *DeviceConnection) CompleteFieldPoll(field StatusField, generation uint6 c.fieldGen[field].applied = generation c.fieldGenMu.Unlock() - c.UpdateStatus(mut) + c.UpdateStatus(func(status *DeviceStatus) { + mut(status) + recordFieldRevision(status, field, generation) + }) return true } @@ -378,10 +444,26 @@ func (c *DeviceConnection) CompleteFieldPoll(field StatusField, generation uint6 func (c *DeviceConnection) ApplyFieldEvent(field StatusField, mut func(*DeviceStatus)) { c.fieldGenMu.Lock() c.fieldGen[field].issued++ - c.fieldGen[field].applied = c.fieldGen[field].issued + generation := c.fieldGen[field].issued + c.fieldGen[field].applied = generation c.fieldGenMu.Unlock() - c.UpdateStatus(mut) + c.UpdateStatus(func(status *DeviceStatus) { + mut(status) + recordFieldRevision(status, field, generation) + }) +} + +// recordFieldRevision publishes the generation that just wrote field, for the +// fields whose ordering a client needs to observe. Only FieldNowPlaying is +// published today: a source selection confirms itself by waiting for a +// now-playing write strictly newer than the one it started from, and the +// aggregate Revision cannot express that, because any other field's merge +// advances it too. +func recordFieldRevision(status *DeviceStatus, field StatusField, generation uint64) { + if field == FieldNowPlaying { + status.NowPlayingRevision = generation + } } // UpdateStatus atomically applies mut to a copy of the current status @@ -397,11 +479,14 @@ func (c *DeviceConnection) ApplyFieldEvent(field StatusField, mut func(*DeviceSt // reader still holding the previous snapshot). Production callers // receive these values fresh from the device API, so this is the // natural shape. +// +// Every successful store advances Revision exactly once. func (c *DeviceConnection) UpdateStatus(mut func(*DeviceStatus)) { for { old := c.status.Load() next := *old mut(&next) + next.Revision = old.Revision + 1 if c.status.CompareAndSwap(old, &next) { return @@ -735,6 +820,12 @@ type BassRequest struct { Level int `json:"level"` } +// SourceRequest represents an exact source selection request. +type SourceRequest struct { + Source string `json:"source"` + Account string `json:"account"` +} + // WebSocketMessage represents messages sent over WebSocket type WebSocketMessage struct { Type string `json:"type"`