chroma: gate candidates on the musicbrainz plugin being enabled

beetsplug/chroma.py

@@ -21,23 +21,23 @@
 import heapq
 import re
 from collections import defaultdict
-from functools import cached_property, partial
+from functools import partial
 from typing import TYPE_CHECKING
 
 import acoustid
 import confuse
 
 from beets import config, ui, util
 from beets.autotag.distance import Distance
-from beets.metadata_plugins import MetadataSourcePlugin
+from beets.metadata_plugins import MetadataSourcePlugin, get_metadata_source
 from beets.util.color import colorize
-from beetsplug.musicbrainz import MusicBrainzPlugin
 
 if TYPE_CHECKING:
     from collections.abc import Iterable, Iterator
 
     from beets.autotag.hooks import TrackInfo
     from beets.library.models import Item
+    from beetsplug.musicbrainz import MusicBrainzPlugin
 
 API_KEY = "1vOwZtEn"
 SCORE_THRESH = 0.5
@@ -196,9 +196,24 @@ def __init__(self):
             self.register_listener("import_task_start", self.fingerprint_task)
         self.register_listener("import_task_apply", apply_acoustid_metadata)
 
-    @cached_property
-    def mb(self) -> MusicBrainzPlugin:
-        return MusicBrainzPlugin()
+    def _get_musicbrainz(self) -> MusicBrainzPlugin | None:
+        """Return the loaded MusicBrainz plugin instance, or ``None``.
+
+        Acoustid fingerprint lookups return MusicBrainz release and
+        recording IDs, so chroma can only produce autotagger candidates
+        by resolving those IDs through the musicbrainz plugin. When the
+        user has not enabled the ``musicbrainz`` plugin, we must not
+        return any candidates — otherwise MusicBrainz-sourced results
+        would silently appear during tagging even though the user
+        opted out of that metadata source.
+        """
+        plugin = get_metadata_source("musicbrainz")
+        if plugin is None:
+            self._log.debug(
+                "musicbrainz plugin not enabled; "
+                "acoustid matches will not produce candidates"
+            )
+        return plugin  # type: ignore[return-value]
 
     def fingerprint_task(self, task, session):
         return fingerprint_task(self._log, task, session)
@@ -214,9 +229,13 @@ def track_distance(self, item, info):
         return dist
 
     def candidates(self, items, artist, album, va_likely):
+        mb = self._get_musicbrainz()
+        if mb is None:
+            return []
+
         albums = []
         for relid in prefix(_all_releases(items), MAX_RELEASES):
-            album = self.mb.album_for_id(relid)
+            album = mb.album_for_id(relid)
             if album:
                 albums.append(album)
 
@@ -227,10 +246,14 @@ def item_candidates(self, item, artist, title) -> Iterable[TrackInfo]:
         if item.path not in _matches:
             return []
 
+        mb = self._get_musicbrainz()
+        if mb is None:
+            return []
+
         recording_ids, _ = _matches[item.path]
         tracks = []
         for recording_id in prefix(recording_ids, MAX_RECORDINGS):
-            track = self.mb.track_for_id(recording_id)
+            track = mb.track_for_id(recording_id)
             if track:
                 tracks.append(track)
         self._log.debug("acoustid item candidates: {}", len(tracks))

docs/changelog.rst

@@ -84,6 +84,12 @@ Bug fixes
   multi-valued fields such as ``genres`` by applying rules to each matching list
   entry. Additionally, apply rewrite rules in config order, so that multiple
   rules can be applied to the same field. :bug:`6515`
+- :doc:`plugins/chroma`: Do not produce MusicBrainz-sourced autotagger
+  candidates when the :doc:`plugins/musicbrainz` plugin is not enabled.
+  Previously, ``chroma`` instantiated its own ``MusicBrainzPlugin`` directly and
+  called ``album_for_id`` / ``track_for_id`` on it regardless of the user's
+  plugin configuration, so MusicBrainz results would surface even for users who
+  had intentionally disabled MusicBrainz. :bug:`6212`
 
 For plugin developers
 ~~~~~~~~~~~~~~~~~~~~~

docs/plugins/chroma.rst

@@ -122,6 +122,16 @@ library.) The generated fingerprints will be stored in the library database. If
 you have the ``import.write`` config option enabled, they will also be written
 to files' metadata.
 
+.. note::
+
+    The ``chroma`` plugin turns Acoustid fingerprint matches into autotagger
+    candidates by resolving them through the :doc:`musicbrainz` plugin, so you
+    need to enable ``musicbrainz`` alongside ``chroma`` to get album and track
+    candidates from acoustid lookups. If ``musicbrainz`` is not enabled, the
+    ``chroma`` plugin will still fingerprint your files and store the
+    ``acoustid_id`` and ``acoustid_fingerprint`` fields, but it will not
+    contribute candidates during autotagging.
+
 .. _submitfp:
 
 Configuration

test/plugins/test_chroma.py

@@ -12,10 +12,16 @@
 # included in all copies or substantial portions of the Software.
 
 
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
+import pytest
+
+import beets.plugins
+from beets import config, metadata_plugins
+from beets.autotag.hooks import AlbumInfo, TrackInfo
 from beets.library import Item
 from beets.test.helper import ImportTestCase, IOMixin, PluginMixin
+from beetsplug import chroma
 
 TEST_TITLE_1 = "TEST_TITLE_1"
 TEST_TITLE_2 = "TEST_TITLE_2"
@@ -77,3 +83,148 @@ def test_chroma_search_close(self, compare_fingerprints):
         output = self.run_search(FINGERPRINT_1_CLOSE)
         assert self.line_count(output) == 2
         assert TEST_TITLE_1 in output.split("\n")[0]
+
+
+# -----------------------------------------------------------------------------
+# Regression tests for issue #6212: chroma must not produce MusicBrainz-sourced
+# autotagger candidates when the musicbrainz plugin is not enabled.
+# -----------------------------------------------------------------------------
+
+
+@pytest.fixture
+def reset_plugin_state():
+    """Fully reset plugin and metadata-plugin state around each test.
+
+    ``beets.plugins._instances`` is a module-level list and
+    ``metadata_plugins.get_metadata_source`` / ``find_metadata_source_plugins``
+    are ``@cache`` decorated, so tests that change the loaded plugin set
+    must clear both.
+    """
+    beets.plugins.BeetsPlugin.listeners.clear()
+    beets.plugins.BeetsPlugin._raw_listeners.clear()
+    beets.plugins._instances.clear()
+    config["plugins"] = []
+    metadata_plugins.find_metadata_source_plugins.cache_clear()
+    metadata_plugins.get_metadata_source.cache_clear()
+    chroma._matches.clear()
+
+    yield
+
+    chroma._matches.clear()
+    beets.plugins.BeetsPlugin.listeners.clear()
+    beets.plugins.BeetsPlugin._raw_listeners.clear()
+    beets.plugins._instances.clear()
+    config["plugins"] = []
+    metadata_plugins.find_metadata_source_plugins.cache_clear()
+    metadata_plugins.get_metadata_source.cache_clear()
+
+
+def _load_plugins(*names: str) -> None:
+    """Load the given plugins into the global beets plugin registry."""
+    config["plugins"] = list(names)
+    beets.plugins.load_plugins()
+
+
+def _seed_acoustid_match(
+    item_path: bytes = b"/fake/path.mp3",
+    recording_ids: list[str] | None = None,
+    release_ids: list[str] | None = None,
+) -> Item:
+    """Seed the chroma module-level match cache as if acoustid had run."""
+    if recording_ids is None:
+        recording_ids = ["rec-id-1"]
+    if release_ids is None:
+        # Three copies to exceed COMMON_REL_THRESH in _all_releases.
+        release_ids = ["rel-id-1", "rel-id-1", "rel-id-1"]
+
+    chroma._matches[item_path] = (recording_ids, release_ids)
+    return Item(path=item_path)
+
+
+@pytest.mark.usefixtures("reset_plugin_state")
+class TestChromaWithoutMusicBrainz:
+    """Regression tests for issue #6212.
+
+    When the ``musicbrainz`` plugin is not loaded, acoustid must not
+    return any candidates. Before the fix, chroma created its own
+    ``MusicBrainzPlugin`` instance directly, bypassing the plugin
+    registry, so MusicBrainz-sourced candidates would surface regardless
+    of the user's plugin configuration.
+    """
+
+    def test_candidates_returns_empty_without_musicbrainz(self):
+        _load_plugins("chroma")
+        plugin = chroma.AcoustidPlugin()
+
+        item = _seed_acoustid_match()
+
+        result = plugin.candidates(
+            [item], artist="A", album="B", va_likely=False
+        )
+
+        assert list(result) == []
+
+    def test_item_candidates_returns_empty_without_musicbrainz(self):
+        _load_plugins("chroma")
+        plugin = chroma.AcoustidPlugin()
+
+        item = _seed_acoustid_match()
+
+        result = plugin.item_candidates(item, artist="A", title="B")
+
+        assert list(result) == []
+
+
+@pytest.mark.usefixtures("reset_plugin_state")
+class TestChromaWithMusicBrainz:
+    """Ensure candidates are still produced when musicbrainz IS loaded.
+
+    The fix routes chroma through ``get_metadata_source("musicbrainz")``
+    rather than a private ``MusicBrainzPlugin`` instance; we verify both
+    that results flow through and that the shared registry instance is
+    the one being invoked.
+    """
+
+    def test_candidates_returns_albums_when_musicbrainz_enabled(
+        self, monkeypatch
+    ):
+        _load_plugins("chroma", "musicbrainz")
+
+        fake_album = AlbumInfo(
+            tracks=[], album_id="rel-id-1", album="Fake Album"
+        )
+        mb_plugin = metadata_plugins.get_metadata_source("musicbrainz")
+        assert mb_plugin is not None
+        monkeypatch.setattr(
+            mb_plugin, "album_for_id", MagicMock(return_value=fake_album)
+        )
+
+        plugin = chroma.AcoustidPlugin()
+        item = _seed_acoustid_match()
+
+        result = list(
+            plugin.candidates([item], artist="A", album="B", va_likely=False)
+        )
+
+        assert result == [fake_album]
+        mb_plugin.album_for_id.assert_called_with("rel-id-1")
+
+    def test_item_candidates_returns_tracks_when_musicbrainz_enabled(
+        self, monkeypatch
+    ):
+        _load_plugins("chroma", "musicbrainz")
+
+        fake_track = TrackInfo(title="Fake Track", track_id="rec-id-1")
+        mb_plugin = metadata_plugins.get_metadata_source("musicbrainz")
+        assert mb_plugin is not None
+        monkeypatch.setattr(
+            mb_plugin, "track_for_id", MagicMock(return_value=fake_track)
+        )
+
+        plugin = chroma.AcoustidPlugin()
+        item = _seed_acoustid_match()
+
+        result = list(plugin.item_candidates(item, artist="A", title="B"))
+
+        assert result == [fake_track]
+        mb_plugin.track_for_id.assert_called_with("rec-id-1")