From 02a5937025a8d0fbe2d4767cbd7ae2275a186944 Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Sun, 14 Feb 2016 02:04:40 +0100 Subject: [PATCH 1/2] search: Use the new Web API for search Spotify has discontinued their Metadata API, which libspotify uses. This means that search in libspotify, and thus pyspotify is not working. This changes the search to use the new Web API which is supported. This is a first step for fixing search, and makes it work again. We probably want to further improve this, and possibly move calling the API into pyspotify. Note that some parts of the Web API require OAuth (i.e. different auth than libspotify requires), but luckily search doesn't require any auth. The search results from the Web API misses some data compared to what libspotify does. The fields that are missing is bitrate and date from tracks, and artists and date from albums. It is possible to additional requests to get the artists and release date for albums, but this is left for a later improvement. Fixes #89. --- README.rst | 8 ++++++ mopidy_spotify/library.py | 2 +- mopidy_spotify/search.py | 37 +++++++++++++++++++-------- mopidy_spotify/translator.py | 24 ++++++++++++++++++ tests/conftest.py | 49 ++++++++++++++++++++++++++++++------ tests/test_search.py | 23 +++++++++++++---- tests/test_translator.py | 35 ++++++++++++++++++++++++++ 7 files changed, 155 insertions(+), 23 deletions(-) diff --git a/README.rst b/README.rst index 9c2d8ee6..5d516a06 100644 --- a/README.rst +++ b/README.rst @@ -148,6 +148,14 @@ Credits Changelog ========= +v2.3.1 (UNRELEASED) +------------------- + +Bug fix release. + +- Use the new Web API for search. Searching through libspotify has been + discontinued and is not working anymore. (Fixes: #89) + v2.3.0 (2016-02-06) ------------------- diff --git a/mopidy_spotify/library.py b/mopidy_spotify/library.py index 3e95580b..6cc6414f 100644 --- a/mopidy_spotify/library.py +++ b/mopidy_spotify/library.py @@ -38,5 +38,5 @@ def lookup(self, uri): def search(self, query=None, uris=None, exact=False): return search.search( - self._config, self._backend._session, + self._config, self._backend._session, self._requests_session, query, uris, exact) diff --git a/mopidy_spotify/search.py b/mopidy_spotify/search.py index d4f732eb..eeec86ba 100644 --- a/mopidy_spotify/search.py +++ b/mopidy_spotify/search.py @@ -5,15 +5,21 @@ from mopidy import models +import requests + import spotify from mopidy_spotify import lookup, translator +_API_BASE_URI = 'https://api.spotify.com/v1/search' +_SEARCH_TYPES = 'album,artist,track' + logger = logging.getLogger(__name__) -def search(config, session, query=None, uris=None, exact=False): +def search(config, session, requests_session, + query=None, uris=None, exact=False): # TODO Respect `uris` argument # TODO Support `exact` search @@ -36,19 +42,30 @@ def search(config, session, query=None, uris=None, exact=False): logger.info('Spotify search aborted: Spotify is offline') return models.SearchResult(uri=uri) - sp_search = session.search( - sp_query, - album_count=config['search_album_count'], - artist_count=config['search_artist_count'], - track_count=config['search_track_count']) - sp_search.load() + try: + response = requests_session.get(_API_BASE_URI, params={ + 'q': sp_query, + 'limit': config['search_track_count'], + 'type': _SEARCH_TYPES}) + except requests.RequestException as exc: + logger.debug('Fetching %s failed: %s', uri, exc) + return models.SearchResult(uri=uri) + + try: + result = response.json() + except ValueError as exc: + logger.debug('JSON decoding failed for %s: %s', uri, exc) + return models.SearchResult(uri=uri) albums = [ - translator.to_album(sp_album) for sp_album in sp_search.albums] + translator.webapi_to_album(sp_album) + for sp_album in result['albums']['items']] artists = [ - translator.to_artist(sp_artist) for sp_artist in sp_search.artists] + translator.webapi_to_artist(sp_artist) + for sp_artist in result['artists']['items']] tracks = [ - translator.to_track(sp_track) for sp_track in sp_search.tracks] + translator.webapi_to_track(sp_track) + for sp_track in result['tracks']['items']] return models.SearchResult( uri=uri, albums=albums, artists=artists, tracks=tracks) diff --git a/mopidy_spotify/translator.py b/mopidy_spotify/translator.py index f0ef8361..830115f5 100644 --- a/mopidy_spotify/translator.py +++ b/mopidy_spotify/translator.py @@ -232,3 +232,27 @@ def _transform_year(date): logger.debug( 'Excluded year from search query: ' 'Cannot parse date "%s"', date) + + +def webapi_to_artist(sp_artist): + return models.Artist(uri=sp_artist['uri'], name=sp_artist['name']) + + +def webapi_to_album(sp_album): + return models.Album(uri=sp_album['uri'], name=sp_album['name']) + + +def webapi_to_track(sp_track): + artists = [ + webapi_to_artist(sp_artist) + for sp_artist in sp_track['artists']] + album = webapi_to_album(sp_track['album']) + + return models.Track( + uri=sp_track['uri'], + name=sp_track['name'], + artists=artists, + album=album, + length=sp_track['duration_ms'], + disc_no=sp_track['disc_number'], + track_no=sp_track['track_number']) diff --git a/tests/conftest.py b/tests/conftest.py index 0719ce51..129e2725 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -272,13 +272,48 @@ def sp_playlist_container_mock(): @pytest.fixture -def sp_search_mock(sp_album_mock, sp_artist_mock, sp_track_mock): - sp_search = mock.Mock(spec=spotify.Search) - sp_search.is_loaded = True - sp_search.albums = [sp_album_mock] - sp_search.artists = [sp_artist_mock] - sp_search.tracks = [sp_track_mock, sp_track_mock] - return sp_search +def webapi_search_mock( + webapi_album_mock, webapi_artist_mock, webapi_track_mock): + return { + 'albums': { + 'items': [webapi_album_mock] + }, + 'artists': { + 'items': [webapi_artist_mock] + }, + 'tracks': { + 'items': [webapi_track_mock, webapi_track_mock] + } + } + + +@pytest.fixture +def webapi_artist_mock(): + return { + 'name': 'ABBA', + 'uri': 'spotify:artist:abba' + } + + +@pytest.fixture +def webapi_album_mock(): + return { + 'name': 'DEF 456', + 'uri': 'spotify:album:def' + } + + +@pytest.fixture +def webapi_track_mock(webapi_artist_mock, webapi_album_mock): + return { + 'album': webapi_album_mock, + 'artists': [webapi_artist_mock], + 'disc_number': 1, + 'duration_ms': 174300, + 'name': 'ABC 123', + 'track_number': 7, + 'uri': 'spotify:track:abc', + } @pytest.fixture diff --git a/tests/test_search.py b/tests/test_search.py index e34131e3..0a25da69 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -1,9 +1,15 @@ from __future__ import unicode_literals +import json + from mopidy import models +import responses + import spotify +import mopidy_spotify + def test_search_with_no_query_returns_nothing(provider, caplog): result = provider.search() @@ -65,15 +71,22 @@ def test_search_when_offline_returns_nothing(session_mock, provider, caplog): assert len(result.tracks) == 0 +@responses.activate def test_search_returns_albums_and_artists_and_tracks( - session_mock, sp_search_mock, provider, caplog): - session_mock.search.return_value = sp_search_mock + webapi_search_mock, provider, caplog): + responses.add( + responses.GET, 'https://api.spotify.com/v1/search', + body=json.dumps(webapi_search_mock)) result = provider.search({'any': ['ABBA']}) - session_mock.search.assert_called_once_with( - '"ABBA"', album_count=20, artist_count=10, track_count=50) - sp_search_mock.load.assert_called_once_with() + assert len(responses.calls) == 1 + assert ( + responses.calls[0].request.url == + 'https://api.spotify.com/v1/search?q=%22ABBA%22&' + 'type=album%2Cartist%2Ctrack&limit=50') + assert responses.calls[0].request.headers['User-Agent'].startswith( + 'Mopidy-Spotify/%s' % mopidy_spotify.__version__) assert 'Searching Spotify for: "ABBA"' in caplog.text() diff --git a/tests/test_translator.py b/tests/test_translator.py index 523b3047..f38d787f 100644 --- a/tests/test_translator.py +++ b/tests/test_translator.py @@ -428,3 +428,38 @@ def test_anything_can_be_combined(self): assert 'album:"Greatest Hits"' in query assert 'track:"Dancing Queen"' in query assert 'year:1970' in query + + +class TestWebapiToArtist(object): + + def test_successful_translation(self, webapi_artist_mock): + artist = translator.webapi_to_artist(webapi_artist_mock) + + assert artist.uri == 'spotify:artist:abba' + assert artist.name == 'ABBA' + + +class TestWebapiToAlbum(object): + + def test_successful_translation(self, webapi_album_mock): + album = translator.webapi_to_album(webapi_album_mock) + + assert album.uri == 'spotify:album:def' + assert album.name == 'DEF 456' + + +class TestWebapiToTrack(object): + + def test_successful_translation(self, webapi_track_mock): + track = translator.webapi_to_track(webapi_track_mock) + + assert track.uri == 'spotify:track:abc' + assert track.name == 'ABC 123' + assert list(track.artists) == [ + models.Artist(uri='spotify:artist:abba', name='ABBA')] + assert track.album == models.Album( + uri='spotify:album:def', + name='DEF 456') + assert track.track_no == 7 + assert track.disc_no == 1 + assert track.length == 174300 From d7dde838468db19403e474b30ecd090a0f347ab9 Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Sun, 14 Feb 2016 02:48:44 +0100 Subject: [PATCH 2/2] fix: Implement search limits correctly for Web API The Web API only has a single limit, which limits the number of items of each type. This sets that limit to the max of the config options search_album_count, search_artist_count and search_track_count. Furthermore it limits the results for each type to its respective config limit to mimic the old behavior. The max value for these config options has been decreased to 50, because this is the max value that the Web API allows. --- README.rst | 10 +++-- mopidy_spotify/__init__.py | 6 +-- mopidy_spotify/search.py | 19 +++++---- tests/conftest.py | 16 ++++++++ tests/test_search.py | 81 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 13 deletions(-) diff --git a/README.rst b/README.rst index 5d516a06..cb8f91b4 100644 --- a/README.rst +++ b/README.rst @@ -111,13 +111,13 @@ The following configuration values are available: Defaults to ``true``. - ``spotify/search_album_count``: Maximum number of albums returned in search - results. Number between 0 and 200. Defaults to 20. + results. Number between 0 and 50. Defaults to 20. - ``spotify/search_artist_count``: Maximum number of artists returned in search - results. Number between 0 and 200. Defaults to 10. + results. Number between 0 and 50. Defaults to 10. - ``spotify/search_track_count``: Maximum number of tracks returned in search - results. Number between 0 and 200. Defaults to 50. + results. Number between 0 and 50. Defaults to 50. - ``spotify/toplist_countries``: Comma separated list of two letter ISO country codes to get toplists for. Defaults to blank, which is interpreted as all @@ -156,6 +156,10 @@ Bug fix release. - Use the new Web API for search. Searching through libspotify has been discontinued and is not working anymore. (Fixes: #89) +- Change the maximum value of search_album_count, search_artist_count and + search_track_count to 50, because this is the maximum value that the Web API + allows. + v2.3.0 (2016-02-06) ------------------- diff --git a/mopidy_spotify/__init__.py b/mopidy_spotify/__init__.py index 4a5ddc52..a19c14f0 100644 --- a/mopidy_spotify/__init__.py +++ b/mopidy_spotify/__init__.py @@ -37,9 +37,9 @@ def get_config_schema(self): schema['allow_network'] = config.Boolean() schema['allow_playlists'] = config.Boolean() - schema['search_album_count'] = config.Integer(minimum=0, maximum=200) - schema['search_artist_count'] = config.Integer(minimum=0, maximum=200) - schema['search_track_count'] = config.Integer(minimum=0, maximum=200) + schema['search_album_count'] = config.Integer(minimum=0, maximum=50) + schema['search_artist_count'] = config.Integer(minimum=0, maximum=50) + schema['search_track_count'] = config.Integer(minimum=0, maximum=50) schema['toplist_countries'] = config.List(optional=True) diff --git a/mopidy_spotify/search.py b/mopidy_spotify/search.py index eeec86ba..a23c9d47 100644 --- a/mopidy_spotify/search.py +++ b/mopidy_spotify/search.py @@ -42,10 +42,15 @@ def search(config, session, requests_session, logger.info('Spotify search aborted: Spotify is offline') return models.SearchResult(uri=uri) + search_count = max( + config['search_album_count'], + config['search_artist_count'], + config['search_track_count']) + try: response = requests_session.get(_API_BASE_URI, params={ 'q': sp_query, - 'limit': config['search_track_count'], + 'limit': search_count, 'type': _SEARCH_TYPES}) except requests.RequestException as exc: logger.debug('Fetching %s failed: %s', uri, exc) @@ -58,14 +63,14 @@ def search(config, session, requests_session, return models.SearchResult(uri=uri) albums = [ - translator.webapi_to_album(sp_album) - for sp_album in result['albums']['items']] + translator.webapi_to_album(sp_album) for sp_album in + result['albums']['items'][:config['search_album_count']]] artists = [ - translator.webapi_to_artist(sp_artist) - for sp_artist in result['artists']['items']] + translator.webapi_to_artist(sp_artist) for sp_artist in + result['artists']['items'][:config['search_artist_count']]] tracks = [ - translator.webapi_to_track(sp_track) - for sp_track in result['tracks']['items']] + translator.webapi_to_track(sp_track) for sp_track in + result['tracks']['items'][:config['search_track_count']]] return models.SearchResult( uri=uri, albums=albums, artists=artists, tracks=tracks) diff --git a/tests/conftest.py b/tests/conftest.py index 129e2725..881a07be 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -287,6 +287,22 @@ def webapi_search_mock( } +@pytest.fixture +def webapi_search_mock_large( + webapi_album_mock, webapi_artist_mock, webapi_track_mock): + return { + 'albums': { + 'items': [webapi_album_mock] * 10 + }, + 'artists': { + 'items': [webapi_artist_mock] * 10 + }, + 'tracks': { + 'items': [webapi_track_mock] * 10 + } + } + + @pytest.fixture def webapi_artist_mock(): return { diff --git a/tests/test_search.py b/tests/test_search.py index 0a25da69..19fe93dc 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -103,6 +103,87 @@ def test_search_returns_albums_and_artists_and_tracks( assert result.tracks[0].uri == 'spotify:track:abc' +@responses.activate +def test_search_limits_number_of_results( + webapi_search_mock_large, provider, config): + config['spotify']['search_album_count'] = 4 + config['spotify']['search_artist_count'] = 5 + config['spotify']['search_track_count'] = 6 + + responses.add( + responses.GET, 'https://api.spotify.com/v1/search', + body=json.dumps(webapi_search_mock_large)) + + result = provider.search({'any': ['ABBA']}) + + assert len(result.albums) == 4 + assert len(result.artists) == 5 + assert len(result.tracks) == 6 + + +@responses.activate +def test_sets_api_limit_to_album_count_when_max( + webapi_search_mock_large, provider, config): + config['spotify']['search_album_count'] = 6 + config['spotify']['search_artist_count'] = 2 + config['spotify']['search_track_count'] = 2 + + responses.add( + responses.GET, 'https://api.spotify.com/v1/search', + body=json.dumps(webapi_search_mock_large)) + + result = provider.search({'any': ['ABBA']}) + + assert ( + responses.calls[0].request.url == + 'https://api.spotify.com/v1/search?q=%22ABBA%22&' + 'type=album%2Cartist%2Ctrack&limit=6') + + assert len(result.albums) == 6 + + +@responses.activate +def test_sets_api_limit_to_artist_count_when_max( + webapi_search_mock_large, provider, config): + config['spotify']['search_album_count'] = 2 + config['spotify']['search_artist_count'] = 6 + config['spotify']['search_track_count'] = 2 + + responses.add( + responses.GET, 'https://api.spotify.com/v1/search', + body=json.dumps(webapi_search_mock_large)) + + result = provider.search({'any': ['ABBA']}) + + assert ( + responses.calls[0].request.url == + 'https://api.spotify.com/v1/search?q=%22ABBA%22&' + 'type=album%2Cartist%2Ctrack&limit=6') + + assert len(result.artists) == 6 + + +@responses.activate +def test_sets_api_limit_to_track_count_when_max( + webapi_search_mock_large, provider, config): + config['spotify']['search_album_count'] = 2 + config['spotify']['search_artist_count'] = 2 + config['spotify']['search_track_count'] = 6 + + responses.add( + responses.GET, 'https://api.spotify.com/v1/search', + body=json.dumps(webapi_search_mock_large)) + + result = provider.search({'any': ['ABBA']}) + + assert ( + responses.calls[0].request.url == + 'https://api.spotify.com/v1/search?q=%22ABBA%22&' + 'type=album%2Cartist%2Ctrack&limit=6') + + assert len(result.tracks) == 6 + + def test_exact_is_ignored(session_mock, sp_track_mock, provider): session_mock.get_link.return_value = sp_track_mock.link