diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 9aaa386e..38e231f1 100644 --- a/mopidy/core/library.py +++ b/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] diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 9186ae42..51ce7fbf 100644 --- a/mopidy/core/tracklist.py +++ b/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) - elif uris is not None: - tracks = [] - track_map = self.core.library.lookup(uris=uris) - for uri in uris: - tracks.extend(track_map[uri]) + uris = [uri] + + tracks = [] + track_map = self.core.library.lookup(uris=uris) + for uri in uris: + tracks.extend(track_map[uri]) tl_tracks = [] diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py index d156b891..4d1c6196 100644 --- a/mopidy/mpd/dispatcher.py +++ b/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 - :class:`mopidy.models.Track` model. If ``lookup`` is false and the - ``path`` is to a track, the returned ``data`` will be a - :class:`mopidy.models.Ref` for the track. + ``data`` is a future which will contain the results from looking up + the URI with :meth:`mopidy.core.LibraryController.lookup` If ``lookup`` + is false and the ``path`` is to a track, the returned ``data`` will be + 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: diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py index d8e1a9d8..ccf6f788 100644 --- a/mopidy/mpd/protocol/current_playlist.py +++ b/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 diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index 59a1f9b6..644da88d 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/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(): - result.extend(translator.track_to_mpd_format(track)) + for uri, tracks in lookup_future.get().items(): + 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() - if tracks: - result.extend(translator.track_to_mpd_format(tracks[0])) + for uri, tracks in lookup_future.get().items(): + if tracks: + result.extend(translator.track_to_mpd_format(tracks[0])) if uri in (None, '', '/'): result.extend(protocol.stored_playlists.listplaylists(context)) diff --git a/tests/core/test_library.py b/tests/core/test_library.py index bebdd8a6..bdefd8f0 100644 --- a/tests/core/test_library.py +++ b/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() diff --git a/tests/local/test_library.py b/tests/local/test_library.py index 7ab67fa6..dfab2c89 100644 --- a/tests/local/test_library.py +++ b/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) - self.assertEqual(tracks, self.tracks[0:1]) + uri = self.tracks[0].uri + 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') - self.assertEqual(tracks, []) + tracks = self.library.lookup(uris=['fake uri']) + self.assertEqual(tracks, {'fake uri': []}) # test backward compatibility with local libraries returning a # single Track