From a82e3f9f52f3a4431499a8773d8fb04bcd123bc8 Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Tue, 28 May 2013 12:57:02 +0200 Subject: [PATCH 1/8] Delegate album_for_id to plugins if no MusicBrainz match is found --- beets/autotag/hooks.py | 8 ++++++-- beets/plugins.py | 16 ++++++++++++++++ beets/ui/commands.py | 15 +++------------ 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index a1a197f41..e877ffbcd 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -167,11 +167,15 @@ TrackMatch = namedtuple('TrackMatch', ['distance', 'info']) # Aggregation of sources. def _album_for_id(album_id): - """Get an album corresponding to a MusicBrainz release ID.""" + """Get an album corresponding to a release ID.""" + out = None try: - return mb.album_for_id(album_id) + out = mb.album_for_id(album_id) except mb.MusicBrainzAPIError as exc: exc.log(log) + if not out: + out = plugins.album_for_id(album_id) + return out def _track_for_id(track_id): """Get an item for a recording MBID.""" diff --git a/beets/plugins.py b/beets/plugins.py index 079dd86b0..2cd0693f1 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -99,6 +99,12 @@ class BeetsPlugin(object): """ return {} + def album_for_id(self, album_id): + """Should return an AlbumInfo object or None if no matching release + was found. + """ + return None + listeners = None @@ -263,6 +269,16 @@ def item_candidates(item, artist, title): out.extend(plugin.item_candidates(item, artist, title)) return out +def album_for_id(album_id): + out = None + for plugin in find_plugins(): + try: + out = plugin.album_for_id(album_id) + except Exception as exc: + exc.log(log) + if out: + return out + def configure(config): """Sends the configuration object to each plugin.""" for plugin in find_plugins(): diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 39f353421..8206212ea 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -579,20 +579,11 @@ def manual_search(singleton): return artist.strip(), name.strip() def manual_id(singleton): - """Input a MusicBrainz ID, either for an album ("release") or a - track ("recording"). If no valid ID is entered, returns None. + """Input an ID, either for an album ("release") or a track ("recording"). """ - prompt = 'Enter MusicBrainz %s ID:' % \ + prompt = 'Enter %s ID:' % \ ('recording' if singleton else 'release') - entry = input_(prompt).strip() - - # Find the first thing that looks like a UUID/MBID. - match = re.search('[a-f0-9]{8}(-[a-f0-9]{4}){3}-[a-f0-9]{12}', entry) - if match: - return match.group() - else: - log.error('Invalid MBID.') - return None + return input_(prompt).strip() class TerminalImportSession(importer.ImportSession): """An import session that runs in a terminal. From e3418be75d3eb7e8a3de30caa4f0ca004ef243d6 Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Tue, 28 May 2013 12:58:05 +0200 Subject: [PATCH 2/8] Add support for manually entered release ID in Discogs --- beetsplug/discogs.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 951c5d593..0d30bb39d 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -60,6 +60,26 @@ class DiscogsPlugin(BeetsPlugin): log.debug('Discogs API Error: %s (query: %s' % (e, query)) return [] + def album_for_id(self, album_id): + """Fetches an album by its Discogs ID and returns an AlbumInfo object + or None if the album is not found. + """ + log.debug('Searching for release with id %s' % str(album_id)) + # discogs-client can handle both int and str, so we just strip + # the leading 'r' that might be accidentally pasted into the form + if not isinstance(album_id, int) and album_id.startswith('r'): + album_id = album_id[1:] + result = Release(album_id) + # Try to obtain title to verify that we indeed have a valid Release + try: + getattr(result, 'title') + except DiscogsAPIError as e: + if e.message != '404 Not Found': + log.debug('Discogs API Error: %s (query: %s)' + % (e, result._uri)) + return None + return self.get_album_info(result) + def get_albums(self, query): """Returns a list of AlbumInfo objects for a discogs search query. """ From f17e8550ca4f4fcd53d70e0e54c0c5b1814d0247 Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Tue, 28 May 2013 13:49:34 +0200 Subject: [PATCH 3/8] Determine IDs from MusicBrainz and Discogs URLs --- beets/autotag/mb.py | 11 +++++++++-- beetsplug/discogs.py | 9 +++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/beets/autotag/mb.py b/beets/autotag/mb.py index 566340e83..6fe7854d3 100644 --- a/beets/autotag/mb.py +++ b/beets/autotag/mb.py @@ -16,6 +16,7 @@ """ import logging import musicbrainzngs +import re import traceback import beets.autotag.hooks @@ -326,13 +327,19 @@ def album_for_id(albumid): object or None if the album is not found. May raise a MusicBrainzAPIError. """ + # Find the first thing that looks like a UUID/MBID. + match = re.search('[a-f0-9]{8}(-[a-f0-9]{4}){3}-[a-f0-9]{12}', albumid) + if not match: + log.error('Invalid MBID.') + return None try: - res = musicbrainzngs.get_release_by_id(albumid, RELEASE_INCLUDES) + res = musicbrainzngs.get_release_by_id(match.group(), + RELEASE_INCLUDES) except musicbrainzngs.ResponseError: log.debug('Album ID match failed.') return None except musicbrainzngs.MusicBrainzError as exc: - raise MusicBrainzAPIError(exc, 'get release by ID', albumid, + raise MusicBrainzAPIError(exc, 'get release by ID', match.group(), traceback.format_exc()) return album_info(res['release']) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 0d30bb39d..2cd872e7f 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -64,12 +64,13 @@ class DiscogsPlugin(BeetsPlugin): """Fetches an album by its Discogs ID and returns an AlbumInfo object or None if the album is not found. """ - log.debug('Searching for release with id %s' % str(album_id)) + log.debug('Searching discogs for release %s' % str(album_id)) # discogs-client can handle both int and str, so we just strip # the leading 'r' that might be accidentally pasted into the form - if not isinstance(album_id, int) and album_id.startswith('r'): - album_id = album_id[1:] - result = Release(album_id) + match = re.search('\d+', album_id) + if not match: + return None + result = Release(match.group()) # Try to obtain title to verify that we indeed have a valid Release try: getattr(result, 'title') From ad66b8796a8e994c52ebec6eb6ef24b358b047e2 Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Tue, 28 May 2013 13:50:20 +0200 Subject: [PATCH 4/8] Fix tests for albums from manual ID --- test/test_mb.py | 16 ++++++++++++++++ test/test_ui.py | 22 ---------------------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/test/test_mb.py b/test/test_mb.py index e23201706..278e5cd1f 100644 --- a/test/test_mb.py +++ b/test/test_mb.py @@ -277,6 +277,22 @@ class MBAlbumInfoTest(unittest.TestCase): self.assertEqual(track.artist_sort, 'TRACK ARTIST SORT NAME') self.assertEqual(track.artist_credit, 'TRACK ARTIST CREDIT') + def test_album_for_id_correct(self): + id_string = "28e32c71-1450-463e-92bf-e0a46446fc11" + out = mb.album_for_id(id_string) + self.assertEqual(out.album_id, id_string) + + def test_album_for_id_non_id_returns_none(self): + id_string = "blah blah" + out = mb.album_for_id(id_string) + self.assertEqual(out, None) + + def test_album_for_id_url_finds_id(self): + id_string = "28e32c71-1450-463e-92bf-e0a46446fc11" + id_url = "http://musicbrainz.org/entity/%s" % id_string + out = mb.album_for_id(id_url) + self.assertEqual(out.album_id, id_string) + class ArtistFlatteningTest(unittest.TestCase): def _credit_dict(self, suffix=''): return { diff --git a/test/test_ui.py b/test/test_ui.py index c9d57a466..b679021f7 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -585,28 +585,6 @@ class ShowdiffTest(_common.TestCase): self.assertEqual(complete_diff, partial_diff) -AN_ID = "28e32c71-1450-463e-92bf-e0a46446fc11" -class ManualIDTest(_common.TestCase): - def setUp(self): - super(ManualIDTest, self).setUp() - _common.log.setLevel(logging.CRITICAL) - self.io.install() - - def test_id_accepted(self): - self.io.addinput(AN_ID) - out = commands.manual_id(False) - self.assertEqual(out, AN_ID) - - def test_non_id_returns_none(self): - self.io.addinput("blah blah") - out = commands.manual_id(False) - self.assertEqual(out, None) - - def test_url_finds_id(self): - self.io.addinput("http://musicbrainz.org/entity/%s?something" % AN_ID) - out = commands.manual_id(False) - self.assertEqual(out, AN_ID) - class ShowChangeTest(_common.TestCase): def setUp(self): super(ShowChangeTest, self).setUp() From adb22c3511a51cc04018cd5dbead27cb7652d751 Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Wed, 29 May 2013 10:53:55 +0200 Subject: [PATCH 5/8] Return a list of candidates in hooks._album_for_id --- beets/autotag/hooks.py | 12 +++++++----- beets/autotag/match.py | 21 +++++++-------------- beets/plugins.py | 13 +++++++------ 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index e877ffbcd..3642e46a0 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -167,15 +167,17 @@ TrackMatch = namedtuple('TrackMatch', ['distance', 'info']) # Aggregation of sources. def _album_for_id(album_id): - """Get an album corresponding to a release ID.""" - out = None + """Get a list of albums corresponding to a release ID.""" + candidates = [] try: out = mb.album_for_id(album_id) except mb.MusicBrainzAPIError as exc: exc.log(log) - if not out: - out = plugins.album_for_id(album_id) - return out + if out: + candidates.append(out) + out = plugins.album_for_id(album_id) + candidates.extend(x for x in out if x is not None) + return candidates def _track_for_id(track_id): """Get an item for a recording MBID.""" diff --git a/beets/autotag/match.py b/beets/autotag/match.py index d4d3ae870..cd490583e 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -437,11 +437,7 @@ def tag_album(items, search_artist=None, search_album=None, candidates = {} # Try to find album indicated by MusicBrainz IDs. - if search_id: - log.debug('Searching for album ID: ' + search_id) - id_info = hooks._album_for_id(search_id) - else: - id_info = match_by_id(items) + id_info = match_by_id(items) if id_info: _add_candidate(items, candidates, id_info) rec = _recommendation(candidates.values()) @@ -454,13 +450,6 @@ def tag_album(items, search_artist=None, search_album=None, log.debug('ID match.') return cur_artist, cur_album, candidates.values(), rec - # If searching by ID, don't continue to metadata search. - if search_id is not None: - if candidates: - return cur_artist, cur_album, candidates.values(), rec - else: - return cur_artist, cur_album, [], recommendation.none - # Search terms. if not (search_artist and search_album): # No explicit search terms -- use current metadata. @@ -474,8 +463,12 @@ def tag_album(items, search_artist=None, search_album=None, log.debug(u'Album might be VA: %s' % str(va_likely)) # Get the results from the data sources. - search_cands = hooks._album_candidates(items, search_artist, search_album, - va_likely) + if search_id: + log.debug('Searching for album ID: ' + search_id) + search_cands = hooks._album_for_id(search_id) + else: + search_cands = hooks._album_candidates(items, search_artist, + search_album, va_likely) log.debug(u'Evaluating %i candidates.' % len(search_cands)) for info in search_cands: _add_candidate(items, candidates, info) diff --git a/beets/plugins.py b/beets/plugins.py index 2cd0693f1..0189e3a14 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -270,14 +270,15 @@ def item_candidates(item, artist, title): return out def album_for_id(album_id): - out = None + out = [] for plugin in find_plugins(): try: - out = plugin.album_for_id(album_id) - except Exception as exc: - exc.log(log) - if out: - return out + out.append(plugin.album_for_id(album_id)) + except Exception: + log.warn('** error running album_for_id in plugin %s' + % plugin.name) + log.warn(traceback.format_exc()) + return out def configure(config): """Sends the configuration object to each plugin.""" From 40a6e03ba606762c187c781600670918c8c299e0 Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Wed, 29 May 2013 10:54:15 +0200 Subject: [PATCH 6/8] Improve regular expression for Discogs IDs --- beetsplug/discogs.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 2cd872e7f..f4ef6d79b 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -65,9 +65,11 @@ class DiscogsPlugin(BeetsPlugin): or None if the album is not found. """ log.debug('Searching discogs for release %s' % str(album_id)) - # discogs-client can handle both int and str, so we just strip - # the leading 'r' that might be accidentally pasted into the form - match = re.search('\d+', album_id) + # Discogs-IDs are simple integers. We only look for those at the end + # of an input string as to avoid confusion with other metadata plugins. + # An optional bracket can follow the integer, as this is how discogs + # displays the release ID on its webpage. + match = re.search(r'(\d+)\]*$', album_id) if not match: return None result = Release(match.group()) From 2d4ea4e49b2028aa467208d98d64fe983aa01bb0 Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Wed, 29 May 2013 10:59:04 +0200 Subject: [PATCH 7/8] Fix bug in discogs.album_for_id for ID-Strings that end in a bracket --- beetsplug/discogs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index f4ef6d79b..d52c5b1fd 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -72,7 +72,7 @@ class DiscogsPlugin(BeetsPlugin): match = re.search(r'(\d+)\]*$', album_id) if not match: return None - result = Release(match.group()) + result = Release(match.group(1)) # Try to obtain title to verify that we indeed have a valid Release try: getattr(result, 'title') From 3df635eca2354fc24df02825c781bd84d85b752c Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Wed, 29 May 2013 20:31:58 +0200 Subject: [PATCH 8/8] Make regex for Discogs ID more specific --- beetsplug/discogs.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index d52c5b1fd..b213c589c 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -69,10 +69,11 @@ class DiscogsPlugin(BeetsPlugin): # of an input string as to avoid confusion with other metadata plugins. # An optional bracket can follow the integer, as this is how discogs # displays the release ID on its webpage. - match = re.search(r'(\d+)\]*$', album_id) + match = re.search(r'(^|\[*r|discogs\.com/.+/release/)(\d+)($|\])', + album_id) if not match: return None - result = Release(match.group(1)) + result = Release(match.group(2)) # Try to obtain title to verify that we indeed have a valid Release try: getattr(result, 'title')