Merge pull request #2917 from jdetrey/fix/1234

mbsync: Map tracks using release track MBIDs instead of recording MBIDs

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)

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:

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',

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')