fix: survive an album with two monitored releases

Fetching every album in one unfiltered request avoided Lidarr's unguarded
per-artist path, but not the exception underneath it. Every album endpoint maps
through AlbumResource.ToResource, which selects the release with
SingleOrDefault(x => x.Monitored). An album with two monitored releases makes
that throw -- "Sequence contains more than one element" -- and the bulk call
loses the entire library to one bad row.

Keep the unfiltered call as the first attempt, since it is a single request and
is still the only path that skips albums whose artist metadata is missing. When
it fails, fall back to one request per artist. That cannot dodge the exception
either, but it confines the loss to whichever artist owns the offending album
and names them, which is the only practical way to find it in a large library.

Album failures are counted separately from artist failures because they do not
mean the same thing. Tracks come from a different endpoint with a different
mapper, so an artist whose albums cannot be fetched still gets indexed and still
matches; it is the cull that cannot run. The report distinguishes the two rather
than lumping them into one warning that overstates the damage.

README.md

@@ -44,12 +44,28 @@ deletes music.
 
 ### Indexing quirks
 
-Albums are fetched from the **unfiltered** `GET /api/v1/album`, not one call per
+Albums are fetched from the **unfiltered** `GET /api/v1/album` first: one
-artist. `?artistId=` is Lidarr's unguarded path: it dereferences the album's
+request, and the only path that skips albums whose artist metadata is missing
-artist metadata with no null check and picks the monitored release with
+rather than dereferencing it.
-`SingleOrDefault`, so it returns a 500 for an album with broken metadata or two
+
-monitored releases. The unfiltered endpoint skips such albums instead, and costs
+That is not enough on its own. Every album endpoint maps through a resource
-N fewer requests.
+that picks the release with `SingleOrDefault(x => x.Monitored)`, which throws
+for an album with **two monitored releases** and takes the whole response with
+it:
+
+```
+HTTP 500: Sequence contains more than one element
+```
+
+When the bulk call dies that way, the indexer falls back to one request per
+artist. It cannot avoid the exception, but it confines it to whichever artist
+owns the offending album and names them in the log — which is the only
+practical way to find it in a large library. Open that artist in Lidarr and
+check the Releases tab of each album: exactly one release may be monitored.
+
+Losing an artist's albums does not cost their tracks, which come from a
+different endpoint with a different mapper, so matching is unaffected. A cull
+would not be, and the report says so.
 
 Tracks and files have no unfiltered endpoint — Lidarr rejects a call with no
 filter — so they stay per artist. If one artist cannot be served, that artist is

music_curator.py

@@ -707,6 +707,40 @@ def parse_added(value):
         return None
 
 
+def fetch_albums(client, artists):
+    """Return albums grouped by artist id, plus the artists whose albums failed.
+
+    Every album endpoint maps through a resource that picks the release with
+    `SingleOrDefault(x => x.Monitored)`, which throws for an album with two
+    monitored releases -- "Sequence contains more than one element" -- and takes
+    the entire response down with it.
+
+    The unfiltered endpoint is one request and is the only one that also skips
+    albums whose artist metadata is missing, so it is tried first. When it dies,
+    asking per artist confines the loss to whichever artist owns the offending
+    album, and names them, which is the only way to find it.
+    """
+    try:
+        grouped = {}
+        for album in client.get("album"):
+            grouped.setdefault(album.get("artistId"), []).append(album)
+        return grouped, []
+    except LidarrError as error:
+        logger.warning("fetching all albums failed (%s)", error)
+        logger.warning("falling back to one request per artist to isolate the bad album")
+
+    grouped, failed = {}, []
+    for artist in artists:
+        artist_id = artist["id"]
+        try:
+            grouped[artist_id] = client.get("album", {"artistId": artist_id})
+        except LidarrError as error:
+            name = artist.get("artistName") or str(artist_id)
+            logger.warning("could not fetch albums for %s: %s", name, error)
+            failed.append(name)
+    return grouped, failed
+
+
 def index_library(client, store):
     """Rebuild the library index from Lidarr. Returns (artists, tracks).
 
@@ -718,13 +752,7 @@ def index_library(client, store):
     artist_rows, album_rows, track_rows = [], [], []
     skipped = []
 
-    # Albums come back in one unfiltered call rather than one per artist. That
+    albums_by_artist, album_failures = fetch_albums(client, artists)
-    # endpoint is the only one Lidarr hydrates defensively -- it skips an album
-    # whose artist metadata is missing, where `?artistId=` dereferences it and
-    # returns a 500 -- and it costs N fewer requests into the bargain.
-    albums_by_artist = {}
-    for album in client.get("album"):
-        albums_by_artist.setdefault(album.get("artistId"), []).append(album)
 
     for artist in artists:
         artist_id = artist["id"]
@@ -790,6 +818,7 @@ def index_library(client, store):
 
     store.replace_library(artist_rows, album_rows, track_rows)
     store.set_state("index_skipped", str(len(skipped)))
+    store.set_state("index_albums_skipped", str(len(album_failures)))
     logger.info(
         "library indexed: %d artists, %d albums, %d tracks (%d with files)",
         len(artist_rows),
@@ -803,6 +832,14 @@ def index_library(client, store):
             len(skipped),
             ", ".join(sorted(skipped)[:10]),
         )
+    if album_failures:
+        logger.warning(
+            "no albums indexed for %d artists: %s."
+            " In Lidarr, open each one and check the Releases tab of their albums:"
+            " exactly one release per album may be monitored.",
+            len(album_failures),
+            ", ".join(sorted(album_failures)[:10]),
+        )
     return len(artist_rows), len(track_rows)
 
 
@@ -876,6 +913,13 @@ def coverage_report(store):
             " figure below is a floor. Do not cull against it.",
             skipped,
         )
+    albums_skipped = int(store.get_state("index_albums_skipped") or 0)
+    if albums_skipped:
+        logger.warning(
+            "%d artists have no albums indexed. Matching is unaffected -- it runs off"
+            " tracks -- but a cull cannot be run until this is fixed.",
+            albums_skipped,
+        )
     for method in ("mbid", "name", "none"):
         row = store.connection.execute(
             "SELECT COUNT(*) AS pairs, COALESCE(SUM(plays), 0) AS plays"

tests/test_music_curator.py

@@ -503,8 +503,7 @@ def test_matching_is_redone_when_new_scrobbles_arrive(tmp_path):
 
 
 def test_albums_come_from_the_unfiltered_endpoint(tmp_path):
-    """`?artistId=` is Lidarr's unguarded path and 500s on data it cannot
+    """One request, and the only path that skips albums it cannot hydrate."""
-    hydrate; the unfiltered one skips such albums instead."""
     api = FakeLidarr(LIBRARY)
     store = store_at(tmp_path)
 
@@ -515,6 +514,33 @@ def test_albums_come_from_the_unfiltered_endpoint(tmp_path):
     assert store.scalar("SELECT COUNT(*) FROM lidarr_album") == 3
 
 
+def test_a_bad_album_falls_back_to_asking_per_artist(tmp_path):
+    """An album with two monitored releases throws in the resource mapper, so
+    the bulk call dies wholesale. Per artist, only its owner is lost."""
+    # The bulk call fails because artist 2 owns the offending album.
+    api = FakeLidarr(LIBRARY, fail=[("album", 0), ("album", 2)])
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_album WHERE artist_id = 2") == 0
+    # The other two artists keep their albums.
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_album") == 2
+    assert store.get_state("index_albums_skipped") == "1"
+
+
+def test_a_bad_album_does_not_cost_that_artist_their_tracks(tmp_path):
+    """Tracks come from a different endpoint with a different mapper, so
+    matching survives an album Lidarr cannot serialise."""
+    api = FakeLidarr(LIBRARY, fail=[("album", 0), ("album", 2)])
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE artist_id = 2") == 1
+    assert store.get_state("index_skipped") == "0"
+
+
 def test_an_artist_lidarr_cannot_serve_does_not_kill_the_index(tmp_path):
     # Artist 2 is AC/DC in LIBRARY; its track lookup fails.
     api = FakeLidarr(LIBRARY, fail=[("track", 2)])