Merge pull request #1221 from brunal/crash-on-invalid-queries

Stop on invalid queries instead of ignoring them

beets/dbcore/query.py

@@ -20,6 +20,14 @@ from beets import util
 from datetime import datetime, timedelta
 
 
+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(InvalidQueryError, self).__init__(message)
+
+
 class Query(object):
     """An abstract class representing a query into the item database.
     """
@@ -139,15 +147,22 @@ 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)
+        try:
+            self.pattern = re.compile(self.pattern)
+        except re.error as exc:
+            # Invalid regular expression.
+            raise InvalidQueryError(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):
@@ -191,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
@@ -203,7 +221,7 @@ class NumericQuery(FieldQuery):
             try:
                 return float(s)
             except ValueError:
-                return None
+                raise InvalidQueryError(s, "an int or a float")
 
     def __init__(self, field, pattern, fast=True):
         super(NumericQuery, self).__init__(field, pattern, fast)

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.InvalidQueryError 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.

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

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, InvalidQueryError
 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(InvalidQueryError) as raised:
+            dbcore.query.NumericQuery('year', '199a')
+        self.assertIn('not an int', str(raised.exception))
+
+        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))
 
 
 class MatchTest(_common.TestCase):