From 54887e7655351b58bf581fbd20f8ccadce367110 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Sun, 1 Feb 2015 09:37:36 +0100 Subject: [PATCH 1/3] Widen usage of InvalidQueryError Replace previous InvalidQueryError with InvalidQueryArgumentTypeError which extends the former as TypeError. However they lack context: the query that caused the error. Raise an InvalidQueryError when a shell-like expression cannot be parsed by shlex. Improve #1290. --- beets/dbcore/__init__.py | 1 + beets/dbcore/query.py | 15 +++++++++++---- beets/library.py | 2 +- test/test_library.py | 7 +++++++ test/test_query.py | 9 +++++---- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/beets/dbcore/__init__.py b/beets/dbcore/__init__.py index 100f546b5..093591882 100644 --- a/beets/dbcore/__init__.py +++ b/beets/dbcore/__init__.py @@ -23,5 +23,6 @@ from .types import Type from .queryparse import query_from_strings from .queryparse import sort_from_strings from .queryparse import parse_sorted_query +from .query import InvalidQueryError # flake8: noqa diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 2f90e0398..8d9c1929c 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -24,11 +24,17 @@ from datetime import datetime, timedelta class InvalidQueryError(ValueError): + def __init__(self, query, explanation): + message = "Invalid query '{0}': {1}".format(query, explanation) + super(InvalidQueryError, self).__init__(message) + + +class InvalidQueryArgumentTypeError(InvalidQueryError, TypeError): def __init__(self, what, expected, detail=None): message = "'{0}' is not {1}".format(what, expected) if detail: message = "{0}: {1}".format(message, detail) - super(InvalidQueryError, self).__init__(message) + super(InvalidQueryArgumentTypeError, self).__init__(None, message) class Query(object): @@ -160,8 +166,9 @@ class RegexpQuery(StringFieldQuery): self.pattern = re.compile(self.pattern) except re.error as exc: # Invalid regular expression. - raise InvalidQueryError(pattern, "a regular expression", - format(exc)) + raise InvalidQueryArgumentTypeError(pattern, + "a regular expression", + format(exc)) @classmethod def string_match(cls, pattern, value): @@ -228,7 +235,7 @@ class NumericQuery(FieldQuery): try: return float(s) except ValueError: - raise InvalidQueryError(s, "an int or a float") + raise InvalidQueryArgumentTypeError(s, "an int or a float") def __init__(self, field, pattern, fast=True): super(NumericQuery, self).__init__(field, pattern, fast) diff --git a/beets/library.py b/beets/library.py index 40051c354..a9885918b 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1085,7 +1085,7 @@ def parse_query_string(s, model_cls): try: parts = [p.decode('utf8') for p in shlex.split(s)] except ValueError as exc: - raise ValueError("Cannot parse {0!r} (error was: {1})".format(s, exc)) + raise dbcore.InvalidQueryError(s, exc) return parse_query_parts(parts, model_cls) diff --git a/test/test_library.py b/test/test_library.py index 3848f2b7c..d3bfe1373 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -31,6 +31,7 @@ from test._common import unittest from test._common import item import beets.library import beets.mediafile +import beets.dbcore from beets import util from beets import plugins from beets import config @@ -1171,6 +1172,12 @@ class ItemReadTest(unittest.TestCase): item.read('/thisfiledoesnotexist') +class ParseQueryTest(unittest.TestCase): + def test_parse_invalid_query_string(self): + with self.assertRaises(beets.dbcore.InvalidQueryError): + beets.library.parse_query_string('foo"', None) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/test/test_query.py b/test/test_query.py index 55450d0ad..077518c36 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -23,8 +23,8 @@ from test import helper import beets.library from beets import dbcore -from beets.dbcore import types -from beets.dbcore.query import NoneQuery, InvalidQueryError +from beets.dbcore import types, InvalidQueryError +from beets.dbcore.query import NoneQuery, InvalidQueryArgumentTypeError from beets.library import Library, Item @@ -282,14 +282,15 @@ class GetTest(DummyDataTestCase): self.assertFalse(results) def test_invalid_query(self): - with self.assertRaises(InvalidQueryError) as raised: + with self.assertRaises(InvalidQueryArgumentTypeError) as raised: dbcore.query.NumericQuery('year', '199a') self.assertIn('not an int', unicode(raised.exception)) - with self.assertRaises(InvalidQueryError) as raised: + with self.assertRaises(InvalidQueryArgumentTypeError) as raised: dbcore.query.RegexpQuery('year', '199(') self.assertIn('not a regular expression', unicode(raised.exception)) self.assertIn('unbalanced parenthesis', unicode(raised.exception)) + self.assertIsInstance(raised.exception, (InvalidQueryError, TypeError)) class MatchTest(_common.TestCase): From f443e0bfc5d1114b1dbc182ea223aab23d9f6a49 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Mon, 9 Feb 2015 15:44:49 +0100 Subject: [PATCH 2/3] InvalidQueryArgumentTypeError does not extend InvalidQueryError Places where InvalidQueryArgumentTypeError may be raised (i.e. all current ones) may not know the query therefore it cannot be an InvalidQueryError. The InvalidQueryArgumentTypeError is caught in beets.library.Library._fetch() and an InvalidQueryError is then raised. Improve #1290. --- beets/dbcore/query.py | 17 ++++++++++++++--- beets/library.py | 13 ++++++++----- test/test_query.py | 4 ++-- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 8d9c1929c..3727f6d7f 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -24,17 +24,28 @@ from datetime import datetime, timedelta class InvalidQueryError(ValueError): + """Represent any kind of invalid query + + The query should be a unicode string or a list, which will be space-joined. + """ def __init__(self, query, explanation): - message = "Invalid query '{0}': {1}".format(query, explanation) + if isinstance(query, list): + query = " ".join(query) + message = "'{0}': {1}".format(query, explanation) super(InvalidQueryError, self).__init__(message) -class InvalidQueryArgumentTypeError(InvalidQueryError, TypeError): +class InvalidQueryArgumentTypeError(TypeError): + """Represent a query argument that could not be converted as expected. + + It exists to be caught in upper stack levels so a meaningful (i.e. with the + query) InvalidQueryError can be raised. + """ def __init__(self, what, expected, detail=None): message = "'{0}' is not {1}".format(what, expected) if detail: message = "{0}: {1}".format(message, detail) - super(InvalidQueryArgumentTypeError, self).__init__(None, message) + super(InvalidQueryArgumentTypeError, self).__init__(message) class Query(object): diff --git a/beets/library.py b/beets/library.py index a9885918b..a57ed1642 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1155,11 +1155,14 @@ class Library(dbcore.Database): in the query string the `sort` argument is ignored. """ # Parse the query, if necessary. - parsed_sort = None - if isinstance(query, basestring): - query, parsed_sort = parse_query_string(query, model_cls) - elif isinstance(query, (list, tuple)): - query, parsed_sort = parse_query_parts(query, model_cls) + try: + parsed_sort = None + if isinstance(query, basestring): + query, parsed_sort = parse_query_string(query, model_cls) + elif isinstance(query, (list, tuple)): + query, parsed_sort = parse_query_parts(query, model_cls) + except dbcore.query.InvalidQueryArgumentTypeError as exc: + raise dbcore.InvalidQueryError(query, exc) # Any non-null sort specified by the parsed query overrides the # provided sort. diff --git a/test/test_query.py b/test/test_query.py index 077518c36..a32d8d60d 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -23,7 +23,7 @@ from test import helper import beets.library from beets import dbcore -from beets.dbcore import types, InvalidQueryError +from beets.dbcore import types from beets.dbcore.query import NoneQuery, InvalidQueryArgumentTypeError from beets.library import Library, Item @@ -290,7 +290,7 @@ class GetTest(DummyDataTestCase): dbcore.query.RegexpQuery('year', '199(') self.assertIn('not a regular expression', unicode(raised.exception)) self.assertIn('unbalanced parenthesis', unicode(raised.exception)) - self.assertIsInstance(raised.exception, (InvalidQueryError, TypeError)) + self.assertIsInstance(raised.exception, TypeError) class MatchTest(_common.TestCase): From 7476d6be463399ce32359302dec4b275339461e5 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Mon, 9 Feb 2015 19:25:23 +0100 Subject: [PATCH 3/3] InvalidQuery*Error extend ParsingError And InvalidQueryArgumentTypeError does not extend TypeError anymore. --- beets/dbcore/query.py | 12 +++++++++--- test/test_library.py | 6 ++++-- test/test_query.py | 5 +++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 3727f6d7f..cd891148e 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -23,8 +23,14 @@ from beets import util from datetime import datetime, timedelta -class InvalidQueryError(ValueError): - """Represent any kind of invalid query +class ParsingError(ValueError): + """Abstract class for any unparseable user-requested album/query + specification. + """ + + +class InvalidQueryError(ParsingError): + """Represent any kind of invalid query. The query should be a unicode string or a list, which will be space-joined. """ @@ -35,7 +41,7 @@ class InvalidQueryError(ValueError): super(InvalidQueryError, self).__init__(message) -class InvalidQueryArgumentTypeError(TypeError): +class InvalidQueryArgumentTypeError(ParsingError): """Represent a query argument that could not be converted as expected. It exists to be caught in upper stack levels so a meaningful (i.e. with the diff --git a/test/test_library.py b/test/test_library.py index d3bfe1373..0bac0f173 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -31,7 +31,7 @@ from test._common import unittest from test._common import item import beets.library import beets.mediafile -import beets.dbcore +import beets.dbcore.query from beets import util from beets import plugins from beets import config @@ -1174,8 +1174,10 @@ class ItemReadTest(unittest.TestCase): class ParseQueryTest(unittest.TestCase): def test_parse_invalid_query_string(self): - with self.assertRaises(beets.dbcore.InvalidQueryError): + with self.assertRaises(beets.dbcore.InvalidQueryError) as raised: beets.library.parse_query_string('foo"', None) + self.assertIsInstance(raised.exception, + beets.dbcore.query.ParsingError) def suite(): diff --git a/test/test_query.py b/test/test_query.py index a32d8d60d..a9b1058bd 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -24,7 +24,8 @@ from test import helper import beets.library from beets import dbcore from beets.dbcore import types -from beets.dbcore.query import NoneQuery, InvalidQueryArgumentTypeError +from beets.dbcore.query import (NoneQuery, ParsingError, + InvalidQueryArgumentTypeError) from beets.library import Library, Item @@ -290,7 +291,7 @@ class GetTest(DummyDataTestCase): dbcore.query.RegexpQuery('year', '199(') self.assertIn('not a regular expression', unicode(raised.exception)) self.assertIn('unbalanced parenthesis', unicode(raised.exception)) - self.assertIsInstance(raised.exception, TypeError) + self.assertIsInstance(raised.exception, ParsingError) class MatchTest(_common.TestCase):