From 225c21b90fb9dc8b6a72bb547ea0b66b2cec71c7 Mon Sep 17 00:00:00 2001 From: Skia Date: Fri, 4 Apr 2025 17:16:16 +0200 Subject: [PATCH 01/11] plugins/thumbnails: fix FFI with GIO on s390x MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using the correct function signature for g_file_new_for_path fixes the tests on s390x. I do not have the full story on why this failed consistently only on s390x, but I guess the big endian might have something to play with this. Here is how the tests were failing: ``` 169s ___________________________ ThumbnailsTest.test_uri ____________________________ 169s 169s self = 169s 169s def test_uri(self): 169s gio = GioURI() 169s if not gio.available: 169s self.skipTest("GIO library not found") 169s 169s > assert gio.uri("/foo") == "file:///" # silent fail 169s E AssertionError: assert '' == 'file:///' 169s E 169s E - file:/// 169s 169s test/plugins/test_thumbnails.py:268: AssertionError ``` You can see a full log here [1] and a history of consistent failure here [2]. Both links are bound to expire at some point, sorry future archeologist 🤷. [1]: https://autopkgtest.ubuntu.com/results/autopkgtest-plucky/plucky/s390x/b/beets/20250403_162414_5d1da@/log.gz#S5 [2]: https://autopkgtest.ubuntu.com/packages/beets/plucky/s390x --- beetsplug/thumbnails.py | 2 +- docs/changelog.rst | 2 ++ test/plugins/test_thumbnails.py | 5 ++++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/beetsplug/thumbnails.py b/beetsplug/thumbnails.py index 3f88248e0..44ffd12de 100644 --- a/beetsplug/thumbnails.py +++ b/beetsplug/thumbnails.py @@ -246,7 +246,7 @@ class GioURI(URIGetter): if self.available: self.libgio.g_type_init() # for glib < 2.36 - self.libgio.g_file_get_uri.argtypes = [ctypes.c_char_p] + self.libgio.g_file_new_for_path.argtypes = [ctypes.c_char_p] self.libgio.g_file_new_for_path.restype = ctypes.c_void_p self.libgio.g_file_get_uri.argtypes = [ctypes.c_void_p] diff --git a/docs/changelog.rst b/docs/changelog.rst index 88d87e32f..47cd6106d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,8 @@ New features: Bug fixes: +* Fix API call to GIO on big endian architectures (like s390x) in thumbnails plugin. + :bug:`5708` * :doc:`plugins/listenbrainz`: Fix rST formatting for URLs of Listenbrainz API Key documentation and config.yaml. * :doc:`plugins/listenbrainz`: Fix ``UnboundLocalError`` in cases where 'mbid' is not defined. * :doc:`plugins/fetchart`: Fix fetchart bug where a tempfile could not be deleted due to never being diff --git a/test/plugins/test_thumbnails.py b/test/plugins/test_thumbnails.py index 3eb36cd25..00cd545d4 100644 --- a/test/plugins/test_thumbnails.py +++ b/test/plugins/test_thumbnails.py @@ -265,7 +265,10 @@ class ThumbnailsTest(BeetsTestCase): if not gio.available: self.skipTest("GIO library not found") - assert gio.uri("/foo") == "file:///" # silent fail + import ctypes + + with pytest.raises(ctypes.ArgumentError): + gio.uri("/foo") assert gio.uri(b"/foo") == "file:///foo" assert gio.uri(b"/foo!") == "file:///foo!" assert ( From f4de44f610eb9afb2ef42c81872ee3af67fa6f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 14 Apr 2025 02:18:38 +0100 Subject: [PATCH 02/11] Add plugin prefix in changelog note --- docs/changelog.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 47cd6106d..183fa006e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,7 +24,8 @@ New features: Bug fixes: -* Fix API call to GIO on big endian architectures (like s390x) in thumbnails plugin. +* :doc:`plugins/thumbnails`: Fix API call to GIO on big endian architectures + (like s390x) in thumbnails plugin. :bug:`5708` * :doc:`plugins/listenbrainz`: Fix rST formatting for URLs of Listenbrainz API Key documentation and config.yaml. * :doc:`plugins/listenbrainz`: Fix ``UnboundLocalError`` in cases where 'mbid' is not defined. From 6a192d0bdb48ba64fd1236df43c13a215c211c5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sat, 15 Feb 2025 16:33:36 +0000 Subject: [PATCH 03/11] albums_for_id -> album_for_id and return a single candidate instead of an iterator --- beets/autotag/hooks.py | 30 ++++++++---------------------- beets/autotag/match.py | 12 +++++------- beets/plugins.py | 39 ++++++++++++++++++++++++++++----------- 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index 3fa80c6f3..81cfd7bb2 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -602,8 +602,7 @@ def album_for_mbid(release_id: str) -> AlbumInfo | None: if the ID is not found. """ try: - album = mb.album_for_id(release_id) - if album: + if album := mb.album_for_id(release_id): plugins.send("albuminfo_received", info=album) return album except mb.MusicBrainzAPIError as exc: @@ -616,8 +615,7 @@ def track_for_mbid(recording_id: str) -> TrackInfo | None: if the ID is not found. """ try: - track = mb.track_for_id(recording_id) - if track: + if track := mb.track_for_id(recording_id): plugins.send("trackinfo_received", info=track) return track except mb.MusicBrainzAPIError as exc: @@ -625,26 +623,14 @@ def track_for_mbid(recording_id: str) -> TrackInfo | None: return None -def albums_for_id(album_id: str) -> Iterable[AlbumInfo]: - """Get a list of albums for an ID.""" - a = album_for_mbid(album_id) - if a: - yield a - for a in plugins.album_for_id(album_id): - if a: - plugins.send("albuminfo_received", info=a) - yield a +def album_for_id(_id: str) -> AlbumInfo | None: + """Get AlbumInfo object for the given ID string.""" + return album_for_mbid(_id) or plugins.album_for_id(_id) -def tracks_for_id(track_id: str) -> Iterable[TrackInfo]: - """Get a list of tracks for an ID.""" - t = track_for_mbid(track_id) - if t: - yield t - for t in plugins.track_for_id(track_id): - if t: - plugins.send("trackinfo_received", info=t) - yield t +def track_for_id(_id: str) -> TrackInfo | None: + """Get TrackInfo object for the given ID string.""" + return track_for_mbid(_id) or plugins.track_for_id(_id) def invoke_mb(call_func: Callable, *args): diff --git a/beets/autotag/match.py b/beets/autotag/match.py index a7121fd34..bc30ccea2 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -510,8 +510,8 @@ def tag_album( if search_ids: for search_id in search_ids: log.debug("Searching for album ID: {0}", search_id) - for album_info_for_id in hooks.albums_for_id(search_id): - _add_candidate(items, candidates, album_info_for_id) + if info := hooks.album_for_id(search_id): + _add_candidate(items, candidates, info) # Use existing metadata or text search. else: @@ -590,11 +590,9 @@ def tag_item( if trackids: for trackid in trackids: log.debug("Searching for track ID: {0}", trackid) - for track_info in hooks.tracks_for_id(trackid): - dist = track_distance(item, track_info, incl_artist=True) - candidates[track_info.track_id] = hooks.TrackMatch( - dist, track_info - ) + if info := hooks.track_for_id(trackid): + dist = track_distance(item, info, incl_artist=True) + candidates[info.track_id] = hooks.TrackMatch(dist, info) # If this is a good match, then don't keep searching. rec = _recommendation(_sort_candidates(candidates.values())) if ( diff --git a/beets/plugins.py b/beets/plugins.py index 299c41815..2ca98649e 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -14,18 +14,25 @@ """Support for beets plugins.""" +from __future__ import annotations + import abc import inspect import re import traceback from collections import defaultdict from functools import wraps +from typing import TYPE_CHECKING import mediafile import beets from beets import logging +if TYPE_CHECKING: + from beets.autotag.hooks import AlbumInfo, TrackInfo + + PLUGIN_NAMESPACE = "beetsplug" # Plugins using the Last.fm API can share the same API key. @@ -290,7 +297,7 @@ def load_plugins(names=()): ) -_instances = {} +_instances: dict[type[BeetsPlugin], BeetsPlugin] = {} def find_plugins(): @@ -397,20 +404,30 @@ def item_candidates(item, artist, title): yield from plugin.item_candidates(item, artist, title) -def album_for_id(album_id): - """Get AlbumInfo objects for a given ID string.""" +def album_for_id(_id: str) -> AlbumInfo | None: + """Get AlbumInfo object for the given ID string. + + A single ID can yield just a single album, so we return the first match. + """ for plugin in find_plugins(): - album = plugin.album_for_id(album_id) - if album: - yield album + if info := plugin.album_for_id(_id): + send("albuminfo_received", info=info) + return info + + return None -def track_for_id(track_id): - """Get TrackInfo objects for a given ID string.""" +def track_for_id(_id: str) -> TrackInfo | None: + """Get TrackInfo object for the given ID string. + + A single ID can yield just a single track, so we return the first match. + """ for plugin in find_plugins(): - track = plugin.track_for_id(track_id) - if track: - yield track + if info := plugin.track_for_id(_id): + send("trackinfo_received", info=info) + return info + + return None def template_funcs(): From 4c1f217ce03ce03c2e266e354d6b943cbc70f507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sat, 15 Feb 2025 21:29:54 +0000 Subject: [PATCH 04/11] missing: support non-musicbrainz data sources --- beetsplug/deezer.py | 2 +- beetsplug/discogs.py | 35 +++-------------------------------- beetsplug/missing.py | 22 ++++++++++++---------- docs/changelog.rst | 1 + docs/plugins/missing.rst | 14 +++++++------- test/plugins/test_discogs.py | 3 +++ 6 files changed, 27 insertions(+), 50 deletions(-) diff --git a/beetsplug/deezer.py b/beetsplug/deezer.py index 6f05474b9..25815e8d3 100644 --- a/beetsplug/deezer.py +++ b/beetsplug/deezer.py @@ -71,7 +71,7 @@ class DeezerPlugin(MetadataSourcePlugin, BeetsPlugin): self._log.error("Error fetching data from {}\n Error: {}", url, e) return None if "error" in data: - self._log.error("Deezer API error: {}", data["error"]["message"]) + self._log.debug("Deezer API error: {}", data["error"]["message"]) return None return data diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 0dc8e8a17..19521b035 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -29,7 +29,6 @@ from string import ascii_lowercase import confuse from discogs_client import Client, Master, Release -from discogs_client import __version__ as dc_string from discogs_client.exceptions import DiscogsAPIError from requests.exceptions import ConnectionError from typing_extensions import TypedDict @@ -64,7 +63,6 @@ class ReleaseFormat(TypedDict): class DiscogsPlugin(BeetsPlugin): def __init__(self): super().__init__() - self.check_discogs_client() self.config.add( { "apikey": API_KEY, @@ -80,24 +78,7 @@ class DiscogsPlugin(BeetsPlugin): self.config["apikey"].redact = True self.config["apisecret"].redact = True self.config["user_token"].redact = True - self.discogs_client = None - self.register_listener("import_begin", self.setup) - - def check_discogs_client(self): - """Ensure python3-discogs-client version >= 2.3.15""" - dc_min_version = [2, 3, 15] - dc_version = [int(elem) for elem in dc_string.split(".")] - min_len = min(len(dc_version), len(dc_min_version)) - gt_min = [ - (elem > elem_min) - for elem, elem_min in zip( - dc_version[:min_len], dc_min_version[:min_len] - ) - ] - if True not in gt_min: - self._log.warning( - "python3-discogs-client version should be >= 2.3.15" - ) + self.setup() def setup(self, session=None): """Create the `discogs_client` field. Authenticate if necessary.""" @@ -179,13 +160,9 @@ class DiscogsPlugin(BeetsPlugin): """Returns a list of AlbumInfo objects for discogs search results matching an album and artist (if not various). """ - if not self.discogs_client: - return - if not album and not artist: self._log.debug( - "Skipping Discogs query. Files missing album and " - "artist tags." + "Skipping Discogs query. Files missing album and artist tags." ) return [] @@ -254,12 +231,9 @@ class DiscogsPlugin(BeetsPlugin): :return: Candidate TrackInfo objects. :rtype: list[beets.autotag.hooks.TrackInfo] """ - if not self.discogs_client: - return [] - if not artist and not title: self._log.debug( - "Skipping Discogs query. File missing artist and " "title tags." + "Skipping Discogs query. File missing artist and title tags." ) return [] @@ -290,9 +264,6 @@ class DiscogsPlugin(BeetsPlugin): """Fetches an album by its Discogs ID and returns an AlbumInfo object or None if the album is not found. """ - if not self.discogs_client: - return - self._log.debug("Searching for release {0}", album_id) discogs_id = extract_discogs_id_regex(album_id) diff --git a/beetsplug/missing.py b/beetsplug/missing.py index d5e4deda1..004749a37 100644 --- a/beetsplug/missing.py +++ b/beetsplug/missing.py @@ -16,6 +16,7 @@ """List missing tracks.""" from collections import defaultdict +from collections.abc import Iterator import musicbrainzngs from musicbrainzngs.musicbrainz import MusicBrainzError @@ -23,7 +24,7 @@ from musicbrainzngs.musicbrainz import MusicBrainzError from beets import config from beets.autotag import hooks from beets.dbcore import types -from beets.library import Item +from beets.library import Album, Item from beets.plugins import BeetsPlugin from beets.ui import Subcommand, decargs, print_ @@ -226,19 +227,20 @@ class MissingPlugin(BeetsPlugin): if total: print(total_missing) - def _missing(self, album): + def _missing(self, album: Album) -> Iterator[Item]: """Query MusicBrainz to determine items missing from `album`.""" - item_mbids = [x.mb_trackid for x in album.items()] - if len(list(album.items())) < album.albumtotal: - # fetch missing items - # TODO: Implement caching that without breaking other stuff - album_info = hooks.album_for_mbid(album.mb_albumid) - for track_info in getattr(album_info, "tracks", []): + if len(album.items()) == album.albumtotal: + return + + item_mbids = {x.mb_trackid for x in album.items()} + # fetch missing items + # TODO: Implement caching that without breaking other stuff + if album_info := hooks.album_for_id(album.mb_albumid): + for track_info in album_info.tracks: if track_info.track_id not in item_mbids: - item = _item(track_info, album_info, album.id) self._log.debug( "track {0} in album {1}", track_info.track_id, album_info.album_id, ) - yield item + yield _item(track_info, album_info, album.id) diff --git a/docs/changelog.rst b/docs/changelog.rst index 183fa006e..4dedabf33 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,6 +21,7 @@ New features: when fetching lyrics. * :doc:`plugins/lyrics`: Rewrite lyrics translation functionality to use Azure AI Translator API and add relevant instructions to the documentation. +* :doc:`plugins/missing`: Add support for all metadata sources. Bug fixes: diff --git a/docs/plugins/missing.rst b/docs/plugins/missing.rst index 38a0d5a08..f4daecc3a 100644 --- a/docs/plugins/missing.rst +++ b/docs/plugins/missing.rst @@ -1,17 +1,17 @@ Missing Plugin ============== -This plugin adds a new command, ``missing`` or ``miss``, which finds -and lists, for every album in your collection, which or how many -tracks are missing. Listing missing files requires one network call to -MusicBrainz. Merely counting missing files avoids any network calls. +This plugin adds a new command, ``missing`` or ``miss``, which finds and lists +missing tracks for albums in your collection. Each album requires one network +call to album data source. Usage ----- -Add the ``missing`` plugin to your configuration (see :ref:`using-plugins`). -By default, the ``beet missing`` command lists the names of tracks that your -library is missing from each album. It can also list the names of albums that +Add the ``missing`` plugin to your configuration (see :ref:`using-plugins`). By +default, the ``beet missing`` command fetches album information from the origin +data source and lists names of the **tracks** that are missing from your +library. It can also list the names of albums that your library is missing from each artist. You can customize the output format, count the number of missing tracks per album, or total up the number of missing diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 8a4609e25..5e327ab27 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -14,6 +14,8 @@ """Tests for discogs plugin.""" +from unittest.mock import Mock, patch + import pytest from beets import config @@ -23,6 +25,7 @@ from beets.util.id_extractors import extract_discogs_id_regex from beetsplug.discogs import DiscogsPlugin +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) class DGAlbumInfoTest(BeetsTestCase): def _make_release(self, tracks=None): """Returns a Bag that mimics a discogs_client.Release. The list From 441cd36e8acc75476dcc11bcc5a88eeca89b3fed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 16 Feb 2025 00:41:36 +0000 Subject: [PATCH 05/11] missing: clarify that only musicbrainz backend supports missing albums for artist And give this functionality a small refactor. --- beetsplug/missing.py | 76 +++++++++++++++++----------------------- docs/plugins/missing.rst | 28 ++++++++------- 2 files changed, 48 insertions(+), 56 deletions(-) diff --git a/beetsplug/missing.py b/beetsplug/missing.py index 004749a37..ccaa65320 100644 --- a/beetsplug/missing.py +++ b/beetsplug/missing.py @@ -24,10 +24,12 @@ from musicbrainzngs.musicbrainz import MusicBrainzError from beets import config from beets.autotag import hooks from beets.dbcore import types -from beets.library import Album, Item +from beets.library import Album, Item, Library from beets.plugins import BeetsPlugin from beets.ui import Subcommand, decargs, print_ +MB_ARTIST_QUERY = r"mb_albumartistid::^\w{8}-\w{4}-\w{4}-\w{4}-\w{12}$" + def _missing_count(album): """Return number of missing items in `album`.""" @@ -166,65 +168,51 @@ class MissingPlugin(BeetsPlugin): for item in self._missing(album): print_(format(item, fmt)) - def _missing_albums(self, lib, query): + def _missing_albums(self, lib: Library, query: list[str]) -> None: """Print a listing of albums missing from each artist in the library matching query. """ - total = self.config["total"].get() + query.append(MB_ARTIST_QUERY) - albums = lib.albums(query) - # build dict mapping artist to list of their albums in library - albums_by_artist = defaultdict(list) - for alb in albums: - artist = (alb["albumartist"], alb["mb_albumartistid"]) - albums_by_artist[artist].append(alb) + # build dict mapping artist to set of their album ids in library + album_ids_by_artist = defaultdict(set) + for album in lib.albums(query): + # TODO(@snejus): Some releases have different `albumartist` for the + # same `mb_albumartistid`. Since we're grouping by the combination + # of these two fields, we end up processing the same + # `mb_albumartistid` multiple times: calling MusicBrainz API and + # reporting the same set of missing albums. Instead, we should + # group by `mb_albumartistid` field only. + artist = (album["albumartist"], album["mb_albumartistid"]) + album_ids_by_artist[artist].add(album) total_missing = 0 - - # build dict mapping artist to list of all albums - for artist, albums in albums_by_artist.items(): - if artist[1] is None or artist[1] == "": - albs_no_mbid = ["'" + a["album"] + "'" for a in albums] - self._log.info( - "No musicbrainz ID for artist '{}' found in album(s) {}; " - "skipping", - artist[0], - ", ".join(albs_no_mbid), - ) - continue - + calculating_total = self.config["total"].get() + for (artist, artist_id), album_ids in album_ids_by_artist.items(): try: - resp = musicbrainzngs.browse_release_groups(artist=artist[1]) - release_groups = resp["release-group-list"] + resp = musicbrainzngs.browse_release_groups(artist=artist_id) except MusicBrainzError as err: self._log.info( "Couldn't fetch info for artist '{}' ({}) - '{}'", - artist[0], - artist[1], + artist, + artist_id, err, ) continue - missing = [] - present = [] - for rg in release_groups: - missing.append(rg) - for alb in albums: - if alb["mb_releasegroupid"] == rg["id"]: - missing.remove(rg) - present.append(rg) - break + missing_titles = [ + f"{artist} - {rg['title']}" + for rg in resp["release-group-list"] + if rg["id"] not in album_ids + ] - total_missing += len(missing) - if total: - continue + if calculating_total: + total_missing += len(missing_titles) + else: + for title in missing_titles: + print(title) - missing_titles = {rg["title"] for rg in missing} - - for release_title in missing_titles: - print_("{} - {}".format(artist[0], release_title)) - - if total: + if calculating_total: print(total_missing) def _missing(self, album: Album) -> Iterator[Item]: diff --git a/docs/plugins/missing.rst b/docs/plugins/missing.rst index f4daecc3a..9cd3fde71 100644 --- a/docs/plugins/missing.rst +++ b/docs/plugins/missing.rst @@ -8,24 +8,28 @@ call to album data source. Usage ----- -Add the ``missing`` plugin to your configuration (see :ref:`using-plugins`). By -default, the ``beet missing`` command fetches album information from the origin -data source and lists names of the **tracks** that are missing from your -library. It can also list the names of albums that -your library is missing from each artist. -You can customize the output format, count -the number of missing tracks per album, or total up the number of missing -tracks over your whole library, using command-line switches:: +Add the ``missing`` plugin to your configuration (see :ref:`using-plugins`). +The ``beet missing`` command fetches album information from the origin data +source and lists names of the **tracks** that are missing from your library. + +It can also list the names of missing **albums** for each artist, although this +is limited to albums from the MusicBrainz data source only. + +You can customize the output format, show missing counts instead of track +titles, or display the total number of missing entities across your entire +library:: -f FORMAT, --format=FORMAT print with custom FORMAT -c, --count count missing tracks per album - -t, --total count total of missing tracks or albums - -a, --album show missing albums for artist instead of tracks + -t, --total count totals across the entire library + -a, --album show missing albums for artist instead of tracks for album -…or by editing corresponding options. +…or by editing the corresponding configuration options. -Note that ``-c`` is ignored when used with ``-a``. +.. warning:: + + Option ``-c`` is ignored when used with ``-a``. Configuration ------------- From d1d681c1ff5166d94278e261a58660c06b96c040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 16 Feb 2025 09:51:58 +0000 Subject: [PATCH 06/11] mbsync: support other data sources --- beetsplug/mbsync.py | 32 +++------------------ docs/changelog.rst | 1 + docs/plugins/mbsync.rst | 15 +++++----- test/plugins/test_mbsync.py | 56 +++++++++++++++++-------------------- 4 files changed, 38 insertions(+), 66 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index 283c40186..5a17473c5 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -12,17 +12,14 @@ # The above copyright notice and this permission notice shall be # included in all copies or substantial portions of the Software. -"""Update library's tags using MusicBrainz.""" +"""Synchronise library metadata with metadata source backends.""" -import re from collections import defaultdict from beets import autotag, library, ui, util from beets.autotag import hooks from beets.plugins import BeetsPlugin, apply_item_changes -MBID_REGEX = r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}" - class MBSyncPlugin(BeetsPlugin): def __init__(self): @@ -84,17 +81,7 @@ class MBSyncPlugin(BeetsPlugin): ) continue - # Do we have a valid MusicBrainz track ID? - if not re.match(MBID_REGEX, item.mb_trackid): - self._log.info( - "Skipping singleton with invalid mb_trackid:" + " {0}", - item_formatted, - ) - continue - - # Get the MusicBrainz recording info. - track_info = hooks.track_for_mbid(item.mb_trackid) - if not track_info: + if not (track_info := hooks.track_for_id(item.mb_trackid)): self._log.info( "Recording ID not found: {0} for track {0}", item.mb_trackid, @@ -121,18 +108,7 @@ class MBSyncPlugin(BeetsPlugin): continue items = list(a.items()) - - # Do we have a valid MusicBrainz album ID? - if not re.match(MBID_REGEX, a.mb_albumid): - self._log.info( - "Skipping album with invalid mb_albumid: {0}", - album_formatted, - ) - continue - - # Get the MusicBrainz album information. - album_info = hooks.album_for_mbid(a.mb_albumid) - if not album_info: + if not (album_info := hooks.album_for_id(a.mb_albumid)): self._log.info( "Release ID {0} not found for album {1}", a.mb_albumid, @@ -179,7 +155,7 @@ class MBSyncPlugin(BeetsPlugin): with lib.transaction(): autotag.apply_metadata(album_info, mapping) changed = False - # Find any changed item to apply MusicBrainz changes to album. + # Find any changed item to apply changes to album. any_changed_item = items[0] for item in items: item_changed = ui.show_model_changes(item) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4dedabf33..32bd91390 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -22,6 +22,7 @@ New features: * :doc:`plugins/lyrics`: Rewrite lyrics translation functionality to use Azure AI Translator API and add relevant instructions to the documentation. * :doc:`plugins/missing`: Add support for all metadata sources. +* :doc:`plugins/mbsync`: Add support for all metadata sorces. Bug fixes: diff --git a/docs/plugins/mbsync.rst b/docs/plugins/mbsync.rst index 1c8663dca..647ff4df8 100644 --- a/docs/plugins/mbsync.rst +++ b/docs/plugins/mbsync.rst @@ -1,14 +1,13 @@ MBSync Plugin ============= -This plugin provides the ``mbsync`` command, which lets you fetch metadata -from MusicBrainz for albums and tracks that already have MusicBrainz IDs. This -is useful for updating tags as they are fixed in the MusicBrainz database, or -when you change your mind about some config options that change how tags are -written to files. If you have a music library that is already nicely tagged by -a program that also uses MusicBrainz like Picard, this can speed up the -initial import if you just import "as-is" and then use ``mbsync`` to get -up-to-date tags that are written to the files according to your beets +This plugin provides the ``mbsync`` command, which lets you synchronize +metadata for albums and tracks that have external data source IDs. + +This is useful for syncing your library with online data or when changing +configuration options that affect tag writing. If your music library already +contains correct tags, you can speed up the initial import by importing files +"as-is" and then using ``mbsync`` to write tags according to your beets configuration. diff --git a/test/plugins/test_mbsync.py b/test/plugins/test_mbsync.py index f65df4256..088165ef5 100644 --- a/test/plugins/test_mbsync.py +++ b/test/plugins/test_mbsync.py @@ -12,7 +12,7 @@ # The above copyright notice and this permission notice shall be # included in all copies or substantial portions of the Software. -from unittest.mock import patch +from unittest.mock import Mock, patch from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.library import Item @@ -22,37 +22,36 @@ from beets.test.helper import PluginTestCase, capture_log class MbsyncCliTest(PluginTestCase): plugin = "mbsync" - @patch("beets.autotag.mb.album_for_id") - @patch("beets.autotag.mb.track_for_id") - def test_update_library(self, track_for_id, album_for_id): + @patch( + "beets.plugins.album_for_id", + Mock( + side_effect=lambda *_: AlbumInfo( + album_id="album id", + album="new album", + tracks=[TrackInfo(track_id="track id", title="new title")], + ) + ), + ) + @patch( + "beets.plugins.track_for_id", + Mock( + side_effect=lambda *_: TrackInfo( + track_id="singleton id", title="new title" + ) + ), + ) + def test_update_library(self): album_item = Item( album="old album", - mb_albumid="81ae60d4-5b75-38df-903a-db2cfa51c2c6", + mb_albumid="album id", mb_trackid="track id", ) self.lib.add_album([album_item]) - singleton = Item( - title="old title", mb_trackid="b8c2cf90-83f9-3b5f-8ccd-31fb866fcf37" - ) + singleton = Item(title="old title", mb_trackid="singleton id") self.lib.add(singleton) - album_for_id.return_value = AlbumInfo( - album_id="album id", - album="new album", - tracks=[ - TrackInfo(track_id=album_item.mb_trackid, title="new title") - ], - ) - track_for_id.return_value = TrackInfo( - track_id=singleton.mb_trackid, title="new title" - ) - - with capture_log() as logs: - self.run_command("mbsync") - - assert "Sending event: albuminfo_received" in logs - assert "Sending event: trackinfo_received" in logs + self.run_command("mbsync") singleton.load() assert singleton.title == "new title" @@ -81,9 +80,6 @@ class MbsyncCliTest(PluginTestCase): with capture_log("beets.mbsync") as logs: self.run_command("mbsync", "-f", "'%if{$album,$album,$title}'") - assert set(logs) == { - "mbsync: Skipping album with no mb_albumid: 'no id'", - "mbsync: Skipping album with invalid mb_albumid: 'invalid id'", - "mbsync: Skipping singleton with no mb_trackid: 'no id'", - "mbsync: Skipping singleton with invalid mb_trackid: 'invalid id'", - } + + assert "mbsync: Skipping album with no mb_albumid: 'no id'" in logs + assert "mbsync: Skipping singleton with no mb_trackid: 'no id'" in logs From 8f209d85b9874999906f83fe390ebab960353312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 14 Apr 2025 02:58:58 +0100 Subject: [PATCH 07/11] Tidy up mbsync logs No need to call `format` since this is done automatically at the point the object is logged (if required). --- beetsplug/mbsync.py | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index 5a17473c5..2e62b7b7e 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -74,18 +74,15 @@ class MBSyncPlugin(BeetsPlugin): query. """ for item in lib.items(query + ["singleton:true"]): - item_formatted = format(item) if not item.mb_trackid: self._log.info( - "Skipping singleton with no mb_trackid: {0}", item_formatted + "Skipping singleton with no mb_trackid: {}", item ) continue if not (track_info := hooks.track_for_id(item.mb_trackid)): self._log.info( - "Recording ID not found: {0} for track {0}", - item.mb_trackid, - item_formatted, + "Recording ID not found: {0.mb_trackid} for track {0}", item ) continue @@ -99,20 +96,14 @@ class MBSyncPlugin(BeetsPlugin): query and their items. """ # Process matching albums. - for a in lib.albums(query): - album_formatted = format(a) - if not a.mb_albumid: - self._log.info( - "Skipping album with no mb_albumid: {0}", album_formatted - ) + for album in lib.albums(query): + if not album.mb_albumid: + self._log.info("Skipping album with no mb_albumid: {}", album) continue - items = list(a.items()) - if not (album_info := hooks.album_for_id(a.mb_albumid)): + if not (album_info := hooks.album_for_id(album.mb_albumid)): self._log.info( - "Release ID {0} not found for album {1}", - a.mb_albumid, - album_formatted, + "Release ID {0.mb_albumid} not found for album {0}", album ) continue @@ -129,6 +120,7 @@ class MBSyncPlugin(BeetsPlugin): # first, if available, and recording MBIDs otherwise). This should # work for albums that have missing or extra tracks. mapping = {} + items = list(album.items()) for item in items: if ( item.mb_releasetrackid @@ -151,7 +143,7 @@ class MBSyncPlugin(BeetsPlugin): break # Apply. - self._log.debug("applying changes to {}", album_formatted) + self._log.debug("applying changes to {}", album) with lib.transaction(): autotag.apply_metadata(album_info, mapping) changed = False @@ -171,10 +163,10 @@ class MBSyncPlugin(BeetsPlugin): if not pretend: # Update album structure to reflect an item in it. for key in library.Album.item_keys: - a[key] = any_changed_item[key] - a.store() + album[key] = any_changed_item[key] + album.store() # Move album art (and any inconsistent items). if move and lib.directory in util.ancestry(items[0].path): - self._log.debug("moving album {0}", album_formatted) - a.move() + self._log.debug("moving album {}", album) + album.move() From 447cc82e041739bc1210c0d07af5ea64e44c2e99 Mon Sep 17 00:00:00 2001 From: Peter Dolan Date: Mon, 14 Apr 2025 11:22:41 -0700 Subject: [PATCH 08/11] Do not write unchanged items to the library in FtInTitle (#5718) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FtInTitle performs a library store operation for every item it processes, whether or not the item has changed. By limiting the `item.store()` call to only those cases when the item has changed, the plugin’s performance when processing an entire library improves by two to three orders of magnitude. --- beetsplug/ftintitle.py | 19 ++++++++++++------- docs/changelog.rst | 2 ++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index e4f51fd10..3f01af593 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -111,10 +111,10 @@ class FtInTitlePlugin(plugins.BeetsPlugin): write = ui.should_write() for item in lib.items(ui.decargs(args)): - self.ft_in_title(item, drop_feat, keep_in_artist_field) - item.store() - if write: - item.try_write() + if self.ft_in_title(item, drop_feat, keep_in_artist_field): + item.store() + if write: + item.try_write() self._command.func = func return [self._command] @@ -125,8 +125,8 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field = self.config["keep_in_artist"].get(bool) for item in task.imported_items(): - self.ft_in_title(item, drop_feat, keep_in_artist_field) - item.store() + if self.ft_in_title(item, drop_feat, keep_in_artist_field): + item.store() def update_metadata(self, item, feat_part, drop_feat, keep_in_artist_field): """Choose how to add new artists to the title and set the new @@ -156,9 +156,12 @@ class FtInTitlePlugin(plugins.BeetsPlugin): self._log.info("title: {0} -> {1}", item.title, new_title) item.title = new_title - def ft_in_title(self, item, drop_feat, keep_in_artist_field): + def ft_in_title(self, item, drop_feat, keep_in_artist_field) -> bool: """Look for featured artists in the item's artist fields and move them to the title. + + Returns: + True if the item has been modified. False otherwise. """ artist = item.artist.strip() albumartist = item.albumartist.strip() @@ -180,5 +183,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): self.update_metadata( item, feat_part, drop_feat, keep_in_artist_field ) + return True else: self._log.info("no featuring artists found") + return False diff --git a/docs/changelog.rst b/docs/changelog.rst index 32bd91390..ca2bb50e3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -100,6 +100,8 @@ Other changes: improve useability for new developers. * :doc:`/plugins/smartplaylist`: URL-encode additional item `fields` within generated EXTM3U playlists instead of JSON-encoding them. +* :doc:`plugins/ftintitle`: Optimize the plugin by avoiding unnecessary writes + to the database. 2.2.0 (December 02, 2024) ------------------------- From 8413de1a85a47b0e75acc162e131a43f8eaf1054 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Mon, 14 Apr 2025 20:29:46 +0200 Subject: [PATCH 09/11] ftintitle: add typings --- beetsplug/ftintitle.py | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 3f01af593..5dd6fbf4c 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -14,13 +14,20 @@ """Moves "featured" artists to the title from the artist field.""" +from __future__ import annotations + import re +from typing import TYPE_CHECKING from beets import plugins, ui from beets.util import displayable_path +if TYPE_CHECKING: + from beets.importer import ImportSession, ImportTask + from beets.library import Item -def split_on_feat(artist): + +def split_on_feat(artist: str) -> tuple[str, str | None]: """Given an artist string, split the "main" artist from any artist on the right-hand side of a string like "feat". Return the main artist, which is always a string, and the featuring artist, which @@ -28,14 +35,15 @@ def split_on_feat(artist): """ # split on the first "feat". regex = re.compile(plugins.feat_tokens(), re.IGNORECASE) - parts = [s.strip() for s in regex.split(artist, 1)] + parts = tuple(s.strip() for s in regex.split(artist, 1)) if len(parts) == 1: return parts[0], None else: - return tuple(parts) + assert len(parts) == 2 # help mypy out + return parts -def contains_feat(title): +def contains_feat(title: str) -> bool: """Determine whether the title contains a "featured" marker.""" return bool( re.search( @@ -46,7 +54,7 @@ def contains_feat(title): ) -def find_feat_part(artist, albumartist): +def find_feat_part(artist: str, albumartist: str) -> str | None: """Attempt to find featured artists in the item's artist fields and return the results. Returns None if no featured artist found. """ @@ -75,7 +83,7 @@ def find_feat_part(artist, albumartist): class FtInTitlePlugin(plugins.BeetsPlugin): - def __init__(self): + def __init__(self) -> None: super().__init__() self.config.add( @@ -103,7 +111,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): if self.config["auto"]: self.import_stages = [self.imported] - def commands(self): + def commands(self) -> list[ui.Subcommand]: def func(lib, opts, args): self.config.set_args(opts) drop_feat = self.config["drop"].get(bool) @@ -119,7 +127,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): self._command.func = func return [self._command] - def imported(self, session, task): + def imported(self, session: ImportSession, task: ImportTask) -> None: """Import hook for moving featuring artist automatically.""" drop_feat = self.config["drop"].get(bool) keep_in_artist_field = self.config["keep_in_artist"].get(bool) @@ -128,7 +136,13 @@ class FtInTitlePlugin(plugins.BeetsPlugin): if self.ft_in_title(item, drop_feat, keep_in_artist_field): item.store() - def update_metadata(self, item, feat_part, drop_feat, keep_in_artist_field): + def update_metadata( + self, + item: Item, + feat_part: str, + drop_feat: bool, + keep_in_artist_field: bool, + ) -> None: """Choose how to add new artists to the title and set the new metadata. Also, print out messages about any changes that are made. If `drop_feat` is set, then do not add the artist to the title; just @@ -156,7 +170,12 @@ class FtInTitlePlugin(plugins.BeetsPlugin): self._log.info("title: {0} -> {1}", item.title, new_title) item.title = new_title - def ft_in_title(self, item, drop_feat, keep_in_artist_field) -> bool: + def ft_in_title( + self, + item: Item, + drop_feat: bool, + keep_in_artist_field: bool, + ) -> bool: """Look for featured artists in the item's artist fields and move them to the title. From b49528612742dc1637236979a58e370eb036019b Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Mon, 14 Apr 2025 20:34:19 +0200 Subject: [PATCH 10/11] ftintitle: flatten code linear code with early exits instead of more complicated nested conditionals --- beetsplug/ftintitle.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 5dd6fbf4c..a85aa9719 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -188,21 +188,22 @@ class FtInTitlePlugin(plugins.BeetsPlugin): # Check whether there is a featured artist on this track and the # artist field does not exactly match the album artist field. In # that case, we attempt to move the featured artist to the title. + if not albumartist or albumartist == artist: + return False + _, featured = split_on_feat(artist) - if featured and albumartist != artist and albumartist: - self._log.info("{}", displayable_path(item.path)) + if not featured: + return False - feat_part = None + self._log.info("{}", displayable_path(item.path)) - # Attempt to find the featured artist. - feat_part = find_feat_part(artist, albumartist) + # Attempt to find the featured artist. + feat_part = find_feat_part(artist, albumartist) - # If we have a featuring artist, move it to the title. - if feat_part: - self.update_metadata( - item, feat_part, drop_feat, keep_in_artist_field - ) - return True - else: - self._log.info("no featuring artists found") - return False + if not feat_part: + self._log.info("no featuring artists found") + return False + + # If we have a featuring artist, move it to the title. + self.update_metadata(item, feat_part, drop_feat, keep_in_artist_field) + return True From b4a634a443e08bba67430720ffbd8fb50e3e62f7 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr <39738318+semohr@users.noreply.github.com> Date: Tue, 15 Apr 2025 11:43:36 +0200 Subject: [PATCH 11/11] Allow to pickle db models by removing the current connection. (#5641) ## Description This might be a quick one, depending on how you feel about it... It allows you to pickle DB model objects. I don't think this is used directly in Beets, but it might be useful in general. For instance, we encountered an issue where we wanted to quickly pickle an Item or Album. This sometimes worked and other times failed, which seemed quite inconsistent. Some DB model methods and properties have the side effect of attaching an SQLite connection to self (._db), which prevents serialization. The fix is quite straightforward, so I thought we might want to integrate this into beets directly. ## To Do - [x] Changelog - [x] Tests --- beets/dbcore/db.py | 9 +++++++++ docs/changelog.rst | 1 + test/test_dbcore.py | 14 ++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 2aa0081d7..dd8401935 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -716,6 +716,15 @@ class Model(ABC, Generic[D]): """Set the object's key to a value represented by a string.""" self[key] = self._parse(key, string) + def __getstate__(self): + """Return the state of the object for pickling. + Remove the database connection as sqlite connections are not + picklable. + """ + state = self.__dict__.copy() + state["_db"] = None + return state + # Database controller and supporting interfaces. diff --git a/docs/changelog.rst b/docs/changelog.rst index ca2bb50e3..77d237e6d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -102,6 +102,7 @@ Other changes: EXTM3U playlists instead of JSON-encoding them. * :doc:`plugins/ftintitle`: Optimize the plugin by avoiding unnecessary writes to the database. +* Database models are now serializable with pickle. 2.2.0 (December 02, 2024) ------------------------- diff --git a/test/test_dbcore.py b/test/test_dbcore.py index 2ff20c3a3..a4bae97c9 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -421,6 +421,20 @@ class ModelTest(unittest.TestCase): with pytest.raises(TypeError, match="must be a string"): dbcore.Model._parse(None, 42) + def test_pickle_dump(self): + """Tries to pickle an item. This tests the __getstate__ method + of the Model ABC""" + import pickle + + model = ModelFixture1(self.db) + model.add(self.db) + model.field_one = 123 + + model.store() + assert model._db is not None + + pickle.dumps(model) + class FormatTest(unittest.TestCase): def test_format_fixed_field_integer(self):