diff --git a/beets/importer/tasks.py b/beets/importer/tasks.py index 646b64e7f..1c1d0e61e 100644 --- a/beets/importer/tasks.py +++ b/beets/importer/tasks.py @@ -680,6 +680,8 @@ class SingletonImportTask(ImportTask): return [self.item] def apply_metadata(self): + if config["import"]["from_scratch"]: + self.item.clear() autotag.apply_item_metadata(self.item, self.match.info) def _emit_imported(self, lib): diff --git a/beets/test/helper.py b/beets/test/helper.py index 207b0e491..6eba85b1b 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -364,15 +364,17 @@ class TestHelper(ConfigMixin): items.append(item) return self.lib.add_album(items) - def create_mediafile_fixture(self, ext="mp3", images=[]): + def create_mediafile_fixture(self, ext="mp3", images=[], target_dir=None): """Copy a fixture mediafile with the extension to `temp_dir`. `images` is a subset of 'png', 'jpg', and 'tiff'. For each specified extension a cover art image is added to the media file. """ + if not target_dir: + target_dir = self.temp_dir src = os.path.join(_common.RSRC, util.bytestring_path(f"full.{ext}")) - handle, path = mkstemp(dir=self.temp_dir) + handle, path = mkstemp(dir=target_dir) path = bytestring_path(path) os.close(handle) shutil.copyfile(syspath(src), syspath(path)) diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py index 6fec62774..ae1476101 100644 --- a/beets/util/artresizer.py +++ b/beets/util/artresizer.py @@ -24,6 +24,7 @@ import platform import re import subprocess from abc import ABC, abstractmethod +from contextlib import suppress from enum import Enum from itertools import chain from typing import TYPE_CHECKING, Any, ClassVar @@ -846,7 +847,8 @@ class ArtResizer: ) finally: if result_path != path_in: - os.unlink(path_in) + with suppress(OSError): + os.unlink(path_in) return result_path @property diff --git a/beetsplug/_utils/art.py b/beetsplug/_utils/art.py index 656c303ce..fce650c5b 100644 --- a/beetsplug/_utils/art.py +++ b/beetsplug/_utils/art.py @@ -206,9 +206,14 @@ def extract_first(log, outpath, items): return real_path +def clear_item(item, log): + if mediafile.MediaFile(syspath(item.path)).images: + log.debug("Clearing art for {}", item) + item.try_write(tags={"images": None}) + + def clear(log, lib, query): items = lib.items(query) log.info("Clearing album art from {} items", len(items)) for item in items: - log.debug("Clearing art for {}", item) - item.try_write(tags={"images": None}) + clear_item(item, log) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 2e837c77f..af1279299 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -279,6 +279,10 @@ class ConvertPlugin(BeetsPlugin): ) = self._get_opts_and_config(empty_opts) items = task.imported_items() + + # Filter items based on should_transcode function + items = [item for item in items if should_transcode(item, fmt)] + self._parallel_convert( dest, False, diff --git a/beetsplug/discogs.py b/beetsplug/discogs/__init__.py similarity index 69% rename from beetsplug/discogs.py rename to beetsplug/discogs/__init__.py index 08d437d2d..dc88e0f14 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs/__init__.py @@ -27,13 +27,12 @@ import time import traceback from functools import cache from string import ascii_lowercase -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING import confuse from discogs_client import Client, Master, Release from discogs_client.exceptions import DiscogsAPIError from requests.exceptions import ConnectionError -from typing_extensions import NotRequired, TypedDict import beets import beets.ui @@ -42,15 +41,20 @@ from beets.autotag.distance import string_dist from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.metadata_plugins import MetadataSourcePlugin +from .states import DISAMBIGUATION_RE, ArtistState, TracklistState + if TYPE_CHECKING: from collections.abc import Callable, Iterable, Sequence from beets.library import Item + from .types import ReleaseFormat, Track + USER_AGENT = f"beets/{beets.__version__} +https://beets.io/" API_KEY = "rAzVUQYRaoFjeBjyWuWZ" API_SECRET = "plxtUTqoCzwxZpqdPysCwGuBSmZNdZVy" + # Exceptions that discogs_client should really handle but does not. CONNECTION_ERRORS = ( ConnectionError, @@ -60,7 +64,6 @@ CONNECTION_ERRORS = ( DiscogsAPIError, ) - TRACK_INDEX_RE = re.compile( r""" (.*?) # medium: everything before medium_index. @@ -76,50 +79,6 @@ TRACK_INDEX_RE = re.compile( re.VERBOSE, ) -DISAMBIGUATION_RE = re.compile(r" \(\d+\)") - - -class ReleaseFormat(TypedDict): - name: str - qty: int - descriptions: list[str] | None - - -class Artist(TypedDict): - name: str - anv: str - join: str - role: str - tracks: str - id: str - resource_url: str - - -class Track(TypedDict): - position: str - type_: str - title: str - duration: str - artists: list[Artist] - extraartists: NotRequired[list[Artist]] - - -class TrackWithSubtracks(Track): - sub_tracks: list[TrackWithSubtracks] - - -class IntermediateTrackInfo(TrackInfo): - """Allows work with string mediums from - get_track_info""" - - def __init__( - self, - medium_str: str | None, - **kwargs, - ) -> None: - self.medium_str = medium_str - super().__init__(**kwargs) - class DiscogsPlugin(MetadataSourcePlugin): def __init__(self): @@ -277,7 +236,6 @@ class DiscogsPlugin(MetadataSourcePlugin): for track in album.tracks: if track.track_id == track_id: return track - return None def get_albums(self, query: str) -> Iterable[AlbumInfo]: @@ -343,25 +301,6 @@ class DiscogsPlugin(MetadataSourcePlugin): return media, albumtype - def get_artist_with_anv( - self, artists: list[Artist], use_anv: bool = False - ) -> tuple[str, str | None]: - """Iterates through a discogs result, fetching data - if the artist anv is to be used, maps that to the name. - Calls the parent class get_artist method.""" - artist_list: list[dict[str | int, str]] = [] - for artist_data in artists: - a: dict[str | int, str] = { - "name": artist_data["name"], - "id": artist_data["id"], - "join": artist_data.get("join", ""), - } - if use_anv and (anv := artist_data.get("anv", "")): - a["name"] = anv - artist_list.append(a) - artist, artist_id = self.get_artist(artist_list, join_key="join") - return self.strip_disambiguation(artist), artist_id - def get_album_info(self, result: Release) -> AlbumInfo | None: """Returns an AlbumInfo object for a discogs Release object.""" # Explicitly reload the `Release` fields, as they might not be yet @@ -391,11 +330,10 @@ class DiscogsPlugin(MetadataSourcePlugin): return None artist_data = [a.data for a in result.artists] - album_artist, album_artist_id = self.get_artist_with_anv(artist_data) - album_artist_anv, _ = self.get_artist_with_anv( - artist_data, use_anv=True + # Information for the album artist + albumartist = ArtistState.from_config( + self.config, artist_data, for_album_artist=True ) - artist_credit = album_artist_anv album = re.sub(r" +", " ", result.title) album_id = result.data["id"] @@ -405,19 +343,13 @@ class DiscogsPlugin(MetadataSourcePlugin): # each make an API call just to get the same data back. tracks = self.get_tracks( result.data["tracklist"], - (album_artist, album_artist_anv, album_artist_id), + ArtistState.from_config(self.config, artist_data), ) - # Assign ANV to the proper fields for tagging - if not self.config["anv"]["artist_credit"]: - artist_credit = album_artist - if self.config["anv"]["album_artist"]: - album_artist = album_artist_anv - # Extract information for the optional AlbumInfo fields, if possible. - va = result.data["artists"][0].get("name", "").lower() == "various" + va = albumartist.artist == config["va_name"].as_str() year = result.data.get("year") - mediums = [t.medium for t in tracks] + mediums = [t["medium"] for t in tracks] country = result.data.get("country") data_url = result.data.get("uri") style = self.format(result.data.get("styles")) @@ -447,11 +379,7 @@ class DiscogsPlugin(MetadataSourcePlugin): cover_art_url = self.select_cover_art(result) # Additional cleanups - # (various artists name, catalog number, media, disambiguation). - if va: - va_name = config["va_name"].as_str() - album_artist = va_name - artist_credit = va_name + # (catalog number, media, disambiguation). if catalogno == "none": catalogno = None # Explicitly set the `media` for the tracks, since it is expected by @@ -474,9 +402,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return AlbumInfo( album=album, album_id=album_id, - artist=album_artist, - artist_credit=artist_credit, - artist_id=album_artist_id, + **albumartist.info, # Unpacks values to satisfy the keyword arguments tracks=tracks, albumtype=albumtype, va=va, @@ -494,7 +420,7 @@ class DiscogsPlugin(MetadataSourcePlugin): data_url=data_url, discogs_albumid=discogs_albumid, discogs_labelid=labelid, - discogs_artistid=album_artist_id, + discogs_artistid=albumartist.artist_id, cover_art_url=cover_art_url, ) @@ -516,63 +442,22 @@ class DiscogsPlugin(MetadataSourcePlugin): else: return None - def _process_clean_tracklist( - self, - clean_tracklist: list[Track], - album_artist_data: tuple[str, str, str | None], - ) -> tuple[list[TrackInfo], dict[int, str], int, list[str], list[str]]: - # Distinct works and intra-work divisions, as defined by index tracks. - tracks: list[TrackInfo] = [] - index_tracks = {} - index = 0 - divisions: list[str] = [] - next_divisions: list[str] = [] - for track in clean_tracklist: - # Only real tracks have `position`. Otherwise, it's an index track. - if track["position"]: - index += 1 - if next_divisions: - # End of a block of index tracks: update the current - # divisions. - divisions += next_divisions - del next_divisions[:] - track_info = self.get_track_info( - track, index, divisions, album_artist_data - ) - track_info.track_alt = track["position"] - tracks.append(track_info) - else: - next_divisions.append(track["title"]) - # We expect new levels of division at the beginning of the - # tracklist (and possibly elsewhere). - try: - divisions.pop() - except IndexError: - pass - index_tracks[index + 1] = track["title"] - return tracks, index_tracks, index, divisions, next_divisions - def get_tracks( self, tracklist: list[Track], - album_artist_data: tuple[str, str, str | None], + albumartistinfo: ArtistState, ) -> list[TrackInfo]: """Returns a list of TrackInfo objects for a discogs tracklist.""" try: - clean_tracklist: list[Track] = self.coalesce_tracks( - cast(list[TrackWithSubtracks], tracklist) - ) + clean_tracklist: list[Track] = self._coalesce_tracks(tracklist) except Exception as exc: # FIXME: this is an extra precaution for making sure there are no # side effects after #2222. It should be removed after further # testing. self._log.debug("{}", traceback.format_exc()) - self._log.error("uncaught exception in coalesce_tracks: {}", exc) + self._log.error("uncaught exception in _coalesce_tracks: {}", exc) clean_tracklist = tracklist - processed = self._process_clean_tracklist( - clean_tracklist, album_artist_data - ) - tracks, index_tracks, *_ = processed + t = TracklistState.build(self, clean_tracklist, albumartistinfo) # Fix up medium and medium_index for each track. Discogs position is # unreliable, but tracks are in order. medium = None @@ -581,32 +466,36 @@ class DiscogsPlugin(MetadataSourcePlugin): # If a medium has two sides (ie. vinyl or cassette), each pair of # consecutive sides should belong to the same medium. - if all([track.medium_str is not None for track in tracks]): - m = sorted({track.medium_str.lower() for track in tracks}) + if all([medium is not None for medium in t.mediums]): + m = sorted( + {medium.lower() if medium else "" for medium in t.mediums} + ) # If all track.medium are single consecutive letters, assume it is # a 2-sided medium. if "".join(m) in ascii_lowercase: sides_per_medium = 2 - for track in tracks: + for i, track in enumerate(t.tracks): # Handle special case where a different medium does not indicate a # new disc, when there is no medium_index and the ordinal of medium # is not sequential. For example, I, II, III, IV, V. Assume these # are the track index, not the medium. # side_count is the number of mediums or medium sides (in the case # of two-sided mediums) that were seen before. + medium_str = t.mediums[i] + medium_index = t.medium_indices[i] medium_is_index = ( - track.medium_str - and not track.medium_index + medium_str + and not medium_index and ( - len(track.medium_str) != 1 + len(medium_str) != 1 or # Not within standard incremental medium values (A, B, C, ...). - ord(track.medium_str) - 64 != side_count + 1 + ord(medium_str) - 64 != side_count + 1 ) ) - if not medium_is_index and medium != track.medium_str: + if not medium_is_index and medium != medium_str: side_count += 1 if sides_per_medium == 2: if side_count % sides_per_medium: @@ -617,7 +506,7 @@ class DiscogsPlugin(MetadataSourcePlugin): # Medium changed. Reset index_count. medium_count += 1 index_count = 0 - medium = track.medium_str + medium = medium_str index_count += 1 medium_count = 1 if medium_count == 0 else medium_count @@ -625,69 +514,25 @@ class DiscogsPlugin(MetadataSourcePlugin): # Get `disctitle` from Discogs index tracks. Assume that an index track # before the first track of each medium is a disc title. - for track in tracks: + for track in t.tracks: if track.medium_index == 1: - if track.index in index_tracks: - disctitle = index_tracks[track.index] + if track.index in t.index_tracks: + disctitle = t.index_tracks[track.index] else: disctitle = None track.disctitle = disctitle - return cast(list[TrackInfo], tracks) + return t.tracks - def coalesce_tracks( - self, raw_tracklist: list[TrackWithSubtracks] - ) -> list[Track]: + def _coalesce_tracks(self, raw_tracklist: list[Track]) -> list[Track]: """Pre-process a tracklist, merging subtracks into a single track. The title for the merged track is the one from the previous index track, if present; otherwise it is a combination of the subtracks titles. """ - - def add_merged_subtracks( - tracklist: list[TrackWithSubtracks], - subtracks: list[TrackWithSubtracks], - ) -> None: - """Modify `tracklist` in place, merging a list of `subtracks` into - a single track into `tracklist`.""" - # Calculate position based on first subtrack, without subindex. - idx, medium_idx, sub_idx = self.get_track_index( - subtracks[0]["position"] - ) - position = f"{idx or ''}{medium_idx or ''}" - - if tracklist and not tracklist[-1]["position"]: - # Assume the previous index track contains the track title. - if sub_idx: - # "Convert" the track title to a real track, discarding the - # subtracks assuming they are logical divisions of a - # physical track (12.2.9 Subtracks). - tracklist[-1]["position"] = position - else: - # Promote the subtracks to real tracks, discarding the - # index track, assuming the subtracks are physical tracks. - index_track = tracklist.pop() - # Fix artists when they are specified on the index track. - if index_track.get("artists"): - for subtrack in subtracks: - if not subtrack.get("artists"): - subtrack["artists"] = index_track["artists"] - # Concatenate index with track title when index_tracks - # option is set - if self.config["index_tracks"]: - for subtrack in subtracks: - subtrack["title"] = ( - f"{index_track['title']}: {subtrack['title']}" - ) - tracklist.extend(subtracks) - else: - # Merge the subtracks, pick a title, and append the new track. - track = subtracks[0].copy() - track["title"] = " / ".join([t["title"] for t in subtracks]) - tracklist.append(track) - # Pre-process the tracklist, trying to identify subtracks. - subtracks: list[TrackWithSubtracks] = [] - tracklist: list[TrackWithSubtracks] = [] + + subtracks: list[Track] = [] + tracklist: list[Track] = [] prev_subindex = "" for track in raw_tracklist: # Regular subtrack (track with subindex). @@ -699,7 +544,7 @@ class DiscogsPlugin(MetadataSourcePlugin): subtracks.append(track) else: # Subtrack part of a new group (..., 1.3, *2.1*, ...). - add_merged_subtracks(tracklist, subtracks) + self._add_merged_subtracks(tracklist, subtracks) subtracks = [track] prev_subindex = subindex.rjust(len(raw_tracklist)) continue @@ -708,21 +553,64 @@ class DiscogsPlugin(MetadataSourcePlugin): if not track["position"] and "sub_tracks" in track: # Append the index track, assuming it contains the track title. tracklist.append(track) - add_merged_subtracks(tracklist, track["sub_tracks"]) + self._add_merged_subtracks(tracklist, track["sub_tracks"]) continue # Regular track or index track without nested sub_tracks. if subtracks: - add_merged_subtracks(tracklist, subtracks) + self._add_merged_subtracks(tracklist, subtracks) subtracks = [] prev_subindex = "" tracklist.append(track) # Merge and add the remaining subtracks, if any. if subtracks: - add_merged_subtracks(tracklist, subtracks) + self._add_merged_subtracks(tracklist, subtracks) - return cast(list[Track], tracklist) + return tracklist + + def _add_merged_subtracks( + self, + tracklist: list[Track], + subtracks: list[Track], + ) -> None: + """Modify `tracklist` in place, merging a list of `subtracks` into + a single track into `tracklist`.""" + # Calculate position based on first subtrack, without subindex. + idx, medium_idx, sub_idx = self.get_track_index( + subtracks[0]["position"] + ) + position = f"{idx or ''}{medium_idx or ''}" + + if tracklist and not tracklist[-1]["position"]: + # Assume the previous index track contains the track title. + if sub_idx: + # "Convert" the track title to a real track, discarding the + # subtracks assuming they are logical divisions of a + # physical track (12.2.9 Subtracks). + tracklist[-1]["position"] = position + else: + # Promote the subtracks to real tracks, discarding the + # index track, assuming the subtracks are physical tracks. + index_track = tracklist.pop() + # Fix artists when they are specified on the index track. + if index_track.get("artists"): + for subtrack in subtracks: + if not subtrack.get("artists"): + subtrack["artists"] = index_track["artists"] + # Concatenate index with track title when index_tracks + # option is set + if self.config["index_tracks"]: + for subtrack in subtracks: + subtrack["title"] = ( + f"{index_track['title']}: {subtrack['title']}" + ) + tracklist.extend(subtracks) + else: + # Merge the subtracks, pick a title, and append the new track. + track = subtracks[0].copy() + track["title"] = " / ".join([t["title"] for t in subtracks]) + tracklist.append(track) def strip_disambiguation(self, text: str) -> str: """Removes discogs specific disambiguations from a string. @@ -737,17 +625,10 @@ class DiscogsPlugin(MetadataSourcePlugin): track: Track, index: int, divisions: list[str], - album_artist_data: tuple[str, str, str | None], - ) -> IntermediateTrackInfo: + albumartistinfo: ArtistState, + ) -> tuple[TrackInfo, str | None, str | None]: """Returns a TrackInfo object for a discogs track.""" - artist, artist_anv, artist_id = album_artist_data - artist_credit = artist_anv - if not self.config["anv"]["artist_credit"]: - artist_credit = artist - if self.config["anv"]["artist"]: - artist = artist_anv - title = track["title"] if self.config["index_tracks"]: prefix = ", ".join(divisions) @@ -756,44 +637,26 @@ class DiscogsPlugin(MetadataSourcePlugin): track_id = None medium, medium_index, _ = self.get_track_index(track["position"]) - # If artists are found on the track, we will use those instead - if artists := track.get("artists", []): - artist, artist_id = self.get_artist_with_anv( - artists, self.config["anv"]["artist"] - ) - artist_credit, _ = self.get_artist_with_anv( - artists, self.config["anv"]["artist_credit"] - ) length = self.get_track_length(track["duration"]) + # If artists are found on the track, we will use those instead + artistinfo = ArtistState.from_config( + self.config, + [ + *(track.get("artists") or albumartistinfo.raw_artists), + *track.get("extraartists", []), + ], + ) - # Add featured artists - if extraartists := track.get("extraartists", []): - featured_list = [ - artist - for artist in extraartists - if "Featuring" in artist["role"] - ] - featured, _ = self.get_artist_with_anv( - featured_list, self.config["anv"]["artist"] - ) - featured_credit, _ = self.get_artist_with_anv( - featured_list, self.config["anv"]["artist_credit"] - ) - if featured: - artist += f" {self.config['featured_string']} {featured}" - artist_credit += ( - f" {self.config['featured_string']} {featured_credit}" - ) - return IntermediateTrackInfo( - title=title, - track_id=track_id, - artist_credit=artist_credit, - artist=artist, - artist_id=artist_id, - length=length, - index=index, - medium_str=medium, - medium_index=medium_index, + return ( + TrackInfo( + title=title, + track_id=track_id, + **artistinfo.info, + length=length, + index=index, + ), + medium, + medium_index, ) @staticmethod diff --git a/beetsplug/discogs/states.py b/beetsplug/discogs/states.py new file mode 100644 index 000000000..2a8173ba5 --- /dev/null +++ b/beetsplug/discogs/states.py @@ -0,0 +1,237 @@ +# This file is part of beets. +# Copyright 2025, Sarunas Nejus, Henry Oberholtzer. +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. + +"""Dataclasses for managing artist credits and tracklists from Discogs.""" + +from __future__ import annotations + +import re +from dataclasses import asdict, dataclass, field +from functools import cached_property +from typing import TYPE_CHECKING, NamedTuple + +from beets import config + +from .types import ArtistInfo + +if TYPE_CHECKING: + from confuse import ConfigView + + from beets.autotag.hooks import TrackInfo + + from . import DiscogsPlugin + from .types import Artist, Track, TracklistInfo + +DISAMBIGUATION_RE = re.compile(r" \(\d+\)") + + +@dataclass +class ArtistState: + """Represent Discogs artist credits. + + This object centralizes the plugin's policy for which Discogs artist fields + to prefer (name vs. ANV), how to treat 'Various', how to format join + phrases, and how to separate featured artists. It exposes both per-artist + components and fully joined strings for common tag targets like 'artist' and + 'artist_credit'. + """ + + class ValidArtist(NamedTuple): + """A normalized, render-ready artist entry extracted from Discogs data. + + Instances represent the subset of Discogs artist information needed for + tagging, including the join token following the artist and whether the + entry is considered a featured appearance. + """ + + id: str + name: str + credit: str + join: str + is_feat: bool + + def get_artist(self, property_name: str) -> str: + """Return the requested display field with its trailing join token. + + The join token is normalized so commas become ', ' and other join + phrases are surrounded with spaces, producing a single fragment that + can be concatenated to form a full artist string. + """ + join = {",": ", ", "": ""}.get(self.join, f" {self.join} ") + return f"{getattr(self, property_name)}{join}" + + raw_artists: list[Artist] + use_anv: bool + use_credit_anv: bool + featured_string: str + should_strip_disambiguation: bool + + @property + def info(self) -> ArtistInfo: + """Expose the state in the shape expected by downstream tag mapping.""" + return {k: getattr(self, k) for k in ArtistInfo.__annotations__} # type: ignore[return-value] + + def strip_disambiguation(self, text: str) -> str: + """Strip Discogs disambiguation suffixes from an artist or label string. + + This removes Discogs-specific numeric suffixes like 'Name (5)' and can + be applied to multi-artist strings as well (e.g., 'A (1) & B (2)'). When + the feature is disabled, the input is returned unchanged. + """ + if self.should_strip_disambiguation: + return DISAMBIGUATION_RE.sub("", text) + return text + + @cached_property + def valid_artists(self) -> list[ValidArtist]: + """Build the ordered, filtered list of artists used for rendering. + + The resulting list normalizes Discogs entries by: + - substituting the configured 'Various Artists' name when Discogs uses + 'Various' + - choosing between name and ANV according to plugin settings + - excluding non-empty roles unless they indicate a featured appearance + - capturing join tokens so the original credit formatting is preserved + """ + va_name = config["va_name"].as_str() + return [ + self.ValidArtist( + str(a["id"]), + self.strip_disambiguation(anv if self.use_anv else name), + self.strip_disambiguation(anv if self.use_credit_anv else name), + a["join"].strip(), + is_feat, + ) + for a in self.raw_artists + if ( + (name := va_name if a["name"] == "Various" else a["name"]) + and (anv := a["anv"] or name) + and ( + (is_feat := ("featuring" in a["role"].lower())) + or not a["role"] + ) + ) + ] + + @property + def artists_ids(self) -> list[str]: + """Return Discogs artist IDs for all valid artists, preserving order.""" + return [a.id for a in self.valid_artists] + + @property + def artist_id(self) -> str: + """Return the primary Discogs artist ID.""" + return self.artists_ids[0] + + @property + def artists(self) -> list[str]: + """Return the per-artist display names used for the 'artist' field.""" + return [a.name for a in self.valid_artists] + + @property + def artists_credit(self) -> list[str]: + """Return the per-artist display names used for the credit field.""" + return [a.credit for a in self.valid_artists] + + @property + def artist(self) -> str: + """Return the fully rendered artist string using display names.""" + return self.join_artists("name") + + @property + def artist_credit(self) -> str: + """Return the fully rendered artist credit string.""" + return self.join_artists("credit") + + def join_artists(self, property_name: str) -> str: + """Render a single artist string with join phrases and featured artists. + + Non-featured artists are concatenated using their join tokens. Featured + artists are appended after the configured 'featured' marker, preserving + Discogs order while keeping featured credits separate from the main + artist string. + """ + non_featured = [a for a in self.valid_artists if not a.is_feat] + featured = [a for a in self.valid_artists if a.is_feat] + + artist = "".join(a.get_artist(property_name) for a in non_featured) + if featured: + if "feat" not in artist: + artist += f" {self.featured_string} " + + artist += ", ".join(a.get_artist(property_name) for a in featured) + + return artist + + @classmethod + def from_config( + cls, + config: ConfigView, + artists: list[Artist], + for_album_artist: bool = False, + ) -> ArtistState: + return cls( + artists, + config["anv"]["album_artist" if for_album_artist else "artist"].get( + bool + ), + config["anv"]["artist_credit"].get(bool), + config["featured_string"].as_str(), + config["strip_disambiguation"].get(bool), + ) + + +@dataclass +class TracklistState: + index: int = 0 + index_tracks: dict[int, str] = field(default_factory=dict) + tracks: list[TrackInfo] = field(default_factory=list) + divisions: list[str] = field(default_factory=list) + next_divisions: list[str] = field(default_factory=list) + mediums: list[str | None] = field(default_factory=list) + medium_indices: list[str | None] = field(default_factory=list) + + @property + def info(self) -> TracklistInfo: + return asdict(self) # type: ignore[return-value] + + @classmethod + def build( + cls, + plugin: DiscogsPlugin, + clean_tracklist: list[Track], + albumartistinfo: ArtistState, + ) -> TracklistState: + state = cls() + for track in clean_tracklist: + if track["position"]: + state.index += 1 + if state.next_divisions: + state.divisions += state.next_divisions + state.next_divisions.clear() + track_info, medium, medium_index = plugin.get_track_info( + track, state.index, state.divisions, albumartistinfo + ) + track_info.track_alt = track["position"] + state.tracks.append(track_info) + state.mediums.append(medium or None) + state.medium_indices.append(medium_index or None) + else: + state.next_divisions.append(track["title"]) + try: + state.divisions.pop() + except IndexError: + pass + state.index_tracks[state.index + 1] = track["title"] + return state diff --git a/beetsplug/discogs/types.py b/beetsplug/discogs/types.py new file mode 100644 index 000000000..e06f51ed5 --- /dev/null +++ b/beetsplug/discogs/types.py @@ -0,0 +1,67 @@ +# This file is part of beets. +# Copyright 2025, Sarunas Nejus, Henry Oberholtzer. +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from typing_extensions import NotRequired, TypedDict + +if TYPE_CHECKING: + from beets.autotag.hooks import TrackInfo + + +class ReleaseFormat(TypedDict): + name: str + qty: int + descriptions: list[str] | None + + +class Artist(TypedDict): + name: str + anv: str + join: str + role: str + tracks: str + id: str + resource_url: str + + +class Track(TypedDict): + position: str + type_: str + title: str + duration: str + artists: list[Artist] + extraartists: NotRequired[list[Artist]] + sub_tracks: NotRequired[list[Track]] + + +class ArtistInfo(TypedDict): + artist: str + artists: list[str] + artist_credit: str + artists_credit: list[str] + artist_id: str + artists_ids: list[str] + + +class TracklistInfo(TypedDict): + index: int + index_tracks: dict[int, str] + tracks: list[TrackInfo] + divisions: list[str] + next_divisions: list[str] + mediums: list[str | None] + medium_indices: list[str | None] diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index cbf40f570..08e63836c 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -62,6 +62,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): "ifempty": False, "remove_art_file": False, "quality": 0, + "clearart_on_import": False, } ) @@ -82,6 +83,9 @@ class EmbedCoverArtPlugin(BeetsPlugin): self.register_listener("art_set", self.process_album) + if self.config["clearart_on_import"].get(bool): + self.register_listener("import_task_files", self.import_task_files) + def commands(self): # Embed command. embed_cmd = ui.Subcommand( @@ -278,3 +282,9 @@ class EmbedCoverArtPlugin(BeetsPlugin): os.remove(syspath(album.artpath)) album.artpath = None album.store() + + def import_task_files(self, session, task): + """Automatically clearart of imported files.""" + for item in task.imported_items(): + self._log.debug("clearart-on-import {.filepath}", item) + art.clear_item(item, self._log) diff --git a/beetsplug/fish.py b/beetsplug/fish.py index b1518f1c4..82e035eb4 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -183,16 +183,16 @@ def get_basic_beet_options(): BL_NEED2.format("-l format-item", "-f -d 'print with custom format'") + BL_NEED2.format("-l format-album", "-f -d 'print with custom format'") + BL_NEED2.format( - "-s l -l library", "-f -r -d 'library database file to use'" + "-s l -l library", "-F -r -d 'library database file to use'" ) + BL_NEED2.format( - "-s d -l directory", "-f -r -d 'destination music directory'" + "-s d -l directory", "-F -r -d 'destination music directory'" ) + BL_NEED2.format( "-s v -l verbose", "-f -d 'print debugging information'" ) + BL_NEED2.format( - "-s c -l config", "-f -r -d 'path to configuration file'" + "-s c -l config", "-F -r -d 'path to configuration file'" ) + BL_NEED2.format( "-s h -l help", "-f -d 'print this help message and exit'" @@ -216,7 +216,7 @@ def get_subcommands(cmd_name_and_help, nobasicfields, extravalues): word += BL_USE3.format( cmdname, f"-a {wrap('$FIELDS')}", - f"-f -d {wrap('fieldname')}", + f"-d {wrap('fieldname')}", ) if extravalues: @@ -270,7 +270,7 @@ def get_all_commands(beetcmds): word += " ".join( BL_USE3.format( name, - f"{cmd_need_arg}{cmd_s}{cmd_l} -f {cmd_arglist}", + f"{cmd_need_arg}{cmd_s}{cmd_l} {cmd_arglist}", cmd_helpstr, ).split() ) @@ -278,7 +278,7 @@ def get_all_commands(beetcmds): word = word + BL_USE3.format( name, - "-s h -l help -f", + "-s h -l help", f"-d {wrap('print help')}", ) return word diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 4e8b429ea..e83345059 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -20,6 +20,7 @@ import enum import math import os import queue +import shutil import signal import subprocess import sys @@ -27,8 +28,9 @@ import warnings from abc import ABC, abstractmethod from dataclasses import dataclass from multiprocessing.pool import ThreadPool +from pathlib import Path from threading import Event, Thread -from typing import TYPE_CHECKING, Any, TypeVar +from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar from beets import ui from beets.plugins import BeetsPlugin @@ -542,10 +544,20 @@ class FfmpegBackend(Backend): # mpgain/aacgain CLI tool backend. +Tool = Literal["mp3rgain", "aacgain", "mp3gain"] + + class CommandBackend(Backend): NAME = "command" + SUPPORTED_FORMATS_BY_TOOL: ClassVar[dict[Tool, set[str]]] = { + "mp3rgain": {"AAC", "MP3"}, + "aacgain": {"AAC", "MP3"}, + "mp3gain": {"MP3"}, + } do_parallel = True + cmd_name: Tool + def __init__(self, config: ConfigView, log: Logger): super().__init__(config, log) config.add( @@ -555,25 +567,21 @@ class CommandBackend(Backend): } ) - self.command: str = config["command"].as_str() + cmd_path: Path = Path(config["command"].as_str()) + supported_tools = set(self.SUPPORTED_FORMATS_BY_TOOL) - if self.command: - # Explicit executable path. - if not os.path.isfile(self.command): - raise FatalReplayGainError( - f"replaygain command does not exist: {self.command}" - ) - else: - # Check whether the program is in $PATH. - for cmd in ("mp3gain", "aacgain"): - try: - call([cmd, "-v"], self._log) - self.command = cmd - except OSError: - pass - if not self.command: + if (cmd_name := cmd_path.name) not in supported_tools: raise FatalReplayGainError( - "no replaygain command found: install mp3gain or aacgain" + f"replaygain.command must be one of {supported_tools!r}," + f" not {cmd_name!r}" + ) + + if command_exec := shutil.which(str(cmd_path)): + self.command = command_exec + self.cmd_name = cmd_name # type: ignore[assignment] + else: + raise FatalReplayGainError( + f"replaygain command not found: {cmd_path}" ) self.noclip = config["noclip"].get(bool) @@ -608,11 +616,7 @@ class CommandBackend(Backend): def format_supported(self, item: Item) -> bool: """Checks whether the given item is supported by the selected tool.""" - if "mp3gain" in self.command and item.format != "MP3": - return False - elif "aacgain" in self.command and item.format not in ("MP3", "AAC"): - return False - return True + return item.format in self.SUPPORTED_FORMATS_BY_TOOL[self.cmd_name] def compute_gain( self, diff --git a/docs/changelog.rst b/docs/changelog.rst index 2ba9f5cbd..d59c7ba1f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -45,9 +45,18 @@ New features: of brackets are supported and a new ``bracket_keywords`` configuration option allows customizing the keywords. Setting ``bracket_keywords`` to an empty list matches any bracket content regardless of keywords. +- :doc:`plugins/discogs`: Added support for multi value fields. :bug:`6068` +- :doc:`plugins/embedart`: Embedded arts can now be cleared during import with + the ``clearart_on_import`` config option. Also, ``beet clearart`` is only + going to update the files matching the query and with an embedded art, leaving + untouched the files without. +- :doc:`plugins/fish`: Filenames are now completed in more places, like after + ``beet import``. Bug fixes: +- Handle potential OSError when unlinking temporary files in ArtResizer. + :bug:`5615` - :doc:`/plugins/spotify`: Updated Spotify API credentials. :bug:`6270` - :doc:`/plugins/smartplaylist`: Fixed an issue where multiple queries in a playlist configuration were not preserving their order, causing items to @@ -90,6 +99,13 @@ Bug fixes: - :doc:`/plugins/ftintitle`: Fixed artist name splitting to prioritize explicit featuring tokens (feat, ft, featuring) over generic separators (&, and), preventing incorrect splits when both are present. +- :doc:`reference/cli`: Fix 'from_scratch' option for singleton imports: delete + all (old) metadata when new metadata is applied. :bug:`3706` +- :doc:`/plugins/convert`: ``auto_keep`` now respects ``no_convert`` and + ``never_convert_lossy_files`` when deciding whether to copy/transcode items, + avoiding extra lossy duplicates. +- :doc:`plugins/discogs`: Fixed unexpected flex attr from the Discogs plugin. + :bug:`6177` For plugin developers: diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index abbe2460d..0140ceff8 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -70,6 +70,8 @@ file. The available options are: :doc:`FetchArt ` plugin to download art with the purpose of directly embedding it into the file's metadata without an "intermediate" album art file. Default: ``no``. +- **clearart_on_import**: Enable automatic embedded art clearing. Default: + ``no``. Note: ``compare_threshold`` option requires ImageMagick_, and ``maxwidth`` requires either ImageMagick_ or Pillow_. @@ -110,4 +112,7 @@ embedded album art: automatically. - ``beet clearart QUERY``: removes all embedded images from all items matching the query. The command prompts for confirmation before making the change - unless you specify the ``-y`` (``--yes``) option. + unless you specify the ``-y`` (``--yes``) option. The files listed for + confirmation are the ones matching the query independently of having an + embedded art. However, only the files with an embedded art are updated, + leaving untouched the files without. diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index c7e51d25d..2973dd959 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -10,9 +10,9 @@ Installation ------------ This plugin can use one of many backends to compute the ReplayGain values: -GStreamer, mp3gain (and its cousin, aacgain), Python Audio Tools or ffmpeg. -ffmpeg and mp3gain can be easier to install. mp3gain supports less audio formats -than the other backend. +GStreamer, mp3gain (and its cousins, aacgain and mp3rgain), Python Audio Tools +or ffmpeg. ffmpeg and mp3gain can be easier to install. mp3gain supports fewer +audio formats than the other backends. Once installed, this plugin analyzes all files during the import process. This can be a slow process; to instead analyze after the fact, disable automatic @@ -51,16 +51,59 @@ configuration file: The GStreamer backend does not support parallel analysis. -mp3gain and aacgain -~~~~~~~~~~~~~~~~~~~ +Supported ``command`` backends +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -In order to use this backend, you will need to install the mp3gain_ command-line -tool or the aacgain_ fork thereof. Here are some hints: +In order to use this backend, you will need to install a supported command-line +tool: + +- mp3gain_ (MP3 only) +- aacgain_ (MP3, AAC/M4A) +- mp3rgain_ (MP3, AAC/M4A) + +mp3gain ++++++++ -- On Mac OS X, you can use Homebrew_. Type ``brew install aacgain``. - On Linux, mp3gain_ is probably in your repositories. On Debian or Ubuntu, for example, you can run ``apt-get install mp3gain``. -- On Windows, download and install the original mp3gain_. +- On Windows, download and install mp3gain_. + +aacgain ++++++++ + +- On macOS, install via Homebrew_: ``brew install aacgain``. +- For other platforms, download from aacgain_ or use a compatible fork if + available for your system. + +mp3rgain +++++++++ + +mp3rgain_ is a modern Rust rewrite of ``mp3gain`` that also supports AAC/M4A +files. It addresses security vulnerability CVE-2019-18359 present in the +original mp3gain and works on modern systems including Windows 11 and macOS with +Apple Silicon. + +- On macOS, install via Homebrew_: ``brew install mp3rgain``. +- On Linux, install via Nix: ``nix-env -iA nixpkgs.mp3rgain`` or from your + distribution packaging (for example, AUR on Arch Linux). +- On Windows, download and install mp3rgain_. + +Configuration ++++++++++++++ + +.. code-block:: yaml + + replaygain: + backend: command + command: # mp3rgain, mp3gain, or aacgain + +If beets doesn't automatically find the command executable, you can configure +the path explicitly like so: + +.. code-block:: yaml + + replaygain: + command: /Applications/MacMP3Gain.app/Contents/Resources/aacgain .. _aacgain: https://aacgain.altosdesign.com @@ -68,21 +111,7 @@ tool or the aacgain_ fork thereof. Here are some hints: .. _mp3gain: http://mp3gain.sourceforge.net/download.php -Then, enable the plugin (see :ref:`using-plugins`) and specify the "command" -backend in your configuration file: - -:: - - replaygain: - backend: command - -If beets doesn't automatically find the ``mp3gain`` or ``aacgain`` executable, -you can configure the path explicitly like so: - -:: - - replaygain: - command: /Applications/MacMP3Gain.app/Contents/Resources/aacgain +.. _mp3rgain: https://github.com/M-Igashi/mp3rgain Python Audio Tools ~~~~~~~~~~~~~~~~~~ @@ -144,10 +173,8 @@ file. The available options are: These options only work with the "command" backend: -- **command**: The path to the ``mp3gain`` or ``aacgain`` executable (if beets - cannot find it by itself). For example: - ``/Applications/MacMP3Gain.app/Contents/Resources/aacgain``. Default: Search - in your ``$PATH``. +- **command**: Name or path to your command backend of choice: either of + ``mp3gain``, ``aacgain`` or ``mp3rgain``. - **noclip**: Reduce the amount of ReplayGain adjustment to whatever amount would keep clipping from occurring. Default: ``yes``. diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index eb65bc588..15d47db6c 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -21,7 +21,19 @@ import pytest from beets import config from beets.test._common import Bag from beets.test.helper import BeetsTestCase, capture_log -from beetsplug.discogs import DiscogsPlugin +from beetsplug.discogs import ArtistState, DiscogsPlugin + + +def _artist(name: str, **kwargs): + return { + "id": 1, + "name": name, + "join": "", + "role": "", + "anv": "", + "tracks": "", + "resource_url": "", + } | kwargs @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) @@ -35,9 +47,7 @@ class DGAlbumInfoTest(BeetsTestCase): "uri": "https://www.discogs.com/release/release/13633721", "title": "ALBUM TITLE", "year": "3001", - "artists": [ - {"name": "ARTIST NAME", "id": "ARTIST ID", "join": ","} - ], + "artists": [_artist("ARTIST NAME", id="ARTIST ID", join=",")], "formats": [ { "descriptions": ["FORMAT DESC 1", "FORMAT DESC 2"], @@ -325,7 +335,7 @@ class DGAlbumInfoTest(BeetsTestCase): "id": 123, "uri": "https://www.discogs.com/release/123456-something", "tracklist": [self._make_track("A", "1", "01:01")], - "artists": [{"name": "ARTIST NAME", "id": 321, "join": ""}], + "artists": [_artist("ARTIST NAME", id=321)], "title": "TITLE", } release = Bag( @@ -385,14 +395,12 @@ class DGAlbumInfoTest(BeetsTestCase): "position": "A", "type_": "track", "duration": "5:44", - "artists": [ - {"name": "TEST ARTIST (5)", "tracks": "", "id": 11146} - ], + "artists": [_artist("TEST ARTIST (5)", id=11146)], } ], "artists": [ - {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, - {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + _artist("ARTIST NAME (2)", id=321, join="&"), + _artist("OTHER ARTIST (5)", id=321), ], "title": "title", "labels": [ @@ -409,7 +417,12 @@ class DGAlbumInfoTest(BeetsTestCase): ) d = DiscogsPlugin().get_album_info(release) assert d.artist == "ARTIST NAME & OTHER ARTIST" + assert d.artists == ["ARTIST NAME", "OTHER ARTIST"] + assert d.artists_ids == ["321", "321"] assert d.tracks[0].artist == "TEST ARTIST" + assert d.tracks[0].artists == ["TEST ARTIST"] + assert d.tracks[0].artist_id == "11146" + assert d.tracks[0].artists_ids == ["11146"] assert d.label == "LABEL NAME" def test_strip_disambiguation_false(self): @@ -424,14 +437,12 @@ class DGAlbumInfoTest(BeetsTestCase): "position": "A", "type_": "track", "duration": "5:44", - "artists": [ - {"name": "TEST ARTIST (5)", "tracks": "", "id": 11146} - ], + "artists": [_artist("TEST ARTIST (5)", id=11146)], } ], "artists": [ - {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, - {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + _artist("ARTIST NAME (2)", id=321, join="&"), + _artist("OTHER ARTIST (5)", id=321), ], "title": "title", "labels": [ @@ -448,35 +459,62 @@ class DGAlbumInfoTest(BeetsTestCase): ) d = DiscogsPlugin().get_album_info(release) assert d.artist == "ARTIST NAME (2) & OTHER ARTIST (5)" + assert d.artists == ["ARTIST NAME (2)", "OTHER ARTIST (5)"] assert d.tracks[0].artist == "TEST ARTIST (5)" + assert d.tracks[0].artists == ["TEST ARTIST (5)"] assert d.label == "LABEL NAME (5)" config["discogs"]["strip_disambiguation"] = True @pytest.mark.parametrize( - "track_artist_anv,track_artist", - [(False, "ARTIST Feat. PERFORMER"), (True, "VARIATION Feat. VARIATION")], -) -@pytest.mark.parametrize( - "album_artist_anv,album_artist", - [(False, "ARTIST & SOLOIST"), (True, "VARIATION & VARIATION")], -) -@pytest.mark.parametrize( - "artist_credit_anv,track_artist_credit,album_artist_credit", + "track_artist_anv,track_artist,track_artists", [ - (False, "ARTIST Feat. PERFORMER", "ARTIST & SOLOIST"), - (True, "VARIATION Feat. VARIATION", "VARIATION & VARIATION"), + (False, "ARTIST Feat. PERFORMER", ["ARTIST", "PERFORMER"]), + (True, "ART Feat. PERF", ["ART", "PERF"]), + ], +) +@pytest.mark.parametrize( + "album_artist_anv,album_artist,album_artists", + [ + (False, "DRUMMER, ARTIST & SOLOIST", ["DRUMMER", "ARTIST", "SOLOIST"]), + (True, "DRUM, ARTY & SOLO", ["DRUM", "ARTY", "SOLO"]), + ], +) +@pytest.mark.parametrize( + ( + "artist_credit_anv,track_artist_credit," + "track_artists_credit,album_artist_credit,album_artists_credit" + ), + [ + ( + False, + "ARTIST Feat. PERFORMER", + ["ARTIST", "PERFORMER"], + "DRUMMER, ARTIST & SOLOIST", + ["DRUMMER", "ARTIST", "SOLOIST"], + ), + ( + True, + "ART Feat. PERF", + ["ART", "PERF"], + "DRUM, ARTY & SOLO", + ["DRUM", "ARTY", "SOLO"], + ), ], ) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_anv( track_artist_anv, track_artist, + track_artists, album_artist_anv, album_artist, + album_artists, artist_credit_anv, track_artist_credit, + track_artists_credit, album_artist_credit, + album_artists_credit, ): """Test using artist name variations.""" data = { @@ -488,27 +526,21 @@ def test_anv( "position": "A", "type_": "track", "duration": "5:44", - "artists": [ - { - "name": "ARTIST", - "tracks": "", - "anv": "VARIATION", - "id": 11146, - } - ], + "artists": [_artist("ARTIST", id=11146, anv="ART")], "extraartists": [ - { - "name": "PERFORMER", - "role": "Featuring", - "anv": "VARIATION", - "id": 787, - } + _artist( + "PERFORMER", + id=787, + role="Featuring", + anv="PERF", + ) ], } ], "artists": [ - {"name": "ARTIST (4)", "anv": "VARIATION", "id": 321, "join": "&"}, - {"name": "SOLOIST", "anv": "VARIATION", "id": 445, "join": ""}, + _artist("DRUMMER", id=445, anv="DRUM", join=", "), + _artist("ARTIST (4)", id=321, anv="ARTY", join="&"), + _artist("SOLOIST", id=445, anv="SOLO"), ], "title": "title", } @@ -522,9 +554,53 @@ def test_anv( config["discogs"]["anv"]["artist_credit"] = artist_credit_anv r = DiscogsPlugin().get_album_info(release) assert r.artist == album_artist + assert r.artists == album_artists assert r.artist_credit == album_artist_credit + assert r.artists_credit == album_artists_credit assert r.tracks[0].artist == track_artist + assert r.tracks[0].artists == track_artists assert r.tracks[0].artist_credit == track_artist_credit + assert r.tracks[0].artists_credit == track_artists_credit + + +@pytest.mark.parametrize("artist_anv", [True, False]) +@pytest.mark.parametrize("albumartist_anv", [True, False]) +@pytest.mark.parametrize("artistcredit_anv", [True, False]) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +def test_anv_no_variation(artist_anv, albumartist_anv, artistcredit_anv): + """Test behavior when there is no ANV but the anv field is set""" + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [ + { + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [_artist("PERFORMER", id=1)], + } + ], + "artists": [_artist("ARTIST", id=2)], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + config["discogs"]["anv"]["album_artist"] = albumartist_anv + config["discogs"]["anv"]["artist"] = artist_anv + config["discogs"]["anv"]["artist_credit"] = artistcredit_anv + r = DiscogsPlugin().get_album_info(release) + assert r.artist == "ARTIST" + assert r.artists == ["ARTIST"] + assert r.artist_credit == "ARTIST" + assert r.artists_credit == ["ARTIST"] + assert r.tracks[0].artist == "PERFORMER" + assert r.tracks[0].artists == ["PERFORMER"] + assert r.tracks[0].artist_credit == "PERFORMER" + assert r.tracks[0].artists_credit == ["PERFORMER"] @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) @@ -543,9 +619,7 @@ def test_anv_album_artist(): "duration": "5:44", } ], - "artists": [ - {"name": "ARTIST (4)", "anv": "VARIATION", "id": 321}, - ], + "artists": [_artist("ARTIST (4)", id=321, anv="VARIATION")], "title": "title", } release = Bag( @@ -558,13 +632,18 @@ def test_anv_album_artist(): config["discogs"]["anv"]["artist_credit"] = False r = DiscogsPlugin().get_album_info(release) assert r.artist == "ARTIST" + assert r.artists == ["ARTIST"] assert r.artist_credit == "ARTIST" + assert r.artist_id == "321" + assert r.artists_credit == ["ARTIST"] assert r.tracks[0].artist == "VARIATION" + assert r.tracks[0].artists == ["VARIATION"] assert r.tracks[0].artist_credit == "ARTIST" + assert r.tracks[0].artists_credit == ["ARTIST"] @pytest.mark.parametrize( - "track, expected_artist", + "track, expected_artist, expected_artists", [ ( { @@ -573,45 +652,32 @@ def test_anv_album_artist(): "position": "1", "duration": "5:00", "artists": [ - {"name": "NEW ARTIST", "tracks": "", "id": 11146}, - {"name": "VOCALIST", "tracks": "", "id": 344, "join": "&"}, + _artist("NEW ARTIST", id=11146, join="&"), + _artist("VOCALIST", id=344, join="feat."), ], "extraartists": [ - { - "name": "SOLOIST", - "id": 3, - "role": "Featuring", - }, - { - "name": "PERFORMER (1)", - "id": 5, - "role": "Other Role, Featuring", - }, - { - "name": "RANDOM", - "id": 8, - "role": "Written-By", - }, - { - "name": "MUSICIAN", - "id": 10, - "role": "Featuring [Uncredited]", - }, + _artist("SOLOIST", id=3, role="Featuring"), + _artist( + "PERFORMER (1)", id=5, role="Other Role, Featuring" + ), + _artist("RANDOM", id=8, role="Written-By"), + _artist("MUSICIAN", id=10, role="Featuring [Uncredited]"), ], }, - "NEW ARTIST, VOCALIST Feat. SOLOIST, PERFORMER, MUSICIAN", + "NEW ARTIST & VOCALIST feat. SOLOIST, PERFORMER, MUSICIAN", + ["NEW ARTIST", "VOCALIST", "SOLOIST", "PERFORMER", "MUSICIAN"], ), ], ) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) -def test_parse_featured_artists(track, expected_artist): +def test_parse_featured_artists(track, expected_artist, expected_artists): """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", "ARTIST CREDIT", 2) - ) + Ignores artists that are not listed as featured.""" + plugin = DiscogsPlugin() + artistinfo = ArtistState.from_config(plugin.config, [_artist("ARTIST")]) + t, _, _ = plugin.get_track_info(track, 1, 1, artistinfo) assert t.artist == expected_artist + assert t.artists == expected_artists @pytest.mark.parametrize( @@ -637,6 +703,32 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): assert result == (expected_media, expected_albumtype) +@pytest.mark.parametrize( + "given_artists,expected_info,config_va_name", + [ + ( + [_artist("Various")], + { + "artist": "VARIOUS ARTISTS", + "artist_id": "1", + "artists": ["VARIOUS ARTISTS"], + "artists_ids": ["1"], + "artist_credit": "VARIOUS ARTISTS", + "artists_credit": ["VARIOUS ARTISTS"], + }, + "VARIOUS ARTISTS", + ) + ], +) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +def test_va_buildartistinfo(given_artists, expected_info, config_va_name): + config["va_name"] = config_va_name + assert ( + ArtistState.from_config(DiscogsPlugin().config, given_artists).info + == expected_info + ) + + @pytest.mark.parametrize( "position, medium, index, subindex", [ diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index d40025374..a7038b152 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -28,6 +28,7 @@ from beets.test import _common from beets.test.helper import ( BeetsTestCase, FetchImageHelper, + ImportHelper, IOMixin, PluginMixin, ) @@ -75,7 +76,9 @@ def require_artresizer_compare(test): return wrapper -class EmbedartCliTest(IOMixin, PluginMixin, FetchImageHelper, BeetsTestCase): +class EmbedartCliTest( + ImportHelper, IOMixin, PluginMixin, FetchImageHelper, BeetsTestCase +): plugin = "embedart" small_artpath = os.path.join(_common.RSRC, b"image-2x3.jpg") abbey_artpath = os.path.join(_common.RSRC, b"abbey.jpg") @@ -225,10 +228,20 @@ class EmbedartCliTest(IOMixin, PluginMixin, FetchImageHelper, BeetsTestCase): item = album.items()[0] self.io.addinput("y") self.run_command("embedart", "-f", self.small_artpath) + embedded_time = os.path.getmtime(syspath(item.path)) + self.io.addinput("y") self.run_command("clearart") mediafile = MediaFile(syspath(item.path)) assert not mediafile.images + clear_time = os.path.getmtime(syspath(item.path)) + assert clear_time > embedded_time + + # A run on a file without an image should not be modified + self.io.addinput("y") + self.run_command("clearart") + no_clear_time = os.path.getmtime(syspath(item.path)) + assert no_clear_time == clear_time def test_clear_art_with_no_input(self): self._setup_data() @@ -273,6 +286,32 @@ class EmbedartCliTest(IOMixin, PluginMixin, FetchImageHelper, BeetsTestCase): mediafile = MediaFile(syspath(item.path)) assert not mediafile.images + def test_clearart_on_import_disabled(self): + file_path = self.create_mediafile_fixture( + images=["jpg"], target_dir=self.import_path + ) + self.import_media.append(file_path) + with self.configure_plugin({"clearart_on_import": False}): + importer = self.setup_importer(autotag=False, write=True) + importer.run() + + item = self.lib.items()[0] + assert MediaFile(os.path.join(item.path)).images + + def test_clearart_on_import_enabled(self): + file_path = self.create_mediafile_fixture( + images=["jpg"], target_dir=self.import_path + ) + self.import_media.append(file_path) + # Force re-init the plugin to register the listener + self.unload_plugins() + with self.configure_plugin({"clearart_on_import": True}): + importer = self.setup_importer(autotag=False, write=True) + importer.run() + + item = self.lib.items()[0] + assert not MediaFile(os.path.join(item.path)).images + class DummyArtResizer(ArtResizer): """An `ArtResizer` which pretends that ImageMagick is available, and has diff --git a/test/test_importer.py b/test/test_importer.py index c1768df3e..6ae7d562b 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -258,6 +258,17 @@ class ImportSingletonTest(AutotagImportTestCase): assert self.lib.items().get().title == "Applied Track 1" assert (self.lib_path / "singletons" / "Applied Track 1.mp3").exists() + def test_apply_from_scratch_removes_other_metadata(self): + config["import"]["from_scratch"] = True + + for mediafile in self.import_media: + mediafile.comments = "Tag Comment" + mediafile.save() + + self.importer.add_choice(importer.Action.APPLY) + self.importer.run() + assert self.lib.items().get().comments == "" + def test_skip_does_not_add_track(self): self.importer.add_choice(importer.Action.SKIP) self.importer.run()