diff --git a/docs/images/ui-devices.png b/docs/images/ui-devices.png index c156ff5..a67b8c1 100644 Binary files a/docs/images/ui-devices.png and b/docs/images/ui-devices.png differ diff --git a/docs/images/ui-migration.png b/docs/images/ui-migration.png index 5541f3c..85f4703 100644 Binary files a/docs/images/ui-migration.png and b/docs/images/ui-migration.png differ diff --git a/docs/images/ui-settings.png b/docs/images/ui-settings.png index cc9491e..dc2ecfd 100644 Binary files a/docs/images/ui-settings.png and b/docs/images/ui-settings.png differ diff --git a/docs/images/ui-sync.png b/docs/images/ui-sync.png index a2996b4..b703c95 100644 Binary files a/docs/images/ui-sync.png and b/docs/images/ui-sync.png differ diff --git a/pkg/service/handlers/web/js/script.js b/pkg/service/handlers/web/js/script.js index fe403d5..728e20e 100644 --- a/pkg/service/handlers/web/js/script.js +++ b/pkg/service/handlers/web/js/script.js @@ -2673,8 +2673,11 @@ function readPlanURLOptions() { // validateURL classifies a string as an OK service URL. // Empty value is valid (means "use the canonical default"). Otherwise // the URL must parse, the scheme must be http or https, the hostname -// must be non-empty, and we reject "localhost" because the speaker -// can't reach this machine via that name. +// must be non-empty, and we flag loopback hostnames because they only +// reach AfterTouch in the on-device-install case (AfterTouch running +// on the speaker itself). For the typical "AfterTouch on a separate +// host" deployment, the speaker can't reach loopback on a different +// machine, so the URL must be a LAN-reachable IP or hostname. function validateURL(value) { const v = (value || "").trim(); if (!v) return {ok: true, error: ""}; @@ -2693,7 +2696,7 @@ function validateURL(value) { if (!u.hostname) return {ok: false, error: "hostname is empty"}; if (u.hostname === "localhost" || u.hostname === "127.0.0.1") { - return {ok: false, error: "use the LAN IP/hostname, not localhost — the speaker can't reach this machine via that name"}; + return {ok: false, error: "loopback URL — speakers can only reach this if AfterTouch is installed on the speaker itself (on-device install). For the typical multi-device setup, use a LAN-reachable IP or hostname."}; } return {ok: true, error: ""}; diff --git a/pkg/service/testing/fakespeaker/fakespeaker.go b/pkg/service/testing/fakespeaker/fakespeaker.go index dbd9a4c..78cf307 100644 --- a/pkg/service/testing/fakespeaker/fakespeaker.go +++ b/pkg/service/testing/fakespeaker/fakespeaker.go @@ -14,12 +14,13 @@ import ( "embed" "errors" "fmt" + "io" "net" "net/http" "time" ) -//go:embed testdata/info.xml testdata/presets.xml testdata/recents.xml +//go:embed testdata/info.xml testdata/presets.xml testdata/recents.xml testdata/networkinfo.xml testdata/sources.xml testdata/supportedurls.xml var fixtures embed.FS // Config configures a fake speaker. The zero value is valid and binds the @@ -120,6 +121,13 @@ func registerRoutes(mux *http.ServeMux) { mux.HandleFunc("/info", serveFixture("testdata/info.xml")) mux.HandleFunc("/presets", serveFixture("testdata/presets.xml")) mux.HandleFunc("/recents", serveFixture("testdata/recents.xml")) + mux.HandleFunc("/networkInfo", serveFixture("testdata/networkinfo.xml")) + mux.HandleFunc("/sources", serveFixture("testdata/sources.xml")) + mux.HandleFunc("/supportedURLs", serveFixture("testdata/supportedurls.xml")) + mux.HandleFunc("/getGroup", serveEmptyGroup) + mux.HandleFunc("/addGroup", handleAddGroup) + mux.HandleFunc("/updateGroup", handleUpdateGroup) + mux.HandleFunc("/removeGroup", handleRemoveGroup) } func serveFixture(path string) http.HandlerFunc { @@ -137,3 +145,124 @@ func serveFixture(path string) http.HandlerFunc { _, _ = w.Write(body) } } + +// serveEmptyGroup mirrors a real device's /getGroup response when it is +// not part of a stereo pair: an empty element. Tests that want +// to assert "no group" round-trip semantics can rely on this shape. +func serveEmptyGroup(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/xml; charset=utf-8") + _, _ = w.Write([]byte(`` + "\n\n")) +} + +// handleAddGroup echoes the posted XML back with +// GROUP_OK appended, matching the success path +// documented for the stereo-pair flow in issue #252 (see also +// soundtouch-cli/cmd_group.go and pkg/service/handlers/handlers_marge.go). +// On GET, returns the same empty-group shape as /getGroup so curl +// smoke-tests don't 405. Anything other than GET/POST gets a 405. +func handleAddGroup(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + serveEmptyGroup(w, r) + return + case http.MethodPost: + default: + w.Header().Set("Allow", "GET, POST") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + + return + } + + body, _ := io.ReadAll(http.MaxBytesReader(w, r.Body, 64*1024)) + + w.Header().Set("Content-Type", "application/xml; charset=utf-8") + + resp := buildAddGroupResponse(body) + _, _ = w.Write(resp) +} + +// handleUpdateGroup mirrors handleAddGroup's contract: POST a +// payload, get the same payload back with GROUP_OK +// appended. Real speakers use this for renames (POST /updateGroup with +// the changed ) and other in-place edits to an existing pair. +// GET returns the same empty-group shape /getGroup uses; non-GET/POST +// gets a 405. +func handleUpdateGroup(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + serveEmptyGroup(w, r) + return + case http.MethodPost: + default: + w.Header().Set("Allow", "GET, POST") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + + return + } + + body, _ := io.ReadAll(http.MaxBytesReader(w, r.Body, 64*1024)) + + w.Header().Set("Content-Type", "application/xml; charset=utf-8") + + resp := buildAddGroupResponse(body) + _, _ = w.Write(resp) +} + +// handleRemoveGroup matches the documented wiki behaviour: GET on the +// master speaker, no body, returns the now-empty group shape. The real +// device dissolves the pair on receipt; the fake is stateless so it +// just always responds as "no group right now". +func handleRemoveGroup(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + w.Header().Set("Allow", "GET") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + + return + } + + serveEmptyGroup(w, r) +} + +// buildAddGroupResponse inserts GROUP_OK before the +// closing tag of the posted body. If the body is empty or does +// not contain , it falls back to a minimal canned success +// response so callers still see a 200 + parseable XML. +func buildAddGroupResponse(posted []byte) []byte { + const closeTag = "" + + const okFragment = " GROUP_OK\n" + + if len(posted) == 0 { + return []byte(`` + "\n\n" + okFragment + closeTag + "\n") + } + + idx := indexOfClose(posted, closeTag) + if idx < 0 { + return []byte(`` + "\n\n" + okFragment + closeTag + "\n") + } + + out := make([]byte, 0, len(posted)+len(okFragment)) + out = append(out, posted[:idx]...) + out = append(out, []byte(okFragment)...) + out = append(out, posted[idx:]...) + + return out +} + +// indexOfClose returns the index of the last occurrence of needle in b, +// or -1 if not present. We scan from the right because real-world +// payloads can technically nest blocks (e.g. inside ), +// even though the documented stereo-pair payload does not. +func indexOfClose(b []byte, needle string) int { + if len(needle) == 0 || len(b) < len(needle) { + return -1 + } + + for i := len(b) - len(needle); i >= 0; i-- { + if string(b[i:i+len(needle)]) == needle { + return i + } + } + + return -1 +} diff --git a/pkg/service/testing/fakespeaker/fakespeaker_test.go b/pkg/service/testing/fakespeaker/fakespeaker_test.go index 3ced3ee..ba13aad 100644 --- a/pkg/service/testing/fakespeaker/fakespeaker_test.go +++ b/pkg/service/testing/fakespeaker/fakespeaker_test.go @@ -1,10 +1,12 @@ package fakespeaker import ( + "bytes" "context" "encoding/xml" "io" "net/http" + "strings" "testing" "time" ) @@ -29,6 +31,11 @@ func TestFakeSpeakerServesFixtures(t *testing.T) { {"/info", "info"}, {"/presets", "presets"}, {"/recents", "recents"}, + {"/networkInfo", "networkInfo"}, + {"/sources", "sources"}, + {"/supportedURLs", "supportedURLs"}, + {"/getGroup", "group"}, + {"/removeGroup", "group"}, } for _, tc := range cases { @@ -62,3 +69,133 @@ func TestFakeSpeakerServesFixtures(t *testing.T) { }) } } + +func TestFakeSpeakerAddGroupEchoesWithGroupOK(t *testing.T) { + s, err := Start(Config{}) + if err != nil { + t.Fatalf("start: %v", err) + } + + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + _ = s.Stop(ctx) + }) + + posted := ` + + TEST + DEADBEEFCAFE + + DEADBEEFCAFELEFT127.0.0.1 + 0000DEADBEEFRIGHT127.0.0.2 + +` + + resp, err := http.Post("http://"+s.HTTPAddr()+"/addGroup", "application/xml", strings.NewReader(posted)) //nolint:noctx + if err != nil { + t.Fatalf("post: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200", resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + + // Echo: the posted name + roles survive in the response. + if !bytes.Contains(body, []byte("TEST")) { + t.Errorf("response missing posted ; body:\n%s", body) + } + + if !bytes.Contains(body, []byte("DEADBEEFCAFE")) { + t.Errorf("response missing posted ; body:\n%s", body) + } + + // Success marker: GROUP_OK appears before . + statusIdx := bytes.Index(body, []byte("GROUP_OK")) + if statusIdx < 0 { + t.Fatalf("response missing GROUP_OK; body:\n%s", body) + } + + closeIdx := bytes.LastIndex(body, []byte("")) + if closeIdx < 0 || statusIdx >= closeIdx { + t.Errorf(" not nested inside ...; body:\n%s", body) + } +} + +func TestFakeSpeakerUpdateGroupEchoesWithGroupOK(t *testing.T) { + s, err := Start(Config{}) + if err != nil { + t.Fatalf("start: %v", err) + } + + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + _ = s.Stop(ctx) + }) + + posted := ` + + RENAMED + DEADBEEFCAFE +` + + resp, err := http.Post("http://"+s.HTTPAddr()+"/updateGroup", "application/xml", strings.NewReader(posted)) //nolint:noctx + if err != nil { + t.Fatalf("post: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200", resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + + if !bytes.Contains(body, []byte("RENAMED")) { + t.Errorf("response missing posted ; body:\n%s", body) + } + + if !bytes.Contains(body, []byte("GROUP_OK")) { + t.Errorf("response missing GROUP_OK; body:\n%s", body) + } +} + +func TestFakeSpeakerRemoveGroupRejectsNonGET(t *testing.T) { + s, err := Start(Config{}) + if err != nil { + t.Fatalf("start: %v", err) + } + + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + _ = s.Stop(ctx) + }) + + resp, err := http.Post("http://"+s.HTTPAddr()+"/removeGroup", "application/xml", strings.NewReader("")) //nolint:noctx + if err != nil { + t.Fatalf("post: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusMethodNotAllowed { + t.Fatalf("status = %d, want 405", resp.StatusCode) + } + + if got := resp.Header.Get("Allow"); got != "GET" { + t.Errorf("Allow header = %q, want %q", got, "GET") + } +} diff --git a/pkg/service/testing/fakespeaker/testdata/networkinfo.xml b/pkg/service/testing/fakespeaker/testdata/networkinfo.xml new file mode 100644 index 0000000..eea3a12 --- /dev/null +++ b/pkg/service/testing/fakespeaker/testdata/networkinfo.xml @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/pkg/service/testing/fakespeaker/testdata/sources.xml b/pkg/service/testing/fakespeaker/testdata/sources.xml new file mode 100644 index 0000000..b06f768 --- /dev/null +++ b/pkg/service/testing/fakespeaker/testdata/sources.xml @@ -0,0 +1,8 @@ + + + AUX IN + + DemoSpotifyAccount + + + \ No newline at end of file diff --git a/pkg/service/testing/fakespeaker/testdata/supportedurls.xml b/pkg/service/testing/fakespeaker/testdata/supportedurls.xml new file mode 100644 index 0000000..8da4128 --- /dev/null +++ b/pkg/service/testing/fakespeaker/testdata/supportedurls.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/scripts/screenshots/manifest.json b/scripts/screenshots/manifest.json index 023e274..f47d1c8 100644 --- a/scripts/screenshots/manifest.json +++ b/scripts/screenshots/manifest.json @@ -5,21 +5,21 @@ "path": "/", "click_selector": "button[onclick*=\"tab-settings\"]", "wait_selector": "#tab-settings.active", - "settle_ms": 300 + "settle_ms": 2000 }, { "name": "ui-devices", "path": "/", "click_selector": "button[onclick*=\"tab-devices\"]", "wait_selector": "#tab-devices.active", - "settle_ms": 500 + "settle_ms": 2500 }, { "name": "ui-sync", "path": "/", "click_selector": "button[onclick*=\"tab-sync\"]", "wait_selector": "#tab-sync.active", - "settle_ms": 300 + "settle_ms": 1000 }, { "name": "ui-migration", @@ -31,4 +31,4 @@ "settle_ms": 4000 } ] -} +} \ No newline at end of file diff --git a/scripts/screenshots/run.sh b/scripts/screenshots/run.sh index adc0c65..3185d8a 100755 --- a/scripts/screenshots/run.sh +++ b/scripts/screenshots/run.sh @@ -39,11 +39,18 @@ go build -o "$LOG_DIR/soundtouch-service" ./cmd/soundtouch-service go build -o "$LOG_DIR/dummy-speaker" ./cmd/dummy-speaker go build -o "$LOG_DIR/screenshots" ./scripts/screenshots -echo "==> seeding settings.json (generic hostname + discovery off to avoid leaking real network info)" +echo "==> seeding settings.json (aftertouch.localhost + discovery off to avoid leaking real network info)" +# `aftertouch.localhost` is RFC 6761: any *.localhost name resolves to +# loopback via the system resolver in milliseconds (verified ~8ms on +# macOS / glibc / systemd-resolved). That gives us a brand-friendly URL +# in the screenshots without the ~5s DNS-timeout cliff that bites on +# unresolvable hostnames like aftertouch.local — that cliff compounds +# across /setup/settings + /setup/summary and pushes past the chromedp +# 30s per-shot budget. cat > "$DATA_DIR/settings.json" <<'EOF' { - "server_url": "http://aftertouch.local:8000", - "https_server_url": "https://aftertouch.local:8443", + "server_url": "http://aftertouch.localhost:8000", + "https_server_url": "https://aftertouch.localhost:8443", "discovery_enabled": false, "discovery_interval": "1h" }