From 3804eb5b52ea482d36cc88ccf676b465c22bddb6 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Tue, 13 Jan 2015 23:45:23 +0100
Subject: [PATCH] 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):