diff --git a/beets/autotag/mb.py b/beets/autotag/mb.py index 7d7932faf..f380cd033 100644 --- a/beets/autotag/mb.py +++ b/beets/autotag/mb.py @@ -466,7 +466,7 @@ def album_info(release: Dict) -> beets.autotag.hooks.AlbumInfo: if release['release-group']['secondary-type-list']: for sec_type in release['release-group']['secondary-type-list']: albumtypes.append(sec_type.lower()) - info.albumtypes = '; '.join(albumtypes) + info.albumtypes = albumtypes # Release events. info.country, release_date = _preferred_release_event(release) diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index 40f6a0805..8c8bfa3f6 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -208,6 +208,27 @@ class String(Type): return self.model_type(value) +class DelimitedString(String): + """A list of Unicode strings, represented in-database by a single string + containing delimiter-separated values. + """ + model_type = list + + def __init__(self, delimiter): + self.delimiter = delimiter + + def format(self, value): + return self.delimiter.join(value) + + def parse(self, string): + if not string: + return [] + return string.split(self.delimiter) + + def to_sql(self, model_value): + return self.delimiter.join(model_value) + + class Boolean(Type): """A boolean type. """ @@ -231,3 +252,4 @@ FLOAT = Float() NULL_FLOAT = NullFloat() STRING = String() BOOLEAN = Boolean() +SEMICOLON_SPACE_DSV = DelimitedString(delimiter='; ') diff --git a/beets/library.py b/beets/library.py index c05dcda18..9d5219b18 100644 --- a/beets/library.py +++ b/beets/library.py @@ -504,7 +504,7 @@ class Item(LibModel): 'mb_releasetrackid': types.STRING, 'trackdisambig': types.STRING, 'albumtype': types.STRING, - 'albumtypes': types.STRING, + 'albumtypes': types.SEMICOLON_SPACE_DSV, 'label': types.STRING, 'acoustid_fingerprint': types.STRING, 'acoustid_id': types.STRING, @@ -1064,7 +1064,7 @@ class Album(LibModel): 'mb_albumid': types.STRING, 'mb_albumartistid': types.STRING, 'albumtype': types.STRING, - 'albumtypes': types.STRING, + 'albumtypes': types.SEMICOLON_SPACE_DSV, 'label': types.STRING, 'mb_releasegroupid': types.STRING, 'asin': types.STRING, diff --git a/beetsplug/albumtypes.py b/beetsplug/albumtypes.py index 47f8dc64e..b54e802e6 100644 --- a/beetsplug/albumtypes.py +++ b/beetsplug/albumtypes.py @@ -55,7 +55,7 @@ class AlbumTypesPlugin(BeetsPlugin): bracket_r = '' res = '' - albumtypes = item.albumtypes.split('; ') + albumtypes = item.albumtypes is_va = item.mb_albumartistid == VARIOUS_ARTISTS_ID for type in types: if type[0] in albumtypes and type[1]: diff --git a/docs/changelog.rst b/docs/changelog.rst index fa24b3bc4..5d11fc145 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -147,6 +147,13 @@ Bug fixes: :bug:`4561` :bug:`4600` * Fix issue where deletion of flexible fields on an album doesn't cascade to items :bug:`4662` +* Fix issue where ``beet write`` continuosly retags the ``albumtypes`` metadata + field in files. Additionally broken data could have been added to the library + when the tag was read from file back into the library using ``beet update``. + It is required for all users to **check if such broken data is present in the + library**. Following the instructions `described here + `_, a + sanity check and potential fix is easily possible. :bug:`4528` For packagers: diff --git a/docs/plugins/albumtypes.rst b/docs/plugins/albumtypes.rst index 7a1a08f95..bf736abca 100644 --- a/docs/plugins/albumtypes.rst +++ b/docs/plugins/albumtypes.rst @@ -11,6 +11,11 @@ you can use in your path formats or elsewhere. .. _MusicBrainz documentation: https://musicbrainz.org/doc/Release_Group/Type +A bug introduced in beets 1.6.0 could have possibly imported broken data into +the ``albumtypes`` library field. Please follow the instructions `described +here `_ for +a sanity check and potential fix. :bug:`4528` + Configuration ------------- diff --git a/test/test_albumtypes.py b/test/test_albumtypes.py index 91808553d..3d329dd7b 100644 --- a/test/test_albumtypes.py +++ b/test/test_albumtypes.py @@ -106,6 +106,6 @@ class AlbumTypesPluginTest(unittest.TestCase, TestHelper): def _create_album(self, album_types: [str], artist_id: str = 0): return self.add_album( - albumtypes='; '.join(album_types), + albumtypes=album_types, mb_albumartistid=artist_id ) diff --git a/test/test_ui.py b/test/test_ui.py index 86c40d204..d3ce4a560 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -701,27 +701,30 @@ class UpdateTest(_common.TestCase): item = self.lib.items().get() self.assertEqual(item.title, 'full') - @unittest.expectedFailure def test_multivalued_albumtype_roundtrip(self): # https://github.com/beetbox/beets/issues/4528 # albumtypes is empty for our test fixtures, so populate it first album = self.album - # setting albumtypes does not set albumtype currently... - # FIXME: When actually fixing the issue 4528, consider whether this - # should be set to "album" or ["album"] - album.albumtype = "album" - album.albumtypes = "album" + correct_albumtypes = ["album", "live"] + + # Setting albumtypes does not set albumtype, currently. + # Using x[0] mirrors https://github.com/beetbox/mediafile/blob/057432ad53b3b84385e5582f69f44dc00d0a725d/mediafile.py#L1928 # noqa: E501 + correct_albumtype = correct_albumtypes[0] + + album.albumtype = correct_albumtype + album.albumtypes = correct_albumtypes album.try_sync(write=True, move=False) album.load() - albumtype_before = album.albumtype - self.assertEqual(albumtype_before, "album") + self.assertEqual(album.albumtype, correct_albumtype) + self.assertEqual(album.albumtypes, correct_albumtypes) self._update() album.load() - self.assertEqual(albumtype_before, album.albumtype) + self.assertEqual(album.albumtype, correct_albumtype) + self.assertEqual(album.albumtypes, correct_albumtypes) class PrintTest(_common.TestCase):