From 65f2285dd3cdded85fbb35242a64ddfe5d602793 Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Sun, 25 Aug 2024 13:50:40 -0400 Subject: [PATCH] fix(import): don't throw away album flexible fields As noted by 5bf4e3d92f5b4010a19cd3e8222fbf28b7b844e0, MusicBrainz external IDs (`*_album_id`) were only saved for items and not albums. This commit addresses that by copying `AlbumInfo` fields to the `Album`, i.e. what's saved in the DB. This is similar to how `TrackInfo` fields are copied to `Item` instances except the copying is done at a different time since we only get an `Album` much later in the import flow. --- beets/autotag/__init__.py | 67 ++++++++++++++++++++++++--------------- beets/importer.py | 15 ++++++--- beets/test/helper.py | 1 + docs/changelog.rst | 4 +++ test/test_importer.py | 6 ++++ 5 files changed, 63 insertions(+), 30 deletions(-) diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index 54a9d5546..abbc07772 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -14,10 +14,10 @@ """Facilities for automatically determining files' correct metadata. """ -from typing import Mapping +from typing import Mapping, Sequence, Union from beets import config, logging -from beets.library import Item +from beets.library import Album, Item # Parts of external interface. from .hooks import ( # noqa @@ -80,6 +80,29 @@ SPECIAL_FIELDS = { # Additional utilities for the main interface. +def _apply_metadata( + info: Union[AlbumInfo, TrackInfo], + db_obj: Union[Album, Item], + nullable_fields: Sequence[str] = [], +): + """Set the db_obj's metadata to match the info.""" + special_fields = SPECIAL_FIELDS[ + "album" if isinstance(info, AlbumInfo) else "track" + ] + + for field, value in info.items(): + # We only overwrite fields that are not already hardcoded. + if field in special_fields: + continue + + # Don't overwrite fields with empty values unless the + # field is explicitly allowed to be overwritten. + if value is None and field not in nullable_fields: + continue + + db_obj[field] = value + + def apply_item_metadata(item: Item, track_info: TrackInfo): """Set an item's metadata from its matched TrackInfo object.""" item.artist = track_info.artist @@ -96,18 +119,17 @@ def apply_item_metadata(item: Item, track_info: TrackInfo): if track_info.artists_ids: item.mb_artistids = track_info.artists_ids - for field, value in track_info.items(): - # We only overwrite fields that are not already hardcoded. - if field in SPECIAL_FIELDS["track"]: - continue - if value is None: - continue - item[field] = value + _apply_metadata(track_info, item) # At the moment, the other metadata is left intact (including album # and track number). Perhaps these should be emptied? +def apply_album_metadata(album_info: AlbumInfo, album: Album): + """Set the album's metadata to match the AlbumInfo object.""" + _apply_metadata(album_info, album) + + def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]): """Set the items' metadata to match an AlbumInfo object using a mapping from Items to TrackInfo objects. @@ -218,21 +240,14 @@ def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]): # Track alt. item.track_alt = track_info.track_alt - # Don't overwrite fields with empty values unless the - # field is explicitly allowed to be overwritten - for field, value in album_info.items(): - if field in SPECIAL_FIELDS["album"]: - continue - clobber = field in config["overwrite_null"]["album"].as_str_seq() - if value is None and not clobber: - continue - item[field] = value + _apply_metadata( + album_info, + item, + nullable_fields=config["overwrite_null"]["album"].as_str_seq(), + ) - for field, value in track_info.items(): - if field in SPECIAL_FIELDS["track"]: - continue - clobber = field in config["overwrite_null"]["track"].as_str_seq() - value = getattr(track_info, field) - if value is None and not clobber: - continue - item[field] = value + _apply_metadata( + track_info, + item, + nullable_fields=config["overwrite_null"]["track"].as_str_seq(), + ) diff --git a/beets/importer.py b/beets/importer.py index f6517b515..3a290a033 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -60,8 +60,7 @@ HISTORY_KEY = "taghistory" # def extend_reimport_fresh_fields_item(): # importer.REIMPORT_FRESH_FIELDS_ITEM.extend(['tidal_track_popularity'] # ) -REIMPORT_FRESH_FIELDS_ALBUM = ["data_source"] -REIMPORT_FRESH_FIELDS_ITEM = [ +REIMPORT_FRESH_FIELDS_ALBUM = [ "data_source", "bandcamp_album_id", "spotify_album_id", @@ -69,6 +68,7 @@ REIMPORT_FRESH_FIELDS_ITEM = [ "beatport_album_id", "tidal_album_id", ] +REIMPORT_FRESH_FIELDS_ITEM = list(REIMPORT_FRESH_FIELDS_ALBUM) # Global logger. log = logging.getLogger("beets") @@ -815,9 +815,16 @@ class ImportTask(BaseImportTask): with lib.transaction(): self.record_replaced(lib) self.remove_replaced(lib) + self.album = lib.add_album(self.imported_items()) - if "data_source" in self.imported_items()[0]: - self.album.data_source = self.imported_items()[0].data_source + if self.choice_flag == action.APPLY: + # Copy album flexible fields to the DB + # TODO: change the flow so we create the `Album` object earlier, + # and we can move this into `self.apply_metadata`, just like + # is done for tracks. + autotag.apply_album_metadata(self.match.info, self.album) + self.album.store() + self.reimport_metadata(lib) def record_replaced(self, lib): diff --git a/beets/test/helper.py b/beets/test/helper.py index 40d2c97b2..cd09ccf66 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -964,6 +964,7 @@ class AutotagStub: artist_id="artistid" + id, albumtype="soundtrack", data_source="match_source", + bandcamp_album_id="bc_url", ) diff --git a/docs/changelog.rst b/docs/changelog.rst index c1ee6c0d2..38997d4a9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,6 +21,10 @@ Bug fixes: * Improved naming of temporary files by separating the random part with the file extension. * Fixed the ``auto`` value for the :ref:`reflink` config option. * Fixed lyrics plugin only getting part of the lyrics from ``Genius.com`` :bug:`4815` +* Album flexible fields are now correctly saved. For instance MusicBrainz external links + such as `bandcamp_album_id` will be available on albums in addition to tracks. + For albums already in your library, a re-import is required for the fields to be added. + Such a re-import can be done with, in this case, `beet import -L data_source:=MusicBrainz`. * :doc:`plugins/autobpm`: Fix the ``TypeError`` where tempo was being returned as a numpy array. Update ``librosa`` dependency constraint to prevent similar issues in the future. diff --git a/test/test_importer.py b/test/test_importer.py index 0ac2a984a..a47903a29 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1654,6 +1654,12 @@ class ReimportTest(ImportTestCase): if new_artpath != old_artpath: self.assertNotExists(old_artpath) + def test_reimported_album_has_new_flexattr(self): + self._setup_session() + assert self._album().get("bandcamp_album_id") is None + self.importer.run() + assert self._album().bandcamp_album_id == "bc_url" + def test_reimported_album_not_preserves_flexattr(self): self._setup_session() assert self._album().data_source == "original_source"