From 3804eb5b52ea482d36cc88ccf676b465c22bddb6 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 23:45:23 +0100 Subject: [PATCH 1/3] 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 f4b4847919f0b50b7096ffe4cfe8f96a465fd217 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Thu, 15 Jan 2015 11:47:35 +0100 Subject: [PATCH 2/3] =?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 3/3] 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