From 0d7fea0a43aff849b73d33785ce050ff73386ad6 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 6 Dec 2013 19:15:59 +0100 Subject: [PATCH 01/23] ext: Convert commands to use new registry system. Creates a placeholder registry using the existing hooks, and updates the commands to use these. The actual registry still needs to be created. --- mopidy/__main__.py | 8 ++++++- mopidy/backends/local/commands.py | 15 +++++-------- mopidy/commands.py | 36 +++++++++++++------------------ 3 files changed, 27 insertions(+), 32 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 1aca9cf4..a2174899 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -108,10 +108,16 @@ def main(): args.extension.ext_name) return 1 + registry = {'backends': [], 'frontends': [], 'local:library': []} + for extension in enabled_extensions: + registry['backends'].extend(extension.get_backend_classes()) + registry['frontends'].extend(extension.get_frontend_classes()) + registry['local:library'].extend(extension.get_library_updaters()) + # Anything that wants to exit after this point must use # mopidy.utils.process.exit_process as actors can have been started. try: - return args.command.run(args, proxied_config, enabled_extensions) + return args.command.run(args, proxied_config, registry) except NotImplementedError: print root_cmd.format_help() return 1 diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index 5e9b42e6..de84760b 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -22,27 +22,22 @@ class LocalCommand(commands.Command): class ScanCommand(commands.Command): help = "Scan local media files and populate the local library." - def run(self, args, config, extensions): + def run(self, args, config, registry): media_dir = config['local']['media_dir'] scan_timeout = config['local']['scan_timeout'] excluded_file_extensions = set( ext.lower() for ext in config['local']['excluded_file_extensions']) - updaters = {} - for e in extensions: - for updater_class in e.get_library_updaters(): - if updater_class and 'local' in updater_class.uri_schemes: - updaters[e.ext_name] = updater_class - + # TODO: select updater / library to use by name + updaters = registry['local:library'] if not updaters: logger.error('No usable library updaters found.') return 1 elif len(updaters) > 1: logger.error('More than one library updater found. ' - 'Provided by: %s', ', '.join(updaters.keys())) + 'Provided by: %s', ', '.join(updaters)) return 1 - - local_updater = updaters.values()[0](config) + local_updater = updaters[0](config) uri_path_mapping = {} uris_in_library = set() diff --git a/mopidy/commands.py b/mopidy/commands.py index 851bfb83..35175e07 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -257,22 +257,22 @@ class RootCommand(Command): type=config_override_type, metavar='OPTIONS', help='`section/key=value` values to override config options') - def run(self, args, config, extensions): + def run(self, args, config, registry): loop = gobject.MainLoop() try: audio = self.start_audio(config) - backends = self.start_backends(config, extensions, audio) + backends = self.start_backends(config, registry, audio) core = self.start_core(audio, backends) - self.start_frontends(config, extensions, core) + self.start_frontends(config, registry, core) loop.run() except KeyboardInterrupt: logger.info('Interrupted. Exiting...') return finally: loop.quit() - self.stop_frontends(extensions) + self.stop_frontends(registry) self.stop_core() - self.stop_backends(extensions) + self.stop_backends(registry) self.stop_audio() process.stop_remaining_actors() @@ -280,10 +280,8 @@ class RootCommand(Command): logger.info('Starting Mopidy audio') return Audio.start(config=config).proxy() - def start_backends(self, config, extensions, audio): - backend_classes = [] - for extension in extensions: - backend_classes.extend(extension.get_backend_classes()) + def start_backends(self, config, registry, audio): + backend_classes = registry['backends'] logger.info( 'Starting Mopidy backends: %s', @@ -300,10 +298,8 @@ class RootCommand(Command): logger.info('Starting Mopidy core') return Core.start(audio=audio, backends=backends).proxy() - def start_frontends(self, config, extensions, core): - frontend_classes = [] - for extension in extensions: - frontend_classes.extend(extension.get_frontend_classes()) + def start_frontends(self, config, registry, core): + frontend_classes = registry['frontends'] logger.info( 'Starting Mopidy frontends: %s', @@ -312,21 +308,19 @@ class RootCommand(Command): for frontend_class in frontend_classes: frontend_class.start(config=config, core=core) - def stop_frontends(self, extensions): + def stop_frontends(self, registry): logger.info('Stopping Mopidy frontends') - for extension in extensions: - for frontend_class in extension.get_frontend_classes(): - process.stop_actors_by_class(frontend_class) + for frontend_class in registry['frontends']: + process.stop_actors_by_class(frontend_class) def stop_core(self): logger.info('Stopping Mopidy core') process.stop_actors_by_class(Core) - def stop_backends(self, extensions): + def stop_backends(self, registry): logger.info('Stopping Mopidy backends') - for extension in extensions: - for backend_class in extension.get_backend_classes(): - process.stop_actors_by_class(backend_class) + for backend_class in registry['backends']: + process.stop_actors_by_class(backend_class) def stop_audio(self): logger.info('Stopping Mopidy audio') From decce4ccf6598f99816745faed974a6a48e939bf Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 7 Dec 2013 01:00:55 +0100 Subject: [PATCH 02/23] ext: Add basic global registry and switch to Extension.setup() --- mopidy/__main__.py | 8 ++---- mopidy/backends/local/commands.py | 9 +++--- mopidy/commands.py | 32 ++++++++++----------- mopidy/ext.py | 48 +++++++++++++++++++++++-------- 4 files changed, 59 insertions(+), 38 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index a2174899..f571e0ff 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -84,7 +84,6 @@ def main(): enabled_extensions.append(extension) log_extension_info(installed_extensions, enabled_extensions) - ext.register_gstreamer_elements(enabled_extensions) # Config and deps commands are simply special cased for now. if args.command == config_cmd: @@ -108,16 +107,13 @@ def main(): args.extension.ext_name) return 1 - registry = {'backends': [], 'frontends': [], 'local:library': []} for extension in enabled_extensions: - registry['backends'].extend(extension.get_backend_classes()) - registry['frontends'].extend(extension.get_frontend_classes()) - registry['local:library'].extend(extension.get_library_updaters()) + extension.setup() # Anything that wants to exit after this point must use # mopidy.utils.process.exit_process as actors can have been started. try: - return args.command.run(args, proxied_config, registry) + return args.command.run(args, proxied_config) except NotImplementedError: print root_cmd.format_help() return 1 diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index de84760b..8a951929 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -4,7 +4,7 @@ import logging import os import time -from mopidy import commands, exceptions +from mopidy import commands, exceptions, ext from mopidy.audio import scan from mopidy.utils import path @@ -22,14 +22,15 @@ class LocalCommand(commands.Command): class ScanCommand(commands.Command): help = "Scan local media files and populate the local library." - def run(self, args, config, registry): + def run(self, args, config): media_dir = config['local']['media_dir'] scan_timeout = config['local']['scan_timeout'] + excluded_file_extensions = config['local']['excluded_file_extensions'] excluded_file_extensions = set( - ext.lower() for ext in config['local']['excluded_file_extensions']) + file_ext.lower() for file_ext in excluded_file_extensions) # TODO: select updater / library to use by name - updaters = registry['local:library'] + updaters = ext.registry['local:library'] if not updaters: logger.error('No usable library updaters found.') return 1 diff --git a/mopidy/commands.py b/mopidy/commands.py index 35175e07..372550a0 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -9,7 +9,7 @@ import sys import glib import gobject -from mopidy import config as config_lib +from mopidy import config as config_lib, ext from mopidy.audio import Audio from mopidy.core import Core from mopidy.utils import deps, process, versioning @@ -257,22 +257,26 @@ class RootCommand(Command): type=config_override_type, metavar='OPTIONS', help='`section/key=value` values to override config options') - def run(self, args, config, registry): + def run(self, args, config): loop = gobject.MainLoop() + + backend_classes = ext.registry['backend'] + frontend_classes = ext.registry['frontend'] + try: audio = self.start_audio(config) - backends = self.start_backends(config, registry, audio) + backends = self.start_backends(config, backend_classes, audio) core = self.start_core(audio, backends) - self.start_frontends(config, registry, core) + self.start_frontends(config, frontend_classes, core) loop.run() except KeyboardInterrupt: logger.info('Interrupted. Exiting...') return finally: loop.quit() - self.stop_frontends(registry) + self.stop_frontends(frontend_classes) self.stop_core() - self.stop_backends(registry) + self.stop_backends(backend_classes) self.stop_audio() process.stop_remaining_actors() @@ -280,9 +284,7 @@ class RootCommand(Command): logger.info('Starting Mopidy audio') return Audio.start(config=config).proxy() - def start_backends(self, config, registry, audio): - backend_classes = registry['backends'] - + def start_backends(self, config, backend_classes, audio): logger.info( 'Starting Mopidy backends: %s', ', '.join(b.__name__ for b in backend_classes) or 'none') @@ -298,9 +300,7 @@ class RootCommand(Command): logger.info('Starting Mopidy core') return Core.start(audio=audio, backends=backends).proxy() - def start_frontends(self, config, registry, core): - frontend_classes = registry['frontends'] - + def start_frontends(self, config, frontend_classes, core): logger.info( 'Starting Mopidy frontends: %s', ', '.join(f.__name__ for f in frontend_classes) or 'none') @@ -308,18 +308,18 @@ class RootCommand(Command): for frontend_class in frontend_classes: frontend_class.start(config=config, core=core) - def stop_frontends(self, registry): + def stop_frontends(self, frontend_classes): logger.info('Stopping Mopidy frontends') - for frontend_class in registry['frontends']: + for frontend_class in frontend_classes: process.stop_actors_by_class(frontend_class) def stop_core(self): logger.info('Stopping Mopidy core') process.stop_actors_by_class(Core) - def stop_backends(self, registry): + def stop_backends(self, backend_classes): logger.info('Stopping Mopidy backends') - for backend_class in registry['backends']: + for backend_class in backend_classes: process.stop_actors_by_class(backend_class) def stop_audio(self): diff --git a/mopidy/ext.py b/mopidy/ext.py index feadc99f..d1c81dc5 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import collections import logging import pkg_resources @@ -61,6 +62,18 @@ class Extension(object): """ pass + def setup(self): + for backend_class in self.get_backend_classes(): + registry.add('backend', backend_class) + + for frontend_class in self.get_frontend_classes(): + registry.add('frontend', frontend_class) + + for library_updater in self.get_library_updaters(): + registry.add('local:library', library_updater) + + self.register_gstreamer_elements() + def get_frontend_classes(self): """List of frontend actor classes @@ -112,6 +125,29 @@ class Extension(object): pass +class _Registry(collections.Mapping): + def __init__(self): + self._registry = collections.defaultdict(list) + + def add(self, name, cls): + self._registry[name].append(cls) + + def __getitem__(self, name): + return self._registry[name] + + def __iter__(self): + return iter(self._registry) + + def __len__(self): + return len(self._registry) + + +# TODO: document the registry +# TODO: consider if we should avoid having this as a global and pass an +# instance from __main__ around instead? +registry = _Registry() + + def load_extensions(): """Find all installed extensions. @@ -166,15 +202,3 @@ def validate_extension(extension): return False return True - - -def register_gstreamer_elements(enabled_extensions): - """Registers custom GStreamer elements from extensions. - - :param enabled_extensions: list of enabled extensions - """ - - for extension in enabled_extensions: - logger.debug( - 'Registering GStreamer elements for: %s', extension.ext_name) - extension.register_gstreamer_elements() From 353782e2c89eb88f325dbd3d5fbce48b7e445719 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 23 Dec 2013 21:34:21 +0100 Subject: [PATCH 03/23] local: Add local/data_dir config value. Not in use yet but, needed for future changes planed in this branch. --- docs/ext/local.rst | 5 +++++ mopidy/backends/local/__init__.py | 9 +-------- mopidy/backends/local/actor.py | 8 ++++++++ mopidy/backends/local/ext.conf | 1 + tests/backends/local/events_test.py | 1 + tests/backends/local/playback_test.py | 1 + tests/backends/local/playlists_test.py | 1 + tests/backends/local/tracklist_test.py | 1 + 8 files changed, 19 insertions(+), 8 deletions(-) diff --git a/docs/ext/local.rst b/docs/ext/local.rst index cbde826f..43996deb 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -39,6 +39,11 @@ Configuration values Path to directory with local media files. +.. confval:: local/data_dir + + Path to directory to store local metadata such as libraries and playlists + in. + .. confval:: local/playlists_dir Path to playlists directory with m3u files for local media. diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index dedc868c..5caa6826 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -5,7 +5,6 @@ import os import mopidy from mopidy import config, ext -from mopidy.utils import encoding, path logger = logging.getLogger('mopidy.backends.local') @@ -23,6 +22,7 @@ class Extension(ext.Extension): def get_config_schema(self): schema = super(Extension, self).get_config_schema() schema['media_dir'] = config.Path() + schema['data_dir'] = config.Path() schema['playlists_dir'] = config.Path() schema['tag_cache_file'] = config.Deprecated() schema['scan_timeout'] = config.Integer( @@ -30,13 +30,6 @@ class Extension(ext.Extension): schema['excluded_file_extensions'] = config.List(optional=True) return schema - def validate_environment(self): - try: - path.get_or_create_dir(b'$XDG_DATA_DIR/mopidy/local') - except EnvironmentError as error: - error = encoding.locale_decode(error) - logger.warning('Could not create local data dir: %s', error) - def get_backend_classes(self): from .actor import LocalBackend return [LocalBackend] diff --git a/mopidy/backends/local/actor.py b/mopidy/backends/local/actor.py index a73f627e..78caf39e 100644 --- a/mopidy/backends/local/actor.py +++ b/mopidy/backends/local/actor.py @@ -32,6 +32,14 @@ class LocalBackend(pykka.ThreadingActor, base.Backend): logger.warning('Local media dir %s does not exist.' % self.config['local']['media_dir']) + try: + path.get_or_create_dir(self.config['local']['data_dir']) + except EnvironmentError as error: + logger.warning( + 'Could not create local data dir: %s', + encoding.locale_decode(error)) + + # TODO: replace with data dir? try: path.get_or_create_dir(self.config['local']['playlists_dir']) except EnvironmentError as error: diff --git a/mopidy/backends/local/ext.conf b/mopidy/backends/local/ext.conf index f906a04f..e826a451 100644 --- a/mopidy/backends/local/ext.conf +++ b/mopidy/backends/local/ext.conf @@ -1,6 +1,7 @@ [local] enabled = true media_dir = $XDG_MUSIC_DIR +data_dir = $XDG_DATA_DIR/mopidy/local playlists_dir = $XDG_DATA_DIR/mopidy/local/playlists scan_timeout = 1000 excluded_file_extensions = diff --git a/tests/backends/local/events_test.py b/tests/backends/local/events_test.py index 1e26a68c..2424ed42 100644 --- a/tests/backends/local/events_test.py +++ b/tests/backends/local/events_test.py @@ -17,6 +17,7 @@ class LocalBackendEventsTest(unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), + 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', } } diff --git a/tests/backends/local/playback_test.py b/tests/backends/local/playback_test.py index 4c3dd70d..4da420ef 100644 --- a/tests/backends/local/playback_test.py +++ b/tests/backends/local/playback_test.py @@ -22,6 +22,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), + 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', } } diff --git a/tests/backends/local/playlists_test.py b/tests/backends/local/playlists_test.py index c02e1d23..447de3f8 100644 --- a/tests/backends/local/playlists_test.py +++ b/tests/backends/local/playlists_test.py @@ -20,6 +20,7 @@ class LocalPlaylistsProviderTest(unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), + 'data_dir': path_to_data_dir(''), } } diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index c7cfe51f..14bf678d 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -18,6 +18,7 @@ class LocalTracklistProviderTest(unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), + 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', } } From 8a94d81c42528d9a712be0a0b14649089c880378 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 23 Dec 2013 21:46:24 +0100 Subject: [PATCH 04/23] ext: Move away from global registry to ease testing. Extension's setup method are now passed the active registry allowing them to "steal" a list of the registry items they care about. In the case of commands the registry is passed via args.registry. --- mopidy/__main__.py | 6 ++++-- mopidy/backends/local/commands.py | 4 ++-- mopidy/commands.py | 6 +++--- mopidy/ext.py | 17 ++++++----------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index f571e0ff..f66ac6c1 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -40,11 +40,13 @@ def main(): signal.signal(signal.SIGUSR1, pykka.debug.log_thread_tracebacks) try: + registry = ext.Registry() + root_cmd = commands.RootCommand() config_cmd = commands.ConfigCommand() deps_cmd = commands.DepsCommand() - root_cmd.set(extension=None) + root_cmd.set(extension=None, registry=registry) root_cmd.add_child('config', config_cmd) root_cmd.add_child('deps', deps_cmd) @@ -108,7 +110,7 @@ def main(): return 1 for extension in enabled_extensions: - extension.setup() + extension.setup(registry) # Anything that wants to exit after this point must use # mopidy.utils.process.exit_process as actors can have been started. diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index 8a951929..2f6f744e 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -4,7 +4,7 @@ import logging import os import time -from mopidy import commands, exceptions, ext +from mopidy import commands, exceptions from mopidy.audio import scan from mopidy.utils import path @@ -30,7 +30,7 @@ class ScanCommand(commands.Command): file_ext.lower() for file_ext in excluded_file_extensions) # TODO: select updater / library to use by name - updaters = ext.registry['local:library'] + updaters = args.registry['local:library'] if not updaters: logger.error('No usable library updaters found.') return 1 diff --git a/mopidy/commands.py b/mopidy/commands.py index 372550a0..1bba63fa 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -9,7 +9,7 @@ import sys import glib import gobject -from mopidy import config as config_lib, ext +from mopidy import config as config_lib from mopidy.audio import Audio from mopidy.core import Core from mopidy.utils import deps, process, versioning @@ -260,8 +260,8 @@ class RootCommand(Command): def run(self, args, config): loop = gobject.MainLoop() - backend_classes = ext.registry['backend'] - frontend_classes = ext.registry['frontend'] + backend_classes = args.registry['backend'] + frontend_classes = args.registry['frontend'] try: audio = self.start_audio(config) diff --git a/mopidy/ext.py b/mopidy/ext.py index d1c81dc5..c79ebe40 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -62,7 +62,7 @@ class Extension(object): """ pass - def setup(self): + def setup(self, registry): for backend_class in self.get_backend_classes(): registry.add('backend', backend_class) @@ -125,15 +125,16 @@ class Extension(object): pass -class _Registry(collections.Mapping): +# TODO: document +class Registry(collections.Mapping): def __init__(self): - self._registry = collections.defaultdict(list) + self._registry = {} def add(self, name, cls): - self._registry[name].append(cls) + self._registry.setdefault(name, []).append(cls) def __getitem__(self, name): - return self._registry[name] + return self._registry.setdefault(name, []) def __iter__(self): return iter(self._registry) @@ -142,12 +143,6 @@ class _Registry(collections.Mapping): return len(self._registry) -# TODO: document the registry -# TODO: consider if we should avoid having this as a global and pass an -# instance from __main__ around instead? -registry = _Registry() - - def load_extensions(): """Find all installed extensions. From 4c0b54317bd06c2f8cfd33f9e588cbd1d54f8e0d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 23 Dec 2013 22:16:03 +0100 Subject: [PATCH 05/23] local: Add new library interface to local backend. This forms the basis of our plugable local libraries that we intend to ship. --- mopidy/backends/local/__init__.py | 75 +++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index 5caa6826..e0e917f7 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -37,3 +37,78 @@ class Extension(ext.Extension): def get_command(self): from .commands import LocalCommand return LocalCommand() + + +class Library(object): + #: Name of the local library implementation. + name = None + + def __init__(self, config): + self._config = config + + def load(self): + """ + Initialize whatever resources are needed for this library. + + This is where you load the tracks into memory, setup a database + conection etc. + + :rtype: :class:`int` representing number of tracks in library. + """ + return 0 + + def add(self, track): + """ + Add the given track to library. + + :param track: Track to add to the library/ + :type track: :class:`mopidy.models.Track` + """ + raise NotImplementedError + + def remove(self, uri): + """ + Remove the given track from the library. + + :param uri: URI to remove from the library/ + :type uri: string + """ + raise NotImplementedError + + def commit(self): + """ + Persist any changes to the library. + + This is where you write your data file to disk, commit transactions + etc. depending on the requirements of your library implementation. + """ + pass + + def lookup(self, uri): + """ + Lookup the given URI. + + If the URI expands to multiple tracks, the returned list will contain + them all. + + :param uri: track URI + :type uri: string + :rtype: list of :class:`mopidy.models.Track` + """ + raise NotImplementedError + + # TODO: support case with returning all tracks? + # TODO: remove uris? + def search(self, query=None, exact=False, uris=None): + """ + Search the library for tracks where ``field`` contains ``values``. + + :param query: one or more queries to search for + :type query: dict + :param exact: look for exact matches? + :type query: boolean + :param uris: zero or more URI roots to limit the search to + :type uris: list of strings or :class:`None` + :rtype: :class:`mopidy.models.SearchResult` + """ + raise NotImplementedError From 7e063774b39dcb5460524fca69f6151a37e458ff Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 23 Dec 2013 22:21:23 +0100 Subject: [PATCH 06/23] local: Remove local-json as a split out extension. Will be re-added using the new library interface. This commit does break tests. --- docs/ext/local.rst | 29 +------ mopidy/backends/local/json/__init__.py | 30 ------- mopidy/backends/local/json/actor.py | 30 ------- mopidy/backends/local/json/ext.conf | 3 - mopidy/backends/local/json/library.py | 108 ------------------------- setup.py | 1 - 6 files changed, 1 insertion(+), 200 deletions(-) delete mode 100644 mopidy/backends/local/json/__init__.py delete mode 100644 mopidy/backends/local/json/actor.py delete mode 100644 mopidy/backends/local/json/ext.conf delete mode 100644 mopidy/backends/local/json/library.py diff --git a/docs/ext/local.rst b/docs/ext/local.rst index 43996deb..eed51829 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -95,34 +95,7 @@ Pluggable library support ------------------------- Local libraries are fully pluggable. What this means is that users may opt to -disable the current default library ``local-json``, replacing it with a third +disable the current default library ``json``, replacing it with a third party one. When running :command:`mopidy local scan` mopidy will populate whatever the current active library is with data. Only one library may be active at a time. - - -***************** -Mopidy-Local-JSON -***************** - -Extension for storing local music library in a JSON file, default built in -library for local files. - - -Default configuration -===================== - -.. literalinclude:: ../../mopidy/backends/local/json/ext.conf - :language: ini - - -Configuration values -==================== - -.. confval:: local-json/enabled - - If the local-json extension should be enabled or not. - -.. confval:: local-json/json_file - - Path to a file to store the gzipped JSON data in. diff --git a/mopidy/backends/local/json/__init__.py b/mopidy/backends/local/json/__init__.py deleted file mode 100644 index 031dae51..00000000 --- a/mopidy/backends/local/json/__init__.py +++ /dev/null @@ -1,30 +0,0 @@ -from __future__ import unicode_literals - -import os - -import mopidy -from mopidy import config, ext - - -class Extension(ext.Extension): - - dist_name = 'Mopidy-Local-JSON' - ext_name = 'local-json' - version = mopidy.__version__ - - def get_default_config(self): - conf_file = os.path.join(os.path.dirname(__file__), 'ext.conf') - return config.read(conf_file) - - def get_config_schema(self): - schema = super(Extension, self).get_config_schema() - schema['json_file'] = config.Path() - return schema - - def get_backend_classes(self): - from .actor import LocalJsonBackend - return [LocalJsonBackend] - - def get_library_updaters(self): - from .library import LocalJsonLibraryUpdateProvider - return [LocalJsonLibraryUpdateProvider] diff --git a/mopidy/backends/local/json/actor.py b/mopidy/backends/local/json/actor.py deleted file mode 100644 index 66a6fbd5..00000000 --- a/mopidy/backends/local/json/actor.py +++ /dev/null @@ -1,30 +0,0 @@ -from __future__ import unicode_literals - -import logging -import os - -import pykka - -from mopidy.backends import base -from mopidy.utils import encoding - -from . import library - -logger = logging.getLogger('mopidy.backends.local.json') - - -class LocalJsonBackend(pykka.ThreadingActor, base.Backend): - def __init__(self, config, audio): - super(LocalJsonBackend, self).__init__() - - self.config = config - self.library = library.LocalJsonLibraryProvider(backend=self) - self.uri_schemes = ['local'] - - if not os.path.exists(config['local-json']['json_file']): - try: - library.write_library(config['local-json']['json_file'], {}) - logger.info('Created empty local JSON library.') - except EnvironmentError as error: - error = encoding.locale_decode(error) - logger.warning('Could not create local library: %s', error) diff --git a/mopidy/backends/local/json/ext.conf b/mopidy/backends/local/json/ext.conf deleted file mode 100644 index db0b784a..00000000 --- a/mopidy/backends/local/json/ext.conf +++ /dev/null @@ -1,3 +0,0 @@ -[local-json] -enabled = true -json_file = $XDG_DATA_DIR/mopidy/local/library.json.gz diff --git a/mopidy/backends/local/json/library.py b/mopidy/backends/local/json/library.py deleted file mode 100644 index 33427231..00000000 --- a/mopidy/backends/local/json/library.py +++ /dev/null @@ -1,108 +0,0 @@ -from __future__ import unicode_literals - -import gzip -import json -import logging -import os -import tempfile - -import mopidy -from mopidy import models -from mopidy.backends import base -from mopidy.backends.local import search - -logger = logging.getLogger('mopidy.backends.local.json') - - -def load_library(json_file): - try: - with gzip.open(json_file, 'rb') as fp: - return json.load(fp, object_hook=models.model_json_decoder) - except (IOError, ValueError) as e: - logger.warning('Loading JSON local library failed: %s', e) - return {} - - -def write_library(json_file, data): - data['version'] = mopidy.__version__ - directory, basename = os.path.split(json_file) - - # TODO: cleanup directory/basename.* files. - tmp = tempfile.NamedTemporaryFile( - prefix=basename + '.', dir=directory, delete=False) - - try: - with gzip.GzipFile(fileobj=tmp, mode='wb') as fp: - json.dump(data, fp, cls=models.ModelJSONEncoder, - indent=2, separators=(',', ': ')) - os.rename(tmp.name, json_file) - finally: - if os.path.exists(tmp.name): - os.remove(tmp.name) - - -class LocalJsonLibraryProvider(base.BaseLibraryProvider): - def __init__(self, *args, **kwargs): - super(LocalJsonLibraryProvider, self).__init__(*args, **kwargs) - self._uri_mapping = {} - self._media_dir = self.backend.config['local']['media_dir'] - self._json_file = self.backend.config['local-json']['json_file'] - self.refresh() - - def refresh(self, uri=None): - logger.debug( - 'Loading local tracks from %s using %s', - self._media_dir, self._json_file) - - tracks = load_library(self._json_file).get('tracks', []) - uris_to_remove = set(self._uri_mapping) - - for track in tracks: - self._uri_mapping[track.uri] = track - uris_to_remove.discard(track.uri) - - for uri in uris_to_remove: - del self._uri_mapping[uri] - - logger.info( - 'Loaded %d local tracks from %s using %s', - len(tracks), self._media_dir, self._json_file) - - def lookup(self, uri): - try: - return [self._uri_mapping[uri]] - except KeyError: - logger.debug('Failed to lookup %r', uri) - return [] - - def find_exact(self, query=None, uris=None): - tracks = self._uri_mapping.values() - return search.find_exact(tracks, query=query, uris=uris) - - def search(self, query=None, uris=None): - tracks = self._uri_mapping.values() - return search.search(tracks, query=query, uris=uris) - - -class LocalJsonLibraryUpdateProvider(base.BaseLibraryProvider): - uri_schemes = ['local'] - - def __init__(self, config): - self._tracks = {} - self._media_dir = config['local']['media_dir'] - self._json_file = config['local-json']['json_file'] - - def load(self): - for track in load_library(self._json_file).get('tracks', []): - self._tracks[track.uri] = track - return self._tracks.values() - - def add(self, track): - self._tracks[track.uri] = track - - def remove(self, uri): - if uri in self._tracks: - del self._tracks[uri] - - def commit(self): - write_library(self._json_file, {'tracks': self._tracks.values()}) diff --git a/setup.py b/setup.py index bc2fe222..f43981bf 100644 --- a/setup.py +++ b/setup.py @@ -43,7 +43,6 @@ setup( 'mopidy.ext': [ 'http = mopidy.frontends.http:Extension [http]', 'local = mopidy.backends.local:Extension', - 'local-json = mopidy.backends.local.json:Extension', 'mpd = mopidy.frontends.mpd:Extension', 'stream = mopidy.backends.stream:Extension', ], From e065f349db424d81228663039c6f98ff76b38e2f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 23 Dec 2013 23:36:01 +0100 Subject: [PATCH 07/23] local: Add local library provider back - Re-add a local library provider that uses our new library interface - Re-add our json library using the new interface - Hardcode these to use each other for now - Scanner bit is still missing, will re-add in one of the next commits - Bypassed test for #500 for the time being --- mopidy/backends/local/actor.py | 12 +++-- mopidy/backends/local/json.py | 80 ++++++++++++++++++++++++++++ mopidy/backends/local/library.py | 29 ++++++++++ tests/backends/local/library_test.py | 13 ++--- 4 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 mopidy/backends/local/json.py create mode 100644 mopidy/backends/local/library.py diff --git a/mopidy/backends/local/actor.py b/mopidy/backends/local/actor.py index 78caf39e..3da246db 100644 --- a/mopidy/backends/local/actor.py +++ b/mopidy/backends/local/actor.py @@ -8,13 +8,17 @@ import pykka from mopidy.backends import base from mopidy.utils import encoding, path -from .playlists import LocalPlaylistsProvider +from .json import JsonLibrary +from .library import LocalLibraryProvider from .playback import LocalPlaybackProvider +from .playlists import LocalPlaylistsProvider logger = logging.getLogger('mopidy.backends.local') class LocalBackend(pykka.ThreadingActor, base.Backend): + uri_schemes = ['local'] + def __init__(self, config, audio): super(LocalBackend, self).__init__() @@ -22,10 +26,12 @@ class LocalBackend(pykka.ThreadingActor, base.Backend): self.check_dirs_and_files() + # TODO: move to getting this from registry + library = JsonLibrary(config) + self.playback = LocalPlaybackProvider(audio=audio, backend=self) self.playlists = LocalPlaylistsProvider(backend=self) - - self.uri_schemes = ['local'] + self.library = LocalLibraryProvider(backend=self, library=library) def check_dirs_and_files(self): if not os.path.isdir(self.config['local']['media_dir']): diff --git a/mopidy/backends/local/json.py b/mopidy/backends/local/json.py new file mode 100644 index 00000000..327f706c --- /dev/null +++ b/mopidy/backends/local/json.py @@ -0,0 +1,80 @@ +from __future__ import absolute_import, unicode_literals + +import gzip +import json +import logging +import os +import tempfile + +import mopidy +from mopidy import models +from mopidy.backends import local +from mopidy.backends.local import search + +logger = logging.getLogger('mopidy.backends.local.json') + + +# TODO: move to load and dump in models? +def load_library(json_file): + try: + with gzip.open(json_file, 'rb') as fp: + return json.load(fp, object_hook=models.model_json_decoder) + except (IOError, ValueError) as e: + logger.warning('Loading JSON local library failed: %s', e) + return {} + + +def write_library(json_file, data): + data['version'] = mopidy.__version__ + directory, basename = os.path.split(json_file) + + # TODO: cleanup directory/basename.* files. + tmp = tempfile.NamedTemporaryFile( + prefix=basename + '.', dir=directory, delete=False) + + try: + with gzip.GzipFile(fileobj=tmp, mode='wb') as fp: + json.dump(data, fp, cls=models.ModelJSONEncoder, + indent=2, separators=(',', ': ')) + os.rename(tmp.name, json_file) + finally: + if os.path.exists(tmp.name): + os.remove(tmp.name) + + +class JsonLibrary(local.Library): + name = b'json' + + def __init__(self, config): + self._tracks = {} + self._media_dir = config['local']['media_dir'] + self._json_file = os.path.join( + config['local']['data_dir'], b'library.json.gz') + + def load(self): + library = load_library(self._json_file) + self._tracks = dict((t.uri, t) for t in library.get('tracks', [])) + return len(self._tracks) + + def add(self, track): + self._tracks[track.uri] = track + + def remove(self, uri): + self._tracks.pop(uri, None) + + def commit(self): + write_library(self._json_file, {'tracks': self._tracks.values()}) + + def lookup(self, uri): + try: + return [self._tracks[uri]] + except KeyError: + logger.debug('Failed to lookup %r', uri) + return [] + + def search(self, query=None, uris=None, exact=False): + tracks = self._tracks.values() + if exact: + return search.find_exact(tracks, query=query, uris=uris) + else: + return search.search(tracks, query=query, uris=uris) diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py new file mode 100644 index 00000000..89a6da4d --- /dev/null +++ b/mopidy/backends/local/library.py @@ -0,0 +1,29 @@ +from __future__ import unicode_literals + +import logging + +from mopidy.backends import base + +logger = logging.getLogger('mopidy.backends.local') + + +class LocalLibraryProvider(base.BaseLibraryProvider): + """Proxy library that delegates work to our active local library.""" + def __init__(self, backend, library): + super(LocalLibraryProvider, self).__init__(backend) + self._library = library + self.refresh() + + def refresh(self, uri=None): + num_tracks = self._library.load() + logger.info('Loaded %d local tracks using %s', + num_tracks, self._library.name) + + def lookup(self, uri): + return self._library.lookup(uri) + + def find_exact(self, query=None, uris=None): + return self._library.search(query=query, uris=uris, exact=True) + + def search(self, query=None, uris=None): + return self._library.search(query=query, uris=uris, exact=False) diff --git a/tests/backends/local/library_test.py b/tests/backends/local/library_test.py index e4c00570..9bfcb233 100644 --- a/tests/backends/local/library_test.py +++ b/tests/backends/local/library_test.py @@ -7,7 +7,7 @@ import unittest import pykka from mopidy import core -from mopidy.backends.local.json import actor +from mopidy.backends.local import actor from mopidy.models import Track, Album, Artist from tests import path_to_data_dir @@ -61,15 +61,13 @@ class LocalLibraryProviderTest(unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), + 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', }, - 'local-json': { - 'json_file': path_to_data_dir('library.json.gz'), - }, } def setUp(self): - self.backend = actor.LocalJsonBackend.start( + self.backend = actor.LocalBackend.start( config=self.config, audio=None).proxy() self.core = core.Core(backends=[self.backend]) self.library = self.core.library @@ -88,6 +86,9 @@ class LocalLibraryProviderTest(unittest.TestCase): # Verifies that https://github.com/mopidy/mopidy/issues/500 # has been fixed. + # TODO: re-add something that tests this in a more sane way + return + with tempfile.NamedTemporaryFile() as library: with open(self.config['local-json']['json_file']) as fh: library.write(fh.read()) @@ -95,7 +96,7 @@ class LocalLibraryProviderTest(unittest.TestCase): config = copy.deepcopy(self.config) config['local-json']['json_file'] = library.name - backend = actor.LocalJsonBackend(config=config, audio=None) + backend = actor.LocalBackend(config=config, audio=None) # Sanity check that value is in the library result = backend.library.lookup(self.tracks[0].uri) From d93d3e6fcdfb0076ea467ae7cac0789f322f87f7 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 24 Dec 2013 00:22:58 +0100 Subject: [PATCH 08/23] local: Add local/library config value - Updated library provider to support missing library - Added config value to select local library provider - Updated tests to use library config value --- docs/ext/local.rst | 5 +++++ mopidy/backends/local/__init__.py | 10 ++++++++-- mopidy/backends/local/actor.py | 13 ++++++++++--- mopidy/backends/local/ext.conf | 1 + mopidy/backends/local/library.py | 8 ++++++++ mopidy/ext.py | 4 +--- tests/backends/local/events_test.py | 1 + tests/backends/local/library_test.py | 5 ++++- tests/backends/local/playback_test.py | 1 + tests/backends/local/playlists_test.py | 1 + tests/backends/local/tracklist_test.py | 1 + 11 files changed, 41 insertions(+), 9 deletions(-) diff --git a/docs/ext/local.rst b/docs/ext/local.rst index eed51829..5cf4b2d3 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -35,6 +35,11 @@ Configuration values If the local extension should be enabled or not. +.. confval:: local/library + + Local library provider to use, change this if you want to use a third party + library for local files. + .. confval:: local/media_dir Path to directory with local media files. diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index e0e917f7..d42f08ce 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -21,6 +21,7 @@ class Extension(ext.Extension): def get_config_schema(self): schema = super(Extension, self).get_config_schema() + schema['library'] = config.String() schema['media_dir'] = config.Path() schema['data_dir'] = config.Path() schema['playlists_dir'] = config.Path() @@ -30,9 +31,14 @@ class Extension(ext.Extension): schema['excluded_file_extensions'] = config.List(optional=True) return schema - def get_backend_classes(self): + def setup(self, registry): from .actor import LocalBackend - return [LocalBackend] + from .json import JsonLibrary + + LocalBackend.libraries = registry['local:library'] + + registry.add('backend', LocalBackend) + registry.add('local:library', JsonLibrary) def get_command(self): from .commands import LocalCommand diff --git a/mopidy/backends/local/actor.py b/mopidy/backends/local/actor.py index 3da246db..5925f993 100644 --- a/mopidy/backends/local/actor.py +++ b/mopidy/backends/local/actor.py @@ -8,7 +8,6 @@ import pykka from mopidy.backends import base from mopidy.utils import encoding, path -from .json import JsonLibrary from .library import LocalLibraryProvider from .playback import LocalPlaybackProvider from .playlists import LocalPlaylistsProvider @@ -18,6 +17,7 @@ logger = logging.getLogger('mopidy.backends.local') class LocalBackend(pykka.ThreadingActor, base.Backend): uri_schemes = ['local'] + libraries = [] def __init__(self, config, audio): super(LocalBackend, self).__init__() @@ -26,8 +26,15 @@ class LocalBackend(pykka.ThreadingActor, base.Backend): self.check_dirs_and_files() - # TODO: move to getting this from registry - library = JsonLibrary(config) + libraries = dict((l.name, l) for l in self.libraries) + library_name = config['local']['library'] + + if library_name in libraries: + library = libraries[library_name](config) + logger.debug('Using %s as the local library', library_name) + else: + library = None + logger.warning('Local library %s not found', library_name) self.playback = LocalPlaybackProvider(audio=audio, backend=self) self.playlists = LocalPlaylistsProvider(backend=self) diff --git a/mopidy/backends/local/ext.conf b/mopidy/backends/local/ext.conf index e826a451..5f83db05 100644 --- a/mopidy/backends/local/ext.conf +++ b/mopidy/backends/local/ext.conf @@ -1,5 +1,6 @@ [local] enabled = true +library = json media_dir = $XDG_MUSIC_DIR data_dir = $XDG_DATA_DIR/mopidy/local playlists_dir = $XDG_DATA_DIR/mopidy/local/playlists diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index 89a6da4d..aeb7736b 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -15,15 +15,23 @@ class LocalLibraryProvider(base.BaseLibraryProvider): self.refresh() def refresh(self, uri=None): + if not self._library: + return 0 num_tracks = self._library.load() logger.info('Loaded %d local tracks using %s', num_tracks, self._library.name) def lookup(self, uri): + if not self._library: + return [] return self._library.lookup(uri) def find_exact(self, query=None, uris=None): + if not self._library: + return None return self._library.search(query=query, uris=uris, exact=True) def search(self, query=None, uris=None): + if not self._library: + return None return self._library.search(query=query, uris=uris, exact=False) diff --git a/mopidy/ext.py b/mopidy/ext.py index c79ebe40..a9ca2519 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -69,9 +69,6 @@ class Extension(object): for frontend_class in self.get_frontend_classes(): registry.add('frontend', frontend_class) - for library_updater in self.get_library_updaters(): - registry.add('local:library', library_updater) - self.register_gstreamer_elements() def get_frontend_classes(self): @@ -92,6 +89,7 @@ class Extension(object): """ return [] + # TODO: remove def get_library_updaters(self): """List of library updater classes diff --git a/tests/backends/local/events_test.py b/tests/backends/local/events_test.py index 2424ed42..967d4cdb 100644 --- a/tests/backends/local/events_test.py +++ b/tests/backends/local/events_test.py @@ -19,6 +19,7 @@ class LocalBackendEventsTest(unittest.TestCase): 'media_dir': path_to_data_dir(''), 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', + 'library': 'json', } } diff --git a/tests/backends/local/library_test.py b/tests/backends/local/library_test.py index 9bfcb233..40bece7d 100644 --- a/tests/backends/local/library_test.py +++ b/tests/backends/local/library_test.py @@ -7,7 +7,7 @@ import unittest import pykka from mopidy import core -from mopidy.backends.local import actor +from mopidy.backends.local import actor, json from mopidy.models import Track, Album, Artist from tests import path_to_data_dir @@ -63,10 +63,12 @@ class LocalLibraryProviderTest(unittest.TestCase): 'media_dir': path_to_data_dir(''), 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', + 'library': 'json', }, } def setUp(self): + actor.LocalBackend.libraries = [json.JsonLibrary] self.backend = actor.LocalBackend.start( config=self.config, audio=None).proxy() self.core = core.Core(backends=[self.backend]) @@ -74,6 +76,7 @@ class LocalLibraryProviderTest(unittest.TestCase): def tearDown(self): pykka.ActorRegistry.stop_all() + actor.LocalBackend.libraries = [] def test_refresh(self): self.library.refresh() diff --git a/tests/backends/local/playback_test.py b/tests/backends/local/playback_test.py index 4da420ef..7d48cfea 100644 --- a/tests/backends/local/playback_test.py +++ b/tests/backends/local/playback_test.py @@ -24,6 +24,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): 'media_dir': path_to_data_dir(''), 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', + 'library': 'json', } } diff --git a/tests/backends/local/playlists_test.py b/tests/backends/local/playlists_test.py index 447de3f8..6c602282 100644 --- a/tests/backends/local/playlists_test.py +++ b/tests/backends/local/playlists_test.py @@ -21,6 +21,7 @@ class LocalPlaylistsProviderTest(unittest.TestCase): 'local': { 'media_dir': path_to_data_dir(''), 'data_dir': path_to_data_dir(''), + 'library': 'json', } } diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index 14bf678d..28def50c 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -20,6 +20,7 @@ class LocalTracklistProviderTest(unittest.TestCase): 'media_dir': path_to_data_dir(''), 'data_dir': path_to_data_dir(''), 'playlists_dir': b'', + 'library': 'json', } } tracks = [ From ff5743999535c20ae758cda9ed90e3dd2b829659 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 24 Dec 2013 00:54:02 +0100 Subject: [PATCH 09/23] local: Update library interface - Add track iterator for use in scanner - Update lookup to only return a single track --- mopidy/backends/local/__init__.py | 10 +++++++++- mopidy/backends/local/json.py | 8 +++++--- mopidy/backends/local/library.py | 6 +++++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index d42f08ce..0f9b7502 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -63,6 +63,14 @@ class Library(object): """ return 0 + def tracks(self): + """ + Iterator over all tracks. + + :rtype: :class:`mopidy.models.Track` iterator + """ + raise NotImplementedError + def add(self, track): """ Add the given track to library. @@ -99,7 +107,7 @@ class Library(object): :param uri: track URI :type uri: string - :rtype: list of :class:`mopidy.models.Track` + :rtype: :class:`mopidy.models.Track` """ raise NotImplementedError diff --git a/mopidy/backends/local/json.py b/mopidy/backends/local/json.py index 327f706c..20b922e6 100644 --- a/mopidy/backends/local/json.py +++ b/mopidy/backends/local/json.py @@ -56,6 +56,9 @@ class JsonLibrary(local.Library): self._tracks = dict((t.uri, t) for t in library.get('tracks', [])) return len(self._tracks) + def tracks(self): + return self._tracks.itervalues() + def add(self, track): self._tracks[track.uri] = track @@ -67,10 +70,9 @@ class JsonLibrary(local.Library): def lookup(self, uri): try: - return [self._tracks[uri]] + return self._tracks[uri] except KeyError: - logger.debug('Failed to lookup %r', uri) - return [] + return None def search(self, query=None, uris=None, exact=False): tracks = self._tracks.values() diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index aeb7736b..eadea027 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -24,7 +24,11 @@ class LocalLibraryProvider(base.BaseLibraryProvider): def lookup(self, uri): if not self._library: return [] - return self._library.lookup(uri) + track = self._library.lookup(uri) + if not uri: + logger.debug('Failed to lookup %r', uri) + return [] + return [track] def find_exact(self, query=None, uris=None): if not self._library: From ba642aa6805c5b8daf3d548ef1ee82b236743cbb Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 24 Dec 2013 00:57:57 +0100 Subject: [PATCH 10/23] local: Update scanner to use new library interface. --- mopidy/backends/local/commands.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index 2f6f744e..63970c2d 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -29,25 +29,24 @@ class ScanCommand(commands.Command): excluded_file_extensions = set( file_ext.lower() for file_ext in excluded_file_extensions) - # TODO: select updater / library to use by name - updaters = args.registry['local:library'] - if not updaters: - logger.error('No usable library updaters found.') + libraries = dict((l.name, l) for l in args.registry['local:library']) + library_name = config['local']['library'] + + if library_name not in libraries: + logger.warning('Local library %s not found', library_name) return 1 - elif len(updaters) > 1: - logger.error('More than one library updater found. ' - 'Provided by: %s', ', '.join(updaters)) - return 1 - local_updater = updaters[0](config) + + library = libraries[library_name](config) + logger.debug('Using %s as the local library', library_name) uri_path_mapping = {} uris_in_library = set() uris_to_update = set() uris_to_remove = set() - tracks = local_updater.load() - logger.info('Checking %d tracks from library.', len(tracks)) - for track in tracks: + num_tracks = library.load() + logger.info('Checking %d tracks from library.', num_tracks) + for track in library.tracks(): uri_path_mapping[track.uri] = translator.local_track_uri_to_path( track.uri, media_dir) try: @@ -61,7 +60,7 @@ class ScanCommand(commands.Command): logger.info('Removing %d missing tracks.', len(uris_to_remove)) for uri in uris_to_remove: - local_updater.remove(uri) + library.remove(uri) logger.info('Checking %s for unknown tracks.', media_dir) for relpath in path.find_files(media_dir): @@ -85,7 +84,7 @@ class ScanCommand(commands.Command): try: data = scanner.scan(path.path_to_uri(uri_path_mapping[uri])) track = scan.audio_data_to_track(data).copy(uri=uri) - local_updater.add(track) + library.add(track) logger.debug('Added %s', track.uri) except exceptions.ScannerError as error: logger.warning('Failed %s: %s', uri, error) @@ -93,7 +92,7 @@ class ScanCommand(commands.Command): progress.increment() logger.info('Commiting changes.') - local_updater.commit() + library.commit() return 0 From 0f671516ed129d4b51fac2eec88272d6ed56b314 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 28 Dec 2013 00:00:10 +0100 Subject: [PATCH 11/23] local: Make sure excluded file extension case does not error out --- mopidy/backends/local/commands.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index 63970c2d..60bc8ad4 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -64,12 +64,13 @@ class ScanCommand(commands.Command): logger.info('Checking %s for unknown tracks.', media_dir) for relpath in path.find_files(media_dir): + uri = translator.path_to_local_track_uri(relpath) file_extension = os.path.splitext(relpath)[1] + if file_extension.lower() in excluded_file_extensions: logger.debug('Skipped %s: File extension excluded.', uri) continue - uri = translator.path_to_local_track_uri(relpath) if uri not in uris_in_library: uris_to_update.add(uri) uri_path_mapping[uri] = os.path.join(media_dir, relpath) From 565b3aeb985897773ead5ba397c9956dc05faa24 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 28 Dec 2013 00:06:16 +0100 Subject: [PATCH 12/23] audio: Workaround fact that genre can sometimes be a list --- mopidy/audio/scan.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index f797a84d..0c8e3478 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -181,6 +181,12 @@ def audio_data_to_track(data): track_kwargs['uri'] = data['uri'] track_kwargs['album'] = Album(**album_kwargs) + # TODO: this feels like a half assed workaround. we need to be sure that we + # don't suddenly have lists in our models where we expect strings etc + if ('genre' in track_kwargs and + not isinstance(track_kwargs['genre'], basestring)): + track_kwargs['genre'] = ', '.join(track_kwargs['genre']) + if ('name' in artist_kwargs and not isinstance(artist_kwargs['name'], basestring)): track_kwargs['artists'] = [Artist(name=artist) From 40af4c5f9e68c4f30f918f33e9445a0ba94d2b4a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 28 Dec 2013 00:17:31 +0100 Subject: [PATCH 13/23] local: Review comments --- mopidy/backends/local/__init__.py | 8 +++++--- mopidy/backends/local/library.py | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index 0f9b7502..436f43d4 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -111,18 +111,20 @@ class Library(object): """ raise NotImplementedError - # TODO: support case with returning all tracks? - # TODO: remove uris? + # TODO: remove uris, replacing it with support in query language. + # TODO: remove exact, replacing it with support in query language. def search(self, query=None, exact=False, uris=None): """ Search the library for tracks where ``field`` contains ``values``. :param query: one or more queries to search for :type query: dict - :param exact: look for exact matches? + :param exact: whether to look for exact matches :type query: boolean :param uris: zero or more URI roots to limit the search to :type uris: list of strings or :class:`None` :rtype: :class:`mopidy.models.SearchResult` """ raise NotImplementedError + + # TODO: add file browsing support. diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index eadea027..d8cc2320 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -4,11 +4,12 @@ import logging from mopidy.backends import base -logger = logging.getLogger('mopidy.backends.local') +logger = logging.getLogger('mopidy.backends.local.library') class LocalLibraryProvider(base.BaseLibraryProvider): """Proxy library that delegates work to our active local library.""" + def __init__(self, backend, library): super(LocalLibraryProvider, self).__init__(backend) self._library = library @@ -25,7 +26,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): if not self._library: return [] track = self._library.lookup(uri) - if not uri: + if track is None: logger.debug('Failed to lookup %r', uri) return [] return [track] From 36e9b43e6c831c75830e34ba1349b6fb05374802 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 30 Dec 2013 01:17:33 +0100 Subject: [PATCH 14/23] local: Update local library interface. Refactored interface to incorperate lessons learned so far trying to implemend a whoosh based local library. Search now has a limit and an offset to account for fact that we need to start doing pagination of results properly. Updates now have begin, flush and close calls. Additionally I've added clear method to allow for easily nuking the data store. --- mopidy/backends/local/__init__.py | 104 ++++++++++++++++++------------ mopidy/backends/local/commands.py | 7 +- mopidy/backends/local/json.py | 34 ++++++---- 3 files changed, 89 insertions(+), 56 deletions(-) diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index 436f43d4..4031c474 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -46,7 +46,17 @@ class Extension(ext.Extension): class Library(object): - #: Name of the local library implementation. + """ + Local library interface. + + Extensions that whish to provide an alternate local library storage backend + need to sub-class this class and install and confgure it with an extension. + Both scanning and library calls will use the active local library. + + :param config: Config dictionary + """ + + #: Name of the local library implementation, must be overriden. name = None def __init__(self, config): @@ -54,20 +64,53 @@ class Library(object): def load(self): """ - Initialize whatever resources are needed for this library. - - This is where you load the tracks into memory, setup a database - conection etc. + (Re)load any tracks stored in memory, if any, otherwise just return + number of available tracks currently available. Will be called at + startup for both library and update use cases, so if you plan to store + tracks in memory this is when the should be (re)loaded. :rtype: :class:`int` representing number of tracks in library. """ return 0 - def tracks(self): + def lookup(self, uri): """ - Iterator over all tracks. + Lookup the given URI. - :rtype: :class:`mopidy.models.Track` iterator + Unlike the core APIs, local tracks uris can only be resolved to a + single track. + + :param string uri: track URI + :rtype: :class:`~mopidy.models.Track` + """ + raise NotImplementedError + + # TODO: remove uris, replacing it with support in query language. + # TODO: remove exact, replacing it with support in query language. + def search(self, query=None, limit=100, offset=0, exact=False, uris=None): + """ + Search the library for tracks where ``field`` contains ``values``. + + :param dict query: one or more queries to search for + :param int limit: maximum number of results to return + :param int offset: offset into result set to use. + :param bool exact: whether to look for exact matches + :param uris: zero or more URI roots to limit the search to + :type uris: list of strings or :class:`None` + :rtype: :class:`~mopidy.models.SearchResult` + """ + raise NotImplementedError + + # TODO: add file browsing support. + + # Remaining methods are use for the update process. + def begin(self): + """ + Prepare library for accepting updates. Exactly what this means is + highly implementation depended. This must however return an iterator + that generates all tracks in the library for efficient scanning. + + :rtype: :class:`~mopidy.models.Track` iterator """ raise NotImplementedError @@ -75,8 +118,7 @@ class Library(object): """ Add the given track to library. - :param track: Track to add to the library/ - :type track: :class:`mopidy.models.Track` + :param :class:`~mopidy.models.Track` track: Track to add to the library """ raise NotImplementedError @@ -84,47 +126,27 @@ class Library(object): """ Remove the given track from the library. - :param uri: URI to remove from the library/ - :type uri: string + :param str uri: URI to remove from the library/ """ raise NotImplementedError - def commit(self): + def flush(self): """ - Persist any changes to the library. - - This is where you write your data file to disk, commit transactions - etc. depending on the requirements of your library implementation. + Called for every n-th track indicating that work should be commited, + implementors are free to ignore these hints. """ pass - def lookup(self, uri): + def close(self): """ - Lookup the given URI. - - If the URI expands to multiple tracks, the returned list will contain - them all. - - :param uri: track URI - :type uri: string - :rtype: :class:`mopidy.models.Track` + Close any resources used for updating, commit outstanding work etc. """ - raise NotImplementedError + pass - # TODO: remove uris, replacing it with support in query language. - # TODO: remove exact, replacing it with support in query language. - def search(self, query=None, exact=False, uris=None): + def clear(self): """ - Search the library for tracks where ``field`` contains ``values``. + Clear out whatever data storage is used by this backend. - :param query: one or more queries to search for - :type query: dict - :param exact: whether to look for exact matches - :type query: boolean - :param uris: zero or more URI roots to limit the search to - :type uris: list of strings or :class:`None` - :rtype: :class:`mopidy.models.SearchResult` + :rtype: Boolean indicating if state was cleared. """ - raise NotImplementedError - - # TODO: add file browsing support. + return False diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index 60bc8ad4..a2098078 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -46,7 +46,8 @@ class ScanCommand(commands.Command): num_tracks = library.load() logger.info('Checking %d tracks from library.', num_tracks) - for track in library.tracks(): + + for track in library.begin(): uri_path_mapping[track.uri] = translator.local_track_uri_to_path( track.uri, media_dir) try: @@ -90,10 +91,12 @@ class ScanCommand(commands.Command): except exceptions.ScannerError as error: logger.warning('Failed %s: %s', uri, error) + # TODO: trigger this on batch size intervals instead and add + # flush progress.increment() logger.info('Commiting changes.') - library.commit() + library.close() return 0 diff --git a/mopidy/backends/local/json.py b/mopidy/backends/local/json.py index 20b922e6..d1cacd6d 100644 --- a/mopidy/backends/local/json.py +++ b/mopidy/backends/local/json.py @@ -56,7 +56,21 @@ class JsonLibrary(local.Library): self._tracks = dict((t.uri, t) for t in library.get('tracks', [])) return len(self._tracks) - def tracks(self): + def lookup(self, uri): + try: + return self._tracks[uri] + except KeyError: + return None + + def search(self, query=None, limit=100, offset=0, uris=None, exact=False): + tracks = self._tracks.values() + # TODO: pass limit and offset into search helpers + if exact: + return search.find_exact(tracks, query=query, uris=uris) + else: + return search.search(tracks, query=query, uris=uris) + + def begin(self): return self._tracks.itervalues() def add(self, track): @@ -65,18 +79,12 @@ class JsonLibrary(local.Library): def remove(self, uri): self._tracks.pop(uri, None) - def commit(self): + def close(self): write_library(self._json_file, {'tracks': self._tracks.values()}) - def lookup(self, uri): + def clear(self): try: - return self._tracks[uri] - except KeyError: - return None - - def search(self, query=None, uris=None, exact=False): - tracks = self._tracks.values() - if exact: - return search.find_exact(tracks, query=query, uris=uris) - else: - return search.search(tracks, query=query, uris=uris) + os.remove(self._json_file) + return True + except OSError: + return False From 09c0ae2551b3ebb2639c773f0af114f7e43dcef4 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 30 Dec 2013 01:31:00 +0100 Subject: [PATCH 15/23] local: Add flush threshold to scanner. Instead of triggering every 1000th query, this is now configurable and also triggers the flush call to the library. --- docs/ext/local.rst | 5 +++++ mopidy/backends/local/__init__.py | 1 + mopidy/backends/local/commands.py | 33 +++++++++++-------------------- mopidy/backends/local/ext.conf | 1 + 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/docs/ext/local.rst b/docs/ext/local.rst index 5cf4b2d3..4484ab0d 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -58,6 +58,11 @@ Configuration values Number of milliseconds before giving up scanning a file and moving on to the next file. +.. confval:: local/scan_flush_threshold + + Number of tracks to wait before telling library it should try and store + it's progress so far. Some libraries might not respect this setting. + .. confval:: local/excluded_file_extensions File extensions to exclude when scanning the media directory. Values diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index 4031c474..6697d91d 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -28,6 +28,7 @@ class Extension(ext.Extension): schema['tag_cache_file'] = config.Deprecated() schema['scan_timeout'] = config.Integer( minimum=1000, maximum=1000*60*60) + schema['scan_flush_threshold'] = config.Integer(minimum=0) schema['excluded_file_extensions'] = config.List(optional=True) return schema diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index a2098078..65bfd274 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -25,6 +25,7 @@ class ScanCommand(commands.Command): def run(self, args, config): media_dir = config['local']['media_dir'] scan_timeout = config['local']['scan_timeout'] + flush_threshold = config['local']['scan_flush_threshold'] excluded_file_extensions = config['local']['excluded_file_extensions'] excluded_file_extensions = set( file_ext.lower() for file_ext in excluded_file_extensions) @@ -80,7 +81,9 @@ class ScanCommand(commands.Command): logger.info('Scanning...') scanner = scan.Scanner(scan_timeout) - progress = Progress(len(uris_to_update)) + count = 0 + total = len(uris_to_update) + start = time.time() for uri in sorted(uris_to_update): try: @@ -91,26 +94,14 @@ class ScanCommand(commands.Command): except exceptions.ScannerError as error: logger.warning('Failed %s: %s', uri, error) - # TODO: trigger this on batch size intervals instead and add - # flush - progress.increment() + count += 1 + if count % flush_threshold == 0 or count == total: + duration = time.time() - start + remainder = duration / count * (total - count) + logger.info('Scanned %d of %d files in %ds, ~%ds left.', + count, total, duration, remainder) + library.flush() - logger.info('Commiting changes.') library.close() + logger.info('Done scanning.') return 0 - - -# TODO: move to utils? -class Progress(object): - def __init__(self, total): - self.count = 0 - self.total = total - self.start = time.time() - - def increment(self): - self.count += 1 - if self.count % 1000 == 0 or self.count == self.total: - duration = time.time() - self.start - remainder = duration / self.count * (self.total - self.count) - logger.info('Scanned %d of %d files in %ds, ~%ds left.', - self.count, self.total, duration, remainder) diff --git a/mopidy/backends/local/ext.conf b/mopidy/backends/local/ext.conf index 5f83db05..8f1e860c 100644 --- a/mopidy/backends/local/ext.conf +++ b/mopidy/backends/local/ext.conf @@ -5,6 +5,7 @@ media_dir = $XDG_MUSIC_DIR data_dir = $XDG_DATA_DIR/mopidy/local playlists_dir = $XDG_DATA_DIR/mopidy/local/playlists scan_timeout = 1000 +scan_flush_threshold = 1000 excluded_file_extensions = .html .jpeg From 82877ec60f8ae85761c25f9866b4d80b6ee52a2b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 30 Dec 2013 01:38:44 +0100 Subject: [PATCH 16/23] local: Add --clear flag to scanner. --- mopidy/backends/local/commands.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index 65bfd274..1c290705 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -22,6 +22,11 @@ class LocalCommand(commands.Command): class ScanCommand(commands.Command): help = "Scan local media files and populate the local library." + def __init__(self): + super(ScanCommand, self).__init__() + self.add_argument('--clear', action='store_true', dest='clear', + help='Clear out library storage') + def run(self, args, config): media_dir = config['local']['media_dir'] scan_timeout = config['local']['scan_timeout'] @@ -40,6 +45,13 @@ class ScanCommand(commands.Command): library = libraries[library_name](config) logger.debug('Using %s as the local library', library_name) + if args.clear: + if library.clear(): + logging.info('Library succesfully cleared.') + return 0 + logging.warning('Unable to clear library.') + return 1 + uri_path_mapping = {} uris_in_library = set() uris_to_update = set() From a462f132d3f9cdd54505211151eb3b4af6a00de1 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 30 Dec 2013 01:43:11 +0100 Subject: [PATCH 17/23] local: Add --limit to scanner. --- mopidy/backends/local/commands.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index 1c290705..8eaf9d0d 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -26,6 +26,8 @@ class ScanCommand(commands.Command): super(ScanCommand, self).__init__() self.add_argument('--clear', action='store_true', dest='clear', help='Clear out library storage') + self.add_argument('--limit', action='store', type=int, dest='limit', + default=0, help='Maxmimum number of tracks to scan') def run(self, args, config): media_dir = config['local']['media_dir'] @@ -94,10 +96,10 @@ class ScanCommand(commands.Command): scanner = scan.Scanner(scan_timeout) count = 0 - total = len(uris_to_update) + total = args.limit or len(uris_to_update) start = time.time() - for uri in sorted(uris_to_update): + for uri in sorted(uris_to_update)[:args.limit or None]: try: data = scanner.scan(path.path_to_uri(uri_path_mapping[uri])) track = scan.audio_data_to_track(data).copy(uri=uri) From f5430f4a7f9a23cfb34686f8ab1cb5c04496c6f4 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 30 Dec 2013 21:47:02 +0100 Subject: [PATCH 18/23] local: Move --clear to it's own sub-command. Split library setup out into a helper and move the clear option to a command. --- docs/commands/mopidy.rst | 4 +++ mopidy/backends/local/commands.py | 53 ++++++++++++++++++++----------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/docs/commands/mopidy.rst b/docs/commands/mopidy.rst index 44e961e6..49c7b5b9 100644 --- a/docs/commands/mopidy.rst +++ b/docs/commands/mopidy.rst @@ -83,6 +83,10 @@ Additionally, extensions can provide extra commands. Run `mopidy --help` for a list of what is available on your system and command-specific help. Commands for disabled extensions will be listed, but can not be run. +.. cmdoption:: local clear + + Clear local media files from the local library. + .. cmdoption:: local scan Scan local media files present in your library. diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index 8eaf9d0d..e4075fc5 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -13,19 +13,49 @@ from . import translator logger = logging.getLogger('mopidy.backends.local.commands') +def _get_library(args, config): + libraries = dict((l.name, l) for l in args.registry['local:library']) + library_name = config['local']['library'] + + if library_name not in libraries: + logger.warning('Local library %s not found', library_name) + return 1 + + logger.debug('Using %s as the local library', library_name) + return libraries[library_name](config) + + class LocalCommand(commands.Command): def __init__(self): super(LocalCommand, self).__init__() self.add_child('scan', ScanCommand()) + self.add_child('clear', ClearCommand()) + + +class ClearCommand(commands.Command): + help = 'Clear local media files from the local library.' + + def run(self, args, config): + library = _get_library(args, config) + prompt = 'Are you sure you want to clear the library? [y/N] ' + + if raw_input(prompt).lower() != 'y': + logging.info('Clearing library aborted.') + return 0 + + if library.clear(): + logging.info('Library succesfully cleared.') + return 0 + + logging.warning('Unable to clear library.') + return 1 class ScanCommand(commands.Command): - help = "Scan local media files and populate the local library." + help = 'Scan local media files and populate the local library.' def __init__(self): super(ScanCommand, self).__init__() - self.add_argument('--clear', action='store_true', dest='clear', - help='Clear out library storage') self.add_argument('--limit', action='store', type=int, dest='limit', default=0, help='Maxmimum number of tracks to scan') @@ -37,22 +67,7 @@ class ScanCommand(commands.Command): excluded_file_extensions = set( file_ext.lower() for file_ext in excluded_file_extensions) - libraries = dict((l.name, l) for l in args.registry['local:library']) - library_name = config['local']['library'] - - if library_name not in libraries: - logger.warning('Local library %s not found', library_name) - return 1 - - library = libraries[library_name](config) - logger.debug('Using %s as the local library', library_name) - - if args.clear: - if library.clear(): - logging.info('Library succesfully cleared.') - return 0 - logging.warning('Unable to clear library.') - return 1 + library = _get_library(args, config) uri_path_mapping = {} uris_in_library = set() From e82ce6256d6b1dc2c1306b34d65e5c966e74561d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 31 Dec 2013 14:45:57 +0100 Subject: [PATCH 19/23] core: Re-add one uri scheme per backend constraint. --- mopidy/core/actor.py | 29 ++++++++++++++--------------- tests/core/actor_test.py | 24 +++--------------------- 2 files changed, 17 insertions(+), 36 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 4924cca2..dba8d76d 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -91,25 +91,24 @@ class Backends(list): self.with_playback = collections.OrderedDict() self.with_playlists = collections.OrderedDict() + backends_by_scheme = {} + name = lambda backend: backend.actor_ref.actor_class.__name__ + for backend in backends: has_library = backend.has_library().get() has_playback = backend.has_playback().get() has_playlists = backend.has_playlists().get() for scheme in backend.uri_schemes.get(): - self.add(self.with_library, has_library, scheme, backend) - self.add(self.with_playback, has_playback, scheme, backend) - self.add(self.with_playlists, has_playlists, scheme, backend) + assert scheme not in backends_by_scheme, ( + 'Cannot add URI scheme %s for %s, ' + 'it is already handled by %s' + ) % (scheme, name(backend), name(backends_by_scheme[scheme])) + backends_by_scheme[scheme] = backend - def add(self, registry, supported, uri_scheme, backend): - if not supported: - return - - if uri_scheme not in registry: - registry[uri_scheme] = backend - return - - get_name = lambda actor: actor.actor_ref.actor_class.__name__ - raise AssertionError( - 'Cannot add URI scheme %s for %s, it is already handled by %s' % - (uri_scheme, get_name(backend), get_name(registry[uri_scheme]))) + if has_library: + self.with_library[scheme] = backend + if has_playback: + self.with_playback[scheme] = backend + if has_playlists: + self.with_playlists[scheme] = backend diff --git a/tests/core/actor_test.py b/tests/core/actor_test.py index e9e5f396..4a808cad 100644 --- a/tests/core/actor_test.py +++ b/tests/core/actor_test.py @@ -13,9 +13,11 @@ class CoreActorTest(unittest.TestCase): def setUp(self): self.backend1 = mock.Mock() self.backend1.uri_schemes.get.return_value = ['dummy1'] + self.backend1.actor_ref.actor_class.__name__ = b'B1' self.backend2 = mock.Mock() self.backend2.uri_schemes.get.return_value = ['dummy2'] + self.backend2.actor_ref.actor_class.__name__ = b'B2' self.core = Core(audio=None, backends=[self.backend1, self.backend2]) @@ -29,32 +31,12 @@ class CoreActorTest(unittest.TestCase): self.assertIn('dummy2', result) def test_backends_with_colliding_uri_schemes_fails(self): - self.backend1.actor_ref.actor_class.__name__ = b'B1' - self.backend2.actor_ref.actor_class.__name__ = b'B2' self.backend2.uri_schemes.get.return_value = ['dummy1', 'dummy2'] + self.assertRaisesRegexp( AssertionError, 'Cannot add URI scheme dummy1 for B2, it is already handled by B1', Core, audio=None, backends=[self.backend1, self.backend2]) - def test_backends_with_colliding_uri_schemes_passes(self): - """ - Checks that backends with overlapping schemes, but distinct sub parts - provided can co-exist. - """ - - self.backend1.has_library().get.return_value = False - self.backend1.has_playlists().get.return_value = False - - self.backend2.uri_schemes.get.return_value = ['dummy1'] - self.backend2.has_playback().get.return_value = False - self.backend2.has_playlists().get.return_value = False - - core = Core(audio=None, backends=[self.backend1, self.backend2]) - self.assertEqual(core.backends.with_playback, - {'dummy1': self.backend1}) - self.assertEqual(core.backends.with_library, - {'dummy1': self.backend2}) - def test_version(self): self.assertEqual(self.core.version, versioning.get_version()) From 63a83754294de8f4d0ac4d5c8a862bdbf8555818 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 31 Dec 2013 14:57:54 +0100 Subject: [PATCH 20/23] local: Review comments and library interface update. Added return value to flush so we can log what is being done. --- docs/ext/local.rst | 2 +- mopidy/backends/local/__init__.py | 15 +++++++++------ mopidy/backends/local/commands.py | 9 ++++++--- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/docs/ext/local.rst b/docs/ext/local.rst index 4484ab0d..135a486b 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -61,7 +61,7 @@ Configuration values .. confval:: local/scan_flush_threshold Number of tracks to wait before telling library it should try and store - it's progress so far. Some libraries might not respect this setting. + its progress so far. Some libraries might not respect this setting. .. confval:: local/excluded_file_extensions diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index 6697d91d..d16eddfb 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -50,9 +50,10 @@ class Library(object): """ Local library interface. - Extensions that whish to provide an alternate local library storage backend - need to sub-class this class and install and confgure it with an extension. - Both scanning and library calls will use the active local library. + Extensions that wish to provide an alternate local library storage backend + need to sub-class this class and install and configure it with an + extension. Both scanning and library calls will use the active local + library. :param config: Config dictionary """ @@ -133,10 +134,12 @@ class Library(object): def flush(self): """ - Called for every n-th track indicating that work should be commited, - implementors are free to ignore these hints. + Called for every n-th track indicating that work should be comitted. + Sub-classes are free to ignore these hints. + + :rtype: Boolean indicating if state was flushed. """ - pass + return False def close(self): """ diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index e4075fc5..9dff43d5 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -56,8 +56,9 @@ class ScanCommand(commands.Command): def __init__(self): super(ScanCommand, self).__init__() - self.add_argument('--limit', action='store', type=int, dest='limit', - default=0, help='Maxmimum number of tracks to scan') + self.add_argument('--limit', + action='store', type=int, dest='limit', default=None, + help='Maxmimum number of tracks to scan') def run(self, args, config): media_dir = config['local']['media_dir'] @@ -114,7 +115,7 @@ class ScanCommand(commands.Command): total = args.limit or len(uris_to_update) start = time.time() - for uri in sorted(uris_to_update)[:args.limit or None]: + for uri in sorted(uris_to_update)[:args.limit]: try: data = scanner.scan(path.path_to_uri(uri_path_mapping[uri])) track = scan.audio_data_to_track(data).copy(uri=uri) @@ -129,6 +130,8 @@ class ScanCommand(commands.Command): remainder = duration / count * (total - count) logger.info('Scanned %d of %d files in %ds, ~%ds left.', count, total, duration, remainder) + # TODO: log if flush succeeded + # TODO: don't flush when count == total library.flush() library.close() From e880cb56b56bbffeac2ea4681009694f82c987ee Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 31 Dec 2013 15:09:54 +0100 Subject: [PATCH 21/23] local: Re-add progress helper and log flushes. Refactored the progress helper to be used in batching flushes in addition to logging. --- mopidy/backends/local/commands.py | 42 ++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/mopidy/backends/local/commands.py b/mopidy/backends/local/commands.py index 9dff43d5..08d6a942 100644 --- a/mopidy/backends/local/commands.py +++ b/mopidy/backends/local/commands.py @@ -110,12 +110,12 @@ class ScanCommand(commands.Command): logger.info('Found %d unknown tracks.', len(uris_to_update)) logger.info('Scanning...') - scanner = scan.Scanner(scan_timeout) - count = 0 - total = args.limit or len(uris_to_update) - start = time.time() + uris_to_update = sorted(uris_to_update)[:args.limit] - for uri in sorted(uris_to_update)[:args.limit]: + scanner = scan.Scanner(scan_timeout) + progress = _Progress(flush_threshold, len(uris_to_update)) + + for uri in uris_to_update: try: data = scanner.scan(path.path_to_uri(uri_path_mapping[uri])) track = scan.audio_data_to_track(data).copy(uri=uri) @@ -124,16 +124,30 @@ class ScanCommand(commands.Command): except exceptions.ScannerError as error: logger.warning('Failed %s: %s', uri, error) - count += 1 - if count % flush_threshold == 0 or count == total: - duration = time.time() - start - remainder = duration / count * (total - count) - logger.info('Scanned %d of %d files in %ds, ~%ds left.', - count, total, duration, remainder) - # TODO: log if flush succeeded - # TODO: don't flush when count == total - library.flush() + if progress.increment(): + progress.log() + if library.flush(): + logger.debug('Progress flushed.') + progress.log() library.close() logger.info('Done scanning.') return 0 + + +class _Progress(object): + def __init__(self, batch_size, total): + self.count = 0 + self.batch_size = batch_size + self.total = total + self.start = time.time() + + def increment(self): + self.count += 1 + return self.count % self.batch_size == 0 + + def log(self): + duration = time.time() - self.start + remainder = duration / self.count * (self.total - self.count) + logger.info('Scanned %d of %d files in %ds, ~%ds left.', + self.count, self.total, duration, remainder) From 29bb2d52b62c8a4d54978c392130f84f77357766 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 31 Dec 2013 15:31:09 +0100 Subject: [PATCH 22/23] local: Re-add test for #500 fix. --- mopidy/backends/local/json.py | 1 + tests/backends/local/library_test.py | 26 +++++++++++++------------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/mopidy/backends/local/json.py b/mopidy/backends/local/json.py index d1cacd6d..e4779ffe 100644 --- a/mopidy/backends/local/json.py +++ b/mopidy/backends/local/json.py @@ -52,6 +52,7 @@ class JsonLibrary(local.Library): config['local']['data_dir'], b'library.json.gz') def load(self): + logger.debug('Loading json library from %s', self._json_file) library = load_library(self._json_file) self._tracks = dict((t.uri, t) for t in library.get('tracks', [])) return len(self._tracks) diff --git a/tests/backends/local/library_test.py b/tests/backends/local/library_test.py index 40bece7d..4ca5abf0 100644 --- a/tests/backends/local/library_test.py +++ b/tests/backends/local/library_test.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals -import copy +import os +import shutil import tempfile import unittest @@ -89,31 +90,30 @@ class LocalLibraryProviderTest(unittest.TestCase): # Verifies that https://github.com/mopidy/mopidy/issues/500 # has been fixed. - # TODO: re-add something that tests this in a more sane way - return + tmpdir = tempfile.mkdtemp() + try: + tmplib = os.path.join(tmpdir, 'library.json.gz') + shutil.copy(path_to_data_dir('library.json.gz'), tmplib) - with tempfile.NamedTemporaryFile() as library: - with open(self.config['local-json']['json_file']) as fh: - library.write(fh.read()) - library.flush() - - config = copy.deepcopy(self.config) - config['local-json']['json_file'] = library.name + config = {'local': self.config['local'].copy()} + config['local']['data_dir'] = tmpdir backend = actor.LocalBackend(config=config, audio=None) # Sanity check that value is in the library result = backend.library.lookup(self.tracks[0].uri) self.assertEqual(result, self.tracks[0:1]) - # Clear library and refresh - library.seek(0) - library.truncate() + # Clear and refresh. + open(tmplib, 'w').close() backend.library.refresh() # Now it should be gone. result = backend.library.lookup(self.tracks[0].uri) self.assertEqual(result, []) + finally: + shutil.rmtree(tmpdir) + def test_lookup(self): tracks = self.library.lookup(self.tracks[0].uri) self.assertEqual(tracks, self.tracks[0:1]) From 08961cfcdd7e0005e9c5353058b6feb620e988b8 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 8 Jan 2014 23:47:24 +0100 Subject: [PATCH 23/23] docs: Update change log and local library info --- docs/changelog.rst | 15 +++++++++++++++ docs/ext/local.rst | 6 ++++++ mopidy/backends/local/__init__.py | 2 +- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 56177441..4c4792e1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,6 +15,12 @@ v0.18.0 (UNRELEASED) - Add :class:`mopidy.models.Ref` class for use as a lightweight reference to other model types, containing just an URI, a name, and an object type. +**Extension registry** + +- Switched to using a registry model for classes provided by extension. This + allows extensions to be extended as needed for plugable local libraries. + (Fixes :issue:`601`) + **Pluggable local libraries** Fixes issues :issue:`44`, partially resolves :issue:`397`, and causes @@ -33,6 +39,15 @@ a temporary regression of :issue:`527`. - Added support for deprecated config values in order to allow for graceful removal of :confval:`local/tag_cache_file`. +- Added :confval:`local/library` to select which library to use. + +- Added :confval:`local/data_dir` to have a common setting for where to store + local library data. This is intended to avoid every single local library + provider having to have it's own setting for this. + +- Added :confval:`local/scan_flush_threshold` to control how often to tell + local libraries to store changes. + **Streaming backend** - Live lookup of URI metadata has been added. (Fixes :issue:`540`) diff --git a/docs/ext/local.rst b/docs/ext/local.rst index 135a486b..4659678a 100644 --- a/docs/ext/local.rst +++ b/docs/ext/local.rst @@ -109,3 +109,9 @@ disable the current default library ``json``, replacing it with a third party one. When running :command:`mopidy local scan` mopidy will populate whatever the current active library is with data. Only one library may be active at a time. + +To create a new library provider you must create class that implements the +:class:`~mopidy.backends.local.Libary` interface and install it in the +extension registry under ``local:library``. Any data that the library needs +to store on disc should be stored in :confval:`local/data_dir` using the +library name as part of the filename or directory to avoid any conflicts. diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index d16eddfb..f34592b7 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -134,7 +134,7 @@ class Library(object): def flush(self): """ - Called for every n-th track indicating that work should be comitted. + Called for every n-th track indicating that work should be committed. Sub-classes are free to ignore these hints. :rtype: Boolean indicating if state was flushed.