From 3804eb5b52ea482d36cc88ccf676b465c22bddb6 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 23:45:23 +0100 Subject: [PATCH 01/13] Stop on invalid queries instead of ignoring them So far an invalid query won't be applied: $ beet ls The Beatles year:196a will be treaded as $ beet ls The Beatles With this commit it stops beets, returns 1 and produces $ invalid query: u'196a' is not an int or a float This applies to any querying and therefore on many command, plugins and some configuration options. Invalid queries exist on numeric fields and on regular expression usage. Compile regular expression queries upon instantiation instead of upon each match test. The reporting can be improved (give more context). Fix #1219. --- beets/dbcore/query.py | 25 ++++++++++++++++++------- beets/ui/__init__.py | 4 ++++ test/test_query.py | 20 ++++++++++---------- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 3ea37524a..2719eb508 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -20,6 +20,14 @@ from beets import util from datetime import datetime, timedelta +class InvalidQuery(ValueError): + def __init__(self, what, expected, detail=None): + message = "{0!r} is not {1}".format(what, expected) + if detail: + message = "{0}: {1}".format(message, detail) + super(InvalidQuery, self).__init__(message) + + class Query(object): """An abstract class representing a query into the item database. """ @@ -140,14 +148,17 @@ class RegexpQuery(StringFieldQuery): """A query that matches a regular expression in a specific item field. """ + def __init__(self, field, pattern, false=True): + super(RegexpQuery, self).__init__(field, pattern, false) + try: + self.pattern = re.compile(self.pattern) + except re.error as exc: + # Invalid regular expression. + raise InvalidQuery(pattern, "a regular expression", format(exc)) + @classmethod def string_match(cls, pattern, value): - try: - res = re.search(pattern, value) - except re.error: - # Invalid regular expression. - return False - return res is not None + return pattern.search(value) is not None class BooleanQuery(MatchQuery): @@ -203,7 +214,7 @@ class NumericQuery(FieldQuery): try: return float(s) except ValueError: - return None + raise InvalidQuery(s, "an int or a float") def __init__(self, field, pattern, fast=True): super(NumericQuery, self).__init__(field, pattern, fast) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 291c768ec..41d384b17 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -38,6 +38,7 @@ from beets.util.functemplate import Template from beets import config from beets.util import confit from beets.autotag import mb +from beets.dbcore import query as db_query # On Windows platforms, use colorama to support "ANSI" terminal colors. if sys.platform == 'win32': @@ -960,6 +961,9 @@ def main(args=None): except confit.ConfigError as exc: log.error(u'configuration error: {0}', exc) sys.exit(1) + except db_query.InvalidQuery as exc: + log.error(u'invalid query: {0}', exc) + sys.exit(1) except IOError as exc: if exc.errno == errno.EPIPE: # "Broken pipe". End silently. diff --git a/test/test_query.py b/test/test_query.py index 879e9ca7d..4d9dbc85a 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -21,7 +21,7 @@ import helper import beets.library from beets import dbcore from beets.dbcore import types -from beets.dbcore.query import NoneQuery +from beets.dbcore.query import NoneQuery, InvalidQuery from beets.library import Library, Item @@ -218,11 +218,6 @@ class GetTest(DummyDataTestCase): 'baz qux', ]) - def test_bad_year(self): - q = 'year:delete from items' - results = self.lib.items(q) - self.assert_matched(results, []) - def test_singleton_true(self): q = 'singleton:true' results = self.lib.items(q) @@ -280,10 +275,15 @@ class GetTest(DummyDataTestCase): results = self.lib.items(q) self.assertFalse(results) - def test_numeric_empty(self): - q = dbcore.query.NumericQuery('year', '') - results = self.lib.items(q) - self.assertTrue(results) + def test_invalid_query(self): + with self.assertRaises(InvalidQuery) as raised: + dbcore.query.NumericQuery('year', '199a') + self.assertIn('not an int', str(raised.exception)) + + with self.assertRaises(InvalidQuery) as raised: + dbcore.query.RegexpQuery('year', '199(') + self.assertIn('not a regular expression', str(raised.exception)) + self.assertIn('unbalanced parenthesis', str(raised.exception)) class MatchTest(_common.TestCase): From 6408904a8c1eabc05594c0d48cde9ee094156bfd Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Wed, 14 Jan 2015 12:15:44 +0100 Subject: [PATCH 02/13] Smartplaylit: fix log format string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit {0.name} → {0[name]} since the argument is a dict. --- beetsplug/smartplaylist.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 15cb46057..4c242efe6 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -67,7 +67,7 @@ class SmartPlaylistPlugin(BeetsPlugin): relative_to = normpath(relative_to) for playlist in playlists: - self._log.debug(u"Creating playlist {0.name}", playlist) + self._log.debug(u"Creating playlist {0[name]}", playlist) items = [] if 'album_query' in playlist: items.extend(_items_for_query(lib, playlist['album_query'], From 0d1fa8065155392b3fc3066a0e253785a8eadcd6 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Wed, 14 Jan 2015 12:21:05 +0100 Subject: [PATCH 03/13] Smartplaylist: don't utf8-encode the name A Template expression expects an unicode, not an utf8-encoded string. Add a test for that. --- beetsplug/smartplaylist.py | 3 +-- test/test_template.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 4c242efe6..3b5be2149 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -76,11 +76,10 @@ class SmartPlaylistPlugin(BeetsPlugin): items.extend(_items_for_query(lib, playlist['query'], False)) m3us = {} - basename = playlist['name'].encode('utf8') # As we allow tags in the m3u names, we'll need to iterate through # the items and generate the correct m3u file names. for item in items: - m3u_name = item.evaluate_template(basename, True) + m3u_name = item.evaluate_template(playlist['name'], True) if m3u_name not in m3us: m3us[m3u_name] = [] item_path = item.path diff --git a/test/test_template.py b/test/test_template.py index 1bc0b2cd7..20c0df8f4 100644 --- a/test/test_template.py +++ b/test/test_template.py @@ -1,3 +1,4 @@ +# -*- coding: utf8 -*- # This file is part of beets. # Copyright 2015, Adrian Sampson. # @@ -14,6 +15,8 @@ """Tests for template engine. """ +import warnings + from _common import unittest from beets.util import functemplate @@ -207,6 +210,13 @@ class ParseTest(unittest.TestCase): self._assert_call(arg_parts[0], u"bar", 1) self.assertEqual(list(_normexpr(arg_parts[0].args[0])), [u'baz']) + def test_fail_on_utf8(self): + parts = u'é'.encode('utf8') + warnings.simplefilter("ignore") + with self.assertRaises(UnicodeDecodeError): + functemplate._parse(parts) + warnings.simplefilter("default") + class EvalTest(unittest.TestCase): def _eval(self, template): From 0c3675ada05b9a5e342fd3d2ba233d3234b3eec7 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Wed, 14 Jan 2015 16:17:20 +0100 Subject: [PATCH 04/13] Fix replaygain: add 'log' to __init__ parameters --- beetsplug/replaygain.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index f7ca68c5b..c2e21cd92 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -215,8 +215,9 @@ class CommandBackend(Backend): # GStreamer-based backend. -class GStreamerBackend(object): - def __init__(self, config): +class GStreamerBackend(Backend): + def __init__(self, config, log): + super(GStreamerBackend, self).__init__(config, log) self._import_gst() # Initialized a GStreamer pipeline of the form filesrc -> From 38c5bb36660a3650776d5196e625e9e279231f64 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 14 Jan 2015 21:53:13 -0800 Subject: [PATCH 05/13] Fix a docstring --- beetsplug/smartplaylist.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 3b5be2149..3bc18fa69 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -24,8 +24,11 @@ import os def _items_for_query(lib, queries, album): - """Get the matching items for a query. - `album` indicates whether the queries are item-level or album-level. + """Get the matching items for a list of queries. + + `queries` can either be a single string or a list of strings. In the + latter case, the results from each query are concatenated. `album` + indicates whether the queries are item-level or album-level. """ request = lib.albums if album else lib.items if isinstance(queries, basestring): From c1ce71f35cc5be610f82e366f1894b041fef230b Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 14 Jan 2015 22:00:30 -0800 Subject: [PATCH 06/13] smartplaylist: Fix album_query (fix #1225) This is far less elegant and functional, but at least it is correct. --- beetsplug/smartplaylist.py | 13 +++++++++---- docs/changelog.rst | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 3bc18fa69..6563e60db 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -15,7 +15,6 @@ """Generates smart playlists based on beets queries. """ from __future__ import print_function -from itertools import chain from beets.plugins import BeetsPlugin from beets import ui @@ -30,11 +29,17 @@ def _items_for_query(lib, queries, album): latter case, the results from each query are concatenated. `album` indicates whether the queries are item-level or album-level. """ - request = lib.albums if album else lib.items if isinstance(queries, basestring): - return request(queries) + queries = [queries] + if album: + for query in queries: + for album in lib.albums(query): + for item in album.items(): + yield item else: - return chain.from_iterable(map(request, queries)) + for query in queries: + for item in lib.items(query): + yield item class SmartPlaylistPlugin(BeetsPlugin): diff --git a/docs/changelog.rst b/docs/changelog.rst index 69aa4da8d..0a46ba6d6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -18,6 +18,8 @@ Fixes: :bug:`1212` * Fix a crash when the importer deals with Unicode metadata in ``--pretend`` mode. :bug:`1214` +* :doc:`/plugins/smartplaylist`: Fix ``album_query`` so that individual files + are added to the playlist instead of directories. :bug:`1225` For developers: The logging system in beets has been overhauled. Plugins now each have their own logger, which helps by automatically adjusting the From b49cd3b73b9356936b9f8c0e74e3a47131dcb33c Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Thu, 15 Jan 2015 11:28:04 +0100 Subject: [PATCH 07/13] Lyrics plugin: fix google backend Give config on backends init so the Google backend can get the API key and Engine Id. Fix #1226 --- beetsplug/lyrics.py | 17 ++++++++++------- test/test_lyrics.py | 9 ++++++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 625e8fff1..5e51b2b7c 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -157,7 +157,7 @@ def search_pairs(item): class Backend(object): - def __init__(self, log): + def __init__(self, config, log): self._log = log @staticmethod @@ -335,6 +335,11 @@ def scrape_lyrics_from_html(html): class Google(Backend): """Fetch lyrics from Google search results.""" + def __init__(self, config, log): + super(Google, self).__init__(config, log) + self.api_key = config['google_API_key'].get(unicode) + self.engine_id = config['google_engine_ID'].get(unicode) + def is_lyrics(self, text, artist=None): """Determine whether the text seems to be valid lyrics. """ @@ -407,10 +412,8 @@ class Google(Backend): def fetch(self, artist, title): query = u"%s %s" % (artist, title) - api_key = self.config['google_API_key'].get(unicode) - engine_id = self.config['google_engine_ID'].get(unicode) url = u'https://www.googleapis.com/customsearch/v1?key=%s&cx=%s&q=%s' % \ - (api_key, engine_id, urllib.quote(query.encode('utf8'))) + (self.api_key, self.engine_id, urllib.quote(query.encode('utf8'))) data = urllib.urlopen(url) data = json.load(data) @@ -464,9 +467,9 @@ class LyricsPlugin(plugins.BeetsPlugin): available_sources.remove('google') self.config['sources'] = plugins.sanitize_choices( self.config['sources'].as_str_seq(), available_sources) - self.backends = [] - for key in self.config['sources'].as_str_seq(): - self.backends.append(self.SOURCE_BACKENDS[key](self._log)) + + self.backends = [self.SOURCE_BACKENDS[key](self.config, self._log) + for key in self.config['sources'].as_str_seq()] def commands(self): cmd = ui.Subcommand('lyrics', help='fetch song lyrics') diff --git a/test/test_lyrics.py b/test/test_lyrics.py index 21eb87256..90e47aa89 100644 --- a/test/test_lyrics.py +++ b/test/test_lyrics.py @@ -18,6 +18,9 @@ import os import _common import sys import re + +from mock import MagicMock + from _common import unittest from beetsplug import lyrics from beets.library import Item @@ -25,8 +28,8 @@ from beets.util import confit from beets import logging log = logging.getLogger('beets.test_lyrics') -raw_backend = lyrics.Backend(log) -google = lyrics.Google(log) +raw_backend = lyrics.Backend({}, log) +google = lyrics.Google(MagicMock(), log) class LyricsPluginTest(unittest.TestCase): @@ -337,7 +340,7 @@ class LyricsGooglePluginTest(unittest.TestCase): lyrics.MusiXmatch], DEFAULT_SOURCES): url = s['url'] + s['path'] if os.path.isfile(url_to_filename(url)): - res = source(log).fetch(s['artist'], s['title']) + res = source({}, log).fetch(s['artist'], s['title']) self.assertTrue(google.is_lyrics(res), url) self.assertTrue(is_lyrics_content_ok(s['title'], res), url) From f4b4847919f0b50b7096ffe4cfe8f96a465fd217 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Thu, 15 Jan 2015 11:47:35 +0100 Subject: [PATCH 08/13] =?UTF-8?q?Rename=20exception:=20InvalidQuery=20?= =?UTF-8?q?=E2=86=92=20InvalidQueryError?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow PEP8. --- beets/dbcore/query.py | 9 +++++---- beets/ui/__init__.py | 2 +- test/test_query.py | 6 +++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 2719eb508..b6d1cfe2d 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -20,12 +20,12 @@ from beets import util from datetime import datetime, timedelta -class InvalidQuery(ValueError): +class InvalidQueryError(ValueError): def __init__(self, what, expected, detail=None): message = "{0!r} is not {1}".format(what, expected) if detail: message = "{0}: {1}".format(message, detail) - super(InvalidQuery, self).__init__(message) + super(InvalidQueryError, self).__init__(message) class Query(object): @@ -154,7 +154,8 @@ class RegexpQuery(StringFieldQuery): self.pattern = re.compile(self.pattern) except re.error as exc: # Invalid regular expression. - raise InvalidQuery(pattern, "a regular expression", format(exc)) + raise InvalidQueryError(pattern, "a regular expression", + format(exc)) @classmethod def string_match(cls, pattern, value): @@ -214,7 +215,7 @@ class NumericQuery(FieldQuery): try: return float(s) except ValueError: - raise InvalidQuery(s, "an int or a float") + raise InvalidQueryError(s, "an int or a float") def __init__(self, field, pattern, fast=True): super(NumericQuery, self).__init__(field, pattern, fast) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 41d384b17..c541ba2de 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -961,7 +961,7 @@ def main(args=None): except confit.ConfigError as exc: log.error(u'configuration error: {0}', exc) sys.exit(1) - except db_query.InvalidQuery as exc: + except db_query.InvalidQueryError as exc: log.error(u'invalid query: {0}', exc) sys.exit(1) except IOError as exc: diff --git a/test/test_query.py b/test/test_query.py index 4d9dbc85a..cead73bfe 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -21,7 +21,7 @@ import helper import beets.library from beets import dbcore from beets.dbcore import types -from beets.dbcore.query import NoneQuery, InvalidQuery +from beets.dbcore.query import NoneQuery, InvalidQueryError from beets.library import Library, Item @@ -276,11 +276,11 @@ class GetTest(DummyDataTestCase): self.assertFalse(results) def test_invalid_query(self): - with self.assertRaises(InvalidQuery) as raised: + with self.assertRaises(InvalidQueryError) as raised: dbcore.query.NumericQuery('year', '199a') self.assertIn('not an int', str(raised.exception)) - with self.assertRaises(InvalidQuery) as raised: + with self.assertRaises(InvalidQueryError) as raised: dbcore.query.RegexpQuery('year', '199(') self.assertIn('not a regular expression', str(raised.exception)) self.assertIn('unbalanced parenthesis', str(raised.exception)) From 08c9ad43fafc8923576c483724d8ed1988be67ee Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Thu, 15 Jan 2015 11:54:46 +0100 Subject: [PATCH 09/13] Document the new behaviour in docstrings & changelog --- beets/dbcore/query.py | 6 ++++++ docs/changelog.rst | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index b6d1cfe2d..bfe6ea6d7 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -147,6 +147,9 @@ class SubstringQuery(StringFieldQuery): class RegexpQuery(StringFieldQuery): """A query that matches a regular expression in a specific item field. + + Raises InvalidQueryError when the pattern is not a valid regular + expression. """ def __init__(self, field, pattern, false=True): super(RegexpQuery, self).__init__(field, pattern, false) @@ -203,6 +206,9 @@ class NumericQuery(FieldQuery): """Matches numeric fields. A syntax using Ruby-style range ellipses (``..``) lets users specify one- or two-sided ranges. For example, ``year:2001..`` finds music released since the turn of the century. + + Raises InvalidQueryError when the pattern does not represent an int or + a float. """ def _convert(self, s): """Convert a string to a numeric type (float or int). If the diff --git a/docs/changelog.rst b/docs/changelog.rst index 69aa4da8d..88694f66c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,10 @@ Changelog 1.3.11 (in development) ----------------------- +Features: + +* Stop on invalid queries instead of ignoring the invalid part. + Fixes: * :doc:`/plugins/lyrics`: Silence a warning about insecure requests in the new From d2726e1c2517fa7effe2816f5f377990d47e9d3f Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Fri, 16 Jan 2015 11:08:24 +0100 Subject: [PATCH 10/13] Fix mpdstats plugin config management Fix #1228: - remove erroneous calls to `self.config` - always use `config['mpd']` and not `self.config` (which is config['mpdstats']) to match the doc --- beetsplug/mpdstats.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/beetsplug/mpdstats.py b/beetsplug/mpdstats.py index 0b2c8ba9b..f361b0ee8 100644 --- a/beetsplug/mpdstats.py +++ b/beetsplug/mpdstats.py @@ -32,6 +32,9 @@ RETRIES = 10 RETRY_INTERVAL = 5 +mpd_config = config['mpd'] + + def is_url(path): """Try to determine if the path is an URL. """ @@ -57,15 +60,15 @@ class MPDClientWrapper(object): self._log = log self.music_directory = ( - self.config['music_directory'].get(unicode)) + mpd_config['music_directory'].get(unicode)) self.client = MPDClient() def connect(self): """Connect to the MPD. """ - host = config['mpd']['host'].get(unicode) - port = config['mpd']['port'].get(int) + host = mpd_config['host'].get(unicode) + port = mpd_config['port'].get(int) if host[0] in ['/', '~']: host = os.path.expanduser(host) @@ -76,7 +79,7 @@ class MPDClientWrapper(object): except socket.error as e: raise ui.UserError('could not connect to MPD: {0}'.format(e)) - password = config['mpd']['password'].get(unicode) + password = mpd_config['password'].get(unicode) if password: try: self.client.password(password) @@ -144,8 +147,8 @@ class MPDStats(object): self.lib = lib self._log = log - self.do_rating = self.config['rating'].get(bool) - self.rating_mix = self.config['rating_mix'].get(float) + self.do_rating = mpd_config['rating'].get(bool) + self.rating_mix = mpd_config['rating_mix'].get(float) self.time_threshold = 10.0 # TODO: maybe add config option? self.now_playing = None @@ -315,12 +318,10 @@ class MPDStatsPlugin(plugins.BeetsPlugin): def __init__(self): super(MPDStatsPlugin, self).__init__() - self.config.add({ + mpd_config.add({ 'music_directory': config['directory'].as_filename(), 'rating': True, 'rating_mix': 0.75, - }) - config['mpd'].add({ 'host': u'localhost', 'port': 6600, 'password': u'', @@ -341,15 +342,15 @@ class MPDStatsPlugin(plugins.BeetsPlugin): help='set the password of the MPD server to connect to') def func(lib, opts, args): - self.config.set_args(opts) + mpd_config.set_args(opts) # Overrides for MPD settings. if opts.host: - config['mpd']['host'] = opts.host.decode('utf8') + mpd_config['host'] = opts.host.decode('utf8') if opts.port: - config['mpd']['host'] = int(opts.port) + mpd_config['host'] = int(opts.port) if opts.password: - config['mpd']['password'] = opts.password.decode('utf8') + mpd_config['password'] = opts.password.decode('utf8') try: MPDStats(lib, self._log).run() From 7c2d5f09489ae610da412d967bea8e6e9699c5f2 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Fri, 16 Jan 2015 17:59:35 +0100 Subject: [PATCH 11/13] Import beets.ui in discogs plugin The discogs plugin uses beets.ui but did not explicitely import it, which might cause it to crash when used in standalone. --- beetsplug/discogs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 0491bcb4c..c5e9dd2f8 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -15,6 +15,7 @@ """Adds Discogs album search support to the autotagger. Requires the discogs-client library. """ +import beets.ui from beets import logging from beets.autotag.hooks import AlbumInfo, TrackInfo, Distance from beets.plugins import BeetsPlugin From bf02855ee1609c0124d69e7e95b27945c666e96b Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 16 Jan 2015 12:15:54 -0800 Subject: [PATCH 12/13] Remove Beatport plugin (c.f. #1229) --- README.rst | 3 +- beetsplug/beatport.py | 304 -------------------------------------- docs/changelog.rst | 9 +- docs/plugins/beatport.rst | 33 ----- docs/plugins/index.rst | 2 - setup.py | 1 - 6 files changed, 7 insertions(+), 345 deletions(-) delete mode 100644 beetsplug/beatport.py delete mode 100644 docs/plugins/beatport.rst diff --git a/README.rst b/README.rst index 8c64e2446..c95479490 100644 --- a/README.rst +++ b/README.rst @@ -31,7 +31,7 @@ imagine for your music collection. Via `plugins`_, beets becomes a panacea: - Fetch or calculate all the metadata you could possibly need: `album art`_, `lyrics`_, `genres`_, `tempos`_, `ReplayGain`_ levels, or `acoustic fingerprints`_. -- Get metadata from `MusicBrainz`_, `Discogs`_, or `Beatport`_. Or guess +- Get metadata from `MusicBrainz`_ or `Discogs`_. Or guess metadata using songs' filenames or their acoustic fingerprints. - `Transcode audio`_ to any format you like. - Check your library for `duplicate tracks and albums`_ or for `albums that @@ -60,7 +60,6 @@ shockingly simple if you know a little Python. http://beets.readthedocs.org/page/plugins/duplicates.html .. _Transcode audio: http://beets.readthedocs.org/page/plugins/convert.html -.. _Beatport: http://www.beatport.com/ .. _Discogs: http://www.discogs.com/ .. _acoustic fingerprints: http://beets.readthedocs.org/page/plugins/chroma.html diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py deleted file mode 100644 index de71562a9..000000000 --- a/beetsplug/beatport.py +++ /dev/null @@ -1,304 +0,0 @@ -# This file is part of beets. -# Copyright 2015, Adrian Sampson. -# -# Permission is hereby granted, free of charge, to any person obtaining -# a copy of this software and associated documentation files (the -# "Software"), to deal in the Software without restriction, including -# without limitation the rights to use, copy, modify, merge, publish, -# distribute, sublicense, and/or sell copies of the Software, and to -# permit persons to whom the Software is furnished to do so, subject to -# the following conditions: -# -# The above copyright notice and this permission notice shall be -# included in all copies or substantial portions of the Software. - -"""Adds Beatport release and track search support to the autotagger -""" -import re -from datetime import datetime, timedelta - -import requests - -from beets.autotag.hooks import AlbumInfo, TrackInfo, Distance -from beets.plugins import BeetsPlugin - - -class BeatportAPIError(Exception): - pass - - -class BeatportObject(object): - def __init__(self, data): - self.beatport_id = data['id'] - self.name = unicode(data['name']) - if 'releaseDate' in data: - self.release_date = datetime.strptime(data['releaseDate'], - '%Y-%m-%d') - if 'artists' in data: - self.artists = [(x['id'], unicode(x['name'])) - for x in data['artists']] - if 'genres' in data: - self.genres = [unicode(x['name']) - for x in data['genres']] - - -class BeatportAPI(object): - API_BASE = 'http://api.beatport.com/' - - @classmethod - def get(cls, endpoint, **kwargs): - try: - response = requests.get(cls.API_BASE + endpoint, params=kwargs) - except Exception as e: - raise BeatportAPIError("Error connection to Beatport API: {}" - .format(e.message)) - if not response: - raise BeatportAPIError( - "Error {0.status_code} for '{0.request.path_url}" - .format(response)) - return response.json()['results'] - - -class BeatportSearch(object): - query = None - release_type = None - - def __unicode__(self): - return u''.format( - self.release_type, self.query, len(self.results)) - - def __init__(self, query, release_type='release', details=True): - self.results = [] - self.query = query - self.release_type = release_type - response = BeatportAPI.get('catalog/3/search', query=query, - facets=['fieldType:{0}' - .format(release_type)], - perPage=5) - for item in response: - if release_type == 'release': - release = BeatportRelease(item) - if details: - release.get_tracks() - self.results.append(release) - elif release_type == 'track': - self.results.append(BeatportTrack(item)) - - -class BeatportRelease(BeatportObject): - API_ENDPOINT = 'catalog/3/beatport/release' - - def __unicode__(self): - if len(self.artists) < 4: - artist_str = ", ".join(x[1] for x in self.artists) - else: - artist_str = "Various Artists" - return u"".format( - artist_str, - self.name, - self.catalog_number, - ) - - def __init__(self, data): - BeatportObject.__init__(self, data) - if 'catalogNumber' in data: - self.catalog_number = data['catalogNumber'] - if 'label' in data: - self.label_name = data['label']['name'] - if 'category' in data: - self.category = data['category'] - if 'slug' in data: - self.url = "http://beatport.com/release/{0}/{1}".format( - data['slug'], data['id']) - - @classmethod - def from_id(cls, beatport_id): - response = BeatportAPI.get(cls.API_ENDPOINT, id=beatport_id) - release = BeatportRelease(response['release']) - release.tracks = [BeatportTrack(x) for x in response['tracks']] - return release - - def get_tracks(self): - response = BeatportAPI.get(self.API_ENDPOINT, id=self.beatport_id) - self.tracks = [BeatportTrack(x) for x in response['tracks']] - - -class BeatportTrack(BeatportObject): - API_ENDPOINT = 'catalog/3/beatport/track' - - def __unicode__(self): - artist_str = ", ".join(x[1] for x in self.artists) - return u"".format(artist_str, - self.name, - self.mix_name) - - def __init__(self, data): - BeatportObject.__init__(self, data) - if 'title' in data: - self.title = unicode(data['title']) - if 'mixName' in data: - self.mix_name = unicode(data['mixName']) - self.length = timedelta(milliseconds=data.get('lengthMs', 0) or 0) - if not self.length: - try: - min, sec = data.get('length', '0:0').split(':') - self.length = timedelta(minutes=int(min), seconds=int(sec)) - except ValueError: - pass - if 'slug' in data: - self.url = "http://beatport.com/track/{0}/{1}".format(data['slug'], - data['id']) - - @classmethod - def from_id(cls, beatport_id): - response = BeatportAPI.get(cls.API_ENDPOINT, id=beatport_id) - return BeatportTrack(response['track']) - - -class BeatportPlugin(BeetsPlugin): - def __init__(self): - super(BeatportPlugin, self).__init__() - self.config.add({ - 'source_weight': 0.5, - }) - - def album_distance(self, items, album_info, mapping): - """Returns the beatport source weight and the maximum source weight - for albums. - """ - dist = Distance() - if album_info.data_source == 'Beatport': - dist.add('source', self.config['source_weight'].as_number()) - return dist - - def track_distance(self, item, track_info): - """Returns the beatport source weight and the maximum source weight - for individual tracks. - """ - dist = Distance() - if track_info.data_source == 'Beatport': - dist.add('source', self.config['source_weight'].as_number()) - return dist - - def candidates(self, items, artist, release, va_likely): - """Returns a list of AlbumInfo objects for beatport search results - matching release and artist (if not various). - """ - if va_likely: - query = release - else: - query = '%s %s' % (artist, release) - try: - return self._get_releases(query) - except BeatportAPIError as e: - self._log.debug(u'API Error: {0} (query: {1})', e, query) - return [] - - def item_candidates(self, item, artist, title): - """Returns a list of TrackInfo objects for beatport search results - matching title and artist. - """ - query = '%s %s' % (artist, title) - try: - return self._get_tracks(query) - except BeatportAPIError as e: - self._log.debug(u'API Error: {0} (query: {1})', e, query) - return [] - - def album_for_id(self, release_id): - """Fetches a release by its Beatport ID and returns an AlbumInfo object - or None if the release is not found. - """ - self._log.debug(u'Searching for release {0}', release_id) - match = re.search(r'(^|beatport\.com/release/.+/)(\d+)$', release_id) - if not match: - return None - release = BeatportRelease.from_id(match.group(2)) - album = self._get_album_info(release) - return album - - def track_for_id(self, track_id): - """Fetches a track by its Beatport ID and returns a TrackInfo object - or None if the track is not found. - """ - self._log.debug(u'Searching for track {0}', track_id) - match = re.search(r'(^|beatport\.com/track/.+/)(\d+)$', track_id) - if not match: - return None - bp_track = BeatportTrack.from_id(match.group(2)) - track = self._get_track_info(bp_track) - return track - - def _get_releases(self, query): - """Returns a list of AlbumInfo objects for a beatport search query. - """ - # Strip non-word characters from query. Things like "!" and "-" can - # cause a query to return no results, even if they match the artist or - # album title. Use `re.UNICODE` flag to avoid stripping non-english - # word characters. - query = re.sub(r'\W+', ' ', query, re.UNICODE) - # Strip medium information from query, Things like "CD1" and "disk 1" - # can also negate an otherwise positive result. - query = re.sub(r'\b(CD|disc)\s*\d+', '', query, re.I) - albums = [self._get_album_info(x) - for x in BeatportSearch(query).results] - return albums - - def _get_album_info(self, release): - """Returns an AlbumInfo object for a Beatport Release object. - """ - va = len(release.artists) > 3 - artist, artist_id = self._get_artist(release.artists) - if va: - artist = u"Various Artists" - tracks = [self._get_track_info(x, index=idx) - for idx, x in enumerate(release.tracks, 1)] - - return AlbumInfo(album=release.name, album_id=release.beatport_id, - artist=artist, artist_id=artist_id, tracks=tracks, - albumtype=release.category, va=va, - year=release.release_date.year, - month=release.release_date.month, - day=release.release_date.day, - label=release.label_name, - catalognum=release.catalog_number, media=u'Digital', - data_source=u'Beatport', data_url=release.url) - - def _get_track_info(self, track, index=None): - """Returns a TrackInfo object for a Beatport Track object. - """ - title = track.name - if track.mix_name != u"Original Mix": - title += u" ({0})".format(track.mix_name) - artist, artist_id = self._get_artist(track.artists) - length = track.length.total_seconds() - - return TrackInfo(title=title, track_id=track.beatport_id, - artist=artist, artist_id=artist_id, - length=length, index=index, - data_source=u'Beatport', data_url=track.url) - - def _get_artist(self, artists): - """Returns an artist string (all artists) and an artist_id (the main - artist) for a list of Beatport release or track artists. - """ - artist_id = None - bits = [] - for artist in artists: - if not artist_id: - artist_id = artist[0] - name = artist[1] - # Strip disambiguation number. - name = re.sub(r' \(\d+\)$', '', name) - # Move articles to the front. - name = re.sub(r'^(.*?), (a|an|the)$', r'\2 \1', name, flags=re.I) - bits.append(name) - artist = ', '.join(bits).replace(' ,', ',') or None - return artist, artist_id - - def _get_tracks(self, query): - """Returns a list of TrackInfo objects for a Beatport query. - """ - bp_tracks = BeatportSearch(query, release_type='track').results - tracks = [self._get_track_info(x) for x in bp_tracks] - return tracks diff --git a/docs/changelog.rst b/docs/changelog.rst index fb9d1899f..d33c4444c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,9 @@ Fixes: mode. :bug:`1214` * :doc:`/plugins/smartplaylist`: Fix ``album_query`` so that individual files are added to the playlist instead of directories. :bug:`1225` +* Remove the ``beatport`` plugin. `Beatport`_ has shut off public access to + their API and denied our request for an account. We have not heard from the + company since 2013, so we are assuming access will not be restored. For developers: The logging system in beets has been overhauled. Plugins now each have their own logger, which helps by automatically adjusting the @@ -947,12 +950,12 @@ And some little enhancements and bug fixes: Thanks to John Hawthorn. * :doc:`/plugins/web`: Item and album counts are now exposed through the API for use with the Tomahawk resolver. Thanks to Uwe L. Korn. -* Python 2.6 compatibility for :doc:`/plugins/beatport`, +* Python 2.6 compatibility for ``beatport``, :doc:`/plugins/missing`, and :doc:`/plugins/duplicates`. Thanks to Wesley Bitter and Pedro Silva. * Don't move the config file during a null migration. Thanks to Theofilos Intzoglou. -* Fix an occasional crash in the :doc:`/plugins/beatport` when a length +* Fix an occasional crash in the ``beatport`` when a length field was missing from the API response. Thanks to Timothy Appnel. * :doc:`/plugins/scrub`: Handle and log I/O errors. * :doc:`/plugins/lyrics`: The Google backend should now turn up more results. @@ -977,7 +980,7 @@ these plugins, the importer will start showing you new kinds of matches: * New :doc:`/plugins/discogs`: Get matches from the `Discogs`_ database. Thanks to Artem Ponomarenko and Tai Lee. -* New :doc:`/plugins/beatport`: Get matches from the `Beatport`_ database. +* New ``beatport`` plugin: Get matches from the `Beatport`_ database. Thanks to Johannes Baiter. We also have two other new plugins that can scan your library to check for diff --git a/docs/plugins/beatport.rst b/docs/plugins/beatport.rst deleted file mode 100644 index b10ee8c59..000000000 --- a/docs/plugins/beatport.rst +++ /dev/null @@ -1,33 +0,0 @@ -Beatport Plugin -=============== - -.. warning:: - - As of October 2013, Beatport has `closed their API`_. We've contacted them - to attempt to gain access as a "partner." Until this happens, though, this - plugin won't work. - -The ``beatport`` plugin adds support for querying the `Beatport`_ catalogue -during the autotagging process. This can potentially be helpful for users -whose collection includes a lot of diverse electronic music releases, for which -both MusicBrainz and (to a lesser degree) Discogs show no matches. - -.. _Beatport: http://beatport.com -.. _closed their API: http://api.beatport.com - -Installation ------------- - -To use the ``beatport`` plugin, first enable it in your configuration (see -:ref:`using-plugins`). Then, install the `requests`_ -library (which we need for querying the Beatport API) by typing:: - - pip install requests - -And you're done. Matches from Beatport should now show up alongside matches -from MusicBrainz and other sources. - -If you have a Beatport ID or a URL for a release or track you want to tag, you -can just enter one of the two at the "enter Id" prompt in the importer. - -.. _requests: http://docs.python-requests.org/en/latest/ diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index 24b2cbf6c..a84222fa0 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -31,7 +31,6 @@ Each plugin has its own set of options that can be defined in a section bearing .. toctree:: :hidden: - beatport bpd bpm bucket @@ -83,7 +82,6 @@ Autotagger Extensions * :doc:`fromfilename`: Guess metadata for untagged tracks from their filenames. -.. _Beatport: http://www.beatport.com/ .. _Discogs: http://www.discogs.com/ Metadata diff --git a/setup.py b/setup.py index 802809a92..36998d39a 100755 --- a/setup.py +++ b/setup.py @@ -96,7 +96,6 @@ setup( # Plugin (optional) dependencies: extras_require={ - 'beatport': ['requests'], 'fetchart': ['requests'], 'chroma': ['pyacoustid'], 'discogs': ['discogs-client>=2.0.0'], From 6f2d9845b50cb756ec5a77ec56bec6b155403916 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 17 Jan 2015 14:20:53 -0800 Subject: [PATCH 13/13] Add an assertion for summarize_items (#1232) --- beets/importer.py | 5 ----- beets/ui/commands.py | 2 ++ 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index deb13fd24..710b1af03 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -464,9 +464,6 @@ class ImportTask(object): """ if self.choice_flag == action.ASIS: return list(self.items) - # FIXME this should be a simple attribute. There should be no - # need to retrieve the keys of `match.mapping`. This requires - # that we remove unmatched items from the list. elif self.choice_flag == action.APPLY: return self.match.mapping.keys() else: @@ -475,8 +472,6 @@ class ImportTask(object): def apply_metadata(self): """Copy metadata from match info to the items. """ - # TODO call should be more descriptive like - # apply_metadata(self.match, self.items) autotag.apply_metadata(self.match.info, self.match.mapping) def duplicate_items(self, lib): diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 2c0863b60..839263fc5 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -419,6 +419,8 @@ def summarize_items(items, singleton): this is an album or single-item import (if the latter, them `items` should only have one element). """ + assert items, "summarizing zero items" + summary_parts = [] if not singleton: summary_parts.append("{0} items".format(len(items)))