Use None as default values for multi-valued fields in hooks.Info

beets/autotag/hooks.py

@@ -97,15 +97,15 @@ def __init__(
         self.artist = artist
         self.artist_credit = artist_credit
         self.artist_id = artist_id
-        self.artists = artists or []
-        self.artists_credit = artists_credit or []
-        self.artists_ids = artists_ids or []
+        self.artists = artists
+        self.artists_credit = artists_credit
+        self.artists_ids = artists_ids
         self.artist_sort = artist_sort
-        self.artists_sort = artists_sort or []
+        self.artists_sort = artists_sort
         self.data_source = data_source
         self.data_url = data_url
         self.genre = None
-        self.genres = genres or []
+        self.genres = genres
         self.media = media
         self.update(kwargs)
 
@@ -160,7 +160,7 @@ def __init__(
         self.albumdisambig = albumdisambig
         self.albumstatus = albumstatus
         self.albumtype = albumtype
-        self.albumtypes = albumtypes or []
+        self.albumtypes = albumtypes
         self.asin = asin
         self.barcode = barcode
         self.catalognum = catalognum

docs/changelog.rst

@@ -44,6 +44,9 @@ Bug fixes
 - :doc:`plugins/zero`: When the ``omit_single_disc`` option is set,
   ``disctotal`` is zeroed alongside ``disc``.
 - :doc:`plugins/fetchart`: Prevent deletion of configured fallback cover art
+- In autotagging, initialise empty multi-valued fields with ``None`` instead of
+  empty list, which caused beets to overwrite existing metadata with empty list
+  values instead of leaving them unchanged. :bug:`6403`
 
 For plugin developers
 ~~~~~~~~~~~~~~~~~~~~~

test/autotag/test_autotag.py

@@ -0,0 +1,317 @@
+# This file is part of beets.
+# Copyright 2016, Adrian Sampson.
+#
+# 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.
+
+"""Tests for autotagging functionality."""
+
+import operator
+
+import pytest
+
+from beets import autotag
+from beets.autotag import AlbumInfo, TrackInfo, correct_list_fields, match
+from beets.library import Item
+from beets.test.helper import BeetsTestCase
+
+
+class TestAssignment:
+    A = "one"
+    B = "two"
+    C = "three"
+
+    @pytest.fixture(autouse=True)
+    def config(self, config):
+        config["match"]["track_length_grace"] = 10
+        config["match"]["track_length_max"] = 30
+
+    @pytest.mark.parametrize(
+        # 'expected' is a tuple of expected (mapping, extra_items, extra_tracks)
+        "item_titles, track_titles, expected",
+        [
+            # items ordering gets corrected
+            ([A, C, B], [A, B, C], ({A: A, B: B, C: C}, [], [])),
+            # unmatched tracks are returned as 'extra_tracks'
+            # the first track is unmatched
+            ([B, C], [A, B, C], ({B: B, C: C}, [], [A])),
+            # the middle track is unmatched
+            ([A, C], [A, B, C], ({A: A, C: C}, [], [B])),
+            # the last track is unmatched
+            ([A, B], [A, B, C], ({A: A, B: B}, [], [C])),
+            # unmatched items are returned as 'extra_items'
+            ([A, C, B], [A, C], ({A: A, C: C}, [B], [])),
+        ],
+    )
+    def test_assign_tracks(self, item_titles, track_titles, expected):
+        expected_mapping, expected_extra_items, expected_extra_tracks = expected
+
+        items = [Item(title=title) for title in item_titles]
+        tracks = [TrackInfo(title=title) for title in track_titles]
+
+        item_info_pairs, extra_items, extra_tracks = match.assign_items(
+            items, tracks
+        )
+
+        assert (
+            {i.title: t.title for i, t in item_info_pairs},
+            [i.title for i in extra_items],
+            [t.title for t in extra_tracks],
+        ) == (expected_mapping, expected_extra_items, expected_extra_tracks)
+
+    def test_order_works_when_track_names_are_entirely_wrong(self):
+        # A real-world test case contributed by a user.
+        def item(i, length):
+            return Item(
+                artist="ben harper",
+                album="burn to shine",
+                title=f"ben harper - Burn to Shine {i}",
+                track=i,
+                length=length,
+            )
+
+        items = []
+        items.append(item(1, 241.37243007106997))
+        items.append(item(2, 342.27781704375036))
+        items.append(item(3, 245.95070222338137))
+        items.append(item(4, 472.87662515485437))
+        items.append(item(5, 279.1759535763187))
+        items.append(item(6, 270.33333768012))
+        items.append(item(7, 247.83435613222923))
+        items.append(item(8, 216.54504531525072))
+        items.append(item(9, 225.72775379800484))
+        items.append(item(10, 317.7643606963552))
+        items.append(item(11, 243.57001238834192))
+        items.append(item(12, 186.45916150485752))
+
+        def info(index, title, length):
+            return TrackInfo(title=title, length=length, index=index)
+
+        trackinfo = []
+        trackinfo.append(info(1, "Alone", 238.893))
+        trackinfo.append(info(2, "The Woman in You", 341.44))
+        trackinfo.append(info(3, "Less", 245.59999999999999))
+        trackinfo.append(info(4, "Two Hands of a Prayer", 470.49299999999999))
+        trackinfo.append(info(5, "Please Bleed", 277.86599999999999))
+        trackinfo.append(info(6, "Suzie Blue", 269.30599999999998))
+        trackinfo.append(info(7, "Steal My Kisses", 245.36000000000001))
+        trackinfo.append(info(8, "Burn to Shine", 214.90600000000001))
+        trackinfo.append(info(9, "Show Me a Little Shame", 224.0929999999999))
+        trackinfo.append(info(10, "Forgiven", 317.19999999999999))
+        trackinfo.append(info(11, "Beloved One", 243.733))
+        trackinfo.append(info(12, "In the Lord's Arms", 186.13300000000001))
+
+        expected = list(zip(items, trackinfo)), [], []
+
+        assert match.assign_items(items, trackinfo) == expected
+
+
+class ApplyTest(BeetsTestCase):
+    def _apply(self, per_disc_numbering=False, artist_credit=False):
+        info = self.info
+        item_info_pairs = list(zip(self.items, info.tracks))
+        self.config["per_disc_numbering"] = per_disc_numbering
+        self.config["artist_credit"] = artist_credit
+        autotag.apply_metadata(self.info, item_info_pairs)
+
+    def setUp(self):
+        super().setUp()
+
+        self.items = [Item(), Item()]
+        self.info = AlbumInfo(
+            tracks=[
+                TrackInfo(
+                    title="title",
+                    track_id="dfa939ec-118c-4d0f-84a0-60f3d1e6522c",
+                    medium=1,
+                    medium_index=1,
+                    medium_total=1,
+                    index=1,
+                    artist="trackArtist",
+                    artist_credit="trackArtistCredit",
+                    artists_credit=["trackArtistCredit"],
+                    artist_sort="trackArtistSort",
+                    artists_sort=["trackArtistSort"],
+                ),
+                TrackInfo(
+                    title="title2",
+                    track_id="40130ed1-a27c-42fd-a328-1ebefb6caef4",
+                    medium=2,
+                    medium_index=1,
+                    index=2,
+                    medium_total=1,
+                ),
+            ],
+            artist="albumArtist",
+            artists=["albumArtist", "albumArtist2"],
+            album="album",
+            album_id="7edb51cb-77d6-4416-a23c-3a8c2994a2c7",
+            artist_id="a6623d39-2d8e-4f70-8242-0a9553b91e50",
+            artists_ids=None,
+            artist_credit="albumArtistCredit",
+            artists_credit=["albumArtistCredit1", "albumArtistCredit2"],
+            artist_sort=None,
+            artists_sort=["albumArtistSort", "albumArtistSort2"],
+            albumtype="album",
+            va=True,
+            mediums=2,
+            data_source="MusicBrainz",
+            year=2013,
+            month=12,
+            day=18,
+            genres=["Rock", "Pop"],
+        )
+
+        common_expected = {
+            "album": "album",
+            "albumartist_credit": "albumArtistCredit",
+            "albumartist_sort": "",
+            "albumartist": "albumArtist",
+            "albumartists": ["albumArtist", "albumArtist2"],
+            "albumartists_credit": [
+                "albumArtistCredit1",
+                "albumArtistCredit2",
+            ],
+            "albumartists_sort": ["albumArtistSort", "albumArtistSort2"],
+            "albumtype": "album",
+            "albumtypes": ["album"],
+            "comp": True,
+            "disctotal": 2,
+            "mb_albumartistid": "a6623d39-2d8e-4f70-8242-0a9553b91e50",
+            "mb_albumartistids": ["a6623d39-2d8e-4f70-8242-0a9553b91e50"],
+            "mb_albumid": "7edb51cb-77d6-4416-a23c-3a8c2994a2c7",
+            "mb_artistid": "a6623d39-2d8e-4f70-8242-0a9553b91e50",
+            "mb_artistids": ["a6623d39-2d8e-4f70-8242-0a9553b91e50"],
+            "tracktotal": 2,
+            "year": 2013,
+            "month": 12,
+            "day": 18,
+            "genres": ["Rock", "Pop"],
+        }
+
+        self.expected_tracks = [
+            {
+                **common_expected,
+                "artist": "trackArtist",
+                "artists": ["albumArtist", "albumArtist2"],
+                "artist_credit": "trackArtistCredit",
+                "artist_sort": "trackArtistSort",
+                "artists_credit": ["trackArtistCredit"],
+                "artists_sort": ["trackArtistSort"],
+                "disc": 1,
+                "mb_trackid": "dfa939ec-118c-4d0f-84a0-60f3d1e6522c",
+                "title": "title",
+                "track": 1,
+            },
+            {
+                **common_expected,
+                "artist": "albumArtist",
+                "artists": ["albumArtist", "albumArtist2"],
+                "artist_credit": "albumArtistCredit",
+                "artist_sort": "",
+                "artists_credit": [
+                    "albumArtistCredit1",
+                    "albumArtistCredit2",
+                ],
+                "artists_sort": ["albumArtistSort", "albumArtistSort2"],
+                "disc": 2,
+                "mb_trackid": "40130ed1-a27c-42fd-a328-1ebefb6caef4",
+                "title": "title2",
+                "track": 2,
+            },
+        ]
+
+    def test_autotag_items(self):
+        self._apply()
+
+        keys = self.expected_tracks[0].keys()
+        get_values = operator.itemgetter(*keys)
+
+        applied_data = [
+            dict(zip(keys, get_values(dict(i)))) for i in self.items
+        ]
+
+        assert applied_data == self.expected_tracks
+
+    def test_per_disc_numbering(self):
+        self._apply(per_disc_numbering=True)
+
+        assert self.items[0].track == 1
+        assert self.items[1].track == 1
+        assert self.items[0].tracktotal == 1
+        assert self.items[1].tracktotal == 1
+
+    def test_artist_credit_prefers_artist_over_albumartist_credit(self):
+        self.info.tracks[0].update(artist="oldArtist", artist_credit=None)
+
+        self._apply(artist_credit=True)
+
+        assert self.items[0].artist == "oldArtist"
+
+    def test_artist_credit_falls_back_to_albumartist(self):
+        self.info.artist_credit = None
+
+        self._apply(artist_credit=True)
+
+        assert self.items[1].artist == "albumArtist"
+
+    def test_date_only_zeroes_month_and_day(self):
+        self.items = [Item(year=1, month=2, day=3)]
+        self.info.update(year=2013, month=None, day=None)
+
+        self._apply()
+
+        assert self.items[0].year == 2013
+        assert self.items[0].month == 0
+        assert self.items[0].day == 0
+
+    def test_missing_date_applies_nothing(self):
+        self.items = [Item(year=1, month=2, day=3)]
+        self.info.update(year=None, month=None, day=None)
+
+        self._apply()
+
+        assert self.items[0].year == 1
+        assert self.items[0].month == 2
+        assert self.items[0].day == 3
+
+
+@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_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)
+
+    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]

test/plugins/test_musicbrainz.py

@@ -539,7 +539,7 @@ def test_no_genres(self):
         config["musicbrainz"]["genres"] = False
         release = self._make_release()
         d = self.mb.album_info(release)
-        assert d.genres == []
+        assert d.genres is None
 
     def test_ignored_media(self):
         config["match"]["ignored_media"] = ["IGNORED1", "IGNORED2"]