From fdc84c3905dc7b0d69414b9a1adcab6587b60fa7 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 17 Mar 2015 23:41:09 +0100 Subject: [PATCH 1/3] core: Add uris argument to library.lookup (Fixes #1008) For now this doesn't add any corresponding APIs to backends, or for that matter tracklist.add(uris). This is just to get the API in for clients in 0.20. --- docs/changelog.rst | 3 +++ mopidy/core/library.py | 35 +++++++++++++++++++++++++++-------- tests/core/test_library.py | 11 +++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 33bfc9f7..33468f60 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -31,6 +31,9 @@ v0.20.0 (UNRELEASED) - Add :class:`mopidy.core.MixerController` which keeps track of volume and mute. (Fixes: :issue:`962`) +- Add ``uris`` argument to :method:`mopidy.core.LibraryController.lookup` + which allows for simpler lookup of multiple URIs. (Fixes: :issue:`1008`) + - **Deprecated:** The old methods on :class:`mopidy.core.PlaybackController` for volume and mute management have been deprecated. (Fixes: :issue:`962`) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 49a4a796..906618d8 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -162,7 +162,7 @@ class LibraryController(object): in self._get_backends_to_uris(uris).items()] return [result for result in pykka.get_all(futures) if result] - def lookup(self, uri): + def lookup(self, uri=None, uris=None): """ Lookup the given URI. @@ -170,14 +170,33 @@ class LibraryController(object): them all. :param uri: track URI - :type uri: string - :rtype: list of :class:`mopidy.models.Track` + :type uri: string or :class:`None` + :param uris: track URIs + :type uris: list of string or :class:`None` + :rtype: list of :class:`mopidy.models.Track` if uri was set or a + ``{uri: list of tracks}`` if uris was set. """ - backend = self._get_backend(uri) - if backend: - return backend.library.lookup(uri).get() - else: - return [] + none_set = uri is None and uris is None + both_set = uri is not None and uris is not None + + if none_set or both_set: + raise ValueError("One of 'uri' or 'uris' must be set") + + futures = {} + result = {} + backends = self._get_backends_to_uris([uri] if uri else uris) + + # TODO: lookup(uris) to backend APIs + for backend, backend_uris in backends.items(): + for u in backend_uris or []: + futures[u] = backend.library.lookup(u) + + for u, future in futures.items(): + result[u] = future.get() + + if uri: + return result.get(uri, []) + return result def refresh(self, uri=None): """ diff --git a/tests/core/test_library.py b/tests/core/test_library.py index ccf1b349..b71e5de5 100644 --- a/tests/core/test_library.py +++ b/tests/core/test_library.py @@ -157,6 +157,17 @@ class CoreLibraryTest(unittest.TestCase): self.assertFalse(self.library1.lookup.called) self.library2.lookup.assert_called_once_with('dummy2:a') + def test_lookup_fails_with_uri_and_uris_set(self): + with self.assertRaises(ValueError): + self.core.library.lookup('dummy1:a', ['dummy2:a']) + + def test_lookup_can_handle_uris(self): + self.library1.lookup().get.return_value = [1234] + self.library2.lookup().get.return_value = [5678] + + result = self.core.library.lookup(uris=['dummy1:a', 'dummy2:a']) + self.assertEqual(result, {'dummy2:a': [5678], 'dummy1:a': [1234]}) + def test_lookup_returns_nothing_for_dummy3_track(self): result = self.core.library.lookup('dummy3:a') From 08bdf5c14bc7790be31586cc1c32852fedb6a777 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 18 Mar 2015 00:10:45 +0100 Subject: [PATCH 2/3] core: Update library.lookup() docstring --- mopidy/core/library.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 906618d8..ec94dccf 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -173,8 +173,13 @@ class LibraryController(object): :type uri: string or :class:`None` :param uris: track URIs :type uris: list of string or :class:`None` - :rtype: list of :class:`mopidy.models.Track` if uri was set or a - ``{uri: list of tracks}`` if uris was set. + :rtype: {uri: list of :class:`mopidy.models.Track`} + + .. versionadded:: 0.20 + The ``uris`` argument. + + .. deprecated:: 0.20 + The ``uri`` argument. Use ``uris`` instead. """ none_set = uri is None and uris is None both_set = uri is not None and uris is not None From 71e2b21b5204b78d2b1f2f220cbce56037532bd4 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 18 Mar 2015 23:09:09 +0100 Subject: [PATCH 3/3] review: Minor fixes and updates --- docs/changelog.rst | 3 ++- mopidy/core/library.py | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 33468f60..f1a33c4b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -32,7 +32,8 @@ v0.20.0 (UNRELEASED) mute. (Fixes: :issue:`962`) - Add ``uris`` argument to :method:`mopidy.core.LibraryController.lookup` - which allows for simpler lookup of multiple URIs. (Fixes: :issue:`1008`) + which allows for simpler lookup of multiple URIs. (Fixes: :issue:`1008`, + PR: :issue:`1047`) - **Deprecated:** The old methods on :class:`mopidy.core.PlaybackController` for volume and mute management have been deprecated. (Fixes: :issue:`962`) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index ec94dccf..f2a8b9bd 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -173,12 +173,13 @@ class LibraryController(object): :type uri: string or :class:`None` :param uris: track URIs :type uris: list of string or :class:`None` - :rtype: {uri: list of :class:`mopidy.models.Track`} + :rtype: list of :class:`mopidy.models.Track` if uri was set or + a {uri: list of :class:`mopidy.models.Track`} if uris was set. - .. versionadded:: 0.20 + .. versionadded:: 1.0 The ``uris`` argument. - .. deprecated:: 0.20 + .. deprecated:: 1.0 The ``uri`` argument. Use ``uris`` instead. """ none_set = uri is None and uris is None