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.

Author: snejus

beets/plugins.py

@@ -19,9 +19,10 @@
 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 @@ def helper(func: TFunc[Item]) -> TFunc[Item]:
         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.
+
+    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.
+    """
+    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 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.
+
+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.
     """
-    for name in names:
-        modname = f"{PLUGIN_NAMESPACE}.{name}"
+    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)
-
-        except Exception:
-            log.warning(
-                "** error loading plugin {}:\n{}",
-                name,
-                traceback.format_exc(),
-            )
+            namespace = __import__(f"{PLUGIN_NAMESPACE}.{name}", None, None)
+        except Exception as exc:
+            raise PluginImportError(name) from 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())
-
-    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
+    if not _instances:
+        names = get_plugin_names(*args, **kwargs)
+        log.info("Loading plugins: {}", ", ".join(sorted(names)))
+        _instances.extend(filter(None, map(_get_plugin, names)))
+
+        send("pluginload")
+
+
+def find_plugins() -> Iterable[BeetsPlugin]:
+    return _instances
 
 
 # Communication with plugins.

beets/test/helper.py

@@ -481,6 +481,11 @@ def teardown_beets(self):
         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,18 +496,15 @@ def load_plugins(self, *plugins: str) -> None:
         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."""
         # FIXME this should eventually be handled by a plugin manager
         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):

beets/ui/__init__.py

@@ -30,7 +30,7 @@
 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.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 @@ def parse_subcommand(self, args):
 # 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)

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: