danvics/soulsync
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1
soulsync/tests/wishlist/test_payloads.py
Broque Thomas 997732ee63 Wishlist: fix three regressions causing all imports to land as track 01 with no year 
Real-world regression triggered by the album-bundle work earlier in
2.6.3. Tracks with full Spotify metadata were importing as
``01 - <title>`` under ``Artist - Album/`` (no year), even when the
source filename carried the correct track number and Spotify's
release_date was available.

Investigation via DB inspection of stored wishlist rows:

```
"Never Gonna Give You Up" → track_number=None,  release_date=""
"idfc"                    → track_number=1,    release_date=""
"No Sleep Till Brooklyn"  → track_number=1,    release_date=""
```

Source-of-truth Spotify metadata had release_date AND real track
positions, but the wishlist row was poisoned. Three regressions
compounded the loss:

**Fix A — ``track_object_to_dict`` (``core/wishlist/payloads.py:295``)
preserved only album.name during Track→dict conversion.**

Pre-fix:
```python
album_name = "Unknown Album"
if hasattr(track_object, "album") and track_object.album:
    if hasattr(track_object.album, "name"):
        album_name = track_object.album.name
    else:
        album_name = str(track_object.album)

result = {
    ...
    "album": {"name": album_name},   # ← release_date / images / etc. all dropped
    ...
}
```

When a wishlist payload arrived as a Track dataclass instead of a
raw spotify_data dict, the Track→dict conversion stripped
release_date, images, album_type, total_tracks, id, and album-level
artists. Every wishlist row added through this path landed in the
DB with ``album={'name': X}`` only.

Post-fix: three branches handle the three album shapes
- ``album_attr`` is a dict → ``dict(album_attr)`` preserves every key
- ``album_attr`` is a sub-object → pull all common Album-dataclass
  attrs (id, release_date, album_type, total_tracks, images, ...)
- ``album_attr`` is a bare string → build a dict from the track
  object's adjacent attrs (release_date, album_id, album_type, ...)
  and surface ``image_url`` as ``album.images``

**Fix B — ``core/discovery/playlist.py:309`` only added
``track_number`` / ``disc_number`` keys when truthy.**

Pre-fix:
```python
matched_data = { 'id': ..., 'name': ..., ... }   # no track_number / disc_number
if track_number:
    matched_data['track_number'] = track_number
if disc_number:
    matched_data['disc_number'] = disc_number
```

Deezer-sourced matches always hit this branch with ``track_number=None``
because the cache enrichment at line 304 reads ``_raw.get('track_number')``
literally, but Deezer's raw shape uses ``track_position``. So the key
was omitted from ``matched_data``, downstream consumers couldn't
distinguish "missing key" from "value is 1", and the chain silently
filled 1.

Post-fix: keys are ALWAYS present (None when unknown). Also adds a
``best_match.track_number`` fallback so the Track-dataclass-mapped
value (which DOES include ``track_position``→``track_number``
mapping) gets used when the cache lookup misses.

**Fix C — Pipeline only consulted ``album_info.track_number`` before
falling to the filename (``core/imports/pipeline.py:645``).**

VA-collection source files like ``417 Fountains of Wayne - Stacys
Mom.flac`` have a leading playlist-position number that isn't the
album track number. The previous chain (album_info → filename →
floor-1) couldn't recover the real position because the filename
extractor either returned 417 (wrong) or None (caught by the floor).
But the wishlist payload's ``track_info.spotify_data.track_number``
HAD the right answer all along — Spotify says Stacy's Mom is track
3 on Welcome Interstate Managers.

Post-fix: resolution chain extracted into ``core/imports/track_number.py:resolve_track_number``
as a pure function:
1. ``album_info.track_number`` (album-bundle dispatch authoritative)
2. ``track_info.track_number`` (per-track flow payload)
3. ``track_info.spotify_data.track_number`` (nested fallback)
4. ``extract_explicit_track_number(file_path)`` (filename, returns
   0 when no numeric prefix — vs the default helper that returns 1)
5. Caller (pipeline) applies the final >=1 floor

Each step coerces to a positive int or falls through to the next.
Pure function = unit-testable in isolation = single place to fix
the rule.

**Test coverage (37 new tests):**

- ``tests/wishlist/test_payloads.py`` (+4) — Track→dict conversion
  preserves full album dict (dict / object / string album shapes) +
  None-track-number stays None.
- ``tests/discovery/test_discovery_playlist.py`` (+2) — matched_data
  always includes track_number/disc_number keys (None when unknown)
  + falls back to best_match attrs when cache misses.
- ``tests/imports/test_track_number_resolver.py`` (+16) — every
  resolution-chain branch pinned: album_info-wins, track_info
  fallback, spotify_data nested, JSON-string parsing, garbage-string
  fall-through, zero / negative / non-numeric / string-numeric
  coercion, filename fallback, explicit extractor vs default
  extractor semantics, defensive None inputs, VA-collection
  filename behaviour, all-sources-missing → None.

1571 wider-suite tests pass (wishlist + imports + discovery +
downloads + metadata). Ruff clean.

**Migration note:** existing wishlist rows that were saved under
the OLD ``track_object_to_dict`` (with stripped album metadata) still
have ``release_date=''`` in the DB blob. Those won't self-heal — the
next attempt loads from the poisoned blob. Users can remove + re-add
those tracks to refresh, or wait for the next sync run that
re-discovers them with full metadata. No automatic migration shipped
in this PR (scope creep — the forward path is fixed, backfill is a
separate concern).
2026-05-27 15:39:22 -07:00

359 lines
13 KiB
Python
Raw Permalink Blame History

from types import SimpleNamespace
from core.wishlist import payloads
def test_sanitize_track_data_for_processing_normalizes_artists_and_album():
track = {
"name": "Song",
"album": 123,
"artists": [{"name": "Artist One"}, "Artist Two", SimpleNamespace(name="Artist Three")],
}
out = payloads.sanitize_track_data_for_processing(track)
assert out["album"] == "123"
assert out["artists"] == ["Artist One", "Artist Two", "namespace(name='Artist Three')"]
def test_get_track_artist_name_prefers_artists_list_then_artist_field():
assert payloads.get_track_artist_name({"artists": [{"name": "Artist One"}]}) == "Artist One"
assert payloads.get_track_artist_name({"artist": "Solo Artist"}) == "Solo Artist"
assert payloads.get_track_artist_name({}) == "Unknown Artist"
def test_ensure_spotify_track_format_preserves_existing_shape():
track = {
"id": "sp-1",
"name": "Song",
"artists": [{"name": "Artist One"}],
"album": {"name": "Album", "album_type": "ep", "total_tracks": 4},
}
out = payloads.ensure_spotify_track_format(track)
assert out is track
def test_ensure_spotify_track_format_builds_webui_shape():
track = {
"name": "Song",
"artist": "Artist One",
"album": {"name": "Album One", "release_date": "2024-01-01"},
"duration_ms": 1234,
"track_number": 7,
"disc_number": 2,
"preview_url": "https://example.test/preview",
"external_urls": {"spotify": "https://open.spotify.com/track/1"},
"popularity": 42,
}
out = payloads.ensure_spotify_track_format(track)
assert out["name"] == "Song"
assert out["artists"] == [{"name": "Artist One"}]
assert out["album"]["name"] == "Album One"
assert out["album"]["album_type"] == "album"
assert out["album"]["total_tracks"] == 0
assert out["source"] == "webui_modal"
def test_ensure_wishlist_track_format_aliases_the_spotify_helper():
track = {
"name": "Song",
"artist": "Artist One",
"album": {"name": "Album One"},
}
out = payloads.ensure_wishlist_track_format(track)
assert out["name"] == "Song"
assert out["artists"] == [{"name": "Artist One"}]
assert out["album"]["name"] == "Album One"
def test_extract_spotify_track_from_modal_info_converts_trackresult_like_object():
track_info = {
"spotify_track": SimpleNamespace(
title="Song Two",
artist="Artist Two",
album="Album Two",
)
}
out = payloads.extract_spotify_track_from_modal_info(track_info)
assert out["source"] == "trackresult"
assert out["name"] == "Song Two"
assert out["artists"] == [{"name": "Artist Two"}]
assert out["album"]["name"] == "Album Two"
def test_extract_spotify_track_from_modal_info_reconstructs_from_slskd_result():
track_info = {
"slskd_result": SimpleNamespace(
title="Song Three",
artist="Artist Three",
album="Album Three",
)
}
out = payloads.extract_spotify_track_from_modal_info(track_info)
assert out["reconstructed"] is True
assert out["name"] == "Song Three"
assert out["artists"] == [{"name": "Artist Three"}]
assert out["album"]["name"] == "Album Three"
# ---------------------------------------------------------------------------
# track_number / disc_number preservation through the wishlist payload
# helpers — pins the bug A fix from PR 2/4. Pre-fix the helpers
# defaulted missing numbers to 1, which locked every wishlist retry
# to track 01 because the import pipeline's filename-extract fallback
# only fires when the value is None (not the pre-filled 1).
# ---------------------------------------------------------------------------
def test_ensure_wishlist_track_format_preserves_real_track_number():
"""Real track positions must survive the format helper. Pre-fix
the helper read ``track_info.get('track_number', 1)`` which always
returned 1 if the upstream payload had dropped the key — the
desired number was lost on every round-trip."""
track = {
"name": "No Sleep Till Brooklyn",
"artist": "Beastie Boys",
"album": {"name": "Licensed to Ill", "release_date": "1986-11-15"},
"track_number": 8,
"disc_number": 1,
}
out = payloads.ensure_wishlist_track_format(track)
assert out["track_number"] == 8
assert out["disc_number"] == 1
def test_ensure_wishlist_track_format_keeps_missing_track_number_as_none():
"""When the upstream payload doesn't carry a track number, the
helper must NOT pre-fill 1 — that poisons the chain and locks the
file to track 01. Leave None so the import pipeline's filename
fallback at ``core/imports/pipeline.py:652`` can fire."""
track = {
"name": "Mystery Track",
"artist": "Artist",
"album": {"name": "Album"},
}
out = payloads.ensure_wishlist_track_format(track)
assert out["track_number"] is None
assert out["disc_number"] is None
def test_build_cancelled_task_wishlist_payload_preserves_track_number():
"""Cancellation→re-add path was the worst offender — the payload
builder dropped track_number from the saved data entirely (didn't
even include the key). Next wishlist cycle saw missing key →
helper defaulted to 1 → file imported as 01. Now both the
cancellation payload AND the helper preserve real positions."""
task = {
"track_info": {
"id": "trk-1", "name": "Brass Monkey",
"artists": [{"name": "Beastie Boys"}],
"album": {"name": "Licensed to Ill", "release_date": "1986-11-15"},
"track_number": 11,
"disc_number": 1,
},
"playlist_name": "Wishlist",
"playlist_id": "p1",
}
out = payloads.build_cancelled_task_wishlist_payload(task)
td = out["track_data"]
assert td["track_number"] == 11
assert td["disc_number"] == 1
# Album release_date survives the round-trip so the path template
# renders the year in the folder name.
assert td["album"]["release_date"] == "1986-11-15"
def test_track_object_to_dict_preserves_full_album_dict():
"""When the input track has an album as a DICT (e.g. raw Spotify
spotify_track_data), every album field must survive the
Track→dict conversion. Pre-fix the conversion built
``album = {'name': X}`` only, silently dropping release_date /
images / album_type / total_tracks. Result: every wishlist row
added from a Track-object path had empty release_date in the DB
→ import path-template rendered without year → user's main
complaint."""
track_obj = SimpleNamespace(
id="track-1",
name="Never Gonna Give You Up",
artists=[{"name": "Rick Astley"}],
album={
"id": "alb-1",
"name": "Whenever You Need Somebody",
"release_date": "1987-11-12",
"album_type": "album",
"total_tracks": 10,
"images": [{"url": "https://cdn.example/cover.jpg"}],
"artists": [{"name": "Rick Astley"}],
},
duration_ms=213000,
track_number=1,
disc_number=1,
)
out = payloads.track_object_to_dict(track_obj)
assert out["album"]["name"] == "Whenever You Need Somebody"
assert out["album"]["release_date"] == "1987-11-12"
assert out["album"]["album_type"] == "album"
assert out["album"]["total_tracks"] == 10
assert out["album"]["id"] == "alb-1"
assert out["album"]["images"] == [{"url": "https://cdn.example/cover.jpg"}]
assert out["album"]["artists"] == [{"name": "Rick Astley"}]
def test_track_object_to_dict_extracts_release_date_from_album_object():
"""When the album is a sub-OBJECT (e.g. Spotify/Deezer Album
dataclass), the conversion must pull release_date / album_type /
total_tracks from its attributes — not assume dict-only access.
Pre-fix the only attr read was ``.name``, dropping everything
else even when the object had it."""
class _AlbumLike:
name = "Licensed to Ill"
id = "alb-li"
release_date = "1986-11-15"
album_type = "album"
total_tracks = 13
artists = ["Beastie Boys"]
images = [{"url": "https://cover.example/li.jpg"}]
track_obj = SimpleNamespace(
id="track-2",
name="No Sleep Till Brooklyn",
artists=[{"name": "Beastie Boys"}],
album=_AlbumLike(),
duration_ms=242000,
track_number=8,
disc_number=1,
)
out = payloads.track_object_to_dict(track_obj)
assert out["album"]["name"] == "Licensed to Ill"
assert out["album"]["release_date"] == "1986-11-15"
assert out["album"]["total_tracks"] == 13
assert out["album"]["id"] == "alb-li"
assert out["track_number"] == 8
def test_track_object_to_dict_string_album_pulls_release_date_from_track_attrs():
"""When the album is a bare STRING (the lean Track dataclass
shape used by some metadata sources), the album dict has to be
built from scratch. Pull release_date + album_type from adjacent
track-object attrs and image_url from the track itself so we
don't lose the path-template inputs entirely."""
track_obj = SimpleNamespace(
id="track-3",
name="Stacy's Mom",
artists=[{"name": "Fountains of Wayne"}],
album="Welcome Interstate Managers",
release_date="2003-06-10",
album_type="album",
total_tracks=15,
image_url="https://cover.example/wim.jpg",
track_number=3,
disc_number=1,
duration_ms=200000,
)
out = payloads.track_object_to_dict(track_obj)
assert out["album"]["name"] == "Welcome Interstate Managers"
assert out["album"]["release_date"] == "2003-06-10"
assert out["album"]["album_type"] == "album"
assert out["album"]["total_tracks"] == 15
assert out["album"]["images"] == [{"url": "https://cover.example/wim.jpg"}]
assert out["track_number"] == 3
def test_track_object_to_dict_missing_track_number_stays_none():
"""Track-object-style sources that genuinely don't know the track
position must surface as None (not pre-filled 1), so the import
pipeline's filename-extract fallback can fire."""
track_obj = SimpleNamespace(
id="track-4",
name="Unknown Track",
artists=[{"name": "Artist"}],
album="Album",
)
out = payloads.track_object_to_dict(track_obj)
assert out["track_number"] is None
assert out["disc_number"] is None
def test_build_cancelled_task_wishlist_payload_string_album_pulls_release_date_from_track_info():
"""When the source ``album`` field is a bare string, the payload
builder constructs an album dict from scratch — it must pull
release_date / album_image_url / etc. from the adjacent
track_info fields rather than dropping them silently."""
task = {
"track_info": {
"id": "trk-2", "name": "Song",
"artists": [{"name": "Artist"}],
"album": "Bare String Album",
"release_date": "2020-06-01",
},
}
out = payloads.build_cancelled_task_wishlist_payload(task)
album = out["track_data"]["album"]
assert album["name"] == "Bare String Album"
assert album["release_date"] == "2020-06-01"
def test_ensure_wishlist_track_format_defaults_non_dict_album_to_album_type():
"""When ``album`` arrives as a non-dict (legacy/reconstruction path) we
must not stamp ``album_type='single'`` — that lies about the origin
and routes the wishlist requeue through the single_path template
instead of album_path, dumping album tracks into the Singles tree.
Default to 'album' / total_tracks=0 (unknown) so downstream code can
fall through to the real release-type detection logic."""
track = {
"name": "Song",
"artist": "Artist One",
"album": "Album From Legacy String",
}
out = payloads.ensure_wishlist_track_format(track)
assert out["album"]["name"] == "Album From Legacy String"
assert out["album"]["album_type"] == "album"
assert out["album"]["total_tracks"] == 0
def test_extract_spotify_track_from_modal_info_slskd_reconstruction_defaults_to_album():
"""Slskd-result reconstruction is a last-resort path; defaulting to
``album_type='single'`` corrupted the requeue routing for album
batches. Same fix as ensure_wishlist_track_format: default 'album'."""
track_info = {
"slskd_result": SimpleNamespace(
title="Song Three",
artist="Artist Three",
album="Album Three",
)
}
out = payloads.extract_spotify_track_from_modal_info(track_info)
assert out["album"]["album_type"] == "album"
assert out["album"]["total_tracks"] == 0
def test_extract_wishlist_track_from_modal_info_uses_track_data_key():
track_info = {
"track_data": {
"id": "track-1",
"name": "Song Four",
"artists": [{"name": "Artist Four"}],
"album": {"name": "Album Four"},
}
}
out = payloads.extract_wishlist_track_from_modal_info(track_info)
assert out["id"] == "track-1"
assert out["name"] == "Song Four"
assert out["artists"] == [{"name": "Artist Four"}]
Reference in a new issue View git blame Copy permalink