From f67e55618cbbd9c121520df5402c7d664ec4e120 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 21 Mar 2015 00:11:15 +0100 Subject: [PATCH 1/2] core: Make lookup(uris=...) return dict with all uris All uris given to lookup should be in the result even if there is no backend to handle the uri, and the lookup result thus is an empty list. As a side effect, future.get() is now called in the order of the URIs in the `uris` list, making it easier to mock out backend.library.lookup() in core layer tests. --- mopidy/core/library.py | 14 ++++++++++---- tests/core/test_library.py | 9 ++++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index f2a8b9bd..b8018b16 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -188,20 +188,26 @@ class LibraryController(object): if none_set or both_set: raise ValueError("One of 'uri' or 'uris' must be set") + if uri is not None: + uris = [uri] + futures = {} result = {} - backends = self._get_backends_to_uris([uri] if uri else uris) + backends = self._get_backends_to_uris(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() + for u in uris: + if u in futures: + result[u] = futures[u].get() + else: + result[u] = [] if uri: - return result.get(uri, []) + return result[uri] return result def refresh(self, uri=None): diff --git a/tests/core/test_library.py b/tests/core/test_library.py index b71e5de5..9eacd1a2 100644 --- a/tests/core/test_library.py +++ b/tests/core/test_library.py @@ -168,13 +168,20 @@ class CoreLibraryTest(unittest.TestCase): 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): + def test_lookup_uri_returns_empty_list_for_dummy3_track(self): result = self.core.library.lookup('dummy3:a') self.assertEqual(result, []) self.assertFalse(self.library1.lookup.called) self.assertFalse(self.library2.lookup.called) + def test_lookup_uris_returns_empty_list_for_dummy3_track(self): + result = self.core.library.lookup(uris=['dummy3:a']) + + self.assertEqual(result, {'dummy3:a': []}) + self.assertFalse(self.library1.lookup.called) + self.assertFalse(self.library2.lookup.called) + def test_refresh_with_uri_selects_dummy1_backend(self): self.core.library.refresh('dummy1:a') From 2bc3db0d0e95e8f5ba90f5fa39b616009427a0f6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 21 Mar 2015 00:16:22 +0100 Subject: [PATCH 2/2] core: Add uris kwarg to tracklist.core() Fixes #1060 --- docs/changelog.rst | 4 ++++ mopidy/core/tracklist.py | 27 +++++++++++++++++++++------ tests/core/test_tracklist.py | 23 +++++++++++++++++++++-- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d63ce7b8..589178e0 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -35,6 +35,10 @@ v1.0.0 (UNRELEASED) which allows for simpler lookup of multiple URIs. (Fixes: :issue:`1008`, PR: :issue:`1047`) +- Add ``uris`` argument to :method:`mopidy.core.TracklistController.add` + which allows for simpler addition of multiple URIs to the tracklist. (Fixes: + :issue:`1060`, PR: :issue:`1065`) + - **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/tracklist.py b/mopidy/core/tracklist.py index 456bddf6..963dcadf 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -299,13 +299,16 @@ class TracklistController(object): return self.get_tl_tracks()[position - 1] - def add(self, tracks=None, at_position=None, uri=None): + def add(self, tracks=None, at_position=None, uri=None, uris=None): """ Add the track or list of tracks to the tracklist. If ``uri`` is given instead of ``tracks``, the URI is looked up in the library and the resulting tracks are added to the tracklist. + If ``uris`` is given instead of ``tracks``, the URIs are looked up in + the library and the resulting tracks are added to the tracklist. + If ``at_position`` is given, the tracks placed at the given position in the tracklist. If ``at_position`` is not given, the tracks are appended to the end of the tracklist. @@ -319,12 +322,24 @@ class TracklistController(object): :param uri: URI for tracks to add :type uri: string :rtype: list of :class:`mopidy.models.TlTrack` - """ - assert tracks is not None or uri is not None, \ - 'tracks or uri must be provided' - if tracks is None and uri is not None: - tracks = self.core.library.lookup(uri) + .. versionadded:: 1.0 + The ``uris`` argument. + + .. deprecated:: 1.0 + The ``tracks`` and ``uri`` arguments. Use ``uris``. + """ + assert tracks is not None or uri is not None or uris is not None, \ + 'tracks, uri or uris must be provided' + + if tracks is None: + if uri is not None: + tracks = self.core.library.lookup(uri=uri) + elif uris is not None: + tracks = [] + track_map = self.core.library.lookup(uris=uris) + for uri in uris: + tracks.extend(track_map[uri]) tl_tracks = [] diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 7b5577f9..415d1fa0 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -26,8 +26,7 @@ class TracklistTest(unittest.TestCase): def test_add_by_uri_looks_up_uri_in_library(self): track = Track(uri='dummy1:x', name='x') - self.library.lookup().get.return_value = [track] - self.library.lookup.reset_mock() + self.library.lookup.return_value.get.return_value = [track] tl_tracks = self.core.tracklist.add(uri='dummy1:x') @@ -36,6 +35,26 @@ class TracklistTest(unittest.TestCase): self.assertEqual(track, tl_tracks[0].track) self.assertEqual(tl_tracks, self.core.tracklist.tl_tracks[-1:]) + def test_add_by_uris_looks_up_uris_in_library(self): + track1 = Track(uri='dummy1:x', name='x') + track2 = Track(uri='dummy1:y1', name='y1') + track3 = Track(uri='dummy1:y2', name='y2') + self.library.lookup.return_value.get.side_effect = [ + [track1], [track2, track3]] + + tl_tracks = self.core.tracklist.add(uris=['dummy1:x', 'dummy1:y']) + + self.library.lookup.assert_has_calls([ + mock.call('dummy1:x'), + mock.call('dummy1:y'), + ]) + self.assertEqual(3, len(tl_tracks)) + self.assertEqual(track1, tl_tracks[0].track) + self.assertEqual(track2, tl_tracks[1].track) + self.assertEqual(track3, tl_tracks[2].track) + self.assertEqual( + tl_tracks, self.core.tracklist.tl_tracks[-len(tl_tracks):]) + def test_remove_removes_tl_tracks_matching_query(self): tl_tracks = self.core.tracklist.remove(name=['foo'])