diff --git a/beets/util/config.py b/beets/util/config.py index 218a9d1339..84346c5e2e 100644 --- a/beets/util/config.py +++ b/beets/util/config.py @@ -27,13 +27,16 @@ def sanitize_choices( def sanitize_pairs( - pairs: Sequence[tuple[str, str]], pairs_all: Sequence[tuple[str, str]] + pairs: Sequence[tuple[str, str]], + pairs_all: Sequence[tuple[str, str]], + raise_on_unknown: bool = False, ) -> list[tuple[str, str]]: """Clean up a single-element mapping configuration attribute as returned by Confuse's `Pairs` template: keep only two-element tuples present in pairs_all, remove duplicate elements, expand ('str', '*') and ('*', '*') wildcards while keeping the original order. Note that ('*', '*') and ('*', 'whatever') have the same effect. + Set raise_on_unknown to raise an error when a provided pair is not recognised For example, @@ -61,6 +64,17 @@ def sanitize_pairs( res.extend(new) elif v == "*": new = [o for o in others if o not in seen and o[0] == k] + + if len(new) == 0 and raise_on_unknown: + raise UnknownPairError(k, v) + seen.update(new) res.extend(new) + elif raise_on_unknown: + raise UnknownPairError(k, v) return res + + +class UnknownPairError(Exception): + def __init__(self, k, v): + super().__init__(f"setting {k}={v} is not recognized") diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 1819d45319..311f01edef 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -33,7 +33,7 @@ from beets.util import bytestring_path, get_temp_filename, sorted_walk, syspath from beets.util.artresizer import ArtResizer from beets.util.color import colorize -from beets.util.config import sanitize_pairs +from beets.util.config import UnknownPairError, sanitize_pairs if TYPE_CHECKING: from collections.abc import Iterable, Iterator, Sequence @@ -1407,16 +1407,7 @@ def __init__(self) -> None: self.import_stages = [self.fetch_art] self.register_listener("import_task_files", self.assign_art) - available_sources = [ - (s_cls.ID, c) - for s_cls in ART_SOURCES - if s_cls.available(self._log, self.config) - for c in s_cls.VALID_MATCHING_CRITERIA - ] - sources = sanitize_pairs( - self.config["sources"].as_pairs(default_value="*"), - available_sources, - ) + sources = self._get_sources() if "remote_priority" in self.config: self._log.warning( @@ -1441,6 +1432,33 @@ def __init__(self) -> None: for s, c in sources ] + def _get_sources(self) -> list[tuple[str, str]]: + available_sources = [ + (s_cls.ID, c) + for s_cls in ART_SOURCES + if s_cls.available(self._log, self.config) + for c in s_cls.VALID_MATCHING_CRITERIA + ] + + if isinstance(self.config["sources"].get(), str): + cfg_sources = [(self.config["sources"].get(), "*")] + else: + cfg_sources = self.config["sources"].as_pairs(default_value="*") + + try: + sources = sanitize_pairs( + cfg_sources, + available_sources, + raise_on_unknown=True, + ) + + if len(sources) == 0: + raise ui.UserError("fetchart: no sources defined in config") + + return sources + except UnknownPairError as e: + raise ui.UserError(e) + @staticmethod def _is_source_file_removal_enabled() -> bool: return config["import"]["delete"].get(bool) or config["import"][ diff --git a/docs/changelog.rst b/docs/changelog.rst index fd3c2f9a35..bc050f1f33 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -37,6 +37,8 @@ New features :bug:`5698` - :doc:`plugins/replaygain`: Conflicting replay gain tags are now removed on write. RG_* tags are removed when setting R128_* and vice versa. +- :doc:`plugins/fetchart`: Error when a configured source does not exist or + sources configuration is empty. Bug fixes ~~~~~~~~~ diff --git a/test/plugins/test_fetchart.py b/test/plugins/test_fetchart.py index f347ed66ad..36fe2ab7db 100644 --- a/test/plugins/test_fetchart.py +++ b/test/plugins/test_fetchart.py @@ -19,6 +19,7 @@ from beets import util from beets.test.helper import IOMixin, PluginTestCase +from beetsplug.fetchart import FetchArtPlugin, FileSystem class FetchartCliTest(IOMixin, PluginTestCase): @@ -103,3 +104,26 @@ def test_colorization(self): self.config["ui"]["color"] = True out = self.run_with_output("fetchart") assert " - the älbum: \x1b[1;31mno art found\x1b[39;49;00m\n" == out + + def test_sources_is_a_string(self): + self.config["fetchart"].set({"sources": "filesystem"}) + fa = FetchArtPlugin() + assert len(fa.sources) == 1 + assert isinstance(fa.sources[0], FileSystem) + + def test_sources_is_an_asterisk(self): + self.config["fetchart"].set({"sources": "*"}) + fa = FetchArtPlugin() + assert len(fa.sources) == 10 + + def test_sources_is_a_string_list(self): + self.config["fetchart"].set({"sources": ["filesystem", "coverart"]}) + fa = FetchArtPlugin() + assert len(fa.sources) == 3 + + def test_sources_is_a_mapping_list(self): + self.config["fetchart"].set( + {"sources": {"filesystem": "*", "coverart": "*"}} + ) + fa = FetchArtPlugin() + assert len(fa.sources) == 3 diff --git a/test/util/test_config.py b/test/util/test_config.py index 7105844ddf..679337bf0f 100644 --- a/test/util/test_config.py +++ b/test/util/test_config.py @@ -1,6 +1,6 @@ import pytest -from beets.util.config import sanitize_choices, sanitize_pairs +from beets.util.config import UnknownPairError, sanitize_choices, sanitize_pairs @pytest.mark.parametrize( @@ -36,3 +36,17 @@ def test_sanitize_pairs(): ("key", "value"), ("foo", "foobar"), ] + + +def test_sanitize_pairs_unknown_key(): + with pytest.raises(UnknownPairError): + sanitize_pairs( + [("bar", "foo")], [("key", "value")], raise_on_unknown=True + ) + + +def test_sanitize_pairs_unknown_key_wildcard_value(): + with pytest.raises(UnknownPairError): + sanitize_pairs( + [("foo", "*")], [("key", "value")], raise_on_unknown=True + )