From bcd57bd2b5f0c67b0ccfb650f96cbf60cf79328a Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Wed, 18 Mar 2015 18:46:24 +0100 Subject: [PATCH] Test queries building sort management in smartplaylist Slighly modify Sort parsing: avoid building MultiplSort() instances comptised of a single sort, but return that sort instead, since it wraps things with any gain. --- beets/dbcore/query.py | 21 +++++++++++++++++++++ beets/dbcore/queryparse.py | 6 ++++-- beetsplug/smartplaylist.py | 15 ++++++++++----- test/test_dbcore.py | 11 ++++++----- test/test_smartplaylist.py | 27 ++++++++++++++++++++++++++- 5 files changed, 67 insertions(+), 13 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index a51805bed..965c13868 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -625,6 +625,12 @@ class Sort(object): """ return False + def __hash__(self): + return 0 + + def __eq__(self, other): + return type(self) == type(other) + class MultipleSort(Sort): """Sort that encapsulates multiple sub-sorts. @@ -686,6 +692,13 @@ class MultipleSort(Sort): def __repr__(self): return u'MultipleSort({0})'.format(repr(self.sorts)) + def __hash__(self): + return hash(tuple(self.sorts)) + + def __eq__(self, other): + return super(MultipleSort, self).__eq__(other) and \ + self.sorts == other.sorts + class FieldSort(Sort): """An abstract sort criterion that orders by a specific field (of @@ -709,6 +722,14 @@ class FieldSort(Sort): '+' if self.ascending else '-', ) + def __hash__(self): + return hash((self.field, self.ascending)) + + def __eq__(self, other): + return super(FieldSort, self).__eq__(other) and \ + self.field == other.field and \ + self.ascending == other.ascending + class FixedFieldSort(FieldSort): """Sort object to sort on a fixed field. diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py index 6628bebf0..1dcf9c4b3 100644 --- a/beets/dbcore/queryparse.py +++ b/beets/dbcore/queryparse.py @@ -152,12 +152,14 @@ def sort_from_strings(model_cls, sort_parts): """Create a `Sort` from a list of sort criteria (strings). """ if not sort_parts: - return query.NullSort() + sort = query.NullSort() + elif len(sort_parts) == 1: + sort = construct_sort_part(model_cls, sort_parts[0]) else: sort = query.MultipleSort() for part in sort_parts: sort.add_sort(construct_sort_part(model_cls, part)) - return sort + return sort def parse_sorted_query(model_cls, parts, prefixes={}, diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 80e1faa89..34fa94979 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -107,14 +107,19 @@ class SmartPlaylistPlugin(BeetsPlugin): queries, sorts = zip(*(parse_query_string(q, Model) for q in qs)) query = OrQuery(queries) - sort = MultipleSort() + final_sorts = [] for s in sorts: if s: - sort.add_sort(s) - if not sort.sorts: + if isinstance(s, MultipleSort): + final_sorts += s.sorts + else: + final_sorts.append(s) + if not final_sorts: sort = None - elif len(sort.sorts) == 1: - sort = sort.sorts[0] + elif len(final_sorts) == 1: + sort, = final_sorts + else: + sort = MultipleSort(final_sorts) query_and_sort = query, sort playlist_data += (query_and_sort,) diff --git a/test/test_dbcore.py b/test/test_dbcore.py index dffe9ae75..39867ceb0 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -449,6 +449,7 @@ class SortFromStringsTest(unittest.TestCase): def test_zero_parts(self): s = self.sfs([]) self.assertIsInstance(s, dbcore.query.NullSort) + self.assertEqual(s, dbcore.query.NullSort()) def test_one_parts(self): s = self.sfs(['field+']) @@ -461,17 +462,17 @@ class SortFromStringsTest(unittest.TestCase): def test_fixed_field_sort(self): s = self.sfs(['field_one+']) - self.assertIsInstance(s, dbcore.query.MultipleSort) - self.assertIsInstance(s.sorts[0], dbcore.query.FixedFieldSort) + self.assertIsInstance(s, dbcore.query.FixedFieldSort) + self.assertEqual(s, dbcore.query.FixedFieldSort('field_one')) def test_flex_field_sort(self): s = self.sfs(['flex_field+']) - self.assertIsInstance(s, dbcore.query.MultipleSort) - self.assertIsInstance(s.sorts[0], dbcore.query.SlowFieldSort) + self.assertIsInstance(s, dbcore.query.SlowFieldSort) + self.assertEqual(s, dbcore.query.SlowFieldSort('flex_field')) def test_special_sort(self): s = self.sfs(['some_sort+']) - self.assertIsInstance(s.sorts[0], TestSort) + self.assertIsInstance(s, TestSort) class ResultsIteratorTest(unittest.TestCase): diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py index 42bffc8a5..4ed2cb4ba 100644 --- a/test/test_smartplaylist.py +++ b/test/test_smartplaylist.py @@ -24,7 +24,7 @@ from mock import Mock, MagicMock from beetsplug.smartplaylist import SmartPlaylistPlugin from beets.library import Item, Album, parse_query_string from beets.dbcore import OrQuery -from beets.dbcore.query import NullSort +from beets.dbcore.query import NullSort, MultipleSort, FixedFieldSort from beets.util import syspath from beets.ui import UserError from beets import config @@ -66,6 +66,31 @@ class SmartPlaylistTest(unittest.TestCase): ('bar', (None, None), (bar_bar, None)), ])) + def test_build_queries_with_sorts(self): + spl = SmartPlaylistPlugin() + config['smartplaylist']['playlists'].set([ + {'name': 'no_sort', 'query': 'foo'}, + {'name': 'one_sort', 'query': 'foo year+'}, + {'name': 'only_empty_sorts', 'query': ['foo', 'bar']}, + {'name': 'one_non_empty_sort', 'query': ['foo year+', 'bar']}, + {'name': 'multiple_sorts', 'query': ['foo year+', 'bar genre-']}, + {'name': 'mixed', 'query': ['foo year+', 'bar', 'baz genre+ id-']} + ]) + + spl.build_queries() + sorts = {name: sort for name, (_, sort), _ in spl._unmatched_playlists} + + asseq = self.assertEqual # less cluttered code + S = FixedFieldSort # short cut since we're only dealing with this + asseq(sorts["no_sort"], NullSort()) + asseq(sorts["one_sort"], S('year')) + asseq(sorts["only_empty_sorts"], None) + asseq(sorts["one_non_empty_sort"], S('year')) + asseq(sorts["multiple_sorts"], + MultipleSort([S('year'), S('genre', False)])) + asseq(sorts["mixed"], + MultipleSort([S('year'), S('genre'), S('id', False)])) + def test_db_changes(self): spl = SmartPlaylistPlugin()