Repository navigation
refactor(core): provider-scoped external identities + app-context cleanup - #40
Merged
Merged
Conversation
…anup Follow-up audit: persisted external identities are now provider-scoped so different providers can use the same external id for different entities without collision or ambiguity; the background entry points carry exactly one obvious app-context boundary. Identity provenance (models + migration): - Artist/Album/Track gain `provider_id` (the plugin id of the provider that supplied the external id; NULL only for self-created identities). Artists are unique per (provider_id, external_id) instead of globally unique on external_id (models + fresh create_all). - services/schema_migrations.py (new, extracted from app.py startup): idempotent SQLite migrations — RENAME identity columns, ADD provider_id, backfill (numeric ids -> the deezer-batch provider that supplied them; self-created prefixed ids stay NULL; albums/tracks inherit the artist's provider), and an artists table rebuild that drops the old single-column UNIQUE (SQLite autoindex) in favor of the per-provider unique. Verified on a copy of the live database (20 artists / 978 albums / 4729 tracks preserved) and by tests/architecture/test_schema_migrations.py. Provenance capture (core, provider-neutral): - MetadataService.search_artist annotates results with the supplying provider; get_artist_discography_with_provider returns (discography, serving provider id); get_artist_discography keeps the plain contract. - Sync/import/add-artist/set-id record provider provenance; discography dedup + prune happen inside the serving provider's namespace; queue cross-artist file reuse is provider-scoped. Lidarr/self-created rows are NULL-provider and lookup under that namespace. Frontend passes the provider through add-artist and folder import. App-context cleanup: - Removed every redundant nested `with app.app_context():` inside _process_track_job, download_manual_match_track and _sync_artist_discography_background — each function now has exactly ONE context boundary covering its entire operational body (structural test keeps asserting no body line escapes it). Type/API cleanup: - import_artist_folder external_id annotation is Optional[str]; no int()/isdigit() assumptions remain anywhere on external ids in core. Tests (all green): architecture 23/23 (incl. new test_schema_migrations.py, provider-neutral identity coexistence + provider-change/removal tests), smoke PASSED, background app-context regression PASSED. Live-verified: DB migrated in place; Pink Floyd added via search (provider recorded), discography synced (33 albums / 267 tracks all under fnack.deezer-batch), 'Another Brick in the Wall, Pt. 2' downloaded/verified through the queue worker.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the provider-neutral audit. One focused PR — no SDK redesign, no new providers, no versioning.
1. External-id provenance
Artist/Album/Tracknow persistprovider_idnext toexternal_id(the plugin id of the provider that supplied the identity; NULL only for self-created identities likeacoustid:<name>).external_id— two providers can use the same external id for different entities.services/schema_migrations.py, extracted from app.py startup, idempotent, AUTOCOMMIT): rename identity columns,ADD provider_id, backfill (numeric ids → the provider that supplied them; albums/tracks inherit the artist's provider), and an artists table rebuild dropping the old single-column UNIQUE (SQLite autoindex) in favor of the per-provider unique. Verified against a copy of the live DB (20 artists / 978 albums / 4729 tracks preserved) +tests/architecture/test_schema_migrations.py(old-schema → provider-scoped, idempotent, coexistence insertable).2. Provenance capture (core stays provider-neutral)
MetadataService.search_artistannotates results with the supplying provider; newget_artist_discography_with_providerreturns(discography, serving provider); the plainget_artist_discographykeeps its contract.providerthrough add-artist and folder import (wire aliases kept).3. App-context cleanup
with app.app_context():inside_process_track_job,download_manual_match_track,_sync_artist_discography_background— each now has exactly one context boundary covering its entire operational body. The structural regression test still asserts no body line escapes it.4. Type/API cleanup
import_artist_folderexternal_id annotation →Optional[str]; noint()/isdigit()assumptions remain on external ids in core.Tests
Architecture 23/23 (new migration test + identity coexistence / provider-change-removal tests), smoke PASSED, background app-context regression PASSED.
Live verification
Container rebuilt; live DB migrated in place. Pink Floyd added via search (provider recorded) → discography synced (33 albums / 267 tracks, all under
fnack.deezer-batch) → Another Brick in the Wall, Pt. 2 downloaded, verified, accepted through the queue worker.