Manual library match: accept non-numeric library track ids (#754)
The save endpoint coerced library_track_id with int(), which rejected every non-numeric id with "Invalid library track id". Library ids are str(ratingKey) — numeric for Plex but GUIDs/hashes for Navidrome, Jellyfin, and other Subsonic servers — and are stored in the TEXT tracks.id column, so the coercion broke manual matching on every non-Plex server. Replace the int() coercion with a normalize_library_track_id() helper that trims and rejects only empty input, passing the opaque string id straight through. Plex numeric ids are unaffected (SQLite INTEGER affinity still stores a clean numeric string as an int, so existing matches are byte-identical) and no schema migration is needed (the INTEGER column already stores non-numeric ids as text). Tests: pure-helper cases (numeric/GUID/whitespace/empty) plus a real-DB round-trip proving a GUID id saves, reads back unchanged, and enriches.
This commit is contained in:
parent
3b5a5518a6
commit
ce9ec3f6f4
4 changed files with 68 additions and 7 deletions
|
|
@ -14,12 +14,27 @@ from utils.logging_config import get_logger
|
|||
logger = get_logger("library.manual_library_match")
|
||||
|
||||
|
||||
def normalize_library_track_id(value: Any) -> Optional[str]:
|
||||
"""Normalize an incoming library_track_id to the opaque string id we store.
|
||||
|
||||
Library track ids are ``str(ratingKey)`` — numeric strings for Plex, but
|
||||
GUIDs/hashes for Jellyfin, Navidrome, and other Subsonic servers (see
|
||||
``tracks.id`` which is TEXT). They must NOT be coerced to int: doing so
|
||||
rejected every non-numeric id with "Invalid library track id". We only
|
||||
reject empty/None here; the caller validates existence against the DB.
|
||||
"""
|
||||
if value is None:
|
||||
return None
|
||||
text = str(value).strip()
|
||||
return text or None
|
||||
|
||||
|
||||
def save_match(
|
||||
db,
|
||||
profile_id: int,
|
||||
source: str,
|
||||
source_track_id: str,
|
||||
library_track_id: int,
|
||||
library_track_id: str,
|
||||
**meta,
|
||||
) -> bool:
|
||||
"""Save (insert or replace) a manual match."""
|
||||
|
|
|
|||
|
|
@ -4440,7 +4440,7 @@ class MusicDatabase:
|
|||
logger.error(f"Error creating manual_library_track_matches table: {e}")
|
||||
|
||||
def save_manual_library_match(self, profile_id: int, source: str, source_track_id: str,
|
||||
library_track_id: int, **meta) -> bool:
|
||||
library_track_id: str, **meta) -> bool:
|
||||
"""Insert or replace a manual match. meta keys: source_title, source_artist,
|
||||
source_album, source_context_json, server_source."""
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -16,6 +16,56 @@ def db(tmp_path):
|
|||
return MusicDatabase(str(tmp_path / "music.db"))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# normalize_library_track_id — regression guard for issue #754
|
||||
# ("Invalid library track id" on Jellyfin/Navidrome/Subsonic servers)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_normalize_passes_numeric_plex_id():
|
||||
# Plex ratingKeys are numeric strings — must survive unchanged (not int-ified).
|
||||
assert mlm.normalize_library_track_id("12345") == "12345"
|
||||
assert mlm.normalize_library_track_id(12345) == "12345"
|
||||
|
||||
|
||||
def test_normalize_passes_guid_id():
|
||||
# The #754 bug: Jellyfin/Navidrome ids are non-numeric. int() rejected them.
|
||||
assert mlm.normalize_library_track_id("a1b2c3d4-e5f6") == "a1b2c3d4-e5f6"
|
||||
assert mlm.normalize_library_track_id("Do I Wanna Know_opus") == "Do I Wanna Know_opus"
|
||||
|
||||
|
||||
def test_normalize_trims_whitespace():
|
||||
assert mlm.normalize_library_track_id(" guid-123 ") == "guid-123"
|
||||
|
||||
|
||||
def test_normalize_rejects_empty_and_none():
|
||||
assert mlm.normalize_library_track_id(None) is None
|
||||
assert mlm.normalize_library_track_id("") is None
|
||||
assert mlm.normalize_library_track_id(" ") is None
|
||||
|
||||
|
||||
def test_guid_library_track_id_round_trips(db):
|
||||
"""End-to-end regression for #754: a non-numeric library id must save,
|
||||
read back identically, and enrich — never get coerced or rejected."""
|
||||
guid = "a1b2c3d4e5f6-jellyfin"
|
||||
norm = mlm.normalize_library_track_id(guid)
|
||||
assert norm == guid
|
||||
ok = db.save_manual_library_match(1, "spotify", "src-track-1", norm,
|
||||
source_title="Do I Wanna Know?",
|
||||
source_artist="Arctic Monkeys")
|
||||
assert ok is True
|
||||
row = db.get_manual_library_match(1, "spotify", "src-track-1")
|
||||
assert row is not None
|
||||
assert row["library_track_id"] == guid # stored as-is, not mangled to int
|
||||
|
||||
# Enrichment must resolve the GUID against tracks.id (TEXT) without error.
|
||||
with patch.object(db, "api_get_tracks_by_ids", return_value=[
|
||||
{"title": "Do I Wanna Know?", "artist_name": "Arctic Monkeys",
|
||||
"album_title": "AM", "file_path": "/m/x.opus", "bitrate": 196}]) as mock_get:
|
||||
enriched = mlm._enrich_match(row, db)
|
||||
mock_get.assert_called_once_with([guid]) # passes the string id straight through
|
||||
assert enriched["library_title"] == "Do I Wanna Know?"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# DB-layer tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -18608,13 +18608,9 @@ def mlm_save():
|
|||
data = request.get_json(force=True) or {}
|
||||
source = data.get('source', '').strip()
|
||||
source_track_id = data.get('source_track_id', '').strip()
|
||||
library_track_id = data.get('library_track_id')
|
||||
library_track_id = mlm.normalize_library_track_id(data.get('library_track_id'))
|
||||
if not source or not source_track_id or not library_track_id:
|
||||
return jsonify({"success": False, "error": "source, source_track_id, library_track_id required"}), 400
|
||||
try:
|
||||
library_track_id = int(library_track_id)
|
||||
except (TypeError, ValueError):
|
||||
return jsonify({"success": False, "error": "Invalid library track id"}), 400
|
||||
db = get_database()
|
||||
profile_id = get_current_profile_id()
|
||||
# Validate library track exists before saving
|
||||
|
|
|
|||
Loading…
Reference in a new issue