From 999f4780106b9b27b15bb2c05468bdf80d43f131 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 16 Jan 2014 23:55:24 +0100 Subject: [PATCH] core: Update browse to use uri isntead of path --- mopidy/backend.py | 3 ++ mopidy/core/actor.py | 10 +++--- mopidy/core/library.py | 64 ++++++++++++-------------------------- tests/core/test_library.py | 36 ++++++++++----------- 4 files changed, 45 insertions(+), 68 deletions(-) diff --git a/mopidy/backend.py b/mopidy/backend.py index 552e4304..8105034a 100644 --- a/mopidy/backend.py +++ b/mopidy/backend.py @@ -37,6 +37,9 @@ class Backend(object): def has_library(self): return self.library is not None + def has_library_browse(self): + return self.has_library() and self.library.root_directory is not None + def has_playback(self): return self.playback is not None diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index b1f4700b..b27bb3cc 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -88,7 +88,7 @@ class Backends(list): super(Backends, self).__init__(backends) self.with_library = collections.OrderedDict() - self.with_browsable_library = collections.OrderedDict() + self.with_library_browse = collections.OrderedDict() self.with_playback = collections.OrderedDict() self.with_playlists = collections.OrderedDict() @@ -97,6 +97,7 @@ class Backends(list): for backend in backends: has_library = backend.has_library().get() + has_library_browse = backend.has_library_browse().get() has_playback = backend.has_playback().get() has_playlists = backend.has_playlists().get() @@ -109,12 +110,9 @@ class Backends(list): if has_library: self.with_library[scheme] = backend + if has_library_browse: + self.with_library_browse[scheme] = backend if has_playback: self.with_playback[scheme] = backend if has_playlists: self.with_playlists[scheme] = backend - - if has_library: - root_dir = backend.library.root_directory.get() - if root_dir is not None: - self.with_browsable_library[root_dir] = backend diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 2adf90b0..8eddfdbe 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -1,13 +1,10 @@ from __future__ import unicode_literals import collections -import re import urlparse import pykka -from mopidy.models import Ref - class LibraryController(object): pykka_traversable = True @@ -32,15 +29,16 @@ class LibraryController(object): (b, None) for b in self.backends.with_library.values()]) return backends_to_uris - def browse(self, path): + def browse(self, uri): """ - Browse directories and tracks at the given ``path``. + Browse directories and tracks at the given ``uri``. - ``path`` is a string that always starts with "/". It points to a - directory in Mopidy's virtual file system. + ``uri`` is a bytestring which represents some directory belonging to + a backend. To get the intial root directories for backends pass None + as the URI. Returns a list of :class:`mopidy.models.Ref` objects for the - directories and tracks at the given ``path``. + directories and tracks at the given ``uri``. The :class:`~mopidy.models.Ref` objects representing tracks keep the track's original URI. A matching pair of objects can look like this:: @@ -49,50 +47,28 @@ class LibraryController(object): Ref.track(uri='dummy:/foo.mp3', name='foo') The :class:`~mopidy.models.Ref` objects representing directories have - plain paths, not including any URI schema. For example, the dummy - library's ``/bar`` directory is returned like this:: + backend specific URIs. These are opaque values, so no one but the + backend that created them should try and derive any meaning from them. + The only valid exception to this is checking the scheme, as it is used + to route browse requests to the correct backend. - Ref.directory(uri='/dummy/bar', name='bar') + For example, the dummy library's ``/bar`` directory could bereturned + like this:: - Note to backend implementors: The ``/dummy`` part of the URI is added - by Mopidy core, not the individual backends. + Ref.directory(uri='dummy:directory:/bar', name='bar') - :param path: path to browse - :type path: string + :param bytestring uri: uri to browse :rtype: list of :class:`mopidy.models.Ref` """ - if not path.startswith('/'): - return [] + if uri is None: + backends = self.backends.with_library_browse.values() + return [b.library.root_directory.get() for b in backends] - mapping = {} - for ref in self.backends.with_browsable_library.keys(): - name = urlparse.urlparse(ref.uri).scheme - mapping[name] = ref - - if path == '/': - return [Ref.directory(uri='/%s' % name, name=name) - for name in sorted(mapping)] - - groups = re.match('/(?P[^/]+)(?P.*)', path).groupdict() - library_name = groups['library'] - backend_path = groups['path'] - if not backend_path.startswith('/'): - backend_path = '/%s' % backend_path - - backend = self.backends.with_browsable_library.get( - mapping.get(library_name), None) + scheme = urlparse.urlparse(uri).scheme + backend = self.backends.with_library_browse.get(scheme) if not backend: return [] - - refs = backend.library.browse(backend_path).get() - result = [] - for ref in refs: - if ref.type == Ref.DIRECTORY: - uri = '/'.join(['', library_name, ref.uri.lstrip('/')]) - result.append(ref.copy(uri=uri)) - else: - result.append(ref) - return result + return backend.library.browse(uri).get() def find_exact(self, query=None, uris=None, **kwargs): """ diff --git a/tests/core/test_library.py b/tests/core/test_library.py index 3cf7facf..7a40194d 100644 --- a/tests/core/test_library.py +++ b/tests/core/test_library.py @@ -27,16 +27,17 @@ class CoreLibraryTest(unittest.TestCase): self.backend3 = mock.Mock() self.backend3.uri_schemes.get.return_value = ['dummy3'] self.backend3.has_library().get.return_value = False + self.backend3.has_library_browse().get.return_value = False self.core = core.Core(audio=None, backends=[ self.backend1, self.backend2, self.backend3]) def test_browse_root_returns_dir_ref_for_each_lib_with_root_dir_name(self): - result = self.core.library.browse('/') + result = self.core.library.browse(None) self.assertEqual(result, [ - Ref.directory(uri='/dummy1', name='dummy1'), - Ref.directory(uri='/dummy2', name='dummy2'), + Ref.directory(uri='dummy1:directory', name='dummy1'), + Ref.directory(uri='dummy2:directory', name='dummy2'), ]) self.assertFalse(self.library1.browse.called) self.assertFalse(self.library2.browse.called) @@ -51,32 +52,32 @@ class CoreLibraryTest(unittest.TestCase): def test_browse_dummy1_selects_dummy1_backend(self): self.library1.browse().get.return_value = [ - Ref.directory(uri='/foo/bar', name='bar'), - Ref.track(uri='dummy1:/foo/baz.mp3', name='Baz'), + Ref.directory(uri='dummy1:directory:/foo/bar', name='bar'), + Ref.track(uri='dummy1:track:/foo/baz.mp3', name='Baz'), ] self.library1.browse.reset_mock() - self.core.library.browse('/dummy1/foo') + self.core.library.browse('dummy1:directory:/foo') self.assertEqual(self.library1.browse.call_count, 1) self.assertEqual(self.library2.browse.call_count, 0) - self.library1.browse.assert_called_with('/foo') + self.library1.browse.assert_called_with('dummy1:directory:/foo') def test_browse_dummy2_selects_dummy2_backend(self): self.library2.browse().get.return_value = [ - Ref.directory(uri='/bar/quux', name='quux'), - Ref.track(uri='dummy2:/foo/baz.mp3', name='Baz'), + Ref.directory(uri='dummy2:directory:/bar/baz', name='quux'), + Ref.track(uri='dummy2:track:/bar/foo.mp3', name='Baz'), ] self.library2.browse.reset_mock() - self.core.library.browse('/dummy2/bar') + self.core.library.browse('dummy2:directory:/bar') self.assertEqual(self.library1.browse.call_count, 0) self.assertEqual(self.library2.browse.call_count, 1) - self.library2.browse.assert_called_with('/bar') + self.library2.browse.assert_called_with('dummy2:directory:/bar') def test_browse_dummy3_returns_nothing(self): - result = self.core.library.browse('/dummy3') + result = self.core.library.browse('dummy3:test') self.assertEqual(result, []) self.assertEqual(self.library1.browse.call_count, 0) @@ -84,16 +85,15 @@ class CoreLibraryTest(unittest.TestCase): def test_browse_dir_returns_subdirs_and_tracks(self): self.library1.browse().get.return_value = [ - Ref.directory(uri='/foo/bar', name='bar'), - Ref.track(uri='dummy1:/foo/baz.mp3', name='Baz'), + Ref.directory(uri='dummy1:directory:/foo/bar', name='Bar'), + Ref.track(uri='dummy1:track:/foo/baz.mp3', name='Baz'), ] self.library1.browse.reset_mock() - result = self.core.library.browse('/dummy1/foo') - + result = self.core.library.browse('dummy1:directory:/foo') self.assertEqual(result, [ - Ref.directory(uri='/dummy1/foo/bar', name='bar'), - Ref.track(uri='dummy1:/foo/baz.mp3', name='Baz'), + Ref.directory(uri='dummy1:directory:/foo/bar', name='Bar'), + Ref.track(uri='dummy1:track:/foo/baz.mp3', name='Baz'), ]) def test_lookup_selects_dummy1_backend(self):