diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py index 8ef356fe8..c3933ed73 100644 --- a/beetsplug/beatport.py +++ b/beetsplug/beatport.py @@ -150,9 +150,11 @@ class BeatportClient(object): :rtype: :py:class:`BeatportRelease` """ response = self._get('/catalog/3/releases', id=beatport_id) - release = BeatportRelease(response[0]) - release.tracks = self.get_release_tracks(beatport_id) - return release + if response: + release = BeatportRelease(response[0]) + release.tracks = self.get_release_tracks(beatport_id) + return release + return None def get_release_tracks(self, beatport_id): """ Get all tracks for a given release. @@ -261,10 +263,10 @@ class BeatportTrack(BeatportObject): self.musical_key = six.text_type(data['key'].get('shortName')) # Use 'subgenre' and if not present, 'genre' as a fallback. - if 'subGenres' in data: + if data.get('subGenres'): self.genre = six.text_type(data['subGenres'][0].get('name')) - if not self.genre and 'genres' in data: - self.genre = six.text_type(data['genres'][0].get('name')) + elif data.get('genres'): + self.genre = six.text_type(data['genres'][0].get('name')) class BeatportPlugin(BeetsPlugin): @@ -375,27 +377,31 @@ class BeatportPlugin(BeetsPlugin): def album_for_id(self, release_id): """Fetches a release by its Beatport ID and returns an AlbumInfo object - or None if the release is not found. + or None if the query is not a valid ID or release is not found. """ self._log.debug(u'Searching for release {0}', release_id) match = re.search(r'(^|beatport\.com/release/.+/)(\d+)$', release_id) if not match: + self._log.debug(u'Not a valid Beatport release ID.') return None release = self.client.get_release(match.group(2)) - album = self._get_album_info(release) - return album + if release: + return self._get_album_info(release) + return None def track_for_id(self, track_id): """Fetches a track by its Beatport ID and returns a TrackInfo object - or None if the track is not found. + or None if the track is not a valid Beatport ID or track is not found. """ self._log.debug(u'Searching for track {0}', track_id) match = re.search(r'(^|beatport\.com/track/.+/)(\d+)$', track_id) if not match: + self._log.debug(u'Not a valid Beatport track ID.') return None bp_track = self.client.get_track(match.group(2)) - track = self._get_track_info(bp_track) - return track + if bp_track is not None: + return self._get_track_info(bp_track) + return None def _get_releases(self, query): """Returns a list of AlbumInfo objects for a beatport search query. diff --git a/docs/changelog.rst b/docs/changelog.rst index aa87929c6..261b74505 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -107,6 +107,8 @@ Fixes: * ``none_rec_action`` does not import automatically when ``timid`` is enabled. Thanks to :user:`RollingStar`. :bug:`3242` +* Fix a bug that caused a crash when tagging items with the beatport plugin. + :bug:`3374` For plugin developers: diff --git a/test/test_beatport.py b/test/test_beatport.py index a1591d58c..8e830df76 100644 --- a/test/test_beatport.py +++ b/test/test_beatport.py @@ -31,7 +31,7 @@ class BeatportTest(_common.TestCase, TestHelper): def _make_release_response(self): """Returns a dict that mimics a response from the beatport API. - The results were retrived from: + The results were retrieved from: https://oauth-api.beatport.com/catalog/3/releases?id=1742984 The list of elements on the returned dict is incomplete, including just those required for the tests on this class. @@ -72,7 +72,7 @@ class BeatportTest(_common.TestCase, TestHelper): def _make_tracks_response(self): """Return a list that mimics a response from the beatport API. - The results were retrived from: + The results were retrieved from: https://oauth-api.beatport.com/catalog/3/tracks?releaseId=1742984 The list of elements on the returned list is incomplete, including just those required for the tests on this class. @@ -425,7 +425,6 @@ class BeatportTest(_common.TestCase, TestHelper): # Set up 'test_album'. self.test_album = self.mk_test_album() - # print(self.test_album.keys()) # Set up 'test_tracks' self.test_tracks = self.test_album.items() @@ -559,6 +558,70 @@ class BeatportTest(_common.TestCase, TestHelper): self.assertEqual(track.genre, test_track.genre) +class BeatportResponseEmptyTest(_common.TestCase, TestHelper): + def _make_tracks_response(self): + results = [{ + "id": 7817567, + "name": "Mirage a Trois", + "genres": [{ + "id": 9, + "name": "Breaks", + "slug": "breaks", + "type": "genre" + }], + "subGenres": [{ + "id": 209, + "name": "Glitch Hop", + "slug": "glitch-hop", + "type": "subgenre" + }], + }] + return results + + def setUp(self): + self.setup_beets() + self.load_plugins('beatport') + self.lib = library.Library(':memory:') + + # Set up 'tracks'. + self.response_tracks = self._make_tracks_response() + self.tracks = [beatport.BeatportTrack(t) for t in self.response_tracks] + + # Make alias to be congruent with class `BeatportTest`. + self.test_tracks = self.response_tracks + + def tearDown(self): + self.unload_plugins() + self.teardown_beets() + + def test_response_tracks_empty(self): + response_tracks = [] + tracks = [beatport.BeatportTrack(t) for t in response_tracks] + self.assertEqual(tracks, []) + + def test_sub_genre_empty_fallback(self): + """No 'sub_genre' is provided. Test if fallback to 'genre' works. + """ + self.response_tracks[0]['subGenres'] = [] + tracks = [beatport.BeatportTrack(t) for t in self.response_tracks] + + self.test_tracks[0]['subGenres'] = [] + + self.assertEqual(tracks[0].genre, + self.test_tracks[0]['genres'][0]['name']) + + def test_genre_empty(self): + """No 'genre' is provided. Test if 'sub_genre' is applied. + """ + self.response_tracks[0]['genres'] = [] + tracks = [beatport.BeatportTrack(t) for t in self.response_tracks] + + self.test_tracks[0]['genres'] = [] + + self.assertEqual(tracks[0].genre, + self.test_tracks[0]['subGenres'][0]['name']) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__)