Merge pull request #308 from kettui/fix/acoustid-scanner

Fix generated IDs in repair flows & acoustid_scanner NPE

core/repair_jobs/acoustid_scanner.py

@@ -84,14 +84,16 @@ class AcoustIDScannerJob(RepairJob):
             checkpoint_id = context.config_manager.get(
                 f'repair.jobs.{self.job_id}.checkpoint_id', None
             )
+        if checkpoint_id is not None:
+            checkpoint_id = str(checkpoint_id)
 
         # Build ordered list of (track_id, info) sorted by ID for deterministic order
-        track_list = sorted(db_tracks.items(), key=lambda x: x[0])
+        track_list = sorted(db_tracks.items(), key=lambda x: str(x[0]))
 
         # Skip past checkpoint if resuming
         if checkpoint_id is not None:
             original_len = len(track_list)
-            track_list = [(tid, info) for tid, info in track_list if tid > checkpoint_id]
+            track_list = [(tid, info) for tid, info in track_list if str(tid) > checkpoint_id]
             if len(track_list) < original_len:
                 logger.info("Resuming AcoustID scan from checkpoint ID %s (%d tracks remaining)",
                             checkpoint_id, len(track_list))
@@ -258,6 +260,13 @@ class AcoustIDScannerJob(RepairJob):
             """)
             for row in cursor.fetchall():
                 track_id = row[0]
+                if track_id is None:
+                    logger.warning(
+                        "Skipping track row with null ID while loading AcoustID scan candidates: %s",
+                        row[3] or "<unknown file>",
+                    )
+                    continue
+                track_id = str(track_id)
                 tracks[track_id] = {
                     'title': row[1] or '',
                     'artist': row[2] or '',

core/repair_jobs/unknown_artist_fixer.py

@@ -8,6 +8,7 @@ import os
 import re
 import shutil
 import sys
+import uuid
 
 from core.metadata_service import get_client_for_source, get_primary_source, get_source_priority
 from core.repair_jobs import register_job
@@ -529,8 +530,16 @@ class UnknownArtistFixerJob(RepairJob):
                 if artist_row:
                     new_artist_id = artist_row[0]
                 else:
-                    cursor.execute("INSERT INTO artists (name) VALUES (?)", (corrected_artist,))
+                    safe_artist_name = re.sub(
-                    new_artist_id = cursor.lastrowid
+                        r'[^A-Za-z0-9_.-]+',
+                        '_',
+                        corrected_artist.strip() or 'unknown'
+                    )
+                    new_artist_id = f"artist_local_{safe_artist_name}_{uuid.uuid4().hex[:8]}"
+                    cursor.execute(
+                        "INSERT INTO artists (id, name) VALUES (?, ?)",
+                        (new_artist_id, corrected_artist),
+                    )
 
                 # Update track's artist_id and file_path
                 cursor.execute("""

core/repair_worker.py

@@ -14,6 +14,7 @@ import shutil
 import sys
 import threading
 import time
+import uuid
 from difflib import SequenceMatcher
 from datetime import datetime, timedelta
 from typing import Any, Dict, List, Optional, Tuple
@@ -1486,8 +1487,16 @@ class RepairWorker:
                 row = cursor.fetchone()
                 new_artist_id = row[0] if row else None
                 if not new_artist_id:
-                    cursor.execute("INSERT INTO artists (name) VALUES (?)", (corrected_artist,))
+                    safe_artist_name = re.sub(
-                    new_artist_id = cursor.lastrowid
+                        r'[^A-Za-z0-9_.-]+',
+                        '_',
+                        corrected_artist.strip() or 'unknown'
+                    )
+                    new_artist_id = f"artist_local_{safe_artist_name}_{uuid.uuid4().hex[:8]}"
+                    cursor.execute(
+                        "INSERT INTO artists (id, name) VALUES (?, ?)",
+                        (new_artist_id, corrected_artist),
+                    )
 
                 cursor.execute("UPDATE tracks SET artist_id = ?, file_path = ? WHERE id = ?",
                                (new_artist_id, final_path, track_id))
@@ -1601,9 +1610,18 @@ class RepairWorker:
                 if row:
                     cursor.execute("UPDATE tracks SET artist_id = ? WHERE id = ?", (row[0], track_id))
                 else:
-                    cursor.execute("INSERT INTO artists (name) VALUES (?)", (aid_artist,))
+                    safe_artist_name = re.sub(
+                        r'[^A-Za-z0-9_.-]+',
+                        '_',
+                        aid_artist.strip() or 'unknown'
+                    )
+                    new_artist_id = f"artist_local_{safe_artist_name}_{uuid.uuid4().hex[:8]}"
+                    cursor.execute(
+                        "INSERT INTO artists (id, name) VALUES (?, ?)",
+                        (new_artist_id, aid_artist),
+                    )
                     cursor.execute("UPDATE tracks SET artist_id = ? WHERE id = ?",
-                                   (cursor.lastrowid, track_id))
+                                   (new_artist_id, track_id))
             conn.commit()
             conn.close()
         except Exception as e:
@@ -2140,6 +2158,12 @@ class RepairWorker:
                     # COPY: duplicate file, create new DB record
                     shutil.copy2(src_path, target_path)
                     action = 'copied'
+                    source_track_id = re.sub(
+                        r'[^A-Za-z0-9_.-]+',
+                        '_',
+                        str(getattr(candidate, 'id', 'unknown'))
+                    )
+                    new_track_id = f"album_fill_{source_track_id}_{uuid.uuid4().hex[:8]}"
 
                     conn = self.db._get_connection()
                     cursor = conn.cursor()
@@ -2149,10 +2173,10 @@ class RepairWorker:
                     target_artist_id = artist_row[0] if artist_row else candidate.artist_id
 
                     cursor.execute("""
-                        INSERT INTO tracks (album_id, artist_id, title, track_number, duration,
+                        INSERT INTO tracks (id, album_id, artist_id, title, track_number, duration,
                                             file_path, bitrate, created_at, updated_at)
-                        VALUES (?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
+                        VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
-                    """, (album_id, target_artist_id, track_name, track_number,
+                    """, (new_track_id, album_id, target_artist_id, track_name, track_number,
                           candidate.duration, target_path, candidate.bitrate))
                     conn.commit()
 

tests/test_acoustid_scanner.py

@@ -0,0 +1,88 @@
+from types import SimpleNamespace
+
+from core.repair_jobs.acoustid_scanner import AcoustIDScannerJob
+
+
+class _FakeCursor:
+    def __init__(self, rows):
+        self._rows = rows
+        self.executed = []
+
+    def execute(self, query, params=None):
+        self.executed.append((query, params))
+        return self
+
+    def fetchall(self):
+        return self._rows
+
+    def fetchone(self):
+        return None
+
+
+class _FakeConnection:
+    def __init__(self, rows):
+        self._cursor = _FakeCursor(rows)
+
+    def cursor(self):
+        return self._cursor
+
+    def close(self):
+        pass
+
+
+def _make_context(rows):
+    conn = _FakeConnection(rows)
+    config_manager = SimpleNamespace(
+        get=lambda key, default=None: default,
+        set=lambda *args, **kwargs: None,
+    )
+    db = SimpleNamespace(_get_connection=lambda: conn)
+    return SimpleNamespace(
+        db=db,
+        transfer_folder="/music",
+        config_manager=config_manager,
+        acoustid_client=object(),
+        create_finding=None,
+        report_progress=lambda **kwargs: None,
+        update_progress=lambda *args, **kwargs: None,
+        check_stop=lambda: False,
+        wait_if_paused=lambda: False,
+        sleep_or_stop=lambda *args, **kwargs: False,
+    )
+
+
+def test_load_db_tracks_skips_null_ids_and_normalizes_track_ids():
+    job = AcoustIDScannerJob()
+    context = _make_context([
+        (None, "Broken Track", "Artist", "/music/broken.flac", 1, "Album", None, None),
+        (42, "Good Track", "Artist", "/music/good.flac", 2, "Album", "album-thumb", "artist-thumb"),
+    ])
+
+    tracks = job._load_db_tracks(context)
+
+    assert list(tracks.keys()) == ["42"]
+    assert tracks["42"]["title"] == "Good Track"
+    assert tracks["42"]["artist"] == "Artist"
+
+
+def test_scan_handles_mixed_track_id_types(monkeypatch):
+    job = AcoustIDScannerJob()
+    context = _make_context([
+        (None, "Broken Track", "Artist", "/music/broken.flac", 1, "Album", None, None),
+        (42, "Good Track", "Artist", "/music/good.flac", 2, "Album", "album-thumb", "artist-thumb"),
+    ])
+
+    monkeypatch.setattr(job, "_resolve_path", lambda file_path, _context: file_path)
+
+    scanned_track_ids = []
+
+    def fake_scan_file(fpath, track_id, expected, acoustid_client, context, result,
+                       fp_threshold, title_threshold, artist_threshold):
+        scanned_track_ids.append(track_id)
+
+    monkeypatch.setattr(job, "_scan_file", fake_scan_file)
+
+    result = job.scan(context)
+
+    assert result.scanned == 1
+    assert scanned_track_ids == ["42"]

tests/test_repair_worker_album_fill.py

@@ -0,0 +1,145 @@
+import sqlite3
+import sys
+import types
+from pathlib import Path
+from types import SimpleNamespace
+
+
+if "spotipy" not in sys.modules:
+    spotipy = types.ModuleType("spotipy")
+
+    class _DummySpotify:
+        def __init__(self, *args, **kwargs):
+            pass
+
+    oauth2 = types.ModuleType("spotipy.oauth2")
+
+    class _DummyOAuth:
+        def __init__(self, *args, **kwargs):
+            pass
+
+    spotipy.Spotify = _DummySpotify
+    oauth2.SpotifyOAuth = _DummyOAuth
+    oauth2.SpotifyClientCredentials = _DummyOAuth
+    spotipy.oauth2 = oauth2
+    sys.modules["spotipy"] = spotipy
+    sys.modules["spotipy.oauth2"] = oauth2
+
+if "config.settings" not in sys.modules:
+    config_pkg = types.ModuleType("config")
+    settings_mod = types.ModuleType("config.settings")
+
+    class _DummyConfigManager:
+        def get(self, key, default=None):
+            return default
+
+    settings_mod.config_manager = _DummyConfigManager()
+    config_pkg.settings = settings_mod
+    sys.modules["config"] = config_pkg
+    sys.modules["config.settings"] = settings_mod
+
+from core.repair_worker import RepairWorker
+
+
+def test_perform_album_fill_copy_branch_generates_track_id(tmp_path, monkeypatch):
+    src_path = tmp_path / "source.flac"
+    src_path.write_bytes(b"fake-audio")
+    album_folder = tmp_path / "album"
+    db_path = tmp_path / "tracks.db"
+
+    with sqlite3.connect(db_path) as conn:
+        conn.execute(
+            """
+            CREATE TABLE tracks (
+                id TEXT PRIMARY KEY,
+                album_id TEXT NOT NULL,
+                artist_id TEXT NOT NULL,
+                title TEXT NOT NULL,
+                track_number INTEGER,
+                duration INTEGER,
+                file_path TEXT,
+                bitrate INTEGER,
+                created_at TEXT,
+                updated_at TEXT
+            )
+            """
+        )
+        conn.execute(
+            """
+            INSERT INTO tracks (id, album_id, artist_id, title, track_number, duration, file_path, bitrate, created_at, updated_at)
+            VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
+            """,
+            ("target-track-1", "target-album", "target-artist", "Existing Track", 1, 180000, str(src_path), 320),
+        )
+        conn.commit()
+
+    class _FakeDB:
+        def _get_connection(self):
+            return sqlite3.connect(db_path)
+
+        def get_tracks_by_album(self, album_id):
+            return [
+                SimpleNamespace(
+                    id="source-track-1",
+                    album_id="source-album",
+                    artist_id="source-artist",
+                    duration=180000,
+                    file_path=str(src_path),
+                    bitrate=320,
+                ),
+                SimpleNamespace(
+                    id="source-track-2",
+                    album_id="source-album",
+                    artist_id="source-artist",
+                    duration=181000,
+                    file_path=str(src_path),
+                    bitrate=320,
+                ),
+            ]
+
+    worker = RepairWorker.__new__(RepairWorker)
+    worker.db = _FakeDB()
+    worker.transfer_folder = str(tmp_path)
+    worker._config_manager = None
+    worker._enhance_file_metadata = None
+    worker._enhance_placed_track = lambda *args, **kwargs: None
+
+    monkeypatch.setattr(
+        "core.repair_worker.uuid.uuid4",
+        lambda: SimpleNamespace(hex="deadbeefcafebabe"),
+    )
+
+    result = worker._perform_album_fill(
+        candidate=SimpleNamespace(
+            id="source-track-1",
+            album_id="source-album",
+            artist_id="source-artist",
+            duration=180000,
+            file_path=str(src_path),
+            bitrate=320,
+        ),
+        album_id="target-album",
+        album_title="Target Album",
+        artist_name="Target Artist",
+        track_name="New Track",
+        track_number=2,
+        disc_number=1,
+        album_folder=str(album_folder),
+        filename_pattern="{num:02d} - {title}",
+        download_folder=None,
+    )
+
+    assert result["success"] is True
+    assert result["action"] == "copied"
+
+    with sqlite3.connect(db_path) as verify_conn:
+        row = verify_conn.execute(
+            "SELECT id, title, file_path FROM tracks WHERE title = ?",
+            ("New Track",),
+        ).fetchone()
+        assert row is not None
+        assert row[0] is not None
+        assert row[0].startswith("album_fill_source-track-1_deadbeef")
+        assert row[1] == "New Track"
+        assert Path(row[2]).exists()
+        assert verify_conn.execute("SELECT COUNT(*) FROM tracks WHERE id IS NULL").fetchone()[0] == 0