From 52bdb58a46cc2fb199c617caa32450b0341fdb67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sat, 19 Jul 2025 23:41:17 +0100 Subject: [PATCH] Simplify plugin loading mechanism Centralise plugin loading in `beets.plugins` and refactor the plugin loading system to be more straightforward and eliminate complex mocking in tests. Replace the two-stage class collection and instantiation process with direct instance creation and storage. Add plugins.PluginImportError and adjust plugin import tests to only complain about plugin import issues. --- beets/plugins.py | 141 ++++++++++++++++++++++++++----------------- beets/test/helper.py | 12 ++-- beets/ui/__init__.py | 68 +++++---------------- docs/changelog.rst | 5 +- test/test_plugins.py | 81 +++++++------------------ 5 files changed, 133 insertions(+), 174 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index b78058d36..db1000b34 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -19,9 +19,10 @@ from __future__ import annotations import abc import inspect import re -import traceback +import sys from collections import defaultdict from functools import wraps +from pathlib import Path from types import GenericAlias from typing import TYPE_CHECKING, Any, ClassVar, TypeVar @@ -76,6 +77,17 @@ class PluginConflictError(Exception): """ +class PluginImportError(ImportError): + """Indicates that a plugin could not be imported. + + This is a subclass of ImportError so that it can be caught separately + from other errors. + """ + + def __init__(self, name: str): + super().__init__(f"Could not import plugin {name}") + + class PluginLogFilter(logging.Filter): """A logging filter that identifies the plugin that emitted a log message. @@ -263,69 +275,90 @@ class BeetsPlugin(metaclass=abc.ABCMeta): return helper -_classes: set[type[BeetsPlugin]] = set() +def get_plugin_names( + include: set[str] | None = None, exclude: set[str] | None = None +) -> set[str]: + """Discover and return the set of plugin names to be loaded. - -def load_plugins(names: Sequence[str] = ()) -> None: - """Imports the modules for a sequence of plugin names. Each name - must be the name of a Python module under the "beetsplug" namespace - package in sys.path; the module indicated should contain the - BeetsPlugin subclasses desired. + Configures the plugin search paths and resolves the final set of plugins + based on configuration settings, inclusion filters, and exclusion rules. + Automatically includes the musicbrainz plugin when enabled in configuration. """ - for name in names: - modname = f"{PLUGIN_NAMESPACE}.{name}" + paths = [ + str(Path(p).expanduser().absolute()) + for p in beets.config["pluginpath"].as_str_seq(split=False) + ] + log.debug("plugin paths: {}", paths) + + # Extend the `beetsplug` package to include the plugin paths. + import beetsplug + + beetsplug.__path__ = paths + list(beetsplug.__path__) + + # For backwards compatibility, also support plugin paths that + # *contain* a `beetsplug` package. + sys.path += paths + plugins = include or set(beets.config["plugins"].as_str_seq()) + # TODO: Remove in v3.0.0 + if "musicbrainz" in beets.config and beets.config["musicbrainz"].get().get( + "enabled" + ): + plugins.add("musicbrainz") + + return plugins - (exclude or set()) + + +def _get_plugin(name: str) -> BeetsPlugin | None: + """Dynamically load and instantiate a plugin class by name. + + Attempts to import the plugin module, locate the appropriate plugin class + within it, and return an instance. Handles import failures gracefully and + logs warnings for missing plugins or loading errors. + """ + try: try: - try: - namespace = __import__(modname, None, None) - except ImportError as exc: - # Again, this is hacky: - if exc.args[0].endswith(" " + name): - log.warning("** plugin {0} not found", name) - else: - raise - else: - for obj in getattr(namespace, name).__dict__.values(): - if ( - inspect.isclass(obj) - and not isinstance( - obj, GenericAlias - ) # seems to be needed for python <= 3.9 only - and issubclass(obj, BeetsPlugin) - and obj != BeetsPlugin - and not inspect.isabstract(obj) - and obj not in _classes - ): - _classes.add(obj) + namespace = __import__(f"{PLUGIN_NAMESPACE}.{name}", None, None) + except Exception as exc: + raise PluginImportError(name) from exc - except Exception: - log.warning( - "** error loading plugin {}:\n{}", - name, - traceback.format_exc(), - ) + for obj in getattr(namespace, name).__dict__.values(): + if ( + inspect.isclass(obj) + and not isinstance( + obj, GenericAlias + ) # seems to be needed for python <= 3.9 only + and issubclass(obj, BeetsPlugin) + and obj != BeetsPlugin + and not inspect.isabstract(obj) + ): + return obj() + + except Exception: + log.warning("** error loading plugin {}", name, exc_info=True) + + return None -_instances: dict[type[BeetsPlugin], BeetsPlugin] = {} +_instances: list[BeetsPlugin] = [] -def find_plugins() -> list[BeetsPlugin]: - """Returns a list of BeetsPlugin subclass instances from all - currently loaded beets plugins. Loads the default plugin set - first. +def load_plugins(*args, **kwargs) -> None: + """Initialize the plugin system by loading all configured plugins. + + Performs one-time plugin discovery and instantiation, storing loaded plugin + instances globally. Emits a pluginload event after successful initialization + to notify other components. """ - if _instances: - # After the first call, use cached instances for performance reasons. - # See https://github.com/beetbox/beets/pull/3810 - return list(_instances.values()) + if not _instances: + names = get_plugin_names(*args, **kwargs) + log.info("Loading plugins: {}", ", ".join(sorted(names))) + _instances.extend(filter(None, map(_get_plugin, names))) - load_plugins() - plugins = [] - for cls in _classes: - # Only instantiate each plugin class once. - if cls not in _instances: - _instances[cls] = cls() - plugins.append(_instances[cls]) - return plugins + send("pluginload") + + +def find_plugins() -> Iterable[BeetsPlugin]: + return _instances # Communication with plugins. diff --git a/beets/test/helper.py b/beets/test/helper.py index 767ff41fe..f1633c110 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -481,6 +481,11 @@ class PluginMixin(ConfigMixin): super().teardown_beets() self.unload_plugins() + def register_plugin( + self, plugin_class: type[beets.plugins.BeetsPlugin] + ) -> None: + beets.plugins._instances.append(plugin_class()) + def load_plugins(self, *plugins: str) -> None: """Load and initialize plugins by names. @@ -491,9 +496,7 @@ class PluginMixin(ConfigMixin): plugins = (self.plugin,) if hasattr(self, "plugin") else plugins self.config["plugins"] = plugins cached_classproperty.cache.clear() - beets.plugins.load_plugins(plugins) - beets.plugins.send("pluginload") - beets.plugins.find_plugins() + beets.plugins.load_plugins() def unload_plugins(self) -> None: """Unload all plugins and remove them from the configuration.""" @@ -501,8 +504,7 @@ class PluginMixin(ConfigMixin): beets.plugins.BeetsPlugin.listeners.clear() beets.plugins.BeetsPlugin._raw_listeners.clear() self.config["plugins"] = [] - beets.plugins._classes = set() - beets.plugins._instances = {} + beets.plugins._instances.clear() @contextmanager def configure_plugin(self, config: Any): diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index b5e2cf579..cc421782f 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -30,7 +30,7 @@ import textwrap import traceback import warnings from difflib import SequenceMatcher -from typing import TYPE_CHECKING, Any, Callable +from typing import Any, Callable import confuse @@ -40,9 +40,6 @@ from beets.dbcore import query as db_query from beets.util import as_string from beets.util.functemplate import template -if TYPE_CHECKING: - from types import ModuleType - # On Windows platforms, use colorama to support "ANSI" terminal colors. if sys.platform == "win32": try: @@ -1573,59 +1570,22 @@ optparse.Option.ALWAYS_TYPED_ACTIONS += ("callback",) # The main entry point and bootstrapping. -def _load_plugins( - options: optparse.Values, config: confuse.LazyConfig -) -> ModuleType: - """Load the plugins specified on the command line or in the configuration.""" - paths = config["pluginpath"].as_str_seq(split=False) - paths = [util.normpath(p) for p in paths] - log.debug("plugin paths: {0}", util.displayable_path(paths)) - - # On Python 3, the search paths need to be unicode. - paths = [os.fsdecode(p) for p in paths] - - # Extend the `beetsplug` package to include the plugin paths. - import beetsplug - - beetsplug.__path__ = paths + list(beetsplug.__path__) - - # For backwards compatibility, also support plugin paths that - # *contain* a `beetsplug` package. - sys.path += paths - - # If we were given any plugins on the command line, use those. - if options.plugins is not None: - plugin_list = ( - options.plugins.split(",") if len(options.plugins) > 0 else [] - ) - else: - plugin_list = config["plugins"].as_str_seq() - # TODO: Remove in v2.4 or v3 - if "musicbrainz" in config and config["musicbrainz"].get().get( - "enabled" - ): - plugin_list.append("musicbrainz") - - # Exclude any plugins that were specified on the command line - if options.exclude is not None: - plugin_list = [ - p for p in plugin_list if p not in options.exclude.split(",") - ] - - plugins.load_plugins(plugin_list) - return plugins - - -def _setup(options, lib=None): +def _setup( + options: optparse.Values, lib: library.Library | None +) -> tuple[list[Subcommand], library.Library]: """Prepare and global state and updates it with command line options. Returns a list of subcommands, a list of plugins, and a library instance. """ config = _configure(options) - plugins = _load_plugins(options, config) + def _parse_list(option: str | None) -> set[str]: + return set((option or "").split(",")) - {""} - plugins.send("pluginload") + plugins.load_plugins( + include=_parse_list(options.plugins), + exclude=_parse_list(options.exclude), + ) # Get the default subcommands. from beets.ui.commands import default_commands @@ -1637,7 +1597,7 @@ def _setup(options, lib=None): lib = _open_library(config) plugins.send("library_opened", lib=lib) - return subcommands, plugins, lib + return subcommands, lib def _configure(options): @@ -1691,7 +1651,7 @@ def _ensure_db_directory_exists(path): os.makedirs(newpath) -def _open_library(config): +def _open_library(config: confuse.LazyConfig) -> library.Library: """Create a new library instance from the configuration.""" dbpath = util.bytestring_path(config["library"].as_filename()) _ensure_db_directory_exists(dbpath) @@ -1718,7 +1678,7 @@ def _open_library(config): return lib -def _raw_main(args, lib=None): +def _raw_main(args: list[str], lib=None) -> None: """A helper function for `main` without top-level exception handling. """ @@ -1785,7 +1745,7 @@ def _raw_main(args, lib=None): return config_edit() test_lib = bool(lib) - subcommands, plugins, lib = _setup(options, lib) + subcommands, lib = _setup(options, lib) parser.add_subcommand(*subcommands) subcommand, suboptions, subargs = parser.parse_subcommand(subargs) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6a53ee5a3..5b0fd546b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -92,10 +92,11 @@ For plugin developers: Old imports are now deprecated and will be removed in version ``3.0.0``. * ``beets.ui.decargs`` is deprecated and will be removed in version ``3.0.0``. -* Beets is now pep 561 compliant, which means that it provides type hints +* Beets is now PEP 561 compliant, which means that it provides type hints for all public APIs. This allows IDEs to provide better autocompletion and type checking for downstream users of the beets API. - +* ``plugins.find_plugins`` function does not anymore load plugins. You need to + explicitly call ``plugins.load_plugins()`` to load them. Other changes: diff --git a/test/test_plugins.py b/test/test_plugins.py index 413d87bb4..da47ad6d0 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -38,53 +38,18 @@ from beets.test.helper import ( AutotagStub, ImportHelper, PluginMixin, + PluginTestCase, TerminalImportMixin, ) -from beets.test.helper import PluginTestCase as BasePluginTestCase from beets.util import displayable_path, syspath -class PluginLoaderTestCase(BasePluginTestCase): - def setup_plugin_loader(self): - # FIXME the mocking code is horrific, but this is the lowest and - # earliest level of the plugin mechanism we can hook into. - self._plugin_loader_patch = patch("beets.plugins.load_plugins") - self._plugin_classes = set() - load_plugins = self._plugin_loader_patch.start() - - def myload(names=()): - plugins._classes.update(self._plugin_classes) - - load_plugins.side_effect = myload - - def teardown_plugin_loader(self): - self._plugin_loader_patch.stop() - - def register_plugin(self, plugin_class): - self._plugin_classes.add(plugin_class) - - def setUp(self): - self.setup_plugin_loader() - super().setUp() - - def tearDown(self): - self.teardown_plugin_loader() - super().tearDown() - - -class PluginImportTestCase(ImportHelper, PluginLoaderTestCase): - def setUp(self): - super().setUp() - self.prepare_album_for_import(2) - - -class ItemTypesTest(PluginLoaderTestCase): +class ItemTypesTest(PluginTestCase): def test_flex_field_type(self): class RatingPlugin(plugins.BeetsPlugin): item_types = {"rating": types.Float()} self.register_plugin(RatingPlugin) - self.config["plugins"] = "rating" item = Item(path="apath", artist="aaa") item.add(self.lib) @@ -104,7 +69,7 @@ class ItemTypesTest(PluginLoaderTestCase): assert "aaa" not in out -class ItemWriteTest(PluginLoaderTestCase): +class ItemWriteTest(PluginTestCase): def setUp(self): super().setUp() @@ -131,33 +96,34 @@ class ItemWriteTest(PluginLoaderTestCase): self.event_listener_plugin.register_listener(event, func) -class ItemTypeConflictTest(PluginLoaderTestCase): - def test_mismatch(self): - class EventListenerPlugin(plugins.BeetsPlugin): - item_types = {"duplicate": types.INTEGER} +class ItemTypeConflictTest(PluginTestCase): + class EventListenerPlugin(plugins.BeetsPlugin): + item_types = {"duplicate": types.INTEGER} + def setUp(self): + super().setUp() + self.register_plugin(self.EventListenerPlugin) + + def test_mismatch(self): class AdventListenerPlugin(plugins.BeetsPlugin): item_types = {"duplicate": types.FLOAT} - self.event_listener_plugin = EventListenerPlugin - self.advent_listener_plugin = AdventListenerPlugin - self.register_plugin(EventListenerPlugin) self.register_plugin(AdventListenerPlugin) with pytest.raises(plugins.PluginConflictError): plugins.types(Item) def test_match(self): - class EventListenerPlugin(plugins.BeetsPlugin): - item_types = {"duplicate": types.INTEGER} - class AdventListenerPlugin(plugins.BeetsPlugin): item_types = {"duplicate": types.INTEGER} - self.event_listener_plugin = EventListenerPlugin - self.advent_listener_plugin = AdventListenerPlugin - self.register_plugin(EventListenerPlugin) self.register_plugin(AdventListenerPlugin) - assert plugins.types(Item) is not None + assert plugins.types(Item) + + +class PluginImportTestCase(ImportHelper, PluginTestCase): + def setUp(self): + super().setUp() + self.prepare_album_for_import(2) class EventsTest(PluginImportTestCase): @@ -223,7 +189,7 @@ class EventsTest(PluginImportTestCase): ] -class ListenersTest(PluginLoaderTestCase): +class ListenersTest(PluginTestCase): def test_register(self): class DummyPlugin(plugins.BeetsPlugin): def __init__(self): @@ -574,11 +540,8 @@ class TestImportAllPlugins(PluginMixin): caplog.set_level(logging.WARNING) caplog.clear() - plugins.load_plugins([plugin_name]) + plugins.load_plugins(include={plugin_name}) - # Check for warnings, is a bit hacky but we can make full use of the beets - # load_plugins code that way - assert len(caplog.records) == 0, ( - f"Plugin '{plugin_name}' has issues during import. ", - caplog.records, + assert "PluginImportError" not in caplog.text, ( + f"Plugin '{plugin_name}' has issues during import." )