Simplify plugin loading (#5887)

This PR centralises plugin loading logic inside `beets.plugins` module.


- Removed intermediatery `_classes` variable by initialising plugins
immediately.
- Simplified listeners registration by defining listener variables in
the base
`BeetsPlugin` class, and making the `register_listener` method a
`@classmethod`.
- Simplified plugin test setup accordingly.

Author: snejus

.github/workflows/ci.yaml

@@ -64,6 +64,12 @@ jobs:
           poe docs
           poe test-with-coverage
 
+      - if: ${{ !cancelled() }}
+        name: Upload test results to Codecov
+        uses: codecov/test-results-action@v1
+        with:
+          token: ${{ secrets.CODECOV_TOKEN }}
+
       - if: ${{ env.IS_MAIN_PYTHON == 'true' }}
         name: Store the coverage report
         uses: actions/upload-artifact@v4
@@ -86,7 +92,7 @@ jobs:
           name: coverage-report
 
       - name: Upload code coverage
-        uses: codecov/codecov-action@v4
+        uses: codecov/codecov-action@v5
         with:
           files: ./coverage.xml
           use_oidc: ${{ !(github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork) }}

beets/event_types.py

@@ -19,24 +19,22 @@
 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, Callable, Sequence, TypeVar
+from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar
 
 import mediafile
 from typing_extensions import ParamSpec
 
 import beets
 from beets import logging
+from beets.util import unique_list
 
 if TYPE_CHECKING:
-    from beets.event_types import EventType
-
-
-if TYPE_CHECKING:
-    from collections.abc import Iterable
+    from collections.abc import Callable, Iterable, Sequence
 
     from confuse import ConfigView
 
@@ -58,7 +56,7 @@
 
     P = ParamSpec("P")
     Ret = TypeVar("Ret", bound=Any)
-    Listener = Callable[..., None]
+    Listener = Callable[..., Any]
     IterF = Callable[P, Iterable[Ret]]
 
 
@@ -67,6 +65,37 @@
 # Plugins using the Last.fm API can share the same API key.
 LASTFM_KEY = "2dc3914abf35f0d9c92d97d8f8e42b43"
 
+EventType = Literal[
+    "after_write",
+    "album_imported",
+    "album_removed",
+    "albuminfo_received",
+    "before_choose_candidate",
+    "before_item_moved",
+    "cli_exit",
+    "database_change",
+    "import",
+    "import_begin",
+    "import_task_apply",
+    "import_task_before_choice",
+    "import_task_choice",
+    "import_task_created",
+    "import_task_files",
+    "import_task_start",
+    "item_copied",
+    "item_hardlinked",
+    "item_imported",
+    "item_linked",
+    "item_moved",
+    "item_reflinked",
+    "item_removed",
+    "library_opened",
+    "mb_album_extract",
+    "mb_track_extract",
+    "pluginload",
+    "trackinfo_received",
+    "write",
+]
 # Global logger.
 log = logging.getLogger("beets")
 
@@ -79,6 +108,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.
@@ -105,6 +145,14 @@ class BeetsPlugin(metaclass=abc.ABCMeta):
     the abstract methods defined here.
     """
 
+    _raw_listeners: ClassVar[dict[EventType, list[Listener]]] = defaultdict(
+        list
+    )
+    listeners: ClassVar[dict[EventType, list[Listener]]] = defaultdict(list)
+    template_funcs: TFuncMap[str] | None = None
+    template_fields: TFuncMap[Item] | None = None
+    album_template_fields: TFuncMap[Album] | None = None
+
     name: str
     config: ConfigView
     early_import_stages: list[ImportStageFunc]
@@ -218,25 +266,13 @@ def add_media_field(
         mediafile.MediaFile.add_field(name, descriptor)
         library.Item._media_fields.add(name)
 
-    _raw_listeners: dict[str, list[Listener]] | None = None
-    listeners: dict[str, list[Listener]] | None = None
-
-    def register_listener(self, event: "EventType", func: Listener):
+    def register_listener(self, event: EventType, func: Listener) -> None:
         """Add a function as a listener for the specified event."""
-        wrapped_func = self._set_log_level_and_params(logging.WARNING, func)
-
-        cls = self.__class__
-
-        if cls.listeners is None or cls._raw_listeners is None:
-            cls._raw_listeners = defaultdict(list)
-            cls.listeners = defaultdict(list)
-        if func not in cls._raw_listeners[event]:
-            cls._raw_listeners[event].append(func)
-            cls.listeners[event].append(wrapped_func)
-
-    template_funcs: TFuncMap[str] | None = None
-    template_fields: TFuncMap[Item] | None = None
-    album_template_fields: TFuncMap[Album] | None = None
+        if func not in self._raw_listeners[event]:
+            self._raw_listeners[event].append(func)
+            self.listeners[event].append(
+                self._set_log_level_and_params(logging.WARNING, func)
+            )
 
     @classmethod
     def template_func(cls, name: str) -> Callable[[TFunc[str]], TFunc[str]]:
@@ -270,69 +306,92 @@ def helper(func: TFunc[Item]) -> TFunc[Item]:
         return helper
 
 
-_classes: set[type[BeetsPlugin]] = set()
+def get_plugin_names() -> list[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.
+    """
+    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 = unique_list(beets.config["plugins"].as_str_seq())
+    # TODO: Remove in v3.0.0
+    if (
+        "musicbrainz" not in plugins
+        and "musicbrainz" in beets.config
+        and beets.config["musicbrainz"].get().get("enabled")
+    ):
+        plugins.append("musicbrainz")
+
+    beets.config.add({"disabled_plugins": []})
+    disabled_plugins = set(beets.config["disabled_plugins"].as_str_seq())
+    return [p for p in plugins if p not in disabled_plugins]
+
+
+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() -> 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()
+        log.info("Loading plugins: {}", ", ".join(sorted(names)))
+        _instances.extend(filter(None, map(_get_plugin, names)))
+
+        send("pluginload")
+
 
-    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
+def find_plugins() -> Iterable[BeetsPlugin]:
+    return _instances
 
 
 # Communication with plugins.
@@ -383,7 +442,9 @@ def named_queries(model_cls: type[AnyModel]) -> dict[str, FieldQueryType]:
     }
 
 
-def notify_info_yielded(event: str) -> Callable[[IterF[P, Ret]], IterF[P, Ret]]:
+def notify_info_yielded(
+    event: EventType,
+) -> Callable[[IterF[P, Ret]], IterF[P, Ret]]:
     """Makes a generator send the event 'event' every time it yields.
     This decorator is supposed to decorate a generator, but any function
     returning an iterable should work.
@@ -474,19 +535,7 @@ def album_field_getters() -> TFuncMap[Album]:
 # Event dispatch.
 
 
-def event_handlers() -> dict[str, list[Listener]]:
-    """Find all event handlers from plugins as a dictionary mapping
-    event names to sequences of callables.
-    """
-    all_handlers: dict[str, list[Listener]] = defaultdict(list)
-    for plugin in find_plugins():
-        if plugin.listeners:
-            for event, handlers in plugin.listeners.items():
-                all_handlers[event] += handlers
-    return all_handlers
-
-
-def send(event: str, **arguments: Any) -> list[Any]:
+def send(event: EventType, **arguments: Any) -> list[Any]:
     """Send an event to all assigned event listeners.
 
     `event` is the name of  the event to send, all other named arguments
@@ -495,12 +544,11 @@ def send(event: str, **arguments: Any) -> list[Any]:
     Return a list of non-None values returned from the handlers.
     """
     log.debug("Sending event: {0}", event)
-    results: list[Any] = []
-    for handler in event_handlers()[event]:
-        result = handler(**arguments)
-        if result is not None:
-            results.append(result)
-    return results
+    return [
+        r
+        for handler in BeetsPlugin.listeners[event]
+        if (r := handler(**arguments)) is not None
+    ]
 
 
 def feat_tokens(for_artist: bool = True) -> str: