diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 86c5b958f..1bf8ad428 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -308,16 +308,44 @@ class CoverArtArchive(RemoteArtSource): VALID_THUMBNAIL_SIZES = [250, 500, 1200] if util.SNI_SUPPORTED: - URL = 'https://coverartarchive.org/release/{mbid}/front' - GROUP_URL = 'https://coverartarchive.org/release-group/{mbid}/front' + URL = 'https://coverartarchive.org/release/{mbid}' + GROUP_URL = 'https://coverartarchive.org/release-group/{mbid}' else: - URL = 'http://coverartarchive.org/release/{mbid}/front' - GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}/front' + URL = 'http://coverartarchive.org/release/{mbid}' + GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}' def get(self, album, plugin, paths): """Return the Cover Art Archive and Cover Art Archive release group URLs using album MusicBrainz release ID and release group ID. """ + + def get_image_urls(url, size_suffix=None): + try: + response = self.request(url) + except requests.RequestException: + self._log.debug(u'{0}: error receiving response' + .format(self.NAME)) + return + + try: + data = response.json() + except ValueError: + self._log.debug(u'{0}: error loading response: {1}' + .format(self.NAME, response.text)) + return + + for item in data.get('images', []): + try: + if 'Front' not in item['types']: + continue + + if size_suffix: + yield item['thumbnails'][size_suffix] + else: + yield item['image'] + except KeyError: + pass + release_url = self.URL.format(mbid=album.mb_albumid) release_group_url = self.GROUP_URL.format(mbid=album.mb_releasegroupid) @@ -330,19 +358,12 @@ class CoverArtArchive(RemoteArtSource): size_suffix = "-" + str(plugin.maxwidth) if 'release' in self.match_by and album.mb_albumid: - if size_suffix: - release_thumbnail_url = release_url + size_suffix - yield self._candidate(url=release_thumbnail_url, - match=Candidate.MATCH_EXACT) - yield self._candidate(url=release_url, - match=Candidate.MATCH_EXACT) + for url in get_image_urls(release_url, size_suffix): + yield self._candidate(url=url, match=Candidate.MATCH_EXACT) + if 'releasegroup' in self.match_by and album.mb_releasegroupid: - if size_suffix: - release_group_thumbnail_url = release_group_url + size_suffix - yield self._candidate(url=release_group_thumbnail_url, - match=Candidate.MATCH_FALLBACK) - yield self._candidate(url=release_group_url, - match=Candidate.MATCH_FALLBACK) + for url in get_image_urls(release_group_url): + yield self._candidate(url=url, match=Candidate.MATCH_FALLBACK) class Amazon(RemoteArtSource): diff --git a/docs/changelog.rst b/docs/changelog.rst index a9d2ac540..c89bdb681 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -148,6 +148,8 @@ New features: Thanks to :user:`logan-arens`. :bug:`2947` * Added flac-specific reporting of samplerate and bitrate when importing duplicates. +* :doc:`/plugins/fetchart`: Cover Art Archive source now iterates over + all front images instead of blindly selecting the first one. Fixes: diff --git a/test/test_art.py b/test/test_art.py index f4b3a6e62..51e5a9fe8 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -76,6 +76,96 @@ class FetchImageHelper(_common.TestCase): file_type, b'').ljust(32, b'\x00')) +class CAAHelper(): + """Helper mixin for mocking requests to the Cover Art Archive.""" + MBID_RELASE = 'rid' + MBID_GROUP = 'rgid' + + RELEASE_URL = 'coverartarchive.org/release/{0}' \ + .format(MBID_RELASE) + GROUP_URL = 'coverartarchive.org/release-group/{0}' \ + .format(MBID_GROUP) + + if util.SNI_SUPPORTED: + RELEASE_URL = "https://" + RELEASE_URL + GROUP_URL = "https://" + GROUP_URL + else: + RELEASE_URL = "http://" + RELEASE_URL + GROUP_URL = "http://" + GROUP_URL + + RESPONSE_RELEASE = """{ + "images": [ + { + "approved": false, + "back": false, + "comment": "GIF", + "edit": 12345, + "front": true, + "id": 12345, + "image": "http://coverartarchive.org/release/rid/12345.gif", + "thumbnails": { + "1200": "http://coverartarchive.org/release/rid/12345-1200.jpg", + "250": "http://coverartarchive.org/release/rid/12345-250.jpg", + "500": "http://coverartarchive.org/release/rid/12345-500.jpg", + "large": "http://coverartarchive.org/release/rid/12345-500.jpg", + "small": "http://coverartarchive.org/release/rid/12345-250.jpg" + }, + "types": [ + "Front" + ] + }, + { + "approved": false, + "back": false, + "comment": "", + "edit": 12345, + "front": false, + "id": 12345, + "image": "http://coverartarchive.org/release/rid/12345.jpg", + "thumbnails": { + "1200": "http://coverartarchive.org/release/rid/12345-1200.jpg", + "250": "http://coverartarchive.org/release/rid/12345-250.jpg", + "500": "http://coverartarchive.org/release/rid/12345-500.jpg", + "large": "http://coverartarchive.org/release/rid/12345-500.jpg", + "small": "http://coverartarchive.org/release/rid/12345-250.jpg" + }, + "types": [ + "Front" + ] + } + ], + "release": "https://musicbrainz.org/release/releaseid" +}""" + RESPONSE_GROUP = """{ + "images": [ + { + "approved": false, + "back": false, + "comment": "", + "edit": 12345, + "front": true, + "id": 12345, + "image": "http://coverartarchive.org/release/releaseid/12345.jpg", + "thumbnails": { + "1200": "http://coverartarchive.org/release/rgid/12345-1200.jpg", + "250": "http://coverartarchive.org/release/rgid/12345-250.jpg", + "500": "http://coverartarchive.org/release/rgid/12345-500.jpg", + "large": "http://coverartarchive.org/release/rgid/12345-500.jpg", + "small": "http://coverartarchive.org/release/rgid/12345-250.jpg" + }, + "types": [ + "Front" + ] + } + ], + "release": "https://musicbrainz.org/release/release-id" + }""" + + def mock_caa_response(self, url, json): + responses.add(responses.GET, url, body=json, + content_type='application/json') + + class FetchImageTest(FetchImageHelper, UseThePlugin): URL = 'http://example.com/test.jpg' @@ -156,15 +246,13 @@ class FSArtTest(UseThePlugin): self.assertEqual(candidates, paths) -class CombinedTest(FetchImageHelper, UseThePlugin): +class CombinedTest(FetchImageHelper, UseThePlugin, CAAHelper): ASIN = 'xxxx' MBID = 'releaseid' AMAZON_URL = 'https://images.amazon.com/images/P/{0}.01.LZZZZZZZ.jpg' \ .format(ASIN) AAO_URL = 'https://www.albumart.org/index_detail.php?asin={0}' \ .format(ASIN) - CAA_URL = 'coverartarchive.org/release/{0}/front' \ - .format(MBID) def setUp(self): super(CombinedTest, self).setUp() @@ -211,17 +299,19 @@ class CombinedTest(FetchImageHelper, UseThePlugin): self.assertEqual(responses.calls[-1].request.url, self.AAO_URL) def test_main_interface_uses_caa_when_mbid_available(self): - self.mock_response("http://" + self.CAA_URL) - self.mock_response("https://" + self.CAA_URL) - album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) + self.mock_caa_response(self.RELEASE_URL, self.RESPONSE_RELEASE) + self.mock_caa_response(self.GROUP_URL, self.RESPONSE_GROUP) + self.mock_response('http://coverartarchive.org/release/rid/12345.gif', + content_type='image/gif') + self.mock_response('http://coverartarchive.org/release/rid/12345.jpg', + content_type='image/jpeg') + album = _common.Bag(mb_albumid=self.MBID_RELASE, + mb_releasegroupid=self.MBID_GROUP, + asin=self.ASIN) candidate = self.plugin.art_for_album(album, None) self.assertIsNotNone(candidate) - self.assertEqual(len(responses.calls), 1) - if util.SNI_SUPPORTED: - url = "https://" + self.CAA_URL - else: - url = "http://" + self.CAA_URL - self.assertEqual(responses.calls[0].request.url, url) + self.assertEqual(len(responses.calls), 3) + self.assertEqual(responses.calls[0].request.url, self.RELEASE_URL) def test_local_only_does_not_access_network(self): album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) @@ -416,6 +506,28 @@ class GoogleImageTest(UseThePlugin): next(self.source.get(album, self.settings, [])) +class CoverArtArchiveTest(UseThePlugin, CAAHelper): + + def setUp(self): + super(CoverArtArchiveTest, self).setUp() + self.source = fetchart.CoverArtArchive(logger, self.plugin.config) + self.settings = Settings(maxwidth=0) + + @responses.activate + def run(self, *args, **kwargs): + super(CoverArtArchiveTest, self).run(*args, **kwargs) + + def test_caa_finds_image(self): + album = _common.Bag(mb_albumid=self.MBID_RELASE, + mb_releasegroupid=self.MBID_GROUP) + self.mock_caa_response(self.RELEASE_URL, self.RESPONSE_RELEASE) + self.mock_caa_response(self.GROUP_URL, self.RESPONSE_GROUP) + candidates = list(self.source.get(album, self.settings, [])) + self.assertEqual(len(candidates), 3) + self.assertEqual(len(responses.calls), 2) + self.assertEqual(responses.calls[0].request.url, self.RELEASE_URL) + + class FanartTVTest(UseThePlugin): RESPONSE_MULTIPLE = u"""{ "name": "artistname",