From 670b30c95483cd848c51e52f89b374f75ae07338 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 7 Oct 2026 10:55:41 -0400 Subject: [PATCH 1/2] fix(lidarr): add an album as the looked-up resource with its artist nested (#5234) Lidarr's POST /api/v1/album validates `artist` as a nested resource (AlbumController: RuleFor(s => s.Artist).NotNull()), so the flat payload we sent was refused with "'Artist' must not be empty" every time. The album add has never worked against a real Lidarr; approved album and track requests sat in the reconciler retrying every 5 minutes. AddAlbum now does what Lidarr's own add-album UI does (getNewAlbum / getNewArtist): look the album up by MBID (album/lookup?term=lidarr:), then POST that resource back with monitored + searchForNewAlbum. When Lidarr doesn't have the artist yet, the nested artist gets the request's quality/metadata profile and root folder, monitors this album only (monitor "none" + albumsToMonitor, which AlbumMonitoredService prefers) and no future releases. An artist Lidarr already has is left as it is. An MBID Lidarr's metadata doesn't know is ErrNotFound with no POST. Co-Authored-By: Claude Opus 5.5 --- internal/lidarr/client.go | 57 +++++++++--- internal/lidarr/client_test.go | 158 ++++++++++++++++++++++++++------- internal/lidarr/lookup_mbid.go | 27 ++++++ internal/lidarr/types.go | 8 +- 4 files changed, 204 insertions(+), 46 deletions(-) diff --git a/internal/lidarr/client.go b/internal/lidarr/client.go index dcbbe0c0..3c6b7d4b 100644 --- a/internal/lidarr/client.go +++ b/internal/lidarr/client.go @@ -324,19 +324,52 @@ func isAlreadyExists(err error) bool { strings.Contains(m, "already in your library") } -// AddAlbum posts to POST /api/v1/album. Returns nil on 2xx; typed error -// otherwise. +// AddAlbum adds one album to Lidarr the way Lidarr's own UI does: look the +// album up by MBID, then POST that album resource back with `monitored`, +// `addOptions` and — when Lidarr doesn't have the artist yet — the +// artist's profiles, root folder and monitoring filled in. +// +// POST /api/v1/album validates `artist` as a nested resource (#5234: a +// flat payload is refused with "'Artist' must not be empty"), and the +// lookup is what supplies it: the existing Lidarr artist when there is one, +// otherwise a new one carrying its metadata. A new artist monitors only +// this album (monitor "none" with albumsToMonitor, which Lidarr applies in +// preference to the monitor type) and no future releases, so approving one +// album never subscribes the library to the artist's whole catalogue. +// +// Returns nil on 2xx, ErrAlreadyExists when Lidarr already has the album, +// ErrNotFound when Lidarr's metadata has no album under that MBID. func (c *Client) AddAlbum(ctx context.Context, p AddAlbumParams) error { - body, err := json.Marshal(map[string]any{ - "foreignAlbumId": p.ForeignAlbumID, - "foreignArtistId": p.ForeignArtistID, - "artistName": p.ArtistName, - "qualityProfileId": p.QualityProfileID, - "metadataProfileId": p.MetadataProfileID, - "rootFolderPath": p.RootFolderPath, - "monitored": true, - "addOptions": map[string]any{"searchForNewAlbum": true}, - }) + album, err := c.lookupAlbumResource(ctx, p.ForeignAlbumID) + if err != nil { + return err + } + album["monitored"] = true + album["addOptions"] = map[string]any{"searchForNewAlbum": true} + + artist, _ := album["artist"].(map[string]any) + if artist == nil { + artist = map[string]any{ + "foreignArtistId": p.ForeignArtistID, + "artistName": p.ArtistName, + } + album["artist"] = artist + } + if id, _ := artist["id"].(float64); id == 0 { + artist["qualityProfileId"] = p.QualityProfileID + artist["metadataProfileId"] = p.MetadataProfileID + artist["rootFolderPath"] = p.RootFolderPath + artist["monitored"] = true + artist["monitorNewItems"] = "none" + artist["addOptions"] = map[string]any{ + "monitor": "none", + "albumsToMonitor": []string{p.ForeignAlbumID}, + "monitored": true, + "searchForMissingAlbums": false, + } + } + + body, err := json.Marshal(album) if err != nil { return fmt.Errorf("%w: marshal: %v", ErrInvalidPayload, err) } diff --git a/internal/lidarr/client_test.go b/internal/lidarr/client_test.go index 6ece5c5f..c797af00 100644 --- a/internal/lidarr/client_test.go +++ b/internal/lidarr/client_test.go @@ -327,45 +327,143 @@ func TestAddArtist_ServerError(t *testing.T) { // --- AddAlbum --- -func TestAddAlbum_PostsCorrectBody(t *testing.T) { - var decoded map[string]any +const ( + addAlbumMBID = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" + addArtistMBID = "069b64b6-7884-4f6a-94cc-e4c1d6c87a01" +) + +// albumLookupStub answers Lidarr's album lookup with lookupBody and records +// the body of the album POST that follows it. +func albumLookupStub(t *testing.T, lookupBody string, postStatus int, postBody string) (*Client, *httptest.Server, *map[string]any, *int) { + t.Helper() + var posted map[string]any + posts := 0 c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost { - t.Errorf("method = %q, want POST", r.Method) + switch { + case r.Method == http.MethodGet && r.URL.Path == "/api/v1/album/lookup": + if got := r.URL.Query().Get("term"); got != "lidarr:"+addAlbumMBID { + t.Errorf("lookup term = %q, want lidarr:%s", got, addAlbumMBID) + } + _, _ = w.Write([]byte(lookupBody)) + case r.Method == http.MethodPost && r.URL.Path == "/api/v1/album": + posts++ + _ = json.NewDecoder(r.Body).Decode(&posted) + w.WriteHeader(postStatus) + _, _ = w.Write([]byte(postBody)) + default: + t.Errorf("unexpected %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) } - if r.URL.Path != "/api/v1/album" { - t.Errorf("path = %q, want /api/v1/album", r.URL.Path) - } - _ = json.NewDecoder(r.Body).Decode(&decoded) - w.WriteHeader(http.StatusCreated) }) + return c, srv, &posted, &posts +} + +var addAlbumParams = AddAlbumParams{ + ForeignAlbumID: addAlbumMBID, + ForeignArtistID: addArtistMBID, + ArtistName: "Boards of Canada", + QualityProfileID: 1, + MetadataProfileID: 3, + RootFolderPath: "/music", +} + +// #5234: Lidarr refuses a flat album add with "'Artist' must not be empty". +// The add has to be the looked-up album resource with its artist nested. +func TestAddAlbum_NewArtist_PostsTheLookedUpAlbumWithTheArtistNested(t *testing.T) { + lookup := `[ + {"foreignAlbumId": "someone-else", "title": "Other"}, + {"foreignAlbumId": "` + addAlbumMBID + `", "title": "Music Has the Right to Children", + "releases": [{"foreignReleaseId": "r1", "monitored": true}], + "artist": {"foreignArtistId": "` + addArtistMBID + `", "artistName": "Boards of Canada"}} + ]` + c, srv, posted, _ := albumLookupStub(t, lookup, http.StatusCreated, `{"id":9}`) defer srv.Close() - err := c.AddAlbum(context.Background(), AddAlbumParams{ - ForeignAlbumID: "a1b2c3d4-e5f6-7890-abcd-ef1234567890", - ForeignArtistID: "069b64b6-7884-4f6a-94cc-e4c1d6c87a01", - QualityProfileID: 1, - RootFolderPath: "/music", - }) - if err != nil { + if err := c.AddAlbum(context.Background(), addAlbumParams); err != nil { t.Fatalf("AddAlbum: %v", err) } + body := *posted + if body["foreignAlbumId"] != addAlbumMBID || body["title"] != "Music Has the Right to Children" { + t.Errorf("posted album = %v / %v, want the looked-up album", body["foreignAlbumId"], body["title"]) + } + if rel, _ := body["releases"].([]any); len(rel) != 1 { + t.Errorf("releases = %v, want the lookup's releases passed through", body["releases"]) + } + if body["monitored"] != true { + t.Errorf("monitored = %v, want true", body["monitored"]) + } + if opts, _ := body["addOptions"].(map[string]any); opts["searchForNewAlbum"] != true { + t.Errorf("addOptions = %v, want searchForNewAlbum true", body["addOptions"]) + } - if decoded["foreignAlbumId"] != "a1b2c3d4-e5f6-7890-abcd-ef1234567890" { - t.Errorf("foreignAlbumId = %v", decoded["foreignAlbumId"]) - } - if decoded["foreignArtistId"] != "069b64b6-7884-4f6a-94cc-e4c1d6c87a01" { - t.Errorf("foreignArtistId = %v", decoded["foreignArtistId"]) - } - if decoded["monitored"] != true { - t.Errorf("monitored = %v, want true", decoded["monitored"]) - } - opts, ok := decoded["addOptions"].(map[string]any) + artist, ok := body["artist"].(map[string]any) if !ok { - t.Fatalf("addOptions missing or wrong type: %T", decoded["addOptions"]) + t.Fatalf("artist = %T, want a nested object", body["artist"]) } - if opts["searchForNewAlbum"] != true { - t.Errorf("addOptions.searchForNewAlbum = %v, want true", opts["searchForNewAlbum"]) + for k, want := range map[string]any{ + "foreignArtistId": addArtistMBID, + "qualityProfileId": float64(1), + "metadataProfileId": float64(3), + "rootFolderPath": "/music", + "monitored": true, + "monitorNewItems": "none", + } { + if artist[k] != want { + t.Errorf("artist.%s = %v, want %v", k, artist[k], want) + } + } + // A new artist monitors this album only: albumsToMonitor wins over the + // monitor type in Lidarr's AlbumMonitoredService. + aOpts, _ := artist["addOptions"].(map[string]any) + if aOpts["monitor"] != "none" || aOpts["searchForMissingAlbums"] != false { + t.Errorf("artist.addOptions = %v, want monitor none, no catalogue search", aOpts) + } + if only, _ := aOpts["albumsToMonitor"].([]any); len(only) != 1 || only[0] != addAlbumMBID { + t.Errorf("artist.addOptions.albumsToMonitor = %v, want [%s]", aOpts["albumsToMonitor"], addAlbumMBID) + } +} + +// An artist Lidarr already has keeps its own profiles and monitoring. +func TestAddAlbum_ExistingArtist_IsLeftAsLidarrHasIt(t *testing.T) { + lookup := `[{"foreignAlbumId": "` + addAlbumMBID + `", + "artist": {"id": 42, "foreignArtistId": "` + addArtistMBID + `", "qualityProfileId": 5, + "metadataProfileId": 2, "rootFolderPath": "/lib", "monitorNewItems": "all"}}]` + c, srv, posted, _ := albumLookupStub(t, lookup, http.StatusCreated, `{"id":9}`) + defer srv.Close() + + if err := c.AddAlbum(context.Background(), addAlbumParams); err != nil { + t.Fatalf("AddAlbum: %v", err) + } + artist, _ := (*posted)["artist"].(map[string]any) + if artist["qualityProfileId"] != float64(5) || artist["rootFolderPath"] != "/lib" || artist["monitorNewItems"] != "all" { + t.Errorf("existing artist was rewritten: %v", artist) + } + if _, set := artist["addOptions"]; set { + t.Errorf("existing artist got addOptions %v", artist["addOptions"]) + } +} + +func TestAddAlbum_NotInLidarrsMetadata_IsNotFoundAndPostsNothing(t *testing.T) { + c, srv, _, posts := albumLookupStub(t, `[{"foreignAlbumId": "someone-else"}]`, http.StatusCreated, "") + defer srv.Close() + + err := c.AddAlbum(context.Background(), addAlbumParams) + if !errors.Is(err, ErrNotFound) { + t.Fatalf("err = %v, want ErrNotFound", err) + } + if *posts != 0 { + t.Errorf("posted %d times, want 0", *posts) + } +} + +func TestAddAlbum_AlreadyAdded_IsErrAlreadyExists(t *testing.T) { + lookup := `[{"foreignAlbumId": "` + addAlbumMBID + `", "artist": {"id": 42}}]` + c, srv, _, _ := albumLookupStub(t, lookup, http.StatusBadRequest, + `[{"propertyName":"ForeignAlbumId","errorMessage":"This album has already been added."}]`) + defer srv.Close() + + if err := c.AddAlbum(context.Background(), addAlbumParams); !errors.Is(err, ErrAlreadyExists) { + t.Fatalf("err = %v, want ErrAlreadyExists", err) } } @@ -374,7 +472,7 @@ func TestAddAlbum_AuthFailed(t *testing.T) { w.WriteHeader(http.StatusUnauthorized) }) defer srv.Close() - err := c.AddAlbum(context.Background(), AddAlbumParams{}) + err := c.AddAlbum(context.Background(), addAlbumParams) if !errors.Is(err, ErrAuthFailed) { t.Fatalf("err = %v, want ErrAuthFailed", err) } diff --git a/internal/lidarr/lookup_mbid.go b/internal/lidarr/lookup_mbid.go index b6e17781..f419597c 100644 --- a/internal/lidarr/lookup_mbid.go +++ b/internal/lidarr/lookup_mbid.go @@ -52,3 +52,30 @@ func (c *Client) LookupAlbumByMBID(ctx context.Context, mbid string) (LidarrAlbu } return rows[0], nil } + +// lookupAlbumResource fetches the album resource Lidarr's metadata holds +// under mbid (GET /api/v1/album/lookup?term=lidarr:), kept as raw +// JSON so AddAlbum can POST it back with every field Lidarr sent — the +// round trip Lidarr's own add-album UI makes. Returns ErrNotFound when no +// result carries that MBID. +func (c *Client) lookupAlbumResource(ctx context.Context, mbid string) (map[string]any, error) { + if mbid == "" { + return nil, fmt.Errorf("lidarr: empty album mbid") + } + resp, err := c.get(ctx, "/api/v1/album/lookup", url.Values{"term": {"lidarr:" + mbid}}) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + + var rows []map[string]any + if err := json.NewDecoder(resp.Body).Decode(&rows); err != nil { + return nil, fmt.Errorf("%w: decode album lookup: %v", ErrInvalidPayload, err) + } + for _, row := range rows { + if id, _ := row["foreignAlbumId"].(string); id == mbid { + return row, nil + } + } + return nil, fmt.Errorf("%w: album %s in Lidarr's metadata", ErrNotFound, mbid) +} diff --git a/internal/lidarr/types.go b/internal/lidarr/types.go index a3775cc5..7a5858f7 100644 --- a/internal/lidarr/types.go +++ b/internal/lidarr/types.go @@ -60,10 +60,10 @@ type AddArtistParams struct { MonitorAll bool // true => monitor="all"; false => "future" } -// AddAlbumParams are the fields Lidarr requires on POST /api/v1/album. -// MetadataProfileID is included for the case where Lidarr has to create -// the parent artist on the fly (when adding an album by an artist not -// yet in the library). +// AddAlbumParams identify the album to add and carry the settings Lidarr +// needs when it has to create the parent artist on the fly (an album by an +// artist not yet in Lidarr). The artist fields are a fallback only: the +// album lookup normally supplies the artist itself. type AddAlbumParams struct { ForeignAlbumID string ForeignArtistID string // Lidarr requires the artist's foreign id too -- 2.54.0 From bf6364b709d5e1518a468790baf9a558700398bc Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 7 Oct 2026 10:58:55 -0400 Subject: [PATCH 2/2] fix(lidarr): send the artist add's monitor choice in addOptions (#5239) ArtistResource has no top-level `monitor`. Lidarr reads the choice from AddOptions (AddArtistOptions, a MonitoringOptions), so the "all"/"future" we sent there was dropped on deserialisation. AddOptions.Monitor stayed Unknown, and AlbumMonitoredService.SetAlbumMonitoredStatus returns early on Unknown. The request's monitoring was never applied. Send monitor and monitored inside addOptions with searchForMissingAlbums, the shape Lidarr's getNewArtist.js posts, and set monitorNewItems "all" explicitly: both choices mean new releases are watched. Co-Authored-By: Claude Opus 5.5 --- internal/lidarr/client.go | 19 +++++++++++++++---- internal/lidarr/client_test.go | 19 +++++++++++++++---- internal/lidarr/types.go | 2 +- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/internal/lidarr/client.go b/internal/lidarr/client.go index 3c6b7d4b..9d90cc51 100644 --- a/internal/lidarr/client.go +++ b/internal/lidarr/client.go @@ -274,13 +274,20 @@ func (c *Client) LookupTrack(ctx context.Context, term string) ([]LookupResult, return out, nil } -// AddArtist posts to POST /api/v1/artist. MonitorAll=true sends monitor="all"; -// false sends "future". Returns nil on 2xx; typed error otherwise. +// AddArtist posts to POST /api/v1/artist. MonitorAll=true monitors every +// album ("all"); false monitors only releases from now on ("future"). New +// releases are monitored either way. Returns nil on 2xx; typed error +// otherwise. // // All four of artistName, foreignArtistId, qualityProfileId, and // metadataProfileId are required by Lidarr. Omitting any one produces a // 400 with field-level validation messages (e.g. "'Metadata Profile Id' // must be greater than '0'"). +// +// The monitor choice belongs in addOptions (AddArtistOptions, a +// MonitoringOptions). ArtistResource has no top-level `monitor`, so a +// choice sent there is dropped, AddOptions.Monitor stays Unknown and +// Lidarr skips applying it (#5239). func (c *Client) AddArtist(ctx context.Context, p AddArtistParams) error { monitor := "future" if p.MonitorAll { @@ -293,8 +300,12 @@ func (c *Client) AddArtist(ctx context.Context, p AddArtistParams) error { "metadataProfileId": p.MetadataProfileID, "rootFolderPath": p.RootFolderPath, "monitored": true, - "monitor": monitor, - "addOptions": map[string]any{"searchForMissingAlbums": true}, + "monitorNewItems": "all", + "addOptions": map[string]any{ + "monitor": monitor, + "monitored": true, + "searchForMissingAlbums": true, + }, }) if err != nil { return fmt.Errorf("%w: marshal: %v", ErrInvalidPayload, err) diff --git a/internal/lidarr/client_test.go b/internal/lidarr/client_test.go index c797af00..43e7b0af 100644 --- a/internal/lidarr/client_test.go +++ b/internal/lidarr/client_test.go @@ -288,13 +288,24 @@ func TestAddArtist_PostsCorrectBody(t *testing.T) { if decoded["monitored"] != true { t.Errorf("monitored = %v, want true", decoded["monitored"]) } - if decoded["monitor"] != "all" { - t.Errorf("monitor = %v, want all (MonitorAll=true)", decoded["monitor"]) + if decoded["monitorNewItems"] != "all" { + t.Errorf("monitorNewItems = %v, want all", decoded["monitorNewItems"]) + } + // #5239: ArtistResource has no top-level monitor; Lidarr reads the + // choice from addOptions and silently drops it anywhere else. + if _, set := decoded["monitor"]; set { + t.Errorf("monitor sent at top level (%v), where Lidarr ignores it", decoded["monitor"]) } opts, ok := decoded["addOptions"].(map[string]any) if !ok { t.Fatalf("addOptions missing or wrong type: %T", decoded["addOptions"]) } + if opts["monitor"] != "all" { + t.Errorf("addOptions.monitor = %v, want all (MonitorAll=true)", opts["monitor"]) + } + if opts["monitored"] != true { + t.Errorf("addOptions.monitored = %v, want true", opts["monitored"]) + } if opts["searchForMissingAlbums"] != true { t.Errorf("addOptions.searchForMissingAlbums = %v, want true", opts["searchForMissingAlbums"]) } @@ -309,8 +320,8 @@ func TestAddArtist_MonitorFuture(t *testing.T) { defer srv.Close() _ = c.AddArtist(context.Background(), AddArtistParams{MonitorAll: false}) - if decoded["monitor"] != "future" { - t.Errorf("monitor = %v, want future (MonitorAll=false)", decoded["monitor"]) + if opts, _ := decoded["addOptions"].(map[string]any); opts["monitor"] != "future" { + t.Errorf("addOptions.monitor = %v, want future (MonitorAll=false)", opts["monitor"]) } } diff --git a/internal/lidarr/types.go b/internal/lidarr/types.go index 7a5858f7..3fd9db36 100644 --- a/internal/lidarr/types.go +++ b/internal/lidarr/types.go @@ -57,7 +57,7 @@ type AddArtistParams struct { QualityProfileID int MetadataProfileID int RootFolderPath string - MonitorAll bool // true => monitor="all"; false => "future" + MonitorAll bool // true => addOptions.monitor="all"; false => "future" } // AddAlbumParams identify the album to add and carry the settings Lidarr -- 2.54.0