From 9ee794beb794dda0424276830f00342cdc885b5c Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Wed, 10 Sep 2014 17:22:23 +0200 Subject: [PATCH 01/51] Model.load() should remove flexible fields --- beets/dbcore/db.py | 2 ++ test/test_dbcore.py | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 050ddbd81..7a912c3a7 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -382,6 +382,8 @@ class Model(object): self._check_db() stored_obj = self._db._get(type(self), self.id) assert stored_obj is not None, "object {0} not in DB".format(self.id) + self._values_fixed = {} + self._values_flex = {} self.update(dict(stored_obj)) self.clear_dirty() diff --git a/test/test_dbcore.py b/test/test_dbcore.py index bca88cbda..9d0dd1d3b 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -249,6 +249,20 @@ class ModelTest(unittest.TestCase): model.some_float_field = None self.assertEqual(model.some_float_field, 0.0) + def test_load_deleted_flex_field(self): + model1 = TestModel1() + model1['flex_field'] = True + model1.add(self.db) + + model2 = self.db._get(TestModel1, model1.id) + self.assertIn('flex_field', model2) + + del model1['flex_field'] + model1.store() + + model2.load() + self.assertNotIn('flex_field', model2) + class FormatTest(unittest.TestCase): def test_format_fixed_field(self): From 3de66ccd6543f6626094709f9789160576c0ea64 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 10 Sep 2014 11:53:46 -0700 Subject: [PATCH 02/51] Remove BitBucket references --- docs/faq.rst | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/faq.rst b/docs/faq.rst index 27bc29d9a..ed88963a9 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -139,15 +139,14 @@ it's helpful to run on the "bleeding edge". To run the latest source: - Use ``pip`` to install the latest snapshot tarball: just type ``pip install https://github.com/sampsyo/beets/tarball/master``. - - Grab the source using Mercurial - (``hg clone https://bitbucket.org/adrian/beets``) or git - (``git clone https://github.com/sampsyo/beets.git``). Then + - Grab the source using Git: + ``git clone https://github.com/sampsyo/beets.git``. Then ``cd beets`` and type ``python setup.py install``. - Use ``pip`` to install an "editable" version of beets based on an automatic source checkout. For example, run - ``pip install -e hg+https://bitbucket.org/adrian/beets#egg=beets`` - to clone beets from BitBucket using Mercurial and install it, - allowing you to modify the source in-place to try out changes. + ``pip install -e git+https://github.com/sampsyo/beets#egg=beets`` + to clone beets and install it, allowing you to modify the source + in-place to try out changes. More details about the beets source are available on the :doc:`developer documentation ` pages. From 6dd6d4770e6006a49984350b3b1e835af2bd81e8 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 10 Sep 2014 19:08:39 -0700 Subject: [PATCH 03/51] Skip zero-track matches (fix #942) --- beets/autotag/match.py | 5 +++++ docs/changelog.rst | 1 + 2 files changed, 6 insertions(+) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index fb031802c..aa0c21dba 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -332,6 +332,11 @@ def _add_candidate(items, results, info): """ log.debug('Candidate: %s - %s' % (info.artist, info.album)) + # Discard albums with zero tracks. + if not info.tracks: + log.debug('No tracks.') + return + # Don't duplicate. if info.album_id in results: log.debug('Duplicate.') diff --git a/docs/changelog.rst b/docs/changelog.rst index c67a38472..db19d7206 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -41,6 +41,7 @@ Fixes: to Bombardment. * :doc:`/plugins/play`: Add a ``relative_to`` config option. Thanks to BrainDamage. +* Fix a crash when a MusicBrainz release has zero tracks. .. _discogs_client: https://github.com/discogs/discogs_client From 832f34c46cbc6d061f84a4d22bd74469c6df0140 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 10 Sep 2014 19:35:34 -0700 Subject: [PATCH 04/51] --version (fix #939) --- beets/ui/__init__.py | 11 +++++++++-- docs/changelog.rst | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 2e61611d9..82205e0e0 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -689,6 +689,8 @@ class SubcommandsOptionParser(optparse.OptionParser): help='path to configuration file') self.add_option('-h', '--help', dest='help', action='store_true', help='how this help message and exit') + self.add_option('--version', dest='version', action='store_true', + help=optparse.SUPPRESS_HELP) # Our root parser needs to stop on the first unrecognized argument. self.disable_interspersed_args() @@ -948,9 +950,14 @@ def _raw_main(args, lib=None): subcommands, plugins, lib = _setup(options, lib) parser.add_subcommand(*subcommands) - subcommand, suboptions, subargs = parser.parse_subcommand(subargs) - subcommand.func(lib, suboptions, subargs) + if options.version: + from beets.ui import commands + commands.version_cmd.func(lib, None, None) + else: + subcommand, suboptions, subargs = parser.parse_subcommand(subargs) + subcommand.func(lib, suboptions, subargs) + plugins.send('cli_exit', lib=lib) diff --git a/docs/changelog.rst b/docs/changelog.rst index db19d7206..04ecfea24 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -42,6 +42,7 @@ Fixes: * :doc:`/plugins/play`: Add a ``relative_to`` config option. Thanks to BrainDamage. * Fix a crash when a MusicBrainz release has zero tracks. +* The ``--version`` flag now works as an alias for the ``version`` command. .. _discogs_client: https://github.com/discogs/discogs_client From 7958469b25386e1e7bd202d87f0c3955c4aa9b7c Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 11 Sep 2014 11:48:34 +0200 Subject: [PATCH 05/51] mediafile: Zero-padded date format Tag files with '2000-08-01' instead of '2000-8-1' --- beets/mediafile.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/beets/mediafile.py b/beets/mediafile.py index 4e83d0543..6c08dac3f 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1122,13 +1122,15 @@ class DateField(MediaField): """ if year is None: self.__delete__(mediafile) - date = [year] + return + + date = [u'{0:04d}'.format(int(year))] if month: - date.append(month) + date.append(u'{0:02d}'.format(int(month))) if month and day: - date.append(day) + date.append(u'{0:02d}'.format(int(day))) date = map(unicode, date) - super(DateField, self).__set__(mediafile, '-'.join(date)) + super(DateField, self).__set__(mediafile, u'-'.join(date)) if hasattr(self, '_year_field'): self._year_field.__set__(mediafile, year) From 5279016db44638df266d600ab01471e5d4301b08 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 11 Sep 2014 16:09:26 +0200 Subject: [PATCH 06/51] config: Add suport for 'key in config' syntax We can use this instead of the cumbersome `config[key].exists()`. --- beets/util/confit.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/beets/util/confit.py b/beets/util/confit.py index 4ffe68cea..16a484b78 100644 --- a/beets/util/confit.py +++ b/beets/util/confit.py @@ -220,6 +220,9 @@ class ConfigView(object): """ self.set({key: value}) + def __contains__(self, key): + return self[key].exists() + def set_args(self, namespace): """Overlay parsed command-line arguments, generated by a library like argparse or optparse, onto this view's value. From 35efa7283f87747d728b9de2588491a7dbd30783 Mon Sep 17 00:00:00 2001 From: gwern Date: Thu, 11 Sep 2014 14:37:01 -0400 Subject: [PATCH 07/51] genres.txt: rm fake genres see issue #943; deleted all suggestions except 'early music' which some people may want/find meaningful --- beetsplug/lastgenre/genres.txt | 7 ------- 1 file changed, 7 deletions(-) diff --git a/beetsplug/lastgenre/genres.txt b/beetsplug/lastgenre/genres.txt index e45cad09a..ad344afec 100644 --- a/beetsplug/lastgenre/genres.txt +++ b/beetsplug/lastgenre/genres.txt @@ -26,7 +26,6 @@ ambient house ambient music americana anarcho punk -anime music anti-folk apala ape haters @@ -51,7 +50,6 @@ avant-garde music axé bac-bal bachata -background music baggy baila baile funk @@ -449,7 +447,6 @@ emocore emotronic enka eremwu eu -essential rock ethereal pop ethereal wave euro @@ -1047,7 +1044,6 @@ new york blues new york house newgrass nganja -niche nightcore nintendocore nisiótika @@ -1310,7 +1306,6 @@ sica siguiriyas silat sinawi -singer-songwriter situational ska ska punk @@ -1338,7 +1333,6 @@ soul soul blues soul jazz soul music -soundtrack southern gospel southern harmony southern hip hop @@ -1472,7 +1466,6 @@ vaudeville venezuela verbunkos verismo -video game music viking metal villanella virelai From 971bff95fa5623d362174fd450f4893ac88022fe Mon Sep 17 00:00:00 2001 From: gwern Date: Thu, 11 Sep 2014 14:38:33 -0400 Subject: [PATCH 08/51] lastbeats: explain that the whitelist has been modified current description sounds like the whitelist is regularly synced and should not be/has not been edited --- beetsplug/lastgenre/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index eba6c6cab..3a9654c7d 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -15,7 +15,8 @@ """Gets genres for imported music based on Last.fm tags. Uses a provided whitelist file to determine which tags are valid genres. -The included (default) genre list was produced by scraping Wikipedia. +The included (default) genre list was originally produced by scraping Wikipedia +and has been edited to remove some questionable entries. The scraper script used is available here: https://gist.github.com/1241307 """ From 524f109339bf5726b70a94266d0ade5e84bb1b5b Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Thu, 11 Sep 2014 12:20:45 -0700 Subject: [PATCH 09/51] Changelog for #944 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 04ecfea24..65b28ff16 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -43,6 +43,8 @@ Fixes: BrainDamage. * Fix a crash when a MusicBrainz release has zero tracks. * The ``--version`` flag now works as an alias for the ``version`` command. +* :doc:`/plugins/lastgenre`: Remove some unhelpful genres from the default + whitelist. Thanks to gwern. .. _discogs_client: https://github.com/discogs/discogs_client From fc5f2126bb51e512f663b00be83dfcef79621024 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 11 Sep 2014 22:46:06 +0200 Subject: [PATCH 10/51] Add test helper for adding items --- test/helper.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/helper.py b/test/helper.py index 3bc95b319..cf2a67ad0 100644 --- a/test/helper.py +++ b/test/helper.py @@ -230,6 +230,28 @@ class TestHelper(object): return TestImportSession(self.lib, logfile=None, query=None, paths=[import_dir]) + # Library fixtures methods + + def add_item(self, **values_): + """Add an item to the library and return it. + + The item receives sensible default values for the title, artist, + and album fields. These default values contain unicode + characters to test for encoding issues. The track title also + includes a counter to make sure we do not create items with the + same attributes. + """ + values = { + 'title': u't\u00eftle {0}'.format(self._get_item_count()), + 'artist': u'the \u00e4rtist', + 'album': u'the \u00e4lbum', + } + values.update(values_) + item = Item(**values) + if hasattr(self, 'lib'): + item.add(self.lib) + return item + def add_item_fixtures(self, ext='mp3', count=1): """Add a number of items with files to the database. """ @@ -283,6 +305,14 @@ class TestHelper(object): for path in self._mediafile_fixtures: os.remove(path) + def _get_item_count(self): + if not hasattr(self, '__item_count'): + count = 0 + self.__item_count = count + 1 + return count + + # Running beets commands + def run_command(self, *args): if hasattr(self, 'lib'): lib = self.lib @@ -295,6 +325,8 @@ class TestHelper(object): self.run_command(*args) return out.getvalue() + # Safe file operations + def create_temp_dir(self): """Create a temporary directory and assign it into `self.temp_dir`. Call `remove_temp_dir` later to delete it. From 469b61689ae355dd6f13b82c914a35957f1acbd2 Mon Sep 17 00:00:00 2001 From: "Fabrice L." Date: Fri, 12 Sep 2014 07:50:50 +0200 Subject: [PATCH 11/51] Link to the internal whitelist raw file content --- docs/plugins/lastgenre.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index 58f3432be..171f8e69f 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -21,8 +21,9 @@ your ``plugins`` line in :doc:`config file `. The plugin chooses genres based on a *whitelist*, meaning that only certain tags can be considered genres. This way, tags like "my favorite music" or "seen live" won't be considered genres. The plugin ships with a fairly extensive -internal whitelist, but you can set your own in the config file using the -``whitelist`` configuration value:: +[internal whitelist](https://raw.githubusercontent.com/sampsyo/beets/master/beetsplug/lastgenre/genres.txt), +but you can set your own in the config file using the ``whitelist`` configuration +value:: lastgenre: whitelist: /path/to/genres.txt From f11bbe6580443ec9cb2c836303e518cd7ae53edb Mon Sep 17 00:00:00 2001 From: "Fabrice L." Date: Fri, 12 Sep 2014 08:16:01 +0200 Subject: [PATCH 12/51] fix link --- docs/plugins/lastgenre.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index 171f8e69f..c2cd59160 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -21,9 +21,8 @@ your ``plugins`` line in :doc:`config file `. The plugin chooses genres based on a *whitelist*, meaning that only certain tags can be considered genres. This way, tags like "my favorite music" or "seen live" won't be considered genres. The plugin ships with a fairly extensive -[internal whitelist](https://raw.githubusercontent.com/sampsyo/beets/master/beetsplug/lastgenre/genres.txt), -but you can set your own in the config file using the ``whitelist`` configuration -value:: +`internal whitelist`_, but you can set your own in the config file using the +``whitelist`` configuration value:: lastgenre: whitelist: /path/to/genres.txt @@ -37,6 +36,7 @@ Wikipedia`_. .. _pip: http://www.pip-installer.org/ .. _pylast: http://code.google.com/p/pylast/ .. _script that scrapes Wikipedia: https://gist.github.com/1241307 +.. _internal whitelist: https://raw.githubusercontent.com/sampsyo/beets/master/beetsplug/lastgenre/genres.txt By default, beets will always fetch new genres, even if the files already have once. To instead leave genres in place in when they pass the whitelist, set From 7328a09644da06740fa76447468674d48a094820 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Fri, 12 Sep 2014 17:23:49 +0200 Subject: [PATCH 13/51] Align dispatch of 'version' command with other commands --- beets/ui/__init__.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 82205e0e0..d224524e9 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -776,6 +776,8 @@ class SubcommandsOptionParser(optparse.OptionParser): # Force the help command if options.help: subargs = ['help'] + elif options.version: + subargs = ['version'] return options, subargs def parse_subcommand(self, args): @@ -951,12 +953,8 @@ def _raw_main(args, lib=None): parser.add_subcommand(*subcommands) - if options.version: - from beets.ui import commands - commands.version_cmd.func(lib, None, None) - else: - subcommand, suboptions, subargs = parser.parse_subcommand(subargs) - subcommand.func(lib, suboptions, subargs) + subcommand, suboptions, subargs = parser.parse_subcommand(subargs) + subcommand.func(lib, suboptions, subargs) plugins.send('cli_exit', lib=lib) From ba93d6b176370d1b6a013a6b599d10718fec999b Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 19:21:52 -0700 Subject: [PATCH 14/51] Move flags back to _raw_main SubcommandsOptionParser is supposed to be generic. --- beets/ui/__init__.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index d224524e9..fb958c489 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -679,18 +679,6 @@ class SubcommandsOptionParser(optparse.OptionParser): # Super constructor. optparse.OptionParser.__init__(self, *args, **kwargs) - self.add_option('-l', '--library', dest='library', - help='library database file to use') - self.add_option('-d', '--directory', dest='directory', - help="destination music directory") - self.add_option('-v', '--verbose', dest='verbose', action='store_true', - help='print debugging information') - self.add_option('-c', '--config', dest='config', - help='path to configuration file') - self.add_option('-h', '--help', dest='help', action='store_true', - help='how this help message and exit') - self.add_option('--version', dest='version', action='store_true', - help=optparse.SUPPRESS_HELP) # Our root parser needs to stop on the first unrecognized argument. self.disable_interspersed_args() @@ -947,6 +935,19 @@ def _raw_main(args, lib=None): """ parser = SubcommandsOptionParser() + parser.add_option('-l', '--library', dest='library', + help='library database file to use') + parser.add_option('-d', '--directory', dest='directory', + help="destination music directory") + parser.add_option('-v', '--verbose', dest='verbose', action='store_true', + help='print debugging information') + parser.add_option('-c', '--config', dest='config', + help='path to configuration file') + parser.add_option('-h', '--help', dest='help', action='store_true', + help='how this help message and exit') + parser.add_option('--version', dest='version', action='store_true', + help=optparse.SUPPRESS_HELP) + options, subargs = parser.parse_global_options(args) subcommands, plugins, lib = _setup(options, lib) From d572bde13b1ae12345b3fa7bd75f478b965d4b2a Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 19:44:17 -0700 Subject: [PATCH 15/51] Tiny docs typos Conflicts: docs/plugins/index.rst --- beets/ui/__init__.py | 4 ---- docs/plugins/index.rst | 3 ++- docs/reference/cli.rst | 5 ++--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index fb958c489..ab836f410 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -876,7 +876,6 @@ def _setup(options, lib=None): def _configure(options): """Amend the global configuration object with command line options. """ - # Add any additional config files specified with --config. This # special handling lets specified plugins get loaded before we # finish parsing the command line. @@ -933,7 +932,6 @@ def _raw_main(args, lib=None): """A helper function for `main` without top-level exception handling. """ - parser = SubcommandsOptionParser() parser.add_option('-l', '--library', dest='library', help='library database file to use') @@ -949,9 +947,7 @@ def _raw_main(args, lib=None): help=optparse.SUPPRESS_HELP) options, subargs = parser.parse_global_options(args) - subcommands, plugins, lib = _setup(options, lib) - parser.add_subcommand(*subcommands) subcommand, suboptions, subargs = parser.parse_subcommand(subargs) diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index cdf802ba0..39c1e5704 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -136,7 +136,8 @@ Miscellaneous * :doc:`info`: Print music files' tags to the console. * :doc:`missing`: List missing tracks. * :doc:`duplicates`: List duplicate tracks or albums. -* :doc:`spotify`: Create Spotify playlists from the Beets library +* :doc:`spotify`: Create Spotify playlists from the Beets library. +* :doc:`types`: Declare types for flexible attributes. .. _MPD: http://www.musicpd.org/ .. _MPD clients: http://mpd.wikia.com/wiki/Clients diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 468ef0664..d8dff4c33 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -174,9 +174,8 @@ 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``. You -can also specify the order used when outputting the results (Read more in -:doc:`query`.) +"vegetables" in the title? Try ``beet list year:2009 title:vegetables``. You +can also specify the sort order. (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 From bb4082fbfc055766f0baef1f2533f8d75d48489a Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 20:56:33 -0700 Subject: [PATCH 16/51] Style cleanup in sorting --- beets/dbcore/query.py | 67 +++++++++++++++++--------------------- beets/dbcore/queryparse.py | 2 +- test/test_sort.py | 24 +++++++------- 3 files changed, 43 insertions(+), 50 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 6a10f2533..df6cd2ff1 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -18,6 +18,10 @@ import re from operator import attrgetter from beets import util from datetime import datetime, timedelta +from collections import namedtuple + + +SortedQuery = namedtuple('SortedQuery', 'query', 'sort') class Query(object): @@ -500,19 +504,21 @@ class DateQuery(FieldQuery): return clause, subvals +# Sorting. + class Sort(object): - """An abstract class representing a sort operation for a query into the - item database. + """An abstract class representing a sort operation for a query into + the item database. """ + def select_clause(self): - """ Generates a select sql fragment if the sort operation requires one, - an empty string otherwise. + """Generate a SELECT fragment (possibly an empty string) for the + sort. """ return "" def union_clause(self): - """ Generates a union sql fragment if the sort operation requires one, - an empty string otherwise. + """Generate a join SQL fragment (possibly an empty string). """ return "" @@ -523,47 +529,32 @@ class Sort(object): return None def sort(self, items): - """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. + """Sort the list of objects and return a list. """ return sorted(items, key=lambda x: x) def is_slow(self): + """Indicate whether this query is *slow*, meaning that it cannot + be executed in SQL and must be executed in Python. + """ 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. + """Sort that encapsulates multiple sub-sorts. """ def __init__(self): self.sorts = [] - def add_criteria(self, sort): + def add_sort(self, sort): self.sorts.append(sort) - def _sql_sorts(self): - """ Returns the list of sort for which sql can be used - """ - # 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() + if self.is_slow(): + return "" select_strings = [] - for sort in sql_sorts: + for sort in self.sorts: select = sort.select_clause() if select: select_strings.append(select) @@ -572,18 +563,20 @@ class MultipleSort(Sort): return select_string def union_clause(self): - sql_sorts = self._sql_sorts() + if self.is_slow(): + return "" union_strings = [] - for sort in sql_sorts: + for sort in self.sorts: union = sort.union_clause() union_strings.append(union) return "".join(union_strings) def order_clause(self): - sql_sorts = self._sql_sorts() + if self.is_slow(): + return None order_strings = [] - for sort in sql_sorts: + for sort in self.sorts: order = sort.order_clause() order_strings.append(order) @@ -621,12 +614,12 @@ class FlexFieldSort(Sort): self.is_ascending = is_ascending def select_clause(self): - """ Return a select sql fragment. + """Return a SELECT fragment. """ return "sort_flexattr{0!s}.value as flex_{0!s} ".format(self.field) def union_clause(self): - """ Returns an union sql fragment. + """Return a JOIN fragment. """ union = ("LEFT JOIN {flextable} as sort_flexattr{index!s} " "ON {table}.id = sort_flexattr{index!s}.entity_id " @@ -637,7 +630,7 @@ class FlexFieldSort(Sort): return union def order_clause(self): - """ Returns an order sql fragment. + """Return an ORDER BY fragment. """ order = "ASC" if self.is_ascending else "DESC" return "flex_{0} {1} ".format(self.field, order) diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py index b51194b33..324274a5a 100644 --- a/beets/dbcore/queryparse.py +++ b/beets/dbcore/queryparse.py @@ -149,5 +149,5 @@ def sort_from_strings(model_cls, sort_parts): return None sort = query.MultipleSort() for part in sort_parts: - sort.add_criteria(construct_sort_part(model_cls, part)) + sort.add_sort(construct_sort_part(model_cls, part)) return sort diff --git a/test/test_sort.py b/test/test_sort.py index 76e5a35cb..1c6cc9741 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -113,8 +113,8 @@ class SortFixedFieldTest(DummyDataTestCase): s1 = dbcore.query.FixedFieldSort("album", True) s2 = dbcore.query.FixedFieldSort("year", True) sort = dbcore.query.MultipleSort() - sort.add_criteria(s1) - sort.add_criteria(s2) + sort.add_sort(s1) + sort.add_sort(s2) results = self.lib.items(q, sort) self.assertLessEqual(results[0]['album'], results[1]['album']) self.assertLessEqual(results[1]['album'], results[2]['album']) @@ -160,8 +160,8 @@ class SortFlexFieldTest(DummyDataTestCase): 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) + sort.add_sort(s1) + sort.add_sort(s2) results = self.lib.items(q, sort) self.assertGreaterEqual(results[0]['flex2'], results[1]['flex2']) self.assertGreaterEqual(results[1]['flex2'], results[2]['flex2']) @@ -205,8 +205,8 @@ class SortAlbumFixedFieldTest(DummyDataTestCase): s1 = dbcore.query.FixedFieldSort("genre", True) s2 = dbcore.query.FixedFieldSort("album", True) sort = dbcore.query.MultipleSort() - sort.add_criteria(s1) - sort.add_criteria(s2) + sort.add_sort(s1) + sort.add_sort(s2) results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['genre'], results[1]['genre']) self.assertLessEqual(results[1]['genre'], results[2]['genre']) @@ -250,8 +250,8 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase): 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) + sort.add_sort(s1) + sort.add_sort(s2) results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['flex2'], results[1]['flex2']) self.assertLessEqual(results[1]['flex2'], results[2]['flex2']) @@ -299,8 +299,8 @@ class SortCombinedFieldTest(DummyDataTestCase): 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) + sort.add_sort(s1) + sort.add_sort(s2) results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['path'], results[1]['path']) self.assertLessEqual(results[1]['path'], results[2]['path']) @@ -314,8 +314,8 @@ class SortCombinedFieldTest(DummyDataTestCase): 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) + sort.add_sort(s1) + sort.add_sort(s2) results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['year'], results[1]['year']) self.assertLessEqual(results[1]['year'], results[2]['year']) From 03d0f9dfb3f97acd7cf817d1391cf09b41bed6a4 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 20:59:19 -0700 Subject: [PATCH 17/51] Fix namedtuple call --- 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 df6cd2ff1..503b47db1 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -21,7 +21,7 @@ from datetime import datetime, timedelta from collections import namedtuple -SortedQuery = namedtuple('SortedQuery', 'query', 'sort') +SortedQuery = namedtuple('SortedQuery', ['query', 'sort']) class Query(object): From db4e74fd57ea79864c8bc7ed6d4ba5fae22efd58 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 21:11:53 -0700 Subject: [PATCH 18/51] Fix mistaken removals --- beets/dbcore/query.py | 26 +++++++++++++++++--------- docs/plugins/index.rst | 1 - 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 503b47db1..3ef5ce30d 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -550,11 +550,23 @@ class MultipleSort(Sort): def add_sort(self, sort): self.sorts.append(sort) + def _sql_sorts(self): + """ Returns the list of sort for which sql can be used + """ + # With several sort criteria, 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): - if self.is_slow(): - return "" select_strings = [] - for sort in self.sorts: + for sort in self._sql_sorts(): select = sort.select_clause() if select: select_strings.append(select) @@ -563,20 +575,16 @@ class MultipleSort(Sort): return select_string def union_clause(self): - if self.is_slow(): - return "" union_strings = [] - for sort in self.sorts: + for sort in self._sql_sorts(): union = sort.union_clause() union_strings.append(union) return "".join(union_strings) def order_clause(self): - if self.is_slow(): - return None order_strings = [] - for sort in self.sorts: + for sort in self._sql_sorts(): order = sort.order_clause() order_strings.append(order) diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index 39c1e5704..cd1ea868e 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -137,7 +137,6 @@ Miscellaneous * :doc:`missing`: List missing tracks. * :doc:`duplicates`: List duplicate tracks or albums. * :doc:`spotify`: Create Spotify playlists from the Beets library. -* :doc:`types`: Declare types for flexible attributes. .. _MPD: http://www.musicpd.org/ .. _MPD clients: http://mpd.wikia.com/wiki/Clients From f5b64314883e07b278dc295a9cc93d7a442562d2 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 21:12:44 -0700 Subject: [PATCH 19/51] Flake8 fix --- beets/ui/__init__.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index ab836f410..37912dd46 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -936,15 +936,15 @@ def _raw_main(args, lib=None): parser.add_option('-l', '--library', dest='library', help='library database file to use') parser.add_option('-d', '--directory', dest='directory', - help="destination music directory") + help="destination music directory") parser.add_option('-v', '--verbose', dest='verbose', action='store_true', - help='print debugging information') + help='print debugging information') parser.add_option('-c', '--config', dest='config', - help='path to configuration file') + help='path to configuration file') parser.add_option('-h', '--help', dest='help', action='store_true', - help='how this help message and exit') + help='how this help message and exit') parser.add_option('--version', dest='version', action='store_true', - help=optparse.SUPPRESS_HELP) + help=optparse.SUPPRESS_HELP) options, subargs = parser.parse_global_options(args) subcommands, plugins, lib = _setup(options, lib) From cfb929223e74ff860e1ec76b49e4dca6e159b396 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 21:28:32 -0700 Subject: [PATCH 20/51] Collapse some duplication in Confit templates --- beets/ui/__init__.py | 2 +- beets/util/confit.py | 34 +++++++++++++++------------------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 37912dd46..7e3e7559c 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -830,7 +830,7 @@ def vararg_callback(option, opt_str, value, parser): def _load_plugins(config): """Load the plugins specified in the configuration. """ - paths = config['pluginpath'].get(confit.EnsureStringList()) + paths = config['pluginpath'].get(confit.StrSeq(split=False)) paths = map(util.normpath, paths) import beetsplug diff --git a/beets/util/confit.py b/beets/util/confit.py index 16a484b78..40cefc1fa 100644 --- a/beets/util/confit.py +++ b/beets/util/confit.py @@ -1055,15 +1055,27 @@ class Choice(Template): class StrSeq(Template): """A template for values that are lists of strings. - Validates both actual YAML string lists and whitespace-separated - strings. + Validates both actual YAML string lists and single strings. Strings + can optionally be split on whitespace. """ + def __init__(self, split=True): + """Create a new template. + + `split` indicates whether, when the underlying value is a single + string, it should be split on whitespace. Otherwise, the + resulting value is a list containing a single string. + """ + self.split = split + def convert(self, value, view): if isinstance(value, bytes): value = value.decode('utf8', 'ignore') if isinstance(value, STRING): - return value.split() + if self.split: + return value.split() + else: + return [value] else: try: value = list(value) @@ -1076,22 +1088,6 @@ class StrSeq(Template): self.fail('must be a list of strings', view, True) -class EnsureStringList(Template): - """Always return a list of strings. - - The raw value may either be a single string or a list of strings. - Otherwise a type error is raised. For single strings a singleton - list is returned. - """ - def convert(self, paths, view): - if isinstance(paths, basestring): - paths = [paths] - if not isinstance(paths, list) or \ - not all(map(lambda p: isinstance(p, basestring), paths)): - self.fail(u'must be string or a list of strings', view, True) - return paths - - class Filename(Template): """A template that validates strings as filenames. From 395be1a0f62e7aca0536b3a626effc2d362cd05d Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 21:40:00 -0700 Subject: [PATCH 21/51] Fix missing super-constructor call --- beets/util/confit.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beets/util/confit.py b/beets/util/confit.py index 40cefc1fa..e5e48ec4a 100644 --- a/beets/util/confit.py +++ b/beets/util/confit.py @@ -1065,6 +1065,7 @@ class StrSeq(Template): string, it should be split on whitespace. Otherwise, the resulting value is a list containing a single string. """ + super(StrSeq, self).__init__() self.split = split def convert(self, value, view): From 0bb6348df376f39ff899881b12d3ac54db72c2f6 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Fri, 12 Sep 2014 11:37:14 +0200 Subject: [PATCH 22/51] fetchart: Add test that reproduces #887 --- test/helper.py | 16 +++++++++++--- test/test_fetchart.py | 49 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 test/test_fetchart.py diff --git a/test/helper.py b/test/helper.py index cf2a67ad0..bfc433528 100644 --- a/test/helper.py +++ b/test/helper.py @@ -241,17 +241,27 @@ class TestHelper(object): includes a counter to make sure we do not create items with the same attributes. """ + item_count = self._get_item_count() values = { - 'title': u't\u00eftle {0}'.format(self._get_item_count()), + 'title': u't\u00eftle {0}', 'artist': u'the \u00e4rtist', 'album': u'the \u00e4lbum', + 'track': item_count, + 'path': 'audio.mp3', } values.update(values_) + values['title'] = values['title'].format(item_count) item = Item(**values) - if hasattr(self, 'lib'): - item.add(self.lib) + item.add(self.lib) + if not 'path' in values_: + item['path'] = item.destination() + item.store() return item + def add_album(self, **values): + item = self.add_item(**values) + return self.lib.add_album([item]) + def add_item_fixtures(self, ext='mp3', count=1): """Add a number of items with files to the database. """ diff --git a/test/test_fetchart.py b/test/test_fetchart.py new file mode 100644 index 000000000..5e36f9145 --- /dev/null +++ b/test/test_fetchart.py @@ -0,0 +1,49 @@ +# This file is part of beets. +# Copyright 2014, Thomas Scholtes. +# +# 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. + +import os.path +from _common import unittest +from helper import TestHelper + + +class FetchartCliTest(unittest.TestCase, TestHelper): + + def setUp(self): + self.setup_beets() + self.load_plugins('fetchart') + + def tearDown(self): + self.unload_plugins() + self.teardown_beets() + + def test_set_art_from_folder(self): + self.config['fetchart']['cover_names'] = 'c\xc3\xb6ver.jpg' + self.config['art_filename'] = 'mycover' + album = self.add_album() + self.touch('c\xc3\xb6ver.jpg', dir=album.path, content='IMAGE') + + self.run_command('fetchart') + cover_path = os.path.join(album.path, 'mycover.jpg') + + album.load() + self.assertEqual(album['artpath'], cover_path) + with open(cover_path, 'r') as f: + self.assertEqual(f.read(), 'IMAGE') + + +def suite(): + return unittest.TestLoader().loadTestsFromName(__name__) + +if __name__ == '__main__': + unittest.main(defaultTest='suite') From 89c82dc63d24fc08b8db668761243981f1f1d7a4 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Fri, 12 Sep 2014 11:07:43 +0200 Subject: [PATCH 23/51] fetchart: correctly handle path encoding * Ensure that `config.as_str_seq()` returns a list of unicode objects. * Map these to bytestring paths so we can compare them to other paths. Fixes #887 --- beets/util/confit.py | 21 +++++++++++++-------- beetsplug/fetchart.py | 1 + test/helper.py | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/beets/util/confit.py b/beets/util/confit.py index e5e48ec4a..c9051de53 100644 --- a/beets/util/confit.py +++ b/beets/util/confit.py @@ -1077,16 +1077,21 @@ class StrSeq(Template): return value.split() else: return [value] - else: - try: - value = list(value) - except TypeError: - self.fail('must be a whitespace-separated string or a list', - view, True) - if all(isinstance(x, BASESTRING) for x in value): - return value + + try: + value = list(value) + except TypeError: + self.fail('must be a whitespace-separated string or a list', + view, True) + + def convert(x): + if isinstance(x, unicode): + return x + elif isinstance(x, BASESTRING): + return x.decode('utf8', 'ignore') else: self.fail('must be a list of strings', view, True) + return map(convert, value) class Filename(Template): diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 632982edc..1474a7b09 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -234,6 +234,7 @@ def art_for_album(album, paths, maxwidth=None, local_only=False): # Local art. cover_names = config['fetchart']['cover_names'].as_str_seq() + cover_names = map(util.bytestring_path, cover_names) cautious = config['fetchart']['cautious'].get(bool) if paths: for path in paths: diff --git a/test/helper.py b/test/helper.py index bfc433528..0688708fd 100644 --- a/test/helper.py +++ b/test/helper.py @@ -253,7 +253,7 @@ class TestHelper(object): values['title'] = values['title'].format(item_count) item = Item(**values) item.add(self.lib) - if not 'path' in values_: + if 'path' not in values_: item['path'] = item.destination() item.store() return item From 9d55179d2d88e586d1051b6e404659e579e640f7 Mon Sep 17 00:00:00 2001 From: Simon Kohlmeyer Date: Sat, 13 Sep 2014 19:07:25 +0200 Subject: [PATCH 24/51] Added never_convert_lossy_files option to convert plugin When set to true, this config option chooses copying over converting when the source file is in a lossy format. At the moment, everything except ape, flac, alac and wav is considered lossy. --- beetsplug/convert.py | 5 +++++ docs/plugins/convert.rst | 4 ++++ test/test_convert.py | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index b840078af..e60d5fb07 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -37,6 +37,8 @@ ALIASES = { u'vorbis': u'ogg', } +LOSSLESS_FORMATS = ['ape', 'flac', 'alac', 'wav'] + def replace_ext(path, ext): """Return the path with its extension replaced by `ext`. @@ -128,6 +130,8 @@ def should_transcode(item, format): """Determine whether the item should be transcoded as part of conversion (i.e., its bitrate is high or it has the wrong format). """ + if config['convert']['never_convert_lossy_files'] and not (item.format.lower() in LOSSLESS_FORMATS): + return False maxbr = config['convert']['max_bitrate'].get(int) return format.lower() != item.format.lower() or \ item.bitrate >= 1000 * maxbr @@ -308,6 +312,7 @@ class ConvertPlugin(BeetsPlugin): u'quiet': False, u'embed': True, u'paths': {}, + u'never_convert_lossy_files': False, }) self.import_stages = [self.auto_convert] diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst index b94f874d7..3a2485b5e 100644 --- a/docs/plugins/convert.rst +++ b/docs/plugins/convert.rst @@ -67,6 +67,10 @@ The plugin offers several configuration options, all of which live under the adding them to your library. * ``quiet`` mode prevents the plugin from announcing every file it processes. Default: false. +* ``never_convert_lossy_files`` means that lossy codecs, such as mp3, ogg vorbis, + etc, are never converted, as converting lossy files to other lossy codecs will + decrease quality further. If set to true, lossy files are always copied. + Default: false * ``paths`` lets you specify the directory structure and naming scheme for the converted files. Use the same format as the top-level ``paths`` section (see :ref:`path-format-config`). By default, the plugin reuses your top-level diff --git a/test/test_convert.py b/test/test_convert.py index 24ed44748..4a6119884 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -124,6 +124,44 @@ class ConvertCliTest(unittest.TestCase, TestHelper): mediafile = MediaFile(converted) self.assertEqual(mediafile.images[0].data, image_data) +class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): + + def setUp(self): + self.setup_beets(disk=True) # Converter is threaded + self.album_ogg = self.add_album_fixture(ext='ogg') + self.album_flac = self.add_album_fixture(ext='flac') + self.load_plugins('convert') + + self.convert_dest = os.path.join(self.temp_dir, 'convert_dest') + self.config['convert'] = { + 'dest': self.convert_dest, + 'paths': {'default': 'converted'}, + 'never_convert_lossy_files': False, + 'format': 'mp3', + 'formats': { + 'mp3': 'cp $source $dest', + 'opus': { + 'command': 'cp $source $dest', + 'extension': 'ops', + } + } + } + + def tearDown(self): + self.unload_plugins() + self.teardown_beets() + + def test_convert_flac_to_mp3_works(self): + with control_stdin('y'): + self.run_command('convert', self.album_flac.items()[0].path) + converted = os.path.join(self.convert_dest, 'converted.mp3') + self.assertTrue(os.path.isfile(converted)) + + def test_convert_ogg_to_mp3_prevented(self): + with control_stdin('y'): + self.run_command('convert', self.album_ogg.items()[0].path) + converted = os.path.join(self.convert_dest, 'converted.mp3') + self.assertTrue(os.path.isfile(converted)) def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From ee6f3dba1b7538bebb88caa1cd67caf23e68ac16 Mon Sep 17 00:00:00 2001 From: Simon Kohlmeyer Date: Sat, 13 Sep 2014 19:51:23 +0200 Subject: [PATCH 25/51] fix coding style errors --- beetsplug/convert.py | 5 +++-- test/test_convert.py | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index e60d5fb07..a72965b43 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -130,8 +130,9 @@ def should_transcode(item, format): """Determine whether the item should be transcoded as part of conversion (i.e., its bitrate is high or it has the wrong format). """ - if config['convert']['never_convert_lossy_files'] and not (item.format.lower() in LOSSLESS_FORMATS): - return False + if config['convert']['never_convert_lossy_files'] and \ + not (item.format.lower() in LOSSLESS_FORMATS): + return False maxbr = config['convert']['max_bitrate'].get(int) return format.lower() != item.format.lower() or \ item.bitrate >= 1000 * maxbr diff --git a/test/test_convert.py b/test/test_convert.py index 4a6119884..68fd77f7e 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -124,6 +124,7 @@ class ConvertCliTest(unittest.TestCase, TestHelper): mediafile = MediaFile(converted) self.assertEqual(mediafile.images[0].data, image_data) + class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): def setUp(self): @@ -163,6 +164,7 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): converted = os.path.join(self.convert_dest, 'converted.mp3') self.assertTrue(os.path.isfile(converted)) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 4870d7e0fa881a1c3b302ca70255026f1db90df9 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 13 Sep 2014 17:16:12 -0700 Subject: [PATCH 26/51] Roll back fast flexible field sorts (#953) Sad to see them go, but happy be rid of the SQL injection. --- beets/dbcore/db.py | 2 +- beets/dbcore/query.py | 71 ++++++++++---------------------------- beets/dbcore/queryparse.py | 7 ++-- test/test_dbcore.py | 2 +- test/test_sort.py | 28 +++++++-------- 5 files changed, 36 insertions(+), 74 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 7a912c3a7..c15f02422 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -751,7 +751,7 @@ class Database(object): Query object, or None (to fetch everything). If provided, `sort_order` is either a SQLite ORDER BY clause for sorting or a Sort object. - """ + """ sql, subvals, query, sort = build_sql(model_cls, query, sort_order) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 3ef5ce30d..23b6029c3 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -613,56 +613,29 @@ class MultipleSort(Sort): return items -class FlexFieldSort(Sort): - """Sort object to sort on a flexible attribute field +class FieldSort(Sort): + """An abstract sort criterion that orders by a specific field (of + any kind). """ - def __init__(self, model_cls, field, is_ascending): - self.model_cls = model_cls + def __init__(self, field, ascending=True): self.field = field - self.is_ascending = is_ascending + self.ascending = ascending - def select_clause(self): - """Return a SELECT fragment. - """ - return "sort_flexattr{0!s}.value as flex_{0!s} ".format(self.field) - - def union_clause(self): - """Return a JOIN fragment. - """ - 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): - """Return an ORDER BY fragment. - """ - 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)) + def sort(self, objs): + # TODO: Conversion and null-detection here. In Python 3, + # comparisons with None fail. We should also support flexible + # attributes with different types without falling over. + return sorted(objs, key=attrgetter(self.field), + reverse=not self.ascending) -class FixedFieldSort(Sort): - """Sort object to sort on a fixed field +class FixedFieldSort(FieldSort): + """Sort object to sort on a fixed field. """ - def __init__(self, field, is_ascending=True): - self.field = field - self.is_ascending = is_ascending - def order_clause(self): - order = "ASC" if self.is_ascending else "DESC" + order = "ASC" if self.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 @@ -695,19 +668,13 @@ 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() - +class SlowFieldSort(FieldSort): + """A sort criterion by some model field other than a fixed field: + i.e., a computed or flexible field. + """ 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} @@ -740,7 +707,7 @@ def build_sql(model_cls, query, sort): order_clause = sort.order_clause() sort_order = " ORDER BY {0}".format(order_clause) \ if order_clause else "" - if sort.is_slow(): + if not sort.is_slow(): sort = None sql = ("SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE " diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py index 324274a5a..0da997bc4 100644 --- a/beets/dbcore/queryparse.py +++ b/beets/dbcore/queryparse.py @@ -131,14 +131,11 @@ def construct_sort_part(model_cls, part): 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. - sort = query.ComputedFieldSort(model_cls, field, is_ascending) 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) + # Flexible or comptued. + sort = query.SlowFieldSort(field, is_ascending) return sort diff --git a/test/test_dbcore.py b/test/test_dbcore.py index 9d0dd1d3b..9167acfa5 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -453,7 +453,7 @@ class SortFromStringsTest(unittest.TestCase): 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) + self.assertIsInstance(s.sorts[0], dbcore.query.SlowFieldSort) def suite(): diff --git a/test/test_sort.py b/test/test_sort.py index 1c6cc9741..316e9800d 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -131,7 +131,7 @@ class SortFixedFieldTest(DummyDataTestCase): class SortFlexFieldTest(DummyDataTestCase): def test_sort_asc(self): q = '' - sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", True) + sort = dbcore.query.SlowFieldSort("flex1", True) results = self.lib.items(q, sort) self.assertLessEqual(results[0]['flex1'], results[1]['flex1']) self.assertEqual(results[0]['flex1'], 'flex1-0') @@ -143,7 +143,7 @@ class SortFlexFieldTest(DummyDataTestCase): def test_sort_desc(self): q = '' - sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", False) + sort = dbcore.query.SlowFieldSort("flex1", False) results = self.lib.items(q, sort) self.assertGreaterEqual(results[0]['flex1'], results[1]['flex1']) self.assertGreaterEqual(results[1]['flex1'], results[2]['flex1']) @@ -157,8 +157,8 @@ class SortFlexFieldTest(DummyDataTestCase): 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) + s1 = dbcore.query.SlowFieldSort("flex2", False) + s2 = dbcore.query.SlowFieldSort("flex1", True) sort = dbcore.query.MultipleSort() sort.add_sort(s1) sort.add_sort(s2) @@ -220,10 +220,10 @@ class SortAlbumFixedFieldTest(DummyDataTestCase): self.assertEqual(r1.id, r2.id) -class SortAlbumFlexdFieldTest(DummyDataTestCase): +class SortAlbumFlexFieldTest(DummyDataTestCase): def test_sort_asc(self): q = '' - sort = dbcore.query.FlexFieldSort(beets.library.Album, "flex1", True) + sort = dbcore.query.SlowFieldSort("flex1", True) results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['flex1'], results[1]['flex1']) self.assertLessEqual(results[1]['flex1'], results[2]['flex1']) @@ -235,7 +235,7 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase): def test_sort_desc(self): q = '' - sort = dbcore.query.FlexFieldSort(beets.library.Album, "flex1", False) + sort = dbcore.query.SlowFieldSort("flex1", False) results = self.lib.albums(q, sort) self.assertGreaterEqual(results[0]['flex1'], results[1]['flex1']) self.assertGreaterEqual(results[1]['flex1'], results[2]['flex1']) @@ -247,8 +247,8 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase): 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) + s1 = dbcore.query.SlowFieldSort("flex2", True) + s2 = dbcore.query.SlowFieldSort("flex1", True) sort = dbcore.query.MultipleSort() sort.add_sort(s1) sort.add_sort(s2) @@ -268,8 +268,7 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase): class SortAlbumComputedFieldTest(DummyDataTestCase): def test_sort_asc(self): q = '' - sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", - True) + sort = dbcore.query.SlowFieldSort("path", True) results = self.lib.albums(q, sort) self.assertLessEqual(results[0]['path'], results[1]['path']) self.assertLessEqual(results[1]['path'], results[2]['path']) @@ -281,8 +280,7 @@ class SortAlbumComputedFieldTest(DummyDataTestCase): def test_sort_desc(self): q = '' - sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", - False) + sort = dbcore.query.SlowFieldSort("path", False) results = self.lib.albums(q, sort) self.assertGreaterEqual(results[0]['path'], results[1]['path']) self.assertGreaterEqual(results[1]['path'], results[2]['path']) @@ -296,7 +294,7 @@ class SortAlbumComputedFieldTest(DummyDataTestCase): class SortCombinedFieldTest(DummyDataTestCase): def test_computed_first(self): q = '' - s1 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True) + s1 = dbcore.query.SlowFieldSort("path", True) s2 = dbcore.query.FixedFieldSort("year", True) sort = dbcore.query.MultipleSort() sort.add_sort(s1) @@ -312,7 +310,7 @@ class SortCombinedFieldTest(DummyDataTestCase): def test_computed_second(self): q = '' s1 = dbcore.query.FixedFieldSort("year", True) - s2 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True) + s2 = dbcore.query.SlowFieldSort("path", True) sort = dbcore.query.MultipleSort() sort.add_sort(s1) sort.add_sort(s2) From 369533d46fbb6f6ec4e79ed97cf5ef4991a8070e Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 13 Sep 2014 19:47:20 -0700 Subject: [PATCH 27/51] Remove special_sorts These are not extensible anyway; we'll need another mechanism for that. --- beets/dbcore/query.py | 31 ++++++++----------------------- beets/dbcore/queryparse.py | 7 ++++--- beets/library.py | 10 ---------- 3 files changed, 12 insertions(+), 36 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 23b6029c3..9c53cf6ed 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -638,34 +638,22 @@ class FixedFieldSort(FieldSort): class SmartArtistSort(Sort): - """ Sort Album or Item on artist sort fields, defaulting back on - artist field if the sort specific field is empty. + """Sort by artist (either album artist or track artist), + prioritizing the sort field over the raw field. """ 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' + if 'albumartist' in self.model_cls._fields: + field = 'albumartist' else: - return "" - - order_str = ('(CASE {0} WHEN NULL THEN {1} ' - 'WHEN "" THEN {1} ' - 'ELSE {0} END) {2} ').format(exp1, exp2, order) - return order_str + field = 'artist' + return ('(CASE {0}_sort WHEN NULL THEN {0} ' + 'WHEN "" THEN {0} ' + 'ELSE {0}_sort END) {2} ').format(field, order) class SlowFieldSort(FieldSort): @@ -676,9 +664,6 @@ class SlowFieldSort(FieldSort): return True -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. Query and sort objects are returned diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py index 0da997bc4..cf8a75ffe 100644 --- a/beets/dbcore/queryparse.py +++ b/beets/dbcore/queryparse.py @@ -131,10 +131,11 @@ def construct_sort_part(model_cls, part): is_ascending = (part[-1] == '+') if field in model_cls._fields: sort = query.FixedFieldSort(field, is_ascending) - elif field in query.special_sorts: - sort = query.special_sorts[field](model_cls, is_ascending) + elif field == 'smartartist': + # Special case for smart artist sort. + sort = query.SmartArtistSort(model_cls, is_ascending) else: - # Flexible or comptued. + # Flexible or computed. sort = query.SlowFieldSort(field, is_ascending) return sort diff --git a/beets/library.py b/beets/library.py index 3119e1bfa..f053728b1 100644 --- a/beets/library.py +++ b/beets/library.py @@ -139,16 +139,6 @@ class MusicalKey(types.String): PF_KEY_DEFAULT = 'default' -# A little SQL utility. -def _orelse(exp1, exp2): - """Generates an SQLite expression that evaluates to exp1 if exp1 is - non-null and non-empty or exp2 otherwise. - """ - return ("""(CASE {0} WHEN NULL THEN {1} - WHEN "" THEN {1} - ELSE {0} END)""").format(exp1, exp2) - - # Exceptions. class FileOperationError(Exception): From 2b921b19fd5a23bc2e86060c2c12137d63dfe1db Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 13 Sep 2014 20:36:39 -0700 Subject: [PATCH 28/51] NullSort instead of None A more descriptive placeholder for "don't sort". --- beets/dbcore/db.py | 1 - beets/dbcore/query.py | 27 +++++++++++++++++++++++---- beets/dbcore/queryparse.py | 26 +++++++++++++++++--------- test/test_dbcore.py | 2 +- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index c15f02422..5eba083f3 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -752,7 +752,6 @@ class Database(object): `sort_order` is either a SQLite ORDER BY clause for sorting or a Sort object. """ - sql, subvals, query, sort = build_sql(model_cls, query, sort_order) with self.transaction() as tx: diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 9c53cf6ed..7779d4598 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -531,7 +531,7 @@ class Sort(object): def sort(self, items): """Sort the list of objects and return a list. """ - return sorted(items, key=lambda x: x) + return sorted(items) def is_slow(self): """Indicate whether this query is *slow*, meaning that it cannot @@ -544,8 +544,8 @@ class MultipleSort(Sort): """Sort that encapsulates multiple sub-sorts. """ - def __init__(self): - self.sorts = [] + def __init__(self, sorts=None): + self.sorts = sorts or [] def add_sort(self, sort): self.sorts.append(sort) @@ -612,6 +612,9 @@ class MultipleSort(Sort): items = sort.sort(items) return items + def __repr__(self): + return u'MultipleSort({0})'.format(repr(self.sorts)) + class FieldSort(Sort): """An abstract sort criterion that orders by a specific field (of @@ -628,6 +631,13 @@ class FieldSort(Sort): return sorted(objs, key=attrgetter(self.field), reverse=not self.ascending) + def __repr__(self): + return u'<{0}: {1}{2}>'.format( + type(self).__name__, + self.field, + '+' if self.ascending else '-', + ) + class FixedFieldSort(FieldSort): """Sort object to sort on a fixed field. @@ -653,7 +663,7 @@ class SmartArtistSort(Sort): field = 'artist' return ('(CASE {0}_sort WHEN NULL THEN {0} ' 'WHEN "" THEN {0} ' - 'ELSE {0}_sort END) {2} ').format(field, order) + 'ELSE {0}_sort END) {1} ').format(field, order) class SlowFieldSort(FieldSort): @@ -664,6 +674,15 @@ class SlowFieldSort(FieldSort): return True +class NullSort(Sort): + """No sorting. Leave results unsorted.""" + def sort(items): + return items + + def __nonzero__(self): + return False + + 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. Query and sort objects are returned diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py index cf8a75ffe..8a50e04d5 100644 --- a/beets/dbcore/queryparse.py +++ b/beets/dbcore/queryparse.py @@ -124,11 +124,18 @@ def query_from_strings(query_cls, model_cls, prefixes, query_parts): def construct_sort_part(model_cls, part): - """ Creates a Sort object from a single criteria. Returns a `Sort` instance. + """Create a `Sort` from a single string criterion. + + `model_cls` is the `Model` being queried. `part` is a single string + ending in ``+`` or ``-`` indicating the sort. """ - sort = None + assert part, "part must be a field name and + or -" field = part[:-1] - is_ascending = (part[-1] == '+') + assert field, "field is missing" + direction = part[-1] + assert direction in ('+', '-'), "part must end with + or -" + is_ascending = direction == '+' + if field in model_cls._fields: sort = query.FixedFieldSort(field, is_ascending) elif field == 'smartartist': @@ -141,11 +148,12 @@ def construct_sort_part(model_cls, part): def sort_from_strings(model_cls, sort_parts): - """Creates a Sort object from a list of sort criteria strings. + """Create a `Sort` from a list of sort criteria (strings). """ if not sort_parts: - return None - sort = query.MultipleSort() - for part in sort_parts: - sort.add_sort(construct_sort_part(model_cls, part)) - return sort + return query.NullSort() + else: + sort = query.MultipleSort() + for part in sort_parts: + sort.add_sort(construct_sort_part(model_cls, part)) + return sort diff --git a/test/test_dbcore.py b/test/test_dbcore.py index 9167acfa5..68a4b61ef 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -434,7 +434,7 @@ class SortFromStringsTest(unittest.TestCase): def test_zero_parts(self): s = self.sfs([]) - self.assertIsNone(s) + self.assertIsInstance(s, dbcore.query.NullSort) def test_one_parts(self): s = self.sfs(['field+']) From 85de214399e8165445cae6d4ad1f34daea3af33c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 13 Sep 2014 21:19:32 -0700 Subject: [PATCH 29/51] Remove SQL ORDER BY sorting option I don't think anything uses this anymore. --- beets/dbcore/query.py | 67 +++++++++---------------------------------- 1 file changed, 13 insertions(+), 54 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 7779d4598..b86512538 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -511,20 +511,9 @@ class Sort(object): the item database. """ - def select_clause(self): - """Generate a SELECT fragment (possibly an empty string) for the - sort. - """ - return "" - - def union_clause(self): - """Generate a join SQL fragment (possibly an empty string). - """ - 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 used in a ORDER BY clause, or + None if no fragment is used (i.e., this is a slow sort). """ return None @@ -551,10 +540,13 @@ class MultipleSort(Sort): self.sorts.append(sort) def _sql_sorts(self): - """ Returns the list of sort for which sql can be used + """Return the list of sub-sorts for which we can be (at least + partially) fast. + + A contiguous suffix of fast (SQL-capable) sub-sorts are + executable in SQL. The remaining, even if they are fast + independently, must be executed slowly. """ - # With several sort criteria, 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: @@ -564,31 +556,13 @@ class MultipleSort(Sort): sql_sorts.reverse() return sql_sorts - def select_clause(self): - select_strings = [] - for sort in self._sql_sorts(): - select = sort.select_clause() - if select: - select_strings.append(select) - - select_string = ",".join(select_strings) - return select_string - - def union_clause(self): - union_strings = [] - for sort in self._sql_sorts(): - union = sort.union_clause() - union_strings.append(union) - - return "".join(union_strings) - def order_clause(self): order_strings = [] for sort in self._sql_sorts(): order = sort.order_clause() order_strings.append(order) - return ",".join(order_strings) + return ", ".join(order_strings) def is_slow(self): for sort in self.sorts: @@ -663,7 +637,7 @@ class SmartArtistSort(Sort): field = 'artist' return ('(CASE {0}_sort WHEN NULL THEN {0} ' 'WHEN "" THEN {0} ' - 'ELSE {0}_sort END) {1} ').format(field, order) + 'ELSE {0}_sort END) {1}').format(field, order) class SlowFieldSort(FieldSort): @@ -693,34 +667,19 @@ def build_sql(model_cls, query, sort): query = None if not sort: - sort_select = "" - sort_union = "" sort_order = "" sort = None - elif isinstance(sort, basestring): - sort_select = "" - sort_union = "" - sort_order = " ORDER BY {0}".format(sort) \ - if sort else "" - 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() order_clause = sort.order_clause() - sort_order = " ORDER BY {0}".format(order_clause) \ + sort_order = "ORDER BY {0}".format(order_clause) \ if order_clause else "" if not sort.is_slow(): sort = None - sql = ("SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE " - "{query_clause} {sort_order}").format( - sort_select=sort_select, - sort_union=sort_union, + sql = ("SELECT * FROM {table} WHERE {query_clause} {sort_order}").format( table=model_cls._table, query_clause=where or '1', - sort_order=sort_order + sort_order=sort_order, ) return sql, subvals, query, sort From c57439770bbcd9c8169f606df6e318cdc4ec087e Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 13 Sep 2014 21:35:19 -0700 Subject: [PATCH 30/51] Collapse build_sql into _fetch (#953) This was getting so short that it made sense to go back from whence it came. --- beets/dbcore/db.py | 25 ++++++++++++++++++------- beets/dbcore/query.py | 28 ---------------------------- 2 files changed, 18 insertions(+), 35 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 5eba083f3..2063cff72 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, build_sql +from .query import MatchQuery, NullSort from .types import BASE_TYPE @@ -745,19 +745,30 @@ class Database(object): # Querying. - def _fetch(self, model_cls, query, sort_order=None): + def _fetch(self, model_cls, query, sort=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, - `sort_order` is either a SQLite ORDER BY clause for sorting or a - Sort object. + Query object, or None (to fetch everything). `sort` is an + optional Sort object. """ - sql, subvals, query, sort = build_sql(model_cls, query, sort_order) + where, subvals = query.clause() + sort = sort or NullSort() + order_by = sort.order_clause() + + sql = ("SELECT * FROM {0} WHERE {1} {2}").format( + model_cls._table, + where or '1', + "ORDER BY {0}".format(order_by) if order_by else '', + ) with self.transaction() as tx: rows = tx.query(sql, subvals) - return Results(model_cls, rows, self, query, sort) + return Results( + model_cls, rows, self, + None if where else query, # Slow query component. + sort if sort.is_slow() else None, # Slow sort component. + ) 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 b86512538..7197d2564 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -655,31 +655,3 @@ class NullSort(Sort): def __nonzero__(self): return False - - -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. Query and sort objects are returned - only for slow query and slow sort operation. - """ - where, subvals = query.clause() - if where is not None: - query = None - - if not sort: - sort_order = "" - sort = None - elif isinstance(sort, Sort): - order_clause = sort.order_clause() - sort_order = "ORDER BY {0}".format(order_clause) \ - if order_clause else "" - if not sort.is_slow(): - sort = None - - sql = ("SELECT * FROM {table} WHERE {query_clause} {sort_order}").format( - table=model_cls._table, - query_clause=where or '1', - sort_order=sort_order, - ) - - return sql, subvals, query, sort From f3e87b5b1b6f23260ac7d0d1564eabde87f82b0c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 13 Sep 2014 23:49:56 -0700 Subject: [PATCH 31/51] Changelog for #956 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 65b28ff16..5ee2cd134 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -14,6 +14,8 @@ Features: ``--summarize`` option. * :doc:`/plugins/mbcollection`: A new option lets you automatically update your collection on import. Thanks to Olin Gay. +* :doc:`/plugins/convert`: A new ``never_convert_lossy_files`` option can + prevent lossy transcoding. Thanks to Simon Kohlmeyer. Fixes: From a37cabb969edfcc34a6ec628b1bd97eda15eeedd Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 11:17:34 +0200 Subject: [PATCH 32/51] Make 'never_convert_lossy_files' tests more meaningful We should expect the original file extensions to be preserved since we do not transcode lossy files. The tests now fail, but a fix is coming up. --- test/test_convert.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/test_convert.py b/test/test_convert.py index 68fd77f7e..d57387fdf 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -126,6 +126,8 @@ class ConvertCliTest(unittest.TestCase, TestHelper): class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): + """Test the effect of the `never_convert_lossy_files` option. + """ def setUp(self): self.setup_beets(disk=True) # Converter is threaded @@ -137,7 +139,7 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): self.config['convert'] = { 'dest': self.convert_dest, 'paths': {'default': 'converted'}, - 'never_convert_lossy_files': False, + 'never_convert_lossy_files': True, 'format': 'mp3', 'formats': { 'mp3': 'cp $source $dest', @@ -161,7 +163,7 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): def test_convert_ogg_to_mp3_prevented(self): with control_stdin('y'): self.run_command('convert', self.album_ogg.items()[0].path) - converted = os.path.join(self.convert_dest, 'converted.mp3') + converted = os.path.join(self.convert_dest, 'converted.ogg') self.assertTrue(os.path.isfile(converted)) From 3197795faaaed07dbd59dc8df41bec791622d759 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 11:27:59 +0200 Subject: [PATCH 33/51] convert: Change file extension only if actually transcoded This makes the tests from a37cabb969edfcc34a6ec628b1bd97eda15eeedd pass. --- beetsplug/convert.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index a72965b43..6e7440cbc 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -150,10 +150,9 @@ def convert_item(dest_dir, keep_new, path_formats, format, pretend=False): # back to its old path or transcode it to a new path. if keep_new: original = dest - converted = replace_ext(item.path, ext) + converted = item.path else: original = item.path - dest = replace_ext(dest, ext) converted = dest # Ensure that only one thread tries to create directories at a @@ -194,6 +193,7 @@ def convert_item(dest_dir, keep_new, path_formats, format, pretend=False): ) util.copy(original, converted) else: + converted = replace_ext(converted, ext) try: encode(command, original, converted, pretend) except subprocess.CalledProcessError: @@ -217,7 +217,12 @@ def convert_item(dest_dir, keep_new, path_formats, format, pretend=False): if album and album.artpath: embed_item(item, album.artpath, itempath=converted) - plugins.send('after_convert', item=item, dest=dest, keepnew=keep_new) + if keep_new: + plugins.send('after_convert', item=item, + dest=dest, keepnew=True) + else: + plugins.send('after_convert', item=item, + dest=converted, keepnew=False) def convert_on_import(lib, item): From 75a28de543d685815d748eab2a0ced231c0c453e Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 11:35:01 +0200 Subject: [PATCH 34/51] Extend and refactor 'never_convert_lossy_files' tests * Each test function only uses on fixture, so we create fixtures in the test functions and not in the `setup()` method. * We test the effect of the option set to true and false. --- test/test_convert.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/test/test_convert.py b/test/test_convert.py index d57387fdf..29cdfc244 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -131,8 +131,6 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): def setUp(self): self.setup_beets(disk=True) # Converter is threaded - self.album_ogg = self.add_album_fixture(ext='ogg') - self.album_flac = self.add_album_fixture(ext='flac') self.load_plugins('convert') self.convert_dest = os.path.join(self.temp_dir, 'convert_dest') @@ -143,10 +141,6 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): 'format': 'mp3', 'formats': { 'mp3': 'cp $source $dest', - 'opus': { - 'command': 'cp $source $dest', - 'extension': 'ops', - } } } @@ -154,15 +148,25 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): self.unload_plugins() self.teardown_beets() - def test_convert_flac_to_mp3_works(self): + def test_transcode_from_lossles(self): + [item] = self.add_item_fixtures(ext='flac') with control_stdin('y'): - self.run_command('convert', self.album_flac.items()[0].path) + self.run_command('convert', item.path) converted = os.path.join(self.convert_dest, 'converted.mp3') self.assertTrue(os.path.isfile(converted)) - def test_convert_ogg_to_mp3_prevented(self): + def test_transcode_from_lossy(self): + self.config['convert']['never_convert_lossy_files'] = False + [item] = self.add_item_fixtures(ext='ogg') with control_stdin('y'): - self.run_command('convert', self.album_ogg.items()[0].path) + self.run_command('convert', item.path) + converted = os.path.join(self.convert_dest, 'converted.mp3') + self.assertTrue(os.path.isfile(converted)) + + def test_transcode_from_lossy_prevented(self): + [item] = self.add_item_fixtures(ext='ogg') + with control_stdin('y'): + self.run_command('convert', item.path) converted = os.path.join(self.convert_dest, 'converted.ogg') self.assertTrue(os.path.isfile(converted)) From 9e9f645e592f3efc197b422730d39ed0e9f6a303 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 11:46:24 +0200 Subject: [PATCH 35/51] convert: add cli flag to skip confirmation The flag mirrors the `--yes` flag from the modify command. --- beetsplug/convert.py | 4 +++- docs/changelog.rst | 2 ++ docs/plugins/convert.rst | 15 ++++++++------- test/test_convert.py | 11 +++++++++++ 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 6e7440cbc..16afd6dc1 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -269,7 +269,7 @@ def convert_func(lib, opts, args): if not pretend: ui.commands.list_items(lib, ui.decargs(args), opts.album, None) - if not ui.input_yn("Convert? (Y/n)"): + if not (opts.yes or ui.input_yn("Convert? (Y/n)")): return if opts.album: @@ -338,6 +338,8 @@ class ConvertPlugin(BeetsPlugin): help='set the destination directory') cmd.parser.add_option('-f', '--format', action='store', dest='format', help='set the destination directory') + cmd.parser.add_option('-y', '--yes', action='store', dest='yes', + help='do not ask for confirmation') cmd.func = convert_func return [cmd] diff --git a/docs/changelog.rst b/docs/changelog.rst index 5ee2cd134..db1b82b7f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,8 @@ Features: your collection on import. Thanks to Olin Gay. * :doc:`/plugins/convert`: A new ``never_convert_lossy_files`` option can prevent lossy transcoding. Thanks to Simon Kohlmeyer. +* :doc:`/plugins/convert`: A new ``--yes`` command-line flag skips the + confirmation. Fixes: diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst index 3a2485b5e..135d239a7 100644 --- a/docs/plugins/convert.rst +++ b/docs/plugins/convert.rst @@ -20,19 +20,20 @@ transcode the audio, so you might want to install it. Usage ----- -To convert a part of your collection, run ``beet convert QUERY``. This -will display all items matching ``QUERY`` and ask you for confirmation before -starting the conversion. The command will then transcode all the -matching files to the destination directory given by the ``-d`` -(``--dest``) option or the ``dest`` configuration. The path layout -mirrors that of your library, but it may be customized through the -``paths`` configuration. +To convert a part of your collection, run ``beet convert QUERY``. The +command will transcode all the files matching the query to the +destination directory given by the ``-d`` (``--dest``) option or the +``dest`` configuration. The path layout mirrors that of your library, +but it may be customized through the ``paths`` configuration. The plugin uses a command-line program to transcode the audio. With the ``-f`` (``--format``) option you can choose the transcoding command and customize the available commands :ref:`through the configuration `. +Unless the ``-y`` (``--yes``) flag is set, the command will list all +the items to be converted and ask for your confirmation. + The ``-a`` (or ``--album``) option causes the command to match albums instead of tracks. diff --git a/test/test_convert.py b/test/test_convert.py index 29cdfc244..3b865ba5c 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -95,6 +95,17 @@ class ConvertCliTest(unittest.TestCase, TestHelper): converted = os.path.join(self.convert_dest, 'converted.mp3') self.assertTrue(os.path.isfile(converted)) + def test_convert_with_auto_confirmation(self): + self.run_command('convert', '--yes', self.item.path) + converted = os.path.join(self.convert_dest, 'converted.mp3') + self.assertTrue(os.path.isfile(converted)) + + def test_rejecet_confirmation(self): + with control_stdin('n'): + self.run_command('convert', self.item.path) + converted = os.path.join(self.convert_dest, 'converted.mp3') + self.assertFalse(os.path.isfile(converted)) + def test_convert_keep_new(self): self.assertEqual(os.path.splitext(self.item.path)[1], '.ogg') From 152672098267872bec10f3c7a110766822b7f653 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 11:49:33 +0200 Subject: [PATCH 36/51] Swap 'if not' and 'else' for readability --- beetsplug/convert.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 16afd6dc1..a86fc017a 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -180,7 +180,13 @@ def convert_item(dest_dir, keep_new, path_formats, format, pretend=False): ) util.move(item.path, original) - if not should_transcode(item, format): + if should_transcode(item, format): + converted = replace_ext(converted, ext) + try: + encode(command, original, converted, pretend) + except subprocess.CalledProcessError: + continue + else: if pretend: log.info(u'cp {0} {1}'.format( util.displayable_path(original), @@ -192,12 +198,6 @@ def convert_item(dest_dir, keep_new, path_formats, format, pretend=False): util.displayable_path(item.path)) ) util.copy(original, converted) - else: - converted = replace_ext(converted, ext) - try: - encode(command, original, converted, pretend) - except subprocess.CalledProcessError: - continue if pretend: continue From 9a732fbd2648ce18e96c64119e8e409d2f66b6b6 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 12:17:05 +0200 Subject: [PATCH 37/51] Converted files are tagged and checked in tests The conversion function either copies files or runs the conversion command on them, depending on the result of the `should_transcode()` function. This commit makes the tests more sensitive to these cases. --- test/test_convert.py | 70 +++++++++++++++++++++++++++++++------------- 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/test/test_convert.py b/test/test_convert.py index 3b865ba5c..be6d12202 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -12,14 +12,52 @@ # The above copyright notice and this permission notice shall be # included in all copies or substantial portions of the Software. +import re import os.path import _common from _common import unittest -from helper import TestHelper, control_stdin +import helper +from helper import control_stdin from beets.mediafile import MediaFile +class TestHelper(helper.TestHelper): + + def tagged_copy_cmd(self, tag): + """Return a conversion command that copies files and appends + `tag` to the copy. + """ + if re.search('[^a-zA-Z0-9]', tag): + raise ValueError(u"tag '{0}' must only contain letters and digits" + .format(tag)) + # FIXME This is not portable. For windows we need to use our own + # python script that performs the same task. + return u'cp $source $dest; printf {0} >> $dest'.format(tag) + + def assertFileTag(self, path, tag): + """Assert that the path is a file and the files content ends with `tag`. + """ + self.assertTrue(os.path.isfile(path), + u'{0} is not a file'.format(path)) + with open(path) as f: + f.seek(-len(tag), os.SEEK_END) + self.assertEqual(f.read(), tag, + u'{0} is not tagged with {1}'.format(path, tag)) + + def assertNoFileTag(self, path, tag): + """Assert that the path is a file and the files content does not + end with `tag`. + """ + self.assertTrue(os.path.isfile(path), + u'{0} is not a file'.format(path)) + with open(path) as f: + f.seek(-len(tag), os.SEEK_END) + self.assertNotEqual(f.read(), tag, + u'{0} is unexpectedly tagged with {1}' + .format(path, tag)) + + class ImportConvertTest(unittest.TestCase, TestHelper): def setUp(self): @@ -29,9 +67,7 @@ class ImportConvertTest(unittest.TestCase, TestHelper): self.config['convert'] = { 'dest': os.path.join(self.temp_dir, 'convert'), - # Append string so we can determine if the file was - # converted - 'command': u'cp $source $dest; printf convert >> $dest', + 'command': self.tagged_copy_cmd('convert'), # Enforce running convert 'max_bitrate': 1, 'auto': True, @@ -45,7 +81,7 @@ class ImportConvertTest(unittest.TestCase, TestHelper): def test_import_converted(self): self.importer.run() item = self.lib.items().get() - self.assertConverted(item.path) + self.assertFileTag(item.path, 'convert') def test_import_original_on_convert_error(self): # `false` exits with non-zero code @@ -56,12 +92,6 @@ class ImportConvertTest(unittest.TestCase, TestHelper): self.assertIsNotNone(item) self.assertTrue(os.path.isfile(item.path)) - def assertConverted(self, path): - with open(path) as f: - f.seek(-7, os.SEEK_END) - self.assertEqual(f.read(), 'convert', - '{0} was not converted'.format(path)) - class ConvertCliTest(unittest.TestCase, TestHelper): @@ -77,9 +107,9 @@ class ConvertCliTest(unittest.TestCase, TestHelper): 'paths': {'default': 'converted'}, 'format': 'mp3', 'formats': { - 'mp3': 'cp $source $dest', + 'mp3': self.tagged_copy_cmd('mp3'), 'opus': { - 'command': 'cp $source $dest', + 'command': self.tagged_copy_cmd('opus'), 'extension': 'ops', } } @@ -93,12 +123,12 @@ class ConvertCliTest(unittest.TestCase, TestHelper): with control_stdin('y'): self.run_command('convert', self.item.path) converted = os.path.join(self.convert_dest, 'converted.mp3') - self.assertTrue(os.path.isfile(converted)) + self.assertFileTag(converted, 'mp3') def test_convert_with_auto_confirmation(self): self.run_command('convert', '--yes', self.item.path) converted = os.path.join(self.convert_dest, 'converted.mp3') - self.assertTrue(os.path.isfile(converted)) + self.assertFileTag(converted, 'mp3') def test_rejecet_confirmation(self): with control_stdin('n'): @@ -119,7 +149,7 @@ class ConvertCliTest(unittest.TestCase, TestHelper): with control_stdin('y'): self.run_command('convert', '--format', 'opus', self.item.path) converted = os.path.join(self.convert_dest, 'converted.ops') - self.assertTrue(os.path.isfile(converted)) + self.assertFileTag(converted, 'opus') def test_embed_album_art(self): self.config['convert']['embed'] = True @@ -151,7 +181,7 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): 'never_convert_lossy_files': True, 'format': 'mp3', 'formats': { - 'mp3': 'cp $source $dest', + 'mp3': self.tagged_copy_cmd('mp3'), } } @@ -164,7 +194,7 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): with control_stdin('y'): self.run_command('convert', item.path) converted = os.path.join(self.convert_dest, 'converted.mp3') - self.assertTrue(os.path.isfile(converted)) + self.assertFileTag(converted, 'mp3') def test_transcode_from_lossy(self): self.config['convert']['never_convert_lossy_files'] = False @@ -172,14 +202,14 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper): with control_stdin('y'): self.run_command('convert', item.path) converted = os.path.join(self.convert_dest, 'converted.mp3') - self.assertTrue(os.path.isfile(converted)) + self.assertFileTag(converted, 'mp3') def test_transcode_from_lossy_prevented(self): [item] = self.add_item_fixtures(ext='ogg') with control_stdin('y'): self.run_command('convert', item.path) converted = os.path.join(self.convert_dest, 'converted.ogg') - self.assertTrue(os.path.isfile(converted)) + self.assertNoFileTag(converted, 'mp3') def suite(): From 3cbe9cbd105771bb7f8de8181d0de904500c0682 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 11 Sep 2014 20:03:21 +0200 Subject: [PATCH 38/51] Plugins can define types of flexible fields This partially solves #647. --- beets/plugins.py | 24 ++++++++++++++++++++++++ beets/ui/__init__.py | 2 ++ 2 files changed, 26 insertions(+) diff --git a/beets/plugins.py b/beets/plugins.py index 3dca22a97..eaaf9e9f7 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -31,6 +31,14 @@ LASTFM_KEY = '2dc3914abf35f0d9c92d97d8f8e42b43' log = logging.getLogger('beets') +class PluginConflictException(Exception): + """Indicates that the services provided by one plugin conflict with + those of another. + + For example two plugins may define different types for flexible fields. + """ + + # Managing the plugins themselves. class BeetsPlugin(object): @@ -247,6 +255,22 @@ def queries(): return out +def types(model_cls): + # Gives us `item_types` and `album_types` + attr_name = '{0}_types'.format(model_cls.__name__.lower()) + types = {} + for plugin in find_plugins(): + plugin_types = getattr(plugin, attr_name, {}) + for field in plugin_types: + if field in types: + raise PluginConflictException( + u'Plugin {0} defines flexible field {1} ' + 'which has already been defined.' + .format(plugin.name,)) + types.update(plugin_types) + return types + + def track_distance(item, info): """Gets the track distance calculated by all loaded plugins. Returns a Distance object. diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 7e3e7559c..c3bb7a158 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -869,6 +869,8 @@ def _setup(options, lib=None): if lib is None: lib = _open_library(config) plugins.send("library_opened", lib=lib) + library.Item._types = plugins.types(library.Item) + library.Album._types = plugins.types(library.Album) return subcommands, plugins, lib From 475d4899eec72090c9d861535ea43b84adfbef70 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 11 Sep 2014 21:02:35 +0200 Subject: [PATCH 39/51] Add tests for plugins providing flexible field types --- beets/plugins.py | 2 +- test/test_plugins.py | 68 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 test/test_plugins.py diff --git a/beets/plugins.py b/beets/plugins.py index eaaf9e9f7..2ee5f88f2 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -144,7 +144,7 @@ class BeetsPlugin(object): >>> @MyPlugin.listen("imported") >>> def importListener(**kwargs): - >>> pass + ... pass """ def helper(func): if cls.listeners is None: diff --git a/test/test_plugins.py b/test/test_plugins.py new file mode 100644 index 000000000..a7590432f --- /dev/null +++ b/test/test_plugins.py @@ -0,0 +1,68 @@ +# This file is part of beets. +# Copyright 2014, Thomas Scholtes. +# +# 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. + +from mock import patch +from _common import unittest +from helper import TestHelper + +from beets import plugins +from beets.library import Item +from beets.dbcore import types + + +class PluginTest(unittest.TestCase, TestHelper): + + def setUp(self): + # FIXME the mocking code is horrific, but this is the lowest and + # earliest level of the plugin mechanism we can hook into. + self._plugin_loader_patch = patch('beets.plugins.load_plugins') + self._plugin_classes = set() + load_plugins = self._plugin_loader_patch.start() + + def myload(names=()): + plugins._classes.update(self._plugin_classes) + load_plugins.side_effect = myload + self.setup_beets() + + def tearDown(self): + self._plugin_loader_patch.stop() + self.unload_plugins() + self.teardown_beets() + + def test_flex_field_type(self): + class RatingPlugin(plugins.BeetsPlugin): + item_types = {'rating': types.Float()} + + self.register_plugin(RatingPlugin) + self.config['plugins'] = 'rating' + + item = Item(path='apath', artist='aaa') + item.add(self.lib) + + # Do not match unset values + out = self.run_with_output('ls', 'rating:1..3') + self.assertNotIn('aaa', out) + + self.run_command('modify', 'rating=2', '--yes') + + # Match in range + out = self.run_with_output('ls', 'rating:1..3') + self.assertIn('aaa', out) + + # Don't match out of range + out = self.run_with_output('ls', 'rating:3..5') + self.assertNotIn('aaa', out) + + def register_plugin(self, plugin_class): + self._plugin_classes.add(plugin_class) From f112c9610c87abb523e4570ebcd288af8368d854 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 11 Sep 2014 23:48:17 +0200 Subject: [PATCH 40/51] Add 'types' plugin for flexible field types Conflicts: beets/library.py --- beets/library.py | 2 + beetsplug/types.py | 42 +++++++++++++ test/test_types_plugin.py | 126 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 170 insertions(+) create mode 100644 beetsplug/types.py create mode 100644 test/test_types_plugin.py diff --git a/beets/library.py b/beets/library.py index f053728b1..3a6829176 100644 --- a/beets/library.py +++ b/beets/library.py @@ -62,6 +62,8 @@ class PathQuery(dbcore.FieldQuery): class DateType(types.Type): + # TODO representation should be `datetime` object + # TODO distinguish beetween date and time types sql = u'REAL' query = dbcore.query.DateQuery null = 0.0 diff --git a/beetsplug/types.py b/beetsplug/types.py new file mode 100644 index 000000000..68aea35c7 --- /dev/null +++ b/beetsplug/types.py @@ -0,0 +1,42 @@ +# This file is part of beets. +# Copyright 2014, Thomas Scholtes. +# +# 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. + +from beets.plugins import BeetsPlugin +from beets.dbcore import types +from beets.util.confit import ConfigValueError +from beets import library + + +class TypesPlugin(BeetsPlugin): + + @property + def item_types(self): + if not self.config.exists(): + return {} + + mytypes = {} + for key, value in self.config.items(): + if value.get() == 'int': + mytypes[key] = types.INTEGER + elif value.get() == 'float': + mytypes[key] = types.FLOAT + elif value.get() == 'bool': + mytypes[key] = types.BOOLEAN + elif value.get() == 'date': + mytypes[key] = library.DateType() + else: + raise ConfigValueError( + u"unknown type '{0}' for the '{1}' field" + .format(value, key)) + return mytypes diff --git a/test/test_types_plugin.py b/test/test_types_plugin.py new file mode 100644 index 000000000..c7f1f4383 --- /dev/null +++ b/test/test_types_plugin.py @@ -0,0 +1,126 @@ +# This file is part of beets. +# Copyright 2014, Thomas Scholtes. +# +# 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. + +import time +from datetime import datetime + +from _common import unittest +from helper import TestHelper + +from beets.util.confit import ConfigValueError + + +class TypesPluginTest(unittest.TestCase, TestHelper): + + def setUp(self): + self.setup_beets() + self.load_plugins('types') + + def tearDown(self): + self.unload_plugins() + self.teardown_beets() + + def test_integer_modify_and_query(self): + self.config['types'] = {'myint': 'int'} + item = self.add_item(artist='aaa') + + # Do not match unset values + out = self.list('myint:1..3') + self.assertEqual('', out) + + self.modify('myint=2') + item.load() + self.assertEqual(item['myint'], 2) + + # Match in range + out = self.list('myint:1..3') + self.assertIn('aaa', out) + + def test_float_modify_and_query(self): + self.config['types'] = {'myfloat': 'float'} + item = self.add_item(artist='aaa') + + self.modify('myfloat=-9.1') + item.load() + self.assertEqual(item['myfloat'], -9.1) + + # Match in range + out = self.list('myfloat:-10..0') + self.assertIn('aaa', out) + + def test_bool_modify_and_query(self): + self.config['types'] = {'mybool': 'bool'} + true = self.add_item(artist='true') + false = self.add_item(artist='false') + self.add_item(artist='unset') + + # Set true + self.modify('mybool=1', 'artist:true') + true.load() + self.assertEqual(true['mybool'], True) + + # Set false + self.modify('mybool=false', 'artist:false') + false.load() + self.assertEqual(false['mybool'], False) + + # Query bools + out = self.list('mybool:true', '$artist $mybool') + self.assertEqual('true True', out) + + out = self.list('mybool:false', '$artist $mybool') + # TODO this should not match the `unset` item + # self.assertEqual('false False', out) + + out = self.list('mybool:', '$artist $mybool') + self.assertIn('unset $mybool', out) + + def test_date_modify_and_query(self): + self.config['types'] = {'mydate': 'date'} + # FIXME parsing should also work with default time format + self.config['time_format'] = '%Y-%m-%d' + old = self.add_item(artist='prince') + new = self.add_item(artist='britney') + + self.modify('mydate=1999-01-01', 'artist:prince') + old.load() + self.assertEqual(old['mydate'], mktime(1999, 01, 01)) + + self.modify('mydate=1999-12-30', 'artist:britney') + new.load() + self.assertEqual(new['mydate'], mktime(1999, 12, 30)) + + # Match in range + out = self.list('mydate:..1999-07', '$artist $mydate') + self.assertEqual('prince 1999-01-01', out) + + # FIXME + self.skipTest('there is a timezone issue here') + out = self.list('mydate:1999-12-30', '$artist $mydate') + self.assertEqual('britney 1999-12-30', out) + + def test_unknown_type_error(self): + self.config['types'] = {'flex': 'unkown type'} + with self.assertRaises(ConfigValueError): + self.run_command('ls') + + def modify(self, *args): + return self.run_with_output('modify', '--yes', '--nowrite', *args) + + def list(self, query, fmt='$artist - $album - $title'): + return self.run_with_output('ls', '-f', fmt, query).strip() + + +def mktime(*args): + return time.mktime(datetime(*args).timetuple()) From aa24fa7c1b5eb4c24310d1412d664eed2c3a5f6e Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Fri, 12 Sep 2014 00:42:43 +0200 Subject: [PATCH 41/51] Fix tests on python2.6 --- test/test_plugins.py | 7 +++++++ test/test_types_plugin.py | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/test/test_plugins.py b/test/test_plugins.py index a7590432f..a48dd10ae 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -66,3 +66,10 @@ class PluginTest(unittest.TestCase, TestHelper): def register_plugin(self, plugin_class): self._plugin_classes.add(plugin_class) + + +def suite(): + return unittest.TestLoader().loadTestsFromName(__name__) + +if __name__ == '__main__': + unittest.main(defaultTest='suite') diff --git a/test/test_types_plugin.py b/test/test_types_plugin.py index c7f1f4383..d4d9a96ef 100644 --- a/test/test_types_plugin.py +++ b/test/test_types_plugin.py @@ -124,3 +124,10 @@ class TypesPluginTest(unittest.TestCase, TestHelper): def mktime(*args): return time.mktime(datetime(*args).timetuple()) + + +def suite(): + return unittest.TestLoader().loadTestsFromName(__name__) + +if __name__ == '__main__': + unittest.main(defaultTest='suite') From d4f72f62eb0399dc73f362900aa46cd7e44dd079 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Fri, 12 Sep 2014 12:12:09 +0200 Subject: [PATCH 42/51] echonest: set types for flexible fields Conflicts: beetsplug/echonest.py --- beetsplug/echonest.py | 18 +++++++++++++++++- test/helper.py | 8 +++++++- test/test_echonest.py | 10 +++++++++- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/beetsplug/echonest.py b/beetsplug/echonest.py index e183eb8ef..e31a8c425 100644 --- a/beetsplug/echonest.py +++ b/beetsplug/echonest.py @@ -23,6 +23,7 @@ from string import Template import subprocess from beets import util, config, plugins, ui +from beets.dbcore import types import pyechonest import pyechonest.song import pyechonest.track @@ -38,7 +39,9 @@ DEVNULL = open(os.devnull, 'wb') ALLOWED_FORMATS = ('MP3', 'OGG', 'AAC') UPLOAD_MAX_SIZE = 50 * 1024 * 1024 -# The attributes we can import and where to store them in beets fields. +# Maps attribute names from echonest to their field names in beets. +# The attributes are retrieved from a songs `audio_summary`. See: +# http://echonest.github.io/pyechonest/song.html#pyechonest.song.profile ATTRIBUTES = { 'energy': 'energy', 'liveness': 'liveness', @@ -49,6 +52,16 @@ ATTRIBUTES = { 'tempo': 'bpm', } +# Types for the flexible fields added by `ATTRIBUTES` +FIELD_TYPES = { + 'energy': types.FLOAT, + 'liveness': types.FLOAT, + 'speechiness': types.FLOAT, + 'acousticness': types.FLOAT, + 'danceability': types.FLOAT, + 'valence': types.FLOAT, +} + MUSICAL_SCALE = ['C', 'C#', 'D', 'D#', 'E' 'F', 'F#', 'G', 'G#', 'A', 'A#', 'B'] @@ -104,6 +117,9 @@ def similar(lib, src_item, threshold=0.15, fmt='${difference}: ${path}'): class EchonestMetadataPlugin(plugins.BeetsPlugin): + + item_types = FIELD_TYPES + def __init__(self): super(EchonestMetadataPlugin, self).__init__() self.config.add({ diff --git a/test/helper.py b/test/helper.py index 0688708fd..f706871a7 100644 --- a/test/helper.py +++ b/test/helper.py @@ -43,7 +43,7 @@ from enum import Enum import beets from beets import config import beets.plugins -from beets.library import Library, Item +from beets.library import Library, Item, Album from beets import importer from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.mediafile import MediaFile @@ -168,18 +168,24 @@ class TestHelper(object): Similar setting a list of plugins in the configuration. Make sure you call ``unload_plugins()`` afterwards. """ + # FIXME this should eventually be handled by a plugin manager beets.config['plugins'] = plugins beets.plugins.load_plugins(plugins) beets.plugins.find_plugins() + Item._types = beets.plugins.types(Item) + Album._types = beets.plugins.types(Album) def unload_plugins(self): """Unload all plugins and remove the from the configuration. """ + # FIXME this should eventually be handled by a plugin manager beets.config['plugins'] = [] for plugin in beets.plugins._classes: plugin.listeners = None beets.plugins._classes = set() beets.plugins._instances = {} + Item._types = {} + Album._types = {} def create_importer(self, item_count=1, album_count=1): """Create files to import and return corresponding session. diff --git a/test/test_echonest.py b/test/test_echonest.py index da05adde3..43f006dfc 100644 --- a/test/test_echonest.py +++ b/test/test_echonest.py @@ -70,9 +70,17 @@ class EchonestCliTest(unittest.TestCase, TestHelper): self.run_command('echonest') item.load() - self.assertEqual(item['danceability'], '0.5') + self.assertEqual(item['danceability'], 0.5) + self.assertEqual(item['liveness'], 0.5) + self.assertEqual(item['bpm'], 120) self.assertEqual(item['initial_key'], 'C#m') + def test_custom_field_range_query(self): + item = Item(liveness=2.2) + item.add(self.lib) + item = self.lib.items('liveness:2.2..3').get() + self.assertEqual(item['liveness'], 2.2) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 5dec867ab31571b2ea6ee23f7e94978525ffecd4 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Fri, 12 Sep 2014 12:17:24 +0200 Subject: [PATCH 43/51] mpdstats: set types for flexible fields --- beetsplug/mpdstats.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/beetsplug/mpdstats.py b/beetsplug/mpdstats.py index 56522a8de..f03e284e3 100644 --- a/beetsplug/mpdstats.py +++ b/beetsplug/mpdstats.py @@ -25,6 +25,7 @@ from beets import config from beets import plugins from beets import library from beets.util import displayable_path +from beets.dbcore import types log = logging.getLogger('beets') @@ -308,6 +309,14 @@ class MPDStats(object): class MPDStatsPlugin(plugins.BeetsPlugin): + + item_types = { + 'play_count': types.INTEGER, + 'skip_count': types.INTEGER, + 'last_played': library.Date(), + 'rating': types.FLOAT, + } + def __init__(self): super(MPDStatsPlugin, self).__init__() self.config.add({ From 044dbfcd38c3996b41e3c13c21185cd9c8b4c955 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 16:15:00 -0700 Subject: [PATCH 44/51] NumericQuery: Check that the field exists --- beets/dbcore/query.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 7197d2564..1f1a9a26a 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -208,7 +208,9 @@ class NumericQuery(FieldQuery): self.rangemax = self._convert(parts[1]) def match(self, item): - value = getattr(item, self.field) + if self.field not in item: + return False + value = item[self.field] if isinstance(value, basestring): value = self._convert(value) From 2314f0f9ff731e41498c12266ae3be3868346c7f Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 16:28:23 -0700 Subject: [PATCH 45/51] Use NONE type affinity for flexattr value column This is what we should have been using all along---since it allows any type to appear---but we didn't. :cry: --- beets/dbcore/db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 2063cff72..0ec24dfd6 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -737,7 +737,7 @@ class Database(object): id INTEGER PRIMARY KEY, entity_id INTEGER, key TEXT, - value TEXT, + value NONE, UNIQUE(entity_id, key) ON CONFLICT REPLACE); CREATE INDEX IF NOT EXISTS {0}_by_entity ON {0} (entity_id); From d081b6a220668106e4cb333d6feba94319282b0b Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 12 Sep 2014 16:51:23 -0700 Subject: [PATCH 46/51] Docs for types plugin --- docs/plugins/index.rst | 2 ++ docs/plugins/types.rst | 17 +++++++++++++++++ test/test_types_plugin.py | 15 +++++++-------- 3 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 docs/plugins/types.rst diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index cd1ea868e..b66d801ea 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -62,6 +62,7 @@ by typing ``beet version``. importadded bpm spotify + types Autotagger Extensions --------------------- @@ -137,6 +138,7 @@ Miscellaneous * :doc:`missing`: List missing tracks. * :doc:`duplicates`: List duplicate tracks or albums. * :doc:`spotify`: Create Spotify playlists from the Beets library. +* :doc:`types`: Declare types for flexible attributes. .. _MPD: http://www.musicpd.org/ .. _MPD clients: http://mpd.wikia.com/wiki/Clients diff --git a/docs/plugins/types.rst b/docs/plugins/types.rst new file mode 100644 index 000000000..41419d758 --- /dev/null +++ b/docs/plugins/types.rst @@ -0,0 +1,17 @@ +Types Plugin +============ + +The ``types`` plugin lets you declare types for attributes you use in your +library. For example, you can declare that a ``rating`` field is numeric so +that you can query it with ranges---which isn't possible when the field is +considered a string, which is the default. + +Enable the plugin as described in :doc:`/plugins/index` and then add a +``types`` section to your :doc:`configuration file `. The +configuration section should map field name to one of ``int``, ``float``, +``bool``, or ``date``. + +Here's an example: + + types: + rating: int diff --git a/test/test_types_plugin.py b/test/test_types_plugin.py index d4d9a96ef..ef7ac7aa9 100644 --- a/test/test_types_plugin.py +++ b/test/test_types_plugin.py @@ -80,11 +80,11 @@ class TypesPluginTest(unittest.TestCase, TestHelper): self.assertEqual('true True', out) out = self.list('mybool:false', '$artist $mybool') - # TODO this should not match the `unset` item - # self.assertEqual('false False', out) - out = self.list('mybool:', '$artist $mybool') - self.assertIn('unset $mybool', out) + # Dealing with unset fields? + # self.assertEqual('false False', out) + # out = self.list('mybool:', '$artist $mybool') + # self.assertIn('unset $mybool', out) def test_date_modify_and_query(self): self.config['types'] = {'mydate': 'date'} @@ -105,10 +105,9 @@ class TypesPluginTest(unittest.TestCase, TestHelper): out = self.list('mydate:..1999-07', '$artist $mydate') self.assertEqual('prince 1999-01-01', out) - # FIXME - self.skipTest('there is a timezone issue here') - out = self.list('mydate:1999-12-30', '$artist $mydate') - self.assertEqual('britney 1999-12-30', out) + # FIXME some sort of timezone issue here + # out = self.list('mydate:1999-12-30', '$artist $mydate') + # self.assertEqual('britney 1999-12-30', out) def test_unknown_type_error(self): self.config['types'] = {'flex': 'unkown type'} From 80f3ec1ed74af137bf75e120e86a4875a32f85c4 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 13:15:47 +0200 Subject: [PATCH 47/51] Document flexible field types in plugins --- docs/dev/plugins.rst | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index ffb2d0721..c585c9001 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -397,3 +397,37 @@ plugin will be used if we issue a command like ``beet ls @something`` or return { '@': ExactMatchQuery } + + +Flexible Field Types +^^^^^^^^^^^^^^^^^^^^ + +If your plugin uses flexible fields to store numbers or other +non-string values you can specify the types of those fields. A rating +plugin, for example might look like this. :: + + from beets.plugins import BeetsPlugin + from beets.dbcore import types + + class RatingPlugin(BeetsPlugin): + item_types = {'rating': types.INTEGER} + + @property + def album_types(self): + return {'rating': types.INTEGER} + +A plugin may define two attributes, `item_types` and `album_types`. +Each of those attributes is a dictionary mapping a flexible field name +to a type instance. You can find the built-in types in the +`beets.dbcore.types` and `beets.library` modules or implement your own +ones. + +Specifying types has the following advantages. + +* The flexible field accessors ``item['my_field']`` return the + specified type instead of a string. + +* Users can use advanced queries (like :ref:`ranges `) + from the command line. + +* User input for flexible fields may be validated. From bd871cbc03fec355c67df40a0ab32161a2021008 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 14:00:34 +0200 Subject: [PATCH 48/51] =?UTF-8?q?Don=E2=80=99t=20interact=20with=20files?= =?UTF-8?q?=20in=20TypePluginTest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/test_types_plugin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_types_plugin.py b/test/test_types_plugin.py index ef7ac7aa9..5e34db9e2 100644 --- a/test/test_types_plugin.py +++ b/test/test_types_plugin.py @@ -115,7 +115,8 @@ class TypesPluginTest(unittest.TestCase, TestHelper): self.run_command('ls') def modify(self, *args): - return self.run_with_output('modify', '--yes', '--nowrite', *args) + return self.run_with_output('modify', '--yes', '--nowrite', + '--nomove', *args) def list(self, query, fmt='$artist - $album - $title'): return self.run_with_output('ls', '-f', fmt, query).strip() From 3fe52a7694d6eb70585fc1db0f6c12498e6da2ed Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 14:16:34 +0200 Subject: [PATCH 49/51] Add test helpers to create database fixtures --- test/helper.py | 54 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/test/helper.py b/test/helper.py index f706871a7..6beac5d5a 100644 --- a/test/helper.py +++ b/test/helper.py @@ -238,32 +238,59 @@ class TestHelper(object): # Library fixtures methods - def add_item(self, **values_): - """Add an item to the library and return it. + def create_item(self, **values): + """Return an `Item` instance with sensible default values. - The item receives sensible default values for the title, artist, - and album fields. These default values contain unicode - characters to test for encoding issues. The track title also - includes a counter to make sure we do not create items with the - same attributes. + The item receives its attributes from `**values` paratmeter. The + `title`, `artist`, `album`, `track`, `format` and `path` + attributes have defaults if they are not given as parameters. + The `title` attribute is formated with a running item count to + prevent duplicates. The default for the `path` attribute + respects the `format` value. + + The item is attached to the database from `self.lib`. """ item_count = self._get_item_count() - values = { + values_ = { 'title': u't\u00eftle {0}', 'artist': u'the \u00e4rtist', 'album': u'the \u00e4lbum', 'track': item_count, - 'path': 'audio.mp3', + 'format': 'MP3', } - values.update(values_) - values['title'] = values['title'].format(item_count) - item = Item(**values) + values_.update(values) + values_['title'] = values_['title'].format(item_count) + values_['db'] = self.lib + item = Item(**values_) + if 'path' not in values: + item['path'] = 'audio.' + item['format'].lower() + return item + + def add_item(self, **values): + """Add an item to the library and return it. + + Creates the item by passing the parameters to `create_item()`. + + If `path` is not set in `values` it is set to `item.destination()`. + """ + item = self.create_item(**values) item.add(self.lib) - if 'path' not in values_: + if 'path' not in values: item['path'] = item.destination() item.store() return item + def add_item_fixture(self, **values): + """Add an item with an actual audio file to the library. + """ + item = self.create_item(**values) + extension = item['format'].lower() + item['path'] = os.path.join(_common.RSRC, 'min.' + extension) + item.add(self.lib) + item.move(copy=True) + item.store() + return item + def add_album(self, **values): item = self.add_item(**values) return self.lib.add_album([item]) @@ -271,6 +298,7 @@ class TestHelper(object): def add_item_fixtures(self, ext='mp3', count=1): """Add a number of items with files to the database. """ + # TODO base this on `add_item()` items = [] path = os.path.join(_common.RSRC, 'full.' + ext) for i in range(count): From b86669fe05cf0c78f27d85edf2c88da9d0dd4e16 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 16:11:13 +0200 Subject: [PATCH 50/51] Add integer query tests --- test/test_query.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/test_query.py b/test/test_query.py index 4d85696d4..0cf53c92b 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -16,8 +16,12 @@ """ import _common from _common import unittest +from helper import TestHelper + import beets.library from beets import dbcore +from beets.dbcore import types +from beets.library import Library, Item class AnyFieldQueryTest(_common.LibTestCase): @@ -374,6 +378,42 @@ class PathQueryTest(_common.LibTestCase, AssertsMixin): self.assert_matched(results, ['path item']) +class IntQueryTest(unittest.TestCase, TestHelper): + + def setUp(self): + self.lib = Library(':memory:') + + def test_exact_value_match(self): + item = self.add_item(bpm=120) + matched = self.lib.items('bpm:120').get() + self.assertEqual(item.id, matched.id) + + def test_range_match(self): + item = self.add_item(bpm=120) + self.add_item(bpm=130) + + matched = self.lib.items('bpm:110..125') + self.assertEqual(1, len(matched)) + self.assertEqual(item.id, matched.get().id) + + def test_flex_range_match(self): + Item._types = {'myint': types.Integer()} + item = self.add_item(myint=2) + matched = self.lib.items('myint:2').get() + self.assertEqual(item.id, matched.id) + + def test_flex_dont_match_missing(self): + Item._types = {'myint': types.Integer()} + self.add_item() + matched = self.lib.items('myint:2').get() + self.assertIsNone(matched) + + def test_no_substring_match(self): + self.add_item(bpm=120) + matched = self.lib.items('bpm:12').get() + self.assertIsNone(matched) + + class DefaultSearchFieldsTest(DummyDataTestCase): def test_albums_matches_album(self): albums = list(self.lib.albums('baz')) From b1a5189f6851a7d59db683f054ca3663c8eb6161 Mon Sep 17 00:00:00 2001 From: Lucas Duailibe Date: Sun, 14 Sep 2014 12:07:08 -0300 Subject: [PATCH 51/51] Add default value for `relative_to` in play plugin --- beetsplug/play.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beetsplug/play.py b/beetsplug/play.py index 9a1bc444c..fb4167124 100644 --- a/beetsplug/play.py +++ b/beetsplug/play.py @@ -113,7 +113,8 @@ class PlayPlugin(BeetsPlugin): config['play'].add({ 'command': None, - 'use_folders': False + 'use_folders': False, + 'relative_to': None, }) def commands(self):