github/Bose-SoundTouch
1
0
Fork 0
mirror of https://github.com/gesellix/Bose-SoundTouch.git synced 2026-08-18 16:46:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(router): consolidate /device subrouter so PUT and DELETE actually resolve

Browse Source 
Issue #285's first fix (5f31616) registered the rename PUT inside a
chi subrouter at `/streaming/account/{account}/device`, alongside the
existing POST handlers. A *second* subrouter was already declared at
`/streaming/account/{account}/device/{device}` for the per-device
sub-resources (presets, recent, group, …). chi's radix tree treats
those two registrations as overlapping prefixes and at request time
prefers the more-specific `/device/{device}` subrouter — which had
no root-level method handlers. A PUT to /device/X fell through to
the [UNHANDLED] catch-all, got proxied to streaming.bose.com, came
back as 401 from CloudFront. Speakers retried in a loop.

The handlers-package regression test passed because the test router
in `pkg/service/handlers/main_test.go` is flatter (one subrouter for
device, no `/device/{device}` nested block). The route snapshot
test passed because `chi.Walk` enumerates each subrouter's
registrations independently — it doesn't simulate how the radix tree
will resolve a runtime request when subrouters overlap.

Reproduced against the actual production setupRouter in
TestPUTRenameRoutesToLocalHandler (new in router_test.go). Before
this commit: 404 / [UNHANDLED] / 401 proxy. After: 200 from
HandleMargeUpdateDevice.

Fix: collapse the two subrouters into one. All `/device` routes —
the POST/PUT/DELETE on the device resource itself plus the GET/POST
sub-resources — share a single `r.Route("/device", ...)` block with
explicit `/{device}/...` paths inside. No radix-tree ambiguity.

Knock-on: the `r.Delete("/device/{device}", server.HandleMargeRemoveDevice)`
that lived at the outer `/account/{account}` level moves into the
unified `/device` subrouter for symmetry. Its prior placement was
also being shadowed by the radix overlap, which is why the route
snapshot's first regeneration after this fix grew by exactly one
DELETE line — that route was never resolvable at runtime under the
old structure either.

Refs #285.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-05-15 18:08:52 +02:00
co-authored by Claude Opus 4.7
parent 596e24595d
commit ff96430f53
3 changed files with 78 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/soundtouch-service/main.go
+22 -15
View File
@@ -936,6 +936,16 @@ func setupRouter(server *handlers.Server) *chi.Mux {
r.Get("/presets/all", server.HandleMargeAccountPresets)
r.Get("/provider_settings", server.HandleMargeProviderSettings)
// All `/device` routes share one chi subrouter. Two
// overlapping subrouters (`/device` + `/device/{device}`)
// caused chi's radix-tree resolver to bind a runtime
// request to the more-specific prefix even when only the
// less-specific subrouter had a matching method handler,
// producing the [UNHANDLED] → upstream-proxy fall-through
// behind issue #285's first-attempted fix. One subrouter
// keeps every device-scoped path resolvable; see
// TestPUTRenameRoutesToLocalHandler for the regression
// against the production router.
r.Route("/device", func(r chi.Router) {
r.Post("/", server.HandleMargeAddDevice)
r.Post("/{device}", server.HandleMargeAddDevice)
@@ -944,21 +954,20 @@ func setupRouter(server *handlers.Server) *chi.Mux {
// when the user renames via Bose App or
// `soundtouch-cli name set`. Issue #285.
r.Put("/{device}", server.HandleMargeUpdateDevice)
})
r.Delete("/{device}", server.HandleMargeRemoveDevice)
r.Route("/device/{device}", func(r chi.Router) {
r.Get("/presets", server.HandleMargePresets)
r.Post("/presets/{presetNumber}", server.HandleMargeUpdatePreset)
r.Put("/preset/{presetNumber}", server.HandleMargeUpdatePreset)
r.Delete("/preset/{presetNumber}", server.HandleMargeRemovePreset)
r.Get("/recent", server.HandleMargeRecents)
r.Get("/recents", server.HandleMargeRecents)
r.Post("/recent", server.HandleMargeAddRecent)
r.Get("/{device}/presets", server.HandleMargePresets)
r.Post("/{device}/presets/{presetNumber}", server.HandleMargeUpdatePreset)
r.Put("/{device}/preset/{presetNumber}", server.HandleMargeUpdatePreset)
r.Delete("/{device}/preset/{presetNumber}", server.HandleMargeRemovePreset)
r.Get("/{device}/recent", server.HandleMargeRecents)
r.Get("/{device}/recents", server.HandleMargeRecents)
r.Post("/{device}/recent", server.HandleMargeAddRecent)
r.Get("/group", server.HandleMargeDeviceGroup)
r.Get("/group/", server.HandleMargeDeviceGroup)
r.Get("/group/server", server.HandleMargeDeviceGroupServer)
r.Get("/group/member", server.HandleMargeDeviceGroupMember)
r.Get("/{device}/group", server.HandleMargeDeviceGroup)
r.Get("/{device}/group/", server.HandleMargeDeviceGroup)
r.Get("/{device}/group/server", server.HandleMargeDeviceGroupServer)
r.Get("/{device}/group/member", server.HandleMargeDeviceGroupMember)
})
// Speakers POST to /group/ (with trailing slash) when forwarding
@@ -968,8 +977,6 @@ func setupRouter(server *handlers.Server) *chi.Mux {
r.Post("/group/", server.HandleMargeAddGroup)
r.Post("/group/{groupId}", server.HandleMargeModifyGroup)
r.Delete("/group/{groupId}", server.HandleMargeDeleteGroup)
r.Delete("/device/{device}", server.HandleMargeRemoveDevice)
})
r.Get("/device/{device}/streaming_token", server.HandleMargeStreamingToken)
cmd/soundtouch-service/router_test.go
+55
View File
@@ -3,6 +3,7 @@ package main
import (
"fmt"
"net/http"
"net/http/httptest"
"os"
"reflect"
"runtime"
@@ -10,6 +11,7 @@ import (
"strings"
"testing"
"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
"github.com/gesellix/bose-soundtouch/pkg/service/handlers"
"github.com/go-chi/chi/v5"
)
@@ -102,3 +104,56 @@ func TestPrintRoutes(t *testing.T) {
t.Errorf("Router routes changed! Diff the snapshot at %s with %s", snapshotPath, actualPath)
}
}
// TestPUTRenameRoutesToLocalHandler reproduces the runtime routing
// behaviour the user saw on their deployed v0.80.0: a PUT to
// /streaming/account/{a}/device/{d} should land on
// HandleMargeUpdateDevice, not fall through to the [UNHANDLED]
// proxy. The handlers-package test (TestIssue285_*) uses a simplified
// router that doesn't have the overlapping `/device` and
// `/device/{device}` route groups, so it can't catch a chi radix-
// tree resolution that prefers the more-specific subrouter.
//
// This test exercises the actual production setupRouter so a
// regression in the route topology is caught against the same chi
// behaviour speakers will see.
func TestPUTRenameRoutesToLocalHandler(t *testing.T) {
tempDir, err := os.MkdirTemp("", "router-rename-")
if err != nil {
t.Fatalf("mkdir temp: %v", err)
}
defer os.RemoveAll(tempDir)
ds := datastore.NewDataStore(tempDir)
_ = ds.Initialize()
server := handlers.NewServer(ds, nil, "http://localhost:8000", false, false, false)
r := setupRouter(server)
ts := httptest.NewServer(r)
defer ts.Close()
body := `<?xml version="1.0" encoding="UTF-8" ?><device deviceid="A81B6A536A98"><name>Sound Machinechen</name><macaddress>A81B6A536A98</macaddress></device>`
req, err := http.NewRequest(http.MethodPut,
ts.URL+"/streaming/account/1111111/device/A81B6A536A98",
strings.NewReader(body))
if err != nil {
t.Fatalf("build request: %v", err)
}
req.Header.Set("Content-Type", "application/xml")
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Fatalf("PUT: %v", err)
}
defer func() { _ = resp.Body.Close() }()
// 200 means our local HandleMargeUpdateDevice handled it.
// 401 / 502 / anything else means the request fell through to
// the [UNHANDLED] proxy and got the upstream response — which
// is exactly the failure mode #285 was supposed to fix.
if resp.StatusCode != http.StatusOK {
t.Fatalf("PUT status = %d, want 200 (local handler). Anything else means the request fell through to [UNHANDLED] proxy — chi is routing to a different subrouter than the PUT registration intended.", resp.StatusCode)
}
}
cmd/soundtouch-service/testdata/router_routes.txt
Vendored
+1
View File
@@ -8,6 +8,7 @@ DELETE /setup/dns-discoveries handlers.(
DELETE /setup/interactions/sessions handlers.(*Server).HandleCleanupSessions-fm
DELETE /setup/interactions/sessions/{session} handlers.(*Server).HandleDeleteSession-fm
DELETE /setup/parity-mismatches handlers.(*Server).HandleClearParityMismatches-fm
DELETE /streaming/account/{account}/device/{device} handlers.(*Server).HandleMargeRemoveDevice-fm
DELETE /streaming/account/{account}/device/{device}/preset/{presetNumber} handlers.(*Server).HandleMargeRemovePreset-fm
DELETE /streaming/account/{account}/group/{groupId} handlers.(*Server).HandleMargeDeleteGroup-fm
GET / handlers.(*Server).HandleRoot-fm