From c0a041b87e629a23826da87bb6b14151065529dc Mon Sep 17 00:00:00 2001 From: wordofglass Date: Tue, 22 Mar 2016 19:42:16 +0100 Subject: [PATCH 01/20] fetchart: Unify handling of local and remote sources. --- beetsplug/fetchart.py | 230 +++++++++++++++++++++++------------------- 1 file changed, 125 insertions(+), 105 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index ad51bb995..6b205ae0c 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -99,11 +99,58 @@ class ArtSource(RequestMixin): self._log = log self._config = config - def get(self, album): + def get(self, album, extra): raise NotImplementedError() + def fetch_image(self, candidate, extra): + raise NotImplementedError() -class CoverArtArchive(ArtSource): +class LocalArtSource(ArtSource): + IS_LOCAL = True + LOC_STR = u'local' + + def fetch_image(self, candidate, extra): + return candidate + +class RemoteArtSource(ArtSource): + IS_LOCAL = False + LOC_STR = u'remote' + + def fetch_image(self, candidate, extra): + """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']: + url = ArtResizer.shared.proxy_url(extra['maxwidth'], candidate) + try: + with closing(self.request(url, stream=True, + message=u'downloading image')) as resp: + if 'Content-Type' not in resp.headers \ + or resp.headers['Content-Type'] not in CONTENT_TYPES: + self._log.debug( + u'not a supported image: {}', + resp.headers.get('Content-Type') or u'no content type', + ) + return None + + # Generate a temporary file with the correct extension. + with NamedTemporaryFile(suffix=DOWNLOAD_EXTENSION, + delete=False) as fh: + for chunk in resp.iter_content(chunk_size=1024): + fh.write(chunk) + self._log.debug(u'downloaded art to: {0}', + util.displayable_path(fh.name)) + return fh.name + + except (IOError, requests.RequestException, TypeError) as exc: + # Handling TypeError works around a urllib3 bug: + # https://github.com/shazow/urllib3/issues/556 + self._log.debug(u'error fetching art: {}', exc) + return None + + +class CoverArtArchive(RemoteArtSource): """Cover Art Archive""" URL = 'http://coverartarchive.org/release/{mbid}/front' GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}/front' @@ -118,7 +165,7 @@ class CoverArtArchive(ArtSource): yield self.GROUP_URL.format(mbid=album.mb_releasegroupid) -class Amazon(ArtSource): +class Amazon(RemoteArtSource): URL = 'http://images.amazon.com/images/P/%s.%02i.LZZZZZZZ.jpg' INDICES = (1, 2) @@ -130,7 +177,7 @@ class Amazon(ArtSource): yield self.URL % (album.asin, index) -class AlbumArtOrg(ArtSource): +class AlbumArtOrg(RemoteArtSource): """AlbumArt.org scraper""" URL = 'http://www.albumart.org/index_detail.php' PAT = r'href\s*=\s*"([^>"]*)"[^>]*title\s*=\s*"View larger image"' @@ -157,7 +204,7 @@ class AlbumArtOrg(ArtSource): self._log.debug(u'no image found on page') -class GoogleImages(ArtSource): +class GoogleImages(RemoteArtSource): URL = u'https://www.googleapis.com/customsearch/v1' def get(self, album): @@ -192,7 +239,7 @@ class GoogleImages(ArtSource): yield item['link'] -class ITunesStore(ArtSource): +class ITunesStore(RemoteArtSource): # Art from the iTunes Store. def get(self, album): """Return art URL from iTunes Store given an album title. @@ -226,7 +273,7 @@ class ITunesStore(ArtSource): self._log.debug(u'album not found in iTunes Store') -class Wikipedia(ArtSource): +class Wikipedia(RemoteArtSource): # Art from Wikipedia (queried through DBpedia) DBPEDIA_URL = 'http://dbpedia.org/sparql' WIKIPEDIA_URL = 'http://en.wikipedia.org/w/api.php' @@ -350,7 +397,7 @@ class Wikipedia(ArtSource): return -class FileSystem(ArtSource): +class FileSystem(LocalArtSource): """Art from the filesystem""" @staticmethod def filename_priority(filename, cover_names): @@ -362,43 +409,51 @@ class FileSystem(ArtSource): """ return [idx for (idx, x) in enumerate(cover_names) if x in filename] - def get(self, path, cover_names, cautious): - """Look for album art files in a specified directory. + def get(self, extra): + """Look for album art files in the specified directories. """ - if not os.path.isdir(path): - return - - # Find all files that look like images in the directory. - images = [] - for fn in os.listdir(path): - for ext in IMAGE_EXTENSIONS: - if fn.lower().endswith(b'.' + ext.encode('utf8')) and \ - os.path.isfile(os.path.join(path, fn)): - images.append(fn) - - # Look for "preferred" filenames. - images = sorted(images, - key=lambda x: self.filename_priority(x, cover_names)) + paths = extra['paths'] + cover_names = extra['cover_names'] + def sortkey(): + return self.filename_priority(x, cover_names) cover_pat = br"(\b|_)({0})(\b|_)".format(b'|'.join(cover_names)) - for fn in images: - if re.search(cover_pat, os.path.splitext(fn)[0], re.I): - self._log.debug(u'using well-named art file {0}', - util.displayable_path(fn)) - return os.path.join(path, fn) + cautious = extra['cautious'] + + for path in paths + if not os.path.isdir(path): + continue - # Fall back to any image in the folder. - if images and not cautious: - self._log.debug(u'using fallback art file {0}', - util.displayable_path(images[0])) - return os.path.join(path, images[0]) + # Find all files that look like images in the directory. + images = [] + for fn in os.listdir(path): + for ext in IMAGE_EXTENSIONS: + if fn.lower().endswith(b'.' + ext.encode('utf8')) and \ + os.path.isfile(os.path.join(path, fn)): + images.append(fn) + + # Look for "preferred" filenames. + images = sorted(images, sortkey) + for fn in images: + if re.search(cover_pat, os.path.splitext(fn)[0], re.I): + self._log.debug(u'using well-named art file {0}', + util.displayable_path(fn)) + yield os.path.join(path, fn) + + # Fall back to any image in the folder. + if images and not cautious: + self._log.debug(u'using fallback art file {0}', + util.displayable_path(images[0])) + yield os.path.join(path, images[0]) # Try each source in turn. -SOURCES_ALL = [u'coverart', u'itunes', u'amazon', u'albumart', +SOURCES_ALL = [u'filesysytem', + u'coverart', u'itunes', u'amazon', u'albumart', u'wikipedia', u'google'] ART_SOURCES = { + u'filesystem': FileSystem, u'coverart': CoverArtArchive, u'itunes': ITunesStore, u'albumart': AlbumArtOrg, @@ -422,7 +477,8 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): 'remote_priority': False, 'cautious': False, 'cover_names': ['cover', 'front', 'art', 'album', 'folder'], - 'sources': ['coverart', 'itunes', 'amazon', 'albumart'], + 'sources': ['filesystem', + 'coverart', 'itunes', 'amazon', 'albumart'], 'google_key': None, 'google_engine': u'001442825323518660753:hrh5ch1gjzm', }) @@ -449,9 +505,18 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): available_sources.remove(u'google') sources_name = plugins.sanitize_choices( self.config['sources'].as_str_seq(), available_sources) + if 'remote_priority' in self.config: + self._log.warning( + u'The `fetch_art.remote_priority` configuration option has ' + u'been deprecated, see the documentation.') + if self.config['remote_priority'].get(bool): + try: + self.sources_name.remove[u'filesystem'] + sources_name.append[u'filesystem'] + except ValueError: + pass self.sources = [ART_SOURCES[s](self._log, self.config) for s in sources_name] - self.fs_source = FileSystem(self._log, self.config) # Asynchronous; after music is added to the library. def fetch_art(self, session, task): @@ -505,37 +570,6 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): # Utilities converted from functions to methods on logging overhaul - def _fetch_image(self, url): - """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. - """ - try: - with closing(self.request(url, stream=True, - message=u'downloading image')) as resp: - if 'Content-Type' not in resp.headers \ - or resp.headers['Content-Type'] not in CONTENT_TYPES: - self._log.debug( - u'not a supported image: {}', - resp.headers.get('Content-Type') or u'no content type', - ) - return None - - # Generate a temporary file with the correct extension. - with NamedTemporaryFile(suffix=DOWNLOAD_EXTENSION, - delete=False) as fh: - for chunk in resp.iter_content(chunk_size=1024): - fh.write(chunk) - self._log.debug(u'downloaded art to: {0}', - util.displayable_path(fh.name)) - return fh.name - - except (IOError, requests.RequestException, TypeError) as exc: - # Handling TypeError works around a urllib3 bug: - # https://github.com/shazow/urllib3/issues/556 - self._log.debug(u'error fetching art: {}', exc) - return None - def _is_valid_image_candidate(self, candidate): """Determine whether the given candidate artwork is valid based on its dimensions (width and ratio). @@ -591,30 +625,33 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): out = None check = None - # Local art. cover_names = self.config['cover_names'].as_str_seq() cover_names = map(util.bytestring_path, cover_names) cautious = self.config['cautious'].get(bool) - if paths: - for path in paths: - candidate = self.fs_source.get(path, cover_names, cautious) - check = self._is_valid_image_candidate(candidate) - if check: - out = candidate - self._log.debug(u'found local image {}', out) - break + extra = {'paths': paths, + 'cover_names': cover_names, + 'cautious': cautious, + 'maxwidth': self.maxwidth} + source_names = {v: k for k, v in ART_SOURCES.items()} - # Web art sources. - remote_priority = self.config['remote_priority'].get(bool) - if not local_only and (remote_priority or not out): - for url in self._source_urls(album): - if self.maxwidth: - url = ArtResizer.shared.proxy_url(self.maxwidth, url) - candidate = self._fetch_image(url) - check = self._is_valid_image_candidate(candidate) - if check: - out = candidate - self._log.debug(u'using remote image {}', out) + for source in self.sources: + if source.is_local or not local_only: + self._log.debug( + u'trying source {0} for album {1.albumartist} - {1.album}', + source_names[type(source)], + album, + ) + # URLs might be invalid at this point, or the image may not + # fulfill the requirements + for candidate in source.get(album, extra): + candidate = source.fetch_image(candidate) + check = self._is_valid_image_candidate(candidate) + if check: + out = candidate + self._log.debug(u'using {0.LOC_STR()} image {1}' + .format(source, out)) + break + if out: break if self.maxwidth and out and check == CANDIDATE_DOWNSCALE: @@ -645,20 +682,3 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): self._log.info(u'{0}: {1}', album, message) - def _source_urls(self, album): - """Generate possible source URLs for an album's art. The URLs are - not guaranteed to work so they each need to be attempted in turn. - This allows the main `art_for_album` function to abort iteration - through this sequence early to avoid the cost of scraping when not - necessary. - """ - source_names = {v: k for k, v in ART_SOURCES.items()} - for source in self.sources: - self._log.debug( - u'trying source {0} for album {1.albumartist} - {1.album}', - source_names[type(source)], - album, - ) - urls = source.get(album) - for url in urls: - yield url From b37cc02f1a5820c4d9c21629efa777d5191d5740 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Tue, 22 Mar 2016 20:52:25 +0100 Subject: [PATCH 02/20] fetchart: Introduce the Candidate class, some fixes to the previous commit. --- beetsplug/fetchart.py | 201 ++++++++++++++++++++++++++---------------- 1 file changed, 124 insertions(+), 77 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 6b205ae0c..63d56ce44 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -45,6 +45,75 @@ CANDIDATE_BAD = 0 CANDIDATE_EXACT = 1 CANDIDATE_DOWNSCALE = 2 +MATCH_EXACT = 0 +MATCH_FALLBACK = 1 + +class Candidate(): + """ + + """ + def __init__(self, path=None, url=None, source=u'', match=None): + self.path = path + self.source = source + self.check = None + self.match = match + self.size = None + + def _validate(self, extra): + """Determine whether the candidate artwork is valid based on + its dimensions (width and ratio). + + Return `CANDIDATE_BAD` if the file is unusable. + Return `CANDIDATE_EXACT` if the file is usable as-is. + Return `CANDIDATE_DOWNSCALE` if the file must be resized. + """ + if not self.path: + return CANDIDATE_BAD + + if not (extra.['enforce_ratio'] or + extra.['minwidth'] or + extra.['maxwidth']): + return CANDIDATE_EXACT + + # get_size returns None if no local imaging backend is available + self.size = ArtResizer.shared.get_size(self.path) + self._log.debug(u'image size: {}', self.size) + + if not self.size: + self._log.warning(u'Could not get size of image (please see ' + u'documentation for dependencies). ' + u'The configuration options `minwidth` and ' + u'`enforce_ratio` may be violated.') + return CANDIDATE_EXACT + + # Check minimum size. + if extra.['minwidth'] and self.size[0] < extra.['minwidth']: + self._log.debug(u'image too small ({} < {})', + self.size[0], extra.['minwidth']) + return CANDIDATE_BAD + + # Check aspect ratio. + if extra.['enforce_ratio'] and self.size[0] != self.size[1]: + self._log.debug(u'image is not square ({} != {})', + self.size[0], self.size[1]) + return CANDIDATE_BAD + + # Check maximum size. + if extra.['maxwidth'] and self.size[0] > extra.['maxwidth']: + self._log.debug(u'image needs resizing ({} > {})', + self.size[0], extra.['maxwidth']) + return CANDIDATE_DOWNSCALE + + return CANDIDATE_EXACT + + def validate(self, extra): + self.check = self._validate(extra) + return self.check + + def resize(self, extra): + if extra.['maxwidth'] and self.check == CANDIDATE_DOWNSCALE: + self.path = ArtResizer.shared.resize(extra.['maxwidth'], self.path) + def _logged_get(log, *args, **kwargs): """Like `requests.get`, but logs the effective URL to the specified @@ -122,9 +191,10 @@ class RemoteArtSource(ArtSource): Otherwise, returns None. """ if extra['maxwidth']: - url = ArtResizer.shared.proxy_url(extra['maxwidth'], candidate) + candidate.url = ArtResizer.shared.proxy_url(extra['maxwidth'], + candidate.url) try: - with closing(self.request(url, stream=True, + with closing(self.request(candidate.url, stream=True, message=u'downloading image')) as resp: if 'Content-Type' not in resp.headers \ or resp.headers['Content-Type'] not in CONTENT_TYPES: @@ -132,7 +202,8 @@ class RemoteArtSource(ArtSource): u'not a supported image: {}', resp.headers.get('Content-Type') or u'no content type', ) - return None + candidate.path = None + return # Generate a temporary file with the correct extension. with NamedTemporaryFile(suffix=DOWNLOAD_EXTENSION, @@ -141,13 +212,15 @@ class RemoteArtSource(ArtSource): fh.write(chunk) self._log.debug(u'downloaded art to: {0}', util.displayable_path(fh.name)) - return fh.name + candidate.path = fh.name + return except (IOError, requests.RequestException, TypeError) as exc: # Handling TypeError works around a urllib3 bug: # https://github.com/shazow/urllib3/issues/556 self._log.debug(u'error fetching art: {}', exc) - return None + candidate.path = None + return class CoverArtArchive(RemoteArtSource): @@ -155,26 +228,32 @@ class CoverArtArchive(RemoteArtSource): URL = 'http://coverartarchive.org/release/{mbid}/front' GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}/front' - def get(self, album): + def get(self, album, extra): """Return the Cover Art Archive and Cover Art Archive release group URLs using album MusicBrainz release ID and release group ID. """ if album.mb_albumid: - yield self.URL.format(mbid=album.mb_albumid) + yield Candidate(url=self.URL.format(mbid=album.mb_albumid), + source=u'coverartarchive.org', + match=MATCH_EXACT) if album.mb_releasegroupid: - yield self.GROUP_URL.format(mbid=album.mb_releasegroupid) + yield Candidate(url=self.GROUP_URL.format(mbid=album.mb_releasegroupid), + source=u'coverartarchive.org', + match=MATCH_FALLBACK) class Amazon(RemoteArtSource): URL = 'http://images.amazon.com/images/P/%s.%02i.LZZZZZZZ.jpg' INDICES = (1, 2) - def get(self, album): + def get(self, album, extra): """Generate URLs using Amazon ID (ASIN) string. """ if album.asin: for index in self.INDICES: - yield self.URL % (album.asin, index) + yield Candidate(url=self.URL % (album.asin, index), + source=u'Amazon', + match=MATCH_EXACT) class AlbumArtOrg(RemoteArtSource): @@ -182,7 +261,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): + def get(self, album, extra): """Return art URL from AlbumArt.org using album ASIN. """ if not album.asin: @@ -199,7 +278,9 @@ class AlbumArtOrg(RemoteArtSource): m = re.search(self.PAT, resp.text) if m: image_url = m.group(1) - yield image_url + yield Candidate(url=image_url, + source=u'AlbumArt.org', + match=MATCH_EXACT) else: self._log.debug(u'no image found on page') @@ -207,7 +288,7 @@ class AlbumArtOrg(RemoteArtSource): class GoogleImages(RemoteArtSource): URL = u'https://www.googleapis.com/customsearch/v1' - def get(self, album): + def get(self, album, extra): """Return art URL from google custom search engine given an album title and interpreter. """ @@ -236,12 +317,14 @@ class GoogleImages(RemoteArtSource): if 'items' in data.keys(): for item in data['items']: - yield item['link'] + yield Candidate(url=item['link'], + source=u'Google images', + match=MATCH_EXACT) class ITunesStore(RemoteArtSource): # Art from the iTunes Store. - def get(self, album): + def get(self, album, extra): """Return art URL from iTunes Store given an album title. """ if not (album.albumartist and album.album): @@ -266,7 +349,9 @@ class ITunesStore(RemoteArtSource): if itunes_album.get_artwork()['100']: small_url = itunes_album.get_artwork()['100'] big_url = small_url.replace('100x100', '1200x1200') - yield big_url + yield Candidate(url=big_url, + source=u'iTunes Store', + match=MATCH_EXACT) else: self._log.debug(u'album has no artwork in iTunes Store') except IndexError: @@ -299,7 +384,7 @@ class Wikipedia(RemoteArtSource): }} Limit 1''' - def get(self, album): + def get(self, album, extra): if not (album.albumartist and album.album): return @@ -391,7 +476,9 @@ class Wikipedia(RemoteArtSource): results = data['query']['pages'] for _, result in results.iteritems(): image_url = result['imageinfo'][0]['url'] - yield image_url + yield Candidate(url=image_url, + source=u'Wikipedia', + match=MATCH_EXACT) except (ValueError, KeyError, IndexError): self._log.debug(u'wikipedia: error scraping imageinfo') return @@ -409,13 +496,11 @@ class FileSystem(LocalArtSource): """ return [idx for (idx, x) in enumerate(cover_names) if x in filename] - def get(self, extra): + def get(self, album, extra): """Look for album art files in the specified directories. """ paths = extra['paths'] cover_names = extra['cover_names'] - def sortkey(): - return self.filename_priority(x, cover_names) cover_pat = br"(\b|_)({0})(\b|_)".format(b'|'.join(cover_names)) cautious = extra['cautious'] @@ -432,18 +517,23 @@ class FileSystem(LocalArtSource): images.append(fn) # Look for "preferred" filenames. - images = sorted(images, sortkey) + images = sorted(images, + lambda x: self.filename_priority(x, cover_names)) for fn in images: if re.search(cover_pat, os.path.splitext(fn)[0], re.I): self._log.debug(u'using well-named art file {0}', util.displayable_path(fn)) - yield os.path.join(path, fn) + yield Candidate(path=os.path.join(path, fn), + source=u'Filesystem', + match=MATCH_EXACT) # Fall back to any image in the folder. if images and not cautious: self._log.debug(u'using fallback art file {0}', util.displayable_path(images[0])) - yield os.path.join(path, images[0]) + yield Candidate(path=os.path.join(path, images[0]), + source=u'Filesystem', + match=MATCH_FALLBACK) # Try each source in turn. @@ -461,6 +551,7 @@ ART_SOURCES = { u'wikipedia': Wikipedia, u'google': GoogleImages, } +SOURCE_NAMES = {v: k for k, v in ART_SOURCES.items()} # PLUGIN LOGIC ############################################################### @@ -570,51 +661,6 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): # Utilities converted from functions to methods on logging overhaul - def _is_valid_image_candidate(self, candidate): - """Determine whether the given candidate artwork is valid based on - its dimensions (width and ratio). - - Return `CANDIDATE_BAD` if the file is unusable. - Return `CANDIDATE_EXACT` if the file is usable as-is. - Return `CANDIDATE_DOWNSCALE` if the file must be resized. - """ - if not candidate: - return CANDIDATE_BAD - - if not (self.enforce_ratio or self.minwidth or self.maxwidth): - return CANDIDATE_EXACT - - # get_size returns None if no local imaging backend is available - size = ArtResizer.shared.get_size(candidate) - self._log.debug(u'image size: {}', size) - - if not size: - self._log.warning(u'Could not get size of image (please see ' - u'documentation for dependencies). ' - u'The configuration options `minwidth` and ' - u'`enforce_ratio` may be violated.') - return CANDIDATE_EXACT - - # Check minimum size. - if self.minwidth and size[0] < self.minwidth: - self._log.debug(u'image too small ({} < {})', - size[0], self.minwidth) - return CANDIDATE_BAD - - # Check aspect ratio. - if self.enforce_ratio and size[0] != size[1]: - self._log.debug(u'image is not square ({} != {})', - size[0], size[1]) - return CANDIDATE_BAD - - # Check maximum size. - if self.maxwidth and size[0] > self.maxwidth: - self._log.debug(u'image needs resizing ({} > {})', - size[0], self.maxwidth) - return CANDIDATE_DOWNSCALE - - return CANDIDATE_EXACT - def art_for_album(self, album, paths, local_only=False): """Given an Album object, returns a path to downloaded art for the album (or None if no art is found). If `maxwidth`, then images are @@ -625,37 +671,38 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): out = None check = None + # all the information any of the sources might need cover_names = self.config['cover_names'].as_str_seq() cover_names = map(util.bytestring_path, cover_names) cautious = self.config['cautious'].get(bool) extra = {'paths': paths, 'cover_names': cover_names, 'cautious': cautious, + 'enforce_ratio': self.enforce_ratio, + 'minwidth': self.minwidth, 'maxwidth': self.maxwidth} - source_names = {v: k for k, v in ART_SOURCES.items()} for source in self.sources: if source.is_local or not local_only: self._log.debug( u'trying source {0} for album {1.albumartist} - {1.album}', - source_names[type(source)], + SOURCE_NAMES[type(source)], album, ) # URLs might be invalid at this point, or the image may not # fulfill the requirements for candidate in source.get(album, extra): - candidate = source.fetch_image(candidate) - check = self._is_valid_image_candidate(candidate) - if check: + source.fetch_image(candidate) + if candidate.validate(extra): out = candidate self._log.debug(u'using {0.LOC_STR()} image {1}' - .format(source, out)) + .format(source, out.path)) break if out: break - if self.maxwidth and out and check == CANDIDATE_DOWNSCALE: - out = ArtResizer.shared.resize(self.maxwidth, out) + if out: + out.resize(extra) return out From 1cfd039da43ebeca210e429171b1c40a3e6bde0c Mon Sep 17 00:00:00 2001 From: wordofglass Date: Tue, 22 Mar 2016 21:09:05 +0100 Subject: [PATCH 03/20] Update fetchart documentation. --- docs/plugins/fetchart.rst | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/plugins/fetchart.rst b/docs/plugins/fetchart.rst index e53dbc219..132efd5b5 100644 --- a/docs/plugins/fetchart.rst +++ b/docs/plugins/fetchart.rst @@ -43,15 +43,14 @@ file. The available options are: pixels. The height is recomputed so that the aspect ratio is preserved. - **enforce_ratio**: Only images with a width:height ratio of 1:1 are considered as valid album art candidates. Default: ``no``. -- **remote_priority**: Query remote sources every time and use local image only - as fallback. - Default: ``no``; remote (Web) art sources are only queried if no local art is - found in the filesystem. - **sources**: List of sources to search for images. An asterisk `*` expands to all available sources. - Default: ``coverart itunes amazon albumart``, i.e., everything but + Default: ``filesystem coverart itunes amazon albumart``, i.e., everything but ``wikipedia`` and ``google``. Enable those two sources for more matches at - the cost of some speed. + the cost of some speed. They are searched in the given order, thus in the + default config, no remote (Web) art source are queried if local art is + found in the filesystem. To use a local image as fallback, move it to the end + of the list. - **google_key**: Your Google API key (to enable the Google Custom Search backend). Default: None. From 8356e7d33d5a6d53152445a52789793a9202305b Mon Sep 17 00:00:00 2001 From: wordofglass Date: Wed, 23 Mar 2016 00:47:11 +0100 Subject: [PATCH 04/20] fix syntax --- beetsplug/fetchart.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 63d56ce44..3e954feff 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -70,9 +70,9 @@ class Candidate(): if not self.path: return CANDIDATE_BAD - if not (extra.['enforce_ratio'] or - extra.['minwidth'] or - extra.['maxwidth']): + if not (extra['enforce_ratio'] or + extra['minwidth'] or + extra['maxwidth']): return CANDIDATE_EXACT # get_size returns None if no local imaging backend is available @@ -87,21 +87,21 @@ class Candidate(): return CANDIDATE_EXACT # Check minimum size. - if extra.['minwidth'] and self.size[0] < extra.['minwidth']: + if extra['minwidth'] and self.size[0] < extra['minwidth']: self._log.debug(u'image too small ({} < {})', - self.size[0], extra.['minwidth']) + self.size[0], extra['minwidth']) return CANDIDATE_BAD # Check aspect ratio. - if extra.['enforce_ratio'] and self.size[0] != self.size[1]: + if extra['enforce_ratio'] and self.size[0] != self.size[1]: self._log.debug(u'image is not square ({} != {})', self.size[0], self.size[1]) return CANDIDATE_BAD # Check maximum size. - if extra.['maxwidth'] and self.size[0] > extra.['maxwidth']: + if extra['maxwidth'] and self.size[0] > extra['maxwidth']: self._log.debug(u'image needs resizing ({} > {})', - self.size[0], extra.['maxwidth']) + self.size[0], extra['maxwidth']) return CANDIDATE_DOWNSCALE return CANDIDATE_EXACT @@ -111,8 +111,8 @@ class Candidate(): return self.check def resize(self, extra): - if extra.['maxwidth'] and self.check == CANDIDATE_DOWNSCALE: - self.path = ArtResizer.shared.resize(extra.['maxwidth'], self.path) + if extra['maxwidth'] and self.check == CANDIDATE_DOWNSCALE: + self.path = ArtResizer.shared.resize(extra['maxwidth'], self.path) def _logged_get(log, *args, **kwargs): @@ -504,7 +504,7 @@ class FileSystem(LocalArtSource): cover_pat = br"(\b|_)({0})(\b|_)".format(b'|'.join(cover_names)) cautious = extra['cautious'] - for path in paths + for path in paths: if not os.path.isdir(path): continue From 50bf28edd4e4707af930ae7e1dcb95ce0d70032e Mon Sep 17 00:00:00 2001 From: wordofglass Date: Wed, 23 Mar 2016 16:46:39 +0100 Subject: [PATCH 05/20] move constants into Candidate --- beetsplug/fetchart.py | 50 +++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 3e954feff..08e8988ff 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -41,17 +41,17 @@ IMAGE_EXTENSIONS = ['png', 'jpg', 'jpeg'] CONTENT_TYPES = ('image/jpeg', 'image/png') DOWNLOAD_EXTENSION = '.jpg' -CANDIDATE_BAD = 0 -CANDIDATE_EXACT = 1 -CANDIDATE_DOWNSCALE = 2 - -MATCH_EXACT = 0 -MATCH_FALLBACK = 1 - class Candidate(): """ """ + CANDIDATE_BAD = 0 + CANDIDATE_EXACT = 1 + CANDIDATE_DOWNSCALE = 2 + + MATCH_EXACT = 0 + MATCH_FALLBACK = 1 + def __init__(self, path=None, url=None, source=u'', match=None): self.path = path self.source = source @@ -68,12 +68,12 @@ class Candidate(): Return `CANDIDATE_DOWNSCALE` if the file must be resized. """ if not self.path: - return CANDIDATE_BAD + return self.CANDIDATE_BAD if not (extra['enforce_ratio'] or extra['minwidth'] or extra['maxwidth']): - return CANDIDATE_EXACT + return self.CANDIDATE_EXACT # get_size returns None if no local imaging backend is available self.size = ArtResizer.shared.get_size(self.path) @@ -84,34 +84,34 @@ class Candidate(): u'documentation for dependencies). ' u'The configuration options `minwidth` and ' u'`enforce_ratio` may be violated.') - return CANDIDATE_EXACT + return self.CANDIDATE_EXACT # Check minimum size. if extra['minwidth'] and self.size[0] < extra['minwidth']: self._log.debug(u'image too small ({} < {})', self.size[0], extra['minwidth']) - return CANDIDATE_BAD + return self.CANDIDATE_BAD # Check aspect ratio. if extra['enforce_ratio'] and self.size[0] != self.size[1]: self._log.debug(u'image is not square ({} != {})', self.size[0], self.size[1]) - return CANDIDATE_BAD + return self.CANDIDATE_BAD # Check maximum size. if extra['maxwidth'] and self.size[0] > extra['maxwidth']: self._log.debug(u'image needs resizing ({} > {})', self.size[0], extra['maxwidth']) - return CANDIDATE_DOWNSCALE + return self.CANDIDATE_DOWNSCALE - return CANDIDATE_EXACT + return self.CANDIDATE_EXACT def validate(self, extra): self.check = self._validate(extra) return self.check def resize(self, extra): - if extra['maxwidth'] and self.check == CANDIDATE_DOWNSCALE: + if extra['maxwidth'] and self.check == self.CANDIDATE_DOWNSCALE: self.path = ArtResizer.shared.resize(extra['maxwidth'], self.path) @@ -235,11 +235,11 @@ class CoverArtArchive(RemoteArtSource): if album.mb_albumid: yield Candidate(url=self.URL.format(mbid=album.mb_albumid), source=u'coverartarchive.org', - match=MATCH_EXACT) + match=Candidate.MATCH_EXACT) if album.mb_releasegroupid: yield Candidate(url=self.GROUP_URL.format(mbid=album.mb_releasegroupid), source=u'coverartarchive.org', - match=MATCH_FALLBACK) + match=Candidate.MATCH_FALLBACK) class Amazon(RemoteArtSource): @@ -253,7 +253,7 @@ class Amazon(RemoteArtSource): for index in self.INDICES: yield Candidate(url=self.URL % (album.asin, index), source=u'Amazon', - match=MATCH_EXACT) + match=Candidate.MATCH_EXACT) class AlbumArtOrg(RemoteArtSource): @@ -280,7 +280,7 @@ class AlbumArtOrg(RemoteArtSource): image_url = m.group(1) yield Candidate(url=image_url, source=u'AlbumArt.org', - match=MATCH_EXACT) + match=Candidate.MATCH_EXACT) else: self._log.debug(u'no image found on page') @@ -319,7 +319,7 @@ class GoogleImages(RemoteArtSource): for item in data['items']: yield Candidate(url=item['link'], source=u'Google images', - match=MATCH_EXACT) + match=Candidate.MATCH_EXACT) class ITunesStore(RemoteArtSource): @@ -351,7 +351,7 @@ class ITunesStore(RemoteArtSource): big_url = small_url.replace('100x100', '1200x1200') yield Candidate(url=big_url, source=u'iTunes Store', - match=MATCH_EXACT) + match=Candidate.MATCH_EXACT) else: self._log.debug(u'album has no artwork in iTunes Store') except IndexError: @@ -478,7 +478,7 @@ class Wikipedia(RemoteArtSource): image_url = result['imageinfo'][0]['url'] yield Candidate(url=image_url, source=u'Wikipedia', - match=MATCH_EXACT) + match=Candidate.MATCH_EXACT) except (ValueError, KeyError, IndexError): self._log.debug(u'wikipedia: error scraping imageinfo') return @@ -525,7 +525,7 @@ class FileSystem(LocalArtSource): util.displayable_path(fn)) yield Candidate(path=os.path.join(path, fn), source=u'Filesystem', - match=MATCH_EXACT) + match=Candidate.MATCH_EXACT) # Fall back to any image in the folder. if images and not cautious: @@ -533,7 +533,7 @@ class FileSystem(LocalArtSource): util.displayable_path(images[0])) yield Candidate(path=os.path.join(path, images[0]), source=u'Filesystem', - match=MATCH_FALLBACK) + match=Candidate.MATCH_FALLBACK) # Try each source in turn. @@ -683,7 +683,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): 'maxwidth': self.maxwidth} for source in self.sources: - if source.is_local or not local_only: + if source.IS_LOCAL or not local_only: self._log.debug( u'trying source {0} for album {1.albumartist} - {1.album}', SOURCE_NAMES[type(source)], From 31aff68150313e6c3a1a2afa315c562b70cf0168 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Fri, 25 Mar 2016 16:25:00 +0100 Subject: [PATCH 06/20] fetchart: Adapt FetchImageTest, FSArtTest --- test/test_art.py | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/test/test_art.py b/test/test_art.py index ebfcd53f4..aa2c74f1c 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -53,15 +53,23 @@ class FetchImageTest(UseThePlugin): responses.add(responses.GET, 'http://example.com', content_type=content_type) + def setUp(self): + super(FetchImageTest, self).setUp() + self.dpath = os.path.join(self.temp_dir, 'arttest') + self.candidate = self.plugin.Candidate(url='http://example.com') + self.source = self.plugin.RemoteArtSource(logger, self.plugin.config) + def test_invalid_type_returns_none(self): self.mock_response('image/watercolour') - artpath = self.plugin._fetch_image('http://example.com') - self.assertEqual(artpath, None) + self.candidate.path = None + self.source.fetch_image(candidate) + self.assertEqual(self.candidate.path, None) def test_jpeg_type_returns_path(self): self.mock_response('image/jpeg') - artpath = self.plugin._fetch_image('http://example.com') - self.assertNotEqual(artpath, None) + self.candidate.path = None + self.source.fetch_image(candidate) + self.assertNotEqual(self.candidate.path, None) class FSArtTest(UseThePlugin): @@ -71,38 +79,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} def test_finds_jpg_in_directory(self): _common.touch(os.path.join(self.dpath, 'a.jpg')) - fn = self.source.get(self.dpath, ('art',), False) - self.assertEqual(fn, os.path.join(self.dpath, 'a.jpg')) + candidate = next(self.source.get(None, self.extra)) + self.assertEqual(candidate.path, os.path.join(self.dpath, 'a.jpg')) def test_appropriately_named_file_takes_precedence(self): _common.touch(os.path.join(self.dpath, 'a.jpg')) _common.touch(os.path.join(self.dpath, 'art.jpg')) - fn = self.source.get(self.dpath, ('art',), False) - self.assertEqual(fn, os.path.join(self.dpath, 'art.jpg')) + candidate = next(self.source.get(None, self.extra)) + self.assertEqual(candidate.path, os.path.join(self.dpath, 'art.jpg')) def test_non_image_file_not_identified(self): _common.touch(os.path.join(self.dpath, 'a.txt')) - fn = self.source.get(self.dpath, ('art',), False) - self.assertEqual(fn, None) + candidate = next(self.source.get(None, self.extra)) + self.assertEqual(candidate.path, None) def test_cautious_skips_fallback(self): _common.touch(os.path.join(self.dpath, 'a.jpg')) - fn = self.source.get(self.dpath, ('art',), True) - self.assertEqual(fn, None) + self.extra['cautious'] = True + candidate = next(self.source.get(None, self.extra)) + self.assertEqual(candidate.path, None) def test_empty_dir(self): - fn = self.source.get(self.dpath, ('art',), True) - self.assertEqual(fn, None) + candidate = next(self.source.get(None, self.extra)) + self.assertEqual(candidate.path, None) def test_precedence_amongst_correct_files(self): _common.touch(os.path.join(self.dpath, 'back.jpg')) _common.touch(os.path.join(self.dpath, 'front.jpg')) _common.touch(os.path.join(self.dpath, 'front-cover.jpg')) - fn = self.source.get(self.dpath, ('cover', 'front', 'back'), False) - self.assertEqual(fn, os.path.join(self.dpath, 'front-cover.jpg')) + self.extra['cover_names'] = ('cover', 'front', 'back') + candidate = next(self.source.get(None, self.extra)) + self.assertEqual(candidate.path, + os.path.join(self.dpath, 'front-cover.jpg')) class CombinedTest(UseThePlugin): From 02892a41aaabea81f1231f7ccc0e942c9e0d3526 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Tue, 5 Apr 2016 21:34:58 +0200 Subject: [PATCH 07/20] fetchart: Adapt CombinedTest --- test/test_art.py | 59 ++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/test/test_art.py b/test/test_art.py index aa2c74f1c..27ee3d6e7 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -96,27 +96,28 @@ class FSArtTest(UseThePlugin): def test_non_image_file_not_identified(self): _common.touch(os.path.join(self.dpath, 'a.txt')) - candidate = next(self.source.get(None, self.extra)) - self.assertEqual(candidate.path, None) + with self.assertRaises(StopIteration): + candidate = next(self.source.get(None, self.extra)) def test_cautious_skips_fallback(self): _common.touch(os.path.join(self.dpath, 'a.jpg')) self.extra['cautious'] = True - candidate = next(self.source.get(None, self.extra)) - self.assertEqual(candidate.path, None) + with self.assertRaises(StopIteration): + candidate = next(self.source.get(None, self.extra)) def test_empty_dir(self): - candidate = next(self.source.get(None, self.extra)) - self.assertEqual(candidate.path, None) + with self.assertRaises(StopIteration): + candidate = next(self.source.get(None, self.extra)) def test_precedence_amongst_correct_files(self): - _common.touch(os.path.join(self.dpath, 'back.jpg')) - _common.touch(os.path.join(self.dpath, 'front.jpg')) - _common.touch(os.path.join(self.dpath, 'front-cover.jpg')) + images = ('front-cover.jpg', 'front.jpg', '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') - candidate = next(self.source.get(None, self.extra)) - self.assertEqual(candidate.path, - os.path.join(self.dpath, 'front-cover.jpg')) + candidates = (candidate.path + for candidate in self.source.get(None, self.extra)) + self.assertEqual(candidates, paths) class CombinedTest(UseThePlugin): @@ -144,27 +145,27 @@ class CombinedTest(UseThePlugin): def test_main_interface_returns_amazon_art(self): self.mock_response(self.AMAZON_URL) album = _common.Bag(asin=self.ASIN) - artpath = self.plugin.art_for_album(album, None) - self.assertNotEqual(artpath, None) + candidate = self.plugin.art_for_album(album, None) + self.assertNotEqual(candidate.path, None) def test_main_interface_returns_none_for_missing_asin_and_path(self): album = _common.Bag() - artpath = self.plugin.art_for_album(album, None) - self.assertEqual(artpath, None) + candidate = self.plugin.art_for_album(album, None) + self.assertEqual(candidate.path, None) def test_main_interface_gives_precedence_to_fs_art(self): _common.touch(os.path.join(self.dpath, 'art.jpg')) self.mock_response(self.AMAZON_URL) album = _common.Bag(asin=self.ASIN) - artpath = self.plugin.art_for_album(album, [self.dpath]) - self.assertEqual(artpath, os.path.join(self.dpath, 'art.jpg')) + candidate = self.plugin.art_for_album(album, [self.dpath]) + self.assertEqual(candidate.path, os.path.join(self.dpath, 'art.jpg')) def test_main_interface_falls_back_to_amazon(self): self.mock_response(self.AMAZON_URL) album = _common.Bag(asin=self.ASIN) - artpath = self.plugin.art_for_album(album, [self.dpath]) - self.assertNotEqual(artpath, None) - self.assertFalse(artpath.startswith(self.dpath)) + candidate = self.plugin.art_for_album(album, [self.dpath]) + self.assertNotEqual(candidate.path, None) + self.assertFalse(candidate.path.startswith(self.dpath)) def test_main_interface_tries_amazon_before_aao(self): self.mock_response(self.AMAZON_URL) @@ -182,24 +183,24 @@ class CombinedTest(UseThePlugin): def test_main_interface_uses_caa_when_mbid_available(self): self.mock_response(self.CAA_URL) album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) - artpath = self.plugin.art_for_album(album, None) - self.assertNotEqual(artpath, None) + candidate = self.plugin.art_for_album(album, None) + self.assertNotEqual(candidate.path, None) self.assertEqual(len(responses.calls), 1) self.assertEqual(responses.calls[0].request.url, self.CAA_URL) def test_local_only_does_not_access_network(self): album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) - artpath = self.plugin.art_for_album(album, [self.dpath], - local_only=True) - self.assertEqual(artpath, None) + candidate = self.plugin.art_for_album(album, [self.dpath], + local_only=True) + self.assertEqual(candidate.path, None) self.assertEqual(len(responses.calls), 0) def test_local_only_gets_fs_image(self): _common.touch(os.path.join(self.dpath, 'art.jpg')) album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) - artpath = self.plugin.art_for_album(album, [self.dpath], - local_only=True) - self.assertEqual(artpath, os.path.join(self.dpath, 'art.jpg')) + candidate = self.plugin.art_for_album(album, [self.dpath], + local_only=True) + self.assertEqual(candidate.path, os.path.join(self.dpath, 'art.jpg')) self.assertEqual(len(responses.calls), 0) From 4b4a0e8bd322d2fa777f1d6c2167f469735e4239 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Tue, 5 Apr 2016 21:52:30 +0200 Subject: [PATCH 08/20] fetchart: Adapt AAOTest, GoogleImageTest --- test/test_art.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/test/test_art.py b/test/test_art.py index 27ee3d6e7..09e940b82 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -211,6 +211,7 @@ class AAOTest(UseThePlugin): def setUp(self): super(AAOTest, self).setUp() self.source = fetchart.AlbumArtOrg(logger, self.plugin.config) + self.extra = dict() @responses.activate def run(self, *args, **kwargs): @@ -230,20 +231,21 @@ class AAOTest(UseThePlugin): """ self.mock_response(self.AAO_URL, body) album = _common.Bag(asin=self.ASIN) - res = self.source.get(album) - self.assertEqual(list(res)[0], 'TARGET_URL') + candidate = next(self.source.get(album, self.extra)) + self.assertEqual(candidate.url, 'TARGET_URL') def test_aao_scraper_returns_no_result_when_no_image_present(self): self.mock_response(self.AAO_URL, b'blah blah') album = _common.Bag(asin=self.ASIN) - res = self.source.get(album) - self.assertEqual(list(res), []) + with assertRaises(StopIteration): + candidate = next(self.source.get(album, self.extra)) class GoogleImageTest(UseThePlugin): def setUp(self): super(GoogleImageTest, self).setUp() self.source = fetchart.GoogleImages(logger, self.plugin.config) + self.extra = dict() @responses.activate def run(self, *args, **kwargs): @@ -257,22 +259,22 @@ class GoogleImageTest(UseThePlugin): album = _common.Bag(albumartist="some artist", album="some album") json = b'{"items": [{"link": "url_to_the_image"}]}' self.mock_response(fetchart.GoogleImages.URL, json) - result_url = self.source.get(album) - self.assertEqual(list(result_url)[0], 'url_to_the_image') + candidate = next(self.source.get(album, self.extra)) + self.assertEqual(candidate.url, 'url_to_the_image') def test_google_art_returns_no_result_when_error_received(self): album = _common.Bag(albumartist="some artist", album="some album") json = b'{"error": {"errors": [{"reason": "some reason"}]}}' self.mock_response(fetchart.GoogleImages.URL, json) - result_url = self.source.get(album) - self.assertEqual(list(result_url), []) + with assertRaises(StopIteration): + candidate = next(self.source.get(album, self.extra)) def test_google_art_returns_no_result_with_malformed_response(self): album = _common.Bag(albumartist="some artist", album="some album") json = b"""bla blup""" self.mock_response(fetchart.GoogleImages.URL, json) - result_url = self.source.get(album) - self.assertEqual(list(result_url), []) + with assertRaises(StopIteration): + candidate = next(self.source.get(album, self.extra)) @_common.slow_test() From 206a88caff310a2c7bc7f81c769e94488847b7ef Mon Sep 17 00:00:00 2001 From: wordofglass Date: Tue, 5 Apr 2016 22:43:17 +0200 Subject: [PATCH 09/20] fetchart: Adapt ArtImporterTest --- test/test_art.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_art.py b/test/test_art.py index 09e940b82..02b8d6287 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -286,7 +286,7 @@ class ArtImporterTest(UseThePlugin): self.art_file = os.path.join(self.temp_dir, 'tmpcover.jpg') _common.touch(self.art_file) self.old_afa = self.plugin.art_for_album - self.afa_response = self.art_file + self.afa_response = fetchart.Candidate(path=self.art_file) def art_for_album(i, p, local_only=False): return self.afa_response @@ -377,7 +377,7 @@ class ArtImporterTest(UseThePlugin): def test_do_not_delete_original_if_already_in_place(self): artdest = os.path.join(os.path.dirname(self.i.path), 'cover.jpg') shutil.copyfile(self.art_file, artdest) - self.afa_response = artdest + self.afa_response = fetchart.Candidate(path=artdest) self._fetch_art(True) def test_fetch_art_if_imported_file_deleted(self): From a2141c6f3abd0a77dba9e2b25e7b58248cde1070 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Tue, 5 Apr 2016 22:44:13 +0200 Subject: [PATCH 10/20] fetchart: fix refactoring --- beetsplug/fetchart.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 08e8988ff..6f84b37c0 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -626,10 +626,10 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): # For any other choices (e.g., TRACKS), do nothing. return - path = self.art_for_album(task.album, task.paths, local) + candidate = self.art_for_album(task.album, task.paths, local) - if path: - self.art_paths[task] = path + if candidate: + self.art_paths[task] = candidate.path # Synchronous; after music files are put in place. def assign_art(self, session, task): @@ -719,9 +719,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): # sources. local_paths = None if force else [album.path] - path = self.art_for_album(album, local_paths) - if path: - album.set_art(path, False) + candidate = self.art_for_album(album, local_paths) + if candidate: + album.set_art(candidate.path, False) album.store() message = ui.colorize('text_success', u'found album art') else: From bbc06e9de9eec6de462e349256c893c6800ac406 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Tue, 5 Apr 2016 23:24:30 +0200 Subject: [PATCH 11/20] fetchart: Adapt ArtForAlbumTest --- beetsplug/fetchart.py | 2 +- test/test_art.py | 36 +++++++++++------------------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 6f84b37c0..b82b64460 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -179,7 +179,7 @@ class LocalArtSource(ArtSource): LOC_STR = u'local' def fetch_image(self, candidate, extra): - return candidate + pass class RemoteArtSource(ArtSource): IS_LOCAL = False diff --git a/test/test_art.py b/test/test_art.py index 02b8d6287..c71a9a820 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -405,44 +405,30 @@ class ArtForAlbumTest(UseThePlugin): def setUp(self): super(ArtForAlbumTest, self).setUp() - self.old_fs_source_get = self.plugin.fs_source.get - self.old_fetch_img = self.plugin._fetch_image - self.old_source_urls = self.plugin._source_urls + self.old_fs_source_get = fetchart.FileSystem.get - def fs_source_get(*_): - return self.image_file + def fs_source_get(album, paths, *_): + if paths: + yield fetchart.Candidate(path=self.image_file) - def source_urls(_): - return [''] - - def fetch_img(_): - return self.image_file - - self.plugin.fs_source.get = fs_source_get - self.plugin._source_urls = source_urls - self.plugin._fetch_image = fetch_img + fetchart.FileSystem.get = fs_source_get def tearDown(self): - self.plugin.fs_source.get = self.old_fs_source_get - self.plugin._source_urls = self.old_source_urls - self.plugin._fetch_image = self.old_fetch_img + fetchart.FileSystem.get = self.old_fs_source_get super(ArtForAlbumTest, self).tearDown() def _assertImageIsValidArt(self, image_file, should_exist): self.assertExists(image_file) self.image_file = image_file - local_artpath = self.plugin.art_for_album(None, [''], True) - remote_artpath = self.plugin.art_for_album(None, [], False) - - self.assertEqual(local_artpath, remote_artpath) + artpath = self.plugin.art_for_album(None, [''], True).path if should_exist: - self.assertEqual(local_artpath, self.image_file) - self.assertExists(local_artpath) - return local_artpath + self.assertEqual(artpath, self.image_file) + self.assertExists(artpath) + return artpath else: - self.assertIsNone(local_artpath) + self.assertIsNone(artpath) def _assertImageResized(self, image_file, should_resize): self.image_file = image_file From bc877a6d7f286de5b4aeea5026ae1ac53e1013bc Mon Sep 17 00:00:00 2001 From: wordofglass Date: Wed, 6 Apr 2016 01:19:44 +0200 Subject: [PATCH 12/20] fetchart: fix tests (mostly pep8, syntax) --- beetsplug/fetchart.py | 44 +++++++++++++++++++----------------- test/test_art.py | 52 ++++++++++++++++++++++--------------------- 2 files changed, 50 insertions(+), 46 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index b82b64460..93a703a73 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -41,9 +41,10 @@ IMAGE_EXTENSIONS = ['png', 'jpg', 'jpeg'] CONTENT_TYPES = ('image/jpeg', 'image/png') DOWNLOAD_EXTENSION = '.jpg' -class Candidate(): - """ +class Candidate(): + """Holds information about a matching artwork, deals with validation of + dimension restrictions and resizing. """ CANDIDATE_BAD = 0 CANDIDATE_EXACT = 1 @@ -54,6 +55,7 @@ class Candidate(): def __init__(self, path=None, url=None, source=u'', match=None): self.path = path + self.url = url self.source = source self.check = None self.match = match @@ -70,8 +72,8 @@ class Candidate(): if not self.path: return self.CANDIDATE_BAD - if not (extra['enforce_ratio'] or - extra['minwidth'] or + if not (extra['enforce_ratio'] or + extra['minwidth'] or extra['maxwidth']): return self.CANDIDATE_EXACT @@ -174,12 +176,14 @@ class ArtSource(RequestMixin): def fetch_image(self, candidate, extra): raise NotImplementedError() + class LocalArtSource(ArtSource): IS_LOCAL = True LOC_STR = u'local' def fetch_image(self, candidate, extra): - pass + pass + class RemoteArtSource(ArtSource): IS_LOCAL = False @@ -191,7 +195,7 @@ class RemoteArtSource(ArtSource): Otherwise, returns None. """ if extra['maxwidth']: - candidate.url = ArtResizer.shared.proxy_url(extra['maxwidth'], + candidate.url = ArtResizer.shared.proxy_url(extra['maxwidth'], candidate.url) try: with closing(self.request(candidate.url, stream=True, @@ -203,7 +207,7 @@ class RemoteArtSource(ArtSource): resp.headers.get('Content-Type') or u'no content type', ) candidate.path = None - return + return # Generate a temporary file with the correct extension. with NamedTemporaryFile(suffix=DOWNLOAD_EXTENSION, @@ -220,7 +224,7 @@ class RemoteArtSource(ArtSource): # https://github.com/shazow/urllib3/issues/556 self._log.debug(u'error fetching art: {}', exc) candidate.path = None - return + return class CoverArtArchive(RemoteArtSource): @@ -237,9 +241,10 @@ class CoverArtArchive(RemoteArtSource): source=u'coverartarchive.org', match=Candidate.MATCH_EXACT) if album.mb_releasegroupid: - yield Candidate(url=self.GROUP_URL.format(mbid=album.mb_releasegroupid), - source=u'coverartarchive.org', - match=Candidate.MATCH_FALLBACK) + yield Candidate( + url=self.GROUP_URL.format(mbid=album.mb_releasegroupid), + source=u'coverartarchive.org', + match=Candidate.MATCH_FALLBACK) class Amazon(RemoteArtSource): @@ -503,7 +508,7 @@ class FileSystem(LocalArtSource): cover_names = extra['cover_names'] cover_pat = br"(\b|_)({0})(\b|_)".format(b'|'.join(cover_names)) cautious = extra['cautious'] - + for path in paths: if not os.path.isdir(path): continue @@ -517,7 +522,7 @@ class FileSystem(LocalArtSource): images.append(fn) # Look for "preferred" filenames. - images = sorted(images, + images = sorted(images, lambda x: self.filename_priority(x, cover_names)) for fn in images: if re.search(cover_pat, os.path.splitext(fn)[0], re.I): @@ -538,7 +543,7 @@ class FileSystem(LocalArtSource): # Try each source in turn. -SOURCES_ALL = [u'filesysytem', +SOURCES_ALL = [u'filesysytem', u'coverart', u'itunes', u'amazon', u'albumart', u'wikipedia', u'google'] @@ -565,10 +570,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): 'minwidth': 0, 'maxwidth': 0, 'enforce_ratio': False, - 'remote_priority': False, 'cautious': False, 'cover_names': ['cover', 'front', 'art', 'album', 'folder'], - 'sources': ['filesystem', + 'sources': ['filesystem', 'coverart', 'itunes', 'amazon', 'albumart'], 'google_key': None, 'google_engine': u'001442825323518660753:hrh5ch1gjzm', @@ -669,14 +673,13 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): are made. """ out = None - check = None # all the information any of the sources might need cover_names = self.config['cover_names'].as_str_seq() cover_names = map(util.bytestring_path, cover_names) cautious = self.config['cautious'].get(bool) - extra = {'paths': paths, - 'cover_names': cover_names, + extra = {'paths': paths, + 'cover_names': cover_names, 'cautious': cautious, 'enforce_ratio': self.enforce_ratio, 'minwidth': self.minwidth, @@ -692,7 +695,7 @@ 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) + source.fetch_image(candidate, extra) if candidate.validate(extra): out = candidate self._log.debug(u'using {0.LOC_STR()} image {1}' @@ -728,4 +731,3 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): message = ui.colorize('text_error', u'no art found') self._log.info(u'{0}: {1}', album, message) - diff --git a/test/test_art.py b/test/test_art.py index c71a9a820..671dfaeaa 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -45,31 +45,32 @@ class UseThePlugin(_common.TestCase): class FetchImageTest(UseThePlugin): + URL = 'http://example.com' + @responses.activate def run(self, *args, **kwargs): super(FetchImageTest, self).run(*args, **kwargs) def mock_response(self, content_type): - responses.add(responses.GET, 'http://example.com', + responses.add(responses.GET, self.URL, content_type=content_type) def setUp(self): super(FetchImageTest, self).setUp() self.dpath = os.path.join(self.temp_dir, 'arttest') - self.candidate = self.plugin.Candidate(url='http://example.com') self.source = self.plugin.RemoteArtSource(logger, self.plugin.config) def test_invalid_type_returns_none(self): self.mock_response('image/watercolour') - self.candidate.path = None + candidate = fetchart.Candidate(url=self.URL) self.source.fetch_image(candidate) - self.assertEqual(self.candidate.path, None) + self.assertEqual(candidate.path, None) def test_jpeg_type_returns_path(self): self.mock_response('image/jpeg') - self.candidate.path = None + candidate = fetchart.Candidate(url=self.URL) self.source.fetch_image(candidate) - self.assertNotEqual(self.candidate.path, None) + self.assertNotEqual(candidate.path, None) class FSArtTest(UseThePlugin): @@ -97,26 +98,26 @@ class FSArtTest(UseThePlugin): def test_non_image_file_not_identified(self): _common.touch(os.path.join(self.dpath, 'a.txt')) with self.assertRaises(StopIteration): - candidate = next(self.source.get(None, self.extra)) + next(self.source.get(None, self.extra)) def test_cautious_skips_fallback(self): _common.touch(os.path.join(self.dpath, 'a.jpg')) self.extra['cautious'] = True with self.assertRaises(StopIteration): - candidate = next(self.source.get(None, self.extra)) + next(self.source.get(None, self.extra)) def test_empty_dir(self): with self.assertRaises(StopIteration): - candidate = next(self.source.get(None, self.extra)) + next(self.source.get(None, self.extra)) def test_precedence_amongst_correct_files(self): - images = ('front-cover.jpg', 'front.jpg', 'back.jpg') - paths = (os.path.join(self.dpath, i) for i in images) + images = ['front-cover.jpg', 'front.jpg', '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') - candidates = (candidate.path - for candidate in self.source.get(None, self.extra)) + self.extra['cover_names'] = ['cover', 'front', 'back'] + candidates = [candidate.path for candidate in + self.source.get(None, self.extra)] self.assertEqual(candidates, paths) @@ -237,8 +238,8 @@ class AAOTest(UseThePlugin): def test_aao_scraper_returns_no_result_when_no_image_present(self): self.mock_response(self.AAO_URL, b'blah blah') album = _common.Bag(asin=self.ASIN) - with assertRaises(StopIteration): - candidate = next(self.source.get(album, self.extra)) + with self.assertRaises(StopIteration): + next(self.source.get(album, self.extra)) class GoogleImageTest(UseThePlugin): @@ -266,15 +267,15 @@ class GoogleImageTest(UseThePlugin): album = _common.Bag(albumartist="some artist", album="some album") json = b'{"error": {"errors": [{"reason": "some reason"}]}}' self.mock_response(fetchart.GoogleImages.URL, json) - with assertRaises(StopIteration): - candidate = next(self.source.get(album, self.extra)) + with self.assertRaises(StopIteration): + next(self.source.get(album, self.extra)) def test_google_art_returns_no_result_with_malformed_response(self): album = _common.Bag(albumartist="some artist", album="some album") json = b"""bla blup""" self.mock_response(fetchart.GoogleImages.URL, json) - with assertRaises(StopIteration): - candidate = next(self.source.get(album, self.extra)) + with self.assertRaises(StopIteration): + next(self.source.get(album, self.extra)) @_common.slow_test() @@ -421,14 +422,15 @@ class ArtForAlbumTest(UseThePlugin): self.assertExists(image_file) self.image_file = image_file - artpath = self.plugin.art_for_album(None, [''], True).path + candidate = self.plugin.art_for_album(None, [''], True) if should_exist: - self.assertEqual(artpath, self.image_file) - self.assertExists(artpath) - return artpath + self.assertNotEqual(candidate, None) + self.assertEqual(candidate.path, self.image_file) + self.assertExists(candidate.path) + return candidate.path else: - self.assertIsNone(artpath) + self.assertIsNone(candidate) def _assertImageResized(self, image_file, should_resize): self.image_file = image_file From a4994d2bf806aa1efb3ed3141e0d89449061f755 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Wed, 6 Apr 2016 01:36:32 +0200 Subject: [PATCH 13/20] fetchart: fix more tests --- beetsplug/fetchart.py | 2 +- test/test_art.py | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 93a703a73..c70db8f6b 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -698,7 +698,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): source.fetch_image(candidate, extra) if candidate.validate(extra): out = candidate - self._log.debug(u'using {0.LOC_STR()} image {1}' + self._log.debug(u'using {0.LOC_STR} image {1}' .format(source, out.path)) break if out: diff --git a/test/test_art.py b/test/test_art.py index 671dfaeaa..5971fe880 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -58,7 +58,7 @@ class FetchImageTest(UseThePlugin): def setUp(self): super(FetchImageTest, self).setUp() self.dpath = os.path.join(self.temp_dir, 'arttest') - self.source = self.plugin.RemoteArtSource(logger, self.plugin.config) + self.source = fetchart.RemoteArtSource(logger, self.plugin.config) def test_invalid_type_returns_none(self): self.mock_response('image/watercolour') @@ -147,25 +147,26 @@ class CombinedTest(UseThePlugin): self.mock_response(self.AMAZON_URL) album = _common.Bag(asin=self.ASIN) candidate = self.plugin.art_for_album(album, None) - self.assertNotEqual(candidate.path, None) + self.assertIsNotNone(candidate) def test_main_interface_returns_none_for_missing_asin_and_path(self): album = _common.Bag() candidate = self.plugin.art_for_album(album, None) - self.assertEqual(candidate.path, None) + self.assertIsNotNone(candidate) def test_main_interface_gives_precedence_to_fs_art(self): _common.touch(os.path.join(self.dpath, 'art.jpg')) self.mock_response(self.AMAZON_URL) album = _common.Bag(asin=self.ASIN) candidate = self.plugin.art_for_album(album, [self.dpath]) + self.assertIsNotNone(candidate) self.assertEqual(candidate.path, os.path.join(self.dpath, 'art.jpg')) def test_main_interface_falls_back_to_amazon(self): self.mock_response(self.AMAZON_URL) album = _common.Bag(asin=self.ASIN) candidate = self.plugin.art_for_album(album, [self.dpath]) - self.assertNotEqual(candidate.path, None) + self.assertIsNotNone(candidate) self.assertFalse(candidate.path.startswith(self.dpath)) def test_main_interface_tries_amazon_before_aao(self): @@ -185,15 +186,14 @@ class CombinedTest(UseThePlugin): self.mock_response(self.CAA_URL) album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) candidate = self.plugin.art_for_album(album, None) - self.assertNotEqual(candidate.path, None) + self.assertIsNotNone(candidate) self.assertEqual(len(responses.calls), 1) self.assertEqual(responses.calls[0].request.url, self.CAA_URL) def test_local_only_does_not_access_network(self): album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) - candidate = self.plugin.art_for_album(album, [self.dpath], - local_only=True) - self.assertEqual(candidate.path, None) + with self.assertRaises(StopIteration): + self.plugin.art_for_album(album, local_only=True) self.assertEqual(len(responses.calls), 0) def test_local_only_gets_fs_image(self): @@ -201,6 +201,7 @@ class CombinedTest(UseThePlugin): album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) candidate = self.plugin.art_for_album(album, [self.dpath], local_only=True) + self.assertIsNotNone(candidate) self.assertEqual(candidate.path, os.path.join(self.dpath, 'art.jpg')) self.assertEqual(len(responses.calls), 0) From 1cc4d11baf94accbd2391a27130e50852dcec763 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Wed, 13 Apr 2016 22:56:18 +0200 Subject: [PATCH 14/20] Fetchart: fix tests and fetachart logic --- beetsplug/fetchart.py | 81 +++++++++++++++++++++++-------------------- test/test_art.py | 27 +++++++-------- 2 files changed, 56 insertions(+), 52 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index c70db8f6b..1eb139602 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -53,7 +53,8 @@ class Candidate(): MATCH_EXACT = 0 MATCH_FALLBACK = 1 - def __init__(self, path=None, url=None, source=u'', match=None): + def __init__(self, log, path=None, url=None, source=u'', match=None): + self._log = log self.path = path self.url = url self.source = source @@ -173,6 +174,9 @@ class ArtSource(RequestMixin): def get(self, album, extra): raise NotImplementedError() + def _candidate(self, **kwargs): + return Candidate(source=self.NAME, log=self._log, **kwargs) + def fetch_image(self, candidate, extra): raise NotImplementedError() @@ -228,7 +232,8 @@ class RemoteArtSource(ArtSource): class CoverArtArchive(RemoteArtSource): - """Cover Art Archive""" + NAME = u"Cover Art Archive" + URL = 'http://coverartarchive.org/release/{mbid}/front' GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}/front' @@ -237,17 +242,16 @@ class CoverArtArchive(RemoteArtSource): using album MusicBrainz release ID and release group ID. """ if album.mb_albumid: - yield Candidate(url=self.URL.format(mbid=album.mb_albumid), - source=u'coverartarchive.org', - match=Candidate.MATCH_EXACT) + yield self._candidate(url=self.URL.format(mbid=album.mb_albumid), + match=Candidate.MATCH_EXACT) if album.mb_releasegroupid: - yield Candidate( + yield self._candidate( url=self.GROUP_URL.format(mbid=album.mb_releasegroupid), - source=u'coverartarchive.org', match=Candidate.MATCH_FALLBACK) class Amazon(RemoteArtSource): + NAME = u"Amazon" URL = 'http://images.amazon.com/images/P/%s.%02i.LZZZZZZZ.jpg' INDICES = (1, 2) @@ -256,13 +260,12 @@ class Amazon(RemoteArtSource): """ if album.asin: for index in self.INDICES: - yield Candidate(url=self.URL % (album.asin, index), - source=u'Amazon', - match=Candidate.MATCH_EXACT) + yield self._candidate(url=self.URL % (album.asin, index), + match=Candidate.MATCH_EXACT) class AlbumArtOrg(RemoteArtSource): - """AlbumArt.org scraper""" + NAME = u"AlbumArt.org scraper" URL = 'http://www.albumart.org/index_detail.php' PAT = r'href\s*=\s*"([^>"]*)"[^>]*title\s*=\s*"View larger image"' @@ -283,14 +286,13 @@ class AlbumArtOrg(RemoteArtSource): m = re.search(self.PAT, resp.text) if m: image_url = m.group(1) - yield Candidate(url=image_url, - source=u'AlbumArt.org', - match=Candidate.MATCH_EXACT) + yield self._candidate(url=image_url, match=Candidate.MATCH_EXACT) else: self._log.debug(u'no image found on page') class GoogleImages(RemoteArtSource): + NAME = u"Google Images" URL = u'https://www.googleapis.com/customsearch/v1' def get(self, album, extra): @@ -322,13 +324,13 @@ class GoogleImages(RemoteArtSource): if 'items' in data.keys(): for item in data['items']: - yield Candidate(url=item['link'], - source=u'Google images', - match=Candidate.MATCH_EXACT) + yield self._candidate(url=item['link'], + match=Candidate.MATCH_EXACT) class ITunesStore(RemoteArtSource): - # Art from the iTunes Store. + NAME = u"iTunes Store" + def get(self, album, extra): """Return art URL from iTunes Store given an album title. """ @@ -354,9 +356,7 @@ class ITunesStore(RemoteArtSource): if itunes_album.get_artwork()['100']: small_url = itunes_album.get_artwork()['100'] big_url = small_url.replace('100x100', '1200x1200') - yield Candidate(url=big_url, - source=u'iTunes Store', - match=Candidate.MATCH_EXACT) + yield self._candidate(url=big_url, match=Candidate.MATCH_EXACT) else: self._log.debug(u'album has no artwork in iTunes Store') except IndexError: @@ -364,7 +364,7 @@ class ITunesStore(RemoteArtSource): class Wikipedia(RemoteArtSource): - # Art from Wikipedia (queried through DBpedia) + NAME = u"Wikipedia (queried through DBpedia)" DBPEDIA_URL = 'http://dbpedia.org/sparql' WIKIPEDIA_URL = 'http://en.wikipedia.org/w/api.php' SPARQL_QUERY = '''PREFIX rdf: @@ -481,16 +481,16 @@ class Wikipedia(RemoteArtSource): results = data['query']['pages'] for _, result in results.iteritems(): image_url = result['imageinfo'][0]['url'] - yield Candidate(url=image_url, - source=u'Wikipedia', - match=Candidate.MATCH_EXACT) + yield self._candidate(url=image_url, + match=Candidate.MATCH_EXACT) except (ValueError, KeyError, IndexError): self._log.debug(u'wikipedia: error scraping imageinfo') return class FileSystem(LocalArtSource): - """Art from the filesystem""" + NAME = u"Filesystem" + @staticmethod def filename_priority(filename, cover_names): """Sort order for image names. @@ -505,6 +505,8 @@ class FileSystem(LocalArtSource): """Look for album art files in the specified directories. """ paths = extra['paths'] + if not paths: + return cover_names = extra['cover_names'] cover_pat = br"(\b|_)({0})(\b|_)".format(b'|'.join(cover_names)) cautious = extra['cautious'] @@ -523,27 +525,29 @@ class FileSystem(LocalArtSource): # Look for "preferred" filenames. images = sorted(images, - lambda x: self.filename_priority(x, cover_names)) + key=lambda x: + self.filename_priority(x, cover_names)) + remaining = [] for fn in images: if re.search(cover_pat, os.path.splitext(fn)[0], re.I): self._log.debug(u'using well-named art file {0}', util.displayable_path(fn)) - yield Candidate(path=os.path.join(path, fn), - source=u'Filesystem', - match=Candidate.MATCH_EXACT) + yield self._candidate(path=os.path.join(path, fn), + match=Candidate.MATCH_EXACT) + else: + remaining.append(fn) # Fall back to any image in the folder. - if images and not cautious: + if remaining and not cautious: self._log.debug(u'using fallback art file {0}', - util.displayable_path(images[0])) - yield Candidate(path=os.path.join(path, images[0]), - source=u'Filesystem', - match=Candidate.MATCH_FALLBACK) + util.displayable_path(remaining[0])) + yield self._candidate(path=os.path.join(path, remaining[0]), + match=Candidate.MATCH_FALLBACK) # Try each source in turn. -SOURCES_ALL = [u'filesysytem', +SOURCES_ALL = [u'filesystem', u'coverart', u'itunes', u'amazon', u'albumart', u'wikipedia', u'google'] @@ -698,8 +702,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): source.fetch_image(candidate, extra) if candidate.validate(extra): out = candidate - self._log.debug(u'using {0.LOC_STR} image {1}' - .format(source, out.path)) + self._log.debug( + u'using {0.LOC_STR} image {1}'.format( + source, util.displayable_path(out.path))) break if out: break diff --git a/test/test_art.py b/test/test_art.py index 5971fe880..2edea0c69 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -59,17 +59,18 @@ class FetchImageTest(UseThePlugin): super(FetchImageTest, self).setUp() self.dpath = os.path.join(self.temp_dir, 'arttest') self.source = fetchart.RemoteArtSource(logger, self.plugin.config) + self.extra = {'maxwidth': 0} def test_invalid_type_returns_none(self): self.mock_response('image/watercolour') - candidate = fetchart.Candidate(url=self.URL) - self.source.fetch_image(candidate) + candidate = fetchart.Candidate(logger, url=self.URL) + self.source.fetch_image(candidate, self.extra) self.assertEqual(candidate.path, None) def test_jpeg_type_returns_path(self): self.mock_response('image/jpeg') - candidate = fetchart.Candidate(url=self.URL) - self.source.fetch_image(candidate) + candidate = fetchart.Candidate(logger, url=self.URL) + self.source.fetch_image(candidate, self.extra) self.assertNotEqual(candidate.path, None) @@ -82,7 +83,7 @@ class FSArtTest(UseThePlugin): self.source = fetchart.FileSystem(logger, self.plugin.config) self.extra = {'cautious': False, 'cover_names': ('art',), - 'paths': self.dpath} + 'paths': [self.dpath]} def test_finds_jpg_in_directory(self): _common.touch(os.path.join(self.dpath, 'a.jpg')) @@ -152,7 +153,7 @@ class CombinedTest(UseThePlugin): def test_main_interface_returns_none_for_missing_asin_and_path(self): album = _common.Bag() candidate = self.plugin.art_for_album(album, None) - self.assertIsNotNone(candidate) + self.assertIsNone(candidate) def test_main_interface_gives_precedence_to_fs_art(self): _common.touch(os.path.join(self.dpath, 'art.jpg')) @@ -192,8 +193,7 @@ class CombinedTest(UseThePlugin): def test_local_only_does_not_access_network(self): album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) - with self.assertRaises(StopIteration): - self.plugin.art_for_album(album, local_only=True) + self.plugin.art_for_album(album, None, local_only=True) self.assertEqual(len(responses.calls), 0) def test_local_only_gets_fs_image(self): @@ -288,7 +288,7 @@ class ArtImporterTest(UseThePlugin): self.art_file = os.path.join(self.temp_dir, 'tmpcover.jpg') _common.touch(self.art_file) self.old_afa = self.plugin.art_for_album - self.afa_response = fetchart.Candidate(path=self.art_file) + self.afa_response = fetchart.Candidate(logger, path=self.art_file) def art_for_album(i, p, local_only=False): return self.afa_response @@ -379,7 +379,7 @@ class ArtImporterTest(UseThePlugin): def test_do_not_delete_original_if_already_in_place(self): artdest = os.path.join(os.path.dirname(self.i.path), 'cover.jpg') shutil.copyfile(self.art_file, artdest) - self.afa_response = fetchart.Candidate(path=artdest) + self.afa_response = fetchart.Candidate(logger, path=artdest) self._fetch_art(True) def test_fetch_art_if_imported_file_deleted(self): @@ -409,9 +409,9 @@ class ArtForAlbumTest(UseThePlugin): self.old_fs_source_get = fetchart.FileSystem.get - def fs_source_get(album, paths, *_): - if paths: - yield fetchart.Candidate(path=self.image_file) + def fs_source_get(_self, album, extra): + if extra['paths']: + yield fetchart.Candidate(logger, path=self.image_file) fetchart.FileSystem.get = fs_source_get @@ -429,7 +429,6 @@ class ArtForAlbumTest(UseThePlugin): self.assertNotEqual(candidate, None) self.assertEqual(candidate.path, self.image_file) self.assertExists(candidate.path) - return candidate.path else: self.assertIsNone(candidate) From de3e91db87a1bf5a1bfef0c39c392ad8c89b333b Mon Sep 17 00:00:00 2001 From: wordofglass Date: Thu, 14 Apr 2016 15:25:10 +0200 Subject: [PATCH 15/20] fetchart: fix tests providing album=None --- test/test_art.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/test_art.py b/test/test_art.py index 2edea0c69..b81c5ea6d 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -415,6 +415,8 @@ class ArtForAlbumTest(UseThePlugin): fetchart.FileSystem.get = fs_source_get + self.album = _common.Bag() + def tearDown(self): fetchart.FileSystem.get = self.old_fs_source_get super(ArtForAlbumTest, self).tearDown() @@ -423,7 +425,7 @@ class ArtForAlbumTest(UseThePlugin): self.assertExists(image_file) self.image_file = image_file - candidate = self.plugin.art_for_album(None, [''], True) + candidate = self.plugin.art_for_album(self.album, [''], True) if should_exist: self.assertNotEqual(candidate, None) @@ -435,7 +437,7 @@ class ArtForAlbumTest(UseThePlugin): def _assertImageResized(self, image_file, should_resize): self.image_file = image_file with patch.object(ArtResizer.shared, 'resize') as mock_resize: - self.plugin.art_for_album(None, [''], True) + self.plugin.art_for_album(self.album, [''], True) self.assertEqual(mock_resize.called, should_resize) def _require_backend(self): From 6bc3fb8f679234c85b28e47f0471328208f38784 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Thu, 14 Apr 2016 15:27:47 +0200 Subject: [PATCH 16/20] fetchart: make Candidate a new-style class --- beetsplug/fetchart.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 1eb139602..0b020c8f1 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -42,7 +42,7 @@ CONTENT_TYPES = ('image/jpeg', 'image/png') DOWNLOAD_EXTENSION = '.jpg' -class Candidate(): +class Candidate(object): """Holds information about a matching artwork, deals with validation of dimension restrictions and resizing. """ From aaf614c869342cbf5db5c76bedc8b4b48e26b67b Mon Sep 17 00:00:00 2001 From: wordofglass Date: Thu, 14 Apr 2016 16:00:24 +0200 Subject: [PATCH 17/20] fetchart: move config reads to __init__ --- beetsplug/fetchart.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 0b020c8f1..89ce8fe39 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -295,6 +295,11 @@ class GoogleImages(RemoteArtSource): NAME = u"Google Images" URL = u'https://www.googleapis.com/customsearch/v1' + def __init__(self, *args, **kwargs): + super(RemoteArtSource, self).__init__(*args, **kwargs) + self.key: self._config['google_key'].get(), + self.cx: self._config['google_engine'].get(), + def get(self, album, extra): """Return art URL from google custom search engine given an album title and interpreter. @@ -303,8 +308,8 @@ class GoogleImages(RemoteArtSource): return search_string = (album.albumartist + ',' + album.album).encode('utf-8') response = self.request(self.URL, params={ - 'key': self._config['google_key'].get(), - 'cx': self._config['google_engine'].get(), + 'key': self.key, + 'cx': self.cx, 'q': search_string, 'searchType': 'image' }) @@ -591,6 +596,13 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): self.maxwidth = self.config['maxwidth'].get(int) self.enforce_ratio = self.config['enforce_ratio'].get(bool) + cover_names = self.config['cover_names'].as_str_seq() + self.cover_names = map(util.bytestring_path, cover_names) + self.cautious = self.config['cautious'].get(bool) + + self.src_removed = (config['import']['delete'].get(bool) or + config['import']['move'].get(bool)) + if self.config['auto']: # Enable two import hooks when fetching is enabled. self.import_stages = [self.fetch_art] @@ -646,11 +658,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): path = self.art_paths.pop(task) album = task.album - src_removed = (config['import']['delete'].get(bool) or - config['import']['move'].get(bool)) - album.set_art(path, not src_removed) + album.set_art(path, not self.src_removed) album.store() - if src_removed: + if self.src_removed: task.prune(path) # Manual album art fetching. @@ -679,12 +689,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): out = None # all the information any of the sources might need - cover_names = self.config['cover_names'].as_str_seq() - cover_names = map(util.bytestring_path, cover_names) - cautious = self.config['cautious'].get(bool) extra = {'paths': paths, - 'cover_names': cover_names, - 'cautious': cautious, + 'cover_names': self.cover_names, + 'cautious': self.cautious, 'enforce_ratio': self.enforce_ratio, 'minwidth': self.minwidth, 'maxwidth': self.maxwidth} From 7322e212a404d5b90a620f7c49a5ed9e7625d14f Mon Sep 17 00:00:00 2001 From: wordofglass Date: Thu, 14 Apr 2016 16:49:06 +0200 Subject: [PATCH 18/20] fetchart: adapt test to earlier config read --- beetsplug/fetchart.py | 4 ++-- test/test_art.py | 7 +------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 89ce8fe39..60d883b73 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -297,8 +297,8 @@ class GoogleImages(RemoteArtSource): def __init__(self, *args, **kwargs): super(RemoteArtSource, self).__init__(*args, **kwargs) - self.key: self._config['google_key'].get(), - self.cx: self._config['google_engine'].get(), + self.key = self._config['google_key'].get(), + self.cx = self._config['google_engine'].get(), def get(self, album, extra): """Return art URL from google custom search engine diff --git a/test/test_art.py b/test/test_art.py index b81c5ea6d..9cf8bd18e 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -367,12 +367,7 @@ class ArtImporterTest(UseThePlugin): self.assertExists(self.art_file) def test_delete_original_file(self): - config['import']['delete'] = True - self._fetch_art(True) - self.assertNotExists(self.art_file) - - def test_move_original_file(self): - config['import']['move'] = True + self.plugin.src_removed = True self._fetch_art(True) self.assertNotExists(self.art_file) From 98d5aa4a1341b88bb139ca78e7b53e6cdf4a3173 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Thu, 14 Apr 2016 16:56:45 +0200 Subject: [PATCH 19/20] fetchart: remove unused import --- test/test_art.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_art.py b/test/test_art.py index 9cf8bd18e..a25c82486 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -29,7 +29,6 @@ from beetsplug import fetchart from beets.autotag import AlbumInfo, AlbumMatch from beets import library from beets import importer -from beets import config from beets import logging from beets import util from beets.util.artresizer import ArtResizer, WEBPROXY From 36e91fc78b33c05acb7b40d44d486880b6ad9fdb Mon Sep 17 00:00:00 2001 From: wordofglass Date: Sat, 16 Apr 2016 14:17:46 +0200 Subject: [PATCH 20/20] fix fanart.tv merge --- beetsplug/fetchart.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 830dab924..88c72cdbd 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -335,7 +335,7 @@ class GoogleImages(RemoteArtSource): match=Candidate.MATCH_EXACT) -class FanartTV(ArtSource): +class FanartTV(RemoteArtSource): """Art from fanart.tv requested using their API""" NAME = u"fanart.tv"