diff --git a/CHANGELOG.org b/CHANGELOG.org index 45b8a60..e0aaf51 100644 --- a/CHANGELOG.org +++ b/CHANGELOG.org @@ -1,6 +1,6 @@ #+title: CHANGELOG -* Version 65.0 [5/5] +* Version 65.0 [7/7] ** DONE [#B] Investigate how historice lastfm imports work :importers:music: :PROPERTIES: :ID: f91fdd53-9da7-4859-9322-f08b2c587061 @@ -145,6 +145,32 @@ scripts with distinct nodenames). Procfile, justfile, Makefile, Drone and Gitea workflows updated accordingly. +** DONE [#A] Before enriching anything, trust the POST data :feature:scrobbles:metadata: +:PROPERTIES: +:ID: db6b05f8-09f4-49f5-9838-fbacc9fe9cd0 +:END: + +Jellyfin/Mopidy webhook scrobbles now trust the POST data instead of blocking +on third-party enrichment. `Track.find_or_create(..., trust_webhook_data=True)` +builds tracks purely from webhook data (no MusicBrainz lookups in the request +path), reusing any existing track found by `musicbrainz_id`. Scrobbles built +this way are tagged `webhook-metadata-only` and a background celery task +(`scrobbles.tasks.enrich_media_from_webhook`) enriches them asynchronously from +the payload in `log["raw_data"]`. + +The async task runs the newly-implemented `Track.fix_metadata()` (MusicBrainz +resolution, run time, featured artists, album linkage). If resolution matches an +existing track by musicbrainz_id, `Track.merge()` repoints scrobbles/favorites +onto the canonical track and deletes the webhook-created duplicate (skipping +in-progress conflicts). Success tags the scrobble `webhook-enriched`; failure +tags it `enrichment-failed` and logs a warning. Notifications still blast the +raw-data track at scrobble time. + +Also fixed a typo in `MOPIDY_POST_KEYS["ARTIST_MB_ID"]` +(`muscibrainz_artist_id` → `musicbrainz_artist_id`). Since enrichment now runs +off the request path, slower subtler matching rules (e.g. a future +`LITERAL_ARTIST_TITLES` exception list) can live in `fix_metadata`. + * Version 64.5 [1/1] ** DONE [#B] Clean up issues with org-mode notes :orgmode:notes:scrobbles: :PROPERTIES: diff --git a/PROJECT.org b/PROJECT.org index 45a1892..feb4255 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 [2/26] :vrobbler:project:personal: +* Backlog [3/26] :vrobbler:project:personal: ** TODO [#C] Configure IMAP folder/start in user profile :imap:settings: *** Description @@ -623,7 +623,30 @@ Certain artists like "Simon & Garfunkel" are actually one artist. While we don't tracks into featured artists, we should have a "LITERAL_ARTIST_TITLES" constant that can have exceptions like this put into it and then we stop trying to pull the artist apart when we run into it. -** TODO [#A] Before enriching anything, trust the POST data :feature:scrobbles:metadata: +** TODO [#A] Update how board game scrobbles work :boardgames: + +*** Description + +When we scrobble a board game from a BGG URL, instead of going to the media +detail page, we should go to the scrobble detail page, with the Edit Log form +expanded by default. + +The Edit log form should have from top to bottom: + + - Board/Variant (one or many BoardGameVariant in a multi-select widget) + - People (which should be similar to the Bird widget on BirdLocation and allow setting per user score, win true/false, rank, new true/false, seat_ordrer) + - Expansion ids (which should a multi-select widget of expansions for this game) + - 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: + +*** Description + +There should be a tool that can look at the track associated with a scrobble and +the actual raw_data from mopidy or jellyfin, and flag all tracks where the +artist or track name or both differ. +** DONE [#A] Before enriching anything, trust the POST data :feature:scrobbles:metadata: :PROPERTIES: :ID: db6b05f8-09f4-49f5-9838-fbacc9fe9cd0 :END: @@ -644,46 +667,29 @@ async with the POST data stored in the log["raw_data"] and used by the celery en to go try to enrich the media instance. Should this enrichment fail, tag the scrobble as "enrichment-failed" log a warning and move on. -** TODO [#A] Update how board game scrobbles work :boardgames: +*** Implementation -*** Description +Implemented for tracks (videos deferred for a follow-up). Jellyfin/Mopidy webhook scrobbles now: -When we scrobble a board game from a BGG URL, instead of going to the media -detail page, we should go to the scrobble detail page, with the Edit Log form -expanded by default. +- `Track.find_or_create(..., trust_webhook_data=True)` builds the track purely from POST data + with zero MusicBrainz lookups in the webhook path. Existing tracks found by `musicbrainz_id` + are reused without re-enriching; otherwise the track is stamped with the POST-provided + artist/album/track mbids and run time and tagged `musicbrainz-provider`. +- The scrobble is tagged `webhook-metadata-only` and a new background celery task + `scrobbles.tasks.enrich_media_from_webhook` is dispatched to enrich asynchronously using the + POST payload in `log["raw_data"]`. +- The async task runs the newly-implemented `Track.fix_metadata()` (MusicBrainz resolution, run + time, featured artists, album linkage). If resolution matches an existing track by + musicbrainz_id, `Track.merge()` repoints scrobbles/favorites onto the canonical track and + deletes the webhook-created duplicate (skipping any in-progress conflicts). +- Success tags the scrobble `webhook-enriched`; failure tags it `enrichment-failed` and logs a + warning. Notifications still blast out with the raw-data track at scrobble time; a later + repoint to a canonical track may show in a subsequent stop notification, which is accepted. -The Edit log form should have from top to bottom: +Also fixed a typo in `MOPIDY_POST_KEYS["ARTIST_MB_ID"]` (`muscibrainz_artist_id` → +`musicbrainz_artist_id`). The slow, subtler matching rules (e.g. a future +`LITERAL_ARTIST_TITLES` exception list) can now live in `fix_metadata` off the request path. - - Board/Variant (one or many BoardGameVariant in a multi-select widget) - - People (which should be similar to the Bird widget on BirdLocation and allow setting per user score, win true/false, rank, new true/false, seat_ordrer) - - Expansion ids (which should a multi-select widget of expansions for this game) - - Location (which should be a drop down of BoardGameLocations for this user) - -** TODO [#A] Add trends tests for concurrent trends :trends:tests:concurrent: -:PROPERTIES: -:ID: ca613753-d203-4429-b062-36e3312bca82 -:END: - -*** Description - -Added a `workouts` Django app so we can scrobble gym sessions. - -- New `Exercise` model with the wrkout/exercises.json catalog (873 exercises), - imported via `python manage.py import_wrkout_exercises`. Image files come from - inside the tarball and are stored via ImageKit specs. -- New `WorkoutRoutine` model (ScrobblableMixin, media_type_label "Workout"), - scrobbled through the existing Scrobble system with a `workout_routine` FK and - `WorkoutLogData` storing sets/reps/weight per exercise plus duration, bodyweight - and RPE. -- Weights are stored canonically in kg; the profile's `weigh_in_units` setting - drives form input and display units. A generic - `apply_media_unit_conversions` helper in scrobbles/utils.py handles both - drinks (`size_ml`) and workout weights, fixing the old GET-only drinks - conversion so imperial POSTs store kg. -- Templates, admin, DRF viewsets (`exercises`, `workout-routines`), MCP tools, - and `tests/workouts_tests/` (unit conversion, form round-trips, importer, - imperial POST) included. -** TODO [#A] Dedup track scrobbles from lastfm import :importers:lastfm:tracks: ** DONE [#A] Fix celery task prioritization especially for agent sessions :celery:tasks:agents: :PROPERTIES: :ID: 672c81bf-bba9-a963-e8ac-52246d976cea diff --git a/tests/scrobbles_tests/test_aggregators.py b/tests/scrobbles_tests/test_aggregators.py index 088efd4..a5a9b51 100644 --- a/tests/scrobbles_tests/test_aggregators.py +++ b/tests/scrobbles_tests/test_aggregators.py @@ -11,8 +11,8 @@ from scrobbles.models import Scrobble def build_scrobbles(client, request_json, num=7, spacing=2, auth_token=None): - from rest_framework.authtoken.models import Token import pytz + from rest_framework.authtoken.models import Token url = reverse("scrobbles:mopidy-webhook") user = get_user_model().objects.create(username="Test User") @@ -43,14 +43,12 @@ def build_scrobbles(client, request_json, num=7, spacing=2, auth_token=None): @pytest.mark.django_db @patch("music.models.get_album_metadata_with_artist", return_value={}) @patch("music.models.get_track_metadata_with_artist", return_value={}) -@patch("music.models.get_recording_mbid_exact", return_value=(None, None)) @patch("music.models.lookup_artist_from_tadb", return_value={}) @patch("music.models.lookup_album_from_tadb", return_value={}) @time_machine.travel(datetime(2022, 3, 4, 1, 24)) def test_scrobble_counts_data( mock_lookup_album_tadb, mock_lookup_artist_tadb, - mock_get_recording, mock_get_track, mock_get_album, client, @@ -73,14 +71,12 @@ def test_scrobble_counts_data( @pytest.mark.django_db @patch("music.models.get_album_metadata_with_artist", return_value={}) @patch("music.models.get_track_metadata_with_artist", return_value={}) -@patch("music.models.get_recording_mbid_exact", return_value=(None, None)) @patch("music.models.lookup_artist_from_tadb", return_value={}) @patch("music.models.lookup_album_from_tadb", return_value={}) @time_machine.travel(datetime(2022, 3, 4, 1, 24)) def test_live_charts( mock_lookup_album_tadb, mock_lookup_artist_tadb, - mock_get_recording, mock_get_track, mock_get_album, client, diff --git a/tests/scrobbles_tests/test_scrobblers.py b/tests/scrobbles_tests/test_scrobblers.py index 0b67c8c..ed343c7 100644 --- a/tests/scrobbles_tests/test_scrobblers.py +++ b/tests/scrobbles_tests/test_scrobblers.py @@ -56,4 +56,8 @@ def test_mopidy_scrobble_handles_missing_mopidy_uri(): artist_name="Test Artist", album_name="Test Album", run_time_seconds=180000, + mbid=None, + artist_mbid=None, + album_mbid=None, + trust_webhook_data=True, ) diff --git a/tests/scrobbles_tests/test_webhook_enrichment.py b/tests/scrobbles_tests/test_webhook_enrichment.py new file mode 100644 index 0000000..ef9a574 --- /dev/null +++ b/tests/scrobbles_tests/test_webhook_enrichment.py @@ -0,0 +1,202 @@ +from unittest.mock import patch + +import pytest +from django.contrib.auth import get_user_model +from django.urls import reverse +from music.models import Album, Artist, Track +from scrobbles.models import FavoriteMedia, Scrobble +from scrobbles.tasks import enrich_media_from_webhook + + +@pytest.mark.django_db +@patch("music.models.resolve_track") +def test_find_or_create_trust_webhook_data_skips_musicbrainz(mock_resolve): + track = Track.find_or_create( + title="TNT", + artist_name="AC/DC", + album_name="TNT", + run_time_seconds=210, + mbid="rec-123", + artist_mbid="art-1", + album_mbid="alb-1", + trust_webhook_data=True, + ) + + assert track.musicbrainz_id == "rec-123" + assert track.base_run_time_seconds == 210 + assert "musicbrainz-provider" in track.tags.names() + assert "musicbrainz-enriched" not in track.tags.names() + mock_resolve.assert_not_called() + assert Artist.objects.filter(name="AC/DC", musicbrainz_id="art-1").exists() + assert Album.objects.filter(name="TNT", musicbrainz_id="alb-1").exists() + + +@pytest.mark.django_db +@patch("music.models.resolve_track") +def test_find_or_create_reuses_existing_track_by_mbid(mock_resolve): + existing = Track.objects.create(title="TNT", musicbrainz_id="rec-123") + + track = Track.find_or_create( + title="TNT", + artist_name="AC/DC", + mbid="rec-123", + trust_webhook_data=True, + ) + + assert track.id == existing.id + mock_resolve.assert_not_called() + + +@pytest.mark.django_db +@patch("music.models.resolve_track") +def test_fix_metadata_enriches_and_tags(mock_resolve): + track = Track.objects.create(title="TNT") + mock_resolve.return_value = ( + {"recording_mbid": "rec-123", "length_ms": 210000}, + "exact", + ) + + assert track.fix_metadata() is True + + track.refresh_from_db() + assert track.musicbrainz_id == "rec-123" + assert track.base_run_time_seconds == 210 + assert "musicbrainz-exact" in track.tags.names() + assert "musicbrainz-enriched" in track.tags.names() + + +@pytest.mark.django_db +@patch("music.models.resolve_track", return_value=(None, "")) +def test_fix_metadata_notfound_returns_false(mock_resolve): + track = Track.objects.create(title="Unknown") + + assert track.fix_metadata() is False + assert "musicbrainz-notfound" in track.tags.names() + + +@pytest.mark.django_db +def test_track_merge_repoints_scrobbles_and_favorites(): + user = get_user_model().objects.create(email="merge@example.com") + duplicate = Track.objects.create(title="TNT", musicbrainz_id="rec-123") + canonical = Track.objects.create(title="TNT", musicbrainz_id="rec-123") + duplicate.tags.add("cool") + scrobble = Scrobble.objects.create( + track=duplicate, media_type="Track", user=user, in_progress=False + ) + fav = FavoriteMedia.objects.create(user=user, track=duplicate, media_type="Track") + + repointed = Track.merge(duplicate, canonical) + + assert repointed == 1 + scrobble.refresh_from_db() + assert scrobble.track == canonical + fav.refresh_from_db() + assert fav.track == canonical + assert "cool" in canonical.tags.names() + assert not Track.objects.filter(id=duplicate.id).exists() + + +@pytest.mark.django_db +def test_track_merge_skips_in_progress_conflict(): + user = get_user_model().objects.create(email="conflict@example.com") + duplicate = Track.objects.create(title="TNT", musicbrainz_id="rec-123") + canonical = Track.objects.create(title="TNT", musicbrainz_id="rec-123") + Scrobble.objects.create( + track=canonical, media_type="Track", user=user, in_progress=True + ) + scrobble = Scrobble.objects.create( + track=duplicate, media_type="Track", user=user, in_progress=True + ) + + repointed = Track.merge(duplicate, canonical) + + assert repointed == 0 + scrobble.refresh_from_db() + assert scrobble.track == duplicate + assert Track.objects.filter(id=duplicate.id).exists() + + +@pytest.mark.django_db +@patch("scrobbles.tasks.enrich_media_from_webhook") +def test_mopidy_webhook_creates_track_from_post_data_and_dispatches( + mock_task, client, mopidy_track, valid_auth_token +): + url = reverse("scrobbles:mopidy-webhook") + headers = {"Authorization": f"Token {valid_auth_token}"} + response = client.post( + url, + mopidy_track.request_data, + content_type="application/json", + headers=headers, + ) + assert response.status_code == 200 + + scrobble = Scrobble.objects.get(id=response.data["scrobble_id"]) + assert scrobble.track.title == "Same in the End" + assert scrobble.track.musicbrainz_id == mopidy_track.musicbrainz_track_id + assert "webhook-metadata-only" in scrobble.tags.names() + mock_task.delay.assert_called_once_with(scrobble.id) + + +@pytest.mark.django_db +@patch("scrobbles.tasks.enrich_media_from_webhook") +def test_jellyfin_webhook_creates_track_from_post_data_and_dispatches( + mock_task, client, jellyfin_track, valid_auth_token +): + url = reverse("scrobbles:jellyfin-webhook") + headers = {"Authorization": f"Token {valid_auth_token}"} + response = client.post( + url, + jellyfin_track.request_json, + content_type="application/json", + headers=headers, + ) + assert response.status_code == 200 + + scrobble = Scrobble.objects.get(id=response.data["scrobble_id"]) + assert scrobble.track.title == "Emotion" + assert scrobble.track.musicbrainz_id == jellyfin_track.musicbrainz_track_id + assert "webhook-metadata-only" in scrobble.tags.names() + mock_task.delay.assert_called_once_with(scrobble.id) + + +@pytest.mark.django_db +@patch("music.models.resolve_track") +def test_enrich_task_merges_onto_canonical_track(mock_resolve): + user = get_user_model().objects.create(email="task@example.com") + artist = Artist.objects.create(name="AC-DC") + canonical = Track.objects.create( + title="TNT", musicbrainz_id="rec-123", base_run_time_seconds=210 + ) + canonical.artists.add(artist) + canonical.tags.add("musicbrainz-enriched") + + raw = Track.objects.create(title="TNT", artist_fk=artist) + raw.artists.add(artist) + scrobble = Scrobble.objects.create(track=raw, media_type="Track", user=user) + + mock_resolve.return_value = ( + {"recording_mbid": "rec-123", "length_ms": 210000}, + "exact", + ) + + enrich_media_from_webhook(scrobble.id) + + scrobble.refresh_from_db() + assert scrobble.track == canonical + assert "webhook-enriched" in scrobble.tags.names() + assert not Track.objects.filter(id=raw.id).exists() + + +@pytest.mark.django_db +@patch("music.models.resolve_track", return_value=(None, "")) +def test_enrich_task_tags_failure(mock_resolve): + user = get_user_model().objects.create(email="fail@example.com") + track = Track.objects.create(title="Unknown Song") + scrobble = Scrobble.objects.create(track=track, media_type="Track", user=user) + + enrich_media_from_webhook(scrobble.id) + + scrobble.refresh_from_db() + assert "enrichment-failed" in scrobble.tags.names() + assert "musicbrainz-notfound" in track.tags.names() diff --git a/vrobbler/apps/music/constants.py b/vrobbler/apps/music/constants.py index ea470fb..9900067 100644 --- a/vrobbler/apps/music/constants.py +++ b/vrobbler/apps/music/constants.py @@ -111,7 +111,7 @@ MOPIDY_POST_KEYS = { "TIMESTAMP": None, "YEAR": None, "PLAYBACK_POSITION_TICKS": "playback_time_ticks", - "ARTIST_MB_ID": "muscibrainz_artist_id", + "ARTIST_MB_ID": "musicbrainz_artist_id", "ALBUM_MB_ID": "musicbrainz_album_id", "RELEASEGROUP_MB_ID": None, "TRACK_MB_ID": "musicbrainz_track_id", diff --git a/vrobbler/apps/music/models.py b/vrobbler/apps/music/models.py index 87de551..c6677c9 100644 --- a/vrobbler/apps/music/models.py +++ b/vrobbler/apps/music/models.py @@ -18,7 +18,6 @@ from music.bandcamp import get_bandcamp_slug from music.musicbrainz import ( extract_featured_artists, get_album_metadata_with_artist, - get_recording_mbid_exact, get_track_metadata_with_artist, resolve_track, ) @@ -637,13 +636,24 @@ class Track(ScrobblableMixin): mbid: str | None = None, enrich: bool = False, commit: bool = True, + artist_mbid: str | None = None, + album_mbid: str | None = None, + trust_webhook_data: bool = False, ) -> "Track": """Given a name, try to find the track by the artist from Musicbrainz. As a basic conceit we trust the source for giving us the track and artist name - Optionally, we can update any found artists with overwrite.""" + Optionally, we can update any found artists with overwrite. + + When ``trust_webhook_data`` is set (e.g. for Jellyfin/Mopidy webhooks) + the track is built purely from the provided POST data with no MusicBrainz + lookups, so scrobbling is never blocked on network calls. An existing + track already known by ``musicbrainz_id`` is returned as-is without + further enrichment; otherwise the track is created and stamped with the + provided mbids, and async enrichment can pick it up later via + :meth:`fix_metadata`.""" from music.utils import parse_artist_names if artist_names is None and artist_name: @@ -652,17 +662,52 @@ class Track(ScrobblableMixin): if not artist_names: artist_names = [] - album = None - if album_name: - logger.info(f"Looking up album for: {album_name}") - first_name = artist_names[0] if artist_names else "" - album = Album.find_or_create(name=album_name, artist_name=first_name) + # Trust-the-webhook fast path: if we already know this track by its + # musicbrainz_id, reuse it and skip enrichment entirely. + if trust_webhook_data and mbid: + existing = cls.objects.filter(musicbrainz_id=mbid).first() + if existing: + logger.info( + "Found existing track by mbid, not enriching", + extra={"mbid": mbid, "track_id": existing.id}, + ) + return existing + album = None artist_objs = [] - for name in artist_names: - artist = Artist.find_or_create(name, track_name=title) - if artist: + if trust_webhook_data: + if album_mbid: + album = Album.objects.filter(musicbrainz_id=album_mbid).first() + if not album and album_name: + album = Album.objects.filter(name=album_name).first() + if not album and album_name: + album = Album.objects.create(name=album_name) + if album_mbid: + album.musicbrainz_id = album_mbid + album.save() + + for name in artist_names: + artist = None + if artist_mbid: + artist = Artist.objects.filter(musicbrainz_id=artist_mbid).first() + if not artist: + artist = Artist.objects.filter(name=clean_artist_name(name)).first() + if not artist: + artist = Artist.objects.create(name=clean_artist_name(name)) + if artist_mbid: + artist.musicbrainz_id = artist_mbid + artist.save() artist_objs.append(artist) + else: + if album_name: + logger.info(f"Looking up album for: {album_name}") + first_name = artist_names[0] if artist_names else "" + album = Album.find_or_create(name=album_name, artist_name=first_name) + + for name in artist_names: + artist = Artist.find_or_create(name, track_name=title) + if artist: + artist_objs.append(artist) track = None if artist_objs: @@ -682,41 +727,142 @@ class Track(ScrobblableMixin): if album: track.albums.add(album) - if enrich or not track.base_run_time_seconds: - if mbid and run_time_seconds: - track.base_run_time_seconds = run_time_seconds + if trust_webhook_data: + if mbid: track.musicbrainz_id = mbid - track.tags.add("musicbrainz-provider", "musicbrainz-enriched") - else: - artist_name_str = " & ".join(artist_names) if artist_names else "" - logger.info( - f"Enriching track {track}", - extra={ - "title": title, - "artist_name": artist_name_str, - "track_id": track.id, - }, - ) - result, method = resolve_track(title, artist_name_str, album_name) - if result and result.get("recording_mbid"): - track.musicbrainz_id = result["recording_mbid"] - length_ms = result.get("length_ms") - if length_ms and not track.base_run_time_seconds: - track.base_run_time_seconds = int(int(length_ms) / 1000) - method_tag = f"musicbrainz-{method}" if method else "" - track.tags.add(method_tag, "musicbrainz-enriched") - cleaned_title, featured_names = extract_featured_artists(title) - for feat_name in featured_names: - artist = Artist.find_or_create(feat_name, track_name=title) - if artist: - track.artists.add(artist) - else: - print("No musicbrainz result found, cannot enrich") - track.tags.add("musicbrainz-notfound") - return track + if run_time_seconds: + track.base_run_time_seconds = run_time_seconds + if mbid or run_time_seconds: + track.tags.add("musicbrainz-provider") + if commit: + track.save() + return track + + if enrich or not track.base_run_time_seconds: + track.fix_metadata() if commit: track.save() return track - def fix_metadata(self, force_update=False): ... + def fix_metadata(self, force_update: bool = False) -> bool: + """Resolve and enrich this track's metadata from MusicBrainz in place. + + Sets ``musicbrainz_id`` and ``base_run_time_seconds``, adds any + featured artists and tags the track with the resolution method. Skips + already-enriched tracks (tagged ``musicbrainz-enriched``) unless + ``force_update`` is set. Returns True when enrichment succeeded, False + otherwise. + + This is safe to call off the request path (e.g. from the async webhook + enrichment task), where it can afford slower lookups and subtler + matching rules.""" + from music.utils import parse_artist_names + + if "musicbrainz-enriched" in self.tags.names() and not force_update: + logger.info( + f"Track {self} already enriched, skipping", + extra={"track_id": self.id}, + ) + return True + + artist_name_str = "" + if self.artist: + artist_name_str = " & ".join(parse_artist_names(str(self.artist))) + album_name = "" + if self.primary_album: + album_name = self.primary_album.name + + logger.info( + f"Enriching track {self}", + extra={ + "title": self.title, + "artist_name": artist_name_str, + "track_id": self.id, + }, + ) + result, method = resolve_track(self.title, artist_name_str, album_name) + if not result or not result.get("recording_mbid"): + logger.warning(f"No musicbrainz result found, cannot enrich {self}") + self.tags.add("musicbrainz-notfound") + return False + + self.musicbrainz_id = result["recording_mbid"] + length_ms = result.get("length_ms") + if length_ms and not self.base_run_time_seconds: + self.base_run_time_seconds = int(int(length_ms) / 1000) + method_tag = f"musicbrainz-{method}" if method else "" + self.tags.add(method_tag, "musicbrainz-enriched") + cleaned_title, featured_names = extract_featured_artists(self.title) + for feat_name in featured_names: + artist = Artist.find_or_create(feat_name, track_name=self.title) + if artist: + self.artists.add(artist) + self.save() + return True + + @classmethod + def merge(cls, duplicate: "Track", canonical: "Track") -> int: + """Merge a duplicate track into its canonical match. + + Repoints any scrobbles and favorites from ``duplicate`` onto + ``canonical``, copies tags, artists and albums over, then deletes the + duplicate. Scrobbles for a user who already has an in-progress + scrobble of the canonical track are left alone to avoid two concurrent + in-progress scrobbles. Returns the number of scrobbles repointed.""" + if duplicate.id == canonical.id: + return 0 + + from scrobbles.models import FavoriteMedia, Scrobble + + logger.info( + f"Merging track {duplicate} into {canonical}", + extra={"duplicate_id": duplicate.id, "canonical_id": canonical.id}, + ) + + repointed = 0 + conflict_ids = [] + for scrobble in Scrobble.objects.filter(track=duplicate): + conflict = Scrobble.objects.filter( + track=canonical, user_id=scrobble.user_id, in_progress=True + ).exists() + if conflict and scrobble.in_progress: + conflict_ids.append(scrobble.id) + logger.info( + "Skipping merge of in-progress scrobble due to conflict", + extra={"scrobble_id": scrobble.id, "canonical_id": canonical.id}, + ) + continue + scrobble.track = canonical + scrobble.save(update_fields=["track"]) + repointed += 1 + + FavoriteMedia.objects.filter(track=duplicate).update(track=canonical) + + for tag in duplicate.tags.all(): + canonical.tags.add(tag.name) + for genre in duplicate.genre.all(): + canonical.genre.add(genre.name) + for artist in duplicate.artists.all(): + canonical.artists.add(artist) + for album in duplicate.albums.all(): + canonical.albums.add(album) + + if not canonical.album and duplicate.album: + canonical.album = duplicate.album + if not canonical.musicbrainz_id and duplicate.musicbrainz_id: + canonical.musicbrainz_id = duplicate.musicbrainz_id + if not canonical.base_run_time_seconds and duplicate.base_run_time_seconds: + canonical.base_run_time_seconds = duplicate.base_run_time_seconds + canonical.save() + + if conflict_ids: + logger.warning( + "Not deleting duplicate; %s in-progress scrobbles reference it", + len(conflict_ids), + extra={"duplicate_id": duplicate.id, "canonical_id": canonical.id}, + ) + return repointed + + duplicate.delete() + return repointed diff --git a/vrobbler/apps/scrobbles/constants.py b/vrobbler/apps/scrobbles/constants.py index 7e06278..0a5f980 100644 --- a/vrobbler/apps/scrobbles/constants.py +++ b/vrobbler/apps/scrobbles/constants.py @@ -13,6 +13,13 @@ class Visibility(models.TextChoices): JELLYFIN_AUDIO_ITEM_TYPES = ["Audio"] +# Tags applied to scrobbles created from webhook POST data whose media has +# not been enriched yet. Enrichment happens asynchronously; success moves the +# scrobble to WEBHOOK_ENRICHED_TAG, failure to ENRICHMENT_FAILED_TAG. +WEBHOOK_METADATA_ONLY_TAG = "webhook-metadata-only" +WEBHOOK_ENRICHED_TAG = "webhook-enriched" +ENRICHMENT_FAILED_TAG = "enrichment-failed" + LONG_PLAY_MEDIA = { "videogames": "VideoGame", "books": "Book", diff --git a/vrobbler/apps/scrobbles/scrobblers.py b/vrobbler/apps/scrobbles/scrobblers.py index 7871346..01b8a7d 100644 --- a/vrobbler/apps/scrobbles/scrobblers.py +++ b/vrobbler/apps/scrobbles/scrobblers.py @@ -88,6 +88,10 @@ def mopidy_scrobble_media(post_data: dict, user_id: int) -> Scrobble: artist_name=post_data.get("artist", ""), album_name=post_data.get("album", ""), run_time_seconds=post_data.get("run_time", 900000), + mbid=post_data.get(MOPIDY_POST_KEYS.get("TRACK_MB_ID")), + artist_mbid=post_data.get(MOPIDY_POST_KEYS.get("ARTIST_MB_ID")), + album_mbid=post_data.get(MOPIDY_POST_KEYS.get("ALBUM_MB_ID")), + trust_webhook_data=True, ) try: album_id = Album.objects.filter(name=post_data.get("album", "")).first().id @@ -106,7 +110,7 @@ def mopidy_scrobble_media(post_data: dict, user_id: int) -> Scrobble: if album_id: log["album_id"] = album_id - return media_obj.scrobble_for_user( + scrobble = media_obj.scrobble_for_user( user_id, source="Mopidy", playback_position_seconds=int( @@ -116,6 +120,11 @@ def mopidy_scrobble_media(post_data: dict, user_id: int) -> Scrobble: log=log, ) + if media_type == Scrobble.MediaType.TRACK and scrobble: + _queue_webhook_enrichment(scrobble) + + return scrobble + def jellyfin_scrobble_media(post_data: dict, user_id: int) -> Optional[Scrobble]: media_type = Scrobble.MediaType.VIDEO @@ -152,6 +161,10 @@ def jellyfin_scrobble_media(post_data: dict, user_id: int) -> Optional[Scrobble] artist_name=post_data.get("Artist", ""), album_name=post_data.get("Album", ""), run_time_seconds=convert_to_seconds(post_data.get("RunTime", 900000)), + mbid=post_data.get(JELLYFIN_POST_KEYS.get("TRACK_MB_ID")), + artist_mbid=post_data.get(JELLYFIN_POST_KEYS.get("ARTIST_MB_ID")), + album_mbid=post_data.get(JELLYFIN_POST_KEYS.get("ALBUM_MB_ID")), + trust_webhook_data=True, ) try: album_id = Album.objects.filter(name=post_data.get("Album", "")).first().id @@ -177,7 +190,7 @@ def jellyfin_scrobble_media(post_data: dict, user_id: int) -> Optional[Scrobble] if album_id: log["album_id"] = album_id - return media_obj.scrobble_for_user( + scrobble = media_obj.scrobble_for_user( user_id, source=post_data.get(JELLYFIN_POST_KEYS.get("SOURCE")), source_id=post_data.get(JELLYFIN_POST_KEYS.get("MEDIA_SOURCE_ID")), @@ -186,6 +199,41 @@ def jellyfin_scrobble_media(post_data: dict, user_id: int) -> Optional[Scrobble] log=log, ) + if media_type == Scrobble.MediaType.TRACK and scrobble: + _queue_webhook_enrichment(scrobble) + + return scrobble + + +def _queue_webhook_enrichment(scrobble: Scrobble) -> None: + """Tag a webhook-created scrobble and kick off async media enrichment. + + Only tracks that have not already been fully enriched (tagged + ``musicbrainz-enriched``) get tagged and dispatched; the task resolves the + track from MusicBrainz and merges onto an existing canonical track when it + finds one by musicbrainz_id. Best effort: never raises out of the webhook + path.""" + try: + from scrobbles.constants import WEBHOOK_METADATA_ONLY_TAG + from scrobbles.tasks import enrich_media_from_webhook + + track = scrobble.track + if not track: + return + if "musicbrainz-enriched" in track.tags.names(): + logger.info( + "Track already enriched, skipping async webhook enrichment", + extra={"scrobble_id": scrobble.id, "track_id": track.id}, + ) + return + scrobble.tags.add(WEBHOOK_METADATA_ONLY_TAG) + enrich_media_from_webhook.delay(scrobble.id) + except Exception as e: + logger.warning( + "Failed to queue async webhook enrichment", + extra={"scrobble_id": scrobble.id, "error": str(e)}, + ) + def web_scrobbler_scrobble_media( youtube_id: str, user_id: int, status: str = "started" diff --git a/vrobbler/apps/scrobbles/tasks.py b/vrobbler/apps/scrobbles/tasks.py index 1e6d12a..aefe09d 100644 --- a/vrobbler/apps/scrobbles/tasks.py +++ b/vrobbler/apps/scrobbles/tasks.py @@ -446,6 +446,78 @@ def push_scrobble_to_archivebox(scrobble_id): webpage.push_to_archivebox(scrobble.user) +@shared_task +def enrich_media_from_webhook(scrobble_id): + """Enrich media created purely from webhook POST data, asynchronously. + + Reads the original POST payload from the scrobble's log and enriches the + track in place (MusicBrainz resolution). If that resolution matches an + existing track by musicbrainz_id, the webhook-created track is merged into + the canonical one. The scrobble is tagged ``webhook-enriched`` on success + or ``enrichment-failed`` on failure. + """ + from music.models import Track + from scrobbles.constants import ENRICHMENT_FAILED_TAG, WEBHOOK_ENRICHED_TAG + from scrobbles.models import Scrobble + + scrobble = Scrobble.objects.filter(id=scrobble_id).first() + if not scrobble: + logger.warning( + "enrich_media_from_webhook: scrobble not found", + extra={"scrobble_id": scrobble_id}, + ) + return + + track = scrobble.track + if not track: + logger.info( + "enrich_media_from_webhook: not a track scrobble, skipping", + extra={"scrobble_id": scrobble_id, "media_type": scrobble.media_type}, + ) + return + + raw_data = scrobble.log.get("raw_data") if isinstance(scrobble.log, dict) else {} + logger.info( + "enrich_media_from_webhook: enriching track from webhook data", + extra={ + "scrobble_id": scrobble_id, + "track_id": track.id, + "title": track.title, + }, + ) + try: + enriched = track.fix_metadata() + if not enriched: + scrobble.tags.add(ENRICHMENT_FAILED_TAG) + logger.warning( + "enrich_media_from_webhook: no musicbrainz match", + extra={"scrobble_id": scrobble_id, "track_id": track.id}, + ) + return + + resolved_mbid = track.musicbrainz_id + if resolved_mbid: + canonical = ( + Track.objects.filter(musicbrainz_id=resolved_mbid) + .exclude(id=track.id) + .first() + ) + if canonical: + Track.merge(track, canonical) + + scrobble.tags.add(WEBHOOK_ENRICHED_TAG) + logger.info( + "enrich_media_from_webhook: enrichment complete", + extra={"scrobble_id": scrobble_id, "track_id": track.id}, + ) + except Exception: + logger.exception( + "enrich_media_from_webhook: enrichment failed", + extra={"scrobble_id": scrobble_id, "track_id": track.id}, + ) + scrobble.tags.add(ENRICHMENT_FAILED_TAG) + + # ── Crontab replacements ────────────────────────────────────────────────────── diff --git a/vrobbler/settings.py b/vrobbler/settings.py index 2101530..0825429 100644 --- a/vrobbler/settings.py +++ b/vrobbler/settings.py @@ -196,6 +196,7 @@ CELERY_TASK_ROUTES = { "scrobbles.tasks.rebuild_yearly_charts": {"queue": "background"}, "scrobbles.tasks.backup_database": {"queue": "background"}, "scrobbles.tasks.backfill_scrobble_sentiment": {"queue": "background"}, + "scrobbles.tasks.enrich_media_from_webhook": {"queue": "background"}, "scrobbles.tasks.check_twitch_channels_for_vods": {"queue": "background"}, "scrobbles.importers.lastfm.dispatch_historical_imports": {"queue": "background"}, "trends.tasks.compute_all_trends": {"queue": "background"},