From 670c300625b0dd7ef7a5c62c3ae2106ed5dfbf15 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Tue, 14 Oct 2025 17:15:02 +0200 Subject: [PATCH 1/4] Fixed issue with legacy plugin copy not copying properties. Also added test for it --- beets/plugins.py | 25 +++++++++++++++---------- docs/changelog.rst | 2 ++ test/test_plugins.py | 20 ++++++++++++++++++++ 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index a8e803efd..9c7a93b7f 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -22,7 +22,7 @@ import re import sys import warnings from collections import defaultdict -from functools import wraps +from functools import cached_property, wraps from importlib import import_module from pathlib import Path from types import GenericAlias @@ -192,15 +192,20 @@ class BeetsPlugin(metaclass=abc.ABCMeta): stacklevel=3, ) - for name, method in inspect.getmembers( - MetadataSourcePlugin, - predicate=lambda f: ( - inspect.isfunction(f) - and f.__name__ not in MetadataSourcePlugin.__abstractmethods__ - and not hasattr(cls, f.__name__) - ), - ): - setattr(cls, name, method) + abstracts = MetadataSourcePlugin.__abstractmethods__ + + for name, method in inspect.getmembers(MetadataSourcePlugin): + # Skip if already defined in the subclass + if hasattr(cls, name) or name in abstracts: + continue + + # Copy functions, methods, and properties + if ( + inspect.isfunction(method) + or inspect.ismethod(method) + or isinstance(method, cached_property) + ): + setattr(cls, name, method) def __init__(self, name: str | None = None): """Perform one-time plugin setup.""" diff --git a/docs/changelog.rst b/docs/changelog.rst index 6d08d6bdb..88c157ec5 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -22,6 +22,8 @@ For packagers: - Fixed dynamic versioning install not disabled for source distribution builds. :bug:`6089` +- Fixed issue with legacy metadata plugins not copying properties from the base + class. Other changes: diff --git a/test/test_plugins.py b/test/test_plugins.py index df338f924..07bbf0966 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -523,3 +523,23 @@ class TestImportPlugin(PluginMixin): assert "PluginImportError" not in caplog.text, ( f"Plugin '{plugin_name}' has issues during import." ) + + +class TestDeprecationCopy: + # TODO: remove this test in Beets 3.0.0 + def test_legacy_metadata_plugin_deprecation(self): + """Test that a MetadataSourcePlugin with 'legacy' data_source + raises a deprecation warning and all function and properties are + copied from the base class. + """ + with pytest.warns(DeprecationWarning, match="LegacyMetadataPlugin"): + + class LegacyMetadataPlugin(plugins.BeetsPlugin): + data_source = "legacy" + + # Assert all methods are present + assert hasattr(LegacyMetadataPlugin, "albums_for_ids") + assert hasattr(LegacyMetadataPlugin, "tracks_for_ids") + assert hasattr(LegacyMetadataPlugin, "data_source_mismatch_penalty") + assert hasattr(LegacyMetadataPlugin, "_extract_id") + assert hasattr(LegacyMetadataPlugin, "get_artist") From f339d8a4d381e5bac50cf6bb3e69cdf126d1e495 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Tue, 14 Oct 2025 17:40:03 +0200 Subject: [PATCH 2/4] slight simplification. --- beets/plugins.py | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index 9c7a93b7f..5e7ac6f96 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -192,20 +192,29 @@ class BeetsPlugin(metaclass=abc.ABCMeta): stacklevel=3, ) - abstracts = MetadataSourcePlugin.__abstractmethods__ - - for name, method in inspect.getmembers(MetadataSourcePlugin): - # Skip if already defined in the subclass - if hasattr(cls, name) or name in abstracts: - continue - - # Copy functions, methods, and properties - if ( - inspect.isfunction(method) - or inspect.ismethod(method) - or isinstance(method, cached_property) - ): - setattr(cls, name, method) + for name, method in inspect.getmembers( + MetadataSourcePlugin, + predicate=lambda f: ( + ( + isinstance(f, cached_property) + and f.attrname is not None + and not hasattr(BeetsPlugin, f.attrname) + ) + or ( + isinstance(f, property) + and f.fget is not None + and f.fget.__name__ is not None + and not hasattr(BeetsPlugin, f.fget.__name__) + ) + or ( + inspect.isfunction(f) + and f.__name__ + not in MetadataSourcePlugin.__abstractmethods__ + and not hasattr(BeetsPlugin, f.__name__) + ) + ), + ): + setattr(cls, name, method) def __init__(self, name: str | None = None): """Perform one-time plugin setup.""" From 365ff6b0303804cd1408c6f2e250eb22f5ad8b0c Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Tue, 14 Oct 2025 18:50:52 +0200 Subject: [PATCH 3/4] Added test additions --- test/autotag/test_distance.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/autotag/test_distance.py b/test/autotag/test_distance.py index b327bbe44..213d32956 100644 --- a/test/autotag/test_distance.py +++ b/test/autotag/test_distance.py @@ -11,6 +11,7 @@ from beets.autotag.distance import ( ) from beets.library import Item from beets.metadata_plugins import MetadataSourcePlugin, get_penalty +from beets.plugins import BeetsPlugin from beets.test.helper import ConfigMixin _p = pytest.param @@ -310,8 +311,13 @@ class TestDataSourceDistance: def candidates(self, *args, **kwargs): ... def item_candidates(self, *args, **kwargs): ... - class OriginalPlugin(TestMetadataSourcePlugin): - pass + # We use BeetsPlugin here to check if our compatibility layer + # for pre 2.4.0 MetadataPlugins is working as expected + # TODO: Replace BeetsPlugin with TestMetadataSourcePlugin in v3.0.0 + with pytest.deprecated_call(): + + class OriginalPlugin(BeetsPlugin): + data_source = "Original" class OtherPlugin(TestMetadataSourcePlugin): @property @@ -332,6 +338,7 @@ class TestDataSourceDistance: [ _p("Original", "Original", 0.5, 1.0, True, MATCH, id="match"), _p("Original", "Other", 0.5, 1.0, True, MISMATCH, id="mismatch"), + _p("Other", "Original", 0.5, 1.0, True, MISMATCH, id="mismatch"), _p("Original", "unknown", 0.5, 1.0, True, MISMATCH, id="mismatch-unknown"), # noqa: E501 _p("Original", None, 0.5, 1.0, True, MISMATCH, id="mismatch-no-info"), # noqa: E501 _p(None, "Other", 0.5, 1.0, True, MISMATCH, id="mismatch-no-original-multiple-sources"), # noqa: E501 From 391ca4ca26fe5a7946bdcbfecec4f02e740d6393 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Tue, 14 Oct 2025 19:45:56 +0200 Subject: [PATCH 4/4] Yet some more simplification. --- beets/plugins.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index 5e7ac6f96..678d653b4 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -192,24 +192,21 @@ class BeetsPlugin(metaclass=abc.ABCMeta): stacklevel=3, ) + method: property | cached_property[Any] | Callable[..., Any] for name, method in inspect.getmembers( MetadataSourcePlugin, - predicate=lambda f: ( + predicate=lambda f: ( # type: ignore[arg-type] ( - isinstance(f, cached_property) - and f.attrname is not None - and not hasattr(BeetsPlugin, f.attrname) - ) - or ( - isinstance(f, property) - and f.fget is not None - and f.fget.__name__ is not None - and not hasattr(BeetsPlugin, f.fget.__name__) + isinstance(f, (property, cached_property)) + and not hasattr( + BeetsPlugin, + getattr(f, "attrname", None) or f.fget.__name__, # type: ignore[union-attr] + ) ) or ( inspect.isfunction(f) and f.__name__ - not in MetadataSourcePlugin.__abstractmethods__ + and not getattr(f, "__isabstractmethod__", False) and not hasattr(BeetsPlugin, f.__name__) ) ),