v0.20.3: one artist spelled two ways
The sync was fine. Everything it put on the Rabbit was there and correct -- 2,432 media files, 208 covers, a manifest whose every field type-checks against the parser -- and andTunes still answered "No library / Couldn't read library.json". Fifteen tracks took all 2,439 songs down with them. Library.group() keyed albumsByKey case-folded and artistsByName raw-case, and only ever built the Artist inside `if (album == null)`. So the second track of an album whose artist is spelled differently found the album already there, skipped the block that would have made the artist, and dereferenced the null that came back. Six artists in trav's library are spelled two ways: RJD2/Rjd2, Toro Y Moi/Toro y Moi, FatBoy Slim/Fatboy Slim, LOVING/Loving, Land Of The Loops/Land of the Loops, Salami Rose Joe Louis/salami rose joe louis. It had worked until the 12th because that sync was the first to carry both spellings of one of those albums. Both maps fold case now, and the artist is fetched-or-made before the album block and held, so no lookup left in group() can come back null. Artist gains a key the way Album always had one, and ListActivity navigates by it -- ALBUM_ROW already passed album.key, so artists just stopped being the exception. The device filesystem is case-insensitive, so those two spellings are one folder there. plan_andtunes_sync compared exact strings and saw every such file as stale *and* missing, deleting and re-copying it over MTP on every sync forever; the diff and the collision rule both fold now. plan.stale still carries the device's own spelling, since that is what _delete_stale unlinks by. Verified on the Rabbit: 2,439 songs listed, RJD2 one row of 12 songs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SKXUgsBBwe3qaHEjeV8ubP
This commit is contained in:
@@ -0,0 +1,160 @@
|
||||
"""Round 54: one artist spelled two ways.
|
||||
|
||||
trav's Rabbit showed `No library / Couldn't read library.json` on a sync that
|
||||
had worked perfectly. The manifest was fine; the app's grouping wasn't.
|
||||
`Library.group()` keyed albums case-folded but artists raw-case, and only
|
||||
created the artist inside the `album == null` branch — so the second track of
|
||||
an album whose artist was spelled with different case ("RJD2" / "Rjd2") found
|
||||
the album already built, skipped artist creation, and dereferenced a null.
|
||||
Fifteen tracks across six artists took all 2,439 songs down with them.
|
||||
|
||||
The Java half has no test harness. What's pinned here is the desktop half of
|
||||
the same root cause: the device filesystem is case-insensitive, so those two
|
||||
spellings are **one** folder there, and `plan_andtunes_sync`'s exact-string
|
||||
diff saw every such file as stale *and* missing and re-copied it over MTP on
|
||||
every sync forever.
|
||||
"""
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from lintunes.andtunes.layout import media_relpath
|
||||
from lintunes.andtunes.sync import AndTunesSyncWorker, plan_andtunes_sync
|
||||
from lintunes.models import Track
|
||||
from lintunes.models.playlist import Playlist, PlaylistType
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _private_cache(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache"))
|
||||
|
||||
|
||||
DATA = b"x" * 64
|
||||
|
||||
|
||||
def _track(tid, name, artist, album="Since We Last Spoke", *, tmp_path):
|
||||
path = tmp_path / "local" / f"{tid}.mp3"
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_bytes(DATA)
|
||||
return Track(track_id=tid, name=name, artist=artist, album=album,
|
||||
location=str(path), total_time=60_000, kind="MPEG audio file")
|
||||
|
||||
|
||||
def _playlist(*tids):
|
||||
return Playlist(name="Mix", persistent_id="AAAA0001",
|
||||
playlist_type=PlaylistType.REGULAR, track_ids=list(tids))
|
||||
|
||||
|
||||
def _root(tmp_path):
|
||||
return tmp_path / "device" / "Music" / "andTunes"
|
||||
|
||||
|
||||
def _on_device(root, rel, data=DATA):
|
||||
path = root / rel
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_bytes(data)
|
||||
return path
|
||||
|
||||
|
||||
def _both_spellings(tmp_path):
|
||||
return {
|
||||
1: _track(1, "Iced Lightning", "RJD2", tmp_path=tmp_path),
|
||||
2: _track(2, "Making Days Longer", "Rjd2", tmp_path=tmp_path),
|
||||
}
|
||||
|
||||
|
||||
# ---- the two spellings are one folder on the device ----
|
||||
|
||||
def test_the_two_spellings_ask_for_one_device_folder(tmp_path):
|
||||
"""Different rel paths, but the device can only hold one of the two."""
|
||||
tracks = _both_spellings(tmp_path)
|
||||
assert media_relpath(tracks[1]).startswith("Media/RJD2/")
|
||||
assert media_relpath(tracks[2]).startswith("Media/Rjd2/")
|
||||
folders = {media_relpath(t).split("/")[1].lower() for t in tracks.values()}
|
||||
assert folders == {"rjd2"}
|
||||
|
||||
|
||||
def test_a_file_filed_under_the_other_spelling_is_kept_not_recopied(tmp_path):
|
||||
"""The churn: it used to be stale *and* missing, every sync, forever."""
|
||||
tracks = _both_spellings(tmp_path)
|
||||
root = _root(tmp_path)
|
||||
# What a case-insensitive filesystem actually ends up holding: whichever
|
||||
# spelling made the folder first wins, and both songs land inside it.
|
||||
_on_device(root, "Media/RJD2/Since We Last Spoke/Iced Lightning.mp3")
|
||||
_on_device(root, "Media/RJD2/Since We Last Spoke/Making Days Longer.mp3")
|
||||
|
||||
plan = plan_andtunes_sync([_playlist(1, 2)], tracks, root)
|
||||
|
||||
assert plan.kept == 2
|
||||
assert plan.copies == []
|
||||
assert plan.stale == []
|
||||
assert plan.bytes_to_copy == 0
|
||||
|
||||
|
||||
def test_a_real_change_still_copies(tmp_path):
|
||||
"""Folding the diff must not blind it to a file whose size moved."""
|
||||
tracks = _both_spellings(tmp_path)
|
||||
root = _root(tmp_path)
|
||||
_on_device(root, "Media/RJD2/Since We Last Spoke/Iced Lightning.mp3")
|
||||
_on_device(root, "Media/RJD2/Since We Last Spoke/Making Days Longer.mp3",
|
||||
b"short")
|
||||
|
||||
plan = plan_andtunes_sync([_playlist(1, 2)], tracks, root)
|
||||
|
||||
assert plan.kept == 1
|
||||
assert [item.track_id for item in plan.copies] == [2]
|
||||
|
||||
|
||||
def test_a_file_under_neither_spelling_is_still_stale(tmp_path):
|
||||
tracks = _both_spellings(tmp_path)
|
||||
root = _root(tmp_path)
|
||||
_on_device(root, "Media/RJD2/Since We Last Spoke/Iced Lightning.mp3")
|
||||
_on_device(root, "Media/RJD2/Since We Last Spoke/Making Days Longer.mp3")
|
||||
_on_device(root, "Media/Someone Else/Old Record/Gone.mp3")
|
||||
|
||||
plan = plan_andtunes_sync([_playlist(1, 2)], tracks, root)
|
||||
|
||||
# The device's own spelling, because _delete_stale unlinks by that name.
|
||||
assert plan.stale == ["Media/Someone Else/Old Record/Gone.mp3"]
|
||||
|
||||
|
||||
# ---- names that collide only by case ----
|
||||
|
||||
def test_two_songs_colliding_only_by_case_both_get_the_id_suffix(tmp_path):
|
||||
"""One file on the device, so one of them would have silently won."""
|
||||
tracks = {
|
||||
1: _track(1, "Iced Lightning", "RJD2", tmp_path=tmp_path),
|
||||
2: _track(2, "Iced Lightning", "Rjd2", tmp_path=tmp_path),
|
||||
}
|
||||
plan = plan_andtunes_sync([_playlist(1, 2)], tracks, _root(tmp_path))
|
||||
|
||||
rels = {item.track_id: item.rel for item in plan.items}
|
||||
assert rels[1].endswith("Iced Lightning [1].mp3")
|
||||
assert rels[2].endswith("Iced Lightning [2].mp3")
|
||||
assert len({rel.lower() for rel in rels.values()}) == 2
|
||||
|
||||
|
||||
# ---- both spellings survive into the manifest ----
|
||||
|
||||
def test_the_manifest_still_carries_both_tracks(tmp_path):
|
||||
tracks = _both_spellings(tmp_path)
|
||||
root = _root(tmp_path)
|
||||
plan = plan_andtunes_sync([_playlist(1, 2)], tracks, root)
|
||||
|
||||
worker = AndTunesSyncWorker(plan)
|
||||
out = {}
|
||||
worker.finished.connect(lambda s: out.setdefault("finished", s))
|
||||
worker.failed.connect(lambda m: out.setdefault("failed", m))
|
||||
worker._run()
|
||||
assert "failed" not in out, out.get("failed")
|
||||
|
||||
manifest = json.loads((root / "library.json").read_text(encoding="utf-8"))
|
||||
by_id = {row["id"]: row for row in manifest["tracks"]}
|
||||
assert by_id[1]["artist"] == "RJD2"
|
||||
assert by_id[2]["artist"] == "Rjd2"
|
||||
# Each keeps its own spelling in the path it asks for; the device resolves
|
||||
# both to the one folder it actually has.
|
||||
assert by_id[1]["path"].startswith("Media/RJD2/")
|
||||
assert by_id[2]["path"].startswith("Media/Rjd2/")
|
||||
assert manifest["playlists"][0]["tracks"] == [1, 2]
|
||||
Reference in New Issue
Block a user