Merge pull request #3028 from arogl/master

mbsync no longer queries MusicBrainz when the either the mb_albumid or mb_trackid field is invalid

beetsplug/mbsync.py

@@ -22,6 +22,10 @@ from beets import autotag, library, ui, util
 from beets.autotag import hooks
 from collections import defaultdict
 
+import re
+
+MBID_REGEX = r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}"
+
 
 def apply_item_changes(lib, item, move, pretend, write):
     """Store, move and write the item according to the arguments.
@@ -82,6 +86,12 @@ class MBSyncPlugin(BeetsPlugin):
                                item_formatted)
                 continue
 
+            # Do we have a valid MusicBrainz track ID?
+            if not re.match(MBID_REGEX, item.mb_trackid):
+                self._log.info(u'Skipping singleton with invalid mb_trackid:' +
+                               ' {0}', item_formatted)
+                continue
+
             # Get the MusicBrainz recording info.
             track_info = hooks.track_for_mbid(item.mb_trackid)
             if not track_info:
@@ -109,6 +119,12 @@ class MBSyncPlugin(BeetsPlugin):
 
             items = list(a.items())
 
+            # Do we have a valid MusicBrainz album ID?
+            if not re.match(MBID_REGEX, a.mb_albumid):
+                self._log.info(u'Skipping album with invalid mb_albumid: {0}',
+                               album_formatted)
+                continue
+
             # Get the MusicBrainz album information.
             album_info = hooks.album_for_mbid(a.mb_albumid)
             if not album_info:

docs/changelog.rst

@@ -43,6 +43,15 @@ New features:
   disambiguation comment. A new ``releasegroupdisambig`` field has been added.
   :bug:`3024`
 
+Changes:
+
+* :doc:`/plugins/mbsync` no longer queries MusicBrainz when either the
+  ``mb_albumid`` or ``mb_trackid`` field is invalid
+  See also the discussion on Google Groups_
+  Thanks to :user:`arogl`.
+
+.. _Groups: https://groups.google.com/forum/#!searchin/beets-users/mbsync|sort:date/beets-users/iwCF6bNdh9A/i1xl4Gx8BQAJ
+
 Fixes:
 
 * A new importer option, :ref:`ignore_data_tracks`, lets you skip audio tracks

test/test_mbsync.py

@@ -51,7 +51,7 @@ class MbsyncCliTest(unittest.TestCase, TestHelper):
 
         album_item = Item(
             album=u'old title',
-            mb_albumid=u'album id',
+            mb_albumid=u'81ae60d4-5b75-38df-903a-db2cfa51c2c6',
             mb_trackid=u'old track id',
             mb_releasetrackid=u'release track id',
             path=''
@@ -60,13 +60,14 @@ class MbsyncCliTest(unittest.TestCase, TestHelper):
 
         item = Item(
             title=u'old title',
-            mb_trackid=u'singleton track id',
+            mb_trackid=u'b8c2cf90-83f9-3b5f-8ccd-31fb866fcf37',
             path='',
         )
         self.lib.add(item)
 
         with capture_log() as logs:
             self.run_command('mbsync')
+
         self.assertIn('Sending event: albuminfo_received', logs)
         self.assertIn('Sending event: trackinfo_received', logs)
 
@@ -135,6 +136,63 @@ class MbsyncCliTest(unittest.TestCase, TestHelper):
         e = u"mbsync: Skipping singleton with no mb_trackid: 'old title'"
         self.assertEqual(e, logs[0])
 
+    def test_message_when_invalid(self):
+        config['format_item'] = u'$artist - $album - $title'
+        config['format_album'] = u'$albumartist - $album'
+
+        # Test album with invalid mb_albumid.
+        # The default format for an album include $albumartist so
+        # set that here, too.
+        album_invalid = Item(
+            albumartist=u'album info',
+            album=u'album info',
+            mb_albumid=u'a1b2c3d4',
+            path=''
+        )
+        self.lib.add_album([album_invalid])
+
+        # default format
+        with capture_log('beets.mbsync') as logs:
+            self.run_command('mbsync')
+        e = u'mbsync: Skipping album with invalid mb_albumid: ' + \
+            u'album info - album info'
+        self.assertEqual(e, logs[0])
+
+        # custom format
+        with capture_log('beets.mbsync') as logs:
+            self.run_command('mbsync', '-f', "'$album'")
+        e = u"mbsync: Skipping album with invalid mb_albumid: 'album info'"
+        self.assertEqual(e, logs[0])
+
+        # restore the config
+        config['format_item'] = u'$artist - $album - $title'
+        config['format_album'] = u'$albumartist - $album'
+
+        # Test singleton with invalid mb_trackid.
+        # The default singleton format includes $artist and $album
+        # so we need to stub them here
+        item_invalid = Item(
+            artist=u'album info',
+            album=u'album info',
+            title=u'old title',
+            mb_trackid=u'a1b2c3d4',
+            path='',
+        )
+        self.lib.add(item_invalid)
+
+        # default format
+        with capture_log('beets.mbsync') as logs:
+            self.run_command('mbsync')
+        e = u'mbsync: Skipping singleton with invalid mb_trackid: ' + \
+            u'album info - album info - old title'
+        self.assertEqual(e, logs[0])
+
+        # custom format
+        with capture_log('beets.mbsync') as logs:
+            self.run_command('mbsync', '-f', "'$title'")
+        e = u"mbsync: Skipping singleton with invalid mb_trackid: 'old title'"
+        self.assertEqual(e, logs[0])
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)