diff --git a/internal/lidarr/client.go b/internal/lidarr/client.go index 48af1d3d..da531e97 100644 --- a/internal/lidarr/client.go +++ b/internal/lidarr/client.go @@ -9,6 +9,8 @@ import ( "net/http" "net/url" "strconv" + "strings" + "time" ) // Client wraps Lidarr's v1 HTTP API. BaseURL is the Lidarr instance @@ -19,6 +21,17 @@ type Client struct { HTTP *http.Client } +// NewClient builds a Client with sensible HTTP defaults (30s timeout, +// matching the pattern in internal/scrobble/listenbrainz). Call sites +// that need a custom client can construct the Client struct directly. +func NewClient(baseURL, apiKey string) *Client { + return &Client{ + BaseURL: baseURL, + APIKey: apiKey, + HTTP: &http.Client{Timeout: 30 * time.Second}, + } +} + // get executes an authenticated GET request and maps HTTP status codes to // typed errors. The caller is responsible for closing the returned body. func (c *Client) get(ctx context.Context, path string, q url.Values) (*http.Response, error) { @@ -26,7 +39,7 @@ func (c *Client) get(ctx context.Context, path string, q url.Values) (*http.Resp if err != nil { return nil, fmt.Errorf("%w: %v", ErrUnreachable, err) } - u.Path = u.Path + path + u.Path = strings.TrimRight(u.Path, "/") + path if q != nil { u.RawQuery = q.Encode() } @@ -62,7 +75,7 @@ func (c *Client) post(ctx context.Context, path string, body []byte) (*http.Resp if err != nil { return nil, fmt.Errorf("%w: %v", ErrUnreachable, err) } - u.Path = u.Path + path + u.Path = strings.TrimRight(u.Path, "/") + path req, err := http.NewRequestWithContext(ctx, http.MethodPost, u.String(), bytes.NewReader(body)) if err != nil { return nil, err @@ -102,15 +115,11 @@ func (c *Client) LookupArtist(ctx context.Context, term string) ([]LookupResult, return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) } var raw []struct { - ForeignArtistID string `json:"foreignArtistId"` - ArtistName string `json:"artistName"` - Genres []string `json:"genres"` - AlbumCount int `json:"albumCount"` - Images []struct { - CoverType string `json:"coverType"` - RemoteURL string `json:"remoteUrl"` - URL string `json:"url"` - } `json:"images"` + ForeignArtistID string `json:"foreignArtistId"` + ArtistName string `json:"artistName"` + Genres []string `json:"genres"` + AlbumCount int `json:"albumCount"` + Images []lidarrImage `json:"images"` } if err := json.Unmarshal(body, &raw); err != nil { return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) @@ -150,16 +159,13 @@ func (c *Client) LookupAlbum(ctx context.Context, term string) ([]LookupResult, return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) } var raw []struct { - ForeignAlbumID string `json:"foreignAlbumId"` - Title string `json:"title"` - ArtistName string `json:"artistName"` - ReleaseDate string `json:"releaseDate"` - TrackCount int `json:"trackCount"` - Images []struct { - CoverType string `json:"coverType"` - RemoteURL string `json:"remoteUrl"` - URL string `json:"url"` - } `json:"images"` + ForeignAlbumID string `json:"foreignAlbumId"` + ForeignArtistID string `json:"foreignArtistId"` + Title string `json:"title"` + ArtistName string `json:"artistName"` + ReleaseDate string `json:"releaseDate"` + TrackCount int `json:"trackCount"` + Images []lidarrImage `json:"images"` } if err := json.Unmarshal(body, &raw); err != nil { return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) @@ -170,18 +176,28 @@ func (c *Client) LookupAlbum(ctx context.Context, term string) ([]LookupResult, if len(r.ReleaseDate) >= 4 { year = r.ReleaseDate[:4] } - secondary := r.ArtistName + secondary := "" + if r.ArtistName != "" { + secondary = r.ArtistName + } if year != "" { - secondary += " · " + year + if secondary != "" { + secondary += " · " + } + secondary += year } if r.TrackCount > 0 { - secondary += " · " + strconv.Itoa(r.TrackCount) + " tracks" + if secondary != "" { + secondary += " · " + } + secondary += strconv.Itoa(r.TrackCount) + " tracks" } out = append(out, LookupResult{ - MBID: r.ForeignAlbumID, - Name: r.Title, - Secondary: secondary, - ImageURL: pickPosterImage(r.Images), + MBID: r.ForeignAlbumID, + ArtistMBID: r.ForeignArtistID, + Name: r.Title, + Secondary: secondary, + ImageURL: pickPosterImage(r.Images), }) } return out, nil @@ -200,10 +216,12 @@ func (c *Client) LookupTrack(ctx context.Context, term string) ([]LookupResult, return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) } var raw []struct { - ForeignTrackID string `json:"foreignTrackId"` - Title string `json:"title"` - AlbumTitle string `json:"albumTitle"` - ArtistName string `json:"artistName"` + ForeignTrackID string `json:"foreignTrackId"` + ForeignAlbumID string `json:"foreignAlbumId"` + ForeignArtistID string `json:"foreignArtistId"` + Title string `json:"title"` + AlbumTitle string `json:"albumTitle"` + ArtistName string `json:"artistName"` } if err := json.Unmarshal(body, &raw); err != nil { return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) @@ -218,9 +236,11 @@ func (c *Client) LookupTrack(ctx context.Context, term string) ([]LookupResult, secondary += r.ArtistName } out = append(out, LookupResult{ - MBID: r.ForeignTrackID, - Name: r.Title, - Secondary: secondary, + MBID: r.ForeignTrackID, + ArtistMBID: r.ForeignArtistID, + AlbumMBID: r.ForeignAlbumID, + Name: r.Title, + Secondary: secondary, }) } return out, nil @@ -233,7 +253,7 @@ func (c *Client) AddArtist(ctx context.Context, p AddArtistParams) error { if p.MonitorAll { monitor = "all" } - body, _ := json.Marshal(map[string]any{ + body, err := json.Marshal(map[string]any{ "foreignArtistId": p.ForeignArtistID, "qualityProfileId": p.QualityProfileID, "rootFolderPath": p.RootFolderPath, @@ -241,6 +261,9 @@ func (c *Client) AddArtist(ctx context.Context, p AddArtistParams) error { "monitor": monitor, "addOptions": map[string]any{"searchForMissingAlbums": true}, }) + if err != nil { + return fmt.Errorf("%w: marshal: %v", ErrInvalidPayload, err) + } resp, err := c.post(ctx, "/api/v1/artist", body) if err != nil { return err @@ -252,7 +275,7 @@ func (c *Client) AddArtist(ctx context.Context, p AddArtistParams) error { // AddAlbum posts to POST /api/v1/album. Returns nil on 2xx; typed error // otherwise. func (c *Client) AddAlbum(ctx context.Context, p AddAlbumParams) error { - body, _ := json.Marshal(map[string]any{ + body, err := json.Marshal(map[string]any{ "foreignAlbumId": p.ForeignAlbumID, "foreignArtistId": p.ForeignArtistID, "qualityProfileId": p.QualityProfileID, @@ -260,6 +283,9 @@ func (c *Client) AddAlbum(ctx context.Context, p AddAlbumParams) error { "monitored": true, "addOptions": map[string]any{"searchForNewAlbum": true}, }) + if err != nil { + return fmt.Errorf("%w: marshal: %v", ErrInvalidPayload, err) + } resp, err := c.post(ctx, "/api/v1/album", body) if err != nil { return err @@ -344,11 +370,7 @@ func (c *Client) Ping(ctx context.Context) (PingResult, error) { // pickPosterImage returns the remote URL (preferred) or local URL for the // first image with coverType=="poster". Returns "" when none match. -func pickPosterImage(imgs []struct { - CoverType string `json:"coverType"` - RemoteURL string `json:"remoteUrl"` - URL string `json:"url"` -}) string { +func pickPosterImage(imgs []lidarrImage) string { for _, img := range imgs { if img.CoverType == "poster" { if img.RemoteURL != "" { diff --git a/internal/lidarr/client_test.go b/internal/lidarr/client_test.go index b8997e2e..5f56f0bf 100644 --- a/internal/lidarr/client_test.go +++ b/internal/lidarr/client_test.go @@ -154,6 +154,12 @@ func TestLookupAlbum_HappyPath(t *testing.T) { if got[0].MBID != "a1b2c3d4-e5f6-7890-abcd-ef1234567890" { t.Errorf("MBID = %q", got[0].MBID) } + if got[0].ArtistMBID != "069b64b6-7884-4f6a-94cc-e4c1d6c87a01" { + t.Errorf("ArtistMBID = %q, want 069b64b6-7884-4f6a-94cc-e4c1d6c87a01", got[0].ArtistMBID) + } + if got[0].AlbumMBID != "" { + t.Errorf("AlbumMBID = %q, want empty on album result", got[0].AlbumMBID) + } if got[0].Name != "Music Has the Right to Children" { t.Errorf("Name = %q", got[0].Name) } @@ -213,6 +219,12 @@ func TestLookupTrack_HappyPath(t *testing.T) { if got[0].Secondary != want { t.Errorf("Secondary = %q, want %q", got[0].Secondary, want) } + if got[0].ArtistMBID != "069b64b6-7884-4f6a-94cc-e4c1d6c87a01" { + t.Errorf("ArtistMBID = %q, want 069b64b6-7884-4f6a-94cc-e4c1d6c87a01", got[0].ArtistMBID) + } + if got[0].AlbumMBID != "a1b2c3d4-e5f6-7890-abcd-ef1234567890" { + t.Errorf("AlbumMBID = %q, want a1b2c3d4-e5f6-7890-abcd-ef1234567890", got[0].AlbumMBID) + } if got[0].ImageURL != "" { t.Errorf("tracks carry no images; ImageURL = %q", got[0].ImageURL) } @@ -406,6 +418,51 @@ func TestListQualityProfiles_BadJSON(t *testing.T) { } } +// --- LookupArtist (additional) --- + +func TestLookupArtist_LookupFailed(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + c := &Client{BaseURL: srv.URL, APIKey: "x", HTTP: srv.Client()} + _, err := c.LookupArtist(context.Background(), "boards") + if !errors.Is(err, ErrLookupFailed) { + t.Fatalf("err = %v, want ErrLookupFailed", err) + } +} + +func TestGet_BaseURLWithTrailingSlash(t *testing.T) { + body, _ := os.ReadFile("testdata/lookup_artist.json") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Verify path is /api/v1/artist/lookup, NOT //api/v1/artist/lookup + if r.URL.Path != "/api/v1/artist/lookup" { + t.Errorf("got path %q; trailing-slash BaseURL produced double-slash", r.URL.Path) + } + _, _ = w.Write(body) + })) + defer srv.Close() + c := &Client{BaseURL: srv.URL + "/", APIKey: "k", HTTP: srv.Client()} + _, err := c.LookupArtist(context.Background(), "boards") + if err != nil { + t.Fatalf("LookupArtist: %v", err) + } +} + +// --- AddArtist (additional) --- + +func TestAddArtist_PostFailed(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + })) + defer srv.Close() + c := &Client{BaseURL: srv.URL, APIKey: "x", HTTP: srv.Client()} + err := c.AddArtist(context.Background(), AddArtistParams{ForeignArtistID: "x", QualityProfileID: 1, RootFolderPath: "/m"}) + if !errors.Is(err, ErrLookupFailed) { + t.Fatalf("err = %v, want ErrLookupFailed", err) + } +} + // --- ListRootFolders --- func TestListRootFolders_HappyPath(t *testing.T) { @@ -439,7 +496,7 @@ func TestListRootFolders_HappyPath(t *testing.T) { } } -func TestListRootFolders_BadJSON(t *testing.T) { +func TestListRootFolders_NullBody(t *testing.T) { c, srv := newTestClient(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(`null`)) }) @@ -454,6 +511,18 @@ func TestListRootFolders_BadJSON(t *testing.T) { } } +func TestListRootFolders_BadJSON(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("{not valid json")) + })) + defer srv.Close() + c := &Client{BaseURL: srv.URL, APIKey: "x", HTTP: srv.Client()} + _, err := c.ListRootFolders(context.Background()) + if !errors.Is(err, ErrInvalidPayload) { + t.Fatalf("err = %v, want ErrInvalidPayload", err) + } +} + // --- Ping --- func TestPing_ReturnsVersion(t *testing.T) { @@ -507,11 +576,7 @@ func TestPing_BadJSON(t *testing.T) { // --- pickPosterImage --- func TestPickPosterImage_PreferRemoteURL(t *testing.T) { - imgs := []struct { - CoverType string `json:"coverType"` - RemoteURL string `json:"remoteUrl"` - URL string `json:"url"` - }{ + imgs := []lidarrImage{ {CoverType: "poster", RemoteURL: "https://remote.example/img.jpg", URL: "https://local.example/img.jpg"}, } got := pickPosterImage(imgs) @@ -521,11 +586,7 @@ func TestPickPosterImage_PreferRemoteURL(t *testing.T) { } func TestPickPosterImage_FallsBackToLocalURL(t *testing.T) { - imgs := []struct { - CoverType string `json:"coverType"` - RemoteURL string `json:"remoteUrl"` - URL string `json:"url"` - }{ + imgs := []lidarrImage{ {CoverType: "poster", RemoteURL: "", URL: "https://local.example/img.jpg"}, } got := pickPosterImage(imgs) @@ -535,11 +596,7 @@ func TestPickPosterImage_FallsBackToLocalURL(t *testing.T) { } func TestPickPosterImage_SkipsNonPoster(t *testing.T) { - imgs := []struct { - CoverType string `json:"coverType"` - RemoteURL string `json:"remoteUrl"` - URL string `json:"url"` - }{ + imgs := []lidarrImage{ {CoverType: "banner", RemoteURL: "https://banner.example/img.jpg"}, {CoverType: "cover", RemoteURL: "https://cover.example/img.jpg"}, } diff --git a/internal/lidarr/testdata/lookup_album.json b/internal/lidarr/testdata/lookup_album.json index c8901a50..b1bb6e35 100644 --- a/internal/lidarr/testdata/lookup_album.json +++ b/internal/lidarr/testdata/lookup_album.json @@ -1,6 +1,7 @@ [ { "foreignAlbumId": "a1b2c3d4-e5f6-7890-abcd-ef1234567890", + "foreignArtistId": "069b64b6-7884-4f6a-94cc-e4c1d6c87a01", "title": "Music Has the Right to Children", "artistName": "Boards of Canada", "releaseDate": "1998-04-27", @@ -12,6 +13,7 @@ }, { "foreignAlbumId": "b2c3d4e5-f6a7-8901-bcde-f12345678901", + "foreignArtistId": "069b64b6-7884-4f6a-94cc-e4c1d6c87a01", "title": "Geogaddi", "artistName": "Boards of Canada", "releaseDate": "2002-02-11", diff --git a/internal/lidarr/testdata/lookup_track.json b/internal/lidarr/testdata/lookup_track.json index 15230230..10751762 100644 --- a/internal/lidarr/testdata/lookup_track.json +++ b/internal/lidarr/testdata/lookup_track.json @@ -1,12 +1,16 @@ [ { "foreignTrackId": "t1a2b3c4-d5e6-7890-abcd-ef1234567890", + "foreignAlbumId": "a1b2c3d4-e5f6-7890-abcd-ef1234567890", + "foreignArtistId": "069b64b6-7884-4f6a-94cc-e4c1d6c87a01", "title": "Roygbiv", "albumTitle": "Music Has the Right to Children", "artistName": "Boards of Canada" }, { "foreignTrackId": "t2b3c4d5-e6f7-8901-bcde-f12345678901", + "foreignAlbumId": "a1b2c3d4-e5f6-7890-abcd-ef1234567890", + "foreignArtistId": "069b64b6-7884-4f6a-94cc-e4c1d6c87a01", "title": "Aquarius", "albumTitle": "Music Has the Right to Children", "artistName": "Boards of Canada" diff --git a/internal/lidarr/types.go b/internal/lidarr/types.go index 0c4441ba..d1c8ec05 100644 --- a/internal/lidarr/types.go +++ b/internal/lidarr/types.go @@ -4,13 +4,25 @@ package lidarr // LookupResult is the normalized shape returned by Lookup{Artist,Album,Track}. -// It is what we store on the request row (via the user's request) and -// what /api/lidarr/search returns to the SPA. +// MBID is the kind's primary identifier (foreignArtistId / foreignAlbumId / +// foreignTrackId from Lidarr). ArtistMBID and AlbumMBID carry parent +// identifiers when the kind has them — request creation and admin +// approval need them to construct the lidarr_requests row and the +// AddArtist/AddAlbum API calls. type LookupResult struct { - MBID string // foreignArtistId / foreignAlbumId / foreignTrackId - Name string // artist name; album/track returns Title here too - Secondary string // genre + album count for artist; year for album; album for track - ImageURL string // cover-art URL Lidarr surfaced (may be empty) + MBID string // foreignArtistId / foreignAlbumId / foreignTrackId + ArtistMBID string // parent artist's foreignArtistId — set on album/track results, empty on artist results + AlbumMBID string // parent album's foreignAlbumId — set on track results only + Name string // artist name; album/track returns Title here too + Secondary string // genre + album count for artist; year for album; album for track + ImageURL string // cover-art URL Lidarr surfaced (may be empty) +} + +// lidarrImage is the wire shape for image entries in Lidarr's API responses. +type lidarrImage struct { + CoverType string `json:"coverType"` + RemoteURL string `json:"remoteUrl"` + URL string `json:"url"` } // QualityProfile is the dropdown choice in /admin/integrations.