diff --git a/pkg/discovery/mediaserver.go b/pkg/discovery/mediaserver.go index 65d874a..d90235e 100644 --- a/pkg/discovery/mediaserver.go +++ b/pkg/discovery/mediaserver.go @@ -3,6 +3,7 @@ package discovery import ( "context" "log/slog" + "net/url" "sync" "time" ) @@ -140,6 +141,13 @@ func mediaServerFromDescription(desc *Description) (MediaServer, bool) { CDSControlURL: svc.ControlURL, } + // Populate Address from the CDS control URL host so callers know which + // host:port to reach the device on. The control URL is absolute after + // FetchDescription resolves it; parse errors leave Address empty. + if u, err := url.Parse(svc.ControlURL); err == nil { + srv.Address = u.Host + } + // Walk sub-devices to fill in UDN / FriendlyName if the root is sparse // (some devices put it all in the sub-device, e.g. FRITZ!Box). fillFromTree(desc, &srv) diff --git a/pkg/discovery/mediaserver_test.go b/pkg/discovery/mediaserver_test.go index 179e65d..f82d9d3 100644 --- a/pkg/discovery/mediaserver_test.go +++ b/pkg/discovery/mediaserver_test.go @@ -47,8 +47,19 @@ func TestMediaServerFromDescription_WithCDS(t *testing.T) { t.Errorf("IconURL %q is not absolute", srv.IconURL) } - t.Logf("MediaServer: UDN=%q FriendlyName=%q CDSControlURL=%q IconURL=%q", - srv.UDN, srv.FriendlyName, srv.CDSControlURL, srv.IconURL) + // Address must be the host:port from the CDS control URL. + // cannedDescriptionXML has URLBase http://192.0.2.1:49000 and CDS + // controlURL /ctl/ContentDir, so Address = "192.0.2.1:49000". + if srv.Address == "" { + t.Error("Address is empty") + } + + if srv.Address != "192.0.2.1:49000" { + t.Errorf("Address = %q, want %q", srv.Address, "192.0.2.1:49000") + } + + t.Logf("MediaServer: UDN=%q FriendlyName=%q CDSControlURL=%q IconURL=%q Address=%q", + srv.UDN, srv.FriendlyName, srv.CDSControlURL, srv.IconURL, srv.Address) } // TestMediaServerFromDescription_WithoutCDS verifies that a description @@ -147,6 +158,12 @@ func TestMediaServerFromDescription_FlatServer(t *testing.T) { if srv.IconURL != wantIcon { t.Errorf("IconURL = %q, want %q", srv.IconURL, wantIcon) } + + // Address must reflect the host:port of the CDS control URL. + // URLBase is http://198.51.100.20:8200 and CDS controlURL is /ctl/ContentDir. + if srv.Address != "198.51.100.20:8200" { + t.Errorf("Address = %q, want %q", srv.Address, "198.51.100.20:8200") + } } // isAbsoluteURL returns true when s starts with "http://" or "https://". diff --git a/pkg/service/soundtouchweb/handler_library.go b/pkg/service/soundtouchweb/handler_library.go index 68d5351..ee56e5e 100644 --- a/pkg/service/soundtouchweb/handler_library.go +++ b/pkg/service/soundtouchweb/handler_library.go @@ -46,6 +46,14 @@ type libraryPage struct { TotalItems int `json:"totalItems"` } +// normalizeUDN strips the "uuid:" prefix that UPnP device descriptions include +// in the UDN field (e.g. "uuid:fa095ecc-e13e-40e7-8e6c-e0286d5bc000") so the +// result matches the bare UUID that a SoundTouch speaker uses as the +// STORED_MUSIC sourceAccount before the "/0" suffix is appended. +func normalizeUDN(s string) string { + return strings.TrimPrefix(s, "uuid:") +} + // HandleDiscoverLibraryServers performs a LAN-wide SSDP sweep for DLNA media // servers and returns them as a JSON array. An optional ?timeout= query // parameter (in seconds, integer) overrides the default 5-second budget. @@ -69,7 +77,7 @@ func (app *WebApp) HandleDiscoverLibraryServers(w http.ResponseWriter, r *http.R out := make([]libraryServer, 0, len(servers)) for _, s := range servers { out = append(out, libraryServer{ - UDN: s.UDN, + UDN: normalizeUDN(s.UDN), Name: s.FriendlyName, Manufacturer: s.Manufacturer, Model: s.ModelName, @@ -164,7 +172,7 @@ func (app *WebApp) HandleAddLibraryServer(w http.ResponseWriter, r *http.Request return } - account := req.UDN + "/0" + account := normalizeUDN(req.UDN) + "/0" if err := device.Client.AddStoredMusicAccount(account, req.Name); err != nil { // Error code 1024 means the account is already registered on the speaker. diff --git a/pkg/service/soundtouchweb/handler_library_test.go b/pkg/service/soundtouchweb/handler_library_test.go index 95c6791..eb2cbe8 100644 --- a/pkg/service/soundtouchweb/handler_library_test.go +++ b/pkg/service/soundtouchweb/handler_library_test.go @@ -467,48 +467,121 @@ func TestHandleDeviceLibraryServers_UnknownDevice(t *testing.T) { // ---- HandleAddLibraryServer -------------------------------------------- +// TestDiscoverLibraryServers_UDNNormalization verifies the mapping logic used +// inside HandleDiscoverLibraryServers: a MediaServer with a "uuid:"-prefixed +// UDN must produce a libraryServer DTO with the bare UUID (no prefix), because +// SoundTouch STORED_MUSIC sourceAccounts use the bare form. +// This exercises normalizeUDN indirectly through the same code path used in +// the handler loop; HandleDiscoverLibraryServers itself cannot be called in a +// unit test because it invokes the real SSDP stack. +func TestDiscoverLibraryServers_UDNNormalization(t *testing.T) { + prefixedUDN := "uuid:fa095ecc-e13e-40e7-8e6c-e0286d5bc000" + want := "fa095ecc-e13e-40e7-8e6c-e0286d5bc000" + + got := libraryServer{ + UDN: normalizeUDN(prefixedUDN), + } + + if got.UDN != want { + t.Errorf("libraryServer UDN after normalizeUDN = %q, want %q", got.UDN, want) + } +} + +// TestNormalizeUDN verifies that normalizeUDN strips the "uuid:" prefix and +// is a no-op when the prefix is absent. +func TestNormalizeUDN(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"uuid:fa095ecc-e13e-40e7-8e6c-e0286d5bc000", "fa095ecc-e13e-40e7-8e6c-e0286d5bc000"}, + {"fa095ecc-e13e-40e7-8e6c-e0286d5bc000", "fa095ecc-e13e-40e7-8e6c-e0286d5bc000"}, + {"uuid:nas-udn", "nas-udn"}, + {"", ""}, + } + + for _, tt := range tests { + got := normalizeUDN(tt.input) + if got != tt.want { + t.Errorf("normalizeUDN(%q) = %q, want %q", tt.input, got, tt.want) + } + } +} + // TestHandleAddLibraryServer_AccountFormat verifies that the speaker receives -// a setMusicServiceAccount call with the account set to "/0". +// a setMusicServiceAccount call with the account set to "/0", i.e. +// any "uuid:" prefix is stripped before the "/0" suffix is appended. func TestHandleAddLibraryServer_AccountFormat(t *testing.T) { - // The client parses the response XML and checks for the success sentinel. - speaker, captured := setupSpeakerMock(t, map[string]string{ - "/setMusicServiceAccount": `/setMusicServiceAccount`, - }) - defer speaker.Close() - - app := newLibraryTestApp(speaker.URL) - - body := strings.NewReader(`{"udn":"uuid:nas-udn","name":"My NAS"}`) - req := httptest.NewRequest("POST", - "/api/control/devices/lib-device/library/servers", - body) - req.Header.Set("Content-Type", "application/json") - req = withChiParams(req, map[string]string{"id": "lib-device"}) - w := httptest.NewRecorder() - - app.HandleAddLibraryServer(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + tests := []struct { + name string + requestUDN string + wantAccount string + }{ + { + name: "bare UDN", + requestUDN: "nas-udn", + wantAccount: "nas-udn/0", + }, + { + name: "uuid-prefixed UDN is normalised", + requestUDN: "uuid:nas-udn", + wantAccount: "nas-udn/0", + }, } - var resp webtypes.APIResponse - if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { - t.Fatalf("decode: %v", err) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // The client parses the response XML and checks for the success sentinel. + speaker, captured := setupSpeakerMock(t, map[string]string{ + "/setMusicServiceAccount": `/setMusicServiceAccount`, + }) + defer speaker.Close() - if !resp.Success { - t.Fatalf("expected success=true, error=%s", resp.Error) - } + app := newLibraryTestApp(speaker.URL) - // The speaker should have been called at the setMusicServiceAccount endpoint. - setXML := captured["/setMusicServiceAccount"] - if setXML == "" { - t.Fatal("speaker /setMusicServiceAccount was never called") - } + bodyStr := `{"udn":"` + tt.requestUDN + `","name":"My NAS"}` + req := httptest.NewRequest("POST", + "/api/control/devices/lib-device/library/servers", + strings.NewReader(bodyStr)) + req.Header.Set("Content-Type", "application/json") + req = withChiParams(req, map[string]string{"id": "lib-device"}) + w := httptest.NewRecorder() - if !strings.Contains(setXML, "uuid:nas-udn/0") { - t.Errorf("setMusicServiceAccount XML should contain 'uuid:nas-udn/0', got:\n%s", setXML) + app.HandleAddLibraryServer(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + + var resp webtypes.APIResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode: %v", err) + } + + if !resp.Success { + t.Fatalf("expected success=true, error=%s", resp.Error) + } + + // The speaker should have been called at the setMusicServiceAccount endpoint. + setXML := captured["/setMusicServiceAccount"] + if setXML == "" { + t.Fatal("speaker /setMusicServiceAccount was never called") + } + + if !strings.Contains(setXML, tt.wantAccount) { + t.Errorf("setMusicServiceAccount XML should contain %q, got:\n%s", tt.wantAccount, setXML) + } + + // The response account field must also be the bare form. + data, ok := resp.Data.(map[string]interface{}) + if !ok { + t.Fatalf("resp.Data is not a map: %T", resp.Data) + } + + if got, _ := data["account"].(string); got != tt.wantAccount { + t.Errorf("response account = %q, want %q", got, tt.wantAccount) + } + }) } }