From 919f3f9c349dfac198ac4e5d01348095125f1d3f Mon Sep 17 00:00:00 2001 From: Andrew Rogl Date: Sun, 9 Sep 2018 15:20:26 +1000 Subject: [PATCH 1/6] mbsync no longer queries MusicBrainz when the either the mb_albumid or mb_trackid field is invalid --- beetsplug/mbsync.py | 159 ++++++++++++++++++++++++-------------------- docs/changelog.rst | 7 ++ test/test_mbsync.py | 62 ++++++++++++++++- 3 files changed, 153 insertions(+), 75 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index 1764a1779..fc763a365 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -82,19 +82,25 @@ class MBSyncPlugin(BeetsPlugin): item_formatted) continue - # Get the MusicBrainz recording info. - track_info = hooks.track_for_mbid(item.mb_trackid) - if not track_info: - self._log.info(u'Recording ID not found: {0} for track {0}', - item.mb_trackid, + # Do we have a valid MusicBrainz TrackId + if re.match(r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}", item.mb_trackid): + # Get the MusicBrainz recording info. + track_info = hooks.track_for_mbid(item.mb_trackid) + if not track_info: + self._log.info(u'Recording ID not found: {0} for track {0}', + item.mb_trackid, + item_formatted) + continue + + # Apply. + with lib.transaction(): + autotag.apply_item_metadata(item, track_info) + apply_item_changes(lib, item, move, pretend, write) + else: + self._log.info(u'Skipping singleton with invalid mb_trackid: {0}', item_formatted) continue - # Apply. - with lib.transaction(): - autotag.apply_item_metadata(item, track_info) - apply_item_changes(lib, item, move, pretend, write) - def albums(self, lib, query, move, pretend, write): """Retrieve and apply info from the autotagger for albums matched by query and their items. @@ -109,69 +115,76 @@ class MBSyncPlugin(BeetsPlugin): items = list(a.items()) - # Get the MusicBrainz album information. - album_info = hooks.album_for_mbid(a.mb_albumid) - if not album_info: - self._log.info(u'Release ID {0} not found for album {1}', - a.mb_albumid, - album_formatted) - continue - - # 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 (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: - if item.mb_releasetrackid and \ - item.mb_releasetrackid in releasetrack_index: - mapping[item] = releasetrack_index[item.mb_releasetrackid] - else: - 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) - with lib.transaction(): - autotag.apply_metadata(album_info, mapping) - changed = False - # Find any changed item to apply MusicBrainz changes to album. - any_changed_item = items[0] - for item in items: - item_changed = ui.show_model_changes(item) - changed |= item_changed - if item_changed: - any_changed_item = item - apply_item_changes(lib, item, move, pretend, write) - - if not changed: - # No change to any item. + # Do we have a valid MusicBrainz AlbumId + if re.match(r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}", a.mb_albumid): + # Get the MusicBrainz album information. + album_info = hooks.album_for_mbid(a.mb_albumid) + if not album_info: + self._log.info(u'Release ID {0} not found for album {1}', + a.mb_albumid, + album_formatted) continue - if not pretend: - # Update album structure to reflect an item in it. - for key in library.Album.item_keys: - a[key] = any_changed_item[key] - a.store() + # 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) - # Move album art (and any inconsistent items). - if move and lib.directory in util.ancestry(items[0].path): - self._log.debug(u'moving album {0}', album_formatted) - a.move() + # 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: + if item.mb_releasetrackid and \ + item.mb_releasetrackid in releasetrack_index: + mapping[item] = releasetrack_index[item.mb_releasetrackid] + else: + 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 {0}', album_formatted) + with lib.transaction(): + autotag.apply_metadata(album_info, mapping) + changed = False + # Find any changed item to apply MusicBrainz changes to album. + any_changed_item = items[0] + for item in items: + item_changed = ui.show_model_changes(item) + changed |= item_changed + if item_changed: + any_changed_item = item + apply_item_changes(lib, item, move, pretend, write) + + if not changed: + # No change to any item. + continue + + if not pretend: + # Update album structure to reflect an item in it. + for key in library.Album.item_keys: + a[key] = any_changed_item[key] + a.store() + + # Move album art (and any inconsistent items). + if move and lib.directory in util.ancestry(items[0].path): + self._log.debug(u'moving album {0}', album_formatted) + a.move() + + else: + self._log.info(u'Skipping album with invalid mb_albumid: {0}', + album_formatted) + continue diff --git a/docs/changelog.rst b/docs/changelog.rst index 95c774ab3..f99b88218 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -43,6 +43,13 @@ New features: disambiguation comment. A new ``releasegroupdisambig`` field has been added. :bug:`3024` +Changes: + +* :doc:`/plugins/mbsync` no longer queries MusicBrainz when the either the + ``mb_albumid`` or ``mb_trackid`` field is invalid + Discussion here https://groups.google.com/forum/#!searchin/beets-users/mbsync|sort:date/beets-users/iwCF6bNdh9A/i1xl4Gx8BQAJ + Thanks to :user:`arogl`. + Fixes: * A new importer option, :ref:`ignore_data_tracks`, lets you skip audio tracks diff --git a/test/test_mbsync.py b/test/test_mbsync.py index c96f33d53..35caa3046 100644 --- a/test/test_mbsync.py +++ b/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__) From c43a957f46050c94afea4159d7f18d9b051e0ae8 Mon Sep 17 00:00:00 2001 From: Andrew Rogl Date: Sun, 9 Sep 2018 15:48:07 +1000 Subject: [PATCH 2/6] Clean up flake errors --- beetsplug/mbsync.py | 47 ++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index fc763a365..c9a7a3232 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -22,6 +22,8 @@ from beets import autotag, library, ui, util from beets.autotag import hooks from collections import defaultdict +import re + def apply_item_changes(lib, item, move, pretend, write): """Store, move and write the item according to the arguments. @@ -82,12 +84,17 @@ class MBSyncPlugin(BeetsPlugin): item_formatted) continue + valid_trackid = re.match( + r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}", + item.mb_trackid) + # Do we have a valid MusicBrainz TrackId - if re.match(r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}", item.mb_trackid): + if valid_trackid: # Get the MusicBrainz recording info. track_info = hooks.track_for_mbid(item.mb_trackid) if not track_info: - self._log.info(u'Recording ID not found: {0} for track {0}', + self._log.info(u'Recording ID not found: {0}' + + ' for track {0}', item.mb_trackid, item_formatted) continue @@ -97,8 +104,8 @@ class MBSyncPlugin(BeetsPlugin): autotag.apply_item_metadata(item, track_info) apply_item_changes(lib, item, move, pretend, write) else: - self._log.info(u'Skipping singleton with invalid mb_trackid: {0}', - item_formatted) + self._log.info(u'Skipping singleton with invalid mb_trackid:' + + ' {0}', item_formatted) continue def albums(self, lib, query, move, pretend, write): @@ -115,8 +122,13 @@ class MBSyncPlugin(BeetsPlugin): items = list(a.items()) + valid_albumid = re.match( + r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}", + a.mb_albumid) + # Do we have a valid MusicBrainz AlbumId - if re.match(r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}", a.mb_albumid): + + if valid_albumid: # Get the MusicBrainz album information. album_info = hooks.album_for_mbid(a.mb_albumid) if not album_info: @@ -126,17 +138,18 @@ class MBSyncPlugin(BeetsPlugin): continue # 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. + # 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 (release track MBIDs - # first, if available, and recording MBIDs otherwise). This should - # work for albums that have missing or extra tracks. + # 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: if item.mb_releasetrackid and \ @@ -147,8 +160,9 @@ class MBSyncPlugin(BeetsPlugin): 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. + # If there are multiple copies of a recording, + # they aredisambiguated using their disc and + # track number. for c in candidates: if (c.medium_index == item.track and c.medium == item.disc): @@ -160,7 +174,8 @@ class MBSyncPlugin(BeetsPlugin): with lib.transaction(): autotag.apply_metadata(album_info, mapping) changed = False - # Find any changed item to apply MusicBrainz changes to album. + # Find any changed item to apply MusicBrainz changes to + # album. any_changed_item = items[0] for item in items: item_changed = ui.show_model_changes(item) @@ -180,8 +195,10 @@ class MBSyncPlugin(BeetsPlugin): a.store() # Move album art (and any inconsistent items). - if move and lib.directory in util.ancestry(items[0].path): - self._log.debug(u'moving album {0}', album_formatted) + if move and \ + lib.directory in util.ancestry(items[0].path): + self._log.debug( + u'moving album {0}', album_formatted) a.move() else: From 308dccab95681f8602dd28159b5e979e08797c54 Mon Sep 17 00:00:00 2001 From: Andrew Rogl Date: Mon, 10 Sep 2018 20:47:58 +1000 Subject: [PATCH 3/6] Address requested changes from Adrian --- beetsplug/mbsync.py | 180 +++++++++++++++++++++----------------------- docs/changelog.rst | 2 +- 2 files changed, 86 insertions(+), 96 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index c9a7a3232..c6e7864a3 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -24,6 +24,8 @@ from collections import defaultdict import re +mb_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. @@ -84,29 +86,25 @@ class MBSyncPlugin(BeetsPlugin): item_formatted) continue - valid_trackid = re.match( - r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}", - item.mb_trackid) + valid_trackid = re.match(mb_regex, item.mb_trackid) # Do we have a valid MusicBrainz TrackId - if valid_trackid: - # Get the MusicBrainz recording info. - track_info = hooks.track_for_mbid(item.mb_trackid) - if not track_info: - self._log.info(u'Recording ID not found: {0}' + - ' for track {0}', - item.mb_trackid, - item_formatted) - continue - - # Apply. - with lib.transaction(): - autotag.apply_item_metadata(item, track_info) - apply_item_changes(lib, item, move, pretend, write) - else: + if not valid_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: + self._log.info(u'Recording ID not found: {0} for track {0}', + item.mb_trackid, + item_formatted) + continue + + # Apply. + with lib.transaction(): + autotag.apply_item_metadata(item, track_info) + apply_item_changes(lib, item, move, pretend, write) def albums(self, lib, query, move, pretend, write): """Retrieve and apply info from the autotagger for albums matched by @@ -122,86 +120,78 @@ class MBSyncPlugin(BeetsPlugin): items = list(a.items()) - valid_albumid = re.match( - r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}", - a.mb_albumid) + valid_albumid = re.match(mb_regex, a.mb_albumid) # Do we have a valid MusicBrainz AlbumId - if valid_albumid: - # Get the MusicBrainz album information. - album_info = hooks.album_for_mbid(a.mb_albumid) - if not album_info: - self._log.info(u'Release ID {0} not found for album {1}', - a.mb_albumid, - album_formatted) - continue - - # 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 (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: - if item.mb_releasetrackid and \ - item.mb_releasetrackid in releasetrack_index: - mapping[item] = releasetrack_index[item.mb_releasetrackid] - else: - candidates = track_index[item.mb_trackid] - if len(candidates) == 1: - mapping[item] = candidates[0] - else: - # If there are multiple copies of a recording, - # they aredisambiguated 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 {0}', album_formatted) - with lib.transaction(): - autotag.apply_metadata(album_info, mapping) - changed = False - # Find any changed item to apply MusicBrainz changes to - # album. - any_changed_item = items[0] - for item in items: - item_changed = ui.show_model_changes(item) - changed |= item_changed - if item_changed: - any_changed_item = item - apply_item_changes(lib, item, move, pretend, write) - - if not changed: - # No change to any item. - continue - - if not pretend: - # Update album structure to reflect an item in it. - for key in library.Album.item_keys: - a[key] = any_changed_item[key] - a.store() - - # Move album art (and any inconsistent items). - if move and \ - lib.directory in util.ancestry(items[0].path): - self._log.debug( - u'moving album {0}', album_formatted) - a.move() - - else: + if not valid_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: + self._log.info(u'Release ID {0} not found for album {1}', + a.mb_albumid, + album_formatted) + continue + + # 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 (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: + if item.mb_releasetrackid and \ + item.mb_releasetrackid in releasetrack_index: + mapping[item] = releasetrack_index[item.mb_releasetrackid] + else: + 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) + with lib.transaction(): + autotag.apply_metadata(album_info, mapping) + changed = False + # Find any changed item to apply MusicBrainz changes to album. + any_changed_item = items[0] + for item in items: + item_changed = ui.show_model_changes(item) + changed |= item_changed + if item_changed: + any_changed_item = item + apply_item_changes(lib, item, move, pretend, write) + + if not changed: + # No change to any item. + continue + + if not pretend: + # Update album structure to reflect an item in it. + for key in library.Album.item_keys: + a[key] = any_changed_item[key] + a.store() + + # Move album art (and any inconsistent items). + if move and lib.directory in util.ancestry(items[0].path): + self._log.debug(u'moving album {0}', album_formatted) + a.move() diff --git a/docs/changelog.rst b/docs/changelog.rst index f99b88218..1e9695520 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -47,7 +47,7 @@ Changes: * :doc:`/plugins/mbsync` no longer queries MusicBrainz when the either the ``mb_albumid`` or ``mb_trackid`` field is invalid - Discussion here https://groups.google.com/forum/#!searchin/beets-users/mbsync|sort:date/beets-users/iwCF6bNdh9A/i1xl4Gx8BQAJ +.. _Discussion here: https://groups.google.com/forum/#!searchin/beets-users/mbsync|sort:date/beets-users/iwCF6bNdh9A/i1xl4Gx8BQAJ Thanks to :user:`arogl`. Fixes: From fd7458edf5ca8f063492fb235854775f0fe834bc Mon Sep 17 00:00:00 2001 From: Andrew Rogl Date: Mon, 10 Sep 2018 20:53:10 +1000 Subject: [PATCH 4/6] Another attempt at the changelog.rst --- docs/changelog.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 1e9695520..edc89022b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -47,9 +47,11 @@ Changes: * :doc:`/plugins/mbsync` no longer queries MusicBrainz when the either the ``mb_albumid`` or ``mb_trackid`` field is invalid -.. _Discussion here: https://groups.google.com/forum/#!searchin/beets-users/mbsync|sort:date/beets-users/iwCF6bNdh9A/i1xl4Gx8BQAJ + Discussion on Google groups here_ Thanks to :user:`arogl`. +.. _here: 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 From 865940746cd0a30719c38dac3197be695a751a28 Mon Sep 17 00:00:00 2001 From: Andrew Rogl Date: Tue, 11 Sep 2018 18:52:55 +1000 Subject: [PATCH 5/6] More clean up requests --- beetsplug/mbsync.py | 14 ++++++-------- docs/changelog.rst | 6 +++--- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index c6e7864a3..dd5e0e944 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -24,7 +24,7 @@ from collections import defaultdict import re -mb_regex = r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}" +MB_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): @@ -86,13 +86,13 @@ class MBSyncPlugin(BeetsPlugin): item_formatted) continue - valid_trackid = re.match(mb_regex, item.mb_trackid) - - # Do we have a valid MusicBrainz TrackId + # Do we have a valid MusicBrainz TrackId? + valid_trackid = re.match(MB_REGEX, item.mb_trackid) if not valid_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: @@ -120,10 +120,8 @@ class MBSyncPlugin(BeetsPlugin): items = list(a.items()) - valid_albumid = re.match(mb_regex, a.mb_albumid) - - # Do we have a valid MusicBrainz AlbumId - + # Do we have a valid MusicBrainz AlbumId? + valid_albumid = re.match(MB_REGEX, a.mb_albumid) if not valid_albumid: self._log.info(u'Skipping album with invalid mb_albumid: {0}', album_formatted) diff --git a/docs/changelog.rst b/docs/changelog.rst index edc89022b..1b4f5f44e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -45,12 +45,12 @@ New features: Changes: -* :doc:`/plugins/mbsync` no longer queries MusicBrainz when the either the +* :doc:`/plugins/mbsync` no longer queries MusicBrainz when either the ``mb_albumid`` or ``mb_trackid`` field is invalid - Discussion on Google groups here_ + See also the discussion on Google Groups_ Thanks to :user:`arogl`. -.. _here: https://groups.google.com/forum/#!searchin/beets-users/mbsync|sort:date/beets-users/iwCF6bNdh9A/i1xl4Gx8BQAJ +.. _Groups: https://groups.google.com/forum/#!searchin/beets-users/mbsync|sort:date/beets-users/iwCF6bNdh9A/i1xl4Gx8BQAJ Fixes: From 8b59b20ef8d29287f110126e9790850d4291d5fe Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sun, 16 Sep 2018 20:56:06 -0400 Subject: [PATCH 6/6] Tiny style fix for #3028 --- beetsplug/mbsync.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index dd5e0e944..b8121d9c9 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -24,7 +24,7 @@ from collections import defaultdict import re -MB_REGEX = r"(\d|\w){8}-(\d|\w){4}-(\d|\w){4}-(\d|\w){4}-(\d|\w){12}" +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): @@ -86,9 +86,8 @@ class MBSyncPlugin(BeetsPlugin): item_formatted) continue - # Do we have a valid MusicBrainz TrackId? - valid_trackid = re.match(MB_REGEX, item.mb_trackid) - if not valid_trackid: + # 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 @@ -120,9 +119,8 @@ class MBSyncPlugin(BeetsPlugin): items = list(a.items()) - # Do we have a valid MusicBrainz AlbumId? - valid_albumid = re.match(MB_REGEX, a.mb_albumid) - if not valid_albumid: + # 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