#843 follow-up: key the no-state cache save by the FIRST artist
The #843 fallback saved the discovery-cache match using the client's original_artist verbatim — but the client sends a joined "A, B, C" string, while EVERY in-memory + sync path keys the cache by the first artist (artists[0]). So a multi-artist fix (the reporter's exact "Cherrymoon Traxx, Hermol, SBM, BELS" case) would have saved under a key the sync never looks up — the fix would "succeed" with no error but silently never apply. Reduce the client artist to the first (split on comma) in the no-state branch so its cache key matches the in-memory/sync convention exactly. Single-artist tracks are unaffected. Test: no-state save now keys by the first artist, and a new test pins that the no-state and in-memory paths produce an IDENTICAL cache key for the same multi-artist track. 74 discovery tests pass.
This commit is contained in:
parent
0afa3c9705
commit
5bc27e6268
2 changed files with 42 additions and 4 deletions
|
|
@ -629,6 +629,12 @@ def update_discovery_match(
|
||||||
return {'error': 'Discovery state not found'}, 404
|
return {'error': 'Discovery state not found'}, 404
|
||||||
if not original_name:
|
if not original_name:
|
||||||
original_name = spotify_track['name']
|
original_name = spotify_track['name']
|
||||||
|
# Key the cache by the FIRST artist — every in-memory + sync path uses
|
||||||
|
# artists[0], but the client may send a joined "A, B, C" string. Without
|
||||||
|
# this, a multi-artist track would save under a key the sync never looks
|
||||||
|
# up (full string ≠ first artist), so the fix would silently never apply.
|
||||||
|
if original_artist:
|
||||||
|
original_artist = original_artist.split(',')[0].strip()
|
||||||
logger.info(
|
logger.info(
|
||||||
f"Manual match (no in-memory state) → discovery cache: "
|
f"Manual match (no in-memory state) → discovery cache: "
|
||||||
f"{source_log_label} - {identifier} - '{original_name}' by '{original_artist}'"
|
f"{source_log_label} - {identifier} - '{original_name}' by '{original_artist}'"
|
||||||
|
|
|
||||||
|
|
@ -966,7 +966,9 @@ def test_update_match_no_state_but_originals_saves_cache():
|
||||||
db = _FakeCacheDB()
|
db = _FakeCacheDB()
|
||||||
gj, kw, _ = _update_kwargs(cache_db=db, json_data={
|
gj, kw, _ = _update_kwargs(cache_db=db, json_data={
|
||||||
'identifier': 'gone-from-memory', 'track_index': 0,
|
'identifier': 'gone-from-memory', 'track_index': 0,
|
||||||
'original_name': 'Acid Dream', 'original_artist': 'Cherrymoon Traxx',
|
'original_name': 'Acid Dream',
|
||||||
|
# multi-artist joined string, exactly like the #843 reporter's track
|
||||||
|
'original_artist': 'Cherrymoon Traxx, Hermol, SBM, BELS',
|
||||||
'spotify_track': {
|
'spotify_track': {
|
||||||
'id': 'sp1', 'name': 'Acid Dream (The Prophet remix)',
|
'id': 'sp1', 'name': 'Acid Dream (The Prophet remix)',
|
||||||
'artists': ['Cherry Moon Trax'], 'album': 'X', 'duration_ms': 1000,
|
'artists': ['Cherry Moon Trax'], 'album': 'X', 'duration_ms': 1000,
|
||||||
|
|
@ -976,11 +978,41 @@ def test_update_match_no_state_but_originals_saves_cache():
|
||||||
|
|
||||||
assert code == 200 and body['success'] is True
|
assert code == 200 and body['success'] is True
|
||||||
assert body['result'] is None # nothing to update in memory
|
assert body['result'] is None # nothing to update in memory
|
||||||
# the durable cache match WAS written, keyed by the original track
|
# the durable cache match WAS written — and CRUCIALLY keyed by the FIRST
|
||||||
|
# artist, matching every in-memory/sync path (which use artists[0]). A
|
||||||
|
# full-string key here would silently never be looked up on sync.
|
||||||
assert len(db.saved) == 1
|
assert len(db.saved) == 1
|
||||||
saved = db.saved[0]
|
saved = db.saved[0]
|
||||||
assert saved[0] == 'acid dream' # cache_key from get_discovery_cache_key
|
assert saved[1] == 'cherrymoon traxx' # cache_key artist = first artist (not the full string)
|
||||||
assert saved[5] == 'Acid Dream' and saved[6] == 'Cherrymoon Traxx' # original name/artist
|
assert saved[6] == 'Cherrymoon Traxx' # original_artist reduced to first
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_match_no_state_key_matches_in_memory_key():
|
||||||
|
"""The no-state save and the normal in-memory save must produce the SAME cache
|
||||||
|
key for the same multi-artist track — else the fix wouldn't apply on sync."""
|
||||||
|
from core.discovery.endpoints import update_discovery_match
|
||||||
|
sp = {'id': 'sp1', 'name': 'M', 'artists': ['Z'], 'album': 'A', 'duration_ms': 1}
|
||||||
|
|
||||||
|
# in-memory path: result carries the original track as an artists LIST
|
||||||
|
db1 = _FakeCacheDB()
|
||||||
|
state = {'discovery_results': [{'tidal_track': {'name': 'Acid Dream',
|
||||||
|
'artists': ['Cherrymoon Traxx', 'Hermol', 'SBM', 'BELS']}}],
|
||||||
|
'spotify_matches': 0}
|
||||||
|
gj1, kw1, _ = _update_kwargs(cache_db=db1, json_data={
|
||||||
|
'identifier': 'p', 'track_index': 0, 'spotify_track': sp})
|
||||||
|
update_discovery_match({'p': state}, gj1, **kw1)
|
||||||
|
|
||||||
|
# no-state path: client sends the joined string
|
||||||
|
db2 = _FakeCacheDB()
|
||||||
|
gj2, kw2, _ = _update_kwargs(cache_db=db2, json_data={
|
||||||
|
'identifier': 'p', 'track_index': 0, 'spotify_track': sp,
|
||||||
|
'original_name': 'Acid Dream',
|
||||||
|
'original_artist': 'Cherrymoon Traxx, Hermol, SBM, BELS'})
|
||||||
|
update_discovery_match({}, gj2, **kw2)
|
||||||
|
|
||||||
|
# same cache key (name, artist) → the fix applies identically either way
|
||||||
|
assert db1.saved[0][0] == db2.saved[0][0]
|
||||||
|
assert db1.saved[0][1] == db2.saved[0][1]
|
||||||
|
|
||||||
|
|
||||||
def test_update_match_no_state_and_no_originals_still_404():
|
def test_update_match_no_state_and_no_originals_still_404():
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue