Skip to content

Commit

Permalink
Use the long-form Spotify URL for user playlists. (mopidy#299)
Browse files Browse the repository at this point in the history
For logged in user 'foo',  Web API endpoint "me/playlists"
returns the same as "users/foo/playlists" (including headers).
When we use the former, the "next" links in the response always use
the latter. This leaves us with a mix of URL formats in our cache
and will make identifying/invalidating these entries harder.

This change ensures all the URLs look the same. No behaviour has
changed.
  • Loading branch information
kingosticks committed Dec 20, 2023
1 parent 106bb53 commit ee4fc8e
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 8 deletions.
4 changes: 3 additions & 1 deletion mopidy_spotify/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,9 @@ def logged_in(self):
return self.user_id is not None

def get_user_playlists(self):
pages = self.get_all("me/playlists", params={"limit": 50})
pages = self.get_all(
f"users/{self.user_id}/playlists", params={"limit": 50}
)
for page in pages:
yield from page.get("items", [])

Expand Down
18 changes: 11 additions & 7 deletions tests/test_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -690,11 +690,13 @@ def test_updated_responses_changed(web_response_mock, oauth_client, mock_time):

@pytest.fixture
def spotify_client(config):
return web.SpotifyOAuthClient(
client = web.SpotifyOAuthClient(
client_id=config["spotify"]["client_id"],
client_secret=config["spotify"]["client_secret"],
proxy_config=None,
)
client.user_id = "alice"
return client


def url(endpoint):
Expand Down Expand Up @@ -799,6 +801,7 @@ def test_configures_urls(self, spotify_client):

@responses.activate
def test_login_alice(self, spotify_client, caplog):
spotify_client.user_id = None
responses.add(responses.GET, url("me"), json={"id": "alice"})

assert spotify_client.login()
Expand All @@ -807,6 +810,7 @@ def test_login_alice(self, spotify_client, caplog):

@responses.activate
def test_login_fails(self, spotify_client, caplog):
spotify_client.user_id = None
responses.add(responses.GET, url("me"), json={})

assert not spotify_client.login()
Expand Down Expand Up @@ -867,7 +871,7 @@ def test_get_all_none(self, spotify_client):

@responses.activate
def test_get_user_playlists_empty(self, spotify_client):
responses.add(responses.GET, url("me/playlists"), json={})
responses.add(responses.GET, url("users/alice/playlists"), json={})

result = list(spotify_client.get_user_playlists())

Expand All @@ -876,29 +880,29 @@ def test_get_user_playlists_empty(self, spotify_client):

@responses.activate
def test_get_user_playlists_sets_params(self, spotify_client):
responses.add(responses.GET, url("me/playlists"), json={})
responses.add(responses.GET, url("users/alice/playlists"), json={})

list(spotify_client.get_user_playlists())

assert len(responses.calls) == 1
encoded_params = urllib.parse.urlencode({"limit": 50})
assert responses.calls[0].request.url == url(
f"me/playlists?{encoded_params}"
f"users/alice/playlists?{encoded_params}"
)

@responses.activate
def test_get_user_playlists(self, spotify_client):
responses.add(
responses.GET,
url("me/playlists?limit=50"),
url("users/alice/playlists?limit=50"),
json={
"next": url("me/playlists?offset=50"),
"next": url("users/alice/playlists?offset=50"),
"items": ["playlist0", "playlist1", "playlist2"],
},
)
responses.add(
responses.GET,
url("me/playlists?limit=50&offset=50"),
url("users/alice/playlists?limit=50&offset=50"),
json={
"next": None,
"items": ["playlist3", "playlist4", "playlist5"],
Expand Down

0 comments on commit ee4fc8e

Please sign in to comment.