From 1303a915c17030ccacd8ccb5695070a4ded8442b Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Wed, 11 Jun 2014 16:21:43 +0200 Subject: [PATCH 01/22] Sort implementation * sort can be sepcified using the 'field_name'(+|-) syntax * supports fixed fields and flexible attributes * includes plugins fix for API changes (might have missed some) --- beets/dbcore/__init__.py | 1 + beets/dbcore/db.py | 25 +++--- beets/dbcore/query.py | 160 +++++++++++++++++++++++++++++++++++++ beets/dbcore/queryparse.py | 28 +++++++ beets/library.py | 50 +++++++----- beetsplug/ihate.py | 4 +- beetsplug/smartplaylist.py | 2 +- test/test_dbcore.py | 31 +++++++ 8 files changed, 263 insertions(+), 38 deletions(-) diff --git a/beets/dbcore/__init__.py b/beets/dbcore/__init__.py index fed65f482..fdf6b4695 100644 --- a/beets/dbcore/__init__.py +++ b/beets/dbcore/__init__.py @@ -19,5 +19,6 @@ from .db import Model, Database from .query import Query, FieldQuery, MatchQuery, AndQuery, OrQuery from .types import Type from .queryparse import query_from_strings +from .queryparse import sort_from_strings # flake8: noqa diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 68f144b7a..e12178784 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -24,7 +24,7 @@ import collections import beets from beets.util.functemplate import Template -from .query import MatchQuery +from .query import MatchQuery, build_sql from .types import BASE_TYPE @@ -509,7 +509,10 @@ class Results(object): ), (row['id'],) ) - values = dict(row) + + cols = dict(row) + values = dict((k, v) for (k, v) in cols.items() + if not k[:4] == 'flex') flex_values = dict((row['key'], row['value']) for row in flex_rows) # Construct the Python object and yield it if it passes the @@ -739,24 +742,20 @@ class Database(object): # Querying. - def _fetch(self, model_cls, query, order_by=None): + def _fetch(self, model_cls, query, sort_order=None): """Fetch the objects of type `model_cls` matching the given query. The query may be given as a string, string sequence, a Query object, or None (to fetch everything). If provided, - `order_by` is a SQLite ORDER BY clause for sorting. - """ - where, subvals = query.clause() + `sort_order` is either a SQLite ORDER BY clause for sorting or a + Sort object. + """ + + sql, subvals, is_slow = build_sql(model_cls, query, sort_order) - sql = "SELECT * FROM {0} WHERE {1}".format( - model_cls._table, - where or '1', - ) - if order_by: - sql += " ORDER BY {0}".format(order_by) with self.transaction() as tx: rows = tx.query(sql, subvals) - return Results(model_cls, rows, self, None if where else query) + return Results(model_cls, rows, self, None if not is_slow else query) def _get(self, model_cls, id): """Get a Model object by its id or None if the id does not diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 805f2cac9..54f6df920 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -497,3 +497,163 @@ class DateQuery(FieldQuery): # Match any date. clause = '1' return clause, subvals + + +class Sort(object): + """An abstract class representing a sort opertation for a query into the + item database. + """ + def select_clause(self): + """ Generates a select sql fragment. + """ + return None + + def union_clause(self): + """ Generates a union sql fragment or None if the Sort is a slow sort. + """ + return None + + def order_clause(self): + """Generates a sql fragment to be use in a ORDER BY clause + or None if it's a slow query + """ + return None + + def sort(self, items): + """Sort the given items list. Meant to be used with slow queries. + """ + return items + + +class MultipleSort(Sort): + """ Sort class that combines several sort criteria. + """ + + def __init__(self): + self.sorts = [] + + def add_criteria(self, sort): + self.sorts.append(sort) + + def select_clause(self): + """ Generate a select sql fragment. + """ + select_strings = [] + index = 0 + for sort in self.sorts: + select = sort.select_clause() + if select is None: + # FIXME : sort for slow sort + break + else: + select_strings.append(select) + index = index + 1 + + select_string = ",".join(select_strings) + return "" if not select_string else ", " + select_string + + def union_clause(self): + """ Returns a union sql fragment. + """ + union_strings = [] + for sort in self.sorts: + union = sort.union_clause() + if union is None: + pass + else: + union_strings.append(union) + + return "".join(union_strings) + + def order_clause(self): + """Returns a sql fragment to be use in a ORDER BY clause + or None if it's a slow query + """ + order_strings = [] + index = 0 + for sort in self.sorts: + order = sort.order_clause() + if order is None: + break + else: + order_strings.append(order) + index = index + 1 + + return ",".join(order_strings) + + def sort(self, items): + # FIXME : sort according to criteria following the first slow sort + return items + + +class FlexFieldSort(Sort): + + def __init__(self, model_cls, field, is_ascending): + self.model_cls = model_cls + self.field = field + self.is_ascending = is_ascending + + def select_clause(self): + """ Return a select sql fragment. + """ + return "sort_flexattr{0!s}.value as flex_{0!s} ".format(self.field) + + def union_clause(self): + """ Returns an union sql fragment. + """ + return "LEFT JOIN {flextable} as sort_flexattr{index!s} \ + ON {table}.id = sort_flexattr{index!s}.entity_id \ + AND sort_flexattr{index!s}.key='{flexattr}' ".format( + flextable=self.model_cls._flex_table, + table=self.model_cls._table, + index=self.field, flexattr=self.field) + + def order_clause(self): + """ Returns an order sql fragment. + """ + order = "ASC" if self.is_ascending else "DESC" + return "flex_{0} {1} ".format(self.field, order) + + +class FixedFieldSort(Sort): + + def __init__(self, field, is_ascending=True): + self.field = field + self.is_ascending = is_ascending + + """ Sort on a fixed field + """ + def order_clause(self): + order = "ASC" if self.is_ascending else "DESC" + return "{0} {1}".format(self.field, order) + + +def build_sql(model_cls, query, sort_order): + """ Generate a sql statement (and the values that must be injected into it) + from a query, sort and a model class. + """ + where, subvals = query.clause() + + if not sort_order: + sort_select = "" + sort_union = "" + sort_order = "" + elif isinstance(sort_order, basestring): + sort_select = "" + sort_union = "" + sort_order = " ORDER BY {0}".format(sort_order) + elif isinstance(sort_order, Sort): + sort_select = sort_order.select_clause() + sort_union = sort_order.union_clause() + sort_order = " ORDER BY {0}".format(sort_order.order_clause()) + + sql = "SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE \ + {query_clause} {sort_order}".format( + sort_select=sort_select, + sort_union=sort_union, + table=model_cls._table, + query_clause=where or '1', + sort_order=sort_order + ) + + return (sql, subvals, where is None) \ No newline at end of file diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py index a767b56d1..05ee28add 100644 --- a/beets/dbcore/queryparse.py +++ b/beets/dbcore/queryparse.py @@ -121,3 +121,31 @@ def query_from_strings(query_cls, model_cls, prefixes, query_parts): if not subqueries: # No terms in query. subqueries = [query.TrueQuery()] return query_cls(subqueries) + + +def construct_sort_part(model_cls, part): + """ Creates a Sort object from a single criteria. Returns a `Sort` instance. + """ + sort = None + field = part[:-1] + is_ascending = (part[-1] == '+') + if field in model_cls._fields: + sort = query.FixedFieldSort(field, is_ascending) + elif field in model_cls._getters(): + # Computed field, all following fields must use the slow path. + pass + else: + # Neither fixed nor computed : must be a flex attr. + sort = query.FlexFieldSort(model_cls, field, is_ascending) + return sort + + +def sort_from_strings(model_cls, sort_parts): + """Creates a Sort object from a list of sort criteria strings. + """ + if not sort_parts: + return None + sort = query.MultipleSort() + for part in sort_parts: + sort.add_criteria(construct_sort_part(model_cls, part)) + return sort \ No newline at end of file diff --git a/beets/library.py b/beets/library.py index 8e3d5f1e9..61ddfab96 100644 --- a/beets/library.py +++ b/beets/library.py @@ -546,7 +546,7 @@ class Item(LibModel): for query, path_format in path_formats: if query == PF_KEY_DEFAULT: continue - query = get_query(query, type(self)) + (query, _) = get_query(query, type(self)) if query.match(self): # The query matches the item! Use the corresponding path # format. @@ -889,7 +889,8 @@ class Album(LibModel): def get_query(val, model_cls): """Take a value which may be None, a query string, a query string - list, or a Query object, and return a suitable Query object. + list, or a Query object, and return a suitable Query object and Sort + object. `model_cls` is the subclass of Model indicating which entity this is a query for (i.e., Album or Item) and is used to determine which @@ -910,7 +911,7 @@ def get_query(val, model_cls): val = [s.decode('utf8') for s in shlex.split(val)] if val is None: - return dbcore.query.TrueQuery() + return (dbcore.query.TrueQuery(), None) elif isinstance(val, list) or isinstance(val, tuple): # Special-case path-like queries, which are non-field queries @@ -928,18 +929,23 @@ def get_query(val, model_cls): path_parts = () non_path_parts = val + # separate query token and sort token + query_val = [s for s in non_path_parts if not s.endswith(('+', '-'))] + sort_val = [s for s in non_path_parts if s.endswith(('+', '-'))] + # Parse remaining parts and construct an AndQuery. query = dbcore.query_from_strings( - dbcore.AndQuery, model_cls, prefixes, non_path_parts + dbcore.AndQuery, model_cls, prefixes, query_val ) + sort = dbcore.sort_from_strings(model_cls, sort_val) # Add path queries to aggregate query. if path_parts: query.subqueries += [PathQuery('path', s) for s in path_parts] - return query + return (query, sort) elif isinstance(val, dbcore.Query): - return val + return (val, None) else: raise ValueError('query must be None or have type Query or str') @@ -1006,30 +1012,30 @@ class Library(dbcore.Database): # Querying. - def _fetch(self, model_cls, query, order_by=None): - """Parse a query and fetch. - """ + def _fetch(self, model_cls, query, sort_order=None): + """Parse a query and fetch. If a sort_order is explicitly given, + any sort order specification present in the query string is ignored. + """ + (query, sort) = get_query(query, model_cls) + sort = sort if sort_order is None else sort_order + return super(Library, self)._fetch( - model_cls, get_query(query, model_cls), order_by + model_cls, query, sort ) - def albums(self, query=None): + def albums(self, query=None, sort_order=None): """Get a sorted list of :class:`Album` objects matching the - given query. + given sort order. If a sort_order is explicitly given, + any sort order specification present in the query string is ignored. """ - order = '{0}, album'.format( - _orelse("albumartist_sort", "albumartist") - ) - return self._fetch(Album, query, order) + return self._fetch(Album, query, sort_order) - def items(self, query=None): + def items(self, query=None, sort_order=None): """Get a sorted list of :class:`Item` objects matching the given - query. + given sort order. If a sort_order is explicitly given, + any sort order specification present in the query string is ignored. """ - order = '{0}, album, disc, track'.format( - _orelse("artist_sort", "artist") - ) - return self._fetch(Item, query, order) + return self._fetch(Item, query, sort_order) # Convenience accessors. diff --git a/beetsplug/ihate.py b/beetsplug/ihate.py index 3cd6ace41..85e3382d4 100644 --- a/beetsplug/ihate.py +++ b/beetsplug/ihate.py @@ -57,9 +57,9 @@ class IHatePlugin(BeetsPlugin): for query_string in action_patterns: query = None if task.is_album: - query = get_query(query_string, Album) + (query, _) = get_query(query_string, Album) else: - query = get_query(query_string, Item) + (query, _) = get_query(query_string, Item) if any(query.match(item) for item in task.imported_items()): return True return False diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 118dc361b..12672ce42 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -42,7 +42,7 @@ def _items_for_query(lib, playlist, album=False): query_strings = [query_strings] model = library.Album if album else library.Item query = dbcore.OrQuery( - [library.get_query(q, model) for q in query_strings] + [library.get_query(q, model)[0] for q in query_strings] ) # Execute query, depending on type. diff --git a/test/test_dbcore.py b/test/test_dbcore.py index a4be181e4..e55bd84db 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -412,6 +412,37 @@ class QueryFromStringsTest(_common.TestCase): self.assertIsInstance(q.subqueries[0], dbcore.query.NumericQuery) +class SortFromStringsTest(_common.TestCase): + def sfs(self, strings): + return dbcore.queryparse.sort_from_strings( + TestModel1, + strings, + ) + + def test_zero_parts(self): + s = self.sfs([]) + self.assertIsNone(s) + + def test_one_parts(self): + s = self.sfs(['field+']) + self.assertIsInstance(s, dbcore.query.Sort) + + def test_two_parts(self): + s = self.sfs(['field+', 'another_field-']) + self.assertIsInstance(s, dbcore.query.MultipleSort) + self.assertEqual(len(s.sorts), 2) + + 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) + + def test_flex_field_sort(self): + s = self.sfs(['flex_field+']) + self.assertIsInstance(s, dbcore.query.MultipleSort) + self.assertIsInstance(s.sorts[0], dbcore.query.FlexFieldSort) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 3d7814c32eadf1966f21a199ccc82011011eb807 Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Wed, 11 Jun 2014 22:26:22 +0200 Subject: [PATCH 02/22] Use implicit continuation for sql generation. Avoid newlines in the query string. --- beets/dbcore/query.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 54f6df920..21ba3b572 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -601,12 +601,13 @@ class FlexFieldSort(Sort): def union_clause(self): """ Returns an union sql fragment. """ - return "LEFT JOIN {flextable} as sort_flexattr{index!s} \ - ON {table}.id = sort_flexattr{index!s}.entity_id \ - AND sort_flexattr{index!s}.key='{flexattr}' ".format( + union = ("LEFT JOIN {flextable} as sort_flexattr{index!s} " + "ON {table}.id = sort_flexattr{index!s}.entity_id " + "AND sort_flexattr{index!s}.key='{flexattr}' ").format( flextable=self.model_cls._flex_table, table=self.model_cls._table, index=self.field, flexattr=self.field) + return union def order_clause(self): """ Returns an order sql fragment. @@ -647,8 +648,8 @@ def build_sql(model_cls, query, sort_order): sort_union = sort_order.union_clause() sort_order = " ORDER BY {0}".format(sort_order.order_clause()) - sql = "SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE \ - {query_clause} {sort_order}".format( + sql = ("SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE " + "{query_clause} {sort_order}").format( sort_select=sort_select, sort_union=sort_union, table=model_cls._table, From 4958ce83c3b81e2c5fc8b104c0cec437415ea4e5 Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Wed, 11 Jun 2014 22:54:39 +0200 Subject: [PATCH 03/22] Add defaut sort on artist_sort field. For items and albums, defaulting on artist field if artist field is empty. --- beets/dbcore/query.py | 30 ++++++++++++++++++++++++++++++ beets/library.py | 19 ++++++++++++------- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 21ba3b572..97b8833bb 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -628,6 +628,36 @@ class FixedFieldSort(Sort): order = "ASC" if self.is_ascending else "DESC" return "{0} {1}".format(self.field, order) +class SmartArtistSort(Sort): + """ Sort Album or Item on artist sort fields, defaulting back on + artist field if the sort specific field is empty. + """ + def __init__(self, model_cls, is_ascending=True): + self.model_cls = model_cls + self.is_ascending = is_ascending + + def select_clause(self): + return "" + + def union_clause(self): + return "" + + def order_clause(self): + order = "ASC" if self.is_ascending else "DESC" + if 'albumartist_sort' in self.model_cls._fields: + exp1 = 'albumartist_sort' + exp2 = 'albumartist' + elif 'artist_sort' in self.model_cls_fields: + exp1 = 'artist_sort' + exp2 = 'artist' + else: + return "" + + order_str = ('(CASE {0} WHEN NULL THEN {1} ' + 'WHEN "" THEN {1} ' + 'ELSE {0} END) {2} ').format(exp1, exp2, order) + return order_str + def build_sql(model_cls, query, sort_order): """ Generate a sql statement (and the values that must be injected into it) diff --git a/beets/library.py b/beets/library.py index 61ddfab96..3965b6fb9 100644 --- a/beets/library.py +++ b/beets/library.py @@ -29,6 +29,7 @@ from beets.util import bytestring_path, syspath, normpath, samefile from beets.util.functemplate import Template from beets import dbcore from beets.dbcore import types +from beets.dbcore.query import SmartArtistSort import beets @@ -1013,11 +1014,11 @@ class Library(dbcore.Database): # Querying. def _fetch(self, model_cls, query, sort_order=None): - """Parse a query and fetch. If a sort_order is explicitly given, - any sort order specification present in the query string is ignored. + """Parse a query and fetch. If a order specification is present in the + query string the sort_order argument is ignored. """ (query, sort) = get_query(query, model_cls) - sort = sort if sort_order is None else sort_order + sort = sort_order if sort is None else sort return super(Library, self)._fetch( model_cls, query, sort @@ -1025,16 +1026,20 @@ class Library(dbcore.Database): def albums(self, query=None, sort_order=None): """Get a sorted list of :class:`Album` objects matching the - given sort order. If a sort_order is explicitly given, - any sort order specification present in the query string is ignored. + given sort order. If a order specification is present in the query + string the sort_order argument is ignored. """ + if sort_order is None: + sort_order = SmartArtistSort(Album) return self._fetch(Album, query, sort_order) def items(self, query=None, sort_order=None): """Get a sorted list of :class:`Item` objects matching the given - given sort order. If a sort_order is explicitly given, - any sort order specification present in the query string is ignored. + given sort order. If a order specification is present in the query + string the sort_order argument is ignored. """ + if sort_order is None: + sort_order = SmartArtistSort(Item) return self._fetch(Item, query, sort_order) # Convenience accessors. From 342636377513e0f1fb4010b366fee44fcf0300df Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Thu, 12 Jun 2014 09:14:05 +0200 Subject: [PATCH 04/22] Add documentation about sort specification. --- docs/reference/cli.rst | 5 +++-- docs/reference/query.rst | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 4ff0346a6..317dbd00d 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -169,8 +169,9 @@ list Want to search for "Gronlandic Edit" by of Montreal? Try ``beet list gronlandic``. Maybe you want to see everything released in 2009 with -"vegetables" in the title? Try ``beet list year:2009 title:vegetables``. (Read -more in :doc:`query`.) +"vegetables" in the title? Try ``beet list year:2009 title:vegetables``. You +can also specify the order used when outputting the results (Read more in +:doc:`query`.) You can use the ``-a`` switch to search for albums instead of individual items. In this case, the queries you use are restricted to album-level fields: for diff --git a/docs/reference/query.rst b/docs/reference/query.rst index cf981bed2..a224c8dc9 100644 --- a/docs/reference/query.rst +++ b/docs/reference/query.rst @@ -183,3 +183,25 @@ equivalent:: Note that this only matches items that are *already in your library*, so a path query won't necessarily find *all* the audio files in a directory---just the ones you've already added to your beets library. + + +Sort Order +---------- + +You can also specify the order used when outputting the results. Of course, this +is only useful when displaying the result, for example with the ``list`` +command, and is useless when the query is used as a filter for an command. Use +the name of the `field` you want to sort on, followed by a ``+`` or ``-`` sign +if you want ascending or descending sort. For example this command:: + + $ beet list -a year+ + +will list all albums in chronological order. + +You can also specify several sort orders, which will be used in the same order at +which they appear in your query:: + + $ beet list -a genre+ year+ + +This command will sort all albums by genre and, in each genre, in chronological +order. From 86c34f8740224cf053e536aeb38d5387066d86dc Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Thu, 12 Jun 2014 10:07:56 +0200 Subject: [PATCH 05/22] Moves default sort specification from code to conf Default sort order can be set in configuration file with the `sort_album` and `sort_item` keys. --- beets/config_default.yaml | 3 +++ beets/dbcore/query.py | 4 ++++ beets/dbcore/queryparse.py | 2 ++ beets/library.py | 6 +----- beets/ui/commands.py | 10 ++++++++-- 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/beets/config_default.yaml b/beets/config_default.yaml index d35368ea5..e43345933 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -54,6 +54,9 @@ list_format_item: $artist - $album - $title list_format_album: $albumartist - $album time_format: '%Y-%m-%d %H:%M:%S' +sort_album: smartartist+ +sort_item: smartartist+ + paths: default: $albumartist/$album%aunique{}/$track $title singleton: Non-Album/$artist/$title diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 97b8833bb..9952fcf4b 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -628,6 +628,7 @@ class FixedFieldSort(Sort): order = "ASC" if self.is_ascending else "DESC" return "{0} {1}".format(self.field, order) + class SmartArtistSort(Sort): """ Sort Album or Item on artist sort fields, defaulting back on artist field if the sort specific field is empty. @@ -659,6 +660,9 @@ class SmartArtistSort(Sort): return order_str +special_sorts = {'smartartist': SmartArtistSort} + + def build_sql(model_cls, query, sort_order): """ Generate a sql statement (and the values that must be injected into it) from a query, sort and a model class. diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py index 05ee28add..1a94c1831 100644 --- a/beets/dbcore/queryparse.py +++ b/beets/dbcore/queryparse.py @@ -134,6 +134,8 @@ def construct_sort_part(model_cls, part): elif field in model_cls._getters(): # Computed field, all following fields must use the slow path. pass + elif field in query.special_sorts: + sort = query.special_sorts[field](model_cls, is_ascending) else: # Neither fixed nor computed : must be a flex attr. sort = query.FlexFieldSort(model_cls, field, is_ascending) diff --git a/beets/library.py b/beets/library.py index 3965b6fb9..16aeed772 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1027,10 +1027,8 @@ class Library(dbcore.Database): def albums(self, query=None, sort_order=None): """Get a sorted list of :class:`Album` objects matching the given sort order. If a order specification is present in the query - string the sort_order argument is ignored. + string the sort_order argument is ignored. """ - if sort_order is None: - sort_order = SmartArtistSort(Album) return self._fetch(Album, query, sort_order) def items(self, query=None, sort_order=None): @@ -1038,8 +1036,6 @@ class Library(dbcore.Database): given sort order. If a order specification is present in the query string the sort_order argument is ignored. """ - if sort_order is None: - sort_order = SmartArtistSort(Item) return self._fetch(Item, query, sort_order) # Convenience accessors. diff --git a/beets/ui/commands.py b/beets/ui/commands.py index c32c1f313..dbd6806f4 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -38,6 +38,7 @@ from beets.util.functemplate import Template from beets import library from beets import config from beets.util.confit import _package_path +from beets.dbcore import sort_from_strings VARIOUS_ARTISTS = u'Various Artists' @@ -898,11 +899,16 @@ def list_items(lib, query, album, fmt): albums instead of single items. """ tmpl = Template(ui._pick_format(album, fmt)) + if album: - for album in lib.albums(query): + sort_order = sort_from_strings(library.Album, + [str(config['sort_album'])]) + for album in lib.albums(query, sort_order): ui.print_obj(album, lib, tmpl) else: - for item in lib.items(query): + sort_order = sort_from_strings(library.Item, + [str(config['sort_item'])]) + for item in lib.items(query, sort_order): ui.print_obj(item, lib, tmpl) From 486289b11ae577defb9a70482ef269b99afa3064 Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Thu, 12 Jun 2014 10:23:23 +0200 Subject: [PATCH 06/22] Add documentation about default sort configuration. --- docs/reference/config.rst | 18 ++++++++++++++++++ docs/reference/query.rst | 5 +++++ 2 files changed, 23 insertions(+) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index edf0f5027..cf10ef48d 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -163,6 +163,24 @@ Format to use when listing *albums* with :ref:`list-cmd` and other commands. Defaults to ``$albumartist - $album``. The ``-f`` command-line option overrides this setting. +.. _sort_item: + +sort_item +~~~~~~~~~ + +Sort order to use when listing *individual items* with the :ref:`list-cmd` +command and other commands that need to print out items. Defaults to +``smartartist+``. Any command-line sort order overrides this setting. + +.. _sort_album: + +sort_album +~~~~~~~~~~ + +Sort order to use when listing *albums* with the :ref:`list-cmd` +command. Defaults to ``smartartist+``. Any command-line sort order overrides +this setting. + .. _original_date: original_date diff --git a/docs/reference/query.rst b/docs/reference/query.rst index a224c8dc9..1702aa215 100644 --- a/docs/reference/query.rst +++ b/docs/reference/query.rst @@ -198,6 +198,11 @@ if you want ascending or descending sort. For example this command:: will list all albums in chronological order. +There is a special ``smartartist`` sort that uses sort-specific field ( +``artist_sort`` for items and ``albumartist_sort`` for albums) but falls back to +standard artist fields if these are empty. When no sort order is specified, +``smartartist+`` is used (but this is configurable). + You can also specify several sort orders, which will be used in the same order at which they appear in your query:: From 1d61088cfc9e2e5233cc537ebd6c0aefda43e172 Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Fri, 13 Jun 2014 13:52:14 +0200 Subject: [PATCH 07/22] Sort on computed field implementation. Sort is implemented in python. When combining sorts on fixed, flex and computed field, make as much as possible in sql and finishes the job in python. --- beets/dbcore/db.py | 72 +++++++++++------ beets/dbcore/query.py | 155 +++++++++++++++++++++++++------------ beets/dbcore/queryparse.py | 2 +- 3 files changed, 154 insertions(+), 75 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index e12178784..c50d2d572 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -483,43 +483,64 @@ class Results(object): """An item query result set. Iterating over the collection lazily constructs LibModel objects that reflect database rows. """ - def __init__(self, model_class, rows, db, query=None): + def __init__(self, model_class, rows, db, query=None, sort=None): """Create a result set that will construct objects of type `model_class`, which should be a subclass of `LibModel`, out of the query result mapping in `rows`. The new objects are - associated with the database `db`. If `query` is provided, it is - used as a predicate to filter the results for a "slow query" that - cannot be evaluated by the database directly. + associated with the database `db`. + If `query` is provided, it is used as a predicate to filter the results + for a "slow query" that cannot be evaluated by the database directly. + If `sort` is provided, it is used to sort the full list of results + before returning. This means it is a "slow sort" and all objects must + be built before returning the first one. """ self.model_class = model_class self.rows = rows self.db = db self.query = query + self.sort = sort def __iter__(self): """Construct Python objects for all rows that pass the query predicate. """ - for row in self.rows: - # Get the flexible attributes for the object. - with self.db.transaction() as tx: - flex_rows = tx.query( - 'SELECT * FROM {0} WHERE entity_id=?'.format( - self.model_class._flex_table - ), - (row['id'],) - ) + if self.sort: + # slow sort, must build the full list first + objects = [] + for row in self.rows: + obj = self._generate_results(row) + # check the predicate if any + if not self.query or self.query.match(obj): + objects.append(obj) + # Now that we have the full list, we can sort it + objects = self.sort.sort(objects) + for o in objects: + yield o + else: + for row in self.rows: + obj = self._generate_results(row) + # check the predicate if any + if not self.query or self.query.match(obj): + yield obj - cols = dict(row) - values = dict((k, v) for (k, v) in cols.items() - if not k[:4] == 'flex') - flex_values = dict((row['key'], row['value']) for row in flex_rows) + def _generate_results(self, row): + # Get the flexible attributes for the object. + with self.db.transaction() as tx: + flex_rows = tx.query( + 'SELECT * FROM {0} WHERE entity_id=?'.format( + self.model_class._flex_table + ), + (row['id'],) + ) - # Construct the Python object and yield it if it passes the - # predicate. - obj = self.model_class._awaken(self.db, values, flex_values) - if not self.query or self.query.match(obj): - yield obj + cols = dict(row) + values = dict((k, v) for (k, v) in cols.items() + if not k[:4] == 'flex') + flex_values = dict((row['key'], row['value']) for row in flex_rows) + + # Construct the Python object + obj = self.model_class._awaken(self.db, values, flex_values) + return obj def __len__(self): """Get the number of matching objects. @@ -750,12 +771,15 @@ class Database(object): Sort object. """ - sql, subvals, is_slow = build_sql(model_cls, query, sort_order) + sql, subvals, slow_query, slow_sort = build_sql(model_cls, query, + sort_order) with self.transaction() as tx: rows = tx.query(sql, subvals) - return Results(model_cls, rows, self, None if not is_slow else query) + return Results(model_cls, rows, self, + None if not slow_query else query, + None if not slow_sort else sort_order) def _get(self, model_cls, id): """Get a Model object by its id or None if the id does not diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 9952fcf4b..14cb2af82 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -15,6 +15,7 @@ """The Query type hierarchy for DBCore. """ import re +from operator import attrgetter from beets import util from datetime import datetime, timedelta @@ -500,33 +501,43 @@ class DateQuery(FieldQuery): class Sort(object): - """An abstract class representing a sort opertation for a query into the + """An abstract class representing a sort operation for a query into the item database. """ def select_clause(self): - """ Generates a select sql fragment. + """ Generates a select sql fragment if the sort operation requires one, + an empty string otherwise. """ - return None + return "" def union_clause(self): - """ Generates a union sql fragment or None if the Sort is a slow sort. + """ Generates a union sql fragment if the sort operation requires one, + an empty string otherwise. """ - return None + return "" def order_clause(self): - """Generates a sql fragment to be use in a ORDER BY clause - or None if it's a slow query + """Generates a sql fragment to be use in a ORDER BY clause or None if + it's a slow query. """ return None def sort(self, items): - """Sort the given items list. Meant to be used with slow queries. + """Return a key function that can be used with the list.sort() method. + Meant to be used with slow sort, it must be implemented even for sort + that can be done with sql, as they might be used in conjunction with + slow sort. """ - return items + return sorted(items, key=lambda x: x) + + def is_slow(self): + return False class MultipleSort(Sort): """ Sort class that combines several sort criteria. + This implementation tries to implement as many sort operation in sql, + falling back to python sort only when necessary. """ def __init__(self): @@ -535,59 +546,75 @@ class MultipleSort(Sort): def add_criteria(self, sort): self.sorts.append(sort) - def select_clause(self): - """ Generate a select sql fragment. + def _sql_sorts(self): + """ Returns the list of sort for which sql can be used """ - select_strings = [] - index = 0 - for sort in self.sorts: - select = sort.select_clause() - if select is None: - # FIXME : sort for slow sort - break + # with several Sort, we can use SQL sorting only if there is only + # SQL-capable Sort or if the list ends with SQl-capable Sort. + sql_sorts = [] + for sort in reversed(self.sorts): + if not sort.order_clause() is None: + sql_sorts.append(sort) else: + break + sql_sorts.reverse() + return sql_sorts + + def select_clause(self): + sql_sorts = self._sql_sorts() + select_strings = [] + for sort in sql_sorts: + select = sort.select_clause() + if select: select_strings.append(select) - index = index + 1 select_string = ",".join(select_strings) return "" if not select_string else ", " + select_string def union_clause(self): - """ Returns a union sql fragment. - """ + sql_sorts = self._sql_sorts() union_strings = [] - for sort in self.sorts: + for sort in sql_sorts: union = sort.union_clause() - if union is None: - pass - else: - union_strings.append(union) + union_strings.append(union) return "".join(union_strings) def order_clause(self): - """Returns a sql fragment to be use in a ORDER BY clause - or None if it's a slow query - """ + sql_sorts = self._sql_sorts() order_strings = [] - index = 0 - for sort in self.sorts: + for sort in sql_sorts: order = sort.order_clause() - if order is None: - break - else: - order_strings.append(order) - index = index + 1 + order_strings.append(order) return ",".join(order_strings) + def is_slow(self): + for sort in self.sorts: + if sort.is_slow(): + return True + return False + def sort(self, items): - # FIXME : sort according to criteria following the first slow sort + slow_sorts = [] + switch_slow = False + for sort in reversed(self.sorts): + if switch_slow: + slow_sorts.append(sort) + elif sort.order_clause() is None: + switch_slow = True + slow_sorts.append(sort) + else: + pass + + for sort in slow_sorts: + items = sort.sort(items) return items class FlexFieldSort(Sort): - + """Sort object to sort on a flexible attribute field + """ def __init__(self, model_cls, field, is_ascending): self.model_cls = model_cls self.field = field @@ -615,19 +642,26 @@ class FlexFieldSort(Sort): order = "ASC" if self.is_ascending else "DESC" return "flex_{0} {1} ".format(self.field, order) + def sort(self, items): + return sorted(items, key=attrgetter(self.field), + reverse=(not self.is_ascending)) + class FixedFieldSort(Sort): - + """Sort object to sort on a fixed field + """ def __init__(self, field, is_ascending=True): self.field = field self.is_ascending = is_ascending - """ Sort on a fixed field - """ def order_clause(self): order = "ASC" if self.is_ascending else "DESC" return "{0} {1}".format(self.field, order) + def sort(self, items): + return sorted(items, key=attrgetter(self.field), + reverse=(not self.is_ascending)) + class SmartArtistSort(Sort): """ Sort Album or Item on artist sort fields, defaulting back on @@ -660,27 +694,48 @@ class SmartArtistSort(Sort): return order_str +class ComputedFieldSort(Sort): + + def __init__(self, model_cls, field, is_ascending=True): + self.is_ascending = is_ascending + self.field = field + self._getters = model_cls._getters() + + def is_slow(self): + return True + + def sort(self, items): + return sorted(items, key=lambda x: self._getters[self.field](x), + reverse=(not self.is_ascending)) + special_sorts = {'smartartist': SmartArtistSort} -def build_sql(model_cls, query, sort_order): +def build_sql(model_cls, query, sort): """ Generate a sql statement (and the values that must be injected into it) from a query, sort and a model class. """ where, subvals = query.clause() + slow_query = where is None - if not sort_order: + if not sort: sort_select = "" sort_union = "" sort_order = "" - elif isinstance(sort_order, basestring): + slow_sort = False + elif isinstance(sort, basestring): sort_select = "" sort_union = "" - sort_order = " ORDER BY {0}".format(sort_order) - elif isinstance(sort_order, Sort): - sort_select = sort_order.select_clause() - sort_union = sort_order.union_clause() - sort_order = " ORDER BY {0}".format(sort_order.order_clause()) + sort_order = " ORDER BY {0}".format(sort) \ + if sort else "" + slow_sort = False + elif isinstance(sort, Sort): + sort_select = sort.select_clause() + sort_union = sort.union_clause() + slow_sort = sort.is_slow() + order_clause = sort.order_clause() + sort_order = " ORDER BY {0}".format(order_clause) \ + if order_clause else "" sql = ("SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE " "{query_clause} {sort_order}").format( @@ -691,4 +746,4 @@ def build_sql(model_cls, query, sort_order): sort_order=sort_order ) - return (sql, subvals, where is None) \ No newline at end of file + return (sql, subvals, slow_query, slow_sort) \ No newline at end of file diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py index 1a94c1831..03536f8ca 100644 --- a/beets/dbcore/queryparse.py +++ b/beets/dbcore/queryparse.py @@ -133,7 +133,7 @@ def construct_sort_part(model_cls, part): sort = query.FixedFieldSort(field, is_ascending) elif field in model_cls._getters(): # Computed field, all following fields must use the slow path. - pass + sort = query.ComputedFieldSort(model_cls, field, is_ascending) elif field in query.special_sorts: sort = query.special_sorts[field](model_cls, is_ascending) else: From ebd2da14dc85ad71dbd48ce4685b9121bac5479e Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Fri, 13 Jun 2014 13:55:51 +0200 Subject: [PATCH 08/22] Fix error on default sort configuration Fix crash when using several criteria for default sort in the configuration file. --- beets/ui/commands.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index dbd6806f4..29d80a74b 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -901,13 +901,15 @@ def list_items(lib, query, album, fmt): tmpl = Template(ui._pick_format(album, fmt)) if album: + sort_parts = str(config['sort_album']).split() sort_order = sort_from_strings(library.Album, - [str(config['sort_album'])]) + sort_parts) for album in lib.albums(query, sort_order): ui.print_obj(album, lib, tmpl) else: + sort_parts = str(config['sort_item']).split() sort_order = sort_from_strings(library.Item, - [str(config['sort_item'])]) + sort_parts) for item in lib.items(query, sort_order): ui.print_obj(item, lib, tmpl) From a90991b29667fe93eb28036e4ccb08816bf1024a Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Fri, 13 Jun 2014 13:57:16 +0200 Subject: [PATCH 09/22] Removes unused import (flake8 warning) --- beets/library.py | 1 - 1 file changed, 1 deletion(-) diff --git a/beets/library.py b/beets/library.py index 16aeed772..9e566626c 100644 --- a/beets/library.py +++ b/beets/library.py @@ -29,7 +29,6 @@ from beets.util import bytestring_path, syspath, normpath, samefile from beets.util.functemplate import Template from beets import dbcore from beets.dbcore import types -from beets.dbcore.query import SmartArtistSort import beets From 1a995ed8d45d872efbfa324f0bd68eb0d64ee6b4 Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Sat, 14 Jun 2014 17:11:57 +0200 Subject: [PATCH 10/22] Fix sorting without using the MultipleSort object. SQL syntax error when using a simple FlexFieldSort. --- beets/dbcore/query.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 14cb2af82..e240d3252 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -569,7 +569,7 @@ class MultipleSort(Sort): select_strings.append(select) select_string = ",".join(select_strings) - return "" if not select_string else ", " + select_string + return select_string def union_clause(self): sql_sorts = self._sql_sorts() @@ -730,7 +730,9 @@ def build_sql(model_cls, query, sort): if sort else "" slow_sort = False elif isinstance(sort, Sort): - sort_select = sort.select_clause() + select_clause = sort.select_clause() + sort_select = " ,{0} ".format(select_clause) \ + if select_clause else "" sort_union = sort.union_clause() slow_sort = sort.is_slow() order_clause = sort.order_clause() From ad1c0b38319209f9a965c8c668c2734c75915b2c Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Sat, 14 Jun 2014 21:24:54 +0200 Subject: [PATCH 11/22] Add unit tests for fixef and flex field sort --- test/test_sort.py | 155 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 test/test_sort.py diff --git a/test/test_sort.py b/test/test_sort.py new file mode 100644 index 000000000..43f43e221 --- /dev/null +++ b/test/test_sort.py @@ -0,0 +1,155 @@ +# This file is part of beets. +# Copyright 2013, Adrian Sampson. +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. + +"""Various tests for querying the library database. +""" +import _common +from _common import unittest +import beets.library +from beets import dbcore + + + +# A test case class providing a library with some dummy data and some +# assertions involving that data. +class DummyDataTestCase(_common.TestCase): + def setUp(self): + super(DummyDataTestCase, self).setUp() + self.lib = beets.library.Library(':memory:') + items = [_common.item() for _ in range(3)] + items[0].title = 'foo bar' + items[0].artist = 'one' + items[0].album = 'baz' + items[0].year = 2001 + items[0].comp = True + items[0].flex1 = "flex1-0" + items[0].flex2 = "flex2-A" + items[1].title = 'baz qux' + items[1].artist = 'two' + items[1].album = 'baz' + items[1].year = 2002 + items[1].comp = True + items[1].flex1 = "flex1-1" + items[1].flex2 = "flex2-A" + items[2].title = 'beets 4 eva' + items[2].artist = 'three' + items[2].album = 'foo' + items[2].year = 2003 + items[2].comp = False + items[2].flex1 = "flex1-2" + items[2].flex2 = "flex1-B" + for item in items: + self.lib.add(item) + self.lib.add_album(items[:2]) + + +class SortFixedFieldTest(DummyDataTestCase): + def test_sort_asc(self): + q = '' + sort = dbcore.query.FixedFieldSort("year", True) + results = self.lib.items(q, sort) + self.assertTrue(results[0]['year']results[1]['year']) + self.assertEqual(results[0]['year'],2003) + # same thing with query string + q = 'year-' + results2 = self.lib.items(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + def test_sort_two_field_asc(self): + q = '' + s1 = dbcore.query.FixedFieldSort("album", True) + s2 = dbcore.query.FixedFieldSort("year", True) + sort = dbcore.query.MultipleSort() + sort.add_criteria(s1) + sort.add_criteria(s2) + results = self.lib.items(q, sort) + self.assertTrue(results[0]['album']<=results[1]['album']) + self.assertTrue(results[1]['album']<=results[2]['album']) + self.assertEqual(results[0]['album'],'baz') + self.assertEqual(results[1]['album'],'baz') + self.assertTrue(results[0]['year']<=results[1]['year']) + #same thing with query string + q = 'album+ year+' + results2 = self.lib.items(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + +class SortFlexFieldTest(DummyDataTestCase): + def test_sort_asc(self): + q = '' + sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", True) + results = self.lib.items(q, sort) + self.assertTrue(results[0]['flex1']results[1]['flex1']) + self.assertEqual(results[0]['flex1'],'flex1-2') + # same thing with query string + q = 'flex1-' + results2 = self.lib.items(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + def test_sort_two_field(self): + q = '' + s1 = dbcore.query.FlexFieldSort(beets.library.Item, "flex2", False) + s2 = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", True) + sort = dbcore.query.MultipleSort() + sort.add_criteria(s1) + sort.add_criteria(s2) + results = self.lib.items(q, sort) + self.assertTrue(results[0]['flex2']>=results[1]['flex2']) + self.assertTrue(results[1]['flex2']>=results[2]['flex2']) + self.assertEqual(results[0]['flex2'],'flex2-A') + self.assertEqual(results[1]['flex2'],'flex2-A') + self.assertTrue(results[0]['flex1']<=results[1]['flex1']) + #same thing with query string + q = 'flex2- flex1+' + results2 = self.lib.items(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + + + + +def suite(): + return unittest.TestLoader().loadTestsFromName(__name__) + + +if __name__ == '__main__': + unittest.main(defaultTest='suite') From 728797718ecdd5001048da85b9a618aba94bcf5c Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Sun, 15 Jun 2014 09:08:28 +0200 Subject: [PATCH 12/22] Add tests cases for sort on albums. On fixed and flex attributes. --- test/_common.py | 26 +++++++- test/test_sort.py | 149 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 164 insertions(+), 11 deletions(-) diff --git a/test/_common.py b/test/_common.py index d209bf4ef..0187cc25b 100644 --- a/test/_common.py +++ b/test/_common.py @@ -82,9 +82,33 @@ def item(lib=None): album_id = None, ) if lib: - lib.add(i) + lib.add_album(i) return i +_album_ident = 0 +def album(lib=None): + global _item_ident + _item_ident += 1 + i = beets.library.Album( + artpath= None, + albumartist = 'some album artist', + albumartist_sort = 'some sort album artist', + albumartist_credit = 'some album artist credit', + album = 'the album', + genre = 'the genre', + year = 2014, + month = 2, + day = 5, + tracktotal = 0, + disctotal = 1, + comp = False, + mb_albumid = 'someID-1', + mb_albumartistid = 'someID-1' + ) + if lib: + lib.add(i) + return i + # Dummy import session. def import_session(lib=None, logfile=None, paths=[], query=[], cli=False): cls = commands.TerminalImportSession if cli else importer.ImportSession diff --git a/test/test_sort.py b/test/test_sort.py index 43f43e221..bb6047ddc 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -27,6 +27,26 @@ class DummyDataTestCase(_common.TestCase): def setUp(self): super(DummyDataTestCase, self).setUp() self.lib = beets.library.Library(':memory:') + + albums = [_common.album() for _ in range(3)] + albums[0].album = "album A" + albums[0].genre = "Rock" + albums[0].year = "2001" + albums[0].flex1 = "flex1-1" + albums[0].flex2 = "flex2-A" + albums[1].album = "album B" + albums[1].genre = "Rock" + albums[1].year = "2001" + albums[1].flex1 = "flex1-2" + albums[1].flex2 = "flex2-A" + albums[2].album = "album C" + albums[2].genre = "Jazz" + albums[2].year = "2005" + albums[2].flex1 = "flex1-1" + albums[2].flex2 = "flex2-B" + for album in albums: + self.lib.add(album) + items = [_common.item() for _ in range(3)] items[0].title = 'foo bar' items[0].artist = 'one' @@ -35,6 +55,7 @@ class DummyDataTestCase(_common.TestCase): items[0].comp = True items[0].flex1 = "flex1-0" items[0].flex2 = "flex2-A" + items[0].album_id = albums[0].id items[1].title = 'baz qux' items[1].artist = 'two' items[1].album = 'baz' @@ -42,6 +63,7 @@ class DummyDataTestCase(_common.TestCase): items[1].comp = True items[1].flex1 = "flex1-1" items[1].flex2 = "flex2-A" + items[1].album_id = albums[0].id items[2].title = 'beets 4 eva' items[2].artist = 'three' items[2].album = 'foo' @@ -49,9 +71,29 @@ class DummyDataTestCase(_common.TestCase): items[2].comp = False items[2].flex1 = "flex1-2" items[2].flex2 = "flex1-B" + items[2].album_id = albums[1].id for item in items: self.lib.add(item) - self.lib.add_album(items[:2]) + + albums = [_common.album() for _ in range(3)] + albums[0].album = "album A" + albums[0].genre = "Jazz" + albums[0].year = "2001" + albums[0].flex1 = "flex1-1" + albums[0].flex2 = "flex2-A" + albums[1].album = "album B" + albums[1].genre = "Jazz" + albums[1].year = "2001" + albums[1].flex1 = "flex1-2" + albums[1].flex2 = "flex2-A" + albums[2].album = "album C" + albums[2].genre = "Rock" + albums[2].year = "2005" + albums[2].flex1 = "flex1-1" + albums[2].flex2 = "flex2-B" + + for album in albums: + self.lib.add(album) class SortFixedFieldTest(DummyDataTestCase): @@ -59,7 +101,7 @@ class SortFixedFieldTest(DummyDataTestCase): q = '' sort = dbcore.query.FixedFieldSort("year", True) results = self.lib.items(q, sort) - self.assertTrue(results[0]['year']results[1]['year']) + self.assertGreaterEqual(results[0]['year'], results[1]['year']) self.assertEqual(results[0]['year'],2003) # same thing with query string q = 'year-' @@ -87,11 +129,11 @@ class SortFixedFieldTest(DummyDataTestCase): sort.add_criteria(s1) sort.add_criteria(s2) results = self.lib.items(q, sort) - self.assertTrue(results[0]['album']<=results[1]['album']) - self.assertTrue(results[1]['album']<=results[2]['album']) + self.assertLessEqual(results[0]['album'], results[1]['album']) + self.assertLessEqual(results[1]['album'], results[2]['album']) self.assertEqual(results[0]['album'],'baz') self.assertEqual(results[1]['album'],'baz') - self.assertTrue(results[0]['year']<=results[1]['year']) + self.assertLessEqual(results[0]['year'], results[1]['year']) #same thing with query string q = 'album+ year+' results2 = self.lib.items(q) @@ -104,7 +146,7 @@ class SortFlexFieldTest(DummyDataTestCase): q = '' sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", True) results = self.lib.items(q, sort) - self.assertTrue(results[0]['flex1']=results[1]['flex2']) - self.assertTrue(results[1]['flex2']>=results[2]['flex2']) + self.assertGreaterEqual(results[0]['flex2'], results[1]['flex2']) + self.assertGreaterEqual(results[1]['flex2'], results[2]['flex2']) self.assertEqual(results[0]['flex2'],'flex2-A') self.assertEqual(results[1]['flex2'],'flex2-A') - self.assertTrue(results[0]['flex1']<=results[1]['flex1']) + self.assertLessEqual(results[0]['flex1'], results[1]['flex1']) #same thing with query string q = 'flex2- flex1+' results2 = self.lib.items(q) @@ -144,7 +186,94 @@ class SortFlexFieldTest(DummyDataTestCase): self.assertEqual(r1.id, r2.id) +class SortAlbumFixedFieldTest(DummyDataTestCase): + def test_sort_asc(self): + q = '' + sort = dbcore.query.FixedFieldSort("year", True) + results = self.lib.albums(q, sort) + self.assertLessEqual(results[0]['year'], results[1]['year']) + self.assertEqual(results[0]['year'],2001) + # same thing with query string + q = 'year+' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + def test_sort_desc(self): + q = '' + sort = dbcore.query.FixedFieldSort("year", False) + results = self.lib.albums(q, sort) + self.assertGreaterEqual(results[0]['year'], results[1]['year']) + self.assertEqual(results[0]['year'],2005) + # same thing with query string + q = 'year-' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + def test_sort_two_field_asc(self): + q = '' + s1 = dbcore.query.FixedFieldSort("genre", True) + s2 = dbcore.query.FixedFieldSort("album", True) + sort = dbcore.query.MultipleSort() + sort.add_criteria(s1) + sort.add_criteria(s2) + results = self.lib.albums(q, sort) + self.assertLessEqual(results[0]['genre'], results[1]['genre']) + self.assertLessEqual(results[1]['genre'], results[2]['genre']) + self.assertEqual(results[0]['genre'],'Jazz') + self.assertEqual(results[1]['genre'],'Jazz') + self.assertLessEqual(results[0]['album'], results[1]['album']) + #same thing with query string + q = 'genre+ album+' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + +class SortAlbumFlexdFieldTest(DummyDataTestCase): + def test_sort_asc(self): + q = '' + sort = dbcore.query.FlexFieldSort(beets.library.Album, "flex1", True) + results = self.lib.albums(q, sort) + self.assertLessEqual(results[0]['flex1'], results[1]['flex1']) + self.assertLessEqual(results[1]['flex1'], results[2]['flex1']) + # same thing with query string + q = 'flex1+' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + def test_sort_desc(self): + q = '' + sort = dbcore.query.FlexFieldSort(beets.library.Album, "flex1", False) + results = self.lib.albums(q, sort) + self.assertGreaterEqual(results[0]['flex1'], results[1]['flex1']) + self.assertGreaterEqual(results[1]['flex1'], results[2]['flex1']) + # same thing with query string + q = 'flex1-' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + def test_sort_two_field_asc(self): + q = '' + s1 = dbcore.query.FlexFieldSort(beets.library.Album, "flex2", True) + s2 = dbcore.query.FlexFieldSort(beets.library.Album, "flex1", True) + sort = dbcore.query.MultipleSort() + sort.add_criteria(s1) + sort.add_criteria(s2) + results = self.lib.albums(q, sort) + self.assertLessEqual(results[0]['flex2'], results[1]['flex2']) + self.assertLessEqual(results[1]['flex2'], results[2]['flex2']) + self.assertEqual(results[0]['flex2'],'flex2-A') + self.assertEqual(results[1]['flex2'],'flex2-A') + self.assertLessEqual(results[0]['flex1'], results[1]['flex1']) + #same thing with query string + q = 'flex2+ flex1+' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) def suite(): From 936acf02362c0c82fe59a068b2685603ef37bc45 Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Sun, 15 Jun 2014 09:16:37 +0200 Subject: [PATCH 13/22] Fix typo in common utility function for tests --- test/_common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/_common.py b/test/_common.py index 0187cc25b..7891f6e80 100644 --- a/test/_common.py +++ b/test/_common.py @@ -82,7 +82,7 @@ def item(lib=None): album_id = None, ) if lib: - lib.add_album(i) + lib.add(i) return i _album_ident = 0 @@ -106,7 +106,7 @@ def album(lib=None): mb_albumartistid = 'someID-1' ) if lib: - lib.add(i) + lib.add_album(i) return i # Dummy import session. From 69213ad574147113648ea4520fc214229726e339 Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Sun, 15 Jun 2014 22:06:05 +0200 Subject: [PATCH 14/22] Add tests for computed attributes. --- test/_common.py | 2 +- test/test_sort.py | 93 ++++++++++++++++++++++++++++++++++------------- 2 files changed, 68 insertions(+), 27 deletions(-) diff --git a/test/_common.py b/test/_common.py index 7891f6e80..94b6255b8 100644 --- a/test/_common.py +++ b/test/_common.py @@ -106,7 +106,7 @@ def album(lib=None): mb_albumartistid = 'someID-1' ) if lib: - lib.add_album(i) + lib.add(i) return i # Dummy import session. diff --git a/test/test_sort.py b/test/test_sort.py index bb6047ddc..472136ce0 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -47,7 +47,7 @@ class DummyDataTestCase(_common.TestCase): for album in albums: self.lib.add(album) - items = [_common.item() for _ in range(3)] + items = [_common.item() for _ in range(4)] items[0].title = 'foo bar' items[0].artist = 'one' items[0].album = 'baz' @@ -72,29 +72,17 @@ class DummyDataTestCase(_common.TestCase): items[2].flex1 = "flex1-2" items[2].flex2 = "flex1-B" items[2].album_id = albums[1].id + items[3].title = 'beets 4 eva' + items[3].artist = 'three' + items[3].album = 'foo2' + items[3].year = 2004 + items[3].comp = False + items[3].flex1 = "flex1-2" + items[3].flex2 = "flex1-C" + items[3].album_id = albums[2].id for item in items: self.lib.add(item) - albums = [_common.album() for _ in range(3)] - albums[0].album = "album A" - albums[0].genre = "Jazz" - albums[0].year = "2001" - albums[0].flex1 = "flex1-1" - albums[0].flex2 = "flex2-A" - albums[1].album = "album B" - albums[1].genre = "Jazz" - albums[1].year = "2001" - albums[1].flex1 = "flex1-2" - albums[1].flex2 = "flex2-A" - albums[2].album = "album C" - albums[2].genre = "Rock" - albums[2].year = "2005" - albums[2].flex1 = "flex1-1" - albums[2].flex2 = "flex2-B" - - for album in albums: - self.lib.add(album) - class SortFixedFieldTest(DummyDataTestCase): def test_sort_asc(self): @@ -114,7 +102,7 @@ class SortFixedFieldTest(DummyDataTestCase): sort = dbcore.query.FixedFieldSort("year", False) results = self.lib.items(q, sort) self.assertGreaterEqual(results[0]['year'], results[1]['year']) - self.assertEqual(results[0]['year'],2003) + self.assertEqual(results[0]['year'],2004) # same thing with query string q = 'year-' results2 = self.lib.items(q) @@ -158,7 +146,9 @@ class SortFlexFieldTest(DummyDataTestCase): q = '' sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", False) results = self.lib.items(q, sort) - self.assertTrue(results[0]['flex1']>results[1]['flex1']) + self.assertGreaterEqual(results[0]['flex1'], results[1]['flex1']) + self.assertGreaterEqual(results[1]['flex1'], results[2]['flex1']) + self.assertGreaterEqual(results[2]['flex1'], results[3]['flex1']) self.assertEqual(results[0]['flex1'],'flex1-2') # same thing with query string q = 'flex1-' @@ -221,9 +211,9 @@ class SortAlbumFixedFieldTest(DummyDataTestCase): results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['genre'], results[1]['genre']) self.assertLessEqual(results[1]['genre'], results[2]['genre']) - self.assertEqual(results[0]['genre'],'Jazz') - self.assertEqual(results[1]['genre'],'Jazz') - self.assertLessEqual(results[0]['album'], results[1]['album']) + self.assertEqual(results[1]['genre'],'Rock') + self.assertEqual(results[2]['genre'],'Rock') + self.assertLessEqual(results[1]['album'], results[2]['album']) #same thing with query string q = 'genre+ album+' results2 = self.lib.albums(q) @@ -276,6 +266,57 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase): self.assertEqual(r1.id, r2.id) +class SortAlbumComputedFieldTest(DummyDataTestCase): + def test_sort_asc(self): + q = '' + sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True) + results = self.lib.albums(q, sort) + self.assertLessEqual(results[0]['path'], results[1]['path']) + self.assertLessEqual(results[1]['path'], results[2]['path']) + # same thing with query string + q = 'path+' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + def test_sort_asc(self): + q = '' + sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", False) + results = self.lib.albums(q, sort) + self.assertGreaterEqual(results[0]['path'], results[1]['path']) + self.assertGreaterEqual(results[1]['path'], results[2]['path']) + # same thing with query string + q = 'path-' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2) : + self.assertEqual(r1.id, r2.id) + + +class SortCombinedFieldTest(DummyDataTestCase): + def test_computed_first(self): + q='' + s1 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True) + s2 = dbcore.query.FixedFieldSort("year", True) + sort = dbcore.query.MultipleSort() + sort.add_criteria(s1) + sort.add_criteria(s2) + results = self.lib.albums(q, sort) + self.assertLessEqual(results[0]['path'], results[1]['path']) + self.assertLessEqual(results[1]['path'], results[2]['path']) + + def test_computed_second(self): + q='' + s1 = dbcore.query.FixedFieldSort("year", True) + s2 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True) + sort = dbcore.query.MultipleSort() + sort.add_criteria(s1) + sort.add_criteria(s2) + results = self.lib.albums(q, sort) + self.assertLessEqual(results[0]['year'], results[1]['year']) + self.assertLessEqual(results[1]['year'], results[2]['year']) + self.assertLessEqual(results[0]['path'], results[1]['path']) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 18539b0bd190f6cbb2197f9a257928d18048c24d Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Sun, 15 Jun 2014 22:28:26 +0200 Subject: [PATCH 15/22] Fix flake8 warnings. --- beets/dbcore/query.py | 2 +- test/test_sort.py | 87 ++++++++++++++++++++++++------------------- 2 files changed, 49 insertions(+), 40 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index e240d3252..458bb7f91 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -730,7 +730,7 @@ def build_sql(model_cls, query, sort): if sort else "" slow_sort = False elif isinstance(sort, Sort): - select_clause = sort.select_clause() + select_clause = sort.select_clause() sort_select = " ,{0} ".format(select_clause) \ if select_clause else "" sort_union = sort.union_clause() diff --git a/test/test_sort.py b/test/test_sort.py index 472136ce0..76e5a35cb 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -20,14 +20,13 @@ import beets.library from beets import dbcore - # A test case class providing a library with some dummy data and some # assertions involving that data. class DummyDataTestCase(_common.TestCase): def setUp(self): super(DummyDataTestCase, self).setUp() self.lib = beets.library.Library(':memory:') - + albums = [_common.album() for _ in range(3)] albums[0].album = "album A" albums[0].genre = "Rock" @@ -90,11 +89,11 @@ class SortFixedFieldTest(DummyDataTestCase): sort = dbcore.query.FixedFieldSort("year", True) results = self.lib.items(q, sort) self.assertLessEqual(results[0]['year'], results[1]['year']) - self.assertEqual(results[0]['year'],2001) + self.assertEqual(results[0]['year'], 2001) # same thing with query string q = 'year+' results2 = self.lib.items(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) def test_sort_desc(self): @@ -102,11 +101,11 @@ class SortFixedFieldTest(DummyDataTestCase): sort = dbcore.query.FixedFieldSort("year", False) results = self.lib.items(q, sort) self.assertGreaterEqual(results[0]['year'], results[1]['year']) - self.assertEqual(results[0]['year'],2004) + self.assertEqual(results[0]['year'], 2004) # same thing with query string q = 'year-' results2 = self.lib.items(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) def test_sort_two_field_asc(self): @@ -119,13 +118,13 @@ class SortFixedFieldTest(DummyDataTestCase): results = self.lib.items(q, sort) self.assertLessEqual(results[0]['album'], results[1]['album']) self.assertLessEqual(results[1]['album'], results[2]['album']) - self.assertEqual(results[0]['album'],'baz') - self.assertEqual(results[1]['album'],'baz') + self.assertEqual(results[0]['album'], 'baz') + self.assertEqual(results[1]['album'], 'baz') self.assertLessEqual(results[0]['year'], results[1]['year']) - #same thing with query string + # same thing with query string q = 'album+ year+' results2 = self.lib.items(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) @@ -135,11 +134,11 @@ class SortFlexFieldTest(DummyDataTestCase): sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", True) results = self.lib.items(q, sort) self.assertLessEqual(results[0]['flex1'], results[1]['flex1']) - self.assertEqual(results[0]['flex1'],'flex1-0') + self.assertEqual(results[0]['flex1'], 'flex1-0') # same thing with query string q = 'flex1+' results2 = self.lib.items(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) def test_sort_desc(self): @@ -149,11 +148,11 @@ class SortFlexFieldTest(DummyDataTestCase): self.assertGreaterEqual(results[0]['flex1'], results[1]['flex1']) self.assertGreaterEqual(results[1]['flex1'], results[2]['flex1']) self.assertGreaterEqual(results[2]['flex1'], results[3]['flex1']) - self.assertEqual(results[0]['flex1'],'flex1-2') + self.assertEqual(results[0]['flex1'], 'flex1-2') # same thing with query string q = 'flex1-' results2 = self.lib.items(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) def test_sort_two_field(self): @@ -166,13 +165,13 @@ class SortFlexFieldTest(DummyDataTestCase): results = self.lib.items(q, sort) self.assertGreaterEqual(results[0]['flex2'], results[1]['flex2']) self.assertGreaterEqual(results[1]['flex2'], results[2]['flex2']) - self.assertEqual(results[0]['flex2'],'flex2-A') - self.assertEqual(results[1]['flex2'],'flex2-A') + self.assertEqual(results[0]['flex2'], 'flex2-A') + self.assertEqual(results[1]['flex2'], 'flex2-A') self.assertLessEqual(results[0]['flex1'], results[1]['flex1']) - #same thing with query string + # same thing with query string q = 'flex2- flex1+' results2 = self.lib.items(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) @@ -182,11 +181,11 @@ class SortAlbumFixedFieldTest(DummyDataTestCase): sort = dbcore.query.FixedFieldSort("year", True) results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['year'], results[1]['year']) - self.assertEqual(results[0]['year'],2001) + self.assertEqual(results[0]['year'], 2001) # same thing with query string q = 'year+' results2 = self.lib.albums(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) def test_sort_desc(self): @@ -194,11 +193,11 @@ class SortAlbumFixedFieldTest(DummyDataTestCase): sort = dbcore.query.FixedFieldSort("year", False) results = self.lib.albums(q, sort) self.assertGreaterEqual(results[0]['year'], results[1]['year']) - self.assertEqual(results[0]['year'],2005) + self.assertEqual(results[0]['year'], 2005) # same thing with query string q = 'year-' results2 = self.lib.albums(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) def test_sort_two_field_asc(self): @@ -211,13 +210,13 @@ class SortAlbumFixedFieldTest(DummyDataTestCase): results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['genre'], results[1]['genre']) self.assertLessEqual(results[1]['genre'], results[2]['genre']) - self.assertEqual(results[1]['genre'],'Rock') - self.assertEqual(results[2]['genre'],'Rock') + self.assertEqual(results[1]['genre'], 'Rock') + self.assertEqual(results[2]['genre'], 'Rock') self.assertLessEqual(results[1]['album'], results[2]['album']) - #same thing with query string + # same thing with query string q = 'genre+ album+' results2 = self.lib.albums(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) @@ -231,7 +230,7 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase): # same thing with query string q = 'flex1+' results2 = self.lib.albums(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) def test_sort_desc(self): @@ -243,7 +242,7 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase): # same thing with query string q = 'flex1-' results2 = self.lib.albums(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) def test_sort_two_field_asc(self): @@ -256,45 +255,47 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase): results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['flex2'], results[1]['flex2']) self.assertLessEqual(results[1]['flex2'], results[2]['flex2']) - self.assertEqual(results[0]['flex2'],'flex2-A') - self.assertEqual(results[1]['flex2'],'flex2-A') + self.assertEqual(results[0]['flex2'], 'flex2-A') + self.assertEqual(results[1]['flex2'], 'flex2-A') self.assertLessEqual(results[0]['flex1'], results[1]['flex1']) - #same thing with query string + # same thing with query string q = 'flex2+ flex1+' results2 = self.lib.albums(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) class SortAlbumComputedFieldTest(DummyDataTestCase): def test_sort_asc(self): q = '' - sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True) + sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", + True) results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['path'], results[1]['path']) self.assertLessEqual(results[1]['path'], results[2]['path']) # same thing with query string q = 'path+' results2 = self.lib.albums(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) - def test_sort_asc(self): + def test_sort_desc(self): q = '' - sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", False) + sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", + False) results = self.lib.albums(q, sort) self.assertGreaterEqual(results[0]['path'], results[1]['path']) self.assertGreaterEqual(results[1]['path'], results[2]['path']) # same thing with query string q = 'path-' results2 = self.lib.albums(q) - for r1, r2 in zip(results, results2) : + for r1, r2 in zip(results, results2): self.assertEqual(r1.id, r2.id) class SortCombinedFieldTest(DummyDataTestCase): def test_computed_first(self): - q='' + q = '' s1 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True) s2 = dbcore.query.FixedFieldSort("year", True) sort = dbcore.query.MultipleSort() @@ -303,9 +304,13 @@ class SortCombinedFieldTest(DummyDataTestCase): results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['path'], results[1]['path']) self.assertLessEqual(results[1]['path'], results[2]['path']) + q = 'path+ year+' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2): + self.assertEqual(r1.id, r2.id) def test_computed_second(self): - q='' + q = '' s1 = dbcore.query.FixedFieldSort("year", True) s2 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True) sort = dbcore.query.MultipleSort() @@ -315,6 +320,10 @@ class SortCombinedFieldTest(DummyDataTestCase): self.assertLessEqual(results[0]['year'], results[1]['year']) self.assertLessEqual(results[1]['year'], results[2]['year']) self.assertLessEqual(results[0]['path'], results[1]['path']) + q = 'year+ path+' + results2 = self.lib.albums(q) + for r1, r2 in zip(results, results2): + self.assertEqual(r1.id, r2.id) def suite(): From cd1dcf69690a95cc4eecb2e4a7aba609691291ff Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Wed, 2 Jul 2014 13:05:33 +0200 Subject: [PATCH 16/22] Fix minor formatting and comments issues. --- beets/dbcore/db.py | 2 +- beets/dbcore/query.py | 2 +- beets/dbcore/queryparse.py | 2 +- beets/library.py | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index c50d2d572..5dff1c0f1 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -505,7 +505,7 @@ class Results(object): predicate. """ if self.sort: - # slow sort, must build the full list first + # Slow sort. Must build the full list first. objects = [] for row in self.rows: obj = self._generate_results(row) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 458bb7f91..b66496e2d 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -535,7 +535,7 @@ class Sort(object): class MultipleSort(Sort): - """ Sort class that combines several sort criteria. + """Sort class that combines several sort criteria. This implementation tries to implement as many sort operation in sql, falling back to python sort only when necessary. """ diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py index 03536f8ca..b51194b33 100644 --- a/beets/dbcore/queryparse.py +++ b/beets/dbcore/queryparse.py @@ -150,4 +150,4 @@ def sort_from_strings(model_cls, sort_parts): sort = query.MultipleSort() for part in sort_parts: sort.add_criteria(construct_sort_part(model_cls, part)) - return sort \ No newline at end of file + return sort diff --git a/beets/library.py b/beets/library.py index 9e566626c..7270dc8c0 100644 --- a/beets/library.py +++ b/beets/library.py @@ -942,10 +942,10 @@ def get_query(val, model_cls): # Add path queries to aggregate query. if path_parts: query.subqueries += [PathQuery('path', s) for s in path_parts] - return (query, sort) + return query, sort elif isinstance(val, dbcore.Query): - return (val, None) + return val, None else: raise ValueError('query must be None or have type Query or str') From 451f17c99ada663b42b0982a05eee32b3f0db4dc Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Wed, 2 Jul 2014 19:06:27 +0200 Subject: [PATCH 17/22] Fix formatting issue --- beets/dbcore/query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index b66496e2d..14ce0a2d4 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -748,4 +748,4 @@ def build_sql(model_cls, query, sort): sort_order=sort_order ) - return (sql, subvals, slow_query, slow_sort) \ No newline at end of file + return sql, subvals, slow_query, slow_sort \ No newline at end of file From d168c6222b246a30393be9cfd35fe8b79684739d Mon Sep 17 00:00:00 2001 From: Pierre Rust Date: Wed, 2 Jul 2014 19:26:27 +0200 Subject: [PATCH 18/22] Fix formatting (again ...) --- beets/dbcore/query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 14ce0a2d4..6b1b2541e 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -748,4 +748,4 @@ def build_sql(model_cls, query, sort): sort_order=sort_order ) - return sql, subvals, slow_query, slow_sort \ No newline at end of file + return sql, subvals, slow_query, slow_sort From 3130a6add9a607f39511371e0a06944070af6ff1 Mon Sep 17 00:00:00 2001 From: PierreRust Date: Thu, 24 Jul 2014 12:51:21 +0200 Subject: [PATCH 19/22] Minor changes from review (mostly style) --- beets/dbcore/db.py | 13 +++++-------- beets/dbcore/query.py | 15 +++++++++------ beets/library.py | 8 ++++---- beetsplug/ihate.py | 6 +++--- beetsplug/smartplaylist.py | 2 +- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 5dff1c0f1..5172ad523 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -508,7 +508,7 @@ class Results(object): # Slow sort. Must build the full list first. objects = [] for row in self.rows: - obj = self._generate_results(row) + obj = self._make_model(row) # check the predicate if any if not self.query or self.query.match(obj): objects.append(obj) @@ -518,12 +518,12 @@ class Results(object): yield o else: for row in self.rows: - obj = self._generate_results(row) + obj = self._make_model(row) # check the predicate if any if not self.query or self.query.match(obj): yield obj - def _generate_results(self, row): + def _make_model(self, row): # Get the flexible attributes for the object. with self.db.transaction() as tx: flex_rows = tx.query( @@ -771,15 +771,12 @@ class Database(object): Sort object. """ - sql, subvals, slow_query, slow_sort = build_sql(model_cls, query, - sort_order) + sql, subvals, query, sort = build_sql(model_cls, query, sort_order) with self.transaction() as tx: rows = tx.query(sql, subvals) - return Results(model_cls, rows, self, - None if not slow_query else query, - None if not slow_sort else sort_order) + return Results(model_cls, rows, self, query, sort) def _get(self, model_cls, id): """Get a Model object by its id or None if the id does not diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 6b1b2541e..6a10f2533 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -713,31 +713,34 @@ special_sorts = {'smartartist': SmartArtistSort} def build_sql(model_cls, query, sort): """ Generate a sql statement (and the values that must be injected into it) - from a query, sort and a model class. + from a query, sort and a model class. Query and sort objects are returned + only for slow query and slow sort operation. """ where, subvals = query.clause() - slow_query = where is None + if where is not None: + query = None if not sort: sort_select = "" sort_union = "" sort_order = "" - slow_sort = False + sort = None elif isinstance(sort, basestring): sort_select = "" sort_union = "" sort_order = " ORDER BY {0}".format(sort) \ if sort else "" - slow_sort = False + sort = None elif isinstance(sort, Sort): select_clause = sort.select_clause() sort_select = " ,{0} ".format(select_clause) \ if select_clause else "" sort_union = sort.union_clause() - slow_sort = sort.is_slow() order_clause = sort.order_clause() sort_order = " ORDER BY {0}".format(order_clause) \ if order_clause else "" + if sort.is_slow(): + sort = None sql = ("SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE " "{query_clause} {sort_order}").format( @@ -748,4 +751,4 @@ def build_sql(model_cls, query, sort): sort_order=sort_order ) - return sql, subvals, slow_query, slow_sort + return sql, subvals, query, sort diff --git a/beets/library.py b/beets/library.py index 7270dc8c0..c0c9ebe1a 100644 --- a/beets/library.py +++ b/beets/library.py @@ -546,7 +546,7 @@ class Item(LibModel): for query, path_format in path_formats: if query == PF_KEY_DEFAULT: continue - (query, _) = get_query(query, type(self)) + (query, _) = get_query_sort(query, type(self)) if query.match(self): # The query matches the item! Use the corresponding path # format. @@ -887,7 +887,7 @@ class Album(LibModel): # Query construction helper. -def get_query(val, model_cls): +def get_query_sort(val, model_cls): """Take a value which may be None, a query string, a query string list, or a Query object, and return a suitable Query object and Sort object. @@ -1016,8 +1016,8 @@ class Library(dbcore.Database): """Parse a query and fetch. If a order specification is present in the query string the sort_order argument is ignored. """ - (query, sort) = get_query(query, model_cls) - sort = sort_order if sort is None else sort + query, sort = get_query_sort(query, model_cls) + sort = sort or sort_order return super(Library, self)._fetch( model_cls, query, sort diff --git a/beetsplug/ihate.py b/beetsplug/ihate.py index 85e3382d4..3e86759c6 100644 --- a/beetsplug/ihate.py +++ b/beetsplug/ihate.py @@ -17,7 +17,7 @@ import logging from beets.plugins import BeetsPlugin from beets.importer import action -from beets.library import get_query +from beets.library import get_query_sort from beets.library import Item from beets.library import Album @@ -57,9 +57,9 @@ class IHatePlugin(BeetsPlugin): for query_string in action_patterns: query = None if task.is_album: - (query, _) = get_query(query_string, Album) + (query, _) = get_query_sort(query_string, Album) else: - (query, _) = get_query(query_string, Item) + (query, _) = get_query_sort(query_string, Item) if any(query.match(item) for item in task.imported_items()): return True return False diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 12672ce42..6beb0ad59 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -42,7 +42,7 @@ def _items_for_query(lib, playlist, album=False): query_strings = [query_strings] model = library.Album if album else library.Item query = dbcore.OrQuery( - [library.get_query(q, model)[0] for q in query_strings] + [library.get_query_sort(q, model)[0] for q in query_strings] ) # Execute query, depending on type. From 91295d8d7bd4eb2d14cf8dfe657e653992c60506 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 22 Aug 2014 12:47:52 -0700 Subject: [PATCH 20/22] Refine changelog for 1.3.7 release --- docs/changelog.rst | 47 +++++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 114fbba67..cc8d279d7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,35 +1,46 @@ Changelog ========= -1.3.7 (in development) ----------------------- +1.3.7 (August 22, 2014) +----------------------- -New stuff +This release of beets fixes all the bugs, and you can be confident that you +will never again find any bugs in beets, ever. +It also adds support for plain old AIFF files and adds three more plugins, +including a nifty one that lets you measure a song's tempo by tapping out the +beat on your keyboard. +The importer deals more elegantly with duplicates and you can broaden your +cover art search to the entire web with Google Image Search. +The big new features are: + +* Support for AIFF files. Tags are stored as ID3 frames in one of the file's + IFF chunks. Thanks to Evan Purkhiser for contributing support to `Mutagen`_. * The new :doc:`/plugins/importadded` reads files' modification times to set their "added" date. Thanks to Stig Inge Lea Bjørnsen. -* Support for AIFF files. Tags are stored as ID3 frames in one of the file's - IFF chunks. -* A new :ref:`required` configuration option for the importer skips matches - that are missing certain data. Thanks to oprietop. * The new :doc:`/plugins/bpm` lets you manually measure the tempo of a playing song. Thanks to aroquen. * The new :doc:`/plugins/spotify` generates playlists for your `Spotify`_ account. Thanks to Olin Gay. +* A new :ref:`required` configuration option for the importer skips matches + that are missing certain data. Thanks to oprietop. * When the importer detects duplicates, it now shows you some details about the potentially-replaced music so you can make an informed decision. Thanks to Howard Jones. +* :doc:`/plugins/fetchart`: You can now optionally search for cover art on + Google Image Search. Thanks to Lemutar. +* A new :ref:`asciify-paths` configuration option replaces all non-ASCII + characters in paths. +.. _Mutagen: https://bitbucket.org/lazka/mutagen .. _Spotify: https://www.spotify.com/ -Little improvements and fixes: +And the multitude of little improvements and fixes: +* Compatibility with the latest version of `Mutagen`_, 1.23. * :doc:`/plugins/web`: Lyrics now display readably with correct line breaks. Also, the detail view scrolls to reveal all of the lyrics. Thanks to Meet Udeshi. -* Compatibility with the latest version of Mutagen, 1.23. -* :doc:`/plugins/fetchart`: You can now optionally search for cover art on - Google Image Search. Thanks to Lemutar. * :doc:`/plugins/play`: The ``command`` config option can now contain arguments (rather than just an executable). Thanks to Alessandro Ghedini. * Fix an error when using the :ref:`modify-cmd` command to remove a flexible @@ -51,11 +62,11 @@ Little improvements and fixes: * Don't display changes for fields that are not in the restricted field set. This fixes :ref:`write-cmd` showing changes for fields that are not written to the file. -* :ref:`write-cmd` command: Don't display the item name if there are no - changes for it. -* When using both :doc:`/plugins/convert` and :doc:`/plugins/scrub`, avoid - scrubbing the source file of conversions. (Fix a regression introduced in - the previous release.) +* The :ref:`write-cmd` command avoids displaying the item name if there are + no changes for it. +* When using both the :doc:`/plugins/convert` and the :doc:`/plugins/scrub`, + avoid scrubbing the source file of conversions. (Fix a regression introduced + in the previous release.) * :doc:`/plugins/replaygain`: Logging is now quieter during import. Thanks to Yevgeny Bezman. * :doc:`/plugins/fetchart`: When loading art from the filesystem, we now @@ -71,7 +82,7 @@ Little improvements and fixes: * :doc:`/plugins/bucket`: You can now customize the definition of alphanumeric "ranges" using regular expressions. And the heuristic for detecting years has been improved. Thanks to sotho. -* Already imported singleton tracks are skipped when resuming an +* Already-imported singleton tracks are skipped when resuming an import. * :doc:`/plugins/chroma`: A new ``auto`` configuration option disables fingerprinting on import. Thanks to ddettrittus. @@ -79,8 +90,6 @@ Little improvements and fixes: transcoding preset from the command-line. * :doc:`/plugins/convert`: Transcoding presets can now omit their filename extensions (extensions default to the name of the preset). -* A new :ref:`asciify-paths` configuration option replaces all non-ASCII - characters in paths. * :doc:`/plugins/convert`: A new ``--pretend`` option lets you preview the commands the plugin will execute without actually taking any action. Thanks to Dietrich Daroch. From 787f0e25c529043201d8397250c542a5c0d6f479 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 22 Aug 2014 12:52:03 -0700 Subject: [PATCH 21/22] Version bump: 1.3.8 --- beets/__init__.py | 2 +- docs/changelog.rst | 6 ++++++ docs/conf.py | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/beets/__init__.py b/beets/__init__.py index d4f4c5db8..a2a0bfbd5 100644 --- a/beets/__init__.py +++ b/beets/__init__.py @@ -12,7 +12,7 @@ # The above copyright notice and this permission notice shall be # included in all copies or substantial portions of the Software. -__version__ = '1.3.7' +__version__ = '1.3.8' __author__ = 'Adrian Sampson ' import beets.library diff --git a/docs/changelog.rst b/docs/changelog.rst index cc8d279d7..072dc57a6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,12 @@ Changelog ========= +1.3.8 (in development) +---------------------- + +Changelog goes here! + + 1.3.7 (August 22, 2014) ----------------------- diff --git a/docs/conf.py b/docs/conf.py index 0040583c1..291685116 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -12,7 +12,7 @@ project = u'beets' copyright = u'2012, Adrian Sampson' version = '1.3' -release = '1.3.7' +release = '1.3.8' pygments_style = 'sphinx' From 132fad847b0dabcbd1de911b8d2212e404392aef Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 22 Aug 2014 15:02:50 -0700 Subject: [PATCH 22/22] Changelog for sorting (#823) --- docs/changelog.rst | 2 +- docs/reference/query.rst | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 072dc57a6..53ed48e0d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,7 +4,7 @@ Changelog 1.3.8 (in development) ---------------------- -Changelog goes here! +This release adds **sorting** to beets queries. See :ref:`query-sort`. 1.3.7 (August 22, 2014) diff --git a/docs/reference/query.rst b/docs/reference/query.rst index 7fdf14640..b85a03962 100644 --- a/docs/reference/query.rst +++ b/docs/reference/query.rst @@ -185,6 +185,8 @@ query won't necessarily find *all* the audio files in a directory---just the ones you've already added to your beets library. +.. _query-sort: + Sort Order ---------- @@ -207,6 +209,6 @@ You can also specify several sort orders, which will be used in the same order a which they appear in your query:: $ beet list -a genre+ year+ - + This command will sort all albums by genre and, in each genre, in chronological order.