Return album candidates from multiple sources when matching by IDs

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:

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,
     )

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"