diff --git a/app.py b/app.py index c075b4d..dd54bde 100644 --- a/app.py +++ b/app.py @@ -1,10 +1,12 @@ #!/usr/bin/env python3 """fnack – Modern Lossless Music Discography Downloader & Manager. -A modular Flask + SocketIO application with Deezer metadata ingestion, -ISRC-first SpotiFLAC / yt-dlp download pipeline, Sonarr-style artist discography views, -and interactive library import. Lidarr emulation ships as the bundled -`fnack.lidarr` plugin (Newznab/Torznab + SABnzbd), enabled by default. +A modular Flask + SocketIO application with provider-neutral metadata +ingestion (capability-based; official providers ship as plugins), +ISRC-first SpotiFLAC / yt-dlp download pipeline, Sonarr-style artist +discography views, and interactive library import. Lidarr emulation ships as +the bundled `fnack.lidarr` plugin (Newznab/Torznab + SABnzbd), enabled by +default. """ import nest_asyncio @@ -438,9 +440,9 @@ def _retry_interval_seconds() -> int: return 24 * 3600 -def _start_bounded_artist_sync(artist_id: int, deezer_artist_id: int, options: dict): +def _start_bounded_artist_sync(artist_id: int, external_id: str, options: dict): """Run a discography sync as a background task without blocking the scheduler, - bounded so bursts of overdue artists cannot hammer the Deezer API.""" + bounded so bursts of overdue artists cannot hammer the metadata provider.""" global _running_artist_syncs if _running_artist_syncs >= _MAX_CONCURRENT_SYNCS: logger.info("[SCHEDULER] Skipping sync for artist %d: %d syncs already in flight", artist_id, _running_artist_syncs) @@ -450,15 +452,16 @@ def _start_bounded_artist_sync(artist_id: int, deezer_artist_id: int, options: d def _wrapped(): global _running_artist_syncs try: - _sync_artist_discography_background(artist_id, deezer_artist_id, options) + _sync_artist_discography_background(artist_id, external_id, options) finally: _running_artist_syncs -= 1 socketio.start_background_task(_wrapped) -def _sync_artist_discography_background(artist_id: int, deezer_artist_id: int, options: dict): - """Background task to fetch artist discography from Deezer and index albums & tracks. +def _sync_artist_discography_background(artist_id: int, external_id: str, options: dict): + """Background task to fetch an artist's discography from the metadata + provider chain and index albums & tracks. Runs as a SocketIO background greenlet (no inherited app context), so the ENTIRE body below executes inside one app context — provider invocations @@ -475,21 +478,22 @@ def _sync_artist_discography_background(artist_id: int, deezer_artist_id: int, o db.session.commit() socketio.emit("artist_updated", {"artist_id": artist_id, "sync_status": "syncing"}) - logger.info("[DEEZER] Starting background discography fetch for artist '%s' (%d)", artist.name, deezer_artist_id) + logger.info("[SYNC] Starting background discography fetch for artist '%s' (external id %s)", + artist.name, external_id) try: # Phase 3: MetadataService (artist.discography capability) owns the - # provider chain — Deezer batch (p10) authoritative, then MusicBrainz - # (p20, enrichment-only), Spotify (p30), iTunes (p40), first usable - # discography wins. No hidden fallback: if no artist.discography - # provider is enabled the service returns the empty shape (MASTER - # rule 3). + # provider chain — providers are tried in registry priority order + # and the FIRST usable discography wins. The provider chain + # interprets the external identity; core only routes the opaque + # value. No hidden fallback: if no artist.discography provider is + # enabled the service returns the empty shape (MASTER rule 3). disco = None served_by = None try: from services.metadata_service import MetadataService disco = MetadataService().get_artist_discography( - str(deezer_artist_id), + external_id, artist_name=artist.name, filter_remixes=options.get("filter_remixes", True), filter_lofi=options.get("filter_lofi", True), @@ -537,14 +541,14 @@ def _sync_artist_discography_background(artist_id: int, deezer_artist_id: int, o # Save albums and tracks for a in disco.get("albums", []): - album = Album.query.filter_by(artist_id=artist.id, deezer_id=str(a["id"])).first() + album = Album.query.filter_by(artist_id=artist.id, external_id=str(a["id"])).first() if not album: album = Album( artist_id=artist.id, name=a["title"], year=a.get("year"), cover_url=a.get("cover_url"), - deezer_id=str(a["id"]), + external_id=str(a["id"]), record_type=a.get("record_type", "album"), ) db.session.add(album) @@ -566,7 +570,7 @@ def _sync_artist_discography_background(artist_id: int, deezer_artist_id: int, o album.mb_year = a["mb_year"] for t in a.get("tracks", []): - track = Track.query.filter_by(album_id=album.id, deezer_id=str(t["id"])).first() + track = Track.query.filter_by(album_id=album.id, external_id=str(t["id"])).first() if not track: track = Track( album_id=album.id, @@ -576,7 +580,7 @@ def _sync_artist_discography_background(artist_id: int, deezer_artist_id: int, o disc_number=t.get("disk_number", 1), duration=t.get("duration"), isrc=t.get("isrc"), - deezer_id=str(t["id"]), + external_id=str(t["id"]), genre=t.get("genre"), status="missing", ) @@ -590,14 +594,17 @@ def _sync_artist_discography_background(artist_id: int, deezer_artist_id: int, o if t.get("genre") and not track.genre: track.genre = t["genre"] - valid_deezer_ids = {str(a["id"]) for a in disco.get("albums", [])} + # The provider chain returns each release under the provider's + # own opaque external id; core stores it verbatim (never + # interprets it) and uses it to prune releases that the + # current discography no longer contains. + current_external_ids = {str(a["id"]) for a in disco.get("albums", [])} - # Prune stale or misattributed albums that are not downloaded and no longer in discography for existing_alb in artist.albums.all(): - if existing_alb.deezer_id not in valid_deezer_ids and not existing_alb.is_downloaded: + if existing_alb.external_id not in current_external_ids and not existing_alb.is_downloaded: has_downloaded = any(t.is_downloaded for t in existing_alb.tracks.all()) if not has_downloaded: - logger.info("[DEEZER] Pruning stale/misattributed album '%s' (id=%d) for artist '%s'", existing_alb.name, existing_alb.id, artist.name) + logger.info("[SYNC] Pruning stale/misattributed album '%s' (id=%d) for artist '%s'", existing_alb.name, existing_alb.id, artist.name) for t in existing_alb.tracks.all(): DownloadJob.query.filter_by(track_id=t.id).delete() db.session.delete(t) @@ -618,18 +625,18 @@ def _sync_artist_discography_background(artist_id: int, deezer_artist_id: int, o db.session.commit() - logger.info("[DEEZER] Discography sync complete for '%s' (%d albums)", artist.name, len(disco.get("albums", []))) + logger.info("[SYNC] Discography sync complete for '%s' (%d albums)", artist.name, len(disco.get("albums", []))) # If auto_download enabled, queue missing tracks if artist.auto_download: queued = queue_artist_missing(app, artist.id, source="auto") - logger.info("[DEEZER] Auto-download queued %d tracks for '%s'", queued, artist.name) + logger.info("[SYNC] Auto-download queued %d tracks for '%s'", queued, artist.name) socketio.emit("artist_synced", {"artist_id": artist_id, "sync_status": "ready"}) socketio.emit("toast", {"message": f"Discography indexed for '{disco['artist_name']}'", "type": "success"}) except Exception as e: - logger.exception("[DEEZER] Discography ingestion failed for artist %d: %s", artist_id, e) + logger.exception("[SYNC] Discography ingestion failed for artist %d: %s", artist_id, e) with app.app_context(): artist = db.session.get(Artist, artist_id) if artist: @@ -642,25 +649,29 @@ def _sync_artist_discography_background(artist_id: int, deezer_artist_id: int, o @app.route("/api/add-artist", methods=["POST"]) def api_add_artist(): data = request.get_json(silent=True) or {} - deezer_id = data.get("id") - if not deezer_id: + external_id = data.get("id") + if not external_id: return jsonify({"error": "No artist ID provided"}), 400 - existing = Artist.query.filter_by(spotify_id=str(deezer_id)).first() + existing = Artist.query.filter_by(external_id=str(external_id)).first() if existing: return jsonify({"message": f"Artist '{existing.name}' is already in your library.", "artist_id": existing.id}), 200 try: - # Phase 4: artist.info capability via MetadataService (the - # fnack.deezer-batch plugin serves it) — core has no Deezer import. + # Phase 4: artist.info capability via MetadataService — the provider + # chain interprets the opaque external identity (the provider that + # owns the id parses/converts it); core never assumes what the id is. from services.metadata_service import MetadataService - artist_info = MetadataService().get_artist_info(str(deezer_id)) or {} + artist_info = MetadataService().get_artist_info(external_id) or {} except Exception as e: - return jsonify({"error": f"Failed to reach Deezer: {e}"}), 500 + return jsonify({"error": f"Metadata lookup failed: {e}"}), 500 + if not artist_info.get("name"): + return jsonify({"error": "The metadata provider could not resolve " + f"artist info for external id '{external_id}'"}), 400 # Save artist record immediately so UI shows placeholder instantly artist = Artist( - spotify_id=str(deezer_id), + external_id=str(external_id), name=artist_info["name"], image_url=artist_info.get("image_url"), monitored=bool(data.get("monitored", True)), @@ -681,7 +692,7 @@ def api_add_artist(): # Start background ingestion socketio.start_background_task( - _sync_artist_discography_background, artist.id, int(deezer_id), data + _sync_artist_discography_background, artist.id, str(external_id), data ) return jsonify({ @@ -842,18 +853,23 @@ def api_delete_album(album_id): return jsonify({"message": f"Album '{album_name}' deleted from discography."}) +# Primary provider-neutral path. `/set-deezer-id` below is a deprecated +# alias kept for older clients; the wire key "deezer_id" is likewise accepted +# as a legacy alias of "external_id" (the value is an opaque provider +# identity — nothing in core assumes it is a Deezer id). +@app.route("/api/artist//set-external-id", methods=["POST"]) @app.route("/api/artist//set-deezer-id", methods=["POST"]) -def api_artist_set_deezer_id(artist_id): +def api_artist_set_external_id(artist_id): artist = db.session.get(Artist, artist_id) if not artist: return jsonify({"error": "Artist not found"}), 404 data = request.get_json(silent=True) or {} - new_deezer_id = str(data.get("deezer_id", "")).strip() - if not new_deezer_id: - return jsonify({"error": "Missing deezer_id"}), 400 + new_external_id = str(data.get("external_id") or data.get("deezer_id") or "").strip() + if not new_external_id: + return jsonify({"error": "Missing external_id"}), 400 - artist.spotify_id = new_deezer_id + artist.external_id = new_external_id db.session.commit() opts = { @@ -865,8 +881,8 @@ def api_artist_set_deezer_id(artist_id): "include_singles": artist.include_singles, "include_compilations": artist.include_compilations, } - socketio.start_background_task(_sync_artist_discography_background, artist.id, int(new_deezer_id), opts) - return jsonify({"message": f"Artist Deezer ID updated to {new_deezer_id}. Re-syncing discography."}) + socketio.start_background_task(_sync_artist_discography_background, artist.id, new_external_id, opts) + return jsonify({"message": f"Artist external id updated to {new_external_id}. Re-syncing discography."}) @app.route("/api/artist//sync", methods=["POST"]) @@ -875,16 +891,17 @@ def api_artist_sync(artist_id): if not artist: return jsonify({"error": "Artist not found"}), 404 - deezer_id = int(artist.spotify_id) if artist.spotify_id.isdigit() else None - if not deezer_id: - # Phase 3: application service (artist.search capability). + external_id = artist.external_id + if not external_id: + # Phase 3: application service (artist.search capability) — find the + # artist's external identity by name when none is stored. from services.metadata_service import MetadataService res = MetadataService().search_artist(artist.name, limit=1) if res: - deezer_id = res[0]["id"] + external_id = res[0]["id"] - if not deezer_id: - return jsonify({"error": "Could not resolve Deezer ID for artist"}), 400 + if not external_id: + return jsonify({"error": "Could not resolve an external id for the artist"}), 400 opts = { "filter_remixes": artist.filter_remixes, @@ -896,7 +913,7 @@ def api_artist_sync(artist_id): "include_compilations": artist.include_compilations, } - socketio.start_background_task(_sync_artist_discography_background, artist.id, deezer_id, opts) + socketio.start_background_task(_sync_artist_discography_background, artist.id, external_id, opts) return jsonify({"message": f"Sync started for '{artist.name}'."}) @@ -1151,7 +1168,7 @@ def api_track_identify(track_id): if matched_artist: artist = Artist.query.filter_by(name=matched_artist).first() if not artist: - artist = Artist(spotify_id=f"acoustid:{matched_artist}", name=matched_artist, source="manual") + artist = Artist(external_id =f"acoustid:{matched_artist}", name=matched_artist, source="manual") db.session.add(artist) db.session.flush() t.artist_id = artist.id @@ -1272,7 +1289,7 @@ def api_track_caution(track_id): if matched_artist: artist = Artist.query.filter_by(name=matched_artist).first() if not artist: - artist = Artist(spotify_id=f"acoustid:{matched_artist}", name=matched_artist, source="manual") + artist = Artist(external_id =f"acoustid:{matched_artist}", name=matched_artist, source="manual") db.session.add(artist) db.session.flush() t.artist_id = artist.id @@ -1447,12 +1464,15 @@ def api_import_candidates(): def api_import_folder(): data = request.get_json(silent=True) or {} folder_name = data.get("folder_name") - deezer_id = data.get("deezer_id") + # Opaque external artist identity from the UI's search suggestion. The + # wire key "deezer_id" is a legacy alias of "external_id" (frontend + # contract); core treats the value as provider-neutral. + external_id = data.get("external_id") or data.get("deezer_id") if not folder_name: return jsonify({"error": "Folder name required"}), 400 music_path = _get_setting("music_path", "/music") - res = import_artist_folder(music_path, folder_name, deezer_artist_id=deezer_id, filter_options=data) + res = import_artist_folder(music_path, folder_name, external_id=external_id, filter_options=data) if "error" in res: return jsonify(res), 400 @@ -1461,7 +1481,7 @@ def api_import_folder(): # ── Bulk folder import (background, non-blocking) ────────────────────────── -# Importing an artist takes ~5-15s (Deezer discography + iTunes fallback + +# Importing an artist takes ~5-15s (metadata discography + enrichment + # DB writes). Running many of those synchronously in the request thread made # the web UI freeze. These are now processed by a single background worker # (bounded greenlet pool) that reports progress over SocketIO, so the page @@ -1512,7 +1532,7 @@ def _run(item, index): try: res = import_artist_folder( music_path, folder_name, - deezer_artist_id=item.get("deezer_id"), + external_id=item.get("external_id") or item.get("deezer_id"), filter_options=opts, ) except Exception as e: @@ -1999,7 +2019,7 @@ def _periodic_discography_sync_loop(): elapsed_h = (now - last).total_seconds() / 3600.0 if elapsed_h < interval_h: continue - artists_to_sync.append((a.id, a.spotify_id, { + artists_to_sync.append((a.id, a.external_id, { "filter_remixes": a.filter_remixes, "filter_lofi": a.filter_lofi, "filter_live": a.filter_live, @@ -2011,10 +2031,9 @@ def _periodic_discography_sync_loop(): if artists_to_sync: logger.info("[SCHEDULER] Running periodic discography check for %d monitored artists", len(artists_to_sync)) - for aid, spot_id, opts in artists_to_sync: - deezer_id = int(spot_id) if spot_id.isdigit() else None - if deezer_id: - _start_bounded_artist_sync(aid, deezer_id, opts) + for aid, artist_external_id, opts in artists_to_sync: + if artist_external_id: + _start_bounded_artist_sync(aid, str(artist_external_id), opts) gevent.sleep(3) except Exception: @@ -2106,6 +2125,24 @@ def _periodic_discography_sync_loop(): conn.execute(db.text(f"ALTER TABLE artists ADD COLUMN {col} INTEGER DEFAULT 0")) except Exception: pass + # Provider-neutral identity columns (final cleanup): the external + # identity columns are no longer named after a provider. Fresh + # databases already have the new names (create_all above); older + # databases are renamed in place (SQLite RENAME COLUMN preserves + # data, NOT NULL/unique constraints, and index definitions). The + # values are opaque provider identities — nothing here interprets + # them. + for table, old_col, new_col in [ + ("artists", "spotify_id", "external_id"), + ("albums", "deezer_id", "external_id"), + ("tracks", "deezer_id", "external_id"), + ("download_jobs", "album_spotify_id", "album_external_id"), + ]: + try: + conn.execute(db.text( + f"ALTER TABLE {table} RENAME COLUMN {old_col} TO {new_col}")) + except Exception: + pass conn.commit() except Exception: pass diff --git a/docs/architecture.md b/docs/architecture.md index eaea340..095acf7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -100,7 +100,7 @@ is the vendored copy. Snapshot of the catalog at the time of writing: |--------|-------------|------| | `fnack.spotiflac` | `download.track` | Primary lossless downloader (FLAC via Tidal/Qobuz/Deezer/SoundCloud, zero-auth) | | `fnack.ytdlp` | `download.track` | Fallback downloader (YouTube / YouTube Music / SoundCloud) | -| `fnack.deezer-batch` | `artist.search`, `artist.discography`, `artist.info`, `track.metadata`, `album.metadata`, `album.search`, `track.search`, `album.tracks` | Authoritative Deezer metadata | +| `fnack.deezer-batch` | `artist.search`, `artist.discography`, `artist.info`, `track.metadata`, `album.metadata`, `album.search`, `track.search`, `album.tracks` | Deezer metadata provider | | `fnack.musicbrainz` | `artist.search`, `artist.discography` (+ `enrich`) | Catalogue enrichment | | `fnack.itunes` | `artist.search`, `artist.discography`, `track.metadata`, `album.tracks` | iTunes metadata fallback | | `fnack.spotify` | `track.resolve` | Spotify track-URL resolution (ISRC-first, zero-auth) | @@ -136,6 +136,20 @@ the out-of-box first-run workflow: `fnack.spotiflac`, `fnack.ytdlp`, - Maintenance tasks run as a core subprocess (`scripts/run_maintenance.py`), not via `library.task` — the library-task plugins are optional. +## Identity model (provider-neutral) + +Core stores each artist/album/track's **external identity** as an opaque +string (`Artist.external_id`, `Album.external_id`, `Track.external_id`): +the id the metadata provider that supplied the record uses. Core never +parses, converts, or int()s it — when the provider chain resolves an +artist/album/track, the selected provider interprets its own identity (a +Deezer provider converts a numeric Deezer id, a MusicBrainz provider an +MBID, and so on). Providers that don't recognize an id decline it +gracefully (`None`/empty), so the chain moves on; a foreign id never +crashes core. Values may be prefixed for uniqueness when a record was +created outside the provider chain (e.g. `acoustid:`); the prefix is +also opaque to core. + ## Configuration model Configuration is split into **core** settings (fnack itself, in the diff --git a/models.py b/models.py index 706de47..e36dced 100644 --- a/models.py +++ b/models.py @@ -10,7 +10,13 @@ class Artist(db.Model): __tablename__ = "artists" id = db.Column(db.Integer, primary_key=True) - spotify_id = db.Column(db.String(64), unique=True, nullable=False) # deezer_id or custom + # Provider-neutral external identity: the opaque id the metadata provider + # that created this artist uses (e.g. a Deezer artist id when the + # fnack.deezer-batch provider supplied it, or a prefixed self-identity + # like "acoustid:" / "lidarr:" for artists created from a + # single unknown track). Core never interprets the value — the provider + # chain owns parsing/conversion. + external_id = db.Column(db.String(64), unique=True, nullable=False) name = db.Column(db.String(256), nullable=False, index=True) image_url = db.Column(db.Text, nullable=True) created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) @@ -60,7 +66,9 @@ class Album(db.Model): name = db.Column(db.String(512), nullable=False, index=True) year = db.Column(db.Integer, nullable=True) cover_url = db.Column(db.Text, nullable=True) - deezer_id = db.Column(db.String(32), nullable=True, index=True) + # Provider-neutral external release identity (opaque; interpreted by the + # metadata provider chain that supplied this release). + external_id = db.Column(db.String(64), nullable=True, index=True) record_type = db.Column(db.String(32), default="album", nullable=False, index=True) # album, single, compile, ep, other created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) is_downloaded = db.Column(db.Boolean, default=False, nullable=False, index=True) @@ -68,7 +76,7 @@ class Album(db.Model): size_bytes = db.Column(db.BigInteger, default=0, nullable=False) local_path = db.Column(db.Text, nullable=True) - # MusicBrainz enrichment (additive only — Deezer stays authoritative) + # MusicBrainz enrichment (additive only; enrichment is provider-owned) mb_release_group_id = db.Column(db.String(64), nullable=True, index=True) mb_title = db.Column(db.String(512), nullable=True) mb_year = db.Column(db.Integer, nullable=True) @@ -93,7 +101,9 @@ class Track(db.Model): track_number = db.Column(db.Integer, nullable=True) disc_number = db.Column(db.Integer, default=1, nullable=True) isrc = db.Column(db.String(64), nullable=True, index=True) - deezer_id = db.Column(db.String(32), nullable=True, index=True) + # Provider-neutral external track identity (opaque; interpreted by the + # metadata provider chain that supplied this track). + external_id = db.Column(db.String(64), nullable=True, index=True) spotify_url = db.Column(db.Text, nullable=True) genre = db.Column(db.String(128), nullable=True) file_path = db.Column(db.Text, default="", nullable=False) @@ -127,7 +137,7 @@ class DownloadJob(db.Model): album_id = db.Column(db.Integer, db.ForeignKey("albums.id"), nullable=True, index=True) artist_id = db.Column(db.Integer, db.ForeignKey("artists.id"), nullable=False, index=True) item_type = db.Column(db.String(16), default="track", nullable=False) # track | album - album_spotify_id = db.Column(db.String(64), nullable=True) + album_external_id = db.Column(db.String(64), nullable=True) album_name = db.Column(db.String(512), nullable=False) album_type = db.Column(db.String(32), default="album", nullable=False) album_url = db.Column(db.Text, default="", nullable=False) diff --git a/plugins/base.py b/plugins/base.py index e8e3378..2737632 100644 --- a/plugins/base.py +++ b/plugins/base.py @@ -32,7 +32,12 @@ class TrackRef: isrc: Optional[str] = None duration: Optional[float] = None # expected duration, seconds spotify_url: Optional[str] = None - deezer_id: Optional[str] = None + # Provider-neutral external track identity (opaque — the metadata + # provider chain that supplied the track interprets it). Core populates + # `external_id`; `deezer_id` is kept as a deprecated alias for plugins + # built against older SDK versions. + external_id: Optional[str] = None + deezer_id: Optional[str] = None # deprecated alias of external_id disc_number: int = 1 track_number: Optional[int] = None @@ -65,7 +70,7 @@ class TaskResult: @dataclass class RecommendationItem: kind: str # "artist" | "album" | "track" - ref_id: str # provider-specific id (e.g. deezer id) + ref_id: str # provider-specific external id (opaque) title: str subtitle: str = "" score: float = 0.0 @@ -153,7 +158,8 @@ def is_rate_limited(self) -> bool: class MetadataProviderPlugin(PluginBase): - """Artist/album/track metadata lookup, e.g. a Deezer/MusicBrainz-style source.""" + """Artist/album/track metadata lookup: a provider supplying + music metadata (the capability chain resolves providers).""" priority: int = 100 diff --git a/plugins/context.py b/plugins/context.py index 8d70397..ee7c6fe 100644 --- a/plugins/context.py +++ b/plugins/context.py @@ -132,14 +132,14 @@ def get_or_create_api_key(self) -> str: def search_albums(self, query: str, limit: int = 10) -> list[dict]: """Live album search (Phase 4: via MetadataService — album.search - capability, served by the fnack.deezer-batch plugin). Provider- - generic: the plugin boundary owns the implementation.""" + capability; the provider chain owns the implementation and the + plugin boundary stays provider-generic.""" from services.metadata_service import MetadataService return MetadataService().search_album(query, limit=limit) def search_tracks(self, query: str, limit: int = 10) -> list[dict]: """Live track search (Phase 4: via MetadataService — track.search - capability, served by the fnack.deezer-batch plugin).""" + capability; the provider chain owns the implementation.""" from services.metadata_service import MetadataService return MetadataService().search_track(query, limit=limit) @@ -157,11 +157,12 @@ def get_track_info(self, track_id: int) -> dict: return MetadataService().get_track_metadata(str(track_id)) or {} def queue_lidarr_grab(self, item_type: str, item_id: int) -> list[int]: - """Expand a Lidarr grab (a Deezer album or track id) into the local - library — creates Artist / Album / Track rows and queues one - DownloadJob per track so the queue worker downloads them like any - other track. Returns the created/queued job ids (moved verbatim from - the former `services/lidarr_service.py::_create_lidarr_grab_job`).""" + """Expand a Lidarr grab (an opaque album or track id from the + Lidarr request) into the local library — creates Artist / Album / + Track rows and queues one DownloadJob per track so the queue worker + downloads them like any other track. Returns the created/queued job + ids (moved verbatim from the former + `services/lidarr_service.py::_create_lidarr_grab_job`).""" from services.metadata_service import MetadataService from models import Album, Artist, DownloadJob, Track, db @@ -173,7 +174,7 @@ def queue_lidarr_grab(self, item_type: str, item_id: int) -> list[int]: artist_name = info.get("artist_name") or "Unknown Artist" track_title = info.get("title") or "Unknown Track" album_title = info.get("album_title") or track_title - album_deezer_id = info.get("album_id") + album_external_id = info.get("album_id") cover_url = None year = None record_type = "single" @@ -188,7 +189,7 @@ def queue_lidarr_grab(self, item_type: str, item_id: int) -> list[int]: info = _md.get_album_metadata(str(item_id)) or {} artist_name = info.get("artist_name") or "Unknown Artist" album_title = info.get("title") or "Unknown Album" - album_deezer_id = info.get("id") or item_id + album_external_id = info.get("id") or item_id cover_url = info.get("cover_url") year = info.get("year") record_type = info.get("record_type") or "album" @@ -201,7 +202,7 @@ def queue_lidarr_grab(self, item_type: str, item_id: int) -> list[int]: artist = Artist.query.filter_by(name=artist_name).first() if not artist: artist = Artist( - spotify_id=f"lidarr:{artist_name}", + external_id =f"lidarr:{artist_name}", name=artist_name, source="lidarr", monitored=True, @@ -210,22 +211,22 @@ def queue_lidarr_grab(self, item_type: str, item_id: int) -> list[int]: db.session.flush() album = None - if album_deezer_id: - album = Album.query.filter_by(artist_id=artist.id, deezer_id=str(album_deezer_id)).first() + if album_external_id: + album = Album.query.filter_by(artist_id=artist.id, external_id=str(album_external_id)).first() if not album: album = Album( artist_id=artist.id, name=album_title, year=year, cover_url=cover_url, - deezer_id=str(album_deezer_id) if album_deezer_id else None, + external_id=str(album_external_id) if album_external_id else None, record_type=record_type, ) db.session.add(album) db.session.flush() for t in tracks_to_queue: - track = Track.query.filter_by(album_id=album.id, deezer_id=str(t["id"])).first() + track = Track.query.filter_by(album_id=album.id, external_id=str(t["id"])).first() if not track: track = Track( album_id=album.id, @@ -234,7 +235,7 @@ def queue_lidarr_grab(self, item_type: str, item_id: int) -> list[int]: track_number=t.get("track_position") or 0, disc_number=t.get("disk_number") or 1, duration=t.get("duration"), - deezer_id=str(t["id"]), + external_id=str(t["id"]), status="missing", ) db.session.add(track) @@ -250,7 +251,7 @@ def queue_lidarr_grab(self, item_type: str, item_id: int) -> list[int]: album_id=album.id, artist_id=artist.id, item_type="track", - album_spotify_id=str(album.deezer_id or ""), + album_external_id=str(album.external_id or ""), album_name=album.name, album_type=album.record_type, album_url="", diff --git a/scripts/reverify_library.py b/scripts/reverify_library.py index c238462..40f178a 100644 --- a/scripts/reverify_library.py +++ b/scripts/reverify_library.py @@ -2,7 +2,7 @@ """fnack library re-verification tool. Checks every downloaded track against the OFFICIAL duration (fetched from the -Deezer public API, no authentication) and marks confirmed mismatches as failed +metadata provider chain) and marks confirmed mismatches as failed so they can be inspected or re-downloaded. Files are NOT deleted. Also repairs the stored track duration with the official value (an earlier bug @@ -57,12 +57,14 @@ def main() -> int: skipped += 1 continue - # Official expected duration: Deezer first, else the stored value + # Official expected duration: ask the metadata chain, else the + # stored value. The provider chain interprets the opaque external + # id (non-matching providers return nothing gracefully). official = None - if t.deezer_id and str(t.deezer_id).isdigit(): + if t.external_id: try: from services.metadata_service import MetadataService - info = MetadataService().get_track_metadata(str(t.deezer_id)) or {} + info = MetadataService().get_track_metadata(str(t.external_id)) or {} official = info.get("duration") except Exception: official = None diff --git a/services/import_service.py b/services/import_service.py index 890db09..fcebea2 100644 --- a/services/import_service.py +++ b/services/import_service.py @@ -1,4 +1,4 @@ -"""Interactive Root Folder Importer Service: Discovers local artist folders, matches with Deezer discographies, and maps local files.""" +"""Interactive Root Folder Importer Service: Discovers local artist folders, matches them with the metadata provider chain's discographies, and maps local files.""" import logging import re @@ -16,13 +16,13 @@ AUDIO_EXTENSIONS = {".flac", ".mp3", ".m4a", ".opus", ".ogg", ".wav", ".aac"} # ── Scan caches ──────────────────────────────────────────────────────────── -# Scanning a big /music root is slow mostly because of the Deezer network +# Scanning a big /music root is slow mostly because of the metadata network # lookups (one search per un-imported folder) and mutagen tag sampling. Cache # both so repeated scans (page refresh, after each import) stay fast. _SCAN_TTL_SECONDS = 30.0 # whole-scan result cache -_SEARCH_TTL_SECONDS = 600.0 # Deezer artist-search cache (per normalized name) +_SEARCH_TTL_SECONDS = 600.0 # artist-search cache (per normalized name) _scan_cache: dict = {} # str(music_path) -> {"ts": float, "candidates": list} -_deezer_search_cache: dict = {} # normalized query -> {"ts": float, "results": list} +_artist_search_cache: dict = {} # normalized query -> {"ts": float, "results": list} def _invalidate_scan_cache() -> None: @@ -36,18 +36,18 @@ def _cached_search_artist(query: str, limit: int = 6) -> list: if not key: return [] now = time.monotonic() - hit = _deezer_search_cache.get(key) + hit = _artist_search_cache.get(key) if hit and now - hit["ts"] < _SEARCH_TTL_SECONDS: return hit["results"] try: - # MetadataService (artist.search capability) — served by the - # fnack.deezer-batch plugin; core has no Deezer implementation. + # MetadataService (artist.search capability) — the provider chain + # owns the search; core never names a provider. from services.metadata_service import MetadataService results = MetadataService().search_artist(query, limit=limit) except Exception as e: logger.debug("[IMPORT] Artist search failed for '%s': %s", query, e) results = [] - _deezer_search_cache[key] = {"ts": now, "results": results} + _artist_search_cache[key] = {"ts": now, "results": results} return results @@ -75,7 +75,7 @@ def _scan_one_folder( existing_by_name: dict, existing_by_id: dict, ) -> Optional[dict]: - """Scan a single artist folder (tag sampling + Deezer suggestion).""" + """Scan a single artist folder (tag sampling + provider suggestion).""" audio_files = [f for f in folder.rglob("*") if f.is_file() and f.suffix.lower() in AUDIO_EXTENSIONS] if not audio_files: return None @@ -121,7 +121,7 @@ def _scan_one_folder( or (top_artist and existing_by_name.get(_normalize(top_artist))) ) - # Search Deezer suggestions if not imported (cached + only when needed) + # Search provider suggestions if not imported (cached + only when needed) suggested = None alternate_matches = [] if not already_id: @@ -164,7 +164,8 @@ def _score_candidate(c): "album_count": len(albums_set) or 1, "is_already_imported": bool(already_id), "existing_artist_id": already_id, - "suggested_deezer": suggested, + "suggested_deezer": suggested, # legacy wire alias (frontend contract) + "suggested_external_id": suggested, "alternate_matches": alternate_matches, } @@ -174,7 +175,7 @@ def scan_root_folder_candidates(music_path: str) -> list[dict]: Scan root music folder and return list of artist folder candidates for interactive import. Uses multi-factor scoring (folder name, albumartist tags, track tags) to accurately identify artists. - The folder walks run concurrently (bounded greenlet pool) and Deezer lookups are cached, + The folder walks run concurrently (bounded greenlet pool) and metadata lookups are cached, so large libraries scan in a few seconds instead of a network round-trip per folder. """ root = Path(music_path) @@ -190,7 +191,7 @@ def scan_root_folder_candidates(music_path: str) -> list[dict]: existing_artists = Artist.query.all() existing_by_name = {_normalize(a.name): a.id for a in existing_artists} - existing_by_id = {a.spotify_id: a.id for a in existing_artists if a.spotify_id} + existing_by_id = {a.external_id: a.id for a in existing_artists if a.external_id} folders = [ folder for folder in sorted(root.iterdir()) @@ -205,7 +206,7 @@ def scan_root_folder_candidates(music_path: str) -> list[dict]: if cand: results.append(cand) else: - # Parallel scan: the network-bound Deezer lookups and mutagen reads are + # Parallel scan: the network-bound metadata lookups and mutagen reads are # greenlet-safe under the app's gevent monkey-patching (requests is # patched), so we can process several folders concurrently. try: @@ -235,11 +236,12 @@ def scan_root_folder_candidates(music_path: str) -> list[dict]: def import_artist_folder( music_path: str, folder_name: str, - deezer_artist_id: Optional[int] = None, + external_id: Optional[int] = None, filter_options: Optional[dict] = None, ) -> dict: """ - Import an artist folder: Pulls Deezer discography and maps existing local audio files. + Import an artist folder: pulls the provider-chain discography for the artist's + external identity and maps existing local audio files. """ root = Path(music_path) artist_dir = root / folder_name @@ -252,29 +254,28 @@ def import_artist_folder( audio_files = [f for f in artist_dir.rglob("*") if f.is_file() and f.suffix.lower() in AUDIO_EXTENSIONS] opts = filter_options or {} - # If deezer_artist_id not provided, try to search (Phase 3: application + # If external_id not provided, try to search (Phase 3: application # service — artist.search capability). - if not deezer_artist_id: + if not external_id: try: from services.metadata_service import MetadataService results = MetadataService().search_artist(folder_name, limit=1) except Exception: results = [] if results: - deezer_artist_id = results[0]["id"] + external_id = results[0]["id"] - if not deezer_artist_id: - return {"error": f"Could not resolve Deezer artist for folder '{folder_name}'"} + if not external_id: + return {"error": f"Could not resolve an artist match for folder '{folder_name}'"} # Fetch discography (Phase 3: MetadataService owns the artist.discography - # chain — Deezer p10 authoritative, then MusicBrainz p20 / Spotify p30 / - # iTunes p40, first usable discography wins. No direct services.deezer - # call, no hidden fallback). + # chain — providers are tried in registry priority order; the provider + # chain interprets the opaque external identity. No hidden fallback). disco = None try: from services.metadata_service import MetadataService disco = MetadataService().get_artist_discography( - str(deezer_artist_id), + str(external_id), artist_name=folder_name, filter_remixes=opts.get("filter_remixes", True), filter_lofi=opts.get("filter_lofi", True), @@ -309,16 +310,16 @@ def import_artist_folder( logger.debug("[MB] enrichment skipped", exc_info=True) artist_name = disco["artist_name"] - artist = Artist.query.filter_by(spotify_id=str(deezer_artist_id)).first() + artist = Artist.query.filter_by(external_id=str(external_id)).first() if not artist: # Check by exact name match before creating artist = Artist.query.filter(db.func.lower(Artist.name) == artist_name.lower()).first() if artist: - artist.spotify_id = str(deezer_artist_id) + artist.external_id = str(external_id) artist.image_url = disco["artist_image"] else: artist = Artist( - spotify_id=str(deezer_artist_id), + external_id=str(external_id), name=artist_name, image_url=disco["artist_image"], source="folder", @@ -340,14 +341,14 @@ def import_artist_folder( isrc_lookup: dict[str, Track] = {} for a in disco.get("albums", []): - album = Album.query.filter_by(artist_id=artist.id, deezer_id=str(a["id"])).first() + album = Album.query.filter_by(artist_id=artist.id, external_id=str(a["id"])).first() if not album: album = Album( artist_id=artist.id, name=a["title"], year=a.get("year"), cover_url=a.get("cover_url"), - deezer_id=str(a["id"]), + external_id=str(a["id"]), record_type=a.get("record_type", "album"), ) db.session.add(album) @@ -362,7 +363,7 @@ def import_artist_folder( album.mb_year = a["mb_year"] for t in a.get("tracks", []): - track = Track.query.filter_by(album_id=album.id, deezer_id=str(t["id"])).first() + track = Track.query.filter_by(album_id=album.id, external_id=str(t["id"])).first() if not track: track = Track( album_id=album.id, @@ -372,7 +373,7 @@ def import_artist_folder( disc_number=t.get("disk_number", 1), duration=t.get("duration"), isrc=t.get("isrc"), - deezer_id=str(t["id"]), + external_id=str(t["id"]), status="missing", ) db.session.add(track) diff --git a/services/metadata_service.py b/services/metadata_service.py index bfbfc76..43b6ada 100644 --- a/services/metadata_service.py +++ b/services/metadata_service.py @@ -4,8 +4,8 @@ metadata resolution. Each method resolves ONE capability through the capability registry (enabled, priority-ordered providers) and invokes the best provider through the manager's ProviderExecutor boundary. Core never -imports services.spotify/deezer/musicbrainz/itunes and never names a -provider. +imports a provider implementation and never names a provider; providers are +selected by capability and registry priority alone. Methods (each resolves a capability): @@ -109,8 +109,9 @@ def resolve_track_url( def search_artist(self, name: str, limit: int = 10) -> list[dict]: """Search artists by name. First provider returning results wins - (Deezer p10 is authoritative today). CapabilityUnavailable when no - artist.search provider is enabled.""" + (providers are tried in registry priority order — the ordering is + configuration/registry policy, not core). CapabilityUnavailable when + no artist.search provider is enabled.""" providers = self._providers_for(ARTIST_SEARCH, "search_artist") for provider in providers: try: @@ -131,14 +132,15 @@ def get_artist_discography( **filters, ) -> dict: """Fetch an artist's discography. First provider returning a usable - discography wins. `provider_artist_id` is the key the PRIMARY - provider understands (Deezer id today); `artist_name` is passed to - providers keyed by name (iTunes) — the service is provider-neutral, - it does not branch on provider IDs. + discography wins. `provider_artist_id` is the opaque external + identity the selected provider understands — the provider owns + parsing/conversion; core never interprets it. `artist_name` is passed + to providers keyed by name. The service is provider-neutral and does + not branch on provider IDs. `filters` (filter_remixes/filter_lofi/...) are passed to providers - that accept them; providers that don't ignore them (the primary - Deezer provider applies them). CapabilityUnavailable when no + that accept them; providers that don't ignore them. Provider-specific + filter semantics live in the provider. CapabilityUnavailable when no artist.discography provider is enabled. """ providers = self._providers_for(ARTIST_DISCOGRAPHY, "get_artist_discography") diff --git a/services/queue_service.py b/services/queue_service.py index 86ddb17..13ba4e0 100644 --- a/services/queue_service.py +++ b/services/queue_service.py @@ -439,7 +439,7 @@ def queue_track(app: Flask, track_id: int, source: str = "manual") -> Optional[D album_id=album.id, artist_id=artist.id, item_type="track", - album_spotify_id=str(album.deezer_id or ""), + album_external_id=str(album.external_id or ""), album_name=album.name, album_type=album.record_type, album_url="", @@ -597,7 +597,7 @@ def _process_track_job(app: Flask, socketio: SocketIO, job_id: int): artist_id = artist.id if artist else 0 track_num = track.track_number or 0 disc_num = track.disc_number or 1 - track_deezer_id = track.deezer_id + track_external_id = track.external_id quality_setting = _get_setting(app, "spotiflac_quality", "LOSSLESS") fallback_format = _get_setting(app, "ytdlp_format") or _get_setting(app, "spotdl_format", "flac") @@ -616,13 +616,14 @@ def _process_track_job(app: Flask, socketio: SocketIO, job_id: int): flagged_caution = None # set when AcoustID flags a kept-but-different file # Auto-resolve ISRC and genre from a metadata provider when missing - # (through MetadataService, capability-based — track.metadata served - # by the fnack.deezer-batch plugin). + # (through MetadataService, capability-based — track.metadata is + # resolved via the registry; the provider that owns the external id + # interprets it). track_genre = track.genre or None - if (not isrc or not track_genre) and track_deezer_id: + if (not isrc or not track_genre) and track_external_id: try: from services.metadata_service import MetadataService - t_info = MetadataService().get_track_metadata(str(track_deezer_id)) or {} + t_info = MetadataService().get_track_metadata(str(track_external_id)) or {} if t_info.get("isrc") and not isrc: isrc = t_info["isrc"] track.isrc = isrc @@ -636,7 +637,7 @@ def _process_track_job(app: Flask, socketio: SocketIO, job_id: int): if t_info.get("genre"): logger.info("[QUEUE] Auto-resolved genre '%s' for '%s - %s'", track_genre, artist_name, track_title) except Exception as ie: - logger.debug("[QUEUE] Deezer ISRC/genre lookup failed for track %d: %s", track_id, ie) + logger.debug("[QUEUE] ISRC/genre metadata lookup failed for track %d: %s", track_id, ie) if job_id in cancel_requested_jobs: _handle_cancellation(app, socketio, job_id, track_id) @@ -670,7 +671,7 @@ def _process_track_job(app: Flask, socketio: SocketIO, job_id: int): try: # Step 0: Cross-Artist & Featuring Deduplication - # If this track (or a matching version with same ISRC / Deezer ID / Title + Duration) was already downloaded under another artist or album, reuse the existing audio file without redownloading. + # If this track (or a matching version with same ISRC / external id / title + duration) was already downloaded under another artist or album, reuse the existing audio file without redownloading. if job_id not in cancel_requested_jobs: existing_match_path = None existing_match_size = None @@ -689,11 +690,11 @@ def _process_track_job(app: Flask, socketio: SocketIO, job_id: int): Track.local_path != "", ).first() - # 2. Match by Deezer ID - if not existing_match and track_deezer_id: + # 2. Match by external id + if not existing_match and track_external_id: existing_match = Track.query.filter( Track.id != track_id, - Track.deezer_id == track_deezer_id, + Track.external_id == track_external_id, Track.is_downloaded == True, Track.local_path.isnot(None), Track.local_path != "", @@ -803,7 +804,7 @@ def _process_track_job(app: Flask, socketio: SocketIO, job_id: int): isrc=isrc, duration=expected_duration, spotify_url=spotify_url, - deezer_id=track_deezer_id, + external_id=track_external_id, disc_number=disc_num or 1, # captured before the dedup app_context closed track_number=track_num, ) @@ -1230,7 +1231,7 @@ def download_manual_match_track( disc_num = track.disc_number or 1 expected_duration = track.duration track_isrc = track.isrc - track_deezer_id = track.deezer_id + track_external_id = track.external_id track_genre = track.genre or None # Capture plain IDs before session expiry/detachment (safe to use outside app context) album_id = album.id if album else 0 @@ -1337,7 +1338,8 @@ def download_manual_match_track( else: last_err = direct_err - # 3. Deezer URL + # 3. User-pasted Deezer track link (manual match; the + # metadata lookup below goes through the capability chain) elif "deezer.com/track/" in target_input: m = re.search(r"deezer\.com/track/(\d+)", target_input) if m: diff --git a/services/tag_normalization_service.py b/services/tag_normalization_service.py index acb8aef..7357415 100644 --- a/services/tag_normalization_service.py +++ b/services/tag_normalization_service.py @@ -10,7 +10,7 @@ Also merges duplicate database album rows (same artist, album name equal ignoring case/whitespace, and one album's track titles a subset of the -other's) — Deezer exposes many releases under two IDs (album + single with +other's) — the metadata chain exposes many releases under two IDs (album + single with identical content), which used to split albums inside Navidrome forever. """ @@ -84,9 +84,14 @@ def _read_simple_tag(mf, keys): def _source_rank(al) -> int: - """Prefer real Deezer albums over iTunes fallback rows (often misspelled).""" - dz = str(al.deezer_id or "") - return 1 if dz and not dz.startswith("itunes_") else 0 + """Prefer the primary metadata row over a fallback/complement row. + + The provider chain writes complement releases with a provider-prefixed + external id (the convention today is 'itunes_' for the iTunes + complement); core treats any prefixed id as the fallback row without + naming the provider.""" + ext = str(al.external_id or "") + return 1 if ext and not re.match(r"^[a-zA-Z]+_", ext) else 0 def _pick_canonical(a, b, set_a, set_b, dl_a, dl_b): @@ -181,8 +186,8 @@ def _merge_duplicate_albums(app) -> dict: Two passes, both restricted to same-artist albums: 1. exact: album names equal ignoring case + whitespace, and - 2. fuzzy: names similar (e.g. "Patander" vs "Patandar" — Deezer and - the iTunes fallback frequently spell the same release + 2. fuzzy: names similar (e.g. "Patander" vs "Patandar" — the primary + provider and its fallback frequently spell the same release differently). A merge only happens when one album's normalized track titles are a subset of the other's, so real single-vs-album pairs with different @@ -252,7 +257,7 @@ def _try_merge(albums: list): _try_merge(albums) # ---- Pass 2: fuzzy name variants within each artist ---- - # Deezer + iTunes fallback often disagree on spelling ("Patander" vs + # Primary + fallback providers often disagree on spelling ("Patander" vs # "Patandar", "Gutt" vs "Gut"); catch those when the track lists match. artist_ids = [r[0] for r in db.session.query(Album.artist_id).distinct().all()] for artist_id in artist_ids: @@ -508,7 +513,7 @@ def normalize_album_tags(app, quiet: bool = True) -> dict: stats = {"checked": 0, "retagged": 0, "moved": 0, "skipped": 0, "errors": 0} - # First collapse duplicate DB albums (Deezer exposes many releases twice). + # First collapse duplicate DB albums (the metadata chain exposes many releases twice). # Without this, files get tagged with two spellings of the same album and # Navidrome keeps showing them split no matter how many times it rescans. merge_stats = _merge_duplicate_albums(app) diff --git a/static/app.js b/static/app.js index c660207..b9ce615 100644 --- a/static/app.js +++ b/static/app.js @@ -1057,7 +1057,7 @@ function renderImportCandidates(candidates) { for (let i = 0; i < candidates.length; i++) { const c = candidates[i]; const isImported = c.is_already_imported; - const suggested = c.suggested_deezer; + const suggested = c.suggested_external_id || c.suggested_deezer; const rowId = _folderRowId(c.folder_name); html += ` @@ -1116,10 +1116,10 @@ function _setImportRowStatus(folderName, html) { function _importItemFor(folderName) { const cand = _importCandidates.find(c => c.folder_name === folderName); - const suggested = cand ? cand.suggested_deezer : null; + const suggested = cand ? (cand.suggested_external_id || cand.suggested_deezer) : null; return { folder_name: folderName, - deezer_id: suggested && suggested.id ? suggested.id : null, + external_id: suggested && suggested.id ? suggested.id : null, }; } @@ -1172,7 +1172,8 @@ async function importArtistFolder(idx, btnEl) { btnEl.innerHTML = 'Queued...'; } const item = _importItemFor(folderName); - if (cand.suggested_deezer && cand.suggested_deezer.id) item.deezer_id = cand.suggested_deezer.id; + const suggestedCand = cand.suggested_external_id || cand.suggested_deezer; + if (suggestedCand && suggestedCand.id) item.external_id = suggestedCand.id; await _postBulkImport([item], () => { showToast(`Import queued for '${folderName}' in the background.`, 'info'); _setImportRowStatus(folderName, 'Queued'); @@ -1319,13 +1320,13 @@ function renderSelectArtistResults(results) { container.innerHTML = html; } -function confirmSelectArtistForFolder(deezerId, name, imageUrl, nbAlbums) { +function confirmSelectArtistForFolder(externalId, name, imageUrl, nbAlbums) { if (!_selectedImportFolder) return; const cand = _importCandidates.find(c => c.folder_name === _selectedImportFolder); if (cand) { - cand.suggested_deezer = { - id: deezerId, + cand.suggested_external_id = cand.suggested_deezer = { + id: externalId, name: name, image_url: imageUrl, nb_album: nbAlbums, diff --git a/tests/architecture/test_background_download_app_context.py b/tests/architecture/test_background_download_app_context.py index bf16b43..bdf6f94 100644 --- a/tests/architecture/test_background_download_app_context.py +++ b/tests/architecture/test_background_download_app_context.py @@ -140,7 +140,7 @@ def _test_runtime_background_download() -> None: import tempfile as _tempfile _tmp_music = Path(_tempfile.mkdtemp(prefix="fnack-music-")) db.session.add(AppSetting(key="music_path", value=str(_tmp_music))) - artist = Artist(spotify_id="ctx-test", name="Ctx Artist") + artist = Artist(external_id="ctx-test", name="Ctx Artist") db.session.add(artist) db.session.flush() album = Album(artist_id=artist.id, name="Ctx Album") diff --git a/tests/architecture/test_provider_neutral_identity.py b/tests/architecture/test_provider_neutral_identity.py new file mode 100644 index 0000000..9e0fb1a --- /dev/null +++ b/tests/architecture/test_provider_neutral_identity.py @@ -0,0 +1,189 @@ +"""Architecture/parity test: provider-neutral identity (final cleanup). + +Proves core artist/metadata workflows operate on opaque provider-neutral +external identities and never assume a Deezer (or integer) id: + +A. A fake provider serving artist.search with a NON-integer, non-Deezer + external id ("fake-artist-123") flows through MetadataService untouched. +B. A second fake provider whose identity format differs ("artist:abc:def") + is treated as opaque provider identity too. +C. The fnack.deezer-batch plugin converts the provider-neutral input into + the Deezer-specific representation it needs (int parsing) INSIDE the + plugin; foreign ids are declined gracefully (None), never crash. +D. Removing the Deezer provider: with only the fake provider(s) enabled the + capability chain still works; with NO provider the service raises + structured CapabilityUnavailable — no hidden Deezer fallback. + +Run from the repo root: + + .venv/bin/python tests/architecture/test_provider_neutral_identity.py +""" + +import importlib.util +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent.parent +sys.path.insert(0, str(ROOT)) + + +def _fake_manager(*providers): + """Minimal manager stand-in (same shape as test_metadata_service).""" + import asyncio + import inspect + from collections import defaultdict + + caps = defaultdict(list) + for p in providers: + if getattr(p, "capability_id", None): + caps[p.capability_id].append(p) + + class _FakeManager: + def has_capability(self, capability): + return capability in caps and bool(caps[capability]) + + def get_capability_providers(self, capability): + return list(caps.get(capability, [])) + + def invoke_provider(self, provider, method_name, *args, timeout=None, **kwargs): + method = getattr(provider, method_name, None) + if method is None: + return None + r = method(*args, **kwargs) + if inspect.isawaitable(r): + return asyncio.run(r) + return r + + return _FakeManager() + + +class _IdentityProvider: + """Fake metadata provider returning results keyed by an opaque external + id (the id is never parsed or interpreted by the harness).""" + + def __init__(self, capability_id, external_id): + self.capability_id = capability_id + self._external_id = external_id + + @property + def manifest(self): + class _M: + id = "com.example.fake" + name = "Fake" + return _M() + + def search_artist(self, name): + # Provider-specific identity: opaque string, arbitrary format. + return [{"id": self._external_id, "name": name}] + + def get_artist_discography(self, provider_artist_id, **filters): + if provider_artist_id != self._external_id: + return {"artist_name": "", "albums": []} + return {"artist_name": "Fake Artist", + "albums": [{"id": "fake-album-9", "title": "Fake Album", + "tracks": [{"id": "fake-track-7", "title": "Fake Song", + "duration": 200.0, "isrc": "FAKE1"}]}]} + + +def _service(*providers): + from services.metadata_service import MetadataService + return MetadataService(manager=_fake_manager(*providers)) + + +def test_a_generic_non_integer_artist_identity_flows_through() -> None: + """Test A: an artist.search result with external id 'fake-artist-123' + (non-integer, non-Deezer) is returned by core without conversion.""" + svc = _service(_IdentityProvider("artist.search", "fake-artist-123"), + _IdentityProvider("artist.discography", "fake-artist-123")) + results = svc.search_artist("Someone") + assert results and results[0]["id"] == "fake-artist-123", results + assert results[0]["name"] == "Someone" + # The identity round-trips into the discography capability unchanged. + disco = svc.get_artist_discography("fake-artist-123", artist_name="Someone") + assert disco["albums"][0]["id"] == "fake-album-9" + + +def test_b_alternate_provider_identity_format_is_opaque() -> None: + """Test B: a second provider's identity format ('artist:abc:def') is + treated as opaque — core never parses, int()s, or prefixes it.""" + svc = _service(_IdentityProvider("artist.search", "artist:abc:def"), + _IdentityProvider("artist.discography", "artist:abc:def")) + results = svc.search_artist("X") + assert results and results[0]["id"] == "artist:abc:def", results + # The matching opaque identity round-trips into the discography capability. + disco = svc.get_artist_discography("artist:abc:def", artist_name="X") + assert disco["albums"][0]["id"] == "fake-album-9" + # An id from ANOTHER provider returns the structured empty shape — the + # provider declines it; never an exception or a hidden fallback. + disco2 = svc.get_artist_discography("some-other-provider-id") + assert disco2 == {"artist_name": "", "albums": []} + + +def test_c_deezer_plugin_owns_id_conversion() -> None: + """Test C: the Deezer plugin converts the provider-neutral input into the + Deezer-specific int representation inside the plugin; non-Deezer ids are + declined (None) — core never does this conversion.""" + plugin_dir = ROOT / "bundled_plugins" / "fnack.deezer-batch" + sys.path.insert(0, str(plugin_dir)) # plugin.py imports its sibling `deezer` + try: + mod_path = plugin_dir / "plugin.py" + spec = importlib.util.spec_from_file_location("fnack_deezer_batch_plugin_under_test", mod_path) + mod = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = mod + spec.loader.exec_module(mod) + finally: + sys.path.remove(str(plugin_dir)) + + seen = [] + # Numeric Deezer id: parsed to the int the Deezer API requires. + result = mod.DeezerBatchProvider._deezer_id_lookup( + "27", lambda i: seen.append(i) or ("ok", i)) + assert seen == [27] and result == ("ok", 27), (seen, result) + # Foreign / non-numeric id: gracefully declined, NOT parsed as Deezer. + seen2 = [] + declined = mod.DeezerBatchProvider._deezer_id_lookup( + "fake-artist-123", lambda i: seen2.append(i) or ("ok", i)) + assert declined is None and seen2 == [], (declined, seen2) + + +def test_d_provider_removal_and_zero_providers() -> None: + """Test D: without the Deezer provider, core still works when another + provider implements the capability; with NO provider it raises + CapabilityUnavailable — no hidden Deezer fallback anywhere.""" + from services.metadata_service import CapabilityUnavailable + + fake = _IdentityProvider("artist.search", "fake-artist-123") + fake_disco = _IdentityProvider("artist.discography", "fake-artist-123") + svc = _service(fake, fake_disco) + assert svc.search_artist("Someone")[0]["id"] == "fake-artist-123" + assert svc.get_artist_discography("fake-artist-123")["albums"][0]["id"] == "fake-album-9" + + # No providers at all -> structured CapabilityUnavailable per capability. + empty = _service() + for call, cap in [ + (lambda: empty.search_artist("A"), "artist.search"), + (lambda: empty.get_artist_discography("1"), "artist.discography"), + (lambda: empty.resolve_track_url("T", "A"), "track.resolve"), + ]: + try: + call() + except CapabilityUnavailable as e: + assert e.capability == cap, f"expected {cap}, got {e.capability}" + else: + raise AssertionError(f"expected CapabilityUnavailable for {cap}") + + # Core source never falls back to a direct Deezer call. + import re + for path in [ROOT / "services" / "metadata_service.py", + ROOT / "services" / "queue_service.py", + ROOT / "app.py"]: + src = path.read_text(encoding="utf-8") + assert not re.search(r"services\.deezer_service|import deezer\b", src), path + + +if __name__ == "__main__": + test_a_generic_non_integer_artist_identity_flows_through() + test_b_alternate_provider_identity_format_is_opaque() + test_c_deezer_plugin_owns_id_conversion() + test_d_provider_removal_and_zero_providers() + print("test_provider_neutral_identity: PASSED") diff --git a/tests/run_smoke_test.py b/tests/run_smoke_test.py index 24bdf3c..d463986 100644 --- a/tests/run_smoke_test.py +++ b/tests/run_smoke_test.py @@ -32,7 +32,7 @@ db.create_all() # Real fnack Track requires an Album (and an Artist for the relationship). - artist = Artist(spotify_id="test-artist-1", name="Test Artist") + artist = Artist(external_id="test-artist-1", name="Test Artist") db.session.add(artist) db.session.flush() album = Album(artist_id=artist.id, name="Test Album") diff --git a/wayfinder/plugin-architecture-map.md b/wayfinder/plugin-architecture-map.md index 6affd74..97b93f8 100644 --- a/wayfinder/plugin-architecture-map.md +++ b/wayfinder/plugin-architecture-map.md @@ -254,6 +254,23 @@ phase. fnack-plugins gains `tests/test_manifest_index_parity.py` (manifest ↔ index parity, deterministic) and its README gains the missing fnack.lidarr row. Smoke + 20 architecture tests green. +- [Provider-neutral core cleanup](tickets/plugin-phase-4-hardening-deletion.md): + core external identities are no longer provider-named or provider-typed. + `Artist.spotify_id` -> `Artist.external_id`, `Album/Track.deezer_id` -> + `external_id`, `DownloadJob.album_spotify_id` -> `album_external_id` + (SQLite RENAME COLUMN migration at startup for existing DBs; data and + indexes preserved). Core never int()s/parses the opaque identity — the + provider chain owns conversion (deezer-batch int()s inside the plugin and + declines foreign ids). artist.search/add/sync/import routes and the + scheduler take the opaque external id; MetadataService docstrings no longer + call any provider authoritative; legacy wire aliases (deezer_id / + suggested_deezer) kept + documented for old clients/frontend, then renamed + in the frontend to external_id. New + tests/architecture/test_provider_neutral_identity.py (fake non-integer and + colon-prefixed identities through the capability chain; deezer plugin id + conversion; provider-removal + CapabilityUnavailable, no hidden fallback). + docs/architecture.md gained a provider-neutral identity section. Smoke + 22 + architecture tests green. - [P0 follow-up: app context by construction + settings-modal UI](tickets/plugin-phase-4-hardening-deletion.md): PR #37's wrapper->impl context fix was functionally correct but structurally fragile (head-only inner `with` invited the "provider calls