Web API playlists v3

mopidy_spotify/lookup.py

@@ -21,7 +21,7 @@ def lookup(config, session, web_client, uri):
 
     try:
         if web_link.type == "playlist":
-            return _lookup_playlist(config, web_client, uri)
+            return _lookup_playlist(config, session, web_client, uri)
         elif sp_link.type is spotify.LinkType.TRACK:
             return list(_lookup_track(config, sp_link))
         elif sp_link.type is spotify.LinkType.ALBUM:
@@ -85,8 +85,10 @@ def _lookup_artist(config, sp_link):
                 yield track
 
 
-def _lookup_playlist(config, web_client, uri):
-    playlist = playlists.playlist_lookup(web_client, uri, config["bitrate"])
+def _lookup_playlist(config, session, web_client, uri):
+    playlist = playlists.playlist_lookup(
+        session, web_client, uri, config["bitrate"]
+    )
     if playlist is None:
         raise spotify.Error("Playlist Web API lookup failed")
     return playlist.tracks

mopidy_spotify/playlists.py

@@ -2,9 +2,12 @@
 
 from mopidy import backend
 
+import spotify
+
 from mopidy_spotify import translator, utils
 
 _cache = {}
+_sp_links = {}
 
 logger = logging.getLogger(__name__)
 
@@ -50,12 +53,18 @@ def lookup(self, uri):
 
     def _get_playlist(self, uri, as_items=False):
         return playlist_lookup(
-            self._backend._web_client, uri, self._backend._bitrate, as_items
+            self._backend._session,
+            self._backend._web_client,
+            uri,
+            self._backend._bitrate,
+            as_items,
         )
 
     def refresh(self):
         with utils.time_logger("Refresh Playlists", logging.INFO):
             _cache.clear()
+            _sp_links.clear()
+            # Want libspotify to get track links so they load in the background
             count = 0
             for playlist_ref in self._get_flattened_playlist_refs():
                 self._get_playlist(playlist_ref.uri)
@@ -74,7 +83,7 @@ def save(self, playlist):
         pass  # TODO
 
 
-def playlist_lookup(web_client, uri, bitrate, as_items=False):
+def playlist_lookup(session, web_client, uri, bitrate, as_items=False):
     if web_client is None:
         return
 
@@ -85,12 +94,31 @@ def playlist_lookup(web_client, uri, bitrate, as_items=False):
         logger.error(f"Failed to lookup Spotify playlist URI {uri}")
         return
 
-    return translator.to_playlist(
+    playlist = translator.to_playlist(
         web_playlist,
         username=web_client.user_id,
         bitrate=bitrate,
         as_items=as_items,
     )
+    if playlist is None:
+        return
+    # Store the libspotify Link for each track so they will be loaded in the
+    # background ready for using later.
+    if session.connection.state is spotify.ConnectionState.LOGGED_IN:
+        if as_items:
+            tracks = playlist
+        else:
+            tracks = playlist.tracks
+
+        for track in tracks:
+            if track.uri in _sp_links:
+                continue
+            try:
+                _sp_links[track.uri] = session.get_link(track.uri)
+            except ValueError as exc:
+                logger.info(f'Failed to get link "{track.uri}": {exc}')
+
+    return playlist
 
 
 def on_playlists_loaded():

tests/test_lookup.py

@@ -161,13 +161,14 @@ def test_lookup_of_artist_uri_ignores_various_artists_albums(
 
 
 def test_lookup_of_playlist_uri(
-    session_mock, web_client_mock, web_playlist_mock, provider
+    session_mock, web_client_mock, web_playlist_mock, sp_track_mock, provider
 ):
     web_client_mock.get_playlist.return_value = web_playlist_mock
+    session_mock.get_link.return_value = sp_track_mock.link
 
     results = provider.lookup("spotify:playlist:alice:foo")
 
-    session_mock.get_link.assert_not_called()
+    session_mock.get_link.assert_called_once_with("spotify:track:abc")
     web_client_mock.get_playlist.assert_called_once_with(
         "spotify:playlist:alice:foo", {}
     )

tests/test_playlists.py

@@ -1,6 +1,7 @@
 from unittest import mock
 
 import pytest
+import spotify
 from mopidy import backend as backend_api
 from mopidy.models import Ref
 
@@ -43,6 +44,8 @@ def get_playlist(*args, **kwargs):
 @pytest.fixture
 def provider(backend_mock, web_client_mock):
     backend_mock._web_client = web_client_mock
+    playlists._cache.clear()
+    playlists._sp_links.clear()
     provider = playlists.SpotifyPlaylistsProvider(backend_mock)
     provider._loaded = True
     return provider
@@ -169,6 +172,15 @@ def test_refresh_clears_web_cache(provider):
     assert len(playlists._cache) == 0
 
 
+def test_refresh_clears_link_cache(provider):
+    playlists._sp_links = {"bar": "foobar", "bar2": "foofoo"}
+
+    provider.refresh()
+
+    assert len(playlists._sp_links) == 1
+    assert list(playlists._sp_links.keys()) == ["spotify:track:abc"]
+
+
 def test_lookup(provider):
     playlist = provider.lookup("spotify:user:alice:playlist:foo")
 
@@ -198,8 +210,90 @@ def test_lookup_of_playlist_with_other_owner(provider):
     assert playlist.name == "Baz (by bob)"
 
 
-def test_lookup_uses_cache(provider, web_client_mock):
-    provider.lookup("spotify:user:alice:playlist:foo")
+@pytest.mark.parametrize("as_items", [(False), (True)])
+def test_playlist_lookup_stores_track_link(
+    session_mock,
+    web_client_mock,
+    sp_track_mock,
+    web_playlist_mock,
+    web_track_mock,
+    as_items,
+):
+    session_mock.get_link.return_value = sp_track_mock.link
+    web_playlist_mock["tracks"]["items"] = [{"track": web_track_mock}] * 5
+    web_client_mock.get_playlist.return_value = web_playlist_mock
+    playlists._sp_links.clear()
+
+    playlists.playlist_lookup(
+        session_mock,
+        web_client_mock,
+        "spotify:user:alice:playlist:foo",
+        None,
+        as_items,
+    )
+
+    session_mock.get_link.assert_called_once_with("spotify:track:abc")
+    assert len(playlists._sp_links) == 1
+
+
+@pytest.mark.parametrize(
+    "connection_state",
+    [
+        (spotify.ConnectionState.OFFLINE),
+        (spotify.ConnectionState.DISCONNECTED),
+        (spotify.ConnectionState.LOGGED_OUT),
+    ],
+)
+def test_playlist_lookup_when_not_logged_in(
+    session_mock, web_client_mock, web_playlist_mock, connection_state
+):
+    web_client_mock.get_playlist.return_value = web_playlist_mock
+    session_mock.connection.state = connection_state
+    playlists._sp_links.clear()
+
+    playlist = playlists.playlist_lookup(
+        session_mock, web_client_mock, "spotify:user:alice:playlist:foo", None
+    )
+
+    assert playlist.uri == "spotify:user:alice:playlist:foo"
+    assert playlist.name == "Foo"
+    assert len(playlists._sp_links) == 0
+
+
+def test_playlist_lookup_when_playlist_is_empty(
+    session_mock, web_client_mock, caplog
+):
+    web_client_mock.get_playlist.return_value = {}
+    playlists._sp_links.clear()
+
+    playlist = playlists.playlist_lookup(
+        session_mock, web_client_mock, "nothing", None
+    )
+
+    assert playlist is None
+    assert "Failed to lookup Spotify playlist URI nothing" in caplog.text
+    assert len(playlists._sp_links) == 0
+
+
+def test_playlist_lookup_when_link_invalid(
+    session_mock, web_client_mock, web_playlist_mock, caplog
+):
+    session_mock.get_link.side_effect = ValueError("an error message")
+    web_client_mock.get_playlist.return_value = web_playlist_mock
+    playlists._sp_links.clear()
+
+    playlist = playlists.playlist_lookup(
+        session_mock, web_client_mock, "spotify:user:alice:playlist:foo", None
+    )
+
+    assert len(playlist.tracks) == 1
+    assert 'Failed to get link "spotify:track:abc"' in caplog.text
+
+
+def test_playlist_lookup_uses_cache(session_mock, web_client_mock):
+    playlists.playlist_lookup(
+        session_mock, web_client_mock, "spotify:user:alice:playlist:foo", None
+    )
 
     web_client_mock.get_playlist.assert_called_once_with(
         "spotify:user:alice:playlist:foo", playlists._cache