From c8309cbe570a50d4706b7b9ce69c3b0e20c658d0 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 11:11:16 +0100 Subject: [PATCH 01/10] Simplify BeetsPlugin.listen() Deduplicate code with BeetsPlugin.register_listener(). --- beets/plugins.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index ff89c856b..d6cb6469e 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -202,9 +202,7 @@ class BeetsPlugin(object): ... pass """ def helper(func): - if cls.listeners is None: - cls.listeners = defaultdict(list) - cls.listeners[event].append(func) + cls.register_listener(event, func) return func return helper From 2e57d8660e47a5897dc1717cfeec4432090d9f75 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 11:29:42 +0100 Subject: [PATCH 02/10] Ensure there's no duplicate plugin listeners A same function cannot be registered twice on a given event. This will permit improvements of some plugins (e.g. smartplaylist) --- beets/plugins.py | 3 ++- test/test_plugins.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/beets/plugins.py b/beets/plugins.py index d6cb6469e..193a209ab 100755 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -185,7 +185,8 @@ class BeetsPlugin(object): """ if cls.listeners is None: cls.listeners = defaultdict(list) - cls.listeners[event].append(func) + if func not in cls.listeners[event]: + cls.listeners[event].append(func) @classmethod def listen(cls, event): diff --git a/test/test_plugins.py b/test/test_plugins.py index 0880e2d27..256fc2ff4 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -161,6 +161,41 @@ class HelpersTest(unittest.TestCase): ('A', 'B', 'C', 'D')), ['D', 'B', 'C', 'A']) +class ListenersTest(unittest.TestCase, TestHelper): + def setUp(self): + self.setup_plugin_loader() + + def tearDown(self): + self.teardown_plugin_loader() + self.teardown_beets() + + def test_register(self): + + class DummyPlugin(plugins.BeetsPlugin): + def __init__(self): + super(DummyPlugin, self).__init__() + self.register_listener('cli_exit', self.dummy) + self.register_listener('cli_exit', self.dummy) + + def dummy(self): + pass + + d = DummyPlugin() + self.assertEqual(DummyPlugin.listeners['cli_exit'], [d.dummy]) + + d2 = DummyPlugin() + DummyPlugin.register_listener('cli_exit', d.dummy) + self.assertEqual(DummyPlugin.listeners['cli_exit'], + [d.dummy, d2.dummy]) + + @DummyPlugin.listen('cli_exit') + def dummy(lib): + pass + + self.assertEqual(DummyPlugin.listeners['cli_exit'], + [d.dummy, d2.dummy, dummy]) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From fdb768c9dbfca33a013b9588e0dadc8c68abc992 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 11:31:42 +0100 Subject: [PATCH 03/10] Simplify smartplaylist flow Suppress the global variable, register listeners if it's needed only. --- beetsplug/smartplaylist.py | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 368d516cc..d299e176c 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -21,10 +21,6 @@ from beets import config, ui, library from beets.util import normpath, syspath import os -# Global variable so that smartplaylist can detect database changes and run -# only once before beets exits. -database_changed = False - def _items_for_query(lib, playlist, album=False): """Get the matching items for a playlist's configured queries. @@ -97,6 +93,9 @@ class SmartPlaylistPlugin(BeetsPlugin): 'playlists': [] }) + if self.config['auto']: + self.register_listener('database_change', self.db_change) + def commands(self): def update(lib, opts, args): update_playlists(lib) @@ -105,15 +104,8 @@ class SmartPlaylistPlugin(BeetsPlugin): spl_update.func = update return [spl_update] + def db_change(self, lib): + self.register_listener('cli_exit', self.update) -@SmartPlaylistPlugin.listen('database_change') -def handle_change(lib): - global database_changed - database_changed = True - - -@SmartPlaylistPlugin.listen('cli_exit') -def update(lib): - auto = config['smartplaylist']['auto'] - if database_changed and auto: + def update(self, lib): update_playlists(lib) From 7c4496c110e7df8901fb756ef53dfc59943f8873 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 11:42:23 +0100 Subject: [PATCH 04/10] Smartplaylist: log messages instead of printing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ui.print_ → self._log.info Also change config['smartplaylist'] into self.config --- beetsplug/smartplaylist.py | 76 +++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index d299e176c..d0d381090 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -48,41 +48,6 @@ def _items_for_query(lib, playlist, album=False): return results -def update_playlists(lib): - ui.print_("Updating smart playlists...") - playlists = config['smartplaylist']['playlists'].get(list) - playlist_dir = config['smartplaylist']['playlist_dir'].as_filename() - relative_to = config['smartplaylist']['relative_to'].get() - if relative_to: - relative_to = normpath(relative_to) - - for playlist in playlists: - items = [] - items.extend(_items_for_query(lib, playlist, True)) - items.extend(_items_for_query(lib, playlist, False)) - - m3us = {} - basename = playlist['name'].encode('utf8') - # As we allow tags in the m3u names, we'll need to iterate through - # the items and generate the correct m3u file names. - for item in items: - m3u_name = item.evaluate_template(basename, True) - if not (m3u_name in m3us): - m3us[m3u_name] = [] - item_path = item.path - if relative_to: - item_path = os.path.relpath(item.path, relative_to) - if item_path not in m3us[m3u_name]: - m3us[m3u_name].append(item_path) - # Now iterate through the m3us that we need to generate - for m3u in m3us: - m3u_path = normpath(os.path.join(playlist_dir, m3u)) - with open(syspath(m3u_path), 'w') as f: - for path in m3us[m3u]: - f.write(path + '\n') - ui.print_("... Done") - - class SmartPlaylistPlugin(BeetsPlugin): def __init__(self): super(SmartPlaylistPlugin, self).__init__() @@ -98,14 +63,47 @@ class SmartPlaylistPlugin(BeetsPlugin): def commands(self): def update(lib, opts, args): - update_playlists(lib) + self.update_playlists(lib) spl_update = ui.Subcommand('splupdate', help='update the smart playlists') spl_update.func = update return [spl_update] def db_change(self, lib): - self.register_listener('cli_exit', self.update) + self.register_listener('cli_exit', self.update_playlists) + + def update_playlists(self, lib): + self._log.info("Updating smart playlists...") + playlists = self.config['playlists'].get(list) + playlist_dir = self.config['playlist_dir'].as_filename() + relative_to = self.config['relative_to'].get() + if relative_to: + relative_to = normpath(relative_to) + + for playlist in playlists: + items = [] + items.extend(_items_for_query(lib, playlist, True)) + items.extend(_items_for_query(lib, playlist, False)) + + m3us = {} + basename = playlist['name'].encode('utf8') + # As we allow tags in the m3u names, we'll need to iterate through + # the items and generate the correct m3u file names. + for item in items: + m3u_name = item.evaluate_template(basename, True) + if not (m3u_name in m3us): + m3us[m3u_name] = [] + item_path = item.path + if relative_to: + item_path = os.path.relpath(item.path, relative_to) + if item_path not in m3us[m3u_name]: + m3us[m3u_name].append(item_path) + # Now iterate through the m3us that we need to generate + for m3u in m3us: + m3u_path = normpath(os.path.join(playlist_dir, m3u)) + with open(syspath(m3u_path), 'w') as f: + for path in m3us[m3u]: + f.write(path + '\n') + self._log.info("... Done") + - def update(self, lib): - update_playlists(lib) From 82772966c8cb90863345ff9579c07a86bffaec21 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 11:53:39 +0100 Subject: [PATCH 05/10] Smartplaylist: fix incorrect doc With auto mode playlists are regenerated *at the end of the session, if a database update happened*, and not after a database update. --- docs/plugins/smartplaylist.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/plugins/smartplaylist.rst b/docs/plugins/smartplaylist.rst index 270e34def..bc39e581e 100644 --- a/docs/plugins/smartplaylist.rst +++ b/docs/plugins/smartplaylist.rst @@ -53,9 +53,9 @@ to albums that have a ``for_travel`` extensible field set to 1:: album_query: 'for_travel:1' query: 'for_travel:1' -By default, all playlists are automatically regenerated after every beets -command that changes the library database. To force regeneration, you can invoke it manually from the -command line:: +By default, all playlists are automatically regenerated at the end of the +session if the library database was changed. To force regeneration, you can +invoke it manually from the command line:: $ beet splupdate From a7beaa6d6e5b71dc33546381cbcea13d5b629525 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 12:11:52 +0100 Subject: [PATCH 06/10] Clean & shorten smartplaylist code - better log messages - more idiomatic code: "X not in Y" instead of "not (X in Y)" - shorten _items_for_query: - pre-detect whether it's album_query or query, hiding conf. spec to the function. - Let library.{items,album} parse the query string, therefore falling back to beets-level sort spec. if none is given in the query --- beetsplug/smartplaylist.py | 49 ++++++++++++++------------------------ 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index d0d381090..a7c83f0b8 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -15,37 +15,22 @@ """Generates smart playlists based on beets queries. """ from __future__ import print_function +from itertools import chain from beets.plugins import BeetsPlugin -from beets import config, ui, library +from beets import ui from beets.util import normpath, syspath import os -def _items_for_query(lib, playlist, album=False): - """Get the matching items for a playlist's configured queries. - `album` indicates whether to process the item-level query or the - album-level query (if any). - """ - key = 'album_query' if album else 'query' - if key not in playlist: - return [] - - # Parse quer(ies). If it's a list, perform the queries and manually - # concatenate the results - query_strings = playlist[key] - if not isinstance(query_strings, (list, tuple)): - query_strings = [query_strings] - model = library.Album if album else library.Item - results = [] - for q in query_strings: - query, sort = library.parse_query_string(q, model) - if album: - new = lib.albums(query, sort) - else: - new = lib.items(query, sort) - results.extend(new) - return results +def _items_for_query(lib, queries, album): + """Get the matching items for a query. + `album` indicates whether the queries are item-level or album-level""" + request = lib.albums if album else lib.items + if isinstance(queries, basestring): + return request(queries) + else: + return chain.from_iterable(map(request, queries)) class SmartPlaylistPlugin(BeetsPlugin): @@ -81,9 +66,13 @@ class SmartPlaylistPlugin(BeetsPlugin): relative_to = normpath(relative_to) for playlist in playlists: + self._log.debug(u"Creating playlist {0.name}", playlist) items = [] - items.extend(_items_for_query(lib, playlist, True)) - items.extend(_items_for_query(lib, playlist, False)) + if 'album_query' in playlist: + items.extend(_items_for_query(lib, playlist['album_query'], + True)) + if 'query' in playlist: + items.extend(_items_for_query(lib, playlist['query'], False)) m3us = {} basename = playlist['name'].encode('utf8') @@ -91,7 +80,7 @@ class SmartPlaylistPlugin(BeetsPlugin): # the items and generate the correct m3u file names. for item in items: m3u_name = item.evaluate_template(basename, True) - if not (m3u_name in m3us): + if m3u_name not in m3us: m3us[m3u_name] = [] item_path = item.path if relative_to: @@ -104,6 +93,4 @@ class SmartPlaylistPlugin(BeetsPlugin): with open(syspath(m3u_path), 'w') as f: for path in m3us[m3u]: f.write(path + '\n') - self._log.info("... Done") - - + self._log.info("{0} playlists updated", len(playlists)) From de86b9b57089f4772d929d065c5280cbee362c0a Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 12:21:45 +0100 Subject: [PATCH 07/10] Clean MPDupdate plugin - no global variable - use logging instead of prints - unicode logging --- beetsplug/mpdupdate.py | 87 ++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 49 deletions(-) diff --git a/beetsplug/mpdupdate.py b/beetsplug/mpdupdate.py index ff6e6dbe2..f8affed37 100644 --- a/beetsplug/mpdupdate.py +++ b/beetsplug/mpdupdate.py @@ -20,17 +20,11 @@ Put something like the following in your config.yaml to configure: port: 6600 password: seekrit """ -from __future__ import print_function - from beets.plugins import BeetsPlugin import os import socket from beets import config -# Global variable so that mpdupdate can detect database changes and run only -# once before beets exits. -database_changed = False - # No need to introduce a dependency on an MPD library for such a # simple use case. Here's a simple socket abstraction to make things @@ -66,37 +60,6 @@ class BufferedSocket(object): self.sock.close() -def update_mpd(host='localhost', port=6600, password=None): - """Sends the "update" command to the MPD server indicated, - possibly authenticating with a password first. - """ - print('Updating MPD database...') - - s = BufferedSocket(host, port) - resp = s.readline() - if 'OK MPD' not in resp: - print('MPD connection failed:', repr(resp)) - return - - if password: - s.send('password "%s"\n' % password) - resp = s.readline() - if 'OK' not in resp: - print('Authentication failed:', repr(resp)) - s.send('close\n') - s.close() - return - - s.send('update\n') - resp = s.readline() - if 'updating_db' not in resp: - print('Update failed:', repr(resp)) - - s.send('close\n') - s.close() - print('... updated.') - - class MPDUpdatePlugin(BeetsPlugin): def __init__(self): super(MPDUpdatePlugin, self).__init__() @@ -112,18 +75,44 @@ class MPDUpdatePlugin(BeetsPlugin): if self.config[key].exists(): config['mpd'][key] = self.config[key].get() + self.register_listener('database_change', self.db_change) -@MPDUpdatePlugin.listen('database_change') -def handle_change(lib=None): - global database_changed - database_changed = True + def db_change(self, lib): + self.register_listener('cli_exit', self.update) + def update(self, lib): + self.update_mpd( + config['mpd']['host'].get(unicode), + config['mpd']['port'].get(int), + config['mpd']['password'].get(unicode), + ) -@MPDUpdatePlugin.listen('cli_exit') -def update(lib=None): - if database_changed: - update_mpd( - config['mpd']['host'].get(unicode), - config['mpd']['port'].get(int), - config['mpd']['password'].get(unicode), - ) + def update_mpd(self, host='localhost', port=6600, password=None): + """Sends the "update" command to the MPD server indicated, + possibly authenticating with a password first. + """ + self._log.info('Updating MPD database...') + + s = BufferedSocket(host, port) + resp = s.readline() + if 'OK MPD' not in resp: + self._log.warning(u'MPD connection failed: {0!r}', resp) + return + + if password: + s.send('password "%s"\n' % password) + resp = s.readline() + if 'OK' not in resp: + self._log.warning(u'Authentication failed: {0!r}', resp) + s.send('close\n') + s.close() + return + + s.send('update\n') + resp = s.readline() + if 'updating_db' not in resp: + self._log.warning(u'Update failed: {0!r}', resp) + + s.send('close\n') + s.close() + self._log.info('Database updated.') From 753388550e4ea7a8b09ddb22189021be3585a5e5 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 12:35:54 +0100 Subject: [PATCH 08/10] Clean PlexUpdate plugin - no global variable - use logging instead of prints --- beetsplug/plexupdate.py | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/beetsplug/plexupdate.py b/beetsplug/plexupdate.py index c61766c43..f132cd4b3 100644 --- a/beetsplug/plexupdate.py +++ b/beetsplug/plexupdate.py @@ -12,11 +12,6 @@ from beets import config from beets.plugins import BeetsPlugin -# Global variable to detect if database is changed that the update -# is only run once before beets exists. -database_changed = False - - def get_music_section(host, port): """Getting the section key for the music library in Plex. """ @@ -55,30 +50,23 @@ class PlexUpdate(BeetsPlugin): u'host': u'localhost', u'port': 32400}) + self.register_listener('database_change', self.listen_for_db_change) -@PlexUpdate.listen('database_change') -def listen_for_db_change(lib=None): - """Listens for beets db change and set global database_changed - variable to True. - """ - global database_changed - database_changed = True + def listen_for_db_change(self, lib): + """Listens for beets db change and register the update for the end""" + self.register_listener('cli_exit', self.update) - -@PlexUpdate.listen('cli_exit') -def update(lib=None): - """When the client exists and the database_changed variable is True - trying to send refresh request to Plex server. - """ - if database_changed: - print('Updating Plex library...') + def update(self, lib): + """When the client exists try to send refresh request to Plex server. + """ + self._log.info('Updating Plex library...') # Try to send update request. try: update_plex( config['plex']['host'].get(), config['plex']['port'].get()) - print('... started.') + self._log.info('... started.') except requests.exceptions.RequestException: - print('Update failed.') + self._log.warning('Update failed.') From ca91bc8920915c9962fe4d33476eb4fd1f6229ba Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 18:53:30 +0100 Subject: [PATCH 09/10] mpdupdate: fix indent --- beetsplug/mpdupdate.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/beetsplug/mpdupdate.py b/beetsplug/mpdupdate.py index f8affed37..42ef55fa8 100644 --- a/beetsplug/mpdupdate.py +++ b/beetsplug/mpdupdate.py @@ -81,11 +81,11 @@ class MPDUpdatePlugin(BeetsPlugin): self.register_listener('cli_exit', self.update) def update(self, lib): - self.update_mpd( - config['mpd']['host'].get(unicode), - config['mpd']['port'].get(int), - config['mpd']['password'].get(unicode), - ) + self.update_mpd( + config['mpd']['host'].get(unicode), + config['mpd']['port'].get(int), + config['mpd']['password'].get(unicode), + ) def update_mpd(self, host='localhost', port=6600, password=None): """Sends the "update" command to the MPD server indicated, From 9a2a9b0144ebb19108118cce1ecfc8a5391fdba9 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 18:53:47 +0100 Subject: [PATCH 10/10] smartplaylist: fix docstring --- beetsplug/smartplaylist.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index a7c83f0b8..15cb46057 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -25,7 +25,8 @@ import os def _items_for_query(lib, queries, album): """Get the matching items for a query. - `album` indicates whether the queries are item-level or album-level""" + `album` indicates whether the queries are item-level or album-level. + """ request = lib.albums if album else lib.items if isinstance(queries, basestring): return request(queries)