fix(lidarr): add an album as the looked-up resource with its artist nested (#5234)
release / web (push) Successful in 1m42s
release / go (push) Successful in 2m7s
release / govulncheck (push) Successful in 37s
release / integration (push) Successful in 5m31s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / android (push) Canceled after 5m14s
release / Build signed APK (releases and dev) (push) Canceled after 4m42s
release / web (push) Successful in 1m42s
release / go (push) Successful in 2m7s
release / govulncheck (push) Successful in 37s
release / integration (push) Successful in 5m31s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / android (push) Canceled after 5m14s
release / Build signed APK (releases and dev) (push) Canceled after 4m42s
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:<mbid>), 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 <noreply@anthropic.com>
This commit is contained in:
+45
-12
@@ -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)
|
||||
}
|
||||
|
||||
+128
-30
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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:<mbid>), 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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user