fix(import): don't throw away album flexible fields (#5388)

I believe this fixes #4785, but I might be misunderstanding the
description and it might already be fixed.

Tagging @JOJ0 since they created both the bug linked above, and
5bf4e3d which mentions it.

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 @@
 # 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(),
+        )

beets/autotag/mb.py

@@ -19,6 +19,7 @@
 import re
 import traceback
 from collections import Counter
+from itertools import product
 from typing import Any, Dict, Iterator, List, Optional, Sequence, Tuple, cast
 from urllib.parse import urljoin
 
@@ -618,65 +619,37 @@ def album_info(release: Dict) -> beets.autotag.hooks.AlbumInfo:
         )
 
     # We might find links to external sources (Discogs, Bandcamp, ...)
-    if any(
-        config["musicbrainz"]["external_ids"].get().values()
-    ) and release.get("url-relation-list"):
-        discogs_url, bandcamp_url, spotify_url = None, None, None
-        deezer_url, beatport_url, tidal_url = None, None, None
-        fetch_discogs, fetch_bandcamp, fetch_spotify = False, False, False
-        fetch_deezer, fetch_beatport, fetch_tidal = False, False, False
-
-        if config["musicbrainz"]["external_ids"]["discogs"].get():
-            fetch_discogs = True
-        if config["musicbrainz"]["external_ids"]["bandcamp"].get():
-            fetch_bandcamp = True
-        if config["musicbrainz"]["external_ids"]["spotify"].get():
-            fetch_spotify = True
-        if config["musicbrainz"]["external_ids"]["deezer"].get():
-            fetch_deezer = True
-        if config["musicbrainz"]["external_ids"]["beatport"].get():
-            fetch_beatport = True
-        if config["musicbrainz"]["external_ids"]["tidal"].get():
-            fetch_tidal = True
-
-        for url in release["url-relation-list"]:
-            if fetch_discogs and url["type"] == "discogs":
-                log.debug("Found link to Discogs release via MusicBrainz")
-                discogs_url = url["target"]
-            if fetch_bandcamp and "bandcamp.com" in url["target"]:
-                log.debug("Found link to Bandcamp release via MusicBrainz")
-                bandcamp_url = url["target"]
-            if fetch_spotify and "spotify.com" in url["target"]:
-                log.debug("Found link to Spotify album via MusicBrainz")
-                spotify_url = url["target"]
-            if fetch_deezer and "deezer.com" in url["target"]:
-                log.debug("Found link to Deezer album via MusicBrainz")
-                deezer_url = url["target"]
-            if fetch_beatport and "beatport.com" in url["target"]:
-                log.debug("Found link to Beatport release via MusicBrainz")
-                beatport_url = url["target"]
-            if fetch_tidal and "tidal.com" in url["target"]:
-                log.debug("Found link to Tidal release via MusicBrainz")
-                tidal_url = url["target"]
-
-        if discogs_url:
-            info.discogs_albumid = extract_discogs_id_regex(discogs_url)
-        if bandcamp_url:
-            info.bandcamp_album_id = bandcamp_url
-        if spotify_url:
+    external_ids = config["musicbrainz"]["external_ids"].get()
+    wanted_sources = {site for site, wanted in external_ids.items() if wanted}
+    if wanted_sources and (url_rels := release.get("url-relation-list")):
+        urls = {}
+
+        for source, url in product(wanted_sources, url_rels):
+            if f"{source}.com" in (target := url["target"]):
+                urls[source] = target
+                log.debug(
+                    "Found link to {} release via MusicBrainz",
+                    source.capitalize(),
+                )
+
+        if "discogs" in urls:
+            info.discogs_albumid = extract_discogs_id_regex(urls["discogs"])
+        if "bandcamp" in urls:
+            info.bandcamp_album_id = urls["bandcamp"]
+        if "spotify" in urls:
             info.spotify_album_id = MetadataSourcePlugin._get_id(
-                "album", spotify_url, spotify_id_regex
+                "album", urls["spotify"], spotify_id_regex
             )
-        if deezer_url:
+        if "deezer" in urls:
             info.deezer_album_id = MetadataSourcePlugin._get_id(
-                "album", deezer_url, deezer_id_regex
+                "album", urls["deezer"], deezer_id_regex
             )
-        if beatport_url:
+        if "beatport" in urls:
             info.beatport_album_id = MetadataSourcePlugin._get_id(
-                "album", beatport_url, beatport_id_regex
+                "album", urls["beatport"], beatport_id_regex
             )
-        if tidal_url:
-            info.tidal_album_id = tidal_url.split("/")[-1]
+        if "tidal" in urls:
+            info.tidal_album_id = urls["tidal"].split("/")[-1]
 
     extra_albumdatas = plugins.send("mb_album_extract", data=release)
     for extra_albumdata in extra_albumdatas:

beets/importer.py

@@ -60,15 +60,15 @@
 # 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",
     "deezer_album_id",
     "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 @@ def add(self, lib):
         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):

beets/test/helper.py

@@ -964,6 +964,7 @@ def _make_album_match(self, artist, album, tracks, distance=0, missing=0):
             artist_id="artistid" + id,
             albumtype="soundtrack",
             data_source="match_source",
+            bandcamp_album_id="bc_url",
         )
 
 

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.

test/test_importer.py

@@ -1654,6 +1654,12 @@ def test_reimported_item_preserves_art(self):
         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"