diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 767509c9a..bb888d520 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,2 +1,5 @@ # assign the entire repo to the maintainers team * @beetbox/maintainers + +# Specific ownerships: +/beets/metadata_plugins.py @semohr \ No newline at end of file diff --git a/beets/logging.py b/beets/logging.py index fd8b1962f..3ed5e5a84 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -20,6 +20,8 @@ use {}-style formatting and can interpolate keywords arguments to the logging calls (`debug`, `info`, etc). """ +from __future__ import annotations + import threading from copy import copy from logging import ( @@ -32,8 +34,10 @@ from logging import ( Handler, Logger, NullHandler, + RootLogger, StreamHandler, ) +from typing import TYPE_CHECKING, Any, Mapping, TypeVar, Union, overload __all__ = [ "DEBUG", @@ -49,8 +53,20 @@ __all__ = [ "getLogger", ] +if TYPE_CHECKING: + T = TypeVar("T") + from types import TracebackType -def logsafe(val): + # see https://github.com/python/typeshed/blob/main/stdlib/logging/__init__.pyi + _SysExcInfoType = Union[ + tuple[type[BaseException], BaseException, Union[TracebackType, None]], + tuple[None, None, None], + ] + _ExcInfoType = Union[None, bool, _SysExcInfoType, BaseException] + _ArgsType = Union[tuple[object, ...], Mapping[str, object]] + + +def _logsafe(val: T) -> str | T: """Coerce `bytes` to `str` to avoid crashes solely due to logging. This is particularly relevant for bytestring paths. Much of our code @@ -83,40 +99,45 @@ class StrFormatLogger(Logger): """ class _LogMessage: - def __init__(self, msg, args, kwargs): + def __init__( + self, + msg: str, + args: _ArgsType, + kwargs: dict[str, Any], + ): self.msg = msg self.args = args self.kwargs = kwargs def __str__(self): - args = [logsafe(a) for a in self.args] - kwargs = {k: logsafe(v) for (k, v) in self.kwargs.items()} + args = [_logsafe(a) for a in self.args] + kwargs = {k: _logsafe(v) for (k, v) in self.kwargs.items()} return self.msg.format(*args, **kwargs) def _log( self, - level, - msg, - args, - exc_info=None, - extra=None, - stack_info=False, + level: int, + msg: object, + args: _ArgsType, + exc_info: _ExcInfoType = None, + extra: Mapping[str, Any] | None = None, + stack_info: bool = False, + stacklevel: int = 1, **kwargs, ): """Log msg.format(*args, **kwargs)""" - m = self._LogMessage(msg, args, kwargs) - stacklevel = kwargs.pop("stacklevel", 1) - stacklevel = {"stacklevel": stacklevel} + if isinstance(msg, str): + msg = self._LogMessage(msg, args, kwargs) return super()._log( level, - m, + msg, (), exc_info=exc_info, extra=extra, stack_info=stack_info, - **stacklevel, + stacklevel=stacklevel, ) @@ -156,9 +177,12 @@ my_manager = copy(Logger.manager) my_manager.loggerClass = BeetsLogger -# Override the `getLogger` to use our machinery. -def getLogger(name=None): # noqa +@overload +def getLogger(name: str) -> BeetsLogger: ... +@overload +def getLogger(name: None = ...) -> RootLogger: ... +def getLogger(name=None) -> BeetsLogger | RootLogger: # noqa: N802 if name: - return my_manager.getLogger(name) + return my_manager.getLogger(name) # type: ignore[return-value] else: return Logger.root diff --git a/beets/plugins.py b/beets/plugins.py index d8d465183..c0dd12e5b 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -22,6 +22,7 @@ import re import sys from collections import defaultdict from functools import wraps +from importlib import import_module from pathlib import Path from types import GenericAlias from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar @@ -365,11 +366,11 @@ def _get_plugin(name: str) -> BeetsPlugin | None: """ try: try: - namespace = __import__(f"{PLUGIN_NAMESPACE}.{name}", None, None) + namespace = import_module(f"{PLUGIN_NAMESPACE}.{name}") except Exception as exc: raise PluginImportError(name) from exc - for obj in getattr(namespace, name).__dict__.values(): + for obj in namespace.__dict__.values(): if ( inspect.isclass(obj) and not isinstance( @@ -378,6 +379,12 @@ def _get_plugin(name: str) -> BeetsPlugin | None: and issubclass(obj, BeetsPlugin) and obj != BeetsPlugin and not inspect.isabstract(obj) + # Only consider this plugin's module or submodules to avoid + # conflicts when plugins import other BeetsPlugin classes + and ( + obj.__module__ == namespace.__name__ + or obj.__module__.startswith(f"{namespace.__name__}.") + ) ): return obj() diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 54c065da4..37e7426f6 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -36,10 +36,10 @@ from beets.util.config import sanitize_pairs if TYPE_CHECKING: from collections.abc import Iterable, Iterator, Sequence - from logging import Logger from beets.importer import ImportSession, ImportTask from beets.library import Album, Library + from beets.logging import BeetsLogger as Logger try: from bs4 import BeautifulSoup, Tag diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index f492ab3cc..d245d6a14 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -42,10 +42,9 @@ from beets.autotag.distance import string_dist from beets.util.config import sanitize_choices if TYPE_CHECKING: - from logging import Logger - from beets.importer import ImportTask from beets.library import Item, Library + from beets.logging import BeetsLogger as Logger from ._typing import ( GeniusAPI, @@ -186,7 +185,7 @@ def slug(text: str) -> str: class RequestHandler: - _log: beets.logging.Logger + _log: Logger def debug(self, message: str, *args) -> None: """Log a debug message with the class name.""" diff --git a/docs/changelog.rst b/docs/changelog.rst index 9476c7ad8..464ce36fd 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -37,6 +37,8 @@ Bug fixes: matches due to query escaping (single vs double quotes). - :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from artists but not labels. :bug:`5366` +- :doc:`plugins/chroma` :doc:`plugins/bpsync` Fix plugin loading issue caused by + an import of another :class:`beets.plugins.BeetsPlugin` class. :bug:`6033` For packagers: @@ -54,6 +56,12 @@ Other changes: - :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific disambiguation stripping. +For developers and plugin authors: + +- Typing improvements in ``beets/logging.py``: ``getLogger`` now returns + ``BeetsLogger`` when called with a name, or ``RootLogger`` when called without + a name. + 2.4.0 (September 13, 2025) -------------------------- diff --git a/test/test_logging.py b/test/test_logging.py index aee0bd61b..48f9cbfd8 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -3,18 +3,21 @@ import logging as log import sys import threading -import unittest -from io import StringIO +from types import ModuleType +from unittest.mock import patch + +import pytest import beets.logging as blog -import beetsplug from beets import plugins, ui from beets.test import _common, helper from beets.test.helper import AsIsImporterMixin, ImportTestCase, PluginMixin -class LoggingTest(unittest.TestCase): - def test_logging_management(self): +class TestStrFormatLogger: + """Tests for the custom str-formatting logger.""" + + def test_logger_creation(self): l1 = log.getLogger("foo123") l2 = blog.getLogger("foo123") assert l1 == l2 @@ -34,49 +37,76 @@ class LoggingTest(unittest.TestCase): l6 = blog.getLogger() assert l1 != l6 - def test_str_format_logging(self): - logger = blog.getLogger("baz123") - stream = StringIO() - handler = log.StreamHandler(stream) + @pytest.mark.parametrize( + "level", [log.DEBUG, log.INFO, log.WARNING, log.ERROR] + ) + @pytest.mark.parametrize( + "msg, args, kwargs, expected", + [ + ("foo {} bar {}", ("oof", "baz"), {}, "foo oof bar baz"), + ( + "foo {bar} baz {foo}", + (), + {"foo": "oof", "bar": "baz"}, + "foo baz baz oof", + ), + ("no args", (), {}, "no args"), + ("foo {} bar {baz}", ("oof",), {"baz": "baz"}, "foo oof bar baz"), + ], + ) + def test_str_format_logging( + self, level, msg, args, kwargs, expected, caplog + ): + logger = blog.getLogger("test_logger") + logger.setLevel(level) - logger.addHandler(handler) - logger.propagate = False + with caplog.at_level(level, logger="test_logger"): + logger.log(level, msg, *args, **kwargs) - logger.warning("foo {} {bar}", "oof", bar="baz") - handler.flush() - assert stream.getvalue(), "foo oof baz" + assert caplog.records, "No log records were captured" + assert str(caplog.records[0].msg) == expected + + +class DummyModule(ModuleType): + class DummyPlugin(plugins.BeetsPlugin): + def __init__(self): + plugins.BeetsPlugin.__init__(self, "dummy") + self.import_stages = [self.import_stage] + self.register_listener("dummy_event", self.listener) + + def log_all(self, name): + self._log.debug("debug {}", name) + self._log.info("info {}", name) + self._log.warning("warning {}", name) + + def commands(self): + cmd = ui.Subcommand("dummy") + cmd.func = lambda _, __, ___: self.log_all("cmd") + return (cmd,) + + def import_stage(self, session, task): + self.log_all("import_stage") + + def listener(self): + self.log_all("listener") + + def __init__(self, *_, **__): + module_name = "beetsplug.dummy" + super().__init__(module_name) + self.DummyPlugin.__module__ = module_name + self.DummyPlugin = self.DummyPlugin class LoggingLevelTest(AsIsImporterMixin, PluginMixin, ImportTestCase): plugin = "dummy" - class DummyModule: - class DummyPlugin(plugins.BeetsPlugin): - def __init__(self): - plugins.BeetsPlugin.__init__(self, "dummy") - self.import_stages = [self.import_stage] - self.register_listener("dummy_event", self.listener) + @classmethod + def setUpClass(cls): + patcher = patch.dict(sys.modules, {"beetsplug.dummy": DummyModule()}) + patcher.start() + cls.addClassCleanup(patcher.stop) - def log_all(self, name): - self._log.debug("debug {}", name) - self._log.info("info {}", name) - self._log.warning("warning {}", name) - - def commands(self): - cmd = ui.Subcommand("dummy") - cmd.func = lambda _, __, ___: self.log_all("cmd") - return (cmd,) - - def import_stage(self, session, task): - self.log_all("import_stage") - - def listener(self): - self.log_all("listener") - - def setUp(self): - sys.modules["beetsplug.dummy"] = self.DummyModule - beetsplug.dummy = self.DummyModule - super().setUp() + super().setUpClass() def test_command_level0(self): self.config["verbose"] = 0