From 5472a4999150306de0494d15a31353ac81d985df Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 21:24:41 -0800 Subject: [PATCH 01/19] Add `candidates` and `item_candidates`, modularize Search API queries --- beetsplug/spotify.py | 175 +++++++++++++++++++++++++++++-------------- test/test_spotify.py | 14 ++-- 2 files changed, 127 insertions(+), 62 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index 246e65a6f..9a08fa709 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -85,7 +85,7 @@ class SpotifyPlugin(BeetsPlugin): except requests.exceptions.HTTPError as e: raise ui.UserError( u'Spotify authorization failed: {}\n{}'.format( - e, response.content + e, response.text ) ) self.access_token = response.json()['access_token'] @@ -106,8 +106,8 @@ class SpotifyPlugin(BeetsPlugin): :param params: (optional) list of tuples or bytes to send in the query string for the :class:`Request`. :type params: dict - :return: class:`Response ` object - :rtype: requests.Response + :return: JSON data for the class:`Response ` object + :rtype: dict """ response = request_type( url, @@ -123,7 +123,7 @@ class SpotifyPlugin(BeetsPlugin): self._handle_response(request_type, url, params=params) else: raise ui.UserError(u'Spotify API error:\n{}', response.text) - return response + return response.json() def _get_spotify_id(self, url_type, id_): """Parse a Spotify ID from its URL if necessary. @@ -155,10 +155,9 @@ class SpotifyPlugin(BeetsPlugin): if spotify_id is None: return None - response = self._handle_response( + response_data = self._handle_response( requests.get, self.album_url + spotify_id ) - response_data = response.json() artist, artist_id = self._get_artist(response_data['artists']) date_parts = [ @@ -244,18 +243,16 @@ class SpotifyPlugin(BeetsPlugin): if spotify_id is None: return None - response_track = self._handle_response( + response_data_track = self._handle_response( requests.get, self.track_url + spotify_id ) - response_data_track = response_track.json() track = self._get_track(response_data_track) # get album's tracks to set the track's index/position on # the entire release - response_album = self._handle_response( + response_data_album = self._handle_response( requests.get, self.album_url + response_data_track['album']['id'] ) - response_data_album = response_album.json() medium_total = 0 for i, track_data in enumerate(response_data_album['tracks']['items']): if track_data['disc_number'] == track.medium: @@ -308,12 +305,92 @@ class SpotifyPlugin(BeetsPlugin): dist.add('source', self.config['source_weight'].as_number()) return dist + def candidates(self, items, artist, album, va_likely): + """Returns a list of AlbumInfo objects for Spotify Search results + matching an ``album`` and ``artist`` (if not various). + """ + query_filters = {'album': album} + if not va_likely: + query_filters['artist'] = artist + response_data = self._search_spotify( + query_type='album', filters=query_filters + ) + return [ + self.album_for_id(album_id=album_data['id']) + for album_data in response_data['albums']['items'] + ] + + def item_candidates(self, item, artist, title): + """Returns a list of TrackInfo objects for Spotify Search results + matching ``title`` and ``artist``. + """ + response_data = self._search_spotify( + query_type='track', keywords=title, filters={'artist': artist} + ) + return [ + self._get_track(track_data) + for track_data in response_data['tracks']['items'] + ] + + @staticmethod + def _construct_search_query(filters=None, keywords=''): + """Construct a query string with the specified filters and keywords to + be provided to the Spotify Search API + (https://developer.spotify.com/documentation/web-api/reference/search/search/). + + :param filters: (Optional) Field filters to apply. + :type filters: dict + :param keywords: (Optional) Query keywords to use. + :type keywords: str + :return: Search query string to be provided to the Search API + (https://developer.spotify.com/documentation/web-api/reference/search/search/#writing-a-query---guidelines) + :rtype: str + """ + query_string = keywords + if filters is not None: + query_string += ' ' + ' '.join( + ':'.join((k, v)) for k, v in filters.items() + ) + return query_string + + def _search_spotify(self, query_type, filters=None, keywords=''): + """Query the Spotify Search API for the specified ``keywords``, applying + the provided filters. + + :param query_type: A comma-separated list of item types to search + across. Valid types are: 'album', 'artist', 'playlist', and + 'track'. Search results include hits from all the specified item + types. + :type query_type: str + :param filters: (Optional) Field filters to apply. + :type filters: dict + :param keywords: (Optional) Query keywords to use. + :return: JSON data for the class:`Response ` object + :rtype: dict + """ + query = self._construct_search_query( + keywords=keywords, filters=filters + ) + if not query: + return None + self._log.debug(u'Searching Spotify for "{}"'.format(query)) + response_data = self._handle_response( + requests.get, + self.search_url, + params={'q': query, 'type': query_type}, + ) + num_results = 0 + for result_type_data in response_data.values(): + num_results += len(result_type_data['items']) + self._log.debug(u'Found {} results from Spotify'.format(num_results)) + return response_data if num_results > 0 else None + def commands(self): def queries(lib, opts, args): - success = self.parse_opts(opts) + success = self._parse_opts(opts) if success: - results = self.query_spotify(lib, ui.decargs(args)) - self.output_results(results) + results = self._query_spotify(lib, ui.decargs(args)) + self._output_results(results) spotify_cmd = ui.Subcommand( 'spotify', help=u'build a Spotify playlist' @@ -335,7 +412,7 @@ class SpotifyPlugin(BeetsPlugin): spotify_cmd.func = queries return [spotify_cmd] - def parse_opts(self, opts): + def _parse_opts(self, opts): if opts.mode: self.config['mode'].set(opts.mode) @@ -351,19 +428,19 @@ class SpotifyPlugin(BeetsPlugin): self.opts = opts return True - def query_spotify(self, lib, query): + def _query_spotify(self, lib, keywords): results = [] failures = [] - items = lib.items(query) + items = lib.items(keywords) if not items: self._log.debug( - u'Your beets query returned no items, skipping Spotify' + u'Your beets query returned no items, skipping Spotify.' ) return - self._log.info(u'Processing {0} tracks...', len(items)) + self._log.info(u'Processing {} tracks...', len(items)) for item in items: # Apply regex transformations if provided @@ -383,81 +460,69 @@ class SpotifyPlugin(BeetsPlugin): # Custom values can be passed in the config (just in case) artist = item[self.config['artist_field'].get()] album = item[self.config['album_field'].get()] - query = item[self.config['track_field'].get()] - query_keywords = '{} album:{} artist:{}'.format( - query, album, artist - ) + keywords = item[self.config['track_field'].get()] # Query the Web API for each track, look for the items' JSON data - try: - response = self._handle_response( - requests.get, - self.search_url, - params={'q': query_keywords, 'type': 'track'}, + query_filters = {'artist': artist, 'album': album} + response_data = self._search_spotify( + query_type='track', keywords=keywords, filters=query_filters + ) + if response_data is None: + query = self._construct_search_query( + keywords=keywords, filters=query_filters ) - except ui.UserError: - failures.append(query_keywords) + failures.append(query) continue - - response_data = response.json()['tracks']['items'] + response_data_tracks = response_data['tracks']['items'] # Apply market filter if requested region_filter = self.config['region_filter'].get() if region_filter: - response_data = [ + response_data_tracks = [ x - for x in response_data + for x in response_data_tracks if region_filter in x['available_markets'] ] - # Simplest, take the first result - chosen_result = None if ( - len(response_data) == 1 + len(response_data_tracks) == 1 or self.config['tiebreak'].get() == 'first' ): self._log.debug( - u'Spotify track(s) found, count: {0}', len(response_data) + u'Spotify track(s) found, count: {}', + len(response_data_tracks), ) - chosen_result = response_data[0] - elif len(response_data) > 1: + chosen_result = response_data_tracks[0] + else: # Use the popularity filter self._log.debug( - u'Most popular track chosen, count: {0}', - len(response_data), + u'Most popular track chosen, count: {}', + len(response_data_tracks), ) chosen_result = max( - response_data, key=lambda x: x['popularity'] + response_data_tracks, key=lambda x: x['popularity'] ) - - if chosen_result: - results.append(chosen_result) - else: - self._log.debug( - u'No Spotify track found for the following query: {}', - query_keywords, - ) - failures.append(query_keywords) + results.append(chosen_result) failure_count = len(failures) if failure_count > 0: if self.config['show_failures'].get(): self._log.info( - u'{0} track(s) did not match a Spotify ID:', failure_count + u'{} track(s) did not match a Spotify ID:', failure_count ) for track in failures: - self._log.info(u'track: {0}', track) + self._log.info(u'track: {}', track) self._log.info(u'') else: self._log.warning( - u'{0} track(s) did not match a Spotify ID;\n' + u'{} track(s) did not match a Spotify ID;\n' u'use --show-failures to display', failure_count, ) return results - def output_results(self, results): + def _output_results(self, results): if results: ids = [x['id'] for x in results] if self.config['mode'].get() == "open": diff --git a/test/test_spotify.py b/test/test_spotify.py index 221c21e74..99e09e208 100644 --- a/test/test_spotify.py +++ b/test/test_spotify.py @@ -47,19 +47,19 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): ) self.spotify = spotify.SpotifyPlugin() opts = ArgumentsMock("list", False) - self.spotify.parse_opts(opts) + self.spotify._parse_opts(opts) def tearDown(self): self.teardown_beets() def test_args(self): opts = ArgumentsMock("fail", True) - self.assertEqual(False, self.spotify.parse_opts(opts)) + self.assertEqual(False, self.spotify._parse_opts(opts)) opts = ArgumentsMock("list", False) - self.assertEqual(True, self.spotify.parse_opts(opts)) + self.assertEqual(True, self.spotify._parse_opts(opts)) def test_empty_query(self): - self.assertEqual(None, self.spotify.query_spotify(self.lib, u"1=2")) + self.assertEqual(None, self.spotify._query_spotify(self.lib, u"1=2")) @responses.activate def test_missing_request(self): @@ -84,7 +84,7 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): length=10, ) item.add(self.lib) - self.assertEqual([], self.spotify.query_spotify(self.lib, u"")) + self.assertEqual([], self.spotify._query_spotify(self.lib, u"")) params = _params(responses.calls[0].request.url) self.assertEqual( @@ -116,10 +116,10 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): length=10, ) item.add(self.lib) - results = self.spotify.query_spotify(self.lib, u"Happy") + results = self.spotify._query_spotify(self.lib, u"Happy") self.assertEqual(1, len(results)) self.assertEqual(u"6NPVjNh8Jhru9xOmyQigds", results[0]['id']) - self.spotify.output_results(results) + self.spotify._output_results(results) params = _params(responses.calls[0].request.url) self.assertEqual( From 265fcc7cea9f7754e25b8849738f165e15861334 Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 21:45:50 -0800 Subject: [PATCH 02/19] utilize `track_for_id` in `item_candidates` --- beetsplug/spotify.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index 9a08fa709..dd80970a2 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -230,12 +230,16 @@ class SpotifyPlugin(BeetsPlugin): data_url=track_data['external_urls']['spotify'], ) - def track_for_id(self, track_id): + def track_for_id(self, track_id=None, track_data=None): """Fetches a track by its Spotify ID or URL and returns a TrackInfo object or None if the track is not found. - :param track_id: Spotify ID or URL for the track + :param track_id: (Optional) Spotify ID or URL for the track. Either + ``track_id`` or ``track_data`` must be provided. :type track_id: str + :param track_data: (Optional) Simplified track object dict. May be + provided instead of ``track_id`` to avoid unnecessary API calls. + :type track_data: dict :return: TrackInfo object for track :rtype: beets.autotag.hooks.TrackInfo """ @@ -243,15 +247,16 @@ class SpotifyPlugin(BeetsPlugin): if spotify_id is None: return None - response_data_track = self._handle_response( - requests.get, self.track_url + spotify_id - ) - track = self._get_track(response_data_track) + if track_data is None: + track_data = self._handle_response( + requests.get, self.track_url + spotify_id + ) + track = self._get_track(track_data) # get album's tracks to set the track's index/position on # the entire release response_data_album = self._handle_response( - requests.get, self.album_url + response_data_track['album']['id'] + requests.get, self.album_url + track_data['album']['id'] ) medium_total = 0 for i, track_data in enumerate(response_data_album['tracks']['items']): @@ -328,7 +333,7 @@ class SpotifyPlugin(BeetsPlugin): query_type='track', keywords=title, filters={'artist': artist} ) return [ - self._get_track(track_data) + self.track_for_id(track_data=track_data) for track_data in response_data['tracks']['items'] ] From aa18f9116dbddc0d889391401da9e988118e68de Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 22:01:30 -0800 Subject: [PATCH 03/19] Refine doc wording --- beetsplug/spotify.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index dd80970a2..cbb43ffef 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -311,7 +311,7 @@ class SpotifyPlugin(BeetsPlugin): return dist def candidates(self, items, artist, album, va_likely): - """Returns a list of AlbumInfo objects for Spotify Search results + """Returns a list of AlbumInfo objects for Spotify Search API results matching an ``album`` and ``artist`` (if not various). """ query_filters = {'album': album} @@ -326,7 +326,7 @@ class SpotifyPlugin(BeetsPlugin): ] def item_candidates(self, item, artist, title): - """Returns a list of TrackInfo objects for Spotify Search results + """Returns a list of TrackInfo objects for Spotify Search API results matching ``title`` and ``artist``. """ response_data = self._search_spotify( From 42e852cc7ed9db3a644b162094240b5e3f7bc55b Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 22:12:56 -0800 Subject: [PATCH 04/19] Clarify `_search_spotify` return type --- beetsplug/spotify.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index cbb43ffef..152bea46d 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -370,8 +370,9 @@ class SpotifyPlugin(BeetsPlugin): :param filters: (Optional) Field filters to apply. :type filters: dict :param keywords: (Optional) Query keywords to use. - :return: JSON data for the class:`Response ` object - :rtype: dict + :return: JSON data for the class:`Response ` object or None + if no search results are returned + :rtype: dict or None """ query = self._construct_search_query( keywords=keywords, filters=filters From 48401c60dc9464bddc8e15f8e379480ba8b3e90f Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 22:27:31 -0800 Subject: [PATCH 05/19] Switch query filter ordering for tests --- beetsplug/spotify.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index 152bea46d..ee73640ae 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -320,6 +320,8 @@ class SpotifyPlugin(BeetsPlugin): response_data = self._search_spotify( query_type='album', filters=query_filters ) + if response_data is None: + return [] return [ self.album_for_id(album_id=album_data['id']) for album_data in response_data['albums']['items'] @@ -332,6 +334,8 @@ class SpotifyPlugin(BeetsPlugin): response_data = self._search_spotify( query_type='track', keywords=title, filters={'artist': artist} ) + if response_data is None: + return [] return [ self.track_for_id(track_data=track_data) for track_data in response_data['tracks']['items'] @@ -469,7 +473,7 @@ class SpotifyPlugin(BeetsPlugin): keywords = item[self.config['track_field'].get()] # Query the Web API for each track, look for the items' JSON data - query_filters = {'artist': artist, 'album': album} + query_filters = {'album': album, 'artist': artist} response_data = self._search_spotify( query_type='track', keywords=keywords, filters=query_filters ) From f63beca39ab3c0f476d3965e731985c3b90da006 Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 22:35:12 -0800 Subject: [PATCH 06/19] Switch filter ordering in test --- beetsplug/spotify.py | 6 +++--- test/test_spotify.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index ee73640ae..a4179ef1c 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -255,11 +255,11 @@ class SpotifyPlugin(BeetsPlugin): # get album's tracks to set the track's index/position on # the entire release - response_data_album = self._handle_response( + album_data = self._handle_response( requests.get, self.album_url + track_data['album']['id'] ) medium_total = 0 - for i, track_data in enumerate(response_data_album['tracks']['items']): + for i, track_data in enumerate(album_data['tracks']['items']): if track_data['disc_number'] == track.medium: medium_total += 1 if track_data['id'] == spotify_id: @@ -473,7 +473,7 @@ class SpotifyPlugin(BeetsPlugin): keywords = item[self.config['track_field'].get()] # Query the Web API for each track, look for the items' JSON data - query_filters = {'album': album, 'artist': artist} + query_filters = {'artist': artist, 'album': album} response_data = self._search_spotify( query_type='track', keywords=keywords, filters=query_filters ) diff --git a/test/test_spotify.py b/test/test_spotify.py index 99e09e208..39eb38113 100644 --- a/test/test_spotify.py +++ b/test/test_spotify.py @@ -124,7 +124,7 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): params = _params(responses.calls[0].request.url) self.assertEqual( params['q'], - [u'Happy album:Despicable Me 2 artist:Pharrell Williams'], + [u'Happy artist:Pharrell Williams album:Despicable Me 2'], ) self.assertEqual(params['type'], [u'track']) From 237792a4fb9a4a80bfde2c3fc0aa9b814d06450b Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 22:40:15 -0800 Subject: [PATCH 07/19] Fix other `test_track_request` case --- test/test_spotify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_spotify.py b/test/test_spotify.py index 39eb38113..25a58911d 100644 --- a/test/test_spotify.py +++ b/test/test_spotify.py @@ -89,7 +89,7 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): params = _params(responses.calls[0].request.url) self.assertEqual( params['q'], - [u'duifhjslkef album:lkajsdflakjsd artist:ujydfsuihse'], + [u'duifhjslkef artist:ujydfsuihse album:lkajsdflakjsd'], ) self.assertEqual(params['type'], [u'track']) From 2cda2b5b3a559ef2061f6bacc5ea0ef919cc53ef Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 22:53:23 -0800 Subject: [PATCH 08/19] Remove hardcoded ordering of filters in tests --- test/test_spotify.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_spotify.py b/test/test_spotify.py index 25a58911d..d6bec780d 100644 --- a/test/test_spotify.py +++ b/test/test_spotify.py @@ -122,10 +122,10 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): self.spotify._output_results(results) params = _params(responses.calls[0].request.url) - self.assertEqual( - params['q'], - [u'Happy artist:Pharrell Williams album:Despicable Me 2'], - ) + query = params['q'] + self.assertIn(u'Happy', query) + self.assertIn(u'artist:Pharrell Williams', query) + self.assertIn(u'album:Despicable Me 2', query) self.assertEqual(params['type'], [u'track']) From 0527edbd48a591597fe274d4073e154897d768a1 Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 23:05:47 -0800 Subject: [PATCH 09/19] Fix test index, add docstrings --- beetsplug/spotify.py | 21 +++++++++++++++++++++ test/test_spotify.py | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index a4179ef1c..3ae22cacb 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -313,6 +313,17 @@ class SpotifyPlugin(BeetsPlugin): def candidates(self, items, artist, album, va_likely): """Returns a list of AlbumInfo objects for Spotify Search API results matching an ``album`` and ``artist`` (if not various). + + :param items: List of tracks on the candidate album + :type items: list[beets.library.Item] + :param artist: The of the candidate album + :type artist: str + :param album: The name of the candidate album + :type album: str + :param va_likely: True if the candidate album likely has Various Artists + :type va_likely: bool + :return: Candidate AlbumInfo objects + :rtype: list[beets.autotag.hooks.AlbumInfo] """ query_filters = {'album': album} if not va_likely: @@ -330,6 +341,15 @@ class SpotifyPlugin(BeetsPlugin): def item_candidates(self, item, artist, title): """Returns a list of TrackInfo objects for Spotify Search API results matching ``title`` and ``artist``. + + :param item: Candidate track + :type item: beets.library.Item + :param artist: The artist of the candidate track + :type artist: str + :param title: The title of the candidate track + :type title: str + :return: Candidate TrackInfo objects + :rtype: list[beets.autotag.hooks.TrackInfo] """ response_data = self._search_spotify( query_type='track', keywords=title, filters={'artist': artist} @@ -374,6 +394,7 @@ class SpotifyPlugin(BeetsPlugin): :param filters: (Optional) Field filters to apply. :type filters: dict :param keywords: (Optional) Query keywords to use. + :type keywords: str :return: JSON data for the class:`Response ` object or None if no search results are returned :rtype: dict or None diff --git a/test/test_spotify.py b/test/test_spotify.py index d6bec780d..e27092011 100644 --- a/test/test_spotify.py +++ b/test/test_spotify.py @@ -122,7 +122,7 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): self.spotify._output_results(results) params = _params(responses.calls[0].request.url) - query = params['q'] + query = params['q'][0] self.assertIn(u'Happy', query) self.assertIn(u'artist:Pharrell Williams', query) self.assertIn(u'album:Despicable Me 2', query) From 77f9a930b7ab60370b470a65a666f2f7eeb4c592 Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 23:15:08 -0800 Subject: [PATCH 10/19] Fix remaining test, use official doc wording --- beetsplug/spotify.py | 17 +++++++++-------- test/test_spotify.py | 8 ++++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index 3ae22cacb..4d3f9afe6 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -313,14 +313,15 @@ class SpotifyPlugin(BeetsPlugin): def candidates(self, items, artist, album, va_likely): """Returns a list of AlbumInfo objects for Spotify Search API results matching an ``album`` and ``artist`` (if not various). - - :param items: List of tracks on the candidate album + + :param items: List of items comprised by an album to be matched :type items: list[beets.library.Item] - :param artist: The of the candidate album + :param artist: The artist of the album to be matched :type artist: str - :param album: The name of the candidate album + :param album: The name of the album to be matched :type album: str - :param va_likely: True if the candidate album likely has Various Artists + :param va_likely: True if the album to be matched likely has + Various Artists :type va_likely: bool :return: Candidate AlbumInfo objects :rtype: list[beets.autotag.hooks.AlbumInfo] @@ -342,11 +343,11 @@ class SpotifyPlugin(BeetsPlugin): """Returns a list of TrackInfo objects for Spotify Search API results matching ``title`` and ``artist``. - :param item: Candidate track + :param item: Singleton item to be matched :type item: beets.library.Item - :param artist: The artist of the candidate track + :param artist: The artist of the track to be matched :type artist: str - :param title: The title of the candidate track + :param title: The title of the track to be matched :type title: str :return: Candidate TrackInfo objects :rtype: list[beets.autotag.hooks.TrackInfo] diff --git a/test/test_spotify.py b/test/test_spotify.py index e27092011..06499e8d7 100644 --- a/test/test_spotify.py +++ b/test/test_spotify.py @@ -87,10 +87,10 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): self.assertEqual([], self.spotify._query_spotify(self.lib, u"")) params = _params(responses.calls[0].request.url) - self.assertEqual( - params['q'], - [u'duifhjslkef artist:ujydfsuihse album:lkajsdflakjsd'], - ) + query = params['q'][0] + self.assertIn(u'duifhjslkef', query) + self.assertIn(u'artist:ujydfsuihse', query) + self.assertIn(u'album:lkajsdflakjsd', query) self.assertEqual(params['type'], [u'track']) @responses.activate From 96fda0df0d40764bf5d052037552940eb50febd6 Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 23:36:51 -0800 Subject: [PATCH 11/19] Docstring formatting --- beetsplug/spotify.py | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index 4d3f9afe6..f0fbf9b52 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -106,7 +106,7 @@ class SpotifyPlugin(BeetsPlugin): :param params: (optional) list of tuples or bytes to send in the query string for the :class:`Request`. :type params: dict - :return: JSON data for the class:`Response ` object + :return: JSON data for the class:`Response ` object. :rtype: dict """ response = request_type( @@ -128,11 +128,11 @@ class SpotifyPlugin(BeetsPlugin): def _get_spotify_id(self, url_type, id_): """Parse a Spotify ID from its URL if necessary. - :param url_type: Type of Spotify URL, either 'album' or 'track' + :param url_type: Type of Spotify URL, either 'album' or 'track'. :type url_type: str - :param id_: Spotify ID or URL + :param id_: Spotify ID or URL. :type id_: str - :return: Spotify ID + :return: Spotify ID. :rtype: str """ # Spotify IDs consist of 22 alphanumeric characters @@ -314,16 +314,16 @@ class SpotifyPlugin(BeetsPlugin): """Returns a list of AlbumInfo objects for Spotify Search API results matching an ``album`` and ``artist`` (if not various). - :param items: List of items comprised by an album to be matched + :param items: List of items comprised by an album to be matched. :type items: list[beets.library.Item] - :param artist: The artist of the album to be matched + :param artist: The artist of the album to be matched. :type artist: str - :param album: The name of the album to be matched + :param album: The name of the album to be matched. :type album: str :param va_likely: True if the album to be matched likely has - Various Artists + Various Artists. :type va_likely: bool - :return: Candidate AlbumInfo objects + :return: Candidate AlbumInfo objects. :rtype: list[beets.autotag.hooks.AlbumInfo] """ query_filters = {'album': album} @@ -343,13 +343,13 @@ class SpotifyPlugin(BeetsPlugin): """Returns a list of TrackInfo objects for Spotify Search API results matching ``title`` and ``artist``. - :param item: Singleton item to be matched + :param item: Singleton item to be matched. :type item: beets.library.Item - :param artist: The artist of the track to be matched + :param artist: The artist of the track to be matched. :type artist: str - :param title: The title of the track to be matched + :param title: The title of the track to be matched. :type title: str - :return: Candidate TrackInfo objects + :return: Candidate TrackInfo objects. :rtype: list[beets.autotag.hooks.TrackInfo] """ response_data = self._search_spotify( @@ -366,14 +366,13 @@ class SpotifyPlugin(BeetsPlugin): def _construct_search_query(filters=None, keywords=''): """Construct a query string with the specified filters and keywords to be provided to the Spotify Search API - (https://developer.spotify.com/documentation/web-api/reference/search/search/). + (https://developer.spotify.com/documentation/web-api/reference/search/search/#writing-a-query---guidelines). :param filters: (Optional) Field filters to apply. :type filters: dict :param keywords: (Optional) Query keywords to use. :type keywords: str - :return: Search query string to be provided to the Search API - (https://developer.spotify.com/documentation/web-api/reference/search/search/#writing-a-query---guidelines) + :return: Query string to be provided to the Search API. :rtype: str """ query_string = keywords @@ -385,7 +384,7 @@ class SpotifyPlugin(BeetsPlugin): def _search_spotify(self, query_type, filters=None, keywords=''): """Query the Spotify Search API for the specified ``keywords``, applying - the provided filters. + the provided ``filters``. :param query_type: A comma-separated list of item types to search across. Valid types are: 'album', 'artist', 'playlist', and @@ -397,7 +396,7 @@ class SpotifyPlugin(BeetsPlugin): :param keywords: (Optional) Query keywords to use. :type keywords: str :return: JSON data for the class:`Response ` object or None - if no search results are returned + if no search results are returned. :rtype: dict or None """ query = self._construct_search_query( From 09fc53eaea7cce41a4c65e5f249bfac37b6f47bc Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Mon, 21 Jan 2019 23:53:19 -0800 Subject: [PATCH 12/19] Only parse Spotify ID when necessary --- beetsplug/spotify.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index f0fbf9b52..31d28c842 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -243,11 +243,10 @@ class SpotifyPlugin(BeetsPlugin): :return: TrackInfo object for track :rtype: beets.autotag.hooks.TrackInfo """ - spotify_id = self._get_spotify_id('track', track_id) - if spotify_id is None: - return None - if track_data is None: + spotify_id = self._get_spotify_id('track', track_id) + if spotify_id is None: + return None track_data = self._handle_response( requests.get, self.track_url + spotify_id ) From 3a67eae46d1a1726089c03e9e9b90f8c6b74d34f Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Tue, 22 Jan 2019 10:41:18 -0800 Subject: [PATCH 13/19] Use track attrs directly, better naming/docstrings --- beetsplug/spotify.py | 52 +++++++++++++++++++++++++++++--------------- test/test_spotify.py | 8 +++---- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index 31d28c842..cc21f8f05 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -261,7 +261,7 @@ class SpotifyPlugin(BeetsPlugin): for i, track_data in enumerate(album_data['tracks']['items']): if track_data['disc_number'] == track.medium: medium_total += 1 - if track_data['id'] == spotify_id: + if track_data['id'] == track.track_id: track.index = i + 1 track.medium_total = medium_total return track @@ -419,8 +419,8 @@ class SpotifyPlugin(BeetsPlugin): def queries(lib, opts, args): success = self._parse_opts(opts) if success: - results = self._query_spotify(lib, ui.decargs(args)) - self._output_results(results) + results = self._match_library_tracks(lib, ui.decargs(args)) + self._output_match_results(results) spotify_cmd = ui.Subcommand( 'spotify', help=u'build a Spotify playlist' @@ -458,11 +458,23 @@ class SpotifyPlugin(BeetsPlugin): self.opts = opts return True - def _query_spotify(self, lib, keywords): + def _match_library_tracks(self, library, keywords): + """ + Get a list of simplified track objects dicts for library tracks + matching the specified ``keywords``. + + :param library: beets library object to query. + :type library: beets.library.Library + :param keywords: Query to match library items against. + :type keywords: str + :return: List of simplified track object dicts for library items + matching the specified query. + :rtype: list[dict] + """ results = [] failures = [] - items = lib.items(keywords) + items = library.items(keywords) if not items: self._log.debug( @@ -509,9 +521,9 @@ class SpotifyPlugin(BeetsPlugin): region_filter = self.config['region_filter'].get() if region_filter: response_data_tracks = [ - x - for x in response_data_tracks - if region_filter in x['available_markets'] + track_data + for track_data in response_data_tracks + if region_filter in track_data['available_markets'] ] if ( @@ -552,16 +564,22 @@ class SpotifyPlugin(BeetsPlugin): return results - def _output_results(self, results): - if results: - ids = [x['id'] for x in results] - if self.config['mode'].get() == "open": - self._log.info(u'Attempting to open Spotify with playlist') - spotify_url = self.playlist_partial + ",".join(ids) - webbrowser.open(spotify_url) + def _output_match_results(self, results): + """ + Open a playlist or print Spotify URLs for the provided track object dicts. + :param results: List of simplified track object dicts + (https://developer.spotify.com/documentation/web-api/reference/object-model/#track-object-simplified) + :type results: list[dict] + """ + if results: + spotify_ids = [track_data['id'] for track_data in results] + if self.config['mode'].get() == 'open': + self._log.info(u'Attempting to open Spotify with playlist') + spotify_url = self.playlist_partial + ",".join(spotify_ids) + webbrowser.open(spotify_url) else: - for item in ids: - print(self.open_track_url + item) + for spotify_id in spotify_ids: + print(self.open_track_url + spotify_id) else: self._log.warning(u'No Spotify tracks found from beets query') diff --git a/test/test_spotify.py b/test/test_spotify.py index 06499e8d7..5c19cb0eb 100644 --- a/test/test_spotify.py +++ b/test/test_spotify.py @@ -59,7 +59,7 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): self.assertEqual(True, self.spotify._parse_opts(opts)) def test_empty_query(self): - self.assertEqual(None, self.spotify._query_spotify(self.lib, u"1=2")) + self.assertEqual(None, self.spotify._match_library_tracks(self.lib, u"1=2")) @responses.activate def test_missing_request(self): @@ -84,7 +84,7 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): length=10, ) item.add(self.lib) - self.assertEqual([], self.spotify._query_spotify(self.lib, u"")) + self.assertEqual([], self.spotify._match_library_tracks(self.lib, u"")) params = _params(responses.calls[0].request.url) query = params['q'][0] @@ -116,10 +116,10 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): length=10, ) item.add(self.lib) - results = self.spotify._query_spotify(self.lib, u"Happy") + results = self.spotify._match_library_tracks(self.lib, u"Happy") self.assertEqual(1, len(results)) self.assertEqual(u"6NPVjNh8Jhru9xOmyQigds", results[0]['id']) - self.spotify._output_results(results) + self.spotify._output_match_results(results) params = _params(responses.calls[0].request.url) query = params['q'][0] From 7b57b0b6087f52aa4d04f00af9eff2a05280d300 Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Tue, 22 Jan 2019 10:53:18 -0800 Subject: [PATCH 14/19] Appease Flake8 --- beetsplug/spotify.py | 5 +++-- test/test_spotify.py | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index cc21f8f05..c387125f6 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -462,7 +462,7 @@ class SpotifyPlugin(BeetsPlugin): """ Get a list of simplified track objects dicts for library tracks matching the specified ``keywords``. - + :param library: beets library object to query. :type library: beets.library.Library :param keywords: Query to match library items against. @@ -566,7 +566,8 @@ class SpotifyPlugin(BeetsPlugin): def _output_match_results(self, results): """ - Open a playlist or print Spotify URLs for the provided track object dicts. + Open a playlist or print Spotify URLs for the provided track + object dicts. :param results: List of simplified track object dicts (https://developer.spotify.com/documentation/web-api/reference/object-model/#track-object-simplified) diff --git a/test/test_spotify.py b/test/test_spotify.py index 5c19cb0eb..ea54a13db 100644 --- a/test/test_spotify.py +++ b/test/test_spotify.py @@ -59,7 +59,9 @@ class SpotifyPluginTest(_common.TestCase, TestHelper): self.assertEqual(True, self.spotify._parse_opts(opts)) def test_empty_query(self): - self.assertEqual(None, self.spotify._match_library_tracks(self.lib, u"1=2")) + self.assertEqual( + None, self.spotify._match_library_tracks(self.lib, u"1=2") + ) @responses.activate def test_missing_request(self): From f7d20090e67a8c0bd74eca62bdd53ca7559f273a Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Tue, 22 Jan 2019 12:14:52 -0800 Subject: [PATCH 15/19] Fix `_handle_response` reauth bug and empty str query construction --- beetsplug/spotify.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index c387125f6..51ef325e0 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -120,7 +120,7 @@ class SpotifyPlugin(BeetsPlugin): 'Spotify access token has expired. Reauthenticating.' ) self._authenticate() - self._handle_response(request_type, url, params=params) + return self._handle_response(request_type, url, params=params) else: raise ui.UserError(u'Spotify API error:\n{}', response.text) return response.json() @@ -374,12 +374,11 @@ class SpotifyPlugin(BeetsPlugin): :return: Query string to be provided to the Search API. :rtype: str """ - query_string = keywords - if filters is not None: - query_string += ' ' + ' '.join( - ':'.join((k, v)) for k, v in filters.items() - ) - return query_string + query_components = [ + keywords, + ' '.join(':'.join((k, v)) for k, v in filters.items()), + ] + return ' '.join([s for s in query_components if s]) def _search_spotify(self, query_type, filters=None, keywords=''): """Query the Spotify Search API for the specified ``keywords``, applying From c6b8f6d1437ec840f762d179702afddde94b5938 Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Tue, 22 Jan 2019 18:09:10 -0800 Subject: [PATCH 16/19] Fix formatting/spelling --- beetsplug/spotify.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index 51ef325e0..c841a3221 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -143,7 +143,7 @@ class SpotifyPlugin(BeetsPlugin): return match.group(2) if match else None def album_for_id(self, album_id): - """Fetches an album by its Spotify ID or URL and returns an + """Fetch an album by its Spotify ID or URL and return an AlbumInfo object or None if the album is not found. :param album_id: Spotify ID or URL for the album @@ -177,7 +177,7 @@ class SpotifyPlugin(BeetsPlugin): else: raise ui.UserError( u"Invalid `release_date_precision` returned " - u"from Spotify API: '{}'".format(release_date_precision) + u"by Spotify API: '{}'".format(release_date_precision) ) tracks = [] @@ -231,7 +231,7 @@ class SpotifyPlugin(BeetsPlugin): ) def track_for_id(self, track_id=None, track_data=None): - """Fetches a track by its Spotify ID or URL and returns a + """Fetch a track by its Spotify ID or URL and return a TrackInfo object or None if the track is not found. :param track_id: (Optional) Spotify ID or URL for the track. Either @@ -252,8 +252,9 @@ class SpotifyPlugin(BeetsPlugin): ) track = self._get_track(track_data) - # get album's tracks to set the track's index/position on - # the entire release + # Get album's tracks to set `track.index` (position on the entire + # release) and `track.medium_total` (total number of tracks on + # the track's disc). album_data = self._handle_response( requests.get, self.album_url + track_data['album']['id'] ) @@ -378,7 +379,7 @@ class SpotifyPlugin(BeetsPlugin): keywords, ' '.join(':'.join((k, v)) for k, v in filters.items()), ] - return ' '.join([s for s in query_components if s]) + return ' '.join([q for q in query_components if q]) def _search_spotify(self, query_type, filters=None, keywords=''): """Query the Spotify Search API for the specified ``keywords``, applying @@ -402,7 +403,9 @@ class SpotifyPlugin(BeetsPlugin): ) if not query: return None - self._log.debug(u'Searching Spotify for "{}"'.format(query)) + self._log.debug( + u'Searching Spotify for "{}"'.format(query.decode('utf8')) + ) response_data = self._handle_response( requests.get, self.search_url, @@ -458,8 +461,7 @@ class SpotifyPlugin(BeetsPlugin): return True def _match_library_tracks(self, library, keywords): - """ - Get a list of simplified track objects dicts for library tracks + """Get a list of simplified track object dicts for library tracks matching the specified ``keywords``. :param library: beets library object to query. @@ -564,8 +566,7 @@ class SpotifyPlugin(BeetsPlugin): return results def _output_match_results(self, results): - """ - Open a playlist or print Spotify URLs for the provided track + """Open a playlist or print Spotify URLs for the provided track object dicts. :param results: List of simplified track object dicts From b91406b252e6176d5e4430b4ae5ef187c7a9d1c4 Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Tue, 22 Jan 2019 18:59:15 -0800 Subject: [PATCH 17/19] Backwords-compatible str/unicode --- beetsplug/spotify.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index c841a3221..e297af940 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -9,6 +9,7 @@ import webbrowser import collections import requests +import six from beets import ui from beets.plugins import BeetsPlugin @@ -379,7 +380,10 @@ class SpotifyPlugin(BeetsPlugin): keywords, ' '.join(':'.join((k, v)) for k, v in filters.items()), ] - return ' '.join([q for q in query_components if q]) + query = ' '.join([q for q in query_components if q]) + if not isinstance(query, six.text_type): + query = query.decode('utf8') + return query def _search_spotify(self, query_type, filters=None, keywords=''): """Query the Spotify Search API for the specified ``keywords``, applying @@ -403,9 +407,7 @@ class SpotifyPlugin(BeetsPlugin): ) if not query: return None - self._log.debug( - u'Searching Spotify for "{}"'.format(query.decode('utf8')) - ) + self._log.debug(u"Searching Spotify for '{}'".format(query)) response_data = self._handle_response( requests.get, self.search_url, @@ -414,7 +416,9 @@ class SpotifyPlugin(BeetsPlugin): num_results = 0 for result_type_data in response_data.values(): num_results += len(result_type_data['items']) - self._log.debug(u'Found {} results from Spotify'.format(num_results)) + self._log.debug( + u"Found {} results from Spotify for '{}'", num_results, query + ) return response_data if num_results > 0 else None def commands(self): From 8c6cc6573c3ce775dc3717ca3c510aed908c929d Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Tue, 22 Jan 2019 19:42:25 -0800 Subject: [PATCH 18/19] Unidecode query string --- beetsplug/spotify.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index e297af940..032306f78 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -8,8 +8,9 @@ import base64 import webbrowser import collections -import requests import six +import unidecode +import requests from beets import ui from beets.plugins import BeetsPlugin @@ -383,7 +384,7 @@ class SpotifyPlugin(BeetsPlugin): query = ' '.join([q for q in query_components if q]) if not isinstance(query, six.text_type): query = query.decode('utf8') - return query + return unidecode.unidecode(query) def _search_spotify(self, query_type, filters=None, keywords=''): """Query the Spotify Search API for the specified ``keywords``, applying From 6a9b62a9c2d1919a036b3493b1db9c95128c6891 Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Tue, 22 Jan 2019 20:54:46 -0800 Subject: [PATCH 19/19] Specify None rtype in doscstrings --- beetsplug/spotify.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index 032306f78..b94d0e8e6 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -151,7 +151,7 @@ class SpotifyPlugin(BeetsPlugin): :param album_id: Spotify ID or URL for the album :type album_id: str :return: AlbumInfo object for album - :rtype: beets.autotag.hooks.AlbumInfo + :rtype: beets.autotag.hooks.AlbumInfo or None """ spotify_id = self._get_spotify_id('album', album_id) if spotify_id is None: @@ -243,7 +243,7 @@ class SpotifyPlugin(BeetsPlugin): provided instead of ``track_id`` to avoid unnecessary API calls. :type track_data: dict :return: TrackInfo object for track - :rtype: beets.autotag.hooks.TrackInfo + :rtype: beets.autotag.hooks.TrackInfo or None """ if track_data is None: spotify_id = self._get_spotify_id('track', track_id)