From 094d5bfb9d7043f44d1f2b2abaddbc07c32b7fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 8 Mar 2026 12:02:07 +0000 Subject: [PATCH] Return album candidates from multiple sources when matching by IDs --- beets/autotag/match.py | 70 +++++++++++++++++------------------- test/autotag/test_match.py | 25 +++++++------ test/test_importer.py | 74 ++++++++++++++++++++------------------ 3 files changed, 86 insertions(+), 83 deletions(-) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index 374ea3c13..4a8056a55 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -99,7 +99,7 @@ def assign_items( return list(mapping.items()), extra_items, extra_tracks -def match_by_id(items: Iterable[Item]) -> AlbumInfo | None: +def match_by_id(items: Iterable[Item]) -> Iterable[AlbumInfo]: """If the items are tagged with an external source ID, return an AlbumInfo object for the corresponding album. Otherwise, returns None. @@ -111,16 +111,16 @@ def match_by_id(items: Iterable[Item]) -> AlbumInfo | None: first = next(albumids) except StopIteration: log.debug("No album ID found.") - return None + return () # Is there a consensus on the MB album ID? for other in albumids: if other != first: log.debug("No album ID consensus.") - return None + return () # If all album IDs are equal, look up the album. log.debug("Searching for discovered album ID: {}", first) - return metadata_plugins.album_for_id(first) + return metadata_plugins.albums_for_ids([first]) def _recommendation( @@ -273,34 +273,33 @@ def tag_album( # Search by explicit ID. if search_ids: - for search_id in search_ids: - log.debug("Searching for album ID: {}", search_id) - if info := metadata_plugins.album_for_id(search_id): - _add_candidate(items, candidates, info) - if opt_candidate := candidates.get(info.album_id): - plugins.send("album_matched", match=opt_candidate) + log.debug("Searching for album IDs: {}", ", ".join(search_ids)) + for _info in metadata_plugins.albums_for_ids(search_ids): + _add_candidate(items, candidates, _info) + if opt_candidate := candidates.get(_info.album_id): + plugins.send("album_matched", match=opt_candidate) # Use existing metadata or text search. else: # Try search based on current ID. - if info := match_by_id(items): + for info in match_by_id(items): _add_candidate(items, candidates, info) for candidate in candidates.values(): plugins.send("album_matched", match=candidate) - rec = _recommendation(list(candidates.values())) - log.debug("Album ID match recommendation is {}", rec) - if candidates and not config["import"]["timid"]: - # If we have a very good MBID match, return immediately. - # Otherwise, this match will compete against metadata-based - # matches. - if rec == Recommendation.strong: - log.debug("ID match.") - return ( - cur_artist, - cur_album, - Proposal(list(candidates.values()), rec), - ) + rec = _recommendation(list(candidates.values())) + log.debug("Album ID match recommendation is {}", rec) + if candidates and not config["import"]["timid"]: + # If we have a very good MBID match, return immediately. + # Otherwise, this match will compete against metadata-based + # matches. + if rec == Recommendation.strong: + log.debug("ID match.") + return ( + cur_artist, + cur_album, + Proposal(list(candidates.values()), rec), + ) # Search terms. if not (search_artist and search_name): @@ -352,19 +351,16 @@ def tag_item( # First, try matching by the external source ID. trackids = search_ids or [t for t in [item.mb_trackid] if t] if trackids: - for trackid in trackids: - log.debug("Searching for track ID: {}", trackid) - if info := metadata_plugins.track_for_id(trackid): - dist = track_distance(item, info, incl_artist=True) - candidates[info.track_id] = hooks.TrackMatch(dist, info) - # If this is a good match, then don't keep searching. - rec = _recommendation(_sort_candidates(candidates.values())) - if ( - rec == Recommendation.strong - and not config["import"]["timid"] - ): - log.debug("Track ID match.") - return Proposal(_sort_candidates(candidates.values()), rec) + log.debug("Searching for track IDs: {}", ", ".join(trackids)) + for info in metadata_plugins.tracks_for_ids(trackids): + dist = track_distance(item, info, incl_artist=True) + candidates[info.track_id] = hooks.TrackMatch(dist, info) + + # If this is a good match, then don't keep searching. + rec = _recommendation(_sort_candidates(candidates.values())) + if rec == Recommendation.strong and not config["import"]["timid"]: + log.debug("Track ID match.") + return Proposal(_sort_candidates(candidates.values()), rec) # If we're searching by ID, don't proceed. if search_ids: diff --git a/test/autotag/test_match.py b/test/autotag/test_match.py index 212944045..e31726e87 100644 --- a/test/autotag/test_match.py +++ b/test/autotag/test_match.py @@ -108,7 +108,10 @@ class TestTagMultipleDataSources: @pytest.fixture(autouse=True) def _setup_plugins(self, monkeypatch, shared_album_id, shared_track_id): - class StubPlugin(metadata_plugins.MetadataSourcePlugin): + class StubPlugin: + data_source: ClassVar[str] + data_source_mismatch_penalty = 0 + @property def track(self): return TrackInfo( @@ -128,11 +131,11 @@ class TestTagMultipleDataSources: data_source=self.data_source, ) - def album_for_id(self, *_): - return self.album + def albums_for_ids(self, *_): + yield self.album - def track_for_id(self, *_): - return self.track + def tracks_for_ids(self, *_): + yield self.track def candidates(self, *_, **__): yield self.album @@ -141,10 +144,10 @@ class TestTagMultipleDataSources: yield self.track class DeezerPlugin(StubPlugin): - pass + data_source = "Deezer" class DiscogsPlugin(StubPlugin): - pass + data_source = "Discogs" monkeypatch.setattr( metadata_plugins, @@ -160,7 +163,7 @@ class TestTagMultipleDataSources: assert set(sources) == {"Discogs", "Deezer"} @pytest.mark.xfail( - reason="Album ID collisions drop extra sources (#6177)", + reason="Same ID from different sources is considered a duplicate (#6181)", raises=AssertionError, strict=True, ) @@ -170,7 +173,7 @@ class TestTagMultipleDataSources: self.check_proposal(proposal) @pytest.mark.xfail( - reason="Album ID collisions drop extra sources (#6177)", + reason="Same ID from different sources is considered a duplicate (#6181)", raises=AssertionError, strict=True, ) @@ -180,7 +183,7 @@ class TestTagMultipleDataSources: self.check_proposal(proposal) @pytest.mark.xfail( - reason="Track ID collisions drop extra sources (#6177)", + reason="The last match wins", raises=AssertionError, strict=True, ) @@ -190,7 +193,7 @@ class TestTagMultipleDataSources: self.check_proposal(proposal) @pytest.mark.xfail( - reason="Track ID collisions drop extra sources (#6177)", + reason="The last match wins", raises=AssertionError, strict=True, ) diff --git a/test/test_importer.py b/test/test_importer.py index fe37072fe..1a8983c11 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1557,7 +1557,7 @@ class ImportPretendTest(IOMixin, AutotagImportTestCase): assert self.__run(importer) == [f"No files imported from {empty_path}"] -def mocked_get_album_by_id(id_): +def mocked_get_albums_by_ids(ids): """Return album candidate for the given id. The two albums differ only in the release title and artist name, so that @@ -1565,32 +1565,34 @@ def mocked_get_album_by_id(id_): ImportHelper.prepare_album_for_import(). """ # Map IDs to (release title, artist), so the distances are different. - album, artist = { + album_artist_map = { ImportIdTest.ID_RELEASE_0: ("VALID_RELEASE_0", "TAG ARTIST"), ImportIdTest.ID_RELEASE_1: ("VALID_RELEASE_1", "DISTANT_MATCH"), - }[id_] + } - return AlbumInfo( - album_id=id_, - album=album, - artist_id="some-id", - artist=artist, - albumstatus="Official", - tracks=[ - TrackInfo( - track_id="bar", - title="foo", - artist_id="some-id", - artist=artist, - length=59, - index=9, - track_allt="A2", - ) - ], - ) + for id_ in ids: + album, artist = album_artist_map[id_] + yield AlbumInfo( + album_id=id_, + album=album, + artist_id="some-id", + artist=artist, + albumstatus="Official", + tracks=[ + TrackInfo( + track_id="bar", + title="foo", + artist_id="some-id", + artist=artist, + length=59, + index=9, + track_allt="A2", + ) + ], + ) -def mocked_get_track_by_id(id_): +def mocked_get_tracks_by_ids(ids): """Return track candidate for the given id. The two tracks differ only in the release title and artist name, so that @@ -1598,27 +1600,29 @@ def mocked_get_track_by_id(id_): ImportHelper.prepare_album_for_import(). """ # Map IDs to (recording title, artist), so the distances are different. - title, artist = { + title_artist_map = { ImportIdTest.ID_RECORDING_0: ("VALID_RECORDING_0", "TAG ARTIST"), ImportIdTest.ID_RECORDING_1: ("VALID_RECORDING_1", "DISTANT_MATCH"), - }[id_] + } - return TrackInfo( - track_id=id_, - title=title, - artist_id="some-id", - artist=artist, - length=59, - ) + for id_ in ids: + title, artist = title_artist_map[id_] + yield TrackInfo( + track_id=id_, + title=title, + artist_id="some-id", + artist=artist, + length=59, + ) @patch( - "beets.metadata_plugins.track_for_id", - Mock(side_effect=mocked_get_track_by_id), + "beets.metadata_plugins.tracks_for_ids", + Mock(side_effect=mocked_get_tracks_by_ids), ) @patch( - "beets.metadata_plugins.album_for_id", - Mock(side_effect=mocked_get_album_by_id), + "beets.metadata_plugins.albums_for_ids", + Mock(side_effect=mocked_get_albums_by_ids), ) class ImportIdTest(ImportTestCase): ID_RELEASE_0 = "00000000-0000-0000-0000-000000000000"