refactor(scan): extract scan_chunk from scan_library
scan_chunk resolves album folders whose (artist, album) sorts after a cursor, up to a limit, returning (new_cursor, imported, skipped, done). scan_library becomes the unbounded case (limit=None), so its behavior and tests are unchanged. Idempotent: all writes are ON CONFLICT, so an overlapping/repeated cursor is safe. Enables chunked scanning in the loop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+73
-27
@@ -91,16 +91,11 @@ def _record_owned(conn, target, quality_class: int, fmt: str, path: str, watched
|
|||||||
return newly
|
return newly
|
||||||
|
|
||||||
|
|
||||||
def scan_library(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, dest_root: str = "/music") -> ScanResult:
|
def _iter_album_entries(dest_root: str):
|
||||||
"""Walk `{dest_root}/{Artist}/{Album}/`, resolve each album against MusicBrainz, and record
|
"""Yield (artist_name, album_folder, album_path) for each candidate album directory,
|
||||||
matched albums as have + monitored + followed. Also browses each followed artist's full
|
in the stable nested-sorted order the scan cursor relies on."""
|
||||||
discography (once per scan run) and stores it as unmonitored releases, matching web-follow
|
|
||||||
behavior. Unmatched or audio-less folders are skipped."""
|
|
||||||
imported = 0
|
|
||||||
skipped = 0
|
|
||||||
browsed: set[str] = set() # artists whose discography we've populated this run
|
|
||||||
if not os.path.isdir(dest_root):
|
if not os.path.isdir(dest_root):
|
||||||
return ScanResult(0, 0)
|
return
|
||||||
for artist_name in sorted(os.listdir(dest_root)):
|
for artist_name in sorted(os.listdir(dest_root)):
|
||||||
artist_path = os.path.join(dest_root, artist_name)
|
artist_path = os.path.join(dest_root, artist_name)
|
||||||
if artist_name.startswith(".") or not os.path.isdir(artist_path):
|
if artist_name.startswith(".") or not os.path.isdir(artist_path):
|
||||||
@@ -109,22 +104,73 @@ def scan_library(conn: psycopg.Connection, resolver, probe: AudioProbe, browser,
|
|||||||
album_path = os.path.join(artist_path, album_folder)
|
album_path = os.path.join(artist_path, album_folder)
|
||||||
if album_folder.startswith(".") or not os.path.isdir(album_path):
|
if album_folder.startswith(".") or not os.path.isdir(album_path):
|
||||||
continue
|
continue
|
||||||
audio = _first_audio(album_path)
|
yield artist_name, album_folder, album_path
|
||||||
if audio is None:
|
|
||||||
continue # no audio → not an album
|
|
||||||
pr = probe.probe(audio)
|
def _cursor_key(artist_name: str, album_folder: str) -> str:
|
||||||
album_name = _parse_album_dir(album_folder)
|
return f"{artist_name}/{album_folder}"
|
||||||
target = resolver.resolve(artist_name, album_name) if artist_name and album_name else None
|
|
||||||
if target is None: # fall back to the file's embedded tags
|
|
||||||
if pr.artist and pr.album and (pr.artist, pr.album) != (artist_name, album_name):
|
def _cursor_tuple(cursor: str) -> tuple[str, str]:
|
||||||
target = resolver.resolve(pr.artist, pr.album)
|
# A cursor is "<artist>/<album>"; artist and album are single path segments (no "/"),
|
||||||
if target is None:
|
# so partitioning on the first "/" recovers them, and tuple order matches the walk.
|
||||||
skipped += 1
|
artist, _, album = (cursor or "").partition("/")
|
||||||
continue
|
return artist, album
|
||||||
watched_id = _ensure_artist(conn, target)
|
|
||||||
if watched_id is not None and target.artist_mbid not in browsed:
|
|
||||||
_populate_discography(conn, browser, watched_id, target.artist_mbid, target.artist)
|
def _process_album(conn, resolver, probe, browser, artist_name, album_folder, album_path, browsed) -> str:
|
||||||
browsed.add(target.artist_mbid)
|
"""Identify and record one album folder. Returns 'imported' (newly recorded),
|
||||||
if _record_owned(conn, target, pr.quality_class, pr.fmt, album_path, watched_id):
|
'recorded' (matched but already present), 'skipped' (no MB match) or 'none' (no audio)."""
|
||||||
imported += 1
|
audio = _first_audio(album_path)
|
||||||
|
if audio is None:
|
||||||
|
return "none" # no audio → not an album
|
||||||
|
pr = probe.probe(audio)
|
||||||
|
album_name = _parse_album_dir(album_folder)
|
||||||
|
target = resolver.resolve(artist_name, album_name) if artist_name and album_name else None
|
||||||
|
if target is None: # fall back to the file's embedded tags
|
||||||
|
if pr.artist and pr.album and (pr.artist, pr.album) != (artist_name, album_name):
|
||||||
|
target = resolver.resolve(pr.artist, pr.album)
|
||||||
|
if target is None:
|
||||||
|
return "skipped"
|
||||||
|
watched_id = _ensure_artist(conn, target)
|
||||||
|
if watched_id is not None and target.artist_mbid not in browsed:
|
||||||
|
_populate_discography(conn, browser, watched_id, target.artist_mbid, target.artist)
|
||||||
|
browsed.add(target.artist_mbid)
|
||||||
|
return "imported" if _record_owned(conn, target, pr.quality_class, pr.fmt, album_path, watched_id) else "recorded"
|
||||||
|
|
||||||
|
|
||||||
|
def scan_chunk(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, dest_root: str = "/music",
|
||||||
|
cursor: str = "", limit: int | None = None) -> tuple[str, int, int, bool]:
|
||||||
|
"""Resolve and record album folders whose (artist, album) sorts after `cursor`, up to
|
||||||
|
`limit` of them (unbounded when limit is None). Returns (new_cursor, imported, skipped,
|
||||||
|
done); `done` is True when the walk was exhausted within this chunk. All writes are
|
||||||
|
ON CONFLICT, so an overlapping or repeated cursor is idempotent."""
|
||||||
|
after = _cursor_tuple(cursor)
|
||||||
|
imported = skipped = processed = 0
|
||||||
|
browsed: set[str] = set() # artists whose discography we've populated this chunk
|
||||||
|
new_cursor = cursor
|
||||||
|
done = True
|
||||||
|
for artist_name, album_folder, album_path in _iter_album_entries(dest_root):
|
||||||
|
if (artist_name, album_folder) <= after:
|
||||||
|
continue # already processed by an earlier chunk
|
||||||
|
if limit is not None and processed >= limit:
|
||||||
|
done = False # stopped early; more albums remain after new_cursor
|
||||||
|
break
|
||||||
|
outcome = _process_album(conn, resolver, probe, browser, artist_name, album_folder, album_path, browsed)
|
||||||
|
if outcome == "imported":
|
||||||
|
imported += 1
|
||||||
|
elif outcome == "skipped":
|
||||||
|
skipped += 1
|
||||||
|
new_cursor = _cursor_key(artist_name, album_folder)
|
||||||
|
processed += 1
|
||||||
|
return new_cursor, imported, skipped, done
|
||||||
|
|
||||||
|
|
||||||
|
def scan_library(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, dest_root: str = "/music") -> ScanResult:
|
||||||
|
"""Walk `{dest_root}/{Artist}/{Album}/` in one unbounded pass, resolving each album
|
||||||
|
against MusicBrainz and recording matched albums as have + monitored + followed (plus
|
||||||
|
each followed artist's full discography as unmonitored releases). Unmatched or audio-less
|
||||||
|
folders are skipped. This is `scan_chunk` with no limit; the worker loop uses the chunked
|
||||||
|
form so a large library doesn't block job-claim."""
|
||||||
|
_cursor, imported, skipped, _done = scan_chunk(conn, resolver, probe, browser, dest_root, cursor="", limit=None)
|
||||||
return ScanResult(imported, skipped)
|
return ScanResult(imported, skipped)
|
||||||
|
|||||||
@@ -0,0 +1,81 @@
|
|||||||
|
from lyra_worker.adapters.fakes import FakeMbBrowser
|
||||||
|
from lyra_worker.browser import ReleaseGroupInfo
|
||||||
|
from lyra_worker.scan import scan_chunk
|
||||||
|
from lyra_worker.types import MBTarget
|
||||||
|
from tests.test_scan import FakeProbe, FakeResolver, _album, _counts
|
||||||
|
|
||||||
|
|
||||||
|
def _tree(tmp_path):
|
||||||
|
"""Three albums across two artists, in nested-sorted order:
|
||||||
|
Artist A/Alpha, Artist A/Beta, Artist B/Gamma."""
|
||||||
|
_album(tmp_path, "Artist A", "Alpha (2001)")
|
||||||
|
_album(tmp_path, "Artist A", "Beta (2002)")
|
||||||
|
_album(tmp_path, "Artist B", "Gamma (2003)")
|
||||||
|
resolver = FakeResolver({
|
||||||
|
("Artist A", "Alpha"): MBTarget(artist="Artist A", album="Alpha", year=2001, rg_mbid="rg-a1", artist_mbid="ma"),
|
||||||
|
("Artist A", "Beta"): MBTarget(artist="Artist A", album="Beta", year=2002, rg_mbid="rg-a2", artist_mbid="ma"),
|
||||||
|
("Artist B", "Gamma"): MBTarget(artist="Artist B", album="Gamma", year=2003, rg_mbid="rg-b1", artist_mbid="mb"),
|
||||||
|
})
|
||||||
|
browser = FakeMbBrowser(releases={
|
||||||
|
"ma": [ReleaseGroupInfo("rg-a1", "Alpha", "Album", (), "2001"),
|
||||||
|
ReleaseGroupInfo("rg-a2", "Beta", "Album", (), "2002")],
|
||||||
|
"mb": [ReleaseGroupInfo("rg-b1", "Gamma", "Album", (), "2003")],
|
||||||
|
})
|
||||||
|
return resolver, browser
|
||||||
|
|
||||||
|
|
||||||
|
def test_scan_chunk_respects_limit_and_returns_cursor(conn, tmp_path):
|
||||||
|
resolver, browser = _tree(tmp_path)
|
||||||
|
cursor, imported, skipped, done = scan_chunk(
|
||||||
|
conn, resolver, FakeProbe(quality_class=2), browser, str(tmp_path), cursor="", limit=2)
|
||||||
|
assert (imported, skipped, done) == (2, 0, False) # processed 2 of 3, more remain
|
||||||
|
assert cursor == "Artist A/Beta (2002)" # last processed key
|
||||||
|
with conn.cursor() as cur:
|
||||||
|
cur.execute('SELECT count(*) FROM "LibraryItem"')
|
||||||
|
assert cur.fetchone()[0] == 2
|
||||||
|
|
||||||
|
|
||||||
|
def test_scan_chunk_resumes_from_cursor(conn, tmp_path):
|
||||||
|
resolver, browser = _tree(tmp_path)
|
||||||
|
cursor, imported, skipped, done = scan_chunk(
|
||||||
|
conn, resolver, FakeProbe(quality_class=2), browser, str(tmp_path),
|
||||||
|
cursor="Artist A/Beta (2002)", limit=25)
|
||||||
|
assert (imported, skipped, done) == (1, 0, True) # only Gamma remains
|
||||||
|
assert cursor == "Artist B/Gamma (2003)"
|
||||||
|
with conn.cursor() as cur:
|
||||||
|
cur.execute('SELECT album FROM "LibraryItem"')
|
||||||
|
assert cur.fetchall() == [("Gamma",)] # only Gamma imported
|
||||||
|
|
||||||
|
|
||||||
|
def test_scan_chunk_cursor_past_end_is_noop(conn, tmp_path):
|
||||||
|
resolver, browser = _tree(tmp_path)
|
||||||
|
cursor, imported, skipped, done = scan_chunk(
|
||||||
|
conn, resolver, FakeProbe(), browser, str(tmp_path), cursor="zzzz/zzzz", limit=10)
|
||||||
|
assert (imported, skipped, done) == (0, 0, True)
|
||||||
|
assert cursor == "zzzz/zzzz" # unchanged
|
||||||
|
assert _counts(conn)["LibraryItem"] == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_scan_chunk_splits_artist_across_chunks_idempotently(conn, tmp_path):
|
||||||
|
# limit=1 forces Artist A's two albums into separate chunks, re-browsing its
|
||||||
|
# discography — which must stay idempotent (ON CONFLICT) and total the same as one scan.
|
||||||
|
resolver, browser = _tree(tmp_path)
|
||||||
|
cursor, done, total_i, total_s, guard = "", False, 0, 0, 0
|
||||||
|
while not done and guard < 10:
|
||||||
|
cursor, i, s, done = scan_chunk(
|
||||||
|
conn, resolver, FakeProbe(quality_class=2), browser, str(tmp_path), cursor=cursor, limit=1)
|
||||||
|
total_i += i
|
||||||
|
total_s += s
|
||||||
|
guard += 1
|
||||||
|
assert (total_i, total_s) == (3, 0)
|
||||||
|
assert guard == 3 # three albums, one per chunk, then done
|
||||||
|
assert _counts(conn) == {"LibraryItem": 3, "MonitoredRelease": 3, "WatchedArtist": 2}
|
||||||
|
|
||||||
|
|
||||||
|
def test_scan_chunk_unbounded_matches_full_walk(conn, tmp_path):
|
||||||
|
resolver, browser = _tree(tmp_path)
|
||||||
|
cursor, imported, skipped, done = scan_chunk(
|
||||||
|
conn, resolver, FakeProbe(quality_class=2), browser, str(tmp_path), cursor="", limit=None)
|
||||||
|
assert (imported, skipped, done) == (3, 0, True)
|
||||||
|
assert cursor == "Artist B/Gamma (2003)"
|
||||||
|
assert _counts(conn) == {"LibraryItem": 3, "MonitoredRelease": 3, "WatchedArtist": 2}
|
||||||
Reference in New Issue
Block a user