From 88e2527b96ea515b020edd3b631143763ce3f06b Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 17 Apr 2026 19:16:15 +0300 Subject: [PATCH 1/2] Fix null-pointer error in acoustid_scanner The root cause (null track ids) needs to be solved elsewhere, but this is a band-aid for now --- core/repair_jobs/acoustid_scanner.py | 13 +++- tests/test_acoustid_scanner.py | 88 ++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 tests/test_acoustid_scanner.py diff --git a/core/repair_jobs/acoustid_scanner.py b/core/repair_jobs/acoustid_scanner.py index e3002430..2f27f741 100644 --- a/core/repair_jobs/acoustid_scanner.py +++ b/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 "", + ) + continue + track_id = str(track_id) tracks[track_id] = { 'title': row[1] or '', 'artist': row[2] or '', diff --git a/tests/test_acoustid_scanner.py b/tests/test_acoustid_scanner.py new file mode 100644 index 00000000..25ebc168 --- /dev/null +++ b/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"] From 0e85931cc898943c7e59f58ca34b1ecc163cf1df Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 17 Apr 2026 19:25:56 +0300 Subject: [PATCH 2/2] Fix track ID generation in repair flows Repair-worker album fills now generate explicit track IDs when copying rows, instead of relying on SQLite auto-assignment that no longer exists for TEXT primary keys. The unknown-artist fixer now does the same for new artists. Also add a regression test for the album-fill copy branch and keep the AcoustID scanner resilient to legacy null-ID rows. --- core/repair_jobs/unknown_artist_fixer.py | 13 +- core/repair_worker.py | 38 ++++-- tests/test_repair_worker_album_fill.py | 145 +++++++++++++++++++++++ 3 files changed, 187 insertions(+), 9 deletions(-) create mode 100644 tests/test_repair_worker_album_fill.py diff --git a/core/repair_jobs/unknown_artist_fixer.py b/core/repair_jobs/unknown_artist_fixer.py index 01086989..db80be1c 100644 --- a/core/repair_jobs/unknown_artist_fixer.py +++ b/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,)) - new_artist_id = cursor.lastrowid + safe_artist_name = re.sub( + 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(""" diff --git a/core/repair_worker.py b/core/repair_worker.py index 230bc2a4..72caa020 100644 --- a/core/repair_worker.py +++ b/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,)) - new_artist_id = cursor.lastrowid + safe_artist_name = re.sub( + 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) - """, (album_id, target_artist_id, track_name, track_number, + VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + """, (new_track_id, album_id, target_artist_id, track_name, track_number, candidate.duration, target_path, candidate.bitrate)) conn.commit() diff --git a/tests/test_repair_worker_album_fill.py b/tests/test_repair_worker_album_fill.py new file mode 100644 index 00000000..c3d40e2b --- /dev/null +++ b/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