From 550a9a82b16844253a035dedf75b33e4a4b5ec74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= <snejus@protonmail.com>
Date: Sat, 14 Dec 2024 21:39:47 +0000
Subject: [PATCH] Fix mb_artistid, mb_albumartistid, albumtype diff issue

---
 beets/autotag/__init__.py | 39 +++++++++++++++++++++++++++++++++++++++
 docs/changelog.rst        |  8 ++++++++
 test/test_autotag.py      | 39 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py
index 70814c1827..ca20204703 100644
--- a/beets/autotag/__init__.py
+++ b/beets/autotag/__init__.py
@@ -146,6 +146,43 @@ def apply_album_metadata(album_info: AlbumInfo, album: Album):
     _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]
+
+
 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.
@@ -250,6 +287,8 @@ 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
 
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 34c1e6413a..9fb3b9e3ff 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -19,6 +19,14 @@ Bug fixes:
   have before the introduction of Poetry.
   :bug:`5531`
   :bug:`5526`
+* :ref:`write-cmd`: Fix the issue where for certain files differences in
+  ``mb_artistid``, ``mb_albumartistid`` and ``albumtype`` fields are shown on
+  every attempt to write tags. Note: your music needs to be reimported with
+  ``beet import -LI`` or synchronised with ``beet mbsync`` in order to fix
+  this!
+  :bug:`5265`
+  :bug:`5371`
+  :bug:`4715`
 
 For packagers:
 
diff --git a/test/test_autotag.py b/test/test_autotag.py
index 6e362f2f4b..1854bd654b 100644
--- a/test/test_autotag.py
+++ b/test/test_autotag.py
@@ -20,7 +20,12 @@
 import pytest
 
 from beets import autotag, config
-from beets.autotag import AlbumInfo, TrackInfo, match
+from beets.autotag import (
+    AlbumInfo,
+    TrackInfo,
+    ensure_consistent_list_fields,
+    match,
+)
 from beets.autotag.hooks import Distance, string_dist
 from beets.library import Item
 from beets.test.helper import BeetsTestCase
@@ -1040,3 +1045,35 @@ def test_ampersand_expansion(self):
     def test_accented_characters(self):
         dist = string_dist("\xe9\xe1\xf1", "ean")
         assert dist == 0.0
+
+
+@pytest.mark.parametrize(
+    "single_field,list_field",
+    [
+        ("mb_artistid", "mb_artistids"),
+        ("mb_albumartistid", "mb_albumartistids"),
+        ("albumtype", "albumtypes"),
+    ],
+)
+@pytest.mark.parametrize(
+    "single_value,list_value",
+    [
+        (None, []),
+        (None, ["1"]),
+        (None, ["1", "2"]),
+        ("1", []),
+        ("1", ["1"]),
+        ("1", ["1", "2"]),
+        ("1", ["2", "1"]),
+    ],
+)
+def test_ensure_consistent_list_fields(
+    single_field, list_field, single_value, list_value
+):
+    data = {single_field: single_value, list_field: list_value}
+    item = Item(**data)
+
+    ensure_consistent_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]