core: Mark library.lookup by uri deprecated

Updates core, mpd and tests to not use deprecated calls or safely catch them
when running with -W error.

mopidy/core/library.py

@@ -162,6 +162,10 @@ class LibraryController(object):
         if none_set or both_set:
             raise ValueError("One of 'uri' or 'uris' must be set")
 
+        if uri:
+            warnings.warn('library.lookup() "uri" argument is deprecated.',
+                          DeprecationWarning)
+
         if uri is not None:
             uris = [uri]
 

mopidy/core/tracklist.py

@@ -334,12 +334,12 @@ class TracklistController(object):
 
         if tracks is None:
             if uri is not None:
-                tracks = self.core.library.lookup(uri=uri)
+                uris = [uri]
-            elif uris is not None:
+
-                tracks = []
+            tracks = []
-                track_map = self.core.library.lookup(uris=uris)
+            track_map = self.core.library.lookup(uris=uris)
-                for uri in uris:
+            for uri in uris:
-                    tracks.extend(track_map[uri])
+                tracks.extend(track_map[uri])
 
         tl_tracks = []
 

mopidy/mpd/dispatcher.py

@@ -267,10 +267,10 @@ class MpdContext(object):
         given path.
 
         If ``lookup`` is true and the ``path`` is to a track, the returned
-        ``data`` is a future which will contain the
+        ``data`` is a future which will contain the results from looking up
-        :class:`mopidy.models.Track` model. If ``lookup`` is false and the
+        the URI with :meth:`mopidy.core.LibraryController.lookup` If ``lookup``
-        ``path`` is to a track, the returned ``data`` will be a
+        is false and the ``path`` is to a track, the returned ``data`` will be
-        :class:`mopidy.models.Ref` for the track.
+        a :class:`mopidy.models.Ref` for the track.
 
         For all entries that are not tracks, the returned ``data`` will be
         :class:`None`.
@@ -302,7 +302,8 @@ class MpdContext(object):
 
                 if ref.type == ref.TRACK:
                     if lookup:
-                        yield (path, self.core.library.lookup(ref.uri))
+                        # TODO: can we lookup all the refs at once now?
+                        yield (path, self.core.library.lookup(uris=[ref.uri]))
                     else:
                         yield (path, ref)
                 else:

mopidy/mpd/protocol/current_playlist.py

@@ -29,7 +29,8 @@ def add(context, uri):
         tracks = []
         for path, lookup_future in context.browse(uri):
             if lookup_future:
-                tracks.extend(lookup_future.get())
+                for result in lookup_future.get().values():
+                    tracks.extend(result)
     except exceptions.MpdNoExistError as e:
         e.message = 'directory or file not found'
         raise

mopidy/mpd/protocol/music_db.py

@@ -331,8 +331,9 @@ def listallinfo(context, uri=None):
         if not lookup_future:
             result.append(('directory', path))
         else:
-            for track in lookup_future.get():
+            for uri, tracks in lookup_future.get().items():
-                result.extend(translator.track_to_mpd_format(track))
+                for track in tracks:
+                    result.extend(translator.track_to_mpd_format(track))
     return result
 
 
@@ -358,9 +359,9 @@ def lsinfo(context, uri=None):
         if not lookup_future:
             result.append(('directory', path.lstrip('/')))
         else:
-            tracks = lookup_future.get()
+            for uri, tracks in lookup_future.get().items():
-            if tracks:
+                if tracks:
-                result.extend(translator.track_to_mpd_format(tracks[0]))
+                    result.extend(translator.track_to_mpd_format(tracks[0]))
 
     if uri in (None, '', '/'):
         result.extend(protocol.stored_playlists.listplaylists(context))

tests/core/test_library.py

@@ -149,18 +149,6 @@ class CoreLibraryTest(BaseCoreLibraryTest):
             Ref.track(uri='dummy1:track:/foo/baz.mp3', name='Baz'),
         ])
 
-    def test_lookup_selects_dummy1_backend(self):
-        self.core.library.lookup('dummy1:a')
-
-        self.library1.lookup.assert_called_once_with('dummy1:a')
-        self.assertFalse(self.library2.lookup.called)
-
-    def test_lookup_selects_dummy2_backend(self):
-        self.core.library.lookup('dummy2:a')
-
-        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'])
@@ -172,13 +160,6 @@ class CoreLibraryTest(BaseCoreLibraryTest):
         result = self.core.library.lookup(uris=['dummy1:a', 'dummy2:a'])
         self.assertEqual(result, {'dummy2:a': [5678], 'dummy1:a': [1234]})
 
-    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'])
 
@@ -378,6 +359,37 @@ class DeprecatedFindExactCoreLibraryTest(BaseCoreLibraryTest):
             query={'any': ['foobar']}, uris=None, exact=True)
 
 
+class DeprecatedLookupCoreLibraryTest(BaseCoreLibraryTest):
+    def setUp(self):  # noqa: N802
+        super(DeprecatedLookupCoreLibraryTest, self).setUp()
+        self._warnings_filters = warnings.filters
+        warnings.filters = warnings.filters[:]
+        warnings.filterwarnings('ignore', 'library.lookup.*"uri" argument.*')
+
+    def tearDown(self):  # noqa: N802
+        super(DeprecatedLookupCoreLibraryTest, self).tearDown()
+        warnings.filters = self._warnings_filters
+
+    def test_lookup_selects_dummy1_backend(self):
+        self.core.library.lookup('dummy1:a')
+
+        self.library1.lookup.assert_called_once_with('dummy1:a')
+        self.assertFalse(self.library2.lookup.called)
+
+    def test_lookup_selects_dummy2_backend(self):
+        self.core.library.lookup('dummy2:a')
+
+        self.assertFalse(self.library1.lookup.called)
+        self.library2.lookup.assert_called_once_with('dummy2:a')
+
+    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)
+
+
 class LegacyFindExactToSearchLibraryTest(unittest.TestCase):
     def setUp(self):  # noqa: N802
         self.backend = mock.Mock()

tests/local/test_library.py

@@ -128,12 +128,13 @@ class LocalLibraryProviderTest(unittest.TestCase):
         pass  # TODO
 
     def test_lookup(self):
-        tracks = self.library.lookup(self.tracks[0].uri)
+        uri = self.tracks[0].uri
-        self.assertEqual(tracks, self.tracks[0:1])
+        result = self.library.lookup(uris=[uri])
+        self.assertEqual(result[uri], self.tracks[0:1])
 
     def test_lookup_unknown_track(self):
-        tracks = self.library.lookup('fake uri')
+        tracks = self.library.lookup(uris=['fake uri'])
-        self.assertEqual(tracks, [])
+        self.assertEqual(tracks, {'fake uri': []})
 
     # test backward compatibility with local libraries returning a
     # single Track