diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index cf58c82d5..9c0bdf99c 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -117,28 +117,35 @@ class MBSyncPlugin(BeetsPlugin): album_formatted) continue - # Map recording MBIDs to their information. Recordings can appear - # multiple times on a release, so each MBID maps to a list of - # TrackInfo objects. + # Map release track and recording MBIDs to their information. + # Recordings can appear multiple times on a release, so each MBID + # maps to a list of TrackInfo objects. + releasetrack_index = dict() track_index = defaultdict(list) for track_info in album_info.tracks: + releasetrack_index[track_info.release_track_id] = track_info track_index[track_info.track_id].append(track_info) - # Construct a track mapping according to MBIDs. This should work - # for albums that have missing or extra tracks. If there are - # multiple copies of a recording, they are disambiguated using - # their disc and track number. + # Construct a track mapping according to MBIDs (release track MBIDs + # first, if available, and recording MBIDs otherwise). This should + # work for albums that have missing or extra tracks. mapping = {} for item in items: - candidates = track_index[item.mb_trackid] - if len(candidates) == 1: - mapping[item] = candidates[0] + if item.mb_releasetrackid and \ + item.mb_releasetrackid in releasetrack_index: + mapping[item] = releasetrack_index[item.mb_releasetrackid] else: - for c in candidates: - if (c.medium_index == item.track and - c.medium == item.disc): - mapping[item] = c - break + candidates = track_index[item.mb_trackid] + if len(candidates) == 1: + mapping[item] = candidates[0] + else: + # If there are multiple copies of a recording, they are + # disambiguated using their disc and track number. + for c in candidates: + if (c.medium_index == item.track and + c.medium == item.disc): + mapping[item] = c + break # Apply. self._log.debug(u'applying changes to {}', album_formatted) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4a51b2f9c..c87935668 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -113,6 +113,8 @@ Fixes: main track list. Thanks to :user:`jdetrey`. :bug:`1638` * :doc:`/plugins/keyfinder`: Avoid a crash when trying to process unmatched tracks. :bug:`2537` +* In the ``mbsync`` plugin, support MusicBrainz recording ID changes, relying + on release track IDs instead. Thanks to :user:`jdetrey`. :bug:`1234` For developers: diff --git a/test/helper.py b/test/helper.py index 5b3bec8b5..21346312c 100644 --- a/test/helper.py +++ b/test/helper.py @@ -557,16 +557,16 @@ class TestImportSession(importer.ImportSession): task.should_merge_duplicates = True -def generate_album_info(album_id, track_ids): +def generate_album_info(album_id, track_values): """Return `AlbumInfo` populated with mock data. Sets the album info's `album_id` field is set to the corresponding - argument. For each value in `track_ids` the `TrackInfo` from - `generate_track_info` is added to the album info's `tracks` field. + argument. For each pair (`id`, `values`) in `track_values` the `TrackInfo` + from `generate_track_info` is added to the album info's `tracks` field. Most other fields of the album and track info are set to "album info" and "track info", respectively. """ - tracks = [generate_track_info(id) for id in track_ids] + tracks = [generate_track_info(id, values) for id, values in track_values] album = AlbumInfo( album_id=u'album info', album=u'album info', diff --git a/test/test_mbsync.py b/test/test_mbsync.py index 411af8d0a..a48844384 100644 --- a/test/test_mbsync.py +++ b/test/test_mbsync.py @@ -41,7 +41,10 @@ class MbsyncCliTest(unittest.TestCase, TestHelper): @patch('beets.autotag.hooks.track_for_mbid') def test_update_library(self, track_for_mbid, album_for_mbid): album_for_mbid.return_value = \ - generate_album_info('album id', ['track id']) + generate_album_info( + 'album id', + [('track id', {'release_track_id': u'release track id'})] + ) track_for_mbid.return_value = \ generate_track_info(u'singleton track id', {'title': u'singleton info'}) @@ -49,7 +52,8 @@ class MbsyncCliTest(unittest.TestCase, TestHelper): album_item = Item( album=u'old title', mb_albumid=u'album id', - mb_trackid=u'track id', + mb_trackid=u'old track id', + mb_releasetrackid=u'release track id', path='' ) album = self.lib.add_album([album_item]) @@ -68,6 +72,7 @@ class MbsyncCliTest(unittest.TestCase, TestHelper): album_item.load() self.assertEqual(album_item.title, u'track info') + self.assertEqual(album_item.mb_trackid, u'track id') album.load() self.assertEqual(album.album, u'album info')