diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index d87a5dc48..91b77c0a4 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -69,7 +69,7 @@ class Candidate(object): self.match = match self.size = size - def _validate(self, extra): + def _validate(self, plugin): """Determine whether the candidate artwork is valid based on its dimensions (width and ratio). @@ -80,9 +80,7 @@ class Candidate(object): if not self.path: return self.CANDIDATE_BAD - if not (extra['enforce_ratio'] or - extra['minwidth'] or - extra['maxwidth']): + if not (plugin.enforce_ratio or plugin.minwidth or plugin.maxwidth): return self.CANDIDATE_EXACT # get_size returns None if no local imaging backend is available @@ -101,22 +99,22 @@ class Candidate(object): long_edge = max(self.size) # Check minimum size. - if extra['minwidth'] and self.size[0] < extra['minwidth']: + if plugin.minwidth and self.size[0] < plugin.minwidth: self._log.debug(u'image too small ({} < {})', - self.size[0], extra['minwidth']) + self.size[0], plugin.minwidth) return self.CANDIDATE_BAD # Check aspect ratio. edge_diff = long_edge - short_edge - if extra['enforce_ratio']: - if extra['margin_px']: - if edge_diff > extra['margin_px']: + if plugin.enforce_ratio: + if plugin.margin_px: + if edge_diff > plugin.margin_px: self._log.debug(u'image is not close enough to being ' u'square, ({} - {} > {})', - long_edge, short_edge, extra['margin_px']) + long_edge, short_edge, plugin.margin_px) return self.CANDIDATE_BAD - elif extra['margin_percent']: - margin_px = extra['margin_percent'] * long_edge + elif plugin.margin_percent: + margin_px = plugin.margin_percent * long_edge if edge_diff > margin_px: self._log.debug(u'image is not close enough to being ' u'square, ({} - {} > {})', @@ -129,20 +127,20 @@ class Candidate(object): return self.CANDIDATE_BAD # Check maximum size. - if extra['maxwidth'] and self.size[0] > extra['maxwidth']: + if plugin.maxwidth and self.size[0] > plugin.maxwidth: self._log.debug(u'image needs resizing ({} > {})', - self.size[0], extra['maxwidth']) + self.size[0], plugin.maxwidth) return self.CANDIDATE_DOWNSCALE return self.CANDIDATE_EXACT - def validate(self, extra): - self.check = self._validate(extra) + def validate(self, plugin): + self.check = self._validate(plugin) return self.check - def resize(self, extra): - if extra['maxwidth'] and self.check == self.CANDIDATE_DOWNSCALE: - self.path = ArtResizer.shared.resize(extra['maxwidth'], self.path) + def resize(self, plugin): + if plugin.maxwidth and self.check == self.CANDIDATE_DOWNSCALE: + self.path = ArtResizer.shared.resize(plugin.maxwidth, self.path) def _logged_get(log, *args, **kwargs): @@ -198,13 +196,13 @@ class ArtSource(RequestMixin): self._log = log self._config = config - def get(self, album, extra): + def get(self, album, plugin, paths): raise NotImplementedError() def _candidate(self, **kwargs): return Candidate(source=self, log=self._log, **kwargs) - def fetch_image(self, candidate, extra): + def fetch_image(self, candidate, plugin): raise NotImplementedError() @@ -212,7 +210,7 @@ class LocalArtSource(ArtSource): IS_LOCAL = True LOC_STR = u'local' - def fetch_image(self, candidate, extra): + def fetch_image(self, candidate, plugin): pass @@ -220,13 +218,13 @@ class RemoteArtSource(ArtSource): IS_LOCAL = False LOC_STR = u'remote' - def fetch_image(self, candidate, extra): + def fetch_image(self, candidate, plugin): """Downloads an image from a URL and checks whether it seems to actually be an image. If so, returns a path to the downloaded image. Otherwise, returns None. """ - if extra['maxwidth']: - candidate.url = ArtResizer.shared.proxy_url(extra['maxwidth'], + if plugin.maxwidth: + candidate.url = ArtResizer.shared.proxy_url(plugin.maxwidth, candidate.url) try: with closing(self.request(candidate.url, stream=True, @@ -299,7 +297,7 @@ class CoverArtArchive(RemoteArtSource): URL = 'http://coverartarchive.org/release/{mbid}/front' GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}/front' - def get(self, album, extra): + 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. """ @@ -317,7 +315,7 @@ class Amazon(RemoteArtSource): URL = 'http://images.amazon.com/images/P/%s.%02i.LZZZZZZZ.jpg' INDICES = (1, 2) - def get(self, album, extra): + def get(self, album, plugin, paths): """Generate URLs using Amazon ID (ASIN) string. """ if album.asin: @@ -331,7 +329,7 @@ class AlbumArtOrg(RemoteArtSource): URL = 'http://www.albumart.org/index_detail.php' PAT = r'href\s*=\s*"([^>"]*)"[^>]*title\s*=\s*"View larger image"' - def get(self, album, extra): + def get(self, album, plugin, paths): """Return art URL from AlbumArt.org using album ASIN. """ if not album.asin: @@ -362,7 +360,7 @@ class GoogleImages(RemoteArtSource): self.key = self._config['google_key'].get(), self.cx = self._config['google_engine'].get(), - def get(self, album, extra): + def get(self, album, plugin, paths): """Return art URL from google custom search engine given an album title and interpreter. """ @@ -406,7 +404,7 @@ class FanartTV(RemoteArtSource): super(FanartTV, self).__init__(*args, **kwargs) self.client_key = self._config['fanarttv_key'].get() - def get(self, album, extra): + def get(self, album, plugin, paths): if not album.mb_releasegroupid: return @@ -457,7 +455,7 @@ class FanartTV(RemoteArtSource): class ITunesStore(RemoteArtSource): NAME = u"iTunes Store" - def get(self, album, extra): + def get(self, album, plugin, paths): """Return art URL from iTunes Store given an album title. """ if not (album.albumartist and album.album): @@ -515,7 +513,7 @@ class Wikipedia(RemoteArtSource): }} Limit 1''' - def get(self, album, extra): + def get(self, album, plugin, paths): if not (album.albumartist and album.album): return @@ -627,16 +625,14 @@ class FileSystem(LocalArtSource): """ return [idx for (idx, x) in enumerate(cover_names) if x in filename] - def get(self, album, extra): + def get(self, album, plugin, paths): """Look for album art files in the specified directories. """ - paths = extra['paths'] if not paths: return - cover_names = list(map(util.bytestring_path, extra['cover_names'])) + cover_names = list(map(util.bytestring_path, plugin.cover_names)) cover_names_str = b'|'.join(cover_names) cover_pat = br''.join([br"(\b|_)(", cover_names_str, br")(\b|_)"]) - cautious = extra['cautious'] for path in paths: if not os.path.isdir(syspath(path)): @@ -666,7 +662,7 @@ class FileSystem(LocalArtSource): remaining.append(fn) # Fall back to any image in the folder. - if remaining and not cautious: + if remaining and not plugin.cautious: self._log.debug(u'using fallback art file {0}', util.displayable_path(remaining[0])) yield self._candidate(path=os.path.join(path, remaining[0]), @@ -845,16 +841,6 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): """ out = None - # all the information any of the sources might need - extra = {'paths': paths, - 'cover_names': self.cover_names, - 'cautious': self.cautious, - 'enforce_ratio': self.enforce_ratio, - 'margin_px': self.margin_px, - 'margin_percent': self.margin_percent, - 'minwidth': self.minwidth, - 'maxwidth': self.maxwidth} - for source in self.sources: if source.IS_LOCAL or not local_only: self._log.debug( @@ -864,9 +850,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): ) # URLs might be invalid at this point, or the image may not # fulfill the requirements - for candidate in source.get(album, extra): - source.fetch_image(candidate, extra) - if candidate.validate(extra): + for candidate in source.get(album, self, paths): + source.fetch_image(candidate, self) + if candidate.validate(self): out = candidate self._log.debug( u'using {0.LOC_STR} image {1}'.format( @@ -876,7 +862,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): break if out: - out.resize(extra) + out.resize(self) return out diff --git a/test/test_art.py b/test/test_art.py index 50ff26b00..d47b280c2 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -39,6 +39,13 @@ from beets.util import confit logger = logging.getLogger('beets.test_art') +class Settings(): + """Used to pass settings to the ArtSources when the plugin isn't fully + instantiated. + """ + pass + + class UseThePlugin(_common.TestCase): def setUp(self): super(UseThePlugin, self).setUp() @@ -73,28 +80,29 @@ class FetchImageTest(FetchImageHelper, UseThePlugin): super(FetchImageTest, self).setUp() self.dpath = os.path.join(self.temp_dir, b'arttest') self.source = fetchart.RemoteArtSource(logger, self.plugin.config) - self.extra = {'maxwidth': 0} + self.settings = Settings() + self.settings.maxwidth = 0 self.candidate = fetchart.Candidate(logger, url=self.URL) def test_invalid_type_returns_none(self): self.mock_response(self.URL, 'image/watercolour') - self.source.fetch_image(self.candidate, self.extra) + self.source.fetch_image(self.candidate, self.settings) self.assertEqual(self.candidate.path, None) def test_jpeg_type_returns_path(self): self.mock_response(self.URL, 'image/jpeg') - self.source.fetch_image(self.candidate, self.extra) + self.source.fetch_image(self.candidate, self.settings) self.assertNotEqual(self.candidate.path, None) def test_extension_set_by_content_type(self): self.mock_response(self.URL, 'image/png') - self.source.fetch_image(self.candidate, self.extra) + self.source.fetch_image(self.candidate, self.settings) self.assertEqual(os.path.splitext(self.candidate.path)[1], b'.png') self.assertExists(self.candidate.path) def test_does_not_rely_on_server_content_type(self): self.mock_response(self.URL, 'image/jpeg', 'image/png') - self.source.fetch_image(self.candidate, self.extra) + self.source.fetch_image(self.candidate, self.settings) self.assertEqual(os.path.splitext(self.candidate.path)[1], b'.png') self.assertExists(self.candidate.path) @@ -106,44 +114,44 @@ class FSArtTest(UseThePlugin): os.mkdir(self.dpath) self.source = fetchart.FileSystem(logger, self.plugin.config) - self.extra = {'cautious': False, - 'cover_names': ('art',), - 'paths': [self.dpath]} + self.settings = Settings() + self.settings.cautious = False + self.settings.cover_names = ('art',) def test_finds_jpg_in_directory(self): _common.touch(os.path.join(self.dpath, b'a.jpg')) - candidate = next(self.source.get(None, self.extra)) + candidate = next(self.source.get(None, self.settings, [self.dpath])) self.assertEqual(candidate.path, os.path.join(self.dpath, b'a.jpg')) def test_appropriately_named_file_takes_precedence(self): _common.touch(os.path.join(self.dpath, b'a.jpg')) _common.touch(os.path.join(self.dpath, b'art.jpg')) - candidate = next(self.source.get(None, self.extra)) + candidate = next(self.source.get(None, self.settings, [self.dpath])) self.assertEqual(candidate.path, os.path.join(self.dpath, b'art.jpg')) def test_non_image_file_not_identified(self): _common.touch(os.path.join(self.dpath, b'a.txt')) with self.assertRaises(StopIteration): - next(self.source.get(None, self.extra)) + next(self.source.get(None, self.settings, [self.dpath])) def test_cautious_skips_fallback(self): _common.touch(os.path.join(self.dpath, b'a.jpg')) - self.extra['cautious'] = True + self.settings.cautious = True with self.assertRaises(StopIteration): - next(self.source.get(None, self.extra)) + next(self.source.get(None, self.settings, [self.dpath])) def test_empty_dir(self): with self.assertRaises(StopIteration): - next(self.source.get(None, self.extra)) + next(self.source.get(None, self.settings, [self.dpath])) def test_precedence_amongst_correct_files(self): images = [b'front-cover.jpg', b'front.jpg', b'back.jpg'] paths = [os.path.join(self.dpath, i) for i in images] for p in paths: _common.touch(p) - self.extra['cover_names'] = ['cover', 'front', 'back'] + self.settings.cover_names = ['cover', 'front', 'back'] candidates = [candidate.path for candidate in - self.source.get(None, self.extra)] + self.source.get(None, self.settings, [self.dpath])] self.assertEqual(candidates, paths) @@ -236,7 +244,7 @@ class AAOTest(UseThePlugin): def setUp(self): super(AAOTest, self).setUp() self.source = fetchart.AlbumArtOrg(logger, self.plugin.config) - self.extra = dict() + self.settings = Settings() @responses.activate def run(self, *args, **kwargs): @@ -256,21 +264,21 @@ class AAOTest(UseThePlugin): """ self.mock_response(self.AAO_URL, body) album = _common.Bag(asin=self.ASIN) - candidate = next(self.source.get(album, self.extra)) + candidate = next(self.source.get(album, self.settings, [])) self.assertEqual(candidate.url, 'TARGET_URL') def test_aao_scraper_returns_no_result_when_no_image_present(self): self.mock_response(self.AAO_URL, 'blah blah') album = _common.Bag(asin=self.ASIN) with self.assertRaises(StopIteration): - next(self.source.get(album, self.extra)) + next(self.source.get(album, self.settings, [])) class GoogleImageTest(UseThePlugin): def setUp(self): super(GoogleImageTest, self).setUp() self.source = fetchart.GoogleImages(logger, self.plugin.config) - self.extra = dict() + self.settings = Settings() @responses.activate def run(self, *args, **kwargs): @@ -284,7 +292,7 @@ class GoogleImageTest(UseThePlugin): album = _common.Bag(albumartist="some artist", album="some album") json = '{"items": [{"link": "url_to_the_image"}]}' self.mock_response(fetchart.GoogleImages.URL, json) - candidate = next(self.source.get(album, self.extra)) + candidate = next(self.source.get(album, self.settings, [])) self.assertEqual(candidate.url, 'url_to_the_image') def test_google_art_returns_no_result_when_error_received(self): @@ -292,14 +300,14 @@ class GoogleImageTest(UseThePlugin): json = '{"error": {"errors": [{"reason": "some reason"}]}}' self.mock_response(fetchart.GoogleImages.URL, json) with self.assertRaises(StopIteration): - next(self.source.get(album, self.extra)) + next(self.source.get(album, self.settings, [])) def test_google_art_returns_no_result_with_malformed_response(self): album = _common.Bag(albumartist="some artist", album="some album") json = """bla blup""" self.mock_response(fetchart.GoogleImages.URL, json) with self.assertRaises(StopIteration): - next(self.source.get(album, self.extra)) + next(self.source.get(album, self.settings, [])) class FanartTVTest(UseThePlugin): @@ -363,7 +371,7 @@ class FanartTVTest(UseThePlugin): def setUp(self): super(FanartTVTest, self).setUp() self.source = fetchart.FanartTV(logger, self.plugin.config) - self.extra = dict() + self.settings = Settings() @responses.activate def run(self, *args, **kwargs): @@ -377,7 +385,7 @@ class FanartTVTest(UseThePlugin): album = _common.Bag(mb_releasegroupid=u'thereleasegroupid') self.mock_response(fetchart.FanartTV.API_ALBUMS + u'thereleasegroupid', self.RESPONSE_MULTIPLE) - candidate = next(self.source.get(album, self.extra)) + candidate = next(self.source.get(album, self.settings, [])) self.assertEqual(candidate.url, 'http://example.com/1.jpg') def test_fanarttv_returns_no_result_when_error_received(self): @@ -385,14 +393,14 @@ class FanartTVTest(UseThePlugin): self.mock_response(fetchart.FanartTV.API_ALBUMS + u'thereleasegroupid', self.RESPONSE_ERROR) with self.assertRaises(StopIteration): - next(self.source.get(album, self.extra)) + next(self.source.get(album, self.settings, [])) def test_fanarttv_returns_no_result_with_malformed_response(self): album = _common.Bag(mb_releasegroupid=u'thereleasegroupid') self.mock_response(fetchart.FanartTV.API_ALBUMS + u'thereleasegroupid', self.RESPONSE_MALFORMED) with self.assertRaises(StopIteration): - next(self.source.get(album, self.extra)) + next(self.source.get(album, self.settings, [])) def test_fanarttv_only_other_images(self): # The source used to fail when there were images present, but no cover @@ -400,7 +408,7 @@ class FanartTVTest(UseThePlugin): self.mock_response(fetchart.FanartTV.API_ALBUMS + u'thereleasegroupid', self.RESPONSE_NO_ART) with self.assertRaises(StopIteration): - next(self.source.get(album, self.extra)) + next(self.source.get(album, self.settings, [])) @_common.slow_test() @@ -528,8 +536,8 @@ class ArtForAlbumTest(UseThePlugin): self.old_fs_source_get = fetchart.FileSystem.get - def fs_source_get(_self, album, extra): - if extra['paths']: + def fs_source_get(_self, album, settings, paths): + if paths: yield fetchart.Candidate(logger, path=self.image_file) fetchart.FileSystem.get = fs_source_get @@ -657,5 +665,6 @@ class EnforceRatioConfigTest(_common.TestCase): def suite(): return unittest.TestLoader().loadTestsFromName(__name__) + if __name__ == '__main__': unittest.main(defaultTest='suite')