Merge pull request #1065 from jodal/fix/1060-tracklist-add-uris

core: Add uris kwarg to tracklist.add()

docs/changelog.rst

@@ -36,6 +36,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`)
 

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):

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 = []
 

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')
 

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'])