diff --git a/core/library/manual_library_match.py b/core/library/manual_library_match.py index 132ad729..ef14af9f 100644 --- a/core/library/manual_library_match.py +++ b/core/library/manual_library_match.py @@ -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.""" diff --git a/database/music_database.py b/database/music_database.py index 4e443253..2855f4ed 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -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: diff --git a/tests/test_manual_library_match.py b/tests/test_manual_library_match.py index 7804eb25..6329a01c 100644 --- a/tests/test_manual_library_match.py +++ b/tests/test_manual_library_match.py @@ -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 # --------------------------------------------------------------------------- diff --git a/web_server.py b/web_server.py index fd86abb0..51724911 100644 --- a/web_server.py +++ b/web_server.py @@ -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