mirror of
https://github.com/beetbox/beets.git
synced 2026-01-07 08:32:06 +01:00
Merge pull request #2415 from beetbox/fetchart_cleanup
fetchart: Clean-up data structures
This commit is contained in:
wordofglass
GitHub
commit
c252b9256b
2 changed files with 76 additions and 81 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -39,6 +39,15 @@ 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.
|
||||
"""
|
||||
def __init__(self, **kwargs):
|
||||
for k, v in kwargs.items():
|
||||
setattr(self, k, v)
|
||||
|
||||
|
||||
class UseThePlugin(_common.TestCase):
|
||||
def setUp(self):
|
||||
super(UseThePlugin, self).setUp()
|
||||
|
|
@ -73,28 +82,28 @@ 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(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 +115,43 @@ 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(cautious=False,
|
||||
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')
|
||||
|
|
|
|||
Loading…
Reference in a new issue