diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index fbe32b497..2ee64a97d 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -70,4 +70,6 @@ d93ddf8dd43e4f9ed072a03829e287c78d2570a2 # Moved dev docs 07549ed896d9649562d40b75cd30702e6fa6e975 # Moved plugin docs Further Reading chapter -33f1a5d0bef8ca08be79ee7a0d02a018d502680d \ No newline at end of file +33f1a5d0bef8ca08be79ee7a0d02a018d502680d +# Moved art.py utility module from beets into beetsplug +28aee0fde463f1e18dfdba1994e2bdb80833722f \ No newline at end of file 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/dbcore/db.py b/beets/dbcore/db.py index 8cd89111e..192cfac70 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -17,15 +17,17 @@ from __future__ import annotations import contextlib +import functools import os import re import sqlite3 +import sys import threading import time from abc import ABC from collections import defaultdict from collections.abc import Generator, Iterable, Iterator, Mapping, Sequence -from sqlite3 import Connection +from sqlite3 import Connection, sqlite_version_info from typing import TYPE_CHECKING, Any, AnyStr, Callable, Generic from typing_extensions import TypeVar # default value support @@ -64,6 +66,16 @@ class DBAccessError(Exception): """ +class DBCustomFunctionError(Exception): + """A sqlite function registered by beets failed.""" + + def __init__(self): + super().__init__( + "beets defined SQLite function failed; " + "see the other errors above for details" + ) + + class FormattedMapping(Mapping[str, str]): """A `dict`-like formatted view of a model. @@ -947,6 +959,12 @@ class Transaction: self._mutated = False self.db._db_lock.release() + if ( + isinstance(exc_value, sqlite3.OperationalError) + and exc_value.args[0] == "user-defined function raised exception" + ): + raise DBCustomFunctionError() + def query( self, statement: str, subvals: Sequence[SQLiteType] = () ) -> list[sqlite3.Row]: @@ -1007,6 +1025,13 @@ class Database: "sqlite3 must be compiled with multi-threading support" ) + # Print tracebacks for exceptions in user defined functions + # See also `self.add_functions` and `DBCustomFunctionError`. + # + # `if`: use feature detection because PyPy doesn't support this. + if hasattr(sqlite3, "enable_callback_tracebacks"): + sqlite3.enable_callback_tracebacks(True) + self.path = path self.timeout = timeout @@ -1102,9 +1127,16 @@ class Database: return bytestring - conn.create_function("regexp", 2, regexp) - conn.create_function("unidecode", 1, unidecode) - conn.create_function("bytelower", 1, bytelower) + create_function = conn.create_function + if sys.version_info >= (3, 8) and sqlite_version_info >= (3, 8, 3): + # Let sqlite make extra optimizations + create_function = functools.partial( + conn.create_function, deterministic=True + ) + + create_function("regexp", 2, regexp) + create_function("unidecode", 1, unidecode) + create_function("bytelower", 1, bytelower) def _close(self): """Close the all connections to the underlying SQLite database 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/metadata_plugins.py b/beets/metadata_plugins.py index 381881b51..56bf8124f 100644 --- a/beets/metadata_plugins.py +++ b/beets/metadata_plugins.py @@ -271,10 +271,9 @@ class MetadataSourcePlugin(BeetsPlugin, metaclass=abc.ABCMeta): """Returns an artist string (all artists) and an artist_id (the main artist) for a list of artist object dicts. - For each artist, this function moves articles (such as 'a', 'an', - and 'the') to the front and strips trailing disambiguation numbers. It - returns a tuple containing the comma-separated string of all - normalized artists and the ``id`` of the main/first artist. + For each artist, this function moves articles (such as 'a', 'an', and 'the') + to the front. It returns a tuple containing the comma-separated string + of all normalized artists and the ``id`` of the main/first artist. Alternatively a keyword can be used to combine artists together into a single string by passing the join_key argument. @@ -298,8 +297,6 @@ class MetadataSourcePlugin(BeetsPlugin, metaclass=abc.ABCMeta): if not artist_id: artist_id = artist[id_key] name = artist[name_key] - # Strip disambiguation number. - name = re.sub(r" \(\d+\)$", "", name) # Move articles to the front. name = re.sub(r"^(.*?), (a|an|the)$", r"\2 \1", name, flags=re.I) # Use a join keyword if requested and available. @@ -371,7 +368,9 @@ class SearchApiMetadataSourcePlugin( album: str, va_likely: bool, ) -> Iterable[AlbumInfo]: - query_filters: SearchFilter = {"album": album} + query_filters: SearchFilter = {} + if album: + query_filters["album"] = album if not va_likely: query_filters["artist"] = artist @@ -413,7 +412,7 @@ class SearchApiMetadataSourcePlugin( :return: Query string to be provided to the search API. """ - components = [query_string, *(f'{k}:"{v}"' for k, v in filters.items())] + components = [query_string, *(f"{k}:'{v}'" for k, v in filters.items())] query = " ".join(filter(None, components)) if self.config["search_query_ascii"].get(): 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/_utils/__init__.py b/beetsplug/_utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/beets/art.py b/beetsplug/_utils/art.py similarity index 100% rename from beets/art.py rename to beetsplug/_utils/art.py diff --git a/beetsplug/convert.py b/beetsplug/convert.py index e9db3592e..e72f8c75a 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -25,12 +25,13 @@ from string import Template import mediafile from confuse import ConfigTypeError, Optional -from beets import art, config, plugins, ui, util +from beets import config, plugins, ui, util from beets.library import Item, parse_query_string from beets.plugins import BeetsPlugin from beets.util import par_map from beets.util.artresizer import ArtResizer from beets.util.m3u import M3UFile +from beetsplug._utils import art _fs_lock = threading.Lock() _temp_files = [] # Keep track of temporary transcoded files for deletion. @@ -121,6 +122,7 @@ class ConvertPlugin(BeetsPlugin): "threads": os.cpu_count(), "format": "mp3", "id3v23": "inherit", + "write_metadata": True, "formats": { "aac": { "command": ( @@ -445,8 +447,9 @@ class ConvertPlugin(BeetsPlugin): if id3v23 == "inherit": id3v23 = None - # Write tags from the database to the converted file. - item.try_write(path=converted, id3v23=id3v23) + # Write tags from the database to the file if requested + if self.config["write_metadata"].get(bool): + item.try_write(path=converted, id3v23=id3v23) if keep_new: # If we're keeping the transcoded file, read it again (after diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index c1c782f3e..3dc962464 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -76,6 +76,8 @@ TRACK_INDEX_RE = re.compile( re.VERBOSE, ) +DISAMBIGUATION_RE = re.compile(r" \(\d+\)") + class ReleaseFormat(TypedDict): name: str @@ -96,6 +98,7 @@ class DiscogsPlugin(MetadataSourcePlugin): "separator": ", ", "index_tracks": False, "append_style_genre": False, + "strip_disambiguation": True, } ) self.config["apikey"].redact = True @@ -336,7 +339,7 @@ class DiscogsPlugin(MetadataSourcePlugin): # convenient `.tracklist` property, which will strip out useful artist # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. - tracks = self.get_tracks(result.data["tracklist"]) + tracks = self.get_tracks(result.data["tracklist"], artist, artist_id) # Extract information for the optional AlbumInfo fields, if possible. va = result.data["artists"][0].get("name", "").lower() == "various" @@ -362,15 +365,20 @@ class DiscogsPlugin(MetadataSourcePlugin): label = catalogno = labelid = None if result.data.get("labels"): - label = result.data["labels"][0].get("name") + label = self.strip_disambiguation( + result.data["labels"][0].get("name") + ) catalogno = result.data["labels"][0].get("catno") labelid = result.data["labels"][0].get("id") cover_art_url = self.select_cover_art(result) - # Additional cleanups (various artists name, catalog number, media). + # Additional cleanups + # (various artists name, catalog number, media, disambiguation). if va: artist = config["va_name"].as_str() + else: + artist = self.strip_disambiguation(artist) if catalogno == "none": catalogno = None # Explicitly set the `media` for the tracks, since it is expected by @@ -378,10 +386,6 @@ class DiscogsPlugin(MetadataSourcePlugin): for track in tracks: track.media = media track.medium_total = mediums.count(track.medium) - if not track.artist: # get_track_info often fails to find artist - track.artist = artist - if not track.artist_id: - track.artist_id = artist_id # Discogs does not have track IDs. Invent our own IDs as proposed # in #2336. track.track_id = f"{album_id}-{track.track_alt}" @@ -438,7 +442,7 @@ class DiscogsPlugin(MetadataSourcePlugin): else: return None - def get_tracks(self, tracklist): + def get_tracks(self, tracklist, album_artist, album_artist_id): """Returns a list of TrackInfo objects for a discogs tracklist.""" try: clean_tracklist = self.coalesce_tracks(tracklist) @@ -463,7 +467,9 @@ class DiscogsPlugin(MetadataSourcePlugin): # divisions. divisions += next_divisions del next_divisions[:] - track_info = self.get_track_info(track, index, divisions) + track_info = self.get_track_info( + track, index, divisions, album_artist, album_artist_id + ) track_info.track_alt = track["position"] tracks.append(track_info) else: @@ -622,7 +628,17 @@ class DiscogsPlugin(MetadataSourcePlugin): return tracklist - def get_track_info(self, track, index, divisions): + def strip_disambiguation(self, text: str) -> str: + """Removes discogs specific disambiguations from a string. + Turns 'Label Name (5)' to 'Label Name' or 'Artist (1) & Another Artist (2)' + to 'Artist & Another Artist'. Does nothing if strip_disambiguation is False.""" + if not self.config["strip_disambiguation"]: + return text + return DISAMBIGUATION_RE.sub("", text) + + def get_track_info( + self, track, index, divisions, album_artist, album_artist_id + ): """Returns a TrackInfo object for a discogs track.""" title = track["title"] if self.config["index_tracks"]: @@ -634,7 +650,21 @@ class DiscogsPlugin(MetadataSourcePlugin): artist, artist_id = self.get_artist( track.get("artists", []), join_key="join" ) + # If no artist and artist is returned, set to match album artist + if not artist: + artist = album_artist + artist_id = album_artist_id length = self.get_track_length(track["duration"]) + # Add featured artists + extraartists = track.get("extraartists", []) + featured = [ + artist["name"] + for artist in extraartists + if "Featuring" in artist["role"] + ] + if featured: + artist = f"{artist} feat. {', '.join(featured)}" + artist = self.strip_disambiguation(artist) return TrackInfo( title=title, track_id=track_id, diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 1a59e4f9c..cbf40f570 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -20,11 +20,12 @@ from mimetypes import guess_extension import requests -from beets import art, config, ui +from beets import config, ui from beets.plugins import BeetsPlugin from beets.ui import print_ from beets.util import bytestring_path, displayable_path, normpath, syspath from beets.util.artresizer import ArtResizer +from beetsplug._utils import art def _confirm(objs, album): 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/ftintitle.py b/beetsplug/ftintitle.py index f0f088099..e17d7bc1c 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -26,14 +26,16 @@ if TYPE_CHECKING: from beets.library import Item -def split_on_feat(artist: str) -> tuple[str, str | None]: +def split_on_feat( + artist: str, for_artist: bool = True +) -> 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 may be a string or None if none is present. """ # split on the first "feat". - regex = re.compile(plugins.feat_tokens(), re.IGNORECASE) + regex = re.compile(plugins.feat_tokens(for_artist), re.IGNORECASE) parts = tuple(s.strip() for s in regex.split(artist, 1)) if len(parts) == 1: return parts[0], None @@ -53,32 +55,35 @@ def contains_feat(title: str) -> bool: ) -def find_feat_part(artist: str, albumartist: str) -> str | None: +def find_feat_part(artist: str, albumartist: str | None) -> str | None: """Attempt to find featured artists in the item's artist fields and return the results. Returns None if no featured artist found. """ - # Look for the album artist in the artist field. If it's not - # present, give up. - albumartist_split = artist.split(albumartist, 1) - if len(albumartist_split) <= 1: - return None + # Handle a wider variety of extraction cases if the album artist is + # contained within the track artist. + if albumartist and albumartist in artist: + albumartist_split = artist.split(albumartist, 1) - # If the last element of the split (the right-hand side of the - # album artist) is nonempty, then it probably contains the - # featured artist. - elif albumartist_split[1] != "": - # Extract the featured artist from the right-hand side. - _, feat_part = split_on_feat(albumartist_split[1]) - return feat_part + # If the last element of the split (the right-hand side of the + # album artist) is nonempty, then it probably contains the + # featured artist. + if albumartist_split[1] != "": + # Extract the featured artist from the right-hand side. + _, feat_part = split_on_feat(albumartist_split[1]) + return feat_part - # Otherwise, if there's nothing on the right-hand side, look for a - # featuring artist on the left-hand side. - else: - lhs, rhs = split_on_feat(albumartist_split[0]) - if lhs: - return lhs + # Otherwise, if there's nothing on the right-hand side, + # look for a featuring artist on the left-hand side. + else: + lhs, _ = split_on_feat(albumartist_split[0]) + if lhs: + return lhs - return None + # Fall back to conservative handling of the track artist without relying + # on albumartist, which covers compilations using a 'Various Artists' + # albumartist and album tracks by a guest artist featuring a third artist. + _, feat_part = split_on_feat(artist, False) + return feat_part class FtInTitlePlugin(plugins.BeetsPlugin): @@ -153,8 +158,9 @@ class FtInTitlePlugin(plugins.BeetsPlugin): "artist: {.artist} (Not changing due to keep_in_artist)", item ) else: - self._log.info("artist: {0.artist} -> {0.albumartist}", item) - item.artist = item.albumartist + track_artist, _ = split_on_feat(item.artist) + self._log.info("artist: {0.artist} -> {1}", item, track_artist) + item.artist = track_artist if item.artist_sort: # Just strip the featured artist from the sort name. @@ -187,7 +193,7 @@ 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: + if albumartist and artist == albumartist: return False _, featured = split_on_feat(artist) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 8c09eefea..1da5ecde4 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -461,6 +461,12 @@ class LastGenrePlugin(plugins.BeetsPlugin): def commands(self): lastgenre_cmd = ui.Subcommand("lastgenre", help="fetch genres") + lastgenre_cmd.parser.add_option( + "-p", + "--pretend", + action="store_true", + help="show actions but do nothing", + ) lastgenre_cmd.parser.add_option( "-f", "--force", @@ -521,45 +527,64 @@ class LastGenrePlugin(plugins.BeetsPlugin): def lastgenre_func(lib, opts, args): write = ui.should_write() + pretend = getattr(opts, "pretend", False) self.config.set_args(opts) if opts.album: # Fetch genres for whole albums for album in lib.albums(args): - album.genre, src = self._get_genre(album) + album_genre, src = self._get_genre(album) + prefix = "Pretend: " if pretend else "" self._log.info( - 'genre for album "{0.album}" ({1}): {0.genre}', + '{}genre for album "{.album}" ({}): {}', + prefix, album, src, + album_genre, ) - if "track" in self.sources: - album.store(inherit=False) - else: - album.store() + if not pretend: + album.genre = album_genre + if "track" in self.sources: + album.store(inherit=False) + else: + album.store() for item in album.items(): # If we're using track-level sources, also look up each # track on the album. if "track" in self.sources: - item.genre, src = self._get_genre(item) - item.store() + item_genre, src = self._get_genre(item) self._log.info( - 'genre for track "{0.title}" ({1}): {0.genre}', + '{}genre for track "{.title}" ({}): {}', + prefix, item, src, + item_genre, ) + if not pretend: + item.genre = item_genre + item.store() - if write: + if write and not pretend: item.try_write() else: # Just query singletons, i.e. items that are not part of # an album for item in lib.items(args): - item.genre, src = self._get_genre(item) - item.store() + item_genre, src = self._get_genre(item) + prefix = "Pretend: " if pretend else "" self._log.info( - "genre for track {0.title} ({1}): {0.genre}", item, src + '{}genre for track "{0.title}" ({1}): {}', + prefix, + item, + src, + item_genre, ) + if not pretend: + item.genre = item_genre + item.store() + if write and not pretend: + item.try_write() lastgenre_cmd.func = lastgenre_func return [lastgenre_cmd] 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/beetsplug/spotify.py b/beetsplug/spotify.py index a0a5c4358..0f6e0012b 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -130,9 +130,6 @@ class SpotifyPlugin( "mode": "list", "tiebreak": "popularity", "show_failures": False, - "artist_field": "albumartist", - "album_field": "album", - "track_field": "title", "region_filter": None, "regex": [], "client_id": "4e414367a1d14c75a5c5129a627fcab8", @@ -563,13 +560,17 @@ class SpotifyPlugin( regex["search"], regex["replace"], value ) - # Custom values can be passed in the config (just in case) - artist = item[self.config["artist_field"].get()] - album = item[self.config["album_field"].get()] - query_string = item[self.config["track_field"].get()] + artist = item["artist"] or item["albumartist"] + album = item["album"] + query_string = item["title"] # Query the Web API for each track, look for the items' JSON data - query_filters: SearchFilter = {"artist": artist, "album": album} + query_filters: SearchFilter = {} + if artist: + query_filters["artist"] = artist + if album: + query_filters["album"] = album + response_data_tracks = self._search_api( query_type="track", query_string=query_string, diff --git a/docs/changelog.rst b/docs/changelog.rst index d01148286..825c09752 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,10 +9,33 @@ Unreleased New features: +- :doc:`plugins/lastgenre`: Add a ``--pretend`` option to preview genre changes + without storing or writing them. +- :doc:`plugins/convert`: Add a config option to disable writing metadata to + converted files. +- :doc:`plugins/discogs`: New config option `strip_disambiguation` to toggle + stripping discogs numeric disambiguation on artist and label fields. +- :doc:`plugins/discogs` Added support for featured artists. + Bug fixes: - :doc:`/plugins/fromfilename`: Fix :bug:`5218`, improve the code (refactor regexps, allow for more cases, add some logging), add tests. +- :doc:`plugins/spotify` Fixed an issue where track matching and lookups could + return incorrect or misleading results when using the Spotify plugin. The + problem occurred primarily when no album was provided or when the album field + was an empty string. :bug:`5189` +- :doc:`plugins/spotify` Removed old and undocumented config options + `artist_field`, `album_field` and `track` that were causing issues with track + matching. :bug:`5189` +- :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from + artists but not labels. :bug:`5366` +- :doc:`plugins/discogs` Fixed issue with ignoring featured artists in the + extraartists field. +- :doc:`plugins/spotify` Fixed an issue where candidate lookup would not find + matches due to query escaping (single vs double quotes). +- :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: @@ -24,6 +47,17 @@ Other changes: match :bug:`6020` - :doc:`guides/tagger`: Section on no matching release found, related to possibly disabled musicbrainz plugin :bug:`6020` +- Moved ``art.py`` utility module from ``beets`` into ``beetsplug`` namespace as + it is not used in the core beets codebase. It can now be found in + ``beetsplug._utils``. +- :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) -------------------------- @@ -150,6 +184,9 @@ Other changes: Autogenerated API references are now located in the ``docs/api`` subdirectory. - :doc:`/plugins/substitute`: Fix rST formatting for example cases so that each case is shown on separate lines. +- :doc:`/plugins/ftintitle`: Process items whose albumartist is not contained in + the artist field, including compilations using Various Artists as an + albumartist and album tracks by guest artists featuring a third artist. - Refactored library.py file by splitting it into multiple modules within the beets/library directory. - Added a test to check that all plugins can be imported without errors. diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst index 8917422c5..ecf60a85b 100644 --- a/docs/plugins/convert.rst +++ b/docs/plugins/convert.rst @@ -97,6 +97,8 @@ The available options are: - **embed**: Embed album art in converted items. Default: ``yes``. - **id3v23**: Can be used to override the global ``id3v23`` option. Default: ``inherit``. +- **write_metadata**: Can be used to disable writing metadata to converted + files. Default: ``true``. - **max_bitrate**: By default, the plugin does not transcode files that are already in the destination format. This option instead also transcodes files with high bitrates, even if they are already in the same format as the output. diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 44c0c0e22..3dac558bd 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -109,6 +109,9 @@ Other configurations available under ``discogs:`` are: - **search_limit**: The maximum number of results to return from Discogs. This is useful if you want to limit the number of results returned to speed up searches. Default: ``5`` +- **strip_disambiguation**: Discogs uses strings like ``"(4)"`` to mark distinct + artists and labels with the same name. If you'd like to use the discogs + disambiguation in your tags, you can disable it. Default: ``True`` .. _discogs guidelines: https://support.discogs.com/hc/en-us/articles/360005055373-Database-Guidelines-12-Tracklisting#Index_Tracks_And_Headings diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index 5ebe2d721..230694b06 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -124,7 +124,7 @@ tags** and will only **fetch new genres for empty tags**. When ``force`` is ``yes`` the setting of the ``whitelist`` option (as documented in Usage_) applies to any existing or newly fetched genres. -The follwing configurations are possible: +The following configurations are possible: **Setup 1** (default) @@ -213,5 +213,9 @@ fetch genres for albums or items matching a certain query. By default, ``beet lastgenre`` matches albums. To match individual tracks or singletons, use the ``-A`` switch: ``beet lastgenre -A [QUERY]``. +To preview the changes that would be made without applying them, use the ``-p`` +or ``--pretend`` flag. This shows which genres would be set but does not write +or store any changes. + To disable automatic genre fetching on import, set the ``auto`` config option to false. diff --git a/docs/plugins/missing.rst b/docs/plugins/missing.rst index 7764f5fe1..10842933c 100644 --- a/docs/plugins/missing.rst +++ b/docs/plugins/missing.rst @@ -39,21 +39,27 @@ Configuration To configure the plugin, make a ``missing:`` section in your configuration file. The available options are: -- **count**: Print a count of missing tracks per album, with ``format`` - defaulting to ``$albumartist - $album: $missing``. Default: ``no``. -- **format**: A specific format with which to print every track. This uses the - same template syntax as beets' :doc:`path formats `. - The usage is inspired by, and therefore similar to, the :ref:`list ` - command. Default: :ref:`format_item`. +- **count**: Print a count of missing tracks per album, with the global + ``format_album`` used for formatting. Default: ``no``. - **total**: Print a single count of missing tracks in all albums. Default: ``no``. +Formatting +~~~~~~~~~~ + +- This plugin uses global formatting options from the main configuration; see + :ref:`format_item` and :ref:`format_album`: +- :ref:`format_item`: Used when listing missing tracks (default item format). +- :ref:`format_album`: Used when showing counts (``-c``) or missing albums + (``-a``). + Here's an example :: + format_album: $albumartist - $album + format_item: $artist - $album - $title missing: - format: $albumartist - $album - $title count: no total: no diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index e3e51042c..3652c0a54 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -374,6 +374,129 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.genre == "GENRE1, GENRE2" assert d.style is None + def test_strip_disambiguation(self): + """Test removing disambiguation from all disambiguated fields.""" + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [ + { + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [ + {"name": "TEST ARTIST (5)", "tracks": "", "id": 11146} + ], + } + ], + "artists": [ + {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, + {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + ], + "title": "title", + "labels": [ + { + "name": "LABEL NAME (5)", + "catno": "catalog number", + } + ], + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + d = DiscogsPlugin().get_album_info(release) + assert d.artist == "ARTIST NAME & OTHER ARTIST" + assert d.tracks[0].artist == "TEST ARTIST" + assert d.label == "LABEL NAME" + + def test_strip_disambiguation_false(self): + """Test disabling disambiguation removal from all disambiguated fields.""" + config["discogs"]["strip_disambiguation"] = False + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [ + { + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [ + {"name": "TEST ARTIST (5)", "tracks": "", "id": 11146} + ], + } + ], + "artists": [ + {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, + {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + ], + "title": "title", + "labels": [ + { + "name": "LABEL NAME (5)", + "catno": "catalog number", + } + ], + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + d = DiscogsPlugin().get_album_info(release) + assert d.artist == "ARTIST NAME (2) & OTHER ARTIST (5)" + assert d.tracks[0].artist == "TEST ARTIST (5)" + assert d.label == "LABEL NAME (5)" + config["discogs"]["strip_disambiguation"] = True + + +@pytest.mark.parametrize( + "track, expected_artist", + [ + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "artists": [ + {"name": "NEW ARTIST", "tracks": "", "id": 11146}, + {"name": "VOCALIST", "tracks": "", "id": 344, "join": "&"}, + ], + "extraartists": [ + { + "name": "SOLOIST", + "role": "Featuring", + }, + { + "name": "PERFORMER (1)", + "role": "Other Role, Featuring", + }, + { + "name": "RANDOM", + "role": "Written-By", + }, + { + "name": "MUSICIAN", + "role": "Featuring [Uncredited]", + }, + ], + }, + "NEW ARTIST, VOCALIST feat. SOLOIST, PERFORMER, MUSICIAN", + ), + ], +) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +def test_parse_featured_artists(track, expected_artist): + """Tests the plugins ability to parse a featured artist. + Initial check with one featured artist, two featured artists, + and three. Ignores artists that are not listed as featured.""" + t = DiscogsPlugin().get_track_info(track, 1, 1, "ARTIST", 2) + assert t.artist == expected_artist + @pytest.mark.parametrize( "formats, expected_media, expected_albumtype", diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index 58d2a5f63..d40025374 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -23,7 +23,7 @@ from unittest.mock import MagicMock, patch import pytest from mediafile import MediaFile -from beets import art, config, logging, ui +from beets import config, logging, ui from beets.test import _common from beets.test.helper import ( BeetsTestCase, @@ -33,6 +33,7 @@ from beets.test.helper import ( ) from beets.util import bytestring_path, displayable_path, syspath from beets.util.artresizer import ArtResizer +from beetsplug._utils import art from test.test_art_resize import DummyIMBackend @@ -283,7 +284,7 @@ class DummyArtResizer(ArtResizer): @patch("beets.util.artresizer.subprocess") -@patch("beets.art.extract") +@patch("beetsplug._utils.art.extract") class ArtSimilarityTest(unittest.TestCase): def setUp(self): self.item = _common.item() diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index 572431b45..005318b11 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -14,8 +14,11 @@ """Tests for the 'ftintitle' plugin.""" -import unittest +from typing import Dict, Generator, Optional, Tuple, Union +import pytest + +from beets.library.models import Item from beets.test.helper import PluginTestCase from beetsplug import ftintitle @@ -23,169 +26,233 @@ from beetsplug import ftintitle class FtInTitlePluginFunctional(PluginTestCase): plugin = "ftintitle" - def _ft_add_item(self, path, artist, title, aartist): - return self.add_item( - path=path, - artist=artist, - artist_sort=artist, - title=title, - albumartist=aartist, - ) - def _ft_set_config( - self, ftformat, drop=False, auto=True, keep_in_artist=False - ): - self.config["ftintitle"]["format"] = ftformat - self.config["ftintitle"]["drop"] = drop - self.config["ftintitle"]["auto"] = auto - self.config["ftintitle"]["keep_in_artist"] = keep_in_artist - - def test_functional_drop(self): - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice") - self.run_command("ftintitle", "-d") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1" - - def test_functional_not_found(self): - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "George") - self.run_command("ftintitle", "-d") - item.load() - # item should be unchanged - assert item["artist"] == "Alice ft Bob" - assert item["title"] == "Song 1" - - def test_functional_custom_format(self): - self._ft_set_config("feat. {}") - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1 feat. Bob" - - self._ft_set_config("featuring {}") - item = self._ft_add_item("/", "Alice feat. Bob", "Song 1", "Alice") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1 featuring Bob" - - self._ft_set_config("with {}") - item = self._ft_add_item("/", "Alice feat Bob", "Song 1", "Alice") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1 with Bob" - - def test_functional_keep_in_artist(self): - self._ft_set_config("feat. {}", keep_in_artist=True) - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice ft Bob" - assert item["title"] == "Song 1 feat. Bob" - - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice") - self.run_command("ftintitle", "-d") - item.load() - assert item["artist"] == "Alice ft Bob" - assert item["title"] == "Song 1" +@pytest.fixture +def env() -> Generator[FtInTitlePluginFunctional, None, None]: + case = FtInTitlePluginFunctional(methodName="runTest") + case.setUp() + try: + yield case + finally: + case.tearDown() -class FtInTitlePluginTest(unittest.TestCase): - def setUp(self): - """Set up configuration""" - ftintitle.FtInTitlePlugin() +def set_config( + env: FtInTitlePluginFunctional, cfg: Optional[Dict[str, Union[str, bool]]] +) -> None: + cfg = {} if cfg is None else cfg + defaults = { + "drop": False, + "auto": True, + "keep_in_artist": False, + } + env.config["ftintitle"].set(defaults) + env.config["ftintitle"].set(cfg) - def test_find_feat_part(self): - test_cases = [ - { - "artist": "Alice ft. Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice feat Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice featuring Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice & Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice and Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice With Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice defeat Bob", - "album_artist": "Alice", - "feat_part": None, - }, - { - "artist": "Alice & Bob", - "album_artist": "Bob", - "feat_part": "Alice", - }, - { - "artist": "Alice ft. Bob", - "album_artist": "Bob", - "feat_part": "Alice", - }, - { - "artist": "Alice ft. Carol", - "album_artist": "Bob", - "feat_part": None, - }, - ] - for test_case in test_cases: - feat_part = ftintitle.find_feat_part( - test_case["artist"], test_case["album_artist"] - ) - assert feat_part == test_case["feat_part"] +def add_item( + env: FtInTitlePluginFunctional, + path: str, + artist: str, + title: str, + albumartist: Optional[str], +) -> Item: + return env.add_item( + path=path, + artist=artist, + artist_sort=artist, + title=title, + albumartist=albumartist, + ) - def test_split_on_feat(self): - parts = ftintitle.split_on_feat("Alice ft. Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice feat Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice feat. Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice featuring Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice & Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice and Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice With Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice defeat Bob") - assert parts == ("Alice defeat Bob", None) - def test_contains_feat(self): - assert ftintitle.contains_feat("Alice ft. Bob") - assert ftintitle.contains_feat("Alice feat. Bob") - assert ftintitle.contains_feat("Alice feat Bob") - assert ftintitle.contains_feat("Alice featuring Bob") - assert ftintitle.contains_feat("Alice (ft. Bob)") - assert ftintitle.contains_feat("Alice (feat. Bob)") - assert ftintitle.contains_feat("Alice [ft. Bob]") - assert ftintitle.contains_feat("Alice [feat. Bob]") - assert not ftintitle.contains_feat("Alice defeat Bob") - assert not ftintitle.contains_feat("Aliceft.Bob") - assert not ftintitle.contains_feat("Alice (defeat Bob)") - assert not ftintitle.contains_feat("Live and Let Go") - assert not ftintitle.contains_feat("Come With Me") +@pytest.mark.parametrize( + "cfg, cmd_args, given, expected", + [ + pytest.param( + None, + ("ftintitle",), + ("Alice", "Song 1", "Alice"), + ("Alice", "Song 1"), + id="no-featured-artist", + ), + pytest.param( + {"format": "feat {0}"}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1", None), + ("Alice", "Song 1 feat Bob"), + id="no-albumartist-custom-format", + ), + pytest.param( + None, + ("ftintitle",), + ("Alice", "Song 1", None), + ("Alice", "Song 1"), + id="no-albumartist-no-feature", + ), + pytest.param( + {"format": "featuring {0}"}, + ("ftintitle",), + ("Alice ft Bob", "Song 1", "George"), + ("Alice", "Song 1 featuring Bob"), + id="guest-artist-custom-format", + ), + pytest.param( + None, + ("ftintitle",), + ("Alice", "Song 1", "George"), + ("Alice", "Song 1"), + id="guest-artist-no-feature", + ), + # ---- drop (-d) variants ---- + pytest.param( + None, + ("ftintitle", "-d"), + ("Alice ft Bob", "Song 1", "Alice"), + ("Alice", "Song 1"), + id="drop-self-ft", + ), + pytest.param( + None, + ("ftintitle", "-d"), + ("Alice", "Song 1", "Alice"), + ("Alice", "Song 1"), + id="drop-self-no-ft", + ), + pytest.param( + None, + ("ftintitle", "-d"), + ("Alice ft Bob", "Song 1", "George"), + ("Alice", "Song 1"), + id="drop-guest-ft", + ), + pytest.param( + None, + ("ftintitle", "-d"), + ("Alice", "Song 1", "George"), + ("Alice", "Song 1"), + id="drop-guest-no-ft", + ), + # ---- custom format variants ---- + pytest.param( + {"format": "feat. {}"}, + ("ftintitle",), + ("Alice ft Bob", "Song 1", "Alice"), + ("Alice", "Song 1 feat. Bob"), + id="custom-format-feat-dot", + ), + pytest.param( + {"format": "featuring {}"}, + ("ftintitle",), + ("Alice feat. Bob", "Song 1", "Alice"), + ("Alice", "Song 1 featuring Bob"), + id="custom-format-featuring", + ), + pytest.param( + {"format": "with {}"}, + ("ftintitle",), + ("Alice feat Bob", "Song 1", "Alice"), + ("Alice", "Song 1 with Bob"), + id="custom-format-with", + ), + # ---- keep_in_artist variants ---- + pytest.param( + {"format": "feat. {}", "keep_in_artist": True}, + ("ftintitle",), + ("Alice ft Bob", "Song 1", "Alice"), + ("Alice ft Bob", "Song 1 feat. Bob"), + id="keep-in-artist-add-to-title", + ), + pytest.param( + {"format": "feat. {}", "keep_in_artist": True}, + ("ftintitle", "-d"), + ("Alice ft Bob", "Song 1", "Alice"), + ("Alice ft Bob", "Song 1"), + id="keep-in-artist-drop-from-title", + ), + ], +) +def test_ftintitle_functional( + env: FtInTitlePluginFunctional, + cfg: Optional[Dict[str, Union[str, bool]]], + cmd_args: Tuple[str, ...], + given: Tuple[str, str, Optional[str]], + expected: Tuple[str, str], +) -> None: + set_config(env, cfg) + ftintitle.FtInTitlePlugin() + + artist, title, albumartist = given + item = add_item(env, "/", artist, title, albumartist) + + env.run_command(*cmd_args) + item.load() + + expected_artist, expected_title = expected + assert item["artist"] == expected_artist + assert item["title"] == expected_title + + +@pytest.mark.parametrize( + "artist,albumartist,expected", + [ + ("Alice ft. Bob", "Alice", "Bob"), + ("Alice feat Bob", "Alice", "Bob"), + ("Alice featuring Bob", "Alice", "Bob"), + ("Alice & Bob", "Alice", "Bob"), + ("Alice and Bob", "Alice", "Bob"), + ("Alice With Bob", "Alice", "Bob"), + ("Alice defeat Bob", "Alice", None), + ("Alice & Bob", "Bob", "Alice"), + ("Alice ft. Bob", "Bob", "Alice"), + ("Alice ft. Carol", "Bob", "Carol"), + ], +) +def test_find_feat_part( + artist: str, + albumartist: str, + expected: Optional[str], +) -> None: + assert ftintitle.find_feat_part(artist, albumartist) == expected + + +@pytest.mark.parametrize( + "given,expected", + [ + ("Alice ft. Bob", ("Alice", "Bob")), + ("Alice feat Bob", ("Alice", "Bob")), + ("Alice feat. Bob", ("Alice", "Bob")), + ("Alice featuring Bob", ("Alice", "Bob")), + ("Alice & Bob", ("Alice", "Bob")), + ("Alice and Bob", ("Alice", "Bob")), + ("Alice With Bob", ("Alice", "Bob")), + ("Alice defeat Bob", ("Alice defeat Bob", None)), + ], +) +def test_split_on_feat( + given: str, + expected: Tuple[str, Optional[str]], +) -> None: + assert ftintitle.split_on_feat(given) == expected + + +@pytest.mark.parametrize( + "given,expected", + [ + ("Alice ft. Bob", True), + ("Alice feat. Bob", True), + ("Alice feat Bob", True), + ("Alice featuring Bob", True), + ("Alice (ft. Bob)", True), + ("Alice (feat. Bob)", True), + ("Alice [ft. Bob]", True), + ("Alice [feat. Bob]", True), + ("Alice defeat Bob", False), + ("Aliceft.Bob", False), + ("Alice (defeat Bob)", False), + ("Live and Let Go", False), + ("Come With Me", False), + ], +) +def test_contains_feat(given: str, expected: bool) -> None: + assert ftintitle.contains_feat(given) is expected diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 72b0d4f00..d6df42f97 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -14,7 +14,7 @@ """Tests for the 'lastgenre' plugin.""" -from unittest.mock import Mock +from unittest.mock import Mock, patch import pytest @@ -131,6 +131,43 @@ class LastGenrePluginTest(BeetsTestCase): "math rock", ] + def test_pretend_option_skips_library_updates(self): + item = self.create_item( + album="Pretend Album", + albumartist="Pretend Artist", + artist="Pretend Artist", + title="Pretend Track", + genre="Original Genre", + ) + album = self.lib.add_album([item]) + + command = self.plugin.commands()[0] + opts, args = command.parser.parse_args(["--pretend"]) + + with patch.object(lastgenre.ui, "should_write", return_value=True): + with patch.object( + self.plugin, + "_get_genre", + return_value=("Mock Genre", "mock stage"), + ) as mock_get_genre: + with patch.object(self.plugin._log, "info") as log_info: + # Mock try_write to verify it's never called in pretend mode + with patch.object(item, "try_write") as mock_try_write: + command.func(self.lib, opts, args) + + mock_get_genre.assert_called_once() + + assert any( + call.args[1] == "Pretend: " for call in log_info.call_args_list + ) + + # Verify that try_write was never called (file operations skipped) + mock_try_write.assert_not_called() + + stored_album = self.lib.get_album(album.id) + assert stored_album.genre == "Original Genre" + assert stored_album.items()[0].genre == "Original Genre" + def test_no_duplicate(self): """Remove duplicated genres.""" self._setup_config(count=99) diff --git a/test/plugins/test_spotify.py b/test/plugins/test_spotify.py index 86b5651b9..bc55485c6 100644 --- a/test/plugins/test_spotify.py +++ b/test/plugins/test_spotify.py @@ -82,8 +82,8 @@ class SpotifyPluginTest(PluginTestCase): params = _params(responses.calls[0].request.url) query = params["q"][0] assert "duifhjslkef" in query - assert 'artist:"ujydfsuihse"' in query - assert 'album:"lkajsdflakjsd"' in query + assert "artist:'ujydfsuihse'" in query + assert "album:'lkajsdflakjsd'" in query assert params["type"] == ["track"] @responses.activate @@ -117,8 +117,8 @@ class SpotifyPluginTest(PluginTestCase): params = _params(responses.calls[0].request.url) query = params["q"][0] assert "Happy" in query - assert 'artist:"Pharrell Williams"' in query - assert 'album:"Despicable Me 2"' in query + assert "artist:'Pharrell Williams'" in query + assert "album:'Despicable Me 2'" in query assert params["type"] == ["track"] @responses.activate @@ -233,8 +233,8 @@ class SpotifyPluginTest(PluginTestCase): params = _params(responses.calls[0].request.url) query = params["q"][0] assert item.title in query - assert f'artist:"{item.albumartist}"' in query - assert f'album:"{item.album}"' in query + assert f"artist:'{item.albumartist}'" in query + assert f"album:'{item.album}'" in query assert not query.isascii() # Is not found in the library if ascii encoding is enabled diff --git a/test/test_dbcore.py b/test/test_dbcore.py index b2ec2e968..653adf298 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -23,6 +23,7 @@ from tempfile import mkstemp import pytest from beets import dbcore +from beets.dbcore.db import DBCustomFunctionError from beets.library import LibModel from beets.test import _common from beets.util import cached_classproperty @@ -31,6 +32,13 @@ from beets.util import cached_classproperty # have multiple models with different numbers of fields. +@pytest.fixture +def db(model): + db = model(":memory:") + yield db + db._connection().close() + + class SortFixture(dbcore.query.FieldSort): pass @@ -81,7 +89,6 @@ class ModelFixture1(LibModel): class DatabaseFixture1(dbcore.Database): _models = (ModelFixture1,) - pass class ModelFixture2(ModelFixture1): @@ -94,7 +101,6 @@ class ModelFixture2(ModelFixture1): class DatabaseFixture2(dbcore.Database): _models = (ModelFixture2,) - pass class ModelFixture3(ModelFixture1): @@ -108,7 +114,6 @@ class ModelFixture3(ModelFixture1): class DatabaseFixture3(dbcore.Database): _models = (ModelFixture3,) - pass class ModelFixture4(ModelFixture1): @@ -123,7 +128,6 @@ class ModelFixture4(ModelFixture1): class DatabaseFixture4(dbcore.Database): _models = (ModelFixture4,) - pass class AnotherModelFixture(ModelFixture1): @@ -145,12 +149,10 @@ class ModelFixture5(ModelFixture1): class DatabaseFixture5(dbcore.Database): _models = (ModelFixture5,) - pass class DatabaseFixtureTwoModels(dbcore.Database): _models = (ModelFixture2, AnotherModelFixture) - pass class ModelFixtureWithGetters(dbcore.Model): @@ -784,3 +786,25 @@ class ResultsIteratorTest(unittest.TestCase): self.db._fetch(ModelFixture1, dbcore.query.FalseQuery()).get() is None ) + + +class TestException: + @pytest.mark.parametrize("model", [DatabaseFixture1]) + @pytest.mark.filterwarnings( + "ignore: .*plz_raise.*: pytest.PytestUnraisableExceptionWarning" + ) + @pytest.mark.filterwarnings( + "error: .*: pytest.PytestUnraisableExceptionWarning" + ) + def test_custom_function_error(self, db: DatabaseFixture1): + def plz_raise(): + raise Exception("i haz raized") + + db._connection().create_function("plz_raise", 0, plz_raise) + + with db.transaction() as tx: + tx.mutate("insert into test (field_one) values (1)") + + with pytest.raises(DBCustomFunctionError): + with db.transaction() as tx: + tx.query("select * from test where plz_raise()") 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