Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web API playlists v3 #235

Merged
merged 15 commits into from
Dec 12, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions mopidy_spotify/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ def on_start(self):
)
self._web_client.login()

if self.playlists is not None:
self.playlists.refresh()

def on_stop(self):
logger.debug("Logging out of Spotify")
self._session.logout()
Expand Down
25 changes: 20 additions & 5 deletions mopidy_spotify/playlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ class SpotifyPlaylistsProvider(backend.PlaylistsProvider):
def __init__(self, backend):
self._backend = backend
self._timeout = self._backend._config["spotify"]["timeout"]
self._loaded = False

def as_list(self):
with utils.time_logger("playlists.as_list()"):
with utils.time_logger("playlists.as_list()", logging.INFO):
if not self._loaded:
return []

return list(self._get_flattened_playlist_refs())

def _get_flattened_playlist_refs(self):
Expand All @@ -34,11 +38,14 @@ def _get_flattened_playlist_refs(self):
yield playlist_ref

def get_items(self, uri):
with utils.time_logger(f"playlist.get_items({uri})"):
with utils.time_logger(f"playlist.get_items({uri})", logging.INFO):
if not self._loaded:
return []

return self._get_playlist(uri, as_items=True)

def lookup(self, uri):
with utils.time_logger(f"playlists.lookup({uri})"):
with utils.time_logger(f"playlists.lookup({uri})", logging.DEBUG):
return self._get_playlist(uri)

def _get_playlist(self, uri, as_items=False):
Expand All @@ -47,7 +54,15 @@ def _get_playlist(self, uri, as_items=False):
)

def refresh(self):
pass # TODO: Clear/invalidate all caches on refresh.
with utils.time_logger("Refresh Playlists", logging.INFO):
_cache.clear()
count = 0
for playlist_ref in self._get_flattened_playlist_refs():
self._get_playlist(playlist_ref.uri)
count = count + 1
logger.info(f"Refreshed {count} playlists")

self._loaded = True

def create(self, name):
pass # TODO
Expand All @@ -63,7 +78,7 @@ def playlist_lookup(web_client, uri, bitrate, as_items=False):
if web_client is None:
return

logger.info(f'Fetching Spotify playlist "{uri}"')
logger.debug(f'Fetching Spotify playlist "{uri}"')
web_playlist = web_client.get_playlist(uri, _cache)

if web_playlist == {}:
Expand Down
2 changes: 1 addition & 1 deletion mopidy_spotify/translator.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def web_to_track_ref(web_track):
uri = web_track.get("linked_from", {}).get("uri") or web_track["uri"]

if not web_track.get("is_playable", False):
logger.warning(f"{uri} is not playable")
logger.debug(f"{uri} is not playable")
return

return models.Ref.track(uri=uri, name=web_track.get("name"))
Expand Down
5 changes: 3 additions & 2 deletions mopidy_spotify/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,9 @@ def updated(self, response):

def __str__(self):
return (
f"URL: {self.url} ETag: {self._etag} "
f"Expires: {datetime.fromtimestamp(self._expires)}"
f"URL: {self.url} "
f"expires at: {datetime.fromtimestamp(self._expires)} "
f"[ETag: {self._etag}]"
)


Expand Down
22 changes: 22 additions & 0 deletions tests/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,28 @@ def test_on_start_logs_in(spotify_mock, web_mock, config):
web_mock.SpotifyOAuthClient.return_value.login.assert_called_once()


def test_on_start_refreshes_playlists(spotify_mock, web_mock, config, caplog):
backend = get_backend(config)
backend.on_start()

client_mock = web_mock.SpotifyOAuthClient.return_value
client_mock.get_user_playlists.assert_called_once()
assert "Refreshed 0 playlists" in caplog.text


def test_on_start_doesnt_refresh_playlists_if_not_allowed(
spotify_mock, web_mock, config, caplog
):
config["spotify"]["allow_playlists"] = False

backend = get_backend(config)
backend.on_start()

client_mock = web_mock.SpotifyOAuthClient.return_value
client_mock.get_user_playlists.assert_not_called()
assert "Refreshed 0 playlists" not in caplog.text


def test_on_stop_logs_out_and_waits_for_logout_to_complete(
spotify_mock, config, caplog
):
Expand Down
71 changes: 65 additions & 6 deletions tests/test_playlists.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest import mock

import pytest
from mopidy import backend as backend_api
from mopidy.models import Ref
Expand Down Expand Up @@ -41,7 +43,9 @@ def get_playlist(*args, **kwargs):
@pytest.fixture
def provider(backend_mock, web_client_mock):
backend_mock._web_client = web_client_mock
return playlists.SpotifyPlaylistsProvider(backend_mock)
provider = playlists.SpotifyPlaylistsProvider(backend_mock)
provider._loaded = True
return provider


def test_is_a_playlists_provider(provider):
Expand All @@ -64,6 +68,14 @@ def test_as_list_when_offline(web_client_mock, provider):
assert len(result) == 0


def test_as_list_blocked_when_not_loaded(provider):
provider._loaded = False

result = provider.as_list()

assert len(result) == 0


def test_as_list_when_playlist_wont_translate(provider, caplog):
result = provider.as_list()

Expand Down Expand Up @@ -99,6 +111,16 @@ def test_get_items_when_playlist_without_tracks(provider):
assert result == []


def test_get_items_blocked_when_not_loaded(provider):
provider._loaded = False

result = provider.get_items("spotify:user:alice:playlist:foo")

assert len(result) == 0

assert result == []


def test_get_items_when_playlist_wont_translate(provider, caplog):
assert provider.get_items("spotify:user:alice:playlist:malformed") is None

Expand All @@ -111,12 +133,40 @@ def test_get_items_when_playlist_is_unknown(provider, caplog):
)


def test_as_get_items_uses_cache(provider, web_client_mock):
provider.get_items("spotify:user:alice:playlist:foo")
def test_refresh_loads_all_playlists(provider, web_client_mock):
provider.refresh()

web_client_mock.get_playlist.assert_called_once_with(
"spotify:user:alice:playlist:foo", playlists._cache
)
web_client_mock.get_user_playlists.assert_called_once()
assert web_client_mock.get_playlist.call_count == 2
expected_calls = [
mock.call("spotify:user:alice:playlist:foo", {}),
mock.call("spotify:user:bob:playlist:baz", {}),
]
web_client_mock.get_playlist.assert_has_calls(expected_calls)


def test_refresh_when_not_loaded(provider, web_client_mock):
provider._loaded = False

provider.refresh()

web_client_mock.get_user_playlists.assert_called_once()
web_client_mock.get_playlist.assert_called()
assert provider._loaded


def test_refresh_counts_playlists(provider, caplog):
provider.refresh()

assert "Refreshed 2 playlists" in caplog.text


def test_refresh_clears_web_cache(provider):
playlists._cache = {"foo": "foobar", "foo2": "foofoo"}

provider.refresh()

assert len(playlists._cache) == 0


def test_lookup(provider):
Expand All @@ -127,6 +177,15 @@ def test_lookup(provider):
assert playlist.tracks[0].bitrate == 160


def test_lookup_when_not_loaded(provider):
provider._loaded = False

playlist = provider.lookup("spotify:user:alice:playlist:foo")

assert playlist.uri == "spotify:user:alice:playlist:foo"
assert playlist.name == "Foo"


def test_lookup_when_playlist_is_empty(provider, caplog):
assert provider.lookup("nothing") is None
assert "Failed to lookup Spotify playlist URI nothing" in caplog.text
Expand Down