Ensure that list fields are corrected for album metadata too

beets/autotag/__init__.py

@@ -18,7 +18,7 @@
 from typing import Union
 
 from beets import config, logging
-from beets.library import Album, Item
+from beets.library import Album, Item, LibModel
 
 # Parts of external interface.
 from .hooks import AlbumInfo, AlbumMatch, Distance, TrackInfo, TrackMatch
@@ -119,6 +119,50 @@ def _apply_metadata(
         db_obj[field] = value
 
 
+def correct_list_fields(m: LibModel) -> None:
+    """Synchronise single and list values for the list fields that we use.
+
+    That is, ensure the same value in the single field and the first element
+    in the list.
+
+    For context, the value we set as, say, ``mb_artistid`` is simply ignored:
+    Under the current :class:`MediaFile` implementation, fields ``albumtype``,
+    ``mb_artistid`` and ``mb_albumartistid`` are mapped to the first element of
+    ``albumtypes``, ``mb_artistids`` and ``mb_albumartistids`` respectively.
+
+    This means setting ``mb_artistid`` has no effect. However, beets
+    functionality still assumes that ``mb_artistid`` is independent and stores
+    its value in the database. If ``mb_artistid`` != ``mb_artistids[0]``,
+    ``beet write`` command thinks that ``mb_artistid`` is modified and tries to
+    update the field in the file. Of course nothing happens, so the same diff
+    is shown every time the command is run.
+
+    We can avoid this issue by ensuring that ``mb_artistid`` has the same value
+    as ``mb_artistids[0]``, and that's what this function does.
+
+    Note: :class:`Album` model does not have ``mb_artistids`` and
+    ``mb_albumartistids`` fields therefore we need to check for their presence.
+    """
+    if atype := m.albumtype:
+        m.albumtypes = list(dict.fromkeys([atype, *m.albumtypes]))
+    elif atypes := m.albumtypes:
+        m.albumtype = atypes[0]
+
+    if hasattr(m, "mb_artistids"):
+        if aid := m.mb_artistid:
+            m.mb_artistids = list(dict.fromkeys([aid, *m.mb_artistids]))
+        elif aids := m.mb_artistids:
+            m.mb_artistid = aids[0]
+
+    if hasattr(m, "mb_albumartistids"):
+        if aaid := m.mb_albumartistid:
+            m.mb_albumartistids = list(
+                dict.fromkeys([aaid, *m.mb_albumartistids])
+            )
+        elif aaids := m.mb_albumartistids:
+            m.mb_albumartistid = aaids[0]
+
+
 def apply_item_metadata(item: Item, track_info: TrackInfo):
     """Set an item's metadata from its matched TrackInfo object."""
     item.artist = track_info.artist
@@ -136,6 +180,7 @@ def apply_item_metadata(item: Item, track_info: TrackInfo):
         item.mb_artistids = track_info.artists_ids
 
     _apply_metadata(track_info, item)
+    correct_list_fields(item)
 
     # At the moment, the other metadata is left intact (including album
     # and track number). Perhaps these should be emptied?
@@ -144,43 +189,7 @@ def apply_item_metadata(item: Item, track_info: TrackInfo):
 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 ensure_consistent_list_fields(i: Item) -> None:
-    """Synchronise single and list values for the list fields that we use.
-
-    That is, ensure the same value in the single field and the first element
-    in the list.
-
-    For context, the value we set as, say, ``mb_artistid`` is simply ignored:
-    Under the current :class:`MediaFile` implementation, fields ``albumtype``,
-    ``mb_artistid`` and ``mb_albumartistid`` are mapped to the first element of
-    ``albumtypes``, ``mb_artistids`` and ``mb_albumartistids`` respectively.
-
-    This means setting ``mb_artistid`` has no effect. However, beets
-    functionality still assumes that ``mb_artistid`` is independent and stores
-    its value in the database. If ``mb_artistid`` != ``mb_artistids[0]``,
-    ``beet write`` command thinks that ``mb_artistid`` is modified and tries to
-    update the field in the file. Of course nothing happens, so the same diff
-    is shown every time the command is run.
-
-    We can avoid this issue by ensuring that ``mb_artistid`` has the same value
-    as ``mb_artistids[0]``, and that's what this function does.
-    """
-    if aid := i.mb_artistid:
-        i.mb_artistids = list(dict.fromkeys([aid, *i.mb_artistids]))
-    elif aids := i.mb_artistids:
-        i.mb_artistid = aids[0]
-
-    if aaid := i.mb_albumartistid:
-        i.mb_albumartistids = list(dict.fromkeys([aaid, *i.mb_albumartistids]))
-    elif aaids := i.mb_albumartistids:
-        i.mb_albumartistid = aaids[0]
-
-    if atype := i.albumtype:
-        i.albumtypes = list(dict.fromkeys([atype, *i.albumtypes]))
-    elif atypes := i.albumtypes:
-        i.albumtype = atypes[0]
+    correct_list_fields(album)
 
 
 def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]):
@@ -287,8 +296,6 @@ def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]):
         item.mb_albumartistids = album_info.artists_ids
         item.mb_releasegroupid = album_info.releasegroup_id
 
-        ensure_consistent_list_fields(item)
-
         # Compilation flag.
         item.comp = album_info.va
 
@@ -306,3 +313,5 @@ def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]):
             item,
             nullable_fields=config["overwrite_null"]["track"].as_str_seq(),
         )
+
+        correct_list_fields(item)

test/test_autotag.py

@@ -20,12 +20,7 @@
 import pytest
 
 from beets import autotag, config
-from beets.autotag import (
-    AlbumInfo,
-    TrackInfo,
-    ensure_consistent_list_fields,
-    match,
-)
+from beets.autotag import AlbumInfo, TrackInfo, correct_list_fields, match
 from beets.autotag.hooks import Distance, string_dist
 from beets.library import Item
 from beets.test.helper import BeetsTestCase
@@ -1067,13 +1062,14 @@ def test_accented_characters(self):
         ("1", ["2", "1"]),
     ],
 )
-def test_ensure_consistent_list_fields(
+def test_correct_list_fields(
     single_field, list_field, single_value, list_value
 ):
+    """Ensure that the first value in a list field matches the single field."""
     data = {single_field: single_value, list_field: list_value}
     item = Item(**data)
 
-    ensure_consistent_list_fields(item)
+    correct_list_fields(item)
 
     single_val, list_val = item[single_field], item[list_field]
     assert (not single_val and not list_val) or single_val == list_val[0]