diff --git a/beets/autotag/match.py b/beets/autotag/match.py index d0f3fd134..7e01c9959 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -98,7 +98,7 @@ def assign_items( return mapping, 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. @@ -110,16 +110,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( @@ -272,32 +272,32 @@ def tag_album( 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): + for _info in metadata_plugins.albums_for_ids(search_id): + _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_album): @@ -351,17 +351,15 @@ def tag_item( if trackids: for trackid in trackids: log.debug("Searching for track ID: {}", trackid) - if info := metadata_plugins.track_for_id(trackid): + for info in metadata_plugins.tracks_for_ids(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) + + # 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/beets/metadata_plugins.py b/beets/metadata_plugins.py index 1c2b6d843..cf794d120 100644 --- a/beets/metadata_plugins.py +++ b/beets/metadata_plugins.py @@ -19,7 +19,7 @@ from typing_extensions import NotRequired from beets.util import cached_classproperty from beets.util.id_extractors import extract_release_id -from .plugins import BeetsPlugin, find_plugins, notify_info_yielded, send +from .plugins import BeetsPlugin, find_plugins, notify_info_yielded if TYPE_CHECKING: from collections.abc import Iterable, Sequence @@ -48,30 +48,18 @@ def item_candidates(*args, **kwargs) -> Iterable[TrackInfo]: yield from plugin.item_candidates(*args, **kwargs) -def album_for_id(_id: str) -> AlbumInfo | None: - """Get AlbumInfo object for the given ID string. - - A single ID can yield just a single album, so we return the first match. - """ +@notify_info_yielded("albuminfo_received") +def albums_for_ids(_id: str) -> Iterable[AlbumInfo]: + """Return matching albums from all metadata sources for the given ID.""" for plugin in find_metadata_source_plugins(): - if info := plugin.album_for_id(_id): - send("albuminfo_received", info=info) - return info - - return None + yield from plugin.albums_for_ids([_id]) -def track_for_id(_id: str) -> TrackInfo | None: - """Get TrackInfo object for the given ID string. - - A single ID can yield just a single track, so we return the first match. - """ +@notify_info_yielded("trackinfo_received") +def tracks_for_ids(_id: str) -> Iterable[TrackInfo]: + """Return matching tracks from all metadata sources for the given ID.""" for plugin in find_metadata_source_plugins(): - if info := plugin.track_for_id(_id): - send("trackinfo_received", info=info) - return info - - return None + yield from plugin.tracks_for_ids([_id]) @cache @@ -169,7 +157,7 @@ class MetadataSourcePlugin(BeetsPlugin, metaclass=abc.ABCMeta): """ raise NotImplementedError - def albums_for_ids(self, ids: Sequence[str]) -> Iterable[AlbumInfo | None]: + def albums_for_ids(self, ids: Sequence[str]) -> Iterable[AlbumInfo]: """Batch lookup of album metadata for a list of album IDs. Given a list of album identifiers, yields corresponding AlbumInfo objects. @@ -178,9 +166,9 @@ class MetadataSourcePlugin(BeetsPlugin, metaclass=abc.ABCMeta): single calls to album_for_id. """ - return (self.album_for_id(id) for id in ids) + return filter(None, (self.album_for_id(id) for id in ids)) - def tracks_for_ids(self, ids: Sequence[str]) -> Iterable[TrackInfo | None]: + def tracks_for_ids(self, ids: Sequence[str]) -> Iterable[TrackInfo]: """Batch lookup of track metadata for a list of track IDs. Given a list of track identifiers, yields corresponding TrackInfo objects. @@ -189,7 +177,7 @@ class MetadataSourcePlugin(BeetsPlugin, metaclass=abc.ABCMeta): single calls to track_for_id. """ - return (self.track_for_id(id) for id in ids) + return filter(None, (self.track_for_id(id) for id in ids)) def _extract_id(self, url: str) -> str | None: """Extract an ID from a URL for this metadata source plugin. diff --git a/test/autotag/test_match.py b/test/autotag/test_match.py index a79615def..6bc974472 100644 --- a/test/autotag/test_match.py +++ b/test/autotag/test_match.py @@ -130,11 +130,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 @@ -162,7 +162,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, ) @@ -172,7 +172,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, ) @@ -182,7 +182,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, ) @@ -192,7 +192,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 c1768df3e..e3bcee7a1 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1517,7 +1517,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 @@ -1525,32 +1525,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 @@ -1558,27 +1560,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"