diff --git a/pkg/service/soundtouchweb/handler_test.go b/pkg/service/soundtouchweb/handler_test.go index c3e7691..b2e9c3c 100644 --- a/pkg/service/soundtouchweb/handler_test.go +++ b/pkg/service/soundtouchweb/handler_test.go @@ -65,7 +65,7 @@ func TestNewWebApp(t *testing.T) { func TestHandleAPIDevices(t *testing.T) { app := createTestApp() - req := httptest.NewRequest("GET", "/api/devices", nil) + req := httptest.NewRequest("GET", "/api/control/devices", nil) w := httptest.NewRecorder() app.HandleAPIDevices(w, req) @@ -111,21 +111,21 @@ func TestHandleAPIDevice(t *testing.T) { }{ { name: "valid device", - path: "/api/device/test-device", + path: "/api/control/devices/test-device", chiID: "test-device", expectedStatus: http.StatusOK, expectSuccess: true, }, { name: "missing device ID", - path: "/api/device/", + path: "/api/control/devices/", chiID: "", expectedStatus: http.StatusBadRequest, expectSuccess: false, }, { name: "unknown device", - path: "/api/device/unknown", + path: "/api/control/devices/unknown", chiID: "unknown", expectedStatus: http.StatusNotFound, expectSuccess: false, @@ -166,7 +166,7 @@ func TestHandleAPIDevice(t *testing.T) { func TestHandleAPIControl_InvalidDevice(t *testing.T) { app := createTestApp() - req := httptest.NewRequest("GET", "/api/control/unknown-device/play", nil) + req := httptest.NewRequest("GET", "/api/control/devices/unknown-device/action/play", nil) req = withChiParams(req, map[string]string{"id": "unknown-device", "action": "play"}) w := httptest.NewRecorder() @@ -197,8 +197,8 @@ func TestHandleAPIControl_InvalidPath(t *testing.T) { name string path string }{ - {"missing action", "/api/control/test-device"}, - {"missing device and action", "/api/control/"}, + {"missing action", "/api/control/devices/test-device/action/"}, + {"missing device and action", "/api/control/devices//action/"}, } for _, tt := range tests { @@ -268,9 +268,9 @@ func TestHandleAPIControl_VolumeValidation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var req *http.Request if tt.body != "" { - req = httptest.NewRequest(tt.method, "/api/control/test-device/volume", strings.NewReader(tt.body)) + req = httptest.NewRequest(tt.method, "/api/control/devices/test-device/volume/50", strings.NewReader(tt.body)) } else { - req = httptest.NewRequest(tt.method, "/api/control/test-device/volume", nil) + req = httptest.NewRequest(tt.method, "/api/control/devices/test-device/volume/50", nil) } req = withChiParams(req, map[string]string{"id": "test-device", "action": "volume"}) req.Header.Set("Content-Type", "application/json") @@ -322,7 +322,7 @@ func TestHandleAPIControl_BassValidation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req := httptest.NewRequest(tt.method, "/api/control/test-device/bass", strings.NewReader(tt.body)) + req := httptest.NewRequest(tt.method, "/api/control/devices/test-device/action/bass", strings.NewReader(tt.body)) req = withChiParams(req, map[string]string{"id": "test-device", "action": "bass"}) req.Header.Set("Content-Type", "application/json") @@ -370,7 +370,7 @@ func TestHandleAPIControl_PresetValidation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req := httptest.NewRequest("GET", "/api/control/test-device/preset"+tt.query, nil) + req := httptest.NewRequest("GET", "/api/control/devices/test-device/action/preset"+tt.query, nil) req = withChiParams(req, map[string]string{"id": "test-device", "action": "preset"}) w := httptest.NewRecorder() @@ -395,7 +395,7 @@ func TestHandleAPIControl_PresetValidation(t *testing.T) { func TestHandleAPIControl_SourceValidation(t *testing.T) { app := createTestApp() - req := httptest.NewRequest("GET", "/api/control/test-device/source", nil) + req := httptest.NewRequest("GET", "/api/control/devices/test-device/action/source", nil) req = withChiParams(req, map[string]string{"id": "test-device", "action": "source"}) w := httptest.NewRecorder() @@ -444,7 +444,7 @@ func TestHandleAPIDiscover(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req := httptest.NewRequest(tt.method, "/api/discover", nil) + req := httptest.NewRequest(tt.method, "/api/control/discover", nil) w := httptest.NewRecorder() app.HandleAPIDiscover(w, req) @@ -511,7 +511,7 @@ func TestHandleWebSocket_InvalidUpgrade(t *testing.T) { func TestHandleAPIControl_UnsupportedAction(t *testing.T) { app := createTestApp() - req := httptest.NewRequest("GET", "/api/control/test-device/unsupported", nil) + req := httptest.NewRequest("GET", "/api/control/devices/test-device/action/unsupported", nil) req = withChiParams(req, map[string]string{"id": "test-device", "action": "unsupported"}) w := httptest.NewRecorder() @@ -542,7 +542,7 @@ func TestHandleAPIVersion(t *testing.T) { app.Date = "2023-01-01" app.RepoURL = "https://github.com/example/repo" - req := httptest.NewRequest("GET", "/api/version", nil) + req := httptest.NewRequest("GET", "/api/control/version", nil) w := httptest.NewRecorder() app.HandleAPIVersion(w, req) @@ -593,7 +593,7 @@ func BenchmarkHandleAPIDevices(b *testing.B) { app.AddDevice(deviceID, conn) } - req := httptest.NewRequest("GET", "/api/devices", nil) + req := httptest.NewRequest("GET", "/api/control/devices", nil) b.ResetTimer() for i := 0; i < b.N; i++ { @@ -604,7 +604,7 @@ func BenchmarkHandleAPIDevices(b *testing.B) { func BenchmarkHandleAPIDevice(b *testing.B) { app := createTestApp() - req := httptest.NewRequest("GET", "/api/device/test-device", nil) + req := httptest.NewRequest("GET", "/api/control/devices/test-device", nil) req = withChiParams(req, map[string]string{"id": "test-device"}) b.ResetTimer() @@ -684,7 +684,7 @@ func TestHandleDevicePlay_SourceAccountFiltering(t *testing.T) { "sourceAccount":"` + tt.sourceAccount + `", "itemName":"Venice Classic Radio" }`) - req := httptest.NewRequest("POST", "/api/device-play/play-device", body) + req := httptest.NewRequest("POST", "/api/control/devices/play-device/play", body) req.Header.Set("Content-Type", "application/json") req = withChiParams(req, map[string]string{"id": "play-device"}) w := httptest.NewRecorder() @@ -750,7 +750,7 @@ func TestHandleSourceControl_ForwardsAccount(t *testing.T) { conn.SetStatus(&webtypes.DeviceStatus{IsConnected: true, LastActivity: time.Now()}) app.AddDevice("source-device", conn) - req := httptest.NewRequest("GET", "/api/control/source-device/source?"+tt.query, nil) + req := httptest.NewRequest("GET", "/api/control/devices/source-device/action/source?"+tt.query, nil) req = withChiParams(req, map[string]string{"id": "source-device", "action": "source"}) w := httptest.NewRecorder() diff --git a/pkg/service/soundtouchweb/mount.go b/pkg/service/soundtouchweb/mount.go index c4ac11c..7e0df27 100644 --- a/pkg/service/soundtouchweb/mount.go +++ b/pkg/service/soundtouchweb/mount.go @@ -25,63 +25,80 @@ func (app *WebApp) Mount(r chi.Router, discoveryService *discovery.UnifiedDiscov // Health / liveness r.Get("/health", app.HandleHealth) - // API endpoints - r.Get("/api/devices", app.HandleAPIDevices) - r.Get("/api/device/{id}", app.HandleAPIDevice) - r.Get("/api/version", app.HandleAPIVersion) - r.Post("/api/discover", func(w http.ResponseWriter, r *http.Request) { - app.HandleAPIDiscover(w, r) + // Player / control API. Per #451 this is the post-merge canonical shape: + // device-scoped actions nest under devices/{id}/, so every direct child of + // /api/control is a literal namespace (devices, tunein, radiobrowser, + // version, discover) — no static-vs-param sibling, so routing never depends + // on chi's static-over-param precedence. + r.Route("/api/control", func(r chi.Router) { + r.Get("/version", app.HandleAPIVersion) - // Trigger discovery - //nolint:contextcheck // Context is created within goroutine - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + r.Post("/discover", func(w http.ResponseWriter, r *http.Request) { + app.HandleAPIDiscover(w, r) - app.BroadcastDiscoveryStatus("starting", app.DeviceCount()) + // Trigger discovery + //nolint:contextcheck // Context is created within goroutine + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() - app.DiscoverDevices(ctx, discoveryService) + app.BroadcastDiscoveryStatus("starting", app.DeviceCount()) - app.BroadcastDiscoveryStatus("completed", app.DeviceCount()) - app.BroadcastDeviceList() - }() + app.DiscoverDevices(ctx, discoveryService) + + app.BroadcastDiscoveryStatus("completed", app.DeviceCount()) + app.BroadcastDeviceList() + }() + }) + + // One /devices subrouter holds both the list and the /{id} subtree (the + // issue #285 single-subrouter lesson). Under /{id} every child is a + // literal action. + r.Route("/devices", func(r chi.Router) { + r.Get("/", app.HandleAPIDevices) + + r.Route("/{id}", func(r chi.Router) { + r.Get("/", app.HandleAPIDevice) + r.Post("/key/{key}", app.HandleDeviceKey) + r.Post("/volume/{volume}", app.HandleDirectVolumeControl) + r.Post("/power", app.HandleDevicePower) + r.Get("/power-status", app.HandleDevicePowerStatus) + r.Get("/recents", app.HandleDeviceRecents) + r.Post("/play", app.HandleDevicePlay) + r.Post("/play-url", app.HandlePlayURL) + // Proxied to the AfterTouch service's /api/setup/tts/speak. + r.Post("/speak", app.HandleAPISpeakText) + // Generic key / preset / source / bass actions. + r.Get("/action/{action}", app.HandleAPIControl) + r.Post("/action/{action}", app.HandleAPIControl) + r.Get("/ws", app.HandleDeviceWebSocket) + + r.Route("/zone", func(r chi.Router) { + r.Get("/", app.HandleGetZone) + r.Post("/add/{slaveId}", app.HandleZoneAdd) + r.Post("/remove/{slaveId}", app.HandleZoneRemove) + r.Post("/dissolve", app.HandleZoneDissolve) + r.Post("/leave", app.HandleZoneLeave) + }) + + r.Post("/tunein/play", app.HandlePlayTuneIn) + r.Post("/radiobrowser/play", app.HandlePlayRadioBrowser) + }) + }) + + // Browse / search (global, not device-scoped). + r.Route("/tunein", func(r chi.Router) { + r.Get("/search", app.HandleTuneInSearch) + r.Get("/search/next", app.HandleTuneInSearchNext) + r.Get("/navigate", app.HandleTuneInNavigate) + r.Get("/navigate/*", app.HandleTuneInNavigate) + }) + + r.Route("/radiobrowser", func(r chi.Router) { + r.Get("/search", app.HandleRadioBrowserSearch) + }) }) - // Device control endpoints (GET for most actions, POST for volume/bass) - r.Get("/api/control/{id}/{action}", app.HandleAPIControl) - r.Post("/api/control/{id}/{action}", app.HandleAPIControl) - - // TuneIn browse, search, and playback - r.Get("/api/tunein/search", app.HandleTuneInSearch) - r.Get("/api/tunein/search/next", app.HandleTuneInSearchNext) - r.Get("/api/tunein/navigate", app.HandleTuneInNavigate) - r.Get("/api/tunein/navigate/*", app.HandleTuneInNavigate) - r.Post("/api/tunein/play/{id}", app.HandlePlayTuneIn) - - // Enhanced device control endpoints - r.Post("/api/device-key/{id}/{key}", app.HandleDeviceKey) - r.Post("/api/device-volume/{id}/{volume}", app.HandleDirectVolumeControl) - r.Post("/api/device-power/{id}", app.HandleDevicePower) - r.Get("/api/device-power-status/{id}", app.HandleDevicePowerStatus) - r.Get("/api/device-recents/{id}", app.HandleDeviceRecents) - r.Post("/api/device-play/{id}", app.HandleDevicePlay) - r.Get("/api/zone/{id}", app.HandleGetZone) - r.Post("/api/zone/{id}/add/{slaveId}", app.HandleZoneAdd) - r.Post("/api/zone/{id}/remove/{slaveId}", app.HandleZoneRemove) - r.Post("/api/zone/{id}/dissolve", app.HandleZoneDissolve) - r.Post("/api/zone/{id}/leave", app.HandleZoneLeave) - r.Get("/api/device-ws/{id}", app.HandleDeviceWebSocket) - - // RadioBrowser search - r.Get("/api/radiobrowser/search", app.HandleRadioBrowserSearch) - r.Post("/api/radiobrowser/play/{id}", app.HandlePlayRadioBrowser) - - // Custom URL playback - r.Post("/api/play-url/{id}", app.HandlePlayURL) - - // Text-to-speech (proxied to the AfterTouch service's /setup/tts/speak) - r.Post("/api/device-speak/{id}", app.HandleAPISpeakText) - // SPA routes — serve index.html for client-side routing r.Get("/", app.serveIndex) r.Get("/devices", app.serveIndex) diff --git a/pkg/service/soundtouchweb/mount_test.go b/pkg/service/soundtouchweb/mount_test.go new file mode 100644 index 0000000..17e11b7 --- /dev/null +++ b/pkg/service/soundtouchweb/mount_test.go @@ -0,0 +1,61 @@ +package soundtouchweb + +import ( + "net/http" + "strings" + "testing" + + "github.com/go-chi/chi/v5" +) + +// TestMountControlAPIShape verifies the issue #451 web API restructure: +// building the router must not panic (catches any chi route-registration +// ambiguity), and every web `/api/*` route must live under `/api/control/*` +// (the post-merge canonical namespace). This is the only test that exercises +// Mount itself; the handler tests call handlers directly with injected params. +func TestMountControlAPIShape(t *testing.T) { + app := NewWebApp() + + r := chi.NewRouter() + app.Mount(r, nil) // must not panic while registering routes + + var apiRoutes []string + + walkErr := chi.Walk(r, func(_, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error { + if strings.HasPrefix(route, "/api/") { + apiRoutes = append(apiRoutes, route) + } + + return nil + }) + if walkErr != nil { + t.Fatalf("walk routes: %v", walkErr) + } + + if len(apiRoutes) == 0 { + t.Fatal("no /api/* routes registered") + } + + // Invariant: the whole web API is under /api/control/* (no flat /api/devices, + // /api/zone, /api/control/{id}/{action}, ... left behind). + for _, route := range apiRoutes { + if !strings.HasPrefix(route, "/api/control/") { + t.Errorf("web API route %q is not under /api/control/ after the #451 restructure", route) + } + } + + // Spot-check a representative endpoint actually registered. + found := false + + for _, route := range apiRoutes { + if route == "/api/control/version" { + found = true + + break + } + } + + if !found { + t.Errorf("expected /api/control/version to be registered; got %v", apiRoutes) + } +} diff --git a/pkg/service/soundtouchweb/static/js/api.js b/pkg/service/soundtouchweb/static/js/api.js index 3690bf6..d56b315 100644 --- a/pkg/service/soundtouchweb/static/js/api.js +++ b/pkg/service/soundtouchweb/static/js/api.js @@ -6,51 +6,51 @@ async function req(url, opts = {}) { } export const api = { - devices: () => req('/api/devices'), - device: (id) => req(`/api/device/${id}`), - discover: () => req('/api/discover', { method: 'POST' }), - key: (id, key) => req(`/api/device-key/${id}/${key}`, { method: 'POST' }), - volume: (id, level) => req(`/api/device-volume/${id}/${level}`, { method: 'POST' }), - bass: (id, level) => req(`/api/control/${id}/bass`, { + devices: () => req('/api/control/devices'), + device: (id) => req(`/api/control/devices/${id}`), + discover: () => req('/api/control/discover', { method: 'POST' }), + key: (id, key) => req(`/api/control/devices/${id}/key/${key}`, { method: 'POST' }), + volume: (id, level) => req(`/api/control/devices/${id}/volume/${level}`, { method: 'POST' }), + bass: (id, level) => req(`/api/control/devices/${id}/action/bass`, { method: 'POST', headers: JSON_HEADERS, body: JSON.stringify({ level }), }), - power: (id) => req(`/api/device-power/${id}`, { method: 'POST' }), - recents: (id) => req(`/api/device-recents/${id}`), - zone: (id) => req(`/api/zone/${id}`), - zoneAdd: (masterId, slaveId) => req(`/api/zone/${masterId}/add/${slaveId}`, { method: 'POST' }), - zoneRemove: (masterId, slaveId) => req(`/api/zone/${masterId}/remove/${slaveId}`, { method: 'POST' }), - zoneDissolve: (id) => req(`/api/zone/${id}/dissolve`, { method: 'POST' }), - zoneLeave: (id) => req(`/api/zone/${id}/leave`, { method: 'POST' }), - play: (id, item) => req(`/api/device-play/${id}`, { + power: (id) => req(`/api/control/devices/${id}/power`, { method: 'POST' }), + recents: (id) => req(`/api/control/devices/${id}/recents`), + zone: (id) => req(`/api/control/devices/${id}/zone`), + zoneAdd: (masterId, slaveId) => req(`/api/control/devices/${masterId}/zone/add/${slaveId}`, { method: 'POST' }), + zoneRemove: (masterId, slaveId) => req(`/api/control/devices/${masterId}/zone/remove/${slaveId}`, { method: 'POST' }), + zoneDissolve: (id) => req(`/api/control/devices/${id}/zone/dissolve`, { method: 'POST' }), + zoneLeave: (id) => req(`/api/control/devices/${id}/zone/leave`, { method: 'POST' }), + play: (id, item) => req(`/api/control/devices/${id}/play`, { method: 'POST', headers: JSON_HEADERS, body: JSON.stringify(item), }), - tuneInBrowse: (path) => req(path ? `/api/tunein/navigate/${path}` : '/api/tunein/navigate'), - tuneInSearch: (q) => req(`/api/tunein/search?q=${encodeURIComponent(q)}`), - tuneInSearchNext: (cursor) => req(`/api/tunein/search/next?cursor=${encodeURIComponent(cursor)}`), - control: (id, action, presetId) => req(`/api/control/${id}/${action}?id=${presetId}`), - storePreset: (id, slotId) => req(`/api/control/${id}/storepreset?id=${slotId}`), - selectSource: (id, source, account) => req(`/api/control/${id}/source?name=${encodeURIComponent(source)}&account=${encodeURIComponent(account || '')}`), - tuneInPlay: (deviceId, item) => req(`/api/tunein/play/${deviceId}`, { + tuneInBrowse: (path) => req(path ? `/api/control/tunein/navigate/${path}` : '/api/control/tunein/navigate'), + tuneInSearch: (q) => req(`/api/control/tunein/search?q=${encodeURIComponent(q)}`), + tuneInSearchNext: (cursor) => req(`/api/control/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 || '')}`), + tuneInPlay: (deviceId, item) => req(`/api/control/devices/${deviceId}/tunein/play`, { method: 'POST', headers: JSON_HEADERS, body: JSON.stringify(item), }), - radioBrowserSearch: (q) => req(`/api/radiobrowser/search?q=${encodeURIComponent(q)}`), - radioBrowserPlay: (deviceId, item) => req(`/api/radiobrowser/play/${deviceId}`, { + radioBrowserSearch: (q) => req(`/api/control/radiobrowser/search?q=${encodeURIComponent(q)}`), + radioBrowserPlay: (deviceId, item) => req(`/api/control/devices/${deviceId}/radiobrowser/play`, { method: 'POST', headers: JSON_HEADERS, body: JSON.stringify(item), }), - playURL: (deviceId, url, name, imageUrl, serviceUrl) => req(`/api/play-url/${deviceId}`, { + playURL: (deviceId, url, name, imageUrl, serviceUrl) => req(`/api/control/devices/${deviceId}/play-url`, { method: 'POST', headers: JSON_HEADERS, body: JSON.stringify({ url, name, imageUrl, serviceUrl }), }), - speak: (deviceId, text) => req(`/api/device-speak/${deviceId}`, { + speak: (deviceId, text) => req(`/api/control/devices/${deviceId}/speak`, { method: 'POST', headers: JSON_HEADERS, body: JSON.stringify({ text }), diff --git a/pkg/service/soundtouchweb/static/js/app.js b/pkg/service/soundtouchweb/static/js/app.js index 01ffb16..3d83f32 100644 --- a/pkg/service/soundtouchweb/static/js/app.js +++ b/pkg/service/soundtouchweb/static/js/app.js @@ -81,7 +81,7 @@ function App() { }; useEffect(() => { - fetch('/api/version') + fetch('/api/control/version') .then(res => res.json()) .then(resp => { if (resp.success) {