Merge pull request 'Lidarr: album adds that Lidarr accepts, artist monitoring that sticks' (#138) from dev into main

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 <noreply@anthropic.com>

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)
@@ -324,19 +335,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)
 	}

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"])
 	}
 }
 
@@ -327,45 +338,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 +483,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)
 	}

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:<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)
+}

internal/lidarr/types.go

@@ -57,13 +57,13 @@ 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 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