diff --git a/beets/util/__init__.py b/beets/util/__init__.py index a1ce55caa..6bc4d14ee 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -803,7 +803,7 @@ def as_string(value: Any) -> str: return str(value) -def plurality(objs: Sequence[T]) -> tuple[T, int]: +def plurality(objs: Iterable[T]) -> tuple[T, int]: """Given a sequence of hashble objects, returns the object that is most common in the set and the its number of appearance. The sequence must contain at least one object. diff --git a/beetsplug/musicbrainz.py b/beetsplug/musicbrainz.py index 34a46715d..bdfeb0968 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -18,6 +18,7 @@ from __future__ import annotations import traceback from collections import Counter +from functools import cached_property from itertools import product from typing import TYPE_CHECKING, Any from urllib.parse import urljoin @@ -383,7 +384,7 @@ class MusicBrainzPlugin(BeetsPlugin): "deezer": False, "tidal": False, }, - "extra_tags": {}, + "extra_tags": [], }, ) hostname = self.config["host"].as_str() @@ -747,6 +748,46 @@ class MusicBrainzPlugin(BeetsPlugin): return info + @cached_property + def extra_mb_field_by_tag(self) -> dict[str, str]: + """Map configured extra tags to their MusicBrainz API field names. + + Process user configuration to determine which additional MusicBrainz + fields should be included in search queries. + """ + mb_field_by_tag = { + t: FIELDS_TO_MB_KEYS[t] + for t in self.config["extra_tags"].as_str_seq() + if t in FIELDS_TO_MB_KEYS + } + if mb_field_by_tag: + self._log.debug("Additional search terms: {}", mb_field_by_tag) + + return mb_field_by_tag + + def get_album_criteria( + self, items: list[Item], artist: str, album: str, va_likely: bool + ) -> dict[str, str]: + # Build search criteria. + criteria = {"release": album.lower().strip()} + if artist is not None: + criteria["artist"] = artist.lower().strip() + else: + # Various Artists search. + criteria["arid"] = VARIOUS_ARTISTS_ID + if track_count := len(items): + criteria["tracks"] = str(track_count) + + for tag, mb_field in self.extra_mb_field_by_tag.items(): + most_common, _ = util.plurality(i.get(tag) for i in items) + value = str(most_common).lower().strip() + if tag == "catalognum": + value = value.replace(" ", "") + if value: + criteria[mb_field] = value + + return criteria + def candidates( self, items: list[Item], @@ -762,27 +803,7 @@ class MusicBrainzPlugin(BeetsPlugin): The query consists of an artist name, an album name, and, optionally, a number of tracks on the album and any other extra tags. """ - # Build search criteria. - criteria = {"release": album.lower().strip()} - if artist is not None: - criteria["artist"] = artist.lower().strip() - else: - # Various Artists search. - criteria["arid"] = VARIOUS_ARTISTS_ID - if track_count := len(items): - criteria["tracks"] = str(track_count) - - if self.config["extra_tags"]: - tag_list = self.config["extra_tags"].get() - self._log.debug("Additional search terms: {0}", tag_list) - for tag, value in tag_list.items(): - if key := FIELDS_TO_MB_KEYS.get(tag): - value = str(value).lower().strip() - if key == "catno": - value = value.replace(" ", "") - if value: - criteria[key] = value - + criteria = self.get_album_criteria(items, artist, album, va_likely) # Abort if we have no search terms. if not any(criteria.values()): return diff --git a/docs/changelog.rst b/docs/changelog.rst index 31da975e2..a01ee8c97 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,11 +8,22 @@ Unreleased New features: +* :doc:`plugins/musicbrainz`: The MusicBrainz autotagger has been moved to + a separate plugin. The default :ref:`plugins-config` includes `musicbrainz`, + but if you've customized your `plugins` list in your configuration, you'll + need to explicitly add `musicbrainz` to continue using this functionality. + Configuration option `musicbrainz.enabled` has thus been deprecated. + :bug:`2686` + :bug:`4605` * :doc:`plugins/web`: Show notifications when a track plays. This uses the Media Session API to customize media notifications. Bug fixes: +* :doc:`plugins/musicbrainz`: fix regression where user configured + ``extra_tags`` have been read incorrectly. + :bug:`5788` + For packagers: Other changes: @@ -39,13 +50,6 @@ been dropped. New features: -* :doc:`plugins/musicbrainz`: The MusicBrainz autotagger has been moved to - a separate plugin. The default :ref:`plugins-config` includes `musicbrainz`, - but if you've customized your `plugins` list in your configuration, you'll - need to explicitly add `musicbrainz` to continue using this functionality. - Configuration option `musicbrainz.enabled` has thus been deprecated. - :bug:`2686` - :bug:`4605` * :doc:`plugins/lastgenre`: The new configuration option, ``keep_existing``, provides more fine-grained control over how pre-populated genre tags are handled. The ``force`` option now behaves in a more conventional manner. diff --git a/test/plugins/test_musicbrainz.py b/test/plugins/test_musicbrainz.py index 0f142a353..d4104a3ba 100644 --- a/test/plugins/test_musicbrainz.py +++ b/test/plugins/test_musicbrainz.py @@ -16,8 +16,11 @@ from unittest import mock +import pytest + from beets import config -from beets.test.helper import BeetsTestCase +from beets.library import Item +from beets.test.helper import BeetsTestCase, PluginMixin from beetsplug import musicbrainz @@ -1063,3 +1066,42 @@ class MBLibraryTest(MusicBrainzTestCase): gp.side_effect = side_effect album = self.mb.album_for_id("d2a6f856-b553-40a0-ac54-a321e8e2da02") assert album.country is None + + +class TestMusicBrainzPlugin(PluginMixin): + plugin = "musicbrainz" + + @pytest.fixture + def mb_plugin(self, plugin_config): + self.config[self.plugin].set(plugin_config) + + return musicbrainz.MusicBrainzPlugin() + + @pytest.mark.parametrize( + "plugin_config,va_likely,expected_additional_criteria", + [ + ({}, False, {"artist": "artist"}), + ({}, True, {"artist": "artist"}), + ( + {"extra_tags": ["label", "catalognum"]}, + False, + {"artist": "artist", "label": "abc", "catno": "abc123"}, + ), + ], + ) + def test_get_album_criteria( + self, mb_plugin, va_likely, expected_additional_criteria + ): + items = [ + Item(catalognum="ABC 123", label="abc"), + Item(catalognum="ABC 123", label="abc"), + Item(catalognum="ABC 123", label="def"), + ] + + assert mb_plugin.get_album_criteria( + items, "Artist ", " Album", va_likely + ) == { + "release": "album", + "tracks": str(len(items)), + **expected_additional_criteria, + }