From a2f421c272df8151b8f9f633ddb5e5372a0b886b Mon Sep 17 00:00:00 2001 From: twisla Date: Sat, 26 Sep 2026 00:23:54 +0000 Subject: [PATCH] Play albums by their Music Assistant URI Names can match near-duplicate library albums that differ only in capitals (Let the/The Tribe Increase), and Music Assistant picked the unplayable one. Assignments now store the album's library URI, scans return it after checking it's still in the library (re-finding it by name, exact spelling first, if not), and HA plays by it. sync-library (formerly fetch-covers) links existing assignments. Co-Authored-By: Claude Opus 5.5 --- README.md | 20 +++--- src/tag_albums/app.py | 36 +++++++++-- src/tag_albums/cli.py | 69 ++++++++++++++------- src/tag_albums/library.py | 36 +++++++++-- src/tag_albums/store.py | 25 +++++--- src/tag_albums/templates/_results.html | 1 + tests/conftest.py | 18 ++++-- tests/test_app.py | 85 +++++++++++++++++++------- 8 files changed, 217 insertions(+), 73 deletions(-) diff --git a/README.md b/README.md index 8e6ebd9..c715afe 100644 --- a/README.md +++ b/README.md @@ -21,11 +21,13 @@ Both endpoints need `Authorization: Bearer `. | Endpoint | Purpose | |---|---| -| `POST /api/scans` `{"tag_id": "53-13-0B-2A"}` | Record a Scan; creates the Tag on first sight. Returns `{"tag_id", "assigned": true, "artist", "album"}` or `{"tag_id", "assigned": false}`. | -| `GET /api/tags/{tag_id}` | Read-only lookup, same response. `404` if the tag has never been seen. | +| `POST /api/scans` `{"tag_id": "53-13-0B-2A"}` | Record a Scan; creates the Tag on first sight. Returns `{"tag_id", "assigned": true, "artist", "album", "uri"}` or `{"tag_id", "assigned": false}`. `uri` is the album's Music Assistant URI (e.g. `library://album/2846`), checked against the library on every scan; `null` means play by name. | +| `GET /api/tags/{tag_id}` | Read-only lookup, same response, with the stored `uri` unchecked. `404` if the tag has never been seen. | Tag IDs are normalised to upper case. +HA plays by `uri` when there is one, because a library can hold near-duplicate albums whose names differ only in capitals, and only one of them may be playable. On each scan the service checks the stored URI is still in the library. If it's gone (e.g. after a library rebuild), it finds the album by name again, preferring the exact spelling, and stores the new URI. + The web page (`/`) has no login: keep the service on the LAN. ## Configuration @@ -102,16 +104,20 @@ env DB_PATH=/var/db/tag_albums/tag_albums.sqlite3 \ su -m tagalbums -c '.venv/bin/tag-albums import-yaml /path/to/tag_albums.yaml' ``` -### Album covers +### Linking assignments to the library -The service stores each assignment's cover and serves it itself. Music Assistant's image URLs are plain http on another host, which browsers block on an https page. Albums assigned from the web page get their cover right away. Imported assignments have none, so fetch them once (this reads the env file for HA access): +Albums assigned from the web page store their library URI and cover straight away. Imported assignments only have names, so link them once. This finds each album (exact spelling first), stores its URI and downloads its cover. It reads the env file for HA access: ```sh cd /usr/local/tag-albums -su -m tagalbums -c 'set -a; . /usr/local/etc/tag_albums.env; set +a; .venv/bin/tag-albums fetch-covers' +su -m tagalbums -c 'set -a; . /usr/local/etc/tag_albums.env; set +a; .venv/bin/tag-albums sync-library' ``` -It lists the albums Music Assistant has no cover for; those keep the grey placeholder. Running it again only looks at assignments still missing a cover. +It lists albums it couldn't find (those keep playing by name) and albums with no cover in Music Assistant. Running it again only looks at assignments still missing something. `fetch-covers` is an older name for the same command. Scanning an assignment without a URI also links it. + +### Album covers + +The service stores each assignment's cover and serves it itself. Music Assistant's image URLs are plain http on another host, which browsers block on an https page. `sync-library` (above) fetches covers for imported assignments; albums with no cover in Music Assistant keep the grey placeholder. The service downloads covers from the address Music Assistant reports: its own host and port 8095, over plain http. If the jail can't reach that, put Music Assistant's image proxy behind the reverse proxy and set `MA_URL` to it. For example, in Caddy (use the same TLS setup as your other sites): @@ -125,7 +131,7 @@ ma.home.knbg { } ``` -Then add `MA_URL=https://ma.home.knbg` to the env file and restart the service. Only `/imageproxy/*` is exposed, not the Music Assistant interface. If `fetch-covers` can't connect, it stops at the first cover and says so. +Then add `MA_URL=https://ma.home.knbg` to the env file and restart the service. Only `/imageproxy/*` is exposed, not the Music Assistant interface. If `sync-library` can't connect, it stops at the first cover and says so. ### Updating diff --git a/src/tag_albums/app.py b/src/tag_albums/app.py index 1413c3e..21aabd3 100644 --- a/src/tag_albums/app.py +++ b/src/tag_albums/app.py @@ -37,10 +37,32 @@ class ScanRequest(BaseModel): tag_id: str -def _tag_response(tag: Tag) -> dict: +def _tag_response(tag: Tag, uri: str | None = None) -> dict: if not tag.assigned: return {"tag_id": tag.tag_id, "assigned": False} - return {"tag_id": tag.tag_id, "assigned": True, "artist": tag.artist, "album": tag.album} + return {"tag_id": tag.tag_id, "assigned": True, "artist": tag.artist, "album": tag.album, "uri": uri} + + +def resolve_uri(store: Store, library: MusicLibrary, tag: Tag) -> str | None: + """The Music Assistant URI HA should play this Assignment by, or None for by-name. + + Checks the stored URI against the library, and finds (and stores) a new + one when it's missing or gone. If the library can't be searched, trusts + the stored URI. + """ + try: + if tag.uri: + uri = library.current_uri(tag.uri, tag.artist, tag.album) + else: + match = library.find_album(tag.artist, tag.album) + uri = match.uri if match else None + except (httpx.HTTPError, RuntimeError) as e: + log.warning("Couldn't check %s against the library: %s", tag.tag_id, e) + return tag.uri + if uri and uri != tag.uri: + log.info("%s now plays %s (was %s)", tag.tag_id, uri, tag.uri) + store.set_uri(tag.tag_id, uri) + return uri def store_cover( @@ -97,15 +119,16 @@ def create_app(settings: Settings, store: Store | None = None, library: MusicLib """Record a Scan and return the Tag's Assignment, if any.""" if not scan.tag_id.strip(): raise HTTPException(status_code=422, detail="tag_id is empty") - return _tag_response(store.record_scan(scan.tag_id)) + tag = store.record_scan(scan.tag_id) + return _tag_response(tag, resolve_uri(store, library, tag) if tag.assigned else None) @app.get("/api/tags/{tag_id}", dependencies=[Depends(require_token)]) def get_tag(tag_id: str) -> dict: - """Read-only lookup: doesn't count as a Scan.""" + """Read-only lookup: doesn't count as a Scan, doesn't touch the library.""" tag = store.get(tag_id) if tag is None: raise HTTPException(status_code=404, detail="Unknown tag") - return _tag_response(tag) + return _tag_response(tag, tag.uri) # --- Web page ------------------------------------------------------------- @@ -171,10 +194,11 @@ def create_app(settings: Settings, store: Store | None = None, library: MusicLib album: str = Form(), image: str = Form(default=""), sig: str = Form(default=""), + uri: str = Form(default=""), ): # Only keep an image URL that came from our own search results. image = image if signed(image, sig) else "" - store.assign(tag_id, artist, album, image) + store.assign(tag_id, artist, album, image, uri) if image: store_cover(store, library, tag_id, image) return RedirectResponse("/", status_code=303) diff --git a/src/tag_albums/cli.py b/src/tag_albums/cli.py index a5f7d2a..b3ca14a 100644 --- a/src/tag_albums/cli.py +++ b/src/tag_albums/cli.py @@ -1,7 +1,8 @@ -"""Command line: run the service, import Assignments, or fill in missing covers.""" +"""Command line: run the service, import Assignments, or sync them with the library.""" import argparse import os +from dataclasses import dataclass, field import httpx import yaml @@ -21,27 +22,44 @@ def import_yaml(path: str, db_path: str) -> int: return len(entries) -def fetch_covers(store: Store, library) -> tuple[list[str], list[str]]: - """Store a cover for every Assignment that has none. +@dataclass +class SyncReport: + linked: list[str] = field(default_factory=list) # got their library URI + unmatched: list[str] = field(default_factory=list) # no album with these names + covers: list[str] = field(default_factory=list) # got a cover + coverless: list[str] = field(default_factory=list) # the library has no cover - Uses the image URL saved at assignment time, or else looks the album up - in the library by its exact artist and album names (imported Assignments - have no URL). Returns the albums that got a cover and those that didn't. + +def sync_library(store: Store, library) -> SyncReport: + """Fill in what Assignments are missing from the music library. + + Imported and older Assignments have only names: this finds their album + (exact spelling first, then ignoring case), stores its URI so HA plays + exactly that album, and stores its cover. Assignments that already have + both are left alone. """ from .app import store_cover - found, missing = [], [] + report = SyncReport() for tag in store.assigned(): - if tag.has_cover: + if tag.uri and tag.has_cover: continue label = f"{tag.artist} – {tag.album}" - image, save_url = tag.image, False - if not image: + match = None + if not tag.uri or not (tag.has_cover or tag.image): try: match = library.find_album(tag.artist, tag.album) except (httpx.HTTPError, RuntimeError) as e: raise SystemExit(f"Library search failed: {e}") - image, save_url = (match.image if match else None), True + if not tag.uri: + if match and match.uri: + store.set_uri(tag.tag_id, match.uri) + report.linked.append(label) + else: + report.unmatched.append(label) + if tag.has_cover: + continue + image, save_url = (tag.image, False) if tag.image else ((match.image if match else None), True) try: stored = bool(image) and store_cover( store, library, tag.tag_id, image, save_url=save_url, raise_unreachable=True @@ -52,8 +70,8 @@ def fetch_covers(store: Store, library) -> tuple[list[str], list[str]]: "Set MA_URL to an address of Music Assistant this host can reach " "(e.g. a reverse proxy in front of its port 8095)." ) - (found if stored else missing).append(label) - return found, missing + (report.covers if stored else report.coverless).append(label) + return report def main() -> None: @@ -67,7 +85,11 @@ def main() -> None: imp = sub.add_parser("import-yaml", help="Import Assignments from a tag_albums.yaml file") imp.add_argument("path") - sub.add_parser("fetch-covers", help="Download covers for Assignments that have none") + sub.add_parser( + "sync-library", + aliases=["fetch-covers"], + help="Link Assignments to their library album and download missing covers", + ) args = parser.parse_args() if args.command == "serve": @@ -78,7 +100,7 @@ def main() -> None: db_path = os.environ.get("DB_PATH", "tag_albums.sqlite3") count = import_yaml(args.path, db_path) print(f"Imported {count} assignments into {db_path}") - elif args.command == "fetch-covers": + elif args.command in ("sync-library", "fetch-covers"): from .config import Settings from .library import MusicLibrary @@ -86,11 +108,16 @@ def main() -> None: store = Store(settings.db_path) try: library = MusicLibrary(settings.ha_url, settings.ha_token, ma_url=settings.ma_url) - found, missing = fetch_covers(store, library) + report = sync_library(store, library) finally: store.close() - print(f"Stored {len(found)} covers.") - if missing: - print(f"No cover found for {len(missing)}:") - for label in missing: - print(f" {label}") + print(f"Linked {len(report.linked)} assignments to their library album.") + print(f"Stored {len(report.covers)} covers.") + for title, labels in ( + ("No library album found (these play by name)", report.unmatched), + ("No cover in the library", report.coverless), + ): + if labels: + print(f"{title}, {len(labels)}:") + for label in labels: + print(f" {label}") diff --git a/src/tag_albums/library.py b/src/tag_albums/library.py index 3730c1e..7866e5c 100644 --- a/src/tag_albums/library.py +++ b/src/tag_albums/library.py @@ -11,6 +11,7 @@ class Album: artist: str album: str image: str | None + uri: str | None = None # Covers from Music Assistant are well under this; anything bigger isn't a cover. @@ -83,16 +84,28 @@ class MusicLibrary: artist=next((a["name"] for a in item.get("artists", [])), ""), album=item["name"], image=item.get("image"), + uri=item.get("uri"), ) for item in albums ] def find_album(self, artist: str, album: str) -> Album | None: - """The library album matching an Assignment's names exactly (ignoring case).""" - for found in self.search_albums(album, limit=25): - if found.album.casefold() == album.casefold() and found.artist.casefold() == artist.casefold(): - return found - return None + """The library album matching an Assignment's names.""" + return _best_match(self.search_albums(album, limit=25), artist, album) + + def current_uri(self, uri: str, artist: str, album: str) -> str | None: + """The URI to play an Assignment by, checked against the library. + + The stored URI if the library still has it; otherwise the URI of the + album now matching the names (e.g. after a library rebuild); None if + nothing matches, so the caller falls back to playing by name. + """ + results = self.search_albums(album, limit=25) + if any(r.uri == uri for r in results): + return uri + match = _best_match(results, artist, album) + return match.uri if match else None + def fetch_image(self, url: str) -> tuple[bytes, str]: """Download a cover image; raises if it isn't an image or is too big.""" @@ -104,3 +117,16 @@ class MusicLibrary: if len(r.content) > MAX_COVER_BYTES: raise ValueError("image too large") return r.content, content_type + + +def _best_match(albums: list[Album], artist: str, album: str) -> Album | None: + """The album with these names: exact spelling first, then ignoring case. + + Libraries can hold near-duplicates that differ only in capitals ("Let the + Tribe Increase" vs "Let The Tribe Increase"), and only one may be playable. + """ + for same in (lambda a, b: a == b, lambda a, b: a.casefold() == b.casefold()): + for found in albums: + if same(found.album, album) and same(found.artist, artist): + return found + return None diff --git a/src/tag_albums/store.py b/src/tag_albums/store.py index 5b0c01f..7237d00 100644 --- a/src/tag_albums/store.py +++ b/src/tag_albums/store.py @@ -19,6 +19,7 @@ CREATE TABLE IF NOT EXISTS tags ( artist TEXT, album TEXT, image TEXT, + uri TEXT, assigned_at TEXT, cover BLOB, cover_type TEXT @@ -26,10 +27,10 @@ CREATE TABLE IF NOT EXISTS tags ( """ # Columns added after the first release, for databases created before them. -MIGRATIONS = {"cover": "BLOB", "cover_type": "TEXT"} +MIGRATIONS = {"cover": "BLOB", "cover_type": "TEXT", "uri": "TEXT"} TAG_COLUMNS = ( - "tag_id, first_seen, last_seen, scan_count, artist, album, image, assigned_at," + "tag_id, first_seen, last_seen, scan_count, artist, album, image, uri, assigned_at," " cover IS NOT NULL AS has_cover" ) @@ -49,6 +50,9 @@ class Tag: artist: str | None album: str | None image: str | None + # The album's Music Assistant URI (e.g. library://album/2846). Names can + # match more than one library entry; the URI can't. + uri: str | None assigned_at: str | None has_cover: bool @@ -104,7 +108,9 @@ class Store: ) return self.get(tag_id) - def assign(self, tag_id: str, artist: str, album: str, image: str | None = None) -> Tag: + def assign( + self, tag_id: str, artist: str, album: str, image: str | None = None, uri: str | None = None + ) -> Tag: """Create or replace the Tag's Assignment, creating the Tag if needed. Any stored cover belongs to the previous Assignment, so it's dropped. @@ -113,14 +119,14 @@ class Store: now = _now() self._conn.execute( """ - INSERT INTO tags (tag_id, first_seen, artist, album, image, assigned_at) - VALUES (?, ?, ?, ?, ?, ?) + INSERT INTO tags (tag_id, first_seen, artist, album, image, uri, assigned_at) + VALUES (?, ?, ?, ?, ?, ?, ?) ON CONFLICT (tag_id) DO UPDATE SET artist = excluded.artist, album = excluded.album, - image = excluded.image, assigned_at = excluded.assigned_at, + image = excluded.image, uri = excluded.uri, assigned_at = excluded.assigned_at, cover = NULL, cover_type = NULL """, - (tag_id, now, artist, album, image or None, now), + (tag_id, now, artist, album, image or None, uri or None, now), ) return self.get(tag_id) @@ -131,6 +137,9 @@ class Store: (data, content_type, image, normalize_tag_id(tag_id)), ) + def set_uri(self, tag_id: str, uri: str | None) -> None: + self._conn.execute("UPDATE tags SET uri = ? WHERE tag_id = ?", (uri, normalize_tag_id(tag_id))) + def get_cover(self, tag_id: str) -> tuple[bytes, str] | None: row = self._conn.execute( "SELECT cover, cover_type FROM tags WHERE tag_id = ? AND cover IS NOT NULL", @@ -140,7 +149,7 @@ class Store: def unassign(self, tag_id: str) -> None: self._conn.execute( - "UPDATE tags SET artist = NULL, album = NULL, image = NULL, assigned_at = NULL," + "UPDATE tags SET artist = NULL, album = NULL, image = NULL, uri = NULL, assigned_at = NULL," " cover = NULL, cover_type = NULL WHERE tag_id = ?", (normalize_tag_id(tag_id),), ) diff --git a/src/tag_albums/templates/_results.html b/src/tag_albums/templates/_results.html index 3de538b..f234d1c 100644 --- a/src/tag_albums/templates/_results.html +++ b/src/tag_albums/templates/_results.html @@ -11,6 +11,7 @@ + diff --git a/tests/conftest.py b/tests/conftest.py index b728998..7145d47 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,9 +1,10 @@ +import httpx import pytest from fastapi.testclient import TestClient from tag_albums.app import create_app from tag_albums.config import Settings -from tag_albums.library import Album +from tag_albums.library import Album, MusicLibrary, _best_match from tag_albums.store import Store TOKEN = "test-token" @@ -14,15 +15,24 @@ class FakeLibrary: def __init__(self): self.queries = [] self.fetched = [] + self.fail_search = False + self.library = [Album(artist="Tool", album="Lateralus", image="http://img/1", uri="library://album/1868")] self.images = {"http://img/1": (b"\x89PNG-lateralus", "image/png")} def search_albums(self, query, limit=12): self.queries.append(query) - return [Album(artist="Tool", album="Lateralus", image="http://img/1")] + return list(self.library) def find_album(self, artist, album): - found = self.search_albums(album)[0] - return found if (found.artist, found.album) == (artist, album) else None + return _best_match(self.search_albums(album), artist, album) + + def current_uri(self, uri, artist, album): + if self.fail_search: + raise httpx.ConnectError("HA down") + return MusicLibrary.current_uri(self, uri, artist, album) + + def image_download_url(self, url): + return url def fetch_image(self, url): self.fetched.append(url) diff --git a/tests/test_app.py b/tests/test_app.py index 1f750e3..53517ec 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -3,7 +3,8 @@ import sqlite3 from conftest import AUTH -from tag_albums.cli import fetch_covers, import_yaml +from tag_albums.cli import import_yaml, sync_library +from tag_albums.library import Album from tag_albums.store import Store @@ -23,7 +24,8 @@ def test_first_scan_creates_unassigned_tag(client, store): def test_scan_of_assigned_tag_returns_album(client, store): store.assign("86-2C-1D-BD", "Tool", "Lateralus") r = client.post("/api/scans", json={"tag_id": "86-2C-1D-BD"}, headers=AUTH) - assert r.json() == {"tag_id": "86-2C-1D-BD", "assigned": True, "artist": "Tool", "album": "Lateralus"} + assert r.json() == {"tag_id": "86-2C-1D-BD", "assigned": True, "artist": "Tool", "album": "Lateralus", + "uri": "library://album/1868"} assert store.get("86-2C-1D-BD").scan_count == 1 @@ -158,27 +160,19 @@ def test_old_database_is_migrated(tmp_path): store.close() -def test_fetch_covers_for_imported_assignments(store, library): - store.assign("A", "Tool", "Lateralus") # imported: no image URL +def test_sync_library_links_and_fetches_covers(store, library): + store.assign("A", "Tool", "Lateralus") # imported: names only store.assign("B", "Amenra", "De Doorn") # not in the library - found, missing = fetch_covers(store, library) - assert found == ["Tool – Lateralus"] and missing == ["Amenra – De Doorn"] + report = sync_library(store, library) + assert report.linked == ["Tool – Lateralus"] and report.unmatched == ["Amenra – De Doorn"] + assert report.covers == ["Tool – Lateralus"] and report.coverless == ["Amenra – De Doorn"] tag = store.get("A") - assert tag.has_cover and tag.image == "http://img/1" - assert fetch_covers(store, library) == ([], ["Amenra – De Doorn"]) # already stored: skipped + assert tag.uri == "library://album/1868" and tag.has_cover and tag.image == "http://img/1" + again = sync_library(store, library) # done ones are skipped + assert (again.linked, again.covers) == ([], []) -def test_import_yaml(tmp_path): - src = tmp_path / "tag_albums.yaml" - src.write_text('"29-92-68-C5":\n artist: ONRUST\n album: Van Woede Tot Wanhoop\n') - db = str(tmp_path / "db.sqlite3") - assert import_yaml(str(src), db) == 1 - store = Store(db) - assert store.get("29-92-68-C5").album == "Van Woede Tot Wanhoop" - store.close() - - -def test_fetch_covers_stops_when_music_assistant_is_unreachable(store, library): +def test_sync_library_stops_when_music_assistant_is_unreachable(store, library): import httpx import pytest @@ -186,11 +180,9 @@ def test_fetch_covers_stops_when_music_assistant_is_unreachable(store, library): raise httpx.ConnectTimeout("timed out") library.fetch_image = unreachable - library.image_download_url = lambda url: url store.assign("A", "Tool", "Lateralus") - store.assign("B", "Kiasmos", "Kiasmos") with pytest.raises(SystemExit, match="Set MA_URL"): - fetch_covers(store, library) + sync_library(store, library) def test_settings_read_ma_url(monkeypatch): @@ -201,3 +193,52 @@ def test_settings_read_ma_url(monkeypatch): assert Settings.from_env().ma_url == "https://ma.example" monkeypatch.delenv("MA_URL") assert Settings.from_env().ma_url is None + + +# --- Playing by URI ----------------------------------------------------------- + +def _scan(client, tag_id): + return client.post("/api/scans", json={"tag_id": tag_id}, headers=AUTH).json() + + +def test_assign_from_search_stores_the_uri(client, store): + client.post("/tags/AA-BB/assign", data=_search_form(client)) + assert store.get("AA-BB").uri == "library://album/1868" + assert _scan(client, "AA-BB")["uri"] == "library://album/1868" + + +def test_scan_links_names_only_assignment_to_exact_spelling(client, store, library): + # The case that broke: two library entries differing only in capitals, + # and only the one spelled like the assignment is playable. + library.library = [ + Album("The Mob", "Let The Tribe Increase", None, "library://album/1183"), + Album("The Mob", "Let the Tribe Increase", None, "library://album/2846"), + ] + store.assign("MOB", "The Mob", "Let the Tribe Increase") + assert _scan(client, "MOB")["uri"] == "library://album/2846" + assert store.get("MOB").uri == "library://album/2846" + + +def test_scan_replaces_a_uri_the_library_no_longer_has(client, store, library): + store.assign("A", "Tool", "Lateralus", uri="library://album/9999") # before a rebuild + assert _scan(client, "A")["uri"] == "library://album/1868" + assert store.get("A").uri == "library://album/1868" + + +def test_scan_falls_back_to_names_when_nothing_matches(client, store, library): + store.assign("A", "Amenra", "De Doorn", uri="library://album/9999") + body = _scan(client, "A") + assert body["uri"] is None and body["album"] == "De Doorn" + assert store.get("A").uri == "library://album/9999" # kept, in case it comes back + + +def test_scan_trusts_stored_uri_when_library_is_unreachable(client, store, library): + store.assign("A", "Tool", "Lateralus", uri="library://album/1868") + library.fail_search = True + assert _scan(client, "A")["uri"] == "library://album/1868" + + +def test_lookup_returns_stored_uri_without_searching(client, store, library): + store.assign("A", "Tool", "Lateralus", uri="library://album/1868") + r = client.get("/api/tags/A", headers=AUTH).json() + assert r["uri"] == "library://album/1868" and library.queries == []