soulsync/tests/test_reorganize_db_methods.py
Broque Thomas 37aefd2ff1 Reorganize queue: race + dedupe fixes from kettui review
Five issues kettui flagged on PR #377:

- Worker race (reorganize_queue.py): _next_queued() picked an item and
  released the lock, then re-acquired to flip status='running'. A
  cancel() landing in that window marked the item cancelled but the
  worker still ran it. Replaced with _claim_next_or_wait() that picks
  AND flips under one lock acquisition.

- Wakeup race (reorganize_queue.py): _wakeup.clear() after the empty
  check could lose an enqueue's _wakeup.set(), parking a freshly-queued
  album for up to 60 seconds. Replaced Lock + Event with a single
  threading.Condition; cond.wait() releases and re-acquires atomically
  on notify.

- Bulk dedupe (reorganize_queue.py:enqueue_many): looped single-item
  enqueue, so a duplicate album_id later in the same batch could slip
  through if the worker finished the first copy before the loop
  reached the second. Now holds the lock for the whole batch and tracks
  a per-batch seen set, so intra-batch duplicates dedupe against each
  other and not just pre-existing items.

- Preview button stuck disabled (library.js:loadReorganizePreview):
  early returns and thrown errors skipped the re-enable line. Moved
  state into a canApply flag committed in finally, so any exit path
  lands the button correctly.

- DB helpers swallowing failures (music_database.py): get_album_display_meta
  and get_artist_albums_for_reorganize used to catch every Exception
  and return None / [], so a real DB outage masqueraded as "album not
  found" / "no albums". Now lets exceptions bubble; the route layer
  already wraps them as 500.

Tests:
- test_cancel_and_run_are_mutually_exclusive — hammers enqueue+cancel
  pairs and asserts the invariant that no successfully-cancelled item
  ever ran (catches regressions to the atomic pick).
- test_enqueue_many_dedupes_batch_internal_duplicates — pins the
  intra-batch dedupe.
- test_get_album_display_meta_propagates_db_errors and
  test_get_artist_albums_for_reorganize_propagates_db_errors — pin
  the bubble-up behavior.

Changelog updated in helper.js and version modal.
2026-04-26 08:40:24 -07:00

213 lines
7.4 KiB
Python

"""Tests for the reorganize-queue DB helpers on `MusicDatabase`:
- ``get_album_display_meta(album_id)`` — returns the title/artist tuple
the queue uses for status-panel display, or None when not found.
- ``get_artist_albums_for_reorganize(artist_id)`` — returns the
bulk-enqueue list ordered by year then title.
These are isolated DB-method tests so the SQL itself is verified
without spinning up Flask, the queue worker, or the orchestrator.
"""
import sqlite3
import sys
import types
import pytest
# ── stubs (same shape used elsewhere in the test suite) ───────────────────
if "spotipy" not in sys.modules:
spotipy = types.ModuleType("spotipy")
spotipy.Spotify = object
oauth2 = types.ModuleType("spotipy.oauth2")
oauth2.SpotifyOAuth = object
oauth2.SpotifyClientCredentials = object
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
def get_active_media_server(self):
return "primary"
settings_mod.config_manager = _DummyConfigManager()
config_pkg.settings = settings_mod
sys.modules["config"] = config_pkg
sys.modules["config.settings"] = settings_mod
from database.music_database import MusicDatabase # noqa: E402
# ── helpers ───────────────────────────────────────────────────────────────
class _InMemoryDB(MusicDatabase):
"""MusicDatabase that uses an in-memory sqlite that survives across
`_get_connection()` calls. Lets tests seed rows once and have the
methods under test see them."""
def __init__(self):
# Skip the real __init__ — it would try to migrate a real db.
self._conn = sqlite3.connect(":memory:")
self._conn.row_factory = sqlite3.Row
def _get_connection(self):
return _NonClosingConn(self._conn)
class _NonClosingConn:
"""Wraps the shared sqlite connection so `with db._get_connection()
as conn:` doesn't close the underlying handle between calls."""
def __init__(self, real):
self._real = real
def cursor(self):
return self._real.cursor()
def commit(self):
return self._real.commit()
def close(self):
pass
def __enter__(self):
return self
def __exit__(self, *args):
pass
def _seed(db, *, artists=(), albums=()):
cur = db._conn.cursor()
cur.execute("CREATE TABLE artists (id TEXT PRIMARY KEY, name TEXT)")
cur.execute("""
CREATE TABLE albums (
id TEXT PRIMARY KEY,
artist_id TEXT,
title TEXT,
year INTEGER
)
""")
for ar in artists:
cur.execute("INSERT INTO artists VALUES (?, ?)", ar)
for al in albums:
cur.execute(
"INSERT INTO albums (id, artist_id, title, year) VALUES (?, ?, ?, ?)",
al,
)
db._conn.commit()
@pytest.fixture
def db():
return _InMemoryDB()
# ── get_album_display_meta ────────────────────────────────────────────────
def test_get_album_display_meta_returns_dict_for_known_album(db):
_seed(db,
artists=[('ar-1', 'Kendrick Lamar')],
albums=[('alb-1', 'ar-1', 'good kid, m.A.A.d city', 2012)])
meta = db.get_album_display_meta('alb-1')
assert meta == {
'album_title': 'good kid, m.A.A.d city',
'artist_id': 'ar-1',
'artist_name': 'Kendrick Lamar',
}
def test_get_album_display_meta_returns_none_for_missing_album(db):
_seed(db, artists=[('ar-1', 'Aerosmith')])
assert db.get_album_display_meta('does-not-exist') is None
def test_get_album_display_meta_falls_back_for_blank_strings(db):
"""Albums with empty title or artist name in the DB still need a
safe display value — the queue UI should never render '(blank)'."""
_seed(db,
artists=[('ar-1', '')],
albums=[('alb-1', 'ar-1', '', 2015)])
meta = db.get_album_display_meta('alb-1')
assert meta['album_title'] == 'Unknown Album'
assert meta['artist_name'] == 'Unknown Artist'
assert meta['artist_id'] == 'ar-1'
# ── get_artist_albums_for_reorganize ──────────────────────────────────────
def test_get_artist_albums_for_reorganize_orders_by_year_then_title(db):
_seed(db,
artists=[('ar-1', 'Aerosmith')],
albums=[
('alb-c', 'ar-1', 'Toys in the Attic', 1975),
('alb-a', 'ar-1', 'Aerosmith', 1973),
('alb-b', 'ar-1', 'Get Your Wings', 1974),
])
rows = db.get_artist_albums_for_reorganize('ar-1')
assert [r['album_id'] for r in rows] == ['alb-a', 'alb-b', 'alb-c']
assert all(r['artist_name'] == 'Aerosmith' for r in rows)
def test_get_artist_albums_for_reorganize_secondary_sorts_by_title(db):
"""Same release year → tiebreak on title alphabetically."""
_seed(db,
artists=[('ar-1', 'X')],
albums=[
('alb-z', 'ar-1', 'Zebra', 1990),
('alb-a', 'ar-1', 'Apple', 1990),
('alb-m', 'ar-1', 'Mango', 1990),
])
rows = db.get_artist_albums_for_reorganize('ar-1')
assert [r['album_title'] for r in rows] == ['Apple', 'Mango', 'Zebra']
def test_get_artist_albums_for_reorganize_returns_empty_for_unknown_artist(db):
_seed(db, artists=[('ar-1', 'Aerosmith')])
assert db.get_artist_albums_for_reorganize('not-a-real-artist') == []
def test_get_artist_albums_for_reorganize_isolates_by_artist(db):
"""Pulling albums for artist A must NOT leak in albums from artist B."""
_seed(db,
artists=[('ar-1', 'A'), ('ar-2', 'B')],
albums=[
('alb-1', 'ar-1', 'A1', 2000),
('alb-2', 'ar-2', 'B1', 2000),
('alb-3', 'ar-1', 'A2', 2001),
])
rows = db.get_artist_albums_for_reorganize('ar-1')
assert {r['album_id'] for r in rows} == {'alb-1', 'alb-3'}
# ── error propagation ────────────────────────────────────────────────────
# Regression for review feedback on the original PR: helpers used to
# swallow every Exception and return None / [], so a real DB outage
# masqueraded as "album not found" / "no albums". Now they let the
# error bubble — the route layer turns it into a 500 — so the user sees
# a real failure instead of a phantom empty state.
def test_get_album_display_meta_propagates_db_errors(db):
"""If the underlying tables don't exist, the helper must raise
rather than swallow it as a missing-album result."""
# Don't seed — the schema is empty, so the SELECT will fail with
# OperationalError ("no such table: albums").
with pytest.raises(sqlite3.OperationalError):
db.get_album_display_meta('alb-1')
def test_get_artist_albums_for_reorganize_propagates_db_errors(db):
with pytest.raises(sqlite3.OperationalError):
db.get_artist_albums_for_reorganize('ar-1')