diff --git a/PROJECT.org b/PROJECT.org index d79cc0c..4483dab 100644 --- a/PROJECT.org +++ b/PROJECT.org @@ -18,7 +18,7 @@ tasks, Todoist tasks, web pages I've read and trails I've hiked has turned out to be sometimes cathartic and sometimes functional as I try to remember when I did a thing. -* Backlog [0/36] :vrobbler:project:personal: +* Backlog [1/36] :vrobbler:project:personal: ** TODO [#C] Configure IMAP folder/start in user profile :imap:settings: *** Description @@ -628,7 +628,10 @@ The Edit log form should have from top to bottom: - Location (which should be a drop down of BoardGameLocations for this user) ** TODO [#A] Dedup track scrobbles from lastfm import :importers:lastfm:tracks: -** TODO [#A] Write a script to check for raw mopidy vs found track discrepancies :tracks:metadata: +** DONE [#A] Write a script to check for raw mopidy vs found track discrepancies :tracks:metadata: +:PROPERTIES: +:ID: 0b98a1b9-2848-433e-8bec-0f5737cfc74b +:END: *** Description diff --git a/tests/music_tests/test_report_mismatched_metadata.py b/tests/music_tests/test_report_mismatched_metadata.py new file mode 100644 index 0000000..6e17c31 --- /dev/null +++ b/tests/music_tests/test_report_mismatched_metadata.py @@ -0,0 +1,145 @@ +import csv +from unittest.mock import patch + +import pytest +from django.contrib.auth import get_user_model +from django.core.management import call_command +from music.models import Album, Artist, Track +from scrobbles.models import Scrobble + + +def _write_fix_csv(path, track_ids): + with open(path, "w", newline="") as f: + writer = csv.writer(f) + writer.writerow( + [ + "track_id", + "track_artist_name", + "track_album_name", + "raw_artist", + "raw_album", + "source", + "mismatch", + ] + ) + for track_id in track_ids: + writer.writerow([track_id, "", "", "", "", "", ""]) + + +def _scrobble(user, track, raw_data): + return Scrobble.objects.create( + track=track, + media_type="Track", + user=user, + log={"raw_data": raw_data}, + ) + + +@pytest.mark.django_db +def test_report_writes_mismatch_csv(tmp_path): + user = get_user_model().objects.create(email="report@example.com") + wrong = Artist.objects.create(name="Wrong Artist") + track = Track.objects.create(title="Song", artist_fk=wrong) + track.artists.add(wrong) + _scrobble(user, track, {"artist": "Right Artist", "musicbrainz_artist_id": "mb-1"}) + + out = tmp_path / "report.csv" + call_command("report_mismatched_metadata", file_path=str(out)) + + with open(out) as f: + rows = list(csv.DictReader(f)) + assert len(rows) == 1 + assert rows[0]["track_id"] == str(track.id) + assert "artist" in rows[0]["mismatch"] + assert rows[0]["raw_artist"] == "Right Artist" + + +@pytest.mark.django_db +def test_fix_file_commits_resolved_artists(tmp_path): + user = get_user_model().objects.create(email="fix@example.com") + wrong = Artist.objects.create(name="Wrong Artist") + right = Artist.objects.create(name="Right Artist") + track = Track.objects.create(title="Song", artist_fk=wrong) + track.artists.add(wrong) + _scrobble(user, track, {"artist": "Right Artist", "musicbrainz_artist_id": "mb-1"}) + + fix = tmp_path / "fix.csv" + _write_fix_csv(fix, [track.id]) + call_command("report_mismatched_metadata", fix_file=str(fix), commit=True) + + track.refresh_from_db() + assert list(track.artists.all().values_list("name", flat=True)) == ["Right Artist"] + assert track.artist_fk.name == "Right Artist" + assert "metadata-fixed" in track.tags.names() + assert not Artist.objects.filter(name="Wrong Artist").exists() + + +@pytest.mark.django_db +def test_fix_file_dry_run_makes_no_changes(tmp_path): + user = get_user_model().objects.create(email="dry-fix@example.com") + wrong = Artist.objects.create(name="Wrong Artist") + right = Artist.objects.create(name="Right Artist") + track = Track.objects.create(title="Song", artist_fk=wrong) + track.artists.add(wrong) + _scrobble(user, track, {"artist": "Right Artist", "musicbrainz_artist_id": "mb-1"}) + + fix = tmp_path / "fix.csv" + _write_fix_csv(fix, [track.id]) + call_command("report_mismatched_metadata", fix_file=str(fix)) + + track.refresh_from_db() + assert sorted(track.artists.all().values_list("name", flat=True)) == [ + "Wrong Artist" + ] + assert not track.tags.filter(name="metadata-fixed").exists() + + +@pytest.mark.django_db +def test_fix_file_only_touches_listed_tracks(tmp_path): + user = get_user_model().objects.create(email="subset@example.com") + wrong = Artist.objects.create(name="Wrong Artist") + keep = Artist.objects.create(name="Keep Artist") + track_a = Track.objects.create(title="Song A", artist_fk=wrong) + track_a.artists.add(wrong) + track_b = Track.objects.create(title="Song B", artist_fk=wrong) + track_b.artists.add(wrong) + _scrobble( + user, track_a, {"artist": "Right Artist", "musicbrainz_artist_id": "mb-1"} + ) + _scrobble( + user, track_b, {"artist": "Right Artist", "musicbrainz_artist_id": "mb-1"} + ) + + fix = tmp_path / "fix.csv" + _write_fix_csv(fix, [track_a.id]) + call_command("report_mismatched_metadata", fix_file=str(fix), commit=True) + + track_a.refresh_from_db() + track_b.refresh_from_db() + assert list(track_a.artists.all().values_list("name", flat=True)) == [ + "Right Artist" + ] + assert sorted(track_b.artists.all().values_list("name", flat=True)) == [ + "Wrong Artist" + ] + + +@pytest.mark.django_db +@patch("music.models.Album.find_or_create") +def test_fix_file_fixes_album(mock_album, tmp_path): + user = get_user_model().objects.create(email="album@example.com") + artist = Artist.objects.create(name="Right Artist") + old_album = Album.objects.create(name="Old Album", album_artist=artist) + new_album = Album.objects.create(name="New Album", album_artist=artist) + mock_album.return_value = new_album + track = Track.objects.create(title="Song", artist_fk=artist, album=old_album) + track.artists.add(artist) + _scrobble(user, track, {"Artist": "Right Artist", "Album": "New Album"}) + + fix = tmp_path / "fix.csv" + _write_fix_csv(fix, [track.id]) + call_command("report_mismatched_metadata", fix_file=str(fix), commit=True) + + track.refresh_from_db() + assert track.album == new_album + mock_album.assert_called_once_with("New Album", "Right Artist") diff --git a/vrobbler/apps/music/management/commands/report_mismatched_metadata.py b/vrobbler/apps/music/management/commands/report_mismatched_metadata.py index cbccba1..87f83c0 100644 --- a/vrobbler/apps/music/management/commands/report_mismatched_metadata.py +++ b/vrobbler/apps/music/management/commands/report_mismatched_metadata.py @@ -2,11 +2,15 @@ import csv import logging from django.core.management.base import BaseCommand +from django.db import transaction +from music.models import Album, Artist, Track from music.utils import ( album_mismatch, artist_mismatch, get_artist_source, get_raw_artist_data, + normalize_name, + resolve_artist_names, ) logger = logging.getLogger(__name__) @@ -15,7 +19,10 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): help = ( "Outputs a CSV of track IDs where raw metadata from scrobble logs " - "does not match the track's stored artists or album" + "does not match the track's stored artists or album. The CSV can be " + "edited (delete the rows for tracks you do NOT want touched) and " + "passed back via --fix-file to resolve only the listed tracks from " + "their raw scrobble data." ) def add_arguments(self, parser): @@ -25,8 +32,30 @@ class Command(BaseCommand): default="/tmp/metadata-report.csv", help="Output CSV file path (default: /tmp/metadata-report.csv)", ) + parser.add_argument( + "--fix-file", + type=str, + default="", + help=( + "Path to a CSV produced by this command. When set, only the " + "track_ids in the CSV are resolved from raw scrobble data." + ), + ) + parser.add_argument( + "--commit", + action="store_true", + help="Commit fixes to the database (dry run by default)", + ) def handle(self, *args, **options): + if options["fix_file"]: + self._fix(options) + else: + self._report(options) + + # -- report ---------------------------------------------------------- + + def _report(self, options): from scrobbles.models import Scrobble file_path = options["file_path"] @@ -37,7 +66,7 @@ class Command(BaseCommand): .exclude(log={}) .select_related("track__album") .prefetch_related("track__artists") - .iterator() + .iterator(chunk_size=2000) ) rows = [] @@ -63,9 +92,14 @@ class Command(BaseCommand): ] track_album_name = track.album.name if track.album else "" - if artist_mismatch(raw_artist, track_artist_names) or album_mismatch( - raw_album, track_album_name - ): + artist_bad = artist_mismatch(raw_artist, track_artist_names) + album_bad = album_mismatch(raw_album, track_album_name) + if artist_bad or album_bad: + issues = [] + if artist_bad: + issues.append("artist") + if album_bad: + issues.append("album") rows.append( { "track_id": track.id, @@ -74,6 +108,7 @@ class Command(BaseCommand): "raw_artist": raw_artist, "raw_album": raw_album, "source": source, + "mismatch": "+".join(issues), } ) @@ -84,6 +119,7 @@ class Command(BaseCommand): "raw_artist", "raw_album", "source", + "mismatch", ] with open(file_path, "w", newline="") as f: writer = csv.DictWriter(f, fieldnames=fieldnames) @@ -93,3 +129,164 @@ class Command(BaseCommand): self.stdout.write( self.style.SUCCESS(f"Wrote {len(rows)} mismatched track(s) to {file_path}") ) + + # -- fix ------------------------------------------------------------- + + def _load_fix_ids(self, fix_file: str) -> list[int]: + ids = [] + with open(fix_file, newline="") as f: + for row in csv.DictReader(f): + track_id = (row.get("track_id") or "").strip() + if track_id: + ids.append(int(track_id)) + return ids + + def _resolve_names( + self, cache: dict, raw_artist: str, artist_mbid: str + ) -> list[str]: + key = (normalize_name(raw_artist), artist_mbid) + if key not in cache: + cache[key] = resolve_artist_names(raw_artist, artist_mbid) + return cache[key] + + def _resolve_artists( + self, cache: dict, expected_names: list[str], track + ) -> list[Artist]: + artists = [] + for name in expected_names: + if name not in cache: + cache[name] = Artist.objects.filter(name=name).first() + if not cache[name]: + cache[name] = Artist.find_or_create(name, track_name=track.title) + if cache[name]: + artists.append(cache[name]) + return artists + + def _artist_is_orphan(self, artist: Artist) -> bool: + if Track.objects.filter(artist_fk=artist).exists(): + return False + if Track.objects.filter(artists=artist).exists(): + return False + if Album.objects.filter(album_artist=artist).exists(): + return False + if Album.objects.filter(artists=artist).exists(): + return False + return True + + def _fix(self, options): + from scrobbles.models import Scrobble + + commit = options["commit"] + fix_file = options["fix_file"] + ids = self._load_fix_ids(fix_file) + if not ids: + self.stdout.write(self.style.ERROR(f"No track_ids found in {fix_file}")) + return + + self.stdout.write(f"Fixing {len(ids)} track(s) listed in {fix_file}") + if not commit: + self.stdout.write( + "Dry run — no changes will be saved. Use --commit to apply." + ) + + resolution_cache = {} + artist_cache = {} + fixed = 0 + skipped = 0 + orphaned_ids = set() + + tracks = ( + Track.objects.filter(id__in=ids) + .select_related("album") + .prefetch_related("artists") + .order_by("id") + ) + with transaction.atomic(): + for track in tracks: + scrobble = ( + Scrobble.objects.filter(track=track) + .exclude(log__isnull=True) + .exclude(log={}) + .order_by("-created") + .first() + ) + raw_data = (scrobble.log if scrobble else {}).get("raw_data") + if not raw_data: + self.stdout.write( + f" Skip track {track.id} '{track.title}': no raw_data" + ) + skipped += 1 + continue + + raw_artist, raw_album, artist_mbid = get_raw_artist_data(raw_data) + if not raw_artist and not raw_album: + skipped += 1 + continue + + stored_names = [ + n for n in track.artists.all().values_list("name", flat=True) + ] + if track.artist_fk and track.artist_fk.name not in stored_names: + stored_names.append(track.artist_fk.name) + stored_album = track.album.name if track.album else "" + + artist_bad = artist_mismatch(raw_artist, stored_names) + album_bad = album_mismatch(raw_album, stored_album) + if not artist_bad and not album_bad: + skipped += 1 + continue + + fixed += 1 + self.stdout.write( + f" Track {track.id} '{track.title}': " + f"artists {stored_names} -> '{raw_artist}', " + f"album '{stored_album}' -> '{raw_album}'" + ) + + if not commit: + continue + + if artist_bad and raw_artist: + expected_names = self._resolve_names( + resolution_cache, raw_artist, artist_mbid or "" + ) + artists = self._resolve_artists(artist_cache, expected_names, track) + if artists: + old_artist_ids = set( + track.artists.all().values_list("id", flat=True) + ) + track.artists.set(artists) + track.artist_fk = artists[0] + track.save(update_fields=["artist_fk"]) + orphaned_ids.update(old_artist_ids - {a.id for a in artists}) + + if album_bad and raw_album: + try: + album = Album.find_or_create(raw_album, raw_artist) + except Exception as e: + logger.warning( + f"Could not resolve album '{raw_album}' for track " + f"{track.id}: {e}" + ) + album = None + if album: + track.album = album + track.save(update_fields=["album"]) + + track.tags.add("metadata-fixed") + + deleted = 0 + if commit and orphaned_ids: + for artist_id in sorted(orphaned_ids): + artist = Artist.objects.filter(id=artist_id).first() + if artist and self._artist_is_orphan(artist): + artist.delete() + deleted += 1 + self.stdout.write(f" Deleted orphaned artist '{artist.name}'") + + self.stdout.write( + f"\nResults (commit={commit}):\n" + f" Tracks fixed: {fixed}\n" + f" Tracks skipped: {skipped}\n" + f" Orphaned artists deleted: {deleted}" + )