From f7834bfea78495063abc42ae61edc887adfcb6d6 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 12:57:19 +0200
Subject: [PATCH 01/70] _fill_try_list(): do not enter loops if not needed

---
 picard/coverart.py | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index ecc098279..c9d989246 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -223,17 +223,22 @@ def coverart(album, metadata, release, try_list=None):
 
 def _fill_try_list(album, release, try_list):
     """Fills ``try_list`` by looking at the relationships in ``release``."""
+    use_whitelist = config.setting['ca_provider_use_whitelist']
+    use_amazon = config.setting['ca_provider_use_amazon']
+    if not (use_whitelist or use_amazon):
+        return
     try:
         if 'relation_list' in release.children:
             for relation_list in release.relation_list:
                 if relation_list.target_type == 'url':
                     for relation in relation_list.relation:
                         # Use the URL of a cover art link directly
-                        if config.setting['ca_provider_use_whitelist']\
-                            and (relation.type == 'cover art link' or
-                                 relation.type == 'has_cover_art_at'):
-                            _try_list_append_image_url(try_list, QUrl(relation.target[0].text))
-                        elif config.setting['ca_provider_use_amazon']\
+                        if use_whitelist \
+                           and (relation.type == 'cover art link' or
+                                relation.type == 'has_cover_art_at'):
+                            url = QUrl(relation.target[0].text)
+                            _try_list_append_image_url(try_list, url)
+                        elif use_amazon \
                             and (relation.type == 'amazon asin' or
                                  relation.type == 'has_Amazon_ASIN'):
                             _process_asin_relation(try_list, relation)

From 55c80f588c52448ae10fa58e64f668da7be2448b Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:12:44 +0200
Subject: [PATCH 02/70] Reduce code redundancy using intermediate variables
 has_front and has_back

---
 picard/coverart.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index c9d989246..00606f8b5 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -184,8 +184,10 @@ def coverart(album, metadata, release, try_list=None):
         if 'cover_art_archive' in release.children:
             caa_node = release.children['cover_art_archive'][0]
             has_caa_artwork = (caa_node.artwork[0].text == 'true')
+            has_front = 'front' in caa_types
+            has_back = 'back' in caa_types
 
-            if len(caa_types) == 2 and ('front' in caa_types or 'back' in caa_types):
+            if len(caa_types) == 2 and (has_front or has_back):
                 # The OR cases are there to still download and process the CAA
                 # JSON file if front or back is enabled but not in the CAA and
                 # another type (that's neither front nor back) is enabled.
@@ -195,13 +197,13 @@ def coverart(album, metadata, release, try_list=None):
                 # as well) but it's still necessary to download the booklet
                 # images by using the fact that back is enabled but there are
                 # no back images in the CAA.
-                front_in_caa = caa_node.front[0].text == 'true' or 'front' not in caa_types
-                back_in_caa = caa_node.back[0].text == 'true' or 'back' not in caa_types
+                front_in_caa = caa_node.front[0].text == 'true' or not has_front
+                back_in_caa = caa_node.back[0].text == 'true' or not has_back
                 has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
 
-            elif len(caa_types) == 1 and ('front' in caa_types or 'back' in caa_types):
-                front_in_caa = caa_node.front[0].text == 'true' and 'front' in caa_types
-                back_in_caa = caa_node.back[0].text == 'true' and 'back' in caa_types
+            elif len(caa_types) == 1 and (has_front or has_back):
+                front_in_caa = caa_node.front[0].text == 'true' and has_front
+                back_in_caa = caa_node.back[0].text == 'true' and has_back
                 has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
 
         if config.setting['ca_provider_use_caa'] and has_caa_artwork\

From a8906f3cec30364a37a4a03c3f4414c420708c9e Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:32:11 +0200
Subject: [PATCH 03/70] Introduce class CoverArt

---
 picard/coverart.py | 139 ++++++++++++++++++++++++---------------------
 1 file changed, 73 insertions(+), 66 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 00606f8b5..29cddcba0 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -74,7 +74,7 @@ def _coverart_http_error(album, http):
     album.error_append(u'Coverart error: %s' % (unicode(http.errorString())))
 
 
-def _coverart_downloaded(album, metadata, release, try_list, coverinfos, data, http, error):
+def _coverart_downloaded(album, metadata, release, coverartobj, coverinfos, data, http, error):
     album._requests -= 1
 
     if error or len(data) < 1000:
@@ -109,14 +109,14 @@ def _coverart_downloaded(album, metadata, release, try_list, coverinfos, data, h
     # If the image already was a front image, there might still be some
     # other front images in the try_list - remove them.
     if is_front_image(coverinfos):
-        for item in try_list[:]:
+        for item in coverartobj.try_list[:]:
             if is_front_image(item) and 'archive.org' not in item['host']:
                 # Hosts other than archive.org only provide front images
-                try_list.remove(item)
-    _walk_try_list(album, metadata, release, try_list)
+                coverartobj.try_list.remove(item)
+    _walk_try_list(album, metadata, release, coverartobj)
 
 
-def _caa_json_downloaded(album, metadata, release, try_list, data, http, error):
+def _caa_json_downloaded(album, metadata, release, coverartobj, data, http, error):
     album._requests -= 1
     caa_front_found = False
     if error:
@@ -139,12 +139,12 @@ def _caa_json_downloaded(album, metadata, release, try_list, data, http, error):
                     if imagetype == "front":
                         caa_front_found = True
                     if imagetype in caa_types:
-                        _caa_append_image_to_trylist(try_list, image)
+                        _caa_append_image_to_trylist(coverartobj, image)
                         break
 
     if error or not caa_front_found:
-        _fill_try_list(album, release, try_list)
-    _walk_try_list(album, metadata, release, try_list)
+        _fill_try_list(album, release, coverartobj)
+    _walk_try_list(album, metadata, release, coverartobj)
 
 _CAA_THUMBNAIL_SIZE_MAP = {
     0: "small",
@@ -152,7 +152,7 @@ _CAA_THUMBNAIL_SIZE_MAP = {
 }
 
 
-def _caa_append_image_to_trylist(try_list, imagedata):
+def _caa_append_image_to_trylist(coverartobj, imagedata):
     """Adds URLs to `try_list` depending on the users CAA image size settings."""
     imagesize = config.setting["caa_image_size"]
     thumbsize = _CAA_THUMBNAIL_SIZE_MAP.get(imagesize, None)
@@ -165,65 +165,72 @@ def _caa_append_image_to_trylist(try_list, imagedata):
         'desc': imagedata["comment"],
         'front': imagedata['front'],  # front image indicator from CAA
     }
-    _try_list_append_image_url(try_list, url, extras)
+    _try_list_append_image_url(coverartobj, url, extras)
 
 
-def coverart(album, metadata, release, try_list=None):
+class CoverArt:
+
+    def __init__(self):
+        self.try_list = []
+
+def coverart(album, metadata, release, coverartobj=None):
     """ Gets all cover art URLs from the metadata and then attempts to
     download the album art. """
 
     # try_list will be None for the first call
-    if try_list is None:
-        try_list = []
+    if coverartobj is not None:
+        return
+    
+    coverartobj = CoverArt()
 
-        # MB web service indicates if CAA has artwork
-        # http://tickets.musicbrainz.org/browse/MBS-4536
-        has_caa_artwork = False
-        caa_types = map(unicode.lower, config.setting["caa_image_types"])
+    # MB web service indicates if CAA has artwork
+    # http://tickets.musicbrainz.org/browse/MBS-4536
+    has_caa_artwork = False
+    caa_types = map(unicode.lower, config.setting["caa_image_types"])
 
-        if 'cover_art_archive' in release.children:
-            caa_node = release.children['cover_art_archive'][0]
-            has_caa_artwork = (caa_node.artwork[0].text == 'true')
-            has_front = 'front' in caa_types
-            has_back = 'back' in caa_types
+    if 'cover_art_archive' in release.children:
+        caa_node = release.children['cover_art_archive'][0]
+        has_caa_artwork = (caa_node.artwork[0].text == 'true')
+        has_front = 'front' in caa_types
+        has_back = 'back' in caa_types
 
-            if len(caa_types) == 2 and (has_front or has_back):
-                # The OR cases are there to still download and process the CAA
-                # JSON file if front or back is enabled but not in the CAA and
-                # another type (that's neither front nor back) is enabled.
-                # For example, if both front and booklet are enabled and the
-                # CAA only has booklet images, the front element in the XML
-                # from the webservice will be false (thus front_in_caa is False
-                # as well) but it's still necessary to download the booklet
-                # images by using the fact that back is enabled but there are
-                # no back images in the CAA.
-                front_in_caa = caa_node.front[0].text == 'true' or not has_front
-                back_in_caa = caa_node.back[0].text == 'true' or not has_back
-                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
+        if len(caa_types) == 2 and (has_front or has_back):
+            # The OR cases are there to still download and process the CAA
+            # JSON file if front or back is enabled but not in the CAA and
+            # another type (that's neither front nor back) is enabled.
+            # For example, if both front and booklet are enabled and the
+            # CAA only has booklet images, the front element in the XML
+            # from the webservice will be false (thus front_in_caa is False
+            # as well) but it's still necessary to download the booklet
+            # images by using the fact that back is enabled but there are
+            # no back images in the CAA.
+            front_in_caa = caa_node.front[0].text == 'true' or not has_front
+            back_in_caa = caa_node.back[0].text == 'true' or not has_back
+            has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
 
-            elif len(caa_types) == 1 and (has_front or has_back):
-                front_in_caa = caa_node.front[0].text == 'true' and has_front
-                back_in_caa = caa_node.back[0].text == 'true' and has_back
-                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
+        elif len(caa_types) == 1 and (has_front or has_back):
+            front_in_caa = caa_node.front[0].text == 'true' and has_front
+            back_in_caa = caa_node.back[0].text == 'true' and has_back
+            has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
 
-        if config.setting['ca_provider_use_caa'] and has_caa_artwork\
-            and len(caa_types) > 0:
-            log.debug("There are suitable images in the cover art archive for %s"
-                      % release.id)
-            album._requests += 1
-            album.tagger.xmlws.download(
-                CAA_HOST, CAA_PORT, "/release/%s/" %
-                metadata["musicbrainz_albumid"],
-                partial(_caa_json_downloaded, album, metadata, release, try_list),
-                priority=True, important=False)
-        else:
-            log.debug("There are no suitable images in the cover art archive for %s"
-                      % release.id)
-            _fill_try_list(album, release, try_list)
-            _walk_try_list(album, metadata, release, try_list)
+    if config.setting['ca_provider_use_caa'] and has_caa_artwork\
+        and len(caa_types) > 0:
+        log.debug("There are suitable images in the cover art archive for %s"
+                    % release.id)
+        album._requests += 1
+        album.tagger.xmlws.download(
+            CAA_HOST, CAA_PORT, "/release/%s/" %
+            metadata["musicbrainz_albumid"],
+            partial(_caa_json_downloaded, album, metadata, release, coverartobj),
+            priority=True, important=False)
+    else:
+        log.debug("There are no suitable images in the cover art archive for %s"
+                    % release.id)
+        _fill_try_list(album, release, coverartobj)
+        _walk_try_list(album, metadata, release, coverartobj)
 
 
-def _fill_try_list(album, release, try_list):
+def _fill_try_list(album, release, coverartobj):
     """Fills ``try_list`` by looking at the relationships in ``release``."""
     use_whitelist = config.setting['ca_provider_use_whitelist']
     use_amazon = config.setting['ca_provider_use_amazon']
@@ -239,26 +246,26 @@ def _fill_try_list(album, release, try_list):
                            and (relation.type == 'cover art link' or
                                 relation.type == 'has_cover_art_at'):
                             url = QUrl(relation.target[0].text)
-                            _try_list_append_image_url(try_list, url)
+                            _try_list_append_image_url(coverartobj, url)
                         elif use_amazon \
                             and (relation.type == 'amazon asin' or
                                  relation.type == 'has_Amazon_ASIN'):
-                            _process_asin_relation(try_list, relation)
+                            _process_asin_relation(coverartobj, relation)
     except AttributeError:
         album.error_append(traceback.format_exc())
 
 
-def _walk_try_list(album, metadata, release, try_list):
+def _walk_try_list(album, metadata, release, coverartobj):
     """Downloads each item in ``try_list``. If there are none left, loading of
     ``album`` will be finalized."""
-    if len(try_list) == 0:
+    if len(coverartobj.try_list) == 0:
         album._finalize_loading(None)
     elif album.id not in album.tagger.albums:
         return
     else:
         # We still have some items to try!
         album._requests += 1
-        coverinfos = try_list.pop(0)
+        coverinfos = coverartobj.try_list.pop(0)
         QObject.tagger.window.set_statusbar_message(
             N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
             {
@@ -269,11 +276,11 @@ def _walk_try_list(album, metadata, release, try_list):
         )
         album.tagger.xmlws.download(
             coverinfos['host'], coverinfos['port'], coverinfos['path'],
-            partial(_coverart_downloaded, album, metadata, release, try_list, coverinfos),
+            partial(_coverart_downloaded, album, metadata, release, coverartobj, coverinfos),
             priority=True, important=False)
 
 
-def _process_asin_relation(try_list, relation):
+def _process_asin_relation(coverartobj, relation):
     amz = parse_amazon_url(relation.target[0].text)
     if amz is not None:
         if amz['host'] in AMAZON_SERVER:
@@ -283,11 +290,11 @@ def _process_asin_relation(try_list, relation):
         host = serverInfo['server']
         path_l = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'L')
         path_m = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'M')
-        _try_list_append_image_url(try_list, QUrl("http://%s:%s" % (host, path_l)))
-        _try_list_append_image_url(try_list, QUrl("http://%s:%s" % (host, path_m)))
+        _try_list_append_image_url(coverartobj, QUrl("http://%s:%s" % (host, path_l)))
+        _try_list_append_image_url(coverartobj, QUrl("http://%s:%s" % (host, path_m)))
 
 
-def _try_list_append_image_url(try_list, parsedUrl, extras=None):
+def _try_list_append_image_url(coverartobj, parsedUrl, extras=None):
     path = str(parsedUrl.encodedPath())
     if parsedUrl.hasQuery():
         path += '?' + parsedUrl.encodedQuery()
@@ -301,4 +308,4 @@ def _try_list_append_image_url(try_list, parsedUrl, extras=None):
     if extras is not None:
         coverinfos.update(extras)
     log.debug("Adding %s image %s", coverinfos['type'], parsedUrl.toString())
-    try_list.append(coverinfos)
+    coverartobj.try_list.append(coverinfos)

From 01a815018a7810fa1cb3147eb42316eefb0fdf9b Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:33:31 +0200
Subject: [PATCH 04/70] Re-order _fill_try_list arguments, place coverartobj in
 first position

---
 picard/coverart.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 29cddcba0..b7bc16a5d 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -143,7 +143,7 @@ def _caa_json_downloaded(album, metadata, release, coverartobj, data, http, erro
                         break
 
     if error or not caa_front_found:
-        _fill_try_list(album, release, coverartobj)
+        _fill_try_list(coverartobj, album, release)
     _walk_try_list(album, metadata, release, coverartobj)
 
 _CAA_THUMBNAIL_SIZE_MAP = {
@@ -226,11 +226,11 @@ def coverart(album, metadata, release, coverartobj=None):
     else:
         log.debug("There are no suitable images in the cover art archive for %s"
                     % release.id)
-        _fill_try_list(album, release, coverartobj)
+        _fill_try_list(coverartobj, album, release)
         _walk_try_list(album, metadata, release, coverartobj)
 
 
-def _fill_try_list(album, release, coverartobj):
+def _fill_try_list(coverartobj, album, release):
     """Fills ``try_list`` by looking at the relationships in ``release``."""
     use_whitelist = config.setting['ca_provider_use_whitelist']
     use_amazon = config.setting['ca_provider_use_amazon']

From 1dd3ba051fbf1e47af925882982626352f7981cf Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:35:24 +0200
Subject: [PATCH 05/70] Re-order _walk_try_list() argument, place coverartobj
 in first position

---
 picard/coverart.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index b7bc16a5d..0c47fd938 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -113,7 +113,7 @@ def _coverart_downloaded(album, metadata, release, coverartobj, coverinfos, data
             if is_front_image(item) and 'archive.org' not in item['host']:
                 # Hosts other than archive.org only provide front images
                 coverartobj.try_list.remove(item)
-    _walk_try_list(album, metadata, release, coverartobj)
+    _walk_try_list(coverartobj, album, metadata, release)
 
 
 def _caa_json_downloaded(album, metadata, release, coverartobj, data, http, error):
@@ -144,7 +144,7 @@ def _caa_json_downloaded(album, metadata, release, coverartobj, data, http, erro
 
     if error or not caa_front_found:
         _fill_try_list(coverartobj, album, release)
-    _walk_try_list(album, metadata, release, coverartobj)
+    _walk_try_list(coverartobj, album, metadata, release)
 
 _CAA_THUMBNAIL_SIZE_MAP = {
     0: "small",
@@ -227,7 +227,7 @@ def coverart(album, metadata, release, coverartobj=None):
         log.debug("There are no suitable images in the cover art archive for %s"
                     % release.id)
         _fill_try_list(coverartobj, album, release)
-        _walk_try_list(album, metadata, release, coverartobj)
+        _walk_try_list(coverartobj, album, metadata, release)
 
 
 def _fill_try_list(coverartobj, album, release):
@@ -255,7 +255,7 @@ def _fill_try_list(coverartobj, album, release):
         album.error_append(traceback.format_exc())
 
 
-def _walk_try_list(album, metadata, release, coverartobj):
+def _walk_try_list(coverartobj, album, metadata, release):
     """Downloads each item in ``try_list``. If there are none left, loading of
     ``album`` will be finalized."""
     if len(coverartobj.try_list) == 0:

From 3287546f002b735f633e70254c11c451f2901bf8 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:36:51 +0200
Subject: [PATCH 06/70] Re-order _caa_json_downloaded() arguments, place
 coverartobj first

---
 picard/coverart.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 0c47fd938..6b32cb4e4 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -116,7 +116,7 @@ def _coverart_downloaded(album, metadata, release, coverartobj, coverinfos, data
     _walk_try_list(coverartobj, album, metadata, release)
 
 
-def _caa_json_downloaded(album, metadata, release, coverartobj, data, http, error):
+def _caa_json_downloaded(coverartobj, album, metadata, release, data, http, error):
     album._requests -= 1
     caa_front_found = False
     if error:
@@ -221,7 +221,7 @@ def coverart(album, metadata, release, coverartobj=None):
         album.tagger.xmlws.download(
             CAA_HOST, CAA_PORT, "/release/%s/" %
             metadata["musicbrainz_albumid"],
-            partial(_caa_json_downloaded, album, metadata, release, coverartobj),
+            partial(_caa_json_downloaded, coverartobj, album, metadata, release),
             priority=True, important=False)
     else:
         log.debug("There are no suitable images in the cover art archive for %s"

From 7a1e01006f06eef18eab87bc9ebbe995c3b55150 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:39:18 +0200
Subject: [PATCH 07/70] Re-order _coverart_downloaded() arguments, place
 coverartobj first

---
 picard/coverart.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 6b32cb4e4..50aae855c 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -74,7 +74,7 @@ def _coverart_http_error(album, http):
     album.error_append(u'Coverart error: %s' % (unicode(http.errorString())))
 
 
-def _coverart_downloaded(album, metadata, release, coverartobj, coverinfos, data, http, error):
+def _coverart_downloaded(coverartobj, album, metadata, release, coverinfos, data, http, error):
     album._requests -= 1
 
     if error or len(data) < 1000:
@@ -276,7 +276,7 @@ def _walk_try_list(coverartobj, album, metadata, release):
         )
         album.tagger.xmlws.download(
             coverinfos['host'], coverinfos['port'], coverinfos['path'],
-            partial(_coverart_downloaded, album, metadata, release, coverartobj, coverinfos),
+            partial(_coverart_downloaded, coverartobj, album, metadata, release, coverinfos),
             priority=True, important=False)
 
 

From c04ffeed9effe39f51e5f322a6fa8b6c09f8613c Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:42:48 +0200
Subject: [PATCH 08/70] Move most code from coverart() to new coverart_init()

---
 picard/coverart.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 50aae855c..c362bd72a 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -173,6 +173,7 @@ class CoverArt:
     def __init__(self):
         self.try_list = []
 
+
 def coverart(album, metadata, release, coverartobj=None):
     """ Gets all cover art URLs from the metadata and then attempts to
     download the album art. """
@@ -180,9 +181,13 @@ def coverart(album, metadata, release, coverartobj=None):
     # try_list will be None for the first call
     if coverartobj is not None:
         return
-    
+
     coverartobj = CoverArt()
 
+    coverart_init(coverartobj, album, metadata, release)
+
+
+def coverart_init(coverartobj, album, metadata, release)
     # MB web service indicates if CAA has artwork
     # http://tickets.musicbrainz.org/browse/MBS-4536
     has_caa_artwork = False

From 7b65ec2e5054fbf76eb084206595bdb5eb3e4b7f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:44:10 +0200
Subject: [PATCH 09/70] Move class CoverArt and coverart() at end of the file

---
 picard/coverart.py | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index c362bd72a..ad6fb7621 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -168,25 +168,6 @@ def _caa_append_image_to_trylist(coverartobj, imagedata):
     _try_list_append_image_url(coverartobj, url, extras)
 
 
-class CoverArt:
-
-    def __init__(self):
-        self.try_list = []
-
-
-def coverart(album, metadata, release, coverartobj=None):
-    """ Gets all cover art URLs from the metadata and then attempts to
-    download the album art. """
-
-    # try_list will be None for the first call
-    if coverartobj is not None:
-        return
-
-    coverartobj = CoverArt()
-
-    coverart_init(coverartobj, album, metadata, release)
-
-
 def coverart_init(coverartobj, album, metadata, release)
     # MB web service indicates if CAA has artwork
     # http://tickets.musicbrainz.org/browse/MBS-4536
@@ -314,3 +295,22 @@ def _try_list_append_image_url(coverartobj, parsedUrl, extras=None):
         coverinfos.update(extras)
     log.debug("Adding %s image %s", coverinfos['type'], parsedUrl.toString())
     coverartobj.try_list.append(coverinfos)
+
+
+class CoverArt:
+
+    def __init__(self):
+        self.try_list = []
+
+
+def coverart(album, metadata, release, coverartobj=None):
+    """ Gets all cover art URLs from the metadata and then attempts to
+    download the album art. """
+
+    # try_list will be None for the first call
+    if coverartobj is not None:
+        return
+
+    coverartobj = CoverArt()
+
+    coverart_init(coverartobj, album, metadata, release)

From d2744cae029f4105cdb76521a5456c347118d19a Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:54:05 +0200
Subject: [PATCH 10/70] Convert most functions to class methods

---
 picard/coverart.py | 433 +++++++++++++++++++++++----------------------
 1 file changed, 218 insertions(+), 215 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index ad6fb7621..1fc2369ec 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -74,77 +74,6 @@ def _coverart_http_error(album, http):
     album.error_append(u'Coverart error: %s' % (unicode(http.errorString())))
 
 
-def _coverart_downloaded(coverartobj, album, metadata, release, coverinfos, data, http, error):
-    album._requests -= 1
-
-    if error or len(data) < 1000:
-        if error:
-            _coverart_http_error(album, http)
-    else:
-        QObject.tagger.window.set_statusbar_message(
-            N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
-            {
-                'type': coverinfos['type'].title(),
-                'albumid': album.id,
-                'host': coverinfos['host']
-            }
-        )
-        mime = mimetype.get_from_data(data, default="image/jpeg")
-
-        try:
-            metadata.make_and_add_image(mime, data,
-                                        imagetype=coverinfos['type'],
-                                        comment=coverinfos['desc'])
-            for track in album._new_tracks:
-                track.metadata.make_and_add_image(mime, data,
-                                                  imagetype=coverinfos['type'],
-                                                  comment=coverinfos['desc'])
-        except (IOError, OSError) as e:
-            album.error_append(e.message)
-            album._finalize_loading(error=True)
-            # It doesn't make sense to store/download more images if we can't
-            # save them in the temporary folder, abort.
-            return
-
-    # If the image already was a front image, there might still be some
-    # other front images in the try_list - remove them.
-    if is_front_image(coverinfos):
-        for item in coverartobj.try_list[:]:
-            if is_front_image(item) and 'archive.org' not in item['host']:
-                # Hosts other than archive.org only provide front images
-                coverartobj.try_list.remove(item)
-    _walk_try_list(coverartobj, album, metadata, release)
-
-
-def _caa_json_downloaded(coverartobj, album, metadata, release, data, http, error):
-    album._requests -= 1
-    caa_front_found = False
-    if error:
-        _coverart_http_error(album, http)
-    else:
-        try:
-            caa_data = json.loads(data)
-        except ValueError:
-            log.debug("Invalid JSON: %s", http.url().toString())
-        else:
-            caa_types = config.setting["caa_image_types"]
-            caa_types = map(unicode.lower, caa_types)
-            for image in caa_data["images"]:
-                if config.setting["caa_approved_only"] and not image["approved"]:
-                    continue
-                if not image["types"] and "unknown" in caa_types:
-                    image["types"] = [u"Unknown"]
-                imagetypes = map(unicode.lower, image["types"])
-                for imagetype in imagetypes:
-                    if imagetype == "front":
-                        caa_front_found = True
-                    if imagetype in caa_types:
-                        _caa_append_image_to_trylist(coverartobj, image)
-                        break
-
-    if error or not caa_front_found:
-        _fill_try_list(coverartobj, album, release)
-    _walk_try_list(coverartobj, album, metadata, release)
 
 _CAA_THUMBNAIL_SIZE_MAP = {
     0: "small",
@@ -152,149 +81,6 @@ _CAA_THUMBNAIL_SIZE_MAP = {
 }
 
 
-def _caa_append_image_to_trylist(coverartobj, imagedata):
-    """Adds URLs to `try_list` depending on the users CAA image size settings."""
-    imagesize = config.setting["caa_image_size"]
-    thumbsize = _CAA_THUMBNAIL_SIZE_MAP.get(imagesize, None)
-    if thumbsize is None:
-        url = QUrl(imagedata["image"])
-    else:
-        url = QUrl(imagedata["thumbnails"][thumbsize])
-    extras = {
-        'type': imagedata["types"][0].lower(),  # FIXME: we pass only 1 type
-        'desc': imagedata["comment"],
-        'front': imagedata['front'],  # front image indicator from CAA
-    }
-    _try_list_append_image_url(coverartobj, url, extras)
-
-
-def coverart_init(coverartobj, album, metadata, release)
-    # MB web service indicates if CAA has artwork
-    # http://tickets.musicbrainz.org/browse/MBS-4536
-    has_caa_artwork = False
-    caa_types = map(unicode.lower, config.setting["caa_image_types"])
-
-    if 'cover_art_archive' in release.children:
-        caa_node = release.children['cover_art_archive'][0]
-        has_caa_artwork = (caa_node.artwork[0].text == 'true')
-        has_front = 'front' in caa_types
-        has_back = 'back' in caa_types
-
-        if len(caa_types) == 2 and (has_front or has_back):
-            # The OR cases are there to still download and process the CAA
-            # JSON file if front or back is enabled but not in the CAA and
-            # another type (that's neither front nor back) is enabled.
-            # For example, if both front and booklet are enabled and the
-            # CAA only has booklet images, the front element in the XML
-            # from the webservice will be false (thus front_in_caa is False
-            # as well) but it's still necessary to download the booklet
-            # images by using the fact that back is enabled but there are
-            # no back images in the CAA.
-            front_in_caa = caa_node.front[0].text == 'true' or not has_front
-            back_in_caa = caa_node.back[0].text == 'true' or not has_back
-            has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
-
-        elif len(caa_types) == 1 and (has_front or has_back):
-            front_in_caa = caa_node.front[0].text == 'true' and has_front
-            back_in_caa = caa_node.back[0].text == 'true' and has_back
-            has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
-
-    if config.setting['ca_provider_use_caa'] and has_caa_artwork\
-        and len(caa_types) > 0:
-        log.debug("There are suitable images in the cover art archive for %s"
-                    % release.id)
-        album._requests += 1
-        album.tagger.xmlws.download(
-            CAA_HOST, CAA_PORT, "/release/%s/" %
-            metadata["musicbrainz_albumid"],
-            partial(_caa_json_downloaded, coverartobj, album, metadata, release),
-            priority=True, important=False)
-    else:
-        log.debug("There are no suitable images in the cover art archive for %s"
-                    % release.id)
-        _fill_try_list(coverartobj, album, release)
-        _walk_try_list(coverartobj, album, metadata, release)
-
-
-def _fill_try_list(coverartobj, album, release):
-    """Fills ``try_list`` by looking at the relationships in ``release``."""
-    use_whitelist = config.setting['ca_provider_use_whitelist']
-    use_amazon = config.setting['ca_provider_use_amazon']
-    if not (use_whitelist or use_amazon):
-        return
-    try:
-        if 'relation_list' in release.children:
-            for relation_list in release.relation_list:
-                if relation_list.target_type == 'url':
-                    for relation in relation_list.relation:
-                        # Use the URL of a cover art link directly
-                        if use_whitelist \
-                           and (relation.type == 'cover art link' or
-                                relation.type == 'has_cover_art_at'):
-                            url = QUrl(relation.target[0].text)
-                            _try_list_append_image_url(coverartobj, url)
-                        elif use_amazon \
-                            and (relation.type == 'amazon asin' or
-                                 relation.type == 'has_Amazon_ASIN'):
-                            _process_asin_relation(coverartobj, relation)
-    except AttributeError:
-        album.error_append(traceback.format_exc())
-
-
-def _walk_try_list(coverartobj, album, metadata, release):
-    """Downloads each item in ``try_list``. If there are none left, loading of
-    ``album`` will be finalized."""
-    if len(coverartobj.try_list) == 0:
-        album._finalize_loading(None)
-    elif album.id not in album.tagger.albums:
-        return
-    else:
-        # We still have some items to try!
-        album._requests += 1
-        coverinfos = coverartobj.try_list.pop(0)
-        QObject.tagger.window.set_statusbar_message(
-            N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
-            {
-                'type': coverinfos['type'],
-                'albumid': album.id,
-                'host': coverinfos['host']
-            }
-        )
-        album.tagger.xmlws.download(
-            coverinfos['host'], coverinfos['port'], coverinfos['path'],
-            partial(_coverart_downloaded, coverartobj, album, metadata, release, coverinfos),
-            priority=True, important=False)
-
-
-def _process_asin_relation(coverartobj, relation):
-    amz = parse_amazon_url(relation.target[0].text)
-    if amz is not None:
-        if amz['host'] in AMAZON_SERVER:
-            serverInfo = AMAZON_SERVER[amz['host']]
-        else:
-            serverInfo = AMAZON_SERVER['amazon.com']
-        host = serverInfo['server']
-        path_l = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'L')
-        path_m = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'M')
-        _try_list_append_image_url(coverartobj, QUrl("http://%s:%s" % (host, path_l)))
-        _try_list_append_image_url(coverartobj, QUrl("http://%s:%s" % (host, path_m)))
-
-
-def _try_list_append_image_url(coverartobj, parsedUrl, extras=None):
-    path = str(parsedUrl.encodedPath())
-    if parsedUrl.hasQuery():
-        path += '?' + parsedUrl.encodedQuery()
-    coverinfos = {
-        'host': str(parsedUrl.host()),
-        'port': parsedUrl.port(80),
-        'path': str(path),
-        'type': 'front',
-        'desc': ''
-    }
-    if extras is not None:
-        coverinfos.update(extras)
-    log.debug("Adding %s image %s", coverinfos['type'], parsedUrl.toString())
-    coverartobj.try_list.append(coverinfos)
 
 
 class CoverArt:
@@ -302,6 +88,223 @@ class CoverArt:
     def __init__(self):
         self.try_list = []
 
+    def _coverart_downloaded(self, album, metadata, release, coverinfos, data, http, error):
+        album._requests -= 1
+
+        if error or len(data) < 1000:
+            if error:
+                _coverart_http_error(album, http)
+        else:
+            QObject.tagger.window.set_statusbar_message(
+                N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
+                {
+                    'type': coverinfos['type'].title(),
+                    'albumid': album.id,
+                    'host': coverinfos['host']
+                }
+            )
+            mime = mimetype.get_from_data(data, default="image/jpeg")
+
+            try:
+                metadata.make_and_add_image(mime, data,
+                                            imagetype=coverinfos['type'],
+                                            comment=coverinfos['desc'])
+                for track in album._new_tracks:
+                    track.metadata.make_and_add_image(mime, data,
+                                                    imagetype=coverinfos['type'],
+                                                    comment=coverinfos['desc'])
+            except (IOError, OSError) as e:
+                album.error_append(e.message)
+                album._finalize_loading(error=True)
+                # It doesn't make sense to store/download more images if we can't
+                # save them in the temporary folder, abort.
+                return
+
+        # If the image already was a front image, there might still be some
+        # other front images in the try_list - remove them.
+        if is_front_image(coverinfos):
+            for item in self.try_list[:]:
+                if is_front_image(item) and 'archive.org' not in item['host']:
+                    # Hosts other than archive.org only provide front images
+                    self.try_list.remove(item)
+        self._walk_try_list(album, metadata, release)
+
+
+    def _caa_json_downloaded(self, album, metadata, release, data, http, error):
+        album._requests -= 1
+        caa_front_found = False
+        if error:
+            _coverart_http_error(album, http)
+        else:
+            try:
+                caa_data = json.loads(data)
+            except ValueError:
+                log.debug("Invalid JSON: %s", http.url().toString())
+            else:
+                caa_types = config.setting["caa_image_types"]
+                caa_types = map(unicode.lower, caa_types)
+                for image in caa_data["images"]:
+                    if config.setting["caa_approved_only"] and not image["approved"]:
+                        continue
+                    if not image["types"] and "unknown" in caa_types:
+                        image["types"] = [u"Unknown"]
+                    imagetypes = map(unicode.lower, image["types"])
+                    for imagetype in imagetypes:
+                        if imagetype == "front":
+                            caa_front_found = True
+                        if imagetype in caa_types:
+                            self._caa_append_image_to_trylist(image)
+                            break
+
+        if error or not caa_front_found:
+            self._fill_try_list(album, release)
+        self._walk_try_list(album, metadata, release)
+
+
+    def _caa_append_image_to_trylist(self, imagedata):
+        """Adds URLs to `try_list` depending on the users CAA image size settings."""
+        imagesize = config.setting["caa_image_size"]
+        thumbsize = _CAA_THUMBNAIL_SIZE_MAP.get(imagesize, None)
+        if thumbsize is None:
+            url = QUrl(imagedata["image"])
+        else:
+            url = QUrl(imagedata["thumbnails"][thumbsize])
+        extras = {
+            'type': imagedata["types"][0].lower(),  # FIXME: we pass only 1 type
+            'desc': imagedata["comment"],
+            'front': imagedata['front'],  # front image indicator from CAA
+        }
+        self._try_list_append_image_url(url, extras)
+
+
+    def coverart_init(self, album, metadata, release):
+        # MB web service indicates if CAA has artwork
+        # http://tickets.musicbrainz.org/browse/MBS-4536
+        has_caa_artwork = False
+        caa_types = map(unicode.lower, config.setting["caa_image_types"])
+
+        if 'cover_art_archive' in release.children:
+            caa_node = release.children['cover_art_archive'][0]
+            has_caa_artwork = (caa_node.artwork[0].text == 'true')
+            has_front = 'front' in caa_types
+            has_back = 'back' in caa_types
+
+            if len(caa_types) == 2 and (has_front or has_back):
+                # The OR cases are there to still download and process the CAA
+                # JSON file if front or back is enabled but not in the CAA and
+                # another type (that's neither front nor back) is enabled.
+                # For example, if both front and booklet are enabled and the
+                # CAA only has booklet images, the front element in the XML
+                # from the webservice will be false (thus front_in_caa is False
+                # as well) but it's still necessary to download the booklet
+                # images by using the fact that back is enabled but there are
+                # no back images in the CAA.
+                front_in_caa = caa_node.front[0].text == 'true' or not has_front
+                back_in_caa = caa_node.back[0].text == 'true' or not has_back
+                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
+
+            elif len(caa_types) == 1 and (has_front or has_back):
+                front_in_caa = caa_node.front[0].text == 'true' and has_front
+                back_in_caa = caa_node.back[0].text == 'true' and has_back
+                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
+
+        if config.setting['ca_provider_use_caa'] and has_caa_artwork\
+            and len(caa_types) > 0:
+            log.debug("There are suitable images in the cover art archive for %s"
+                        % release.id)
+            album._requests += 1
+            album.tagger.xmlws.download(
+                CAA_HOST, CAA_PORT, "/release/%s/" %
+                metadata["musicbrainz_albumid"],
+                partial(self._caa_json_downloaded, album, metadata, release),
+                priority=True, important=False)
+        else:
+            log.debug("There are no suitable images in the cover art archive for %s"
+                        % release.id)
+            self._fill_try_list(album, release)
+            self._walk_try_list(album, metadata, release)
+
+
+    def _fill_try_list(self, album, release):
+        """Fills ``try_list`` by looking at the relationships in ``release``."""
+        use_whitelist = config.setting['ca_provider_use_whitelist']
+        use_amazon = config.setting['ca_provider_use_amazon']
+        if not (use_whitelist or use_amazon):
+            return
+        try:
+            if 'relation_list' in release.children:
+                for relation_list in release.relation_list:
+                    if relation_list.target_type == 'url':
+                        for relation in relation_list.relation:
+                            # Use the URL of a cover art link directly
+                            if use_whitelist \
+                            and (relation.type == 'cover art link' or
+                                    relation.type == 'has_cover_art_at'):
+                                url = QUrl(relation.target[0].text)
+                                self._try_list_append_image_url(url)
+                            elif use_amazon \
+                                and (relation.type == 'amazon asin' or
+                                    relation.type == 'has_Amazon_ASIN'):
+                                self._process_asin_relation(relation)
+        except AttributeError:
+            album.error_append(traceback.format_exc())
+
+
+    def _walk_try_list(self, album, metadata, release):
+        """Downloads each item in ``try_list``. If there are none left, loading of
+        ``album`` will be finalized."""
+        if len(self.try_list) == 0:
+            album._finalize_loading(None)
+        elif album.id not in album.tagger.albums:
+            return
+        else:
+            # We still have some items to try!
+            album._requests += 1
+            coverinfos = self.try_list.pop(0)
+            QObject.tagger.window.set_statusbar_message(
+                N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
+                {
+                    'type': coverinfos['type'],
+                    'albumid': album.id,
+                    'host': coverinfos['host']
+                }
+            )
+            album.tagger.xmlws.download(
+                coverinfos['host'], coverinfos['port'], coverinfos['path'],
+                partial(self._coverart_downloaded, album, metadata, release, coverinfos),
+                priority=True, important=False)
+
+
+    def _process_asin_relation(self, relation):
+        amz = parse_amazon_url(relation.target[0].text)
+        if amz is not None:
+            if amz['host'] in AMAZON_SERVER:
+                serverInfo = AMAZON_SERVER[amz['host']]
+            else:
+                serverInfo = AMAZON_SERVER['amazon.com']
+            host = serverInfo['server']
+            path_l = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'L')
+            path_m = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'M')
+            self._try_list_append_image_url(QUrl("http://%s:%s" % (host, path_l)))
+            self._try_list_append_image_url(QUrl("http://%s:%s" % (host, path_m)))
+
+
+    def _try_list_append_image_url(self, parsedUrl, extras=None):
+        path = str(parsedUrl.encodedPath())
+        if parsedUrl.hasQuery():
+            path += '?' + parsedUrl.encodedQuery()
+        coverinfos = {
+            'host': str(parsedUrl.host()),
+            'port': parsedUrl.port(80),
+            'path': str(path),
+            'type': 'front',
+            'desc': ''
+        }
+        if extras is not None:
+            coverinfos.update(extras)
+        log.debug("Adding %s image %s", coverinfos['type'], parsedUrl.toString())
+        self.try_list.append(coverinfos)
+
 
 def coverart(album, metadata, release, coverartobj=None):
     """ Gets all cover art URLs from the metadata and then attempts to
@@ -313,4 +316,4 @@ def coverart(album, metadata, release, coverartobj=None):
 
     coverartobj = CoverArt()
 
-    coverart_init(coverartobj, album, metadata, release)
+    coverartobj.coverart_init(album, metadata, release)

From 48bd904cf8cf9452111c928bfd71c5bcb3c015d7 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 13:57:30 +0200
Subject: [PATCH 11/70] Move coverart_init() code to __init__()

---
 picard/coverart.py | 100 ++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 52 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 1fc2369ec..129ec9da5 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -85,9 +85,55 @@ _CAA_THUMBNAIL_SIZE_MAP = {
 
 class CoverArt:
 
-    def __init__(self):
+    def __init__(self, album, metadata, release):
         self.try_list = []
 
+        # MB web service indicates if CAA has artwork
+        # http://tickets.musicbrainz.org/browse/MBS-4536
+        has_caa_artwork = False
+        caa_types = map(unicode.lower, config.setting["caa_image_types"])
+
+        if 'cover_art_archive' in release.children:
+            caa_node = release.children['cover_art_archive'][0]
+            has_caa_artwork = (caa_node.artwork[0].text == 'true')
+            has_front = 'front' in caa_types
+            has_back = 'back' in caa_types
+
+            if len(caa_types) == 2 and (has_front or has_back):
+                # The OR cases are there to still download and process the CAA
+                # JSON file if front or back is enabled but not in the CAA and
+                # another type (that's neither front nor back) is enabled.
+                # For example, if both front and booklet are enabled and the
+                # CAA only has booklet images, the front element in the XML
+                # from the webservice will be false (thus front_in_caa is False
+                # as well) but it's still necessary to download the booklet
+                # images by using the fact that back is enabled but there are
+                # no back images in the CAA.
+                front_in_caa = caa_node.front[0].text == 'true' or not has_front
+                back_in_caa = caa_node.back[0].text == 'true' or not has_back
+                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
+
+            elif len(caa_types) == 1 and (has_front or has_back):
+                front_in_caa = caa_node.front[0].text == 'true' and has_front
+                back_in_caa = caa_node.back[0].text == 'true' and has_back
+                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
+
+        if config.setting['ca_provider_use_caa'] and has_caa_artwork\
+            and len(caa_types) > 0:
+            log.debug("There are suitable images in the cover art archive for %s"
+                        % release.id)
+            album._requests += 1
+            album.tagger.xmlws.download(
+                CAA_HOST, CAA_PORT, "/release/%s/" %
+                metadata["musicbrainz_albumid"],
+                partial(self._caa_json_downloaded, album, metadata, release),
+                priority=True, important=False)
+        else:
+            log.debug("There are no suitable images in the cover art archive for %s"
+                        % release.id)
+            self._fill_try_list(album, release)
+            self._walk_try_list(album, metadata, release)
+
     def _coverart_downloaded(self, album, metadata, release, coverinfos, data, http, error):
         album._requests -= 1
 
@@ -177,54 +223,6 @@ class CoverArt:
         self._try_list_append_image_url(url, extras)
 
 
-    def coverart_init(self, album, metadata, release):
-        # MB web service indicates if CAA has artwork
-        # http://tickets.musicbrainz.org/browse/MBS-4536
-        has_caa_artwork = False
-        caa_types = map(unicode.lower, config.setting["caa_image_types"])
-
-        if 'cover_art_archive' in release.children:
-            caa_node = release.children['cover_art_archive'][0]
-            has_caa_artwork = (caa_node.artwork[0].text == 'true')
-            has_front = 'front' in caa_types
-            has_back = 'back' in caa_types
-
-            if len(caa_types) == 2 and (has_front or has_back):
-                # The OR cases are there to still download and process the CAA
-                # JSON file if front or back is enabled but not in the CAA and
-                # another type (that's neither front nor back) is enabled.
-                # For example, if both front and booklet are enabled and the
-                # CAA only has booklet images, the front element in the XML
-                # from the webservice will be false (thus front_in_caa is False
-                # as well) but it's still necessary to download the booklet
-                # images by using the fact that back is enabled but there are
-                # no back images in the CAA.
-                front_in_caa = caa_node.front[0].text == 'true' or not has_front
-                back_in_caa = caa_node.back[0].text == 'true' or not has_back
-                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
-
-            elif len(caa_types) == 1 and (has_front or has_back):
-                front_in_caa = caa_node.front[0].text == 'true' and has_front
-                back_in_caa = caa_node.back[0].text == 'true' and has_back
-                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
-
-        if config.setting['ca_provider_use_caa'] and has_caa_artwork\
-            and len(caa_types) > 0:
-            log.debug("There are suitable images in the cover art archive for %s"
-                        % release.id)
-            album._requests += 1
-            album.tagger.xmlws.download(
-                CAA_HOST, CAA_PORT, "/release/%s/" %
-                metadata["musicbrainz_albumid"],
-                partial(self._caa_json_downloaded, album, metadata, release),
-                priority=True, important=False)
-        else:
-            log.debug("There are no suitable images in the cover art archive for %s"
-                        % release.id)
-            self._fill_try_list(album, release)
-            self._walk_try_list(album, metadata, release)
-
-
     def _fill_try_list(self, album, release):
         """Fills ``try_list`` by looking at the relationships in ``release``."""
         use_whitelist = config.setting['ca_provider_use_whitelist']
@@ -314,6 +312,4 @@ def coverart(album, metadata, release, coverartobj=None):
     if coverartobj is not None:
         return
 
-    coverartobj = CoverArt()
-
-    coverartobj.coverart_init(album, metadata, release)
+    coverartobj = CoverArt(album, metadata, release)

From 007c30dd214164bcc450183313bc2f6b52ed84a2 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 14:01:30 +0200
Subject: [PATCH 12/70] Convert album argument to class property

---
 picard/coverart.py | 55 +++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 129ec9da5..6290d38e4 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -87,6 +87,7 @@ class CoverArt:
 
     def __init__(self, album, metadata, release):
         self.try_list = []
+        self.album = album
 
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
@@ -122,30 +123,30 @@ class CoverArt:
             and len(caa_types) > 0:
             log.debug("There are suitable images in the cover art archive for %s"
                         % release.id)
-            album._requests += 1
-            album.tagger.xmlws.download(
+            self.album._requests += 1
+            self.album.tagger.xmlws.download(
                 CAA_HOST, CAA_PORT, "/release/%s/" %
                 metadata["musicbrainz_albumid"],
-                partial(self._caa_json_downloaded, album, metadata, release),
+                partial(self._caa_json_downloaded, metadata, release),
                 priority=True, important=False)
         else:
             log.debug("There are no suitable images in the cover art archive for %s"
                         % release.id)
-            self._fill_try_list(album, release)
-            self._walk_try_list(album, metadata, release)
+            self._fill_try_list(release)
+            self._walk_try_list(metadata, release)
 
-    def _coverart_downloaded(self, album, metadata, release, coverinfos, data, http, error):
-        album._requests -= 1
+    def _coverart_downloaded(self, metadata, release, coverinfos, data, http, error):
+        self.album._requests -= 1
 
         if error or len(data) < 1000:
             if error:
-                _coverart_http_error(album, http)
+                _coverart_http_error(self.album, http)
         else:
             QObject.tagger.window.set_statusbar_message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
                 {
                     'type': coverinfos['type'].title(),
-                    'albumid': album.id,
+                    'albumid': self.album.id,
                     'host': coverinfos['host']
                 }
             )
@@ -155,13 +156,13 @@ class CoverArt:
                 metadata.make_and_add_image(mime, data,
                                             imagetype=coverinfos['type'],
                                             comment=coverinfos['desc'])
-                for track in album._new_tracks:
+                for track in self.album._new_tracks:
                     track.metadata.make_and_add_image(mime, data,
                                                     imagetype=coverinfos['type'],
                                                     comment=coverinfos['desc'])
             except (IOError, OSError) as e:
-                album.error_append(e.message)
-                album._finalize_loading(error=True)
+                self.album.error_append(e.message)
+                self.album._finalize_loading(error=True)
                 # It doesn't make sense to store/download more images if we can't
                 # save them in the temporary folder, abort.
                 return
@@ -173,14 +174,14 @@ class CoverArt:
                 if is_front_image(item) and 'archive.org' not in item['host']:
                     # Hosts other than archive.org only provide front images
                     self.try_list.remove(item)
-        self._walk_try_list(album, metadata, release)
+        self._walk_try_list(metadata, release)
 
 
-    def _caa_json_downloaded(self, album, metadata, release, data, http, error):
-        album._requests -= 1
+    def _caa_json_downloaded(self, metadata, release, data, http, error):
+        self.album._requests -= 1
         caa_front_found = False
         if error:
-            _coverart_http_error(album, http)
+            _coverart_http_error(self.album, http)
         else:
             try:
                 caa_data = json.loads(data)
@@ -203,8 +204,8 @@ class CoverArt:
                             break
 
         if error or not caa_front_found:
-            self._fill_try_list(album, release)
-        self._walk_try_list(album, metadata, release)
+            self._fill_try_list(release)
+        self._walk_try_list(metadata, release)
 
 
     def _caa_append_image_to_trylist(self, imagedata):
@@ -223,7 +224,7 @@ class CoverArt:
         self._try_list_append_image_url(url, extras)
 
 
-    def _fill_try_list(self, album, release):
+    def _fill_try_list(self, release):
         """Fills ``try_list`` by looking at the relationships in ``release``."""
         use_whitelist = config.setting['ca_provider_use_whitelist']
         use_amazon = config.setting['ca_provider_use_amazon']
@@ -245,31 +246,31 @@ class CoverArt:
                                     relation.type == 'has_Amazon_ASIN'):
                                 self._process_asin_relation(relation)
         except AttributeError:
-            album.error_append(traceback.format_exc())
+            self.album.error_append(traceback.format_exc())
 
 
-    def _walk_try_list(self, album, metadata, release):
+    def _walk_try_list(self, metadata, release):
         """Downloads each item in ``try_list``. If there are none left, loading of
         ``album`` will be finalized."""
         if len(self.try_list) == 0:
-            album._finalize_loading(None)
-        elif album.id not in album.tagger.albums:
+            self.album._finalize_loading(None)
+        elif self.album.id not in self.album.tagger.albums:
             return
         else:
             # We still have some items to try!
-            album._requests += 1
+            self.album._requests += 1
             coverinfos = self.try_list.pop(0)
             QObject.tagger.window.set_statusbar_message(
                 N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
                 {
                     'type': coverinfos['type'],
-                    'albumid': album.id,
+                    'albumid': self.album.id,
                     'host': coverinfos['host']
                 }
             )
-            album.tagger.xmlws.download(
+            self.album.tagger.xmlws.download(
                 coverinfos['host'], coverinfos['port'], coverinfos['path'],
-                partial(self._coverart_downloaded, album, metadata, release, coverinfos),
+                partial(self._coverart_downloaded, metadata, release, coverinfos),
                 priority=True, important=False)
 
 

From d6cd3eb467ac2795ef3bcebfb8aa7d3b90e92a7b Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 14:05:10 +0200
Subject: [PATCH 13/70] Convert metadata argument to class property

---
 picard/coverart.py | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 6290d38e4..ff9298f12 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -88,6 +88,7 @@ class CoverArt:
     def __init__(self, album, metadata, release):
         self.try_list = []
         self.album = album
+        self.metadata = metadata
 
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
@@ -126,16 +127,16 @@ class CoverArt:
             self.album._requests += 1
             self.album.tagger.xmlws.download(
                 CAA_HOST, CAA_PORT, "/release/%s/" %
-                metadata["musicbrainz_albumid"],
-                partial(self._caa_json_downloaded, metadata, release),
+                self.metadata["musicbrainz_albumid"],
+                partial(self._caa_json_downloaded, release),
                 priority=True, important=False)
         else:
             log.debug("There are no suitable images in the cover art archive for %s"
                         % release.id)
             self._fill_try_list(release)
-            self._walk_try_list(metadata, release)
+            self._walk_try_list(release)
 
-    def _coverart_downloaded(self, metadata, release, coverinfos, data, http, error):
+    def _coverart_downloaded(self, release, coverinfos, data, http, error):
         self.album._requests -= 1
 
         if error or len(data) < 1000:
@@ -153,7 +154,7 @@ class CoverArt:
             mime = mimetype.get_from_data(data, default="image/jpeg")
 
             try:
-                metadata.make_and_add_image(mime, data,
+                self.metadata.make_and_add_image(mime, data,
                                             imagetype=coverinfos['type'],
                                             comment=coverinfos['desc'])
                 for track in self.album._new_tracks:
@@ -174,10 +175,10 @@ class CoverArt:
                 if is_front_image(item) and 'archive.org' not in item['host']:
                     # Hosts other than archive.org only provide front images
                     self.try_list.remove(item)
-        self._walk_try_list(metadata, release)
+        self._walk_try_list(release)
 
 
-    def _caa_json_downloaded(self, metadata, release, data, http, error):
+    def _caa_json_downloaded(self, release, data, http, error):
         self.album._requests -= 1
         caa_front_found = False
         if error:
@@ -205,7 +206,7 @@ class CoverArt:
 
         if error or not caa_front_found:
             self._fill_try_list(release)
-        self._walk_try_list(metadata, release)
+        self._walk_try_list(release)
 
 
     def _caa_append_image_to_trylist(self, imagedata):
@@ -249,7 +250,7 @@ class CoverArt:
             self.album.error_append(traceback.format_exc())
 
 
-    def _walk_try_list(self, metadata, release):
+    def _walk_try_list(self, release):
         """Downloads each item in ``try_list``. If there are none left, loading of
         ``album`` will be finalized."""
         if len(self.try_list) == 0:
@@ -270,7 +271,7 @@ class CoverArt:
             )
             self.album.tagger.xmlws.download(
                 coverinfos['host'], coverinfos['port'], coverinfos['path'],
-                partial(self._coverart_downloaded, metadata, release, coverinfos),
+                partial(self._coverart_downloaded, release, coverinfos),
                 priority=True, important=False)
 
 

From 7ab4677a4e6adcdd6de704879aa8270c5ae93407 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 14:07:36 +0200
Subject: [PATCH 14/70] Move _coverart_http_error() to class CoverArt

---
 picard/coverart.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index ff9298f12..1eca4db39 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -70,11 +70,6 @@ AMAZON_SERVER = {
 AMAZON_IMAGE_PATH = '/images/P/%s.%s.%sZZZZZZZ.jpg'
 
 
-def _coverart_http_error(album, http):
-    album.error_append(u'Coverart error: %s' % (unicode(http.errorString())))
-
-
-
 _CAA_THUMBNAIL_SIZE_MAP = {
     0: "small",
     1: "large",
@@ -136,12 +131,15 @@ class CoverArt:
             self._fill_try_list(release)
             self._walk_try_list(release)
 
+    def _coverart_http_error(self, http):
+        self.album.error_append(u'Coverart error: %s' % (unicode(http.errorString())))
+
     def _coverart_downloaded(self, release, coverinfos, data, http, error):
         self.album._requests -= 1
 
         if error or len(data) < 1000:
             if error:
-                _coverart_http_error(self.album, http)
+                self._coverart_http_error(http)
         else:
             QObject.tagger.window.set_statusbar_message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
@@ -182,7 +180,7 @@ class CoverArt:
         self.album._requests -= 1
         caa_front_found = False
         if error:
-            _coverart_http_error(self.album, http)
+            self._coverart_http_error(http)
         else:
             try:
                 caa_data = json.loads(data)

From 6cdecc328e143a355ea0ef9e3be4eb5b751bca85 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 14:10:55 +0200
Subject: [PATCH 15/70] Convert release argument to class property

---
 picard/coverart.py | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 1eca4db39..d3861b3e2 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -84,14 +84,15 @@ class CoverArt:
         self.try_list = []
         self.album = album
         self.metadata = metadata
+        self.release = release
 
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
         has_caa_artwork = False
         caa_types = map(unicode.lower, config.setting["caa_image_types"])
 
-        if 'cover_art_archive' in release.children:
-            caa_node = release.children['cover_art_archive'][0]
+        if 'cover_art_archive' in self.release.children:
+            caa_node = self.release.children['cover_art_archive'][0]
             has_caa_artwork = (caa_node.artwork[0].text == 'true')
             has_front = 'front' in caa_types
             has_back = 'back' in caa_types
@@ -118,23 +119,23 @@ class CoverArt:
         if config.setting['ca_provider_use_caa'] and has_caa_artwork\
             and len(caa_types) > 0:
             log.debug("There are suitable images in the cover art archive for %s"
-                        % release.id)
+                        % self.release.id)
             self.album._requests += 1
             self.album.tagger.xmlws.download(
                 CAA_HOST, CAA_PORT, "/release/%s/" %
                 self.metadata["musicbrainz_albumid"],
-                partial(self._caa_json_downloaded, release),
+                partial(self._caa_json_downloaded),
                 priority=True, important=False)
         else:
             log.debug("There are no suitable images in the cover art archive for %s"
-                        % release.id)
-            self._fill_try_list(release)
-            self._walk_try_list(release)
+                        % self.release.id)
+            self._fill_try_list()
+            self._walk_try_list()
 
     def _coverart_http_error(self, http):
         self.album.error_append(u'Coverart error: %s' % (unicode(http.errorString())))
 
-    def _coverart_downloaded(self, release, coverinfos, data, http, error):
+    def _coverart_downloaded(self, coverinfos, data, http, error):
         self.album._requests -= 1
 
         if error or len(data) < 1000:
@@ -173,10 +174,10 @@ class CoverArt:
                 if is_front_image(item) and 'archive.org' not in item['host']:
                     # Hosts other than archive.org only provide front images
                     self.try_list.remove(item)
-        self._walk_try_list(release)
+        self._walk_try_list()
 
 
-    def _caa_json_downloaded(self, release, data, http, error):
+    def _caa_json_downloaded(self, data, http, error):
         self.album._requests -= 1
         caa_front_found = False
         if error:
@@ -203,8 +204,8 @@ class CoverArt:
                             break
 
         if error or not caa_front_found:
-            self._fill_try_list(release)
-        self._walk_try_list(release)
+            self._fill_try_list()
+        self._walk_try_list()
 
 
     def _caa_append_image_to_trylist(self, imagedata):
@@ -223,15 +224,15 @@ class CoverArt:
         self._try_list_append_image_url(url, extras)
 
 
-    def _fill_try_list(self, release):
+    def _fill_try_list(self):
         """Fills ``try_list`` by looking at the relationships in ``release``."""
         use_whitelist = config.setting['ca_provider_use_whitelist']
         use_amazon = config.setting['ca_provider_use_amazon']
         if not (use_whitelist or use_amazon):
             return
         try:
-            if 'relation_list' in release.children:
-                for relation_list in release.relation_list:
+            if 'relation_list' in self.release.children:
+                for relation_list in self.release.relation_list:
                     if relation_list.target_type == 'url':
                         for relation in relation_list.relation:
                             # Use the URL of a cover art link directly
@@ -248,7 +249,7 @@ class CoverArt:
             self.album.error_append(traceback.format_exc())
 
 
-    def _walk_try_list(self, release):
+    def _walk_try_list(self):
         """Downloads each item in ``try_list``. If there are none left, loading of
         ``album`` will be finalized."""
         if len(self.try_list) == 0:
@@ -269,7 +270,7 @@ class CoverArt:
             )
             self.album.tagger.xmlws.download(
                 coverinfos['host'], coverinfos['port'], coverinfos['path'],
-                partial(self._coverart_downloaded, release, coverinfos),
+                partial(self._coverart_downloaded, coverinfos),
                 priority=True, important=False)
 
 

From f3edf6cefe695c04fdad1fcb8bb258f45c7a900e Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 14:17:02 +0200
Subject: [PATCH 16/70] Drop '_try_list' prefix/suffix and shorten method names

---
 picard/coverart.py | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index d3861b3e2..52f4c3f2b 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -129,8 +129,8 @@ class CoverArt:
         else:
             log.debug("There are no suitable images in the cover art archive for %s"
                         % self.release.id)
-            self._fill_try_list()
-            self._walk_try_list()
+            self._fill()
+            self._walk()
 
     def _coverart_http_error(self, http):
         self.album.error_append(u'Coverart error: %s' % (unicode(http.errorString())))
@@ -174,7 +174,7 @@ class CoverArt:
                 if is_front_image(item) and 'archive.org' not in item['host']:
                     # Hosts other than archive.org only provide front images
                     self.try_list.remove(item)
-        self._walk_try_list()
+        self._walk()
 
 
     def _caa_json_downloaded(self, data, http, error):
@@ -204,8 +204,8 @@ class CoverArt:
                             break
 
         if error or not caa_front_found:
-            self._fill_try_list()
-        self._walk_try_list()
+            self._fill()
+        self._walk()
 
 
     def _caa_append_image_to_trylist(self, imagedata):
@@ -221,10 +221,10 @@ class CoverArt:
             'desc': imagedata["comment"],
             'front': imagedata['front'],  # front image indicator from CAA
         }
-        self._try_list_append_image_url(url, extras)
+        self._append_image_url(url, extras)
 
 
-    def _fill_try_list(self):
+    def _fill(self):
         """Fills ``try_list`` by looking at the relationships in ``release``."""
         use_whitelist = config.setting['ca_provider_use_whitelist']
         use_amazon = config.setting['ca_provider_use_amazon']
@@ -240,7 +240,7 @@ class CoverArt:
                             and (relation.type == 'cover art link' or
                                     relation.type == 'has_cover_art_at'):
                                 url = QUrl(relation.target[0].text)
-                                self._try_list_append_image_url(url)
+                                self._append_image_url(url)
                             elif use_amazon \
                                 and (relation.type == 'amazon asin' or
                                     relation.type == 'has_Amazon_ASIN'):
@@ -249,7 +249,7 @@ class CoverArt:
             self.album.error_append(traceback.format_exc())
 
 
-    def _walk_try_list(self):
+    def _walk(self):
         """Downloads each item in ``try_list``. If there are none left, loading of
         ``album`` will be finalized."""
         if len(self.try_list) == 0:
@@ -284,11 +284,11 @@ class CoverArt:
             host = serverInfo['server']
             path_l = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'L')
             path_m = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'M')
-            self._try_list_append_image_url(QUrl("http://%s:%s" % (host, path_l)))
-            self._try_list_append_image_url(QUrl("http://%s:%s" % (host, path_m)))
+            self._append_image_url(QUrl("http://%s:%s" % (host, path_l)))
+            self._append_image_url(QUrl("http://%s:%s" % (host, path_m)))
 
 
-    def _try_list_append_image_url(self, parsedUrl, extras=None):
+    def _append_image_url(self, parsedUrl, extras=None):
         path = str(parsedUrl.encodedPath())
         if parsedUrl.hasQuery():
             path += '?' + parsedUrl.encodedQuery()

From 139ab5b1e8390b9818f0777848b659faecd487b3 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 14:25:23 +0200
Subject: [PATCH 17/70] Store accepted CAA types and number of them in class
 properties

---
 picard/coverart.py | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 52f4c3f2b..06b744d01 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -85,19 +85,20 @@ class CoverArt:
         self.album = album
         self.metadata = metadata
         self.release = release
+        self.caa_types = map(unicode.lower, config.setting["caa_image_types"])
+        self.len_caa_types = len(self.caa_types)
 
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
         has_caa_artwork = False
-        caa_types = map(unicode.lower, config.setting["caa_image_types"])
 
         if 'cover_art_archive' in self.release.children:
             caa_node = self.release.children['cover_art_archive'][0]
             has_caa_artwork = (caa_node.artwork[0].text == 'true')
-            has_front = 'front' in caa_types
-            has_back = 'back' in caa_types
+            has_front = 'front' in self.caa_types
+            has_back = 'back' in self.caa_types
 
-            if len(caa_types) == 2 and (has_front or has_back):
+            if self.len_caa_types == 2 and (has_front or has_back):
                 # The OR cases are there to still download and process the CAA
                 # JSON file if front or back is enabled but not in the CAA and
                 # another type (that's neither front nor back) is enabled.
@@ -111,13 +112,13 @@ class CoverArt:
                 back_in_caa = caa_node.back[0].text == 'true' or not has_back
                 has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
 
-            elif len(caa_types) == 1 and (has_front or has_back):
+            elif self.len_caa_types == 1 and (has_front or has_back):
                 front_in_caa = caa_node.front[0].text == 'true' and has_front
                 back_in_caa = caa_node.back[0].text == 'true' and has_back
                 has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
 
         if config.setting['ca_provider_use_caa'] and has_caa_artwork\
-            and len(caa_types) > 0:
+            and self.len_caa_types > 0:
             log.debug("There are suitable images in the cover art archive for %s"
                         % self.release.id)
             self.album._requests += 1
@@ -188,18 +189,16 @@ class CoverArt:
             except ValueError:
                 log.debug("Invalid JSON: %s", http.url().toString())
             else:
-                caa_types = config.setting["caa_image_types"]
-                caa_types = map(unicode.lower, caa_types)
                 for image in caa_data["images"]:
                     if config.setting["caa_approved_only"] and not image["approved"]:
                         continue
-                    if not image["types"] and "unknown" in caa_types:
+                    if not image["types"] and "unknown" in self.caa_types:
                         image["types"] = [u"Unknown"]
                     imagetypes = map(unicode.lower, image["types"])
                     for imagetype in imagetypes:
                         if imagetype == "front":
                             caa_front_found = True
-                        if imagetype in caa_types:
+                        if imagetype in self.caa_types:
                             self._caa_append_image_to_trylist(image)
                             break
 

From b5625207c4315c45811eb8470f81b3a14b723192 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 14:34:25 +0200
Subject: [PATCH 18/70] Tidy up, PEP8 conformance

---
 picard/coverart.py | 63 +++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 06b744d01..ef305cbe0 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -76,8 +76,6 @@ _CAA_THUMBNAIL_SIZE_MAP = {
 }
 
 
-
-
 class CoverArt:
 
     def __init__(self, album, metadata, release):
@@ -118,23 +116,27 @@ class CoverArt:
                 has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
 
         if config.setting['ca_provider_use_caa'] and has_caa_artwork\
-            and self.len_caa_types > 0:
+                and self.len_caa_types > 0:
             log.debug("There are suitable images in the cover art archive for %s"
-                        % self.release.id)
+                      % self.release.id)
             self.album._requests += 1
             self.album.tagger.xmlws.download(
-                CAA_HOST, CAA_PORT, "/release/%s/" %
-                self.metadata["musicbrainz_albumid"],
-                partial(self._caa_json_downloaded),
-                priority=True, important=False)
+                CAA_HOST,
+                CAA_PORT,
+                "/release/%s/" % self.metadata["musicbrainz_albumid"],
+                self._caa_json_downloaded,
+                priority=True,
+                important=False
+            )
         else:
             log.debug("There are no suitable images in the cover art archive for %s"
-                        % self.release.id)
+                      % self.release.id)
             self._fill()
             self._walk()
 
     def _coverart_http_error(self, http):
-        self.album.error_append(u'Coverart error: %s' % (unicode(http.errorString())))
+        self.album.error_append(u'Coverart error: %s' %
+                                (unicode(http.errorString())))
 
     def _coverart_downloaded(self, coverinfos, data, http, error):
         self.album._requests -= 1
@@ -154,13 +156,19 @@ class CoverArt:
             mime = mimetype.get_from_data(data, default="image/jpeg")
 
             try:
-                self.metadata.make_and_add_image(mime, data,
-                                            imagetype=coverinfos['type'],
-                                            comment=coverinfos['desc'])
+                self.metadata.make_and_add_image(
+                    mime,
+                    data,
+                    imagetype=coverinfos['type'],
+                    comment=coverinfos['desc']
+                )
                 for track in self.album._new_tracks:
-                    track.metadata.make_and_add_image(mime, data,
-                                                    imagetype=coverinfos['type'],
-                                                    comment=coverinfos['desc'])
+                    track.metadata.make_and_add_image(
+                        mime,
+                        data,
+                        imagetype=coverinfos['type'],
+                        comment=coverinfos['desc']
+                    )
             except (IOError, OSError) as e:
                 self.album.error_append(e.message)
                 self.album._finalize_loading(error=True)
@@ -177,7 +185,6 @@ class CoverArt:
                     self.try_list.remove(item)
         self._walk()
 
-
     def _caa_json_downloaded(self, data, http, error):
         self.album._requests -= 1
         caa_front_found = False
@@ -206,7 +213,6 @@ class CoverArt:
             self._fill()
         self._walk()
 
-
     def _caa_append_image_to_trylist(self, imagedata):
         """Adds URLs to `try_list` depending on the users CAA image size settings."""
         imagesize = config.setting["caa_image_size"]
@@ -222,7 +228,6 @@ class CoverArt:
         }
         self._append_image_url(url, extras)
 
-
     def _fill(self):
         """Fills ``try_list`` by looking at the relationships in ``release``."""
         use_whitelist = config.setting['ca_provider_use_whitelist']
@@ -236,18 +241,17 @@ class CoverArt:
                         for relation in relation_list.relation:
                             # Use the URL of a cover art link directly
                             if use_whitelist \
-                            and (relation.type == 'cover art link' or
-                                    relation.type == 'has_cover_art_at'):
+                                and (relation.type == 'cover art link' or
+                                     relation.type == 'has_cover_art_at'):
                                 url = QUrl(relation.target[0].text)
                                 self._append_image_url(url)
                             elif use_amazon \
                                 and (relation.type == 'amazon asin' or
-                                    relation.type == 'has_Amazon_ASIN'):
+                                     relation.type == 'has_Amazon_ASIN'):
                                 self._process_asin_relation(relation)
         except AttributeError:
             self.album.error_append(traceback.format_exc())
 
-
     def _walk(self):
         """Downloads each item in ``try_list``. If there are none left, loading of
         ``album`` will be finalized."""
@@ -268,10 +272,13 @@ class CoverArt:
                 }
             )
             self.album.tagger.xmlws.download(
-                coverinfos['host'], coverinfos['port'], coverinfos['path'],
+                coverinfos['host'],
+                coverinfos['port'],
+                coverinfos['path'],
                 partial(self._coverart_downloaded, coverinfos),
-                priority=True, important=False)
-
+                priority=True,
+                important=False
+            )
 
     def _process_asin_relation(self, relation):
         amz = parse_amazon_url(relation.target[0].text)
@@ -286,7 +293,6 @@ class CoverArt:
             self._append_image_url(QUrl("http://%s:%s" % (host, path_l)))
             self._append_image_url(QUrl("http://%s:%s" % (host, path_m)))
 
-
     def _append_image_url(self, parsedUrl, extras=None):
         path = str(parsedUrl.encodedPath())
         if parsedUrl.hasQuery():
@@ -300,7 +306,8 @@ class CoverArt:
         }
         if extras is not None:
             coverinfos.update(extras)
-        log.debug("Adding %s image %s", coverinfos['type'], parsedUrl.toString())
+        log.debug("Adding %s image %s",
+                  coverinfos['type'], parsedUrl.toString())
         self.try_list.append(coverinfos)
 
 

From 0908b0e6c97f67fbd7803fe81420a8d944a29850 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 14:47:02 +0200
Subject: [PATCH 19/70] _caa_append_image_to_trylist() -> _append_caa_image()

---
 picard/coverart.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index ef305cbe0..5e707a4bc 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -206,14 +206,14 @@ class CoverArt:
                         if imagetype == "front":
                             caa_front_found = True
                         if imagetype in self.caa_types:
-                            self._caa_append_image_to_trylist(image)
+                            self._append_caa_image(image)
                             break
 
         if error or not caa_front_found:
             self._fill()
         self._walk()
 
-    def _caa_append_image_to_trylist(self, imagedata):
+    def _append_caa_image(self, imagedata):
         """Adds URLs to `try_list` depending on the users CAA image size settings."""
         imagesize = config.setting["caa_image_size"]
         thumbsize = _CAA_THUMBNAIL_SIZE_MAP.get(imagesize, None)

From 84ffb3f32c8b1c0b171cf46715885162aa8f780d Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 14:47:38 +0200
Subject: [PATCH 20/70] imagedata -> image

---
 picard/coverart.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 5e707a4bc..d18c2628b 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -213,18 +213,18 @@ class CoverArt:
             self._fill()
         self._walk()
 
-    def _append_caa_image(self, imagedata):
+    def _append_caa_image(self, image):
         """Adds URLs to `try_list` depending on the users CAA image size settings."""
         imagesize = config.setting["caa_image_size"]
         thumbsize = _CAA_THUMBNAIL_SIZE_MAP.get(imagesize, None)
         if thumbsize is None:
-            url = QUrl(imagedata["image"])
+            url = QUrl(image["image"])
         else:
-            url = QUrl(imagedata["thumbnails"][thumbsize])
+            url = QUrl(image["thumbnails"][thumbsize])
         extras = {
-            'type': imagedata["types"][0].lower(),  # FIXME: we pass only 1 type
-            'desc': imagedata["comment"],
-            'front': imagedata['front'],  # front image indicator from CAA
+            'type': image["types"][0].lower(),  # FIXME: we pass only 1 type
+            'desc': image["comment"],
+            'front': image['front'],  # front image indicator from CAA
         }
         self._append_image_url(url, extras)
 

From c5efe95ff0b6a5b5b2de60732718fc7abacf3a03 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Wed, 7 May 2014 18:00:58 +0200
Subject: [PATCH 21/70] Wrap QObject.tagger.window.set_statusbar_message() in
 new message() class method

---
 picard/coverart.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index d18c2628b..6c847acfb 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -134,6 +134,9 @@ class CoverArt:
             self._fill()
             self._walk()
 
+    def message(self, *args, **kwargs):
+        QObject.tagger.window.set_statusbar_message(*args, **kwargs)
+
     def _coverart_http_error(self, http):
         self.album.error_append(u'Coverart error: %s' %
                                 (unicode(http.errorString())))
@@ -145,7 +148,7 @@ class CoverArt:
             if error:
                 self._coverart_http_error(http)
         else:
-            QObject.tagger.window.set_statusbar_message(
+            self.message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
                 {
                     'type': coverinfos['type'].title(),
@@ -263,7 +266,7 @@ class CoverArt:
             # We still have some items to try!
             self.album._requests += 1
             coverinfos = self.try_list.pop(0)
-            QObject.tagger.window.set_statusbar_message(
+            self.message(
                 N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
                 {
                     'type': coverinfos['type'],

From f7bf4acfa1ea9875b41b66f8ac28391fb792305f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 15:00:49 +0200
Subject: [PATCH 22/70] Pass url as text to _append_image_url() instead of QUrl
 object

---
 picard/coverart.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 6c847acfb..4fa895442 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -221,9 +221,9 @@ class CoverArt:
         imagesize = config.setting["caa_image_size"]
         thumbsize = _CAA_THUMBNAIL_SIZE_MAP.get(imagesize, None)
         if thumbsize is None:
-            url = QUrl(image["image"])
+            url = image["image"]
         else:
-            url = QUrl(image["thumbnails"][thumbsize])
+            url = image["thumbnails"][thumbsize]
         extras = {
             'type': image["types"][0].lower(),  # FIXME: we pass only 1 type
             'desc': image["comment"],
@@ -246,7 +246,7 @@ class CoverArt:
                             if use_whitelist \
                                 and (relation.type == 'cover art link' or
                                      relation.type == 'has_cover_art_at'):
-                                url = QUrl(relation.target[0].text)
+                                url = relation.target[0].text
                                 self._append_image_url(url)
                             elif use_amazon \
                                 and (relation.type == 'amazon asin' or
@@ -293,10 +293,11 @@ class CoverArt:
             host = serverInfo['server']
             path_l = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'L')
             path_m = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'M')
-            self._append_image_url(QUrl("http://%s:%s" % (host, path_l)))
-            self._append_image_url(QUrl("http://%s:%s" % (host, path_m)))
+            self._append_image_url("http://%s:%s" % (host, path_l))
+            self._append_image_url("http://%s:%s" % (host, path_m))
 
-    def _append_image_url(self, parsedUrl, extras=None):
+    def _append_image_url(self, url, extras=None):
+        parsedUrl = QUrl(url)
         path = str(parsedUrl.encodedPath())
         if parsedUrl.hasQuery():
             path += '?' + parsedUrl.encodedQuery()

From a43569755d124cfa75766cb13d74af2199ad4932 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 15:40:45 +0200
Subject: [PATCH 23/70] Introduce CoverArtImage class and simplify code

- move is_front_image() from metadata.py to class CoverArtImage as it is only used here
---
 picard/coverart.py | 101 +++++++++++++++++++++++++--------------------
 picard/metadata.py |   9 ----
 2 files changed, 56 insertions(+), 54 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 4fa895442..e741c6e14 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -26,7 +26,7 @@ import re
 import traceback
 from functools import partial
 from picard import config, log
-from picard.metadata import Image, is_front_image
+from picard.metadata import Image
 from picard.util import mimetype, parse_amazon_url
 from picard.const import CAA_HOST, CAA_PORT
 from PyQt4.QtCore import QUrl, QObject
@@ -75,6 +75,30 @@ _CAA_THUMBNAIL_SIZE_MAP = {
     1: "large",
 }
 
+class CoverArtImage:
+
+    def __init__(self, url, type='front', desc='', front=None):
+        self.url = QUrl(url)
+        path = str(self.url.encodedPath())
+        if self.url.hasQuery():
+            path += '?' + self.url.encodedQuery()
+        self.host = str(self.url.host())
+        self.port = self.url.port(80)
+        self.path = str(path)
+        self.type = type
+        self.desc = desc
+        self.front = front
+
+    def is_front_image(self):
+        # CAA has a flag for "front" image, use it in priority
+        if self.front is None:
+            # no caa front flag, use type instead
+            return (self.type == 'front')
+        return self.front
+
+    def __repr__(self):
+        return "type: %r from %s" % (self.type, self.url.toString())
+
 
 class CoverArt:
 
@@ -141,7 +165,7 @@ class CoverArt:
         self.album.error_append(u'Coverart error: %s' %
                                 (unicode(http.errorString())))
 
-    def _coverart_downloaded(self, coverinfos, data, http, error):
+    def _coverart_downloaded(self, coverartimage, data, http, error):
         self.album._requests -= 1
 
         if error or len(data) < 1000:
@@ -151,9 +175,9 @@ class CoverArt:
             self.message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
                 {
-                    'type': coverinfos['type'].title(),
+                    'type': coverartimage.type,
                     'albumid': self.album.id,
-                    'host': coverinfos['host']
+                    'host': coverartimage.host
                 }
             )
             mime = mimetype.get_from_data(data, default="image/jpeg")
@@ -162,15 +186,15 @@ class CoverArt:
                 self.metadata.make_and_add_image(
                     mime,
                     data,
-                    imagetype=coverinfos['type'],
-                    comment=coverinfos['desc']
+                    imagetype=coverartimage.type,
+                    comment=coverartimage.desc
                 )
                 for track in self.album._new_tracks:
                     track.metadata.make_and_add_image(
                         mime,
                         data,
-                        imagetype=coverinfos['type'],
-                        comment=coverinfos['desc']
+                        imagetype=coverartimage.type,
+                        comment=coverartimage.desc
                     )
             except (IOError, OSError) as e:
                 self.album.error_append(e.message)
@@ -181,9 +205,9 @@ class CoverArt:
 
         # If the image already was a front image, there might still be some
         # other front images in the try_list - remove them.
-        if is_front_image(coverinfos):
+        if coverartimage.is_front_image():
             for item in self.try_list[:]:
-                if is_front_image(item) and 'archive.org' not in item['host']:
+                if item.is_front_image() and 'archive.org' not in item.host:
                     # Hosts other than archive.org only provide front images
                     self.try_list.remove(item)
         self._walk()
@@ -224,12 +248,13 @@ class CoverArt:
             url = image["image"]
         else:
             url = image["thumbnails"][thumbsize]
-        extras = {
-            'type': image["types"][0].lower(),  # FIXME: we pass only 1 type
-            'desc': image["comment"],
-            'front': image['front'],  # front image indicator from CAA
-        }
-        self._append_image_url(url, extras)
+        coverartimage = CoverArtImage(
+            url,
+            type = image["types"][0].lower(),  # FIXME: we pass only 1 type
+            desc = image["comment"],
+            front = image['front'],  # front image indicator from CAA
+        )
+        self._append_image(coverartimage)
 
     def _fill(self):
         """Fills ``try_list`` by looking at the relationships in ``release``."""
@@ -247,7 +272,7 @@ class CoverArt:
                                 and (relation.type == 'cover art link' or
                                      relation.type == 'has_cover_art_at'):
                                 url = relation.target[0].text
-                                self._append_image_url(url)
+                                self._append_image(CoverArtImage(url))
                             elif use_amazon \
                                 and (relation.type == 'amazon asin' or
                                      relation.type == 'has_Amazon_ASIN'):
@@ -265,20 +290,20 @@ class CoverArt:
         else:
             # We still have some items to try!
             self.album._requests += 1
-            coverinfos = self.try_list.pop(0)
+            coverartimage = self.try_list.pop(0)
             self.message(
                 N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
                 {
-                    'type': coverinfos['type'],
+                    'type': coverartimage.type,
                     'albumid': self.album.id,
-                    'host': coverinfos['host']
+                    'host': coverartimage.host
                 }
             )
             self.album.tagger.xmlws.download(
-                coverinfos['host'],
-                coverinfos['port'],
-                coverinfos['path'],
-                partial(self._coverart_downloaded, coverinfos),
+                coverartimage.host,
+                coverartimage.port,
+                coverartimage.path,
+                partial(self._coverart_downloaded, coverartimage),
                 priority=True,
                 important=False
             )
@@ -291,28 +316,14 @@ class CoverArt:
             else:
                 serverInfo = AMAZON_SERVER['amazon.com']
             host = serverInfo['server']
-            path_l = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'L')
-            path_m = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], 'M')
-            self._append_image_url("http://%s:%s" % (host, path_l))
-            self._append_image_url("http://%s:%s" % (host, path_m))
+            for size in ('L', 'M'):
+                path = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], size)
+                url = "http://%s:%s" % (host, path)
+                self._append_image(CoverArtImage(url))
 
-    def _append_image_url(self, url, extras=None):
-        parsedUrl = QUrl(url)
-        path = str(parsedUrl.encodedPath())
-        if parsedUrl.hasQuery():
-            path += '?' + parsedUrl.encodedQuery()
-        coverinfos = {
-            'host': str(parsedUrl.host()),
-            'port': parsedUrl.port(80),
-            'path': str(path),
-            'type': 'front',
-            'desc': ''
-        }
-        if extras is not None:
-            coverinfos.update(extras)
-        log.debug("Adding %s image %s",
-                  coverinfos['type'], parsedUrl.toString())
-        self.try_list.append(coverinfos)
+    def _append_image(self, coverartimage):
+        log.debug("Appending cover art image %r", coverartimage)
+        self.try_list.append(coverartimage)
 
 
 def coverart(album, metadata, release, coverartobj=None):
diff --git a/picard/metadata.py b/picard/metadata.py
index f7542fccc..78ed1d138 100644
--- a/picard/metadata.py
+++ b/picard/metadata.py
@@ -45,15 +45,6 @@ from picard.mbxml import artist_credit_from_node
 MULTI_VALUED_JOINER = '; '
 
 
-def is_front_image(image):
-    # CAA has a flag for "front" image, use it in priority
-    caa_front = image.get('front', None)
-    if caa_front is None:
-        # no caa front flag, use type instead
-        return (image['type'] == 'front')
-    return caa_front
-
-
 def save_this_image_to_tags(image):
     if not config.setting["save_only_front_images_to_tags"]:
         return True

From e31d77f8d52fb9f1c1f26c1efc86f5696aaed88c Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 16:05:56 +0200
Subject: [PATCH 24/70] Wraps self.album.tagger.xmlws.download() in new
 _download() method

---
 picard/coverart.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index e741c6e14..d0dcfa307 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -143,8 +143,7 @@ class CoverArt:
                 and self.len_caa_types > 0:
             log.debug("There are suitable images in the cover art archive for %s"
                       % self.release.id)
-            self.album._requests += 1
-            self.album.tagger.xmlws.download(
+            self._download(
                 CAA_HOST,
                 CAA_PORT,
                 "/release/%s/" % self.metadata["musicbrainz_albumid"],
@@ -161,6 +160,10 @@ class CoverArt:
     def message(self, *args, **kwargs):
         QObject.tagger.window.set_statusbar_message(*args, **kwargs)
 
+    def _download(self, *args, **kwargs):
+        self.album._requests += 1
+        self.album.tagger.xmlws.download(*args, **kwargs)
+
     def _coverart_http_error(self, http):
         self.album.error_append(u'Coverart error: %s' %
                                 (unicode(http.errorString())))
@@ -289,7 +292,6 @@ class CoverArt:
             return
         else:
             # We still have some items to try!
-            self.album._requests += 1
             coverartimage = self.try_list.pop(0)
             self.message(
                 N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
@@ -299,7 +301,7 @@ class CoverArt:
                     'host': coverartimage.host
                 }
             )
-            self.album.tagger.xmlws.download(
+            self._download(
                 coverartimage.host,
                 coverartimage.port,
                 coverartimage.path,

From cdec8f4e929ec838e445147ca76d756f4a987d28 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 16:09:53 +0200
Subject: [PATCH 25/70] No need to calculate length of the list to test if
 empty.

---
 picard/coverart.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index d0dcfa307..44db21da4 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -286,7 +286,7 @@ class CoverArt:
     def _walk(self):
         """Downloads each item in ``try_list``. If there are none left, loading of
         ``album`` will be finalized."""
-        if len(self.try_list) == 0:
+        if not self.try_list:
             self.album._finalize_loading(None)
         elif self.album.id not in self.album.tagger.albums:
             return

From 4ea310c9831e0b1a5a7f5e35f71eed1345459f96 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 19:06:19 +0200
Subject: [PATCH 26/70] _process_asin_relation(): reduce indentation level

---
 picard/coverart.py | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 44db21da4..1c8b4e6f3 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -312,16 +312,17 @@ class CoverArt:
 
     def _process_asin_relation(self, relation):
         amz = parse_amazon_url(relation.target[0].text)
-        if amz is not None:
-            if amz['host'] in AMAZON_SERVER:
-                serverInfo = AMAZON_SERVER[amz['host']]
-            else:
-                serverInfo = AMAZON_SERVER['amazon.com']
-            host = serverInfo['server']
-            for size in ('L', 'M'):
-                path = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], size)
-                url = "http://%s:%s" % (host, path)
-                self._append_image(CoverArtImage(url))
+        if amz is None:
+            return
+        if amz['host'] in AMAZON_SERVER:
+            serverInfo = AMAZON_SERVER[amz['host']]
+        else:
+            serverInfo = AMAZON_SERVER['amazon.com']
+        host = serverInfo['server']
+        for size in ('L', 'M'):
+            path = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], size)
+            url = "http://%s:%s" % (host, path)
+            self._append_image(CoverArtImage(url))
 
     def _append_image(self, coverartimage):
         log.debug("Appending cover art image %r", coverartimage)

From 2df6c9759ef1cd7a3e3dd64e81be89716ec90136 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 19:07:36 +0200
Subject: [PATCH 27/70] _walk(): reduce indentation level

---
 picard/coverart.py | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 1c8b4e6f3..0f525f75a 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -288,27 +288,29 @@ class CoverArt:
         ``album`` will be finalized."""
         if not self.try_list:
             self.album._finalize_loading(None)
-        elif self.album.id not in self.album.tagger.albums:
             return
-        else:
-            # We still have some items to try!
-            coverartimage = self.try_list.pop(0)
-            self.message(
-                N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
-                {
-                    'type': coverartimage.type,
-                    'albumid': self.album.id,
-                    'host': coverartimage.host
-                }
-            )
-            self._download(
-                coverartimage.host,
-                coverartimage.port,
-                coverartimage.path,
-                partial(self._coverart_downloaded, coverartimage),
-                priority=True,
-                important=False
-            )
+
+        if self.album.id not in self.album.tagger.albums:
+            return
+
+        # We still have some items to try!
+        coverartimage = self.try_list.pop(0)
+        self.message(
+            N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
+            {
+                'type': coverartimage.type,
+                'albumid': self.album.id,
+                'host': coverartimage.host
+            }
+        )
+        self._download(
+            coverartimage.host,
+            coverartimage.port,
+            coverartimage.path,
+            partial(self._coverart_downloaded, coverartimage),
+            priority=True,
+            important=False
+        )
 
     def _process_asin_relation(self, relation):
         amz = parse_amazon_url(relation.target[0].text)

From 825abdda5d5e8637dd01305f579d9b31469a772f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 19:13:30 +0200
Subject: [PATCH 28/70] _fill() -> _fill_from_relationships()

---
 picard/coverart.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 0f525f75a..99acad25d 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -154,7 +154,7 @@ class CoverArt:
         else:
             log.debug("There are no suitable images in the cover art archive for %s"
                       % self.release.id)
-            self._fill()
+            self._fill_from_relationships()
             self._walk()
 
     def message(self, *args, **kwargs):
@@ -240,7 +240,7 @@ class CoverArt:
                             break
 
         if error or not caa_front_found:
-            self._fill()
+            self._fill_from_relationships()
         self._walk()
 
     def _append_caa_image(self, image):
@@ -259,7 +259,7 @@ class CoverArt:
         )
         self._append_image(coverartimage)
 
-    def _fill(self):
+    def _fill_from_relationships(self):
         """Fills ``try_list`` by looking at the relationships in ``release``."""
         use_whitelist = config.setting['ca_provider_use_whitelist']
         use_amazon = config.setting['ca_provider_use_amazon']

From 3a4f062694f34ce2951a0acfcd2a942c0c1e6248 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 19:55:39 +0200
Subject: [PATCH 29/70] _coverart_downloaded(): better handling of not enough
 data case

At least log a message in debug mode in this case.
Btw, i'm not sure why this condition was added, even if it makes sense.
---
 picard/coverart.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 99acad25d..2ba3b6c7c 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -171,9 +171,10 @@ class CoverArt:
     def _coverart_downloaded(self, coverartimage, data, http, error):
         self.album._requests -= 1
 
-        if error or len(data) < 1000:
-            if error:
-                self._coverart_http_error(http)
+        if error:
+            self._coverart_http_error(http)
+        elif len(data) < 1000:
+            log.debug("Not enough data, skipping image")
         else:
             self.message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),

From 46649c633274bffaddf8d0521ea19523e66e582a Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 21:06:49 +0200
Subject: [PATCH 30/70] Drop useless coverartobj argument, cleanup

---
 picard/coverart.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 2ba3b6c7c..345d9c605 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -110,6 +110,10 @@ class CoverArt:
         self.caa_types = map(unicode.lower, config.setting["caa_image_types"])
         self.len_caa_types = len(self.caa_types)
 
+    def __repr__(self):
+        return "CoverArt for %r" % (self.album)
+
+    def retrieve(self):
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
         has_caa_artwork = False
@@ -332,12 +336,10 @@ class CoverArt:
         self.try_list.append(coverartimage)
 
 
-def coverart(album, metadata, release, coverartobj=None):
+def coverart(album, metadata, release):
     """ Gets all cover art URLs from the metadata and then attempts to
     download the album art. """
 
-    # try_list will be None for the first call
-    if coverartobj is not None:
-        return
-
-    coverartobj = CoverArt(album, metadata, release)
+    coverart = CoverArt(album, metadata, release)
+    coverart.retrieve()
+    log.debug("New %r", coverart)

From 67c9225767f9932ee326e31b51913979f0d27252 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 21:53:39 +0200
Subject: [PATCH 31/70] Use types intersection to select caa images to
 download, clarify code

- lowercase types from json, to match lowercased types from options
- fix up logic concerning caa_front_found
- add few comments and debug info
---
 picard/coverart.py | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 345d9c605..a0527c666 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -234,17 +234,21 @@ class CoverArt:
                 for image in caa_data["images"]:
                     if config.setting["caa_approved_only"] and not image["approved"]:
                         continue
-                    if not image["types"] and "unknown" in self.caa_types:
-                        image["types"] = [u"Unknown"]
-                    imagetypes = map(unicode.lower, image["types"])
-                    for imagetype in imagetypes:
-                        if imagetype == "front":
-                            caa_front_found = True
-                        if imagetype in self.caa_types:
-                            self._append_caa_image(image)
-                            break
+                    # if image has no type set, we still want it to match
+                    # pseudo type 'unknown'
+                    if not image["types"]:
+                        image["types"] = [u"unknown"]
+                    else:
+                        image["types"] = map(unicode.lower, image["types"])
+                    # only keep enabled caa types
+                    types = set(image["types"]).intersection(set(self.caa_types))
+                    if types:
+                        if not caa_front_found:
+                            caa_front_found = u'front' in types
+                        self._append_caa_image(image)
 
         if error or not caa_front_found:
+            log.debug("Trying to get cover art from release relationships")
             self._fill_from_relationships()
         self._walk()
 
@@ -258,7 +262,7 @@ class CoverArt:
             url = image["thumbnails"][thumbsize]
         coverartimage = CoverArtImage(
             url,
-            type = image["types"][0].lower(),  # FIXME: we pass only 1 type
+            type = image["types"][0],  # FIXME: we pass only 1 type
             desc = image["comment"],
             front = image['front'],  # front image indicator from CAA
         )

From d5b4bfec914a76a4efd978e9359632e52790ee5b Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 22:37:49 +0200
Subject: [PATCH 32/70] Subclass CoverArtImage and add support_types property

- CaaCoverArtImage is only used for CAA images
- drop host comparaison (vs 'archive.org'), use a much cleaner "flag"
- all current providers but CAA don't support multiple types, so we defaults to a "front" type
- once we have one image from one of those providers we are done
- we don't want to remove all images from CAA having front type
---
 picard/coverart.py | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index a0527c666..5725934a9 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -77,6 +77,8 @@ _CAA_THUMBNAIL_SIZE_MAP = {
 
 class CoverArtImage:
 
+    support_types = False
+
     def __init__(self, url, type='front', desc='', front=None):
         self.url = QUrl(url)
         path = str(self.url.encodedPath())
@@ -90,6 +92,9 @@ class CoverArtImage:
         self.front = front
 
     def is_front_image(self):
+        if not self.support_types:
+            # consider all images as front if types aren't supported by provider
+            return True
         # CAA has a flag for "front" image, use it in priority
         if self.front is None:
             # no caa front flag, use type instead
@@ -100,6 +105,11 @@ class CoverArtImage:
         return "type: %r from %s" % (self.type, self.url.toString())
 
 
+class CaaCoverArtImage(CoverArtImage):
+
+    support_types = True
+
+
 class CoverArt:
 
     def __init__(self, album, metadata, release):
@@ -212,11 +222,10 @@ class CoverArt:
                 return
 
         # If the image already was a front image, there might still be some
-        # other front images in the try_list - remove them.
+        # other non-CAA front images in the try_list - remove them.
         if coverartimage.is_front_image():
             for item in self.try_list[:]:
-                if item.is_front_image() and 'archive.org' not in item.host:
-                    # Hosts other than archive.org only provide front images
+                if not item.support_types:
                     self.try_list.remove(item)
         self._walk()
 
@@ -260,7 +269,7 @@ class CoverArt:
             url = image["image"]
         else:
             url = image["thumbnails"][thumbsize]
-        coverartimage = CoverArtImage(
+        coverartimage = CaaCoverArtImage(
             url,
             type = image["types"][0],  # FIXME: we pass only 1 type
             desc = image["comment"],

From 73c57108f6699f3664bf7302ff9c3e124f4d45e8 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 May 2014 23:06:21 +0200
Subject: [PATCH 33/70] Use an explicit is_front property for CAA front flag
 indicator

---
 picard/coverart.py | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 5725934a9..f4768d997 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -78,8 +78,10 @@ _CAA_THUMBNAIL_SIZE_MAP = {
 class CoverArtImage:
 
     support_types = False
+    # consider all images as front if types aren't supported by provider
+    is_front = True
 
-    def __init__(self, url, type='front', desc='', front=None):
+    def __init__(self, url, type='front', desc=''):
         self.url = QUrl(url)
         path = str(self.url.encodedPath())
         if self.url.hasQuery():
@@ -89,17 +91,13 @@ class CoverArtImage:
         self.path = str(path)
         self.type = type
         self.desc = desc
-        self.front = front
 
     def is_front_image(self):
-        if not self.support_types:
-            # consider all images as front if types aren't supported by provider
-            return True
         # CAA has a flag for "front" image, use it in priority
-        if self.front is None:
-            # no caa front flag, use type instead
-            return (self.type == 'front')
-        return self.front
+        if self.is_front:
+            return True
+        # no caa front flag, use type instead
+        return (self.type == 'front')
 
     def __repr__(self):
         return "type: %r from %s" % (self.type, self.url.toString())
@@ -107,6 +105,7 @@ class CoverArtImage:
 
 class CaaCoverArtImage(CoverArtImage):
 
+    is_front = False
     support_types = True
 
 
@@ -273,8 +272,8 @@ class CoverArt:
             url,
             type = image["types"][0],  # FIXME: we pass only 1 type
             desc = image["comment"],
-            front = image['front'],  # front image indicator from CAA
         )
+        coverartimage.is_front = bool(image['front'])  # front image indicator from CAA
         self._append_image(coverartimage)
 
     def _fill_from_relationships(self):

From 5b5ca9a481d06857cf4986789981fa02553c9f21 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Mon, 5 May 2014 17:32:48 +0200
Subject: [PATCH 34/70] Add image description to debug message if available

---
 picard/coverart.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index f4768d997..d2bc014da 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -100,7 +100,10 @@ class CoverArtImage:
         return (self.type == 'front')
 
     def __repr__(self):
-        return "type: %r from %s" % (self.type, self.url.toString())
+        if self.desc:
+            return "types: %r (%r) from %s" % (self.types, self.desc, self.url.toString())
+        else:
+            return "types: %r from %s" % (self.types, self.url.toString())
 
 
 class CaaCoverArtImage(CoverArtImage):

From 7a1607e8c352ab928b64c5ce7c302e2d6a7ad5b4 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Mon, 5 May 2014 17:33:28 +0200
Subject: [PATCH 35/70] Remove useless import

---
 picard/coverart.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index d2bc014da..6091a85cd 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -26,7 +26,6 @@ import re
 import traceback
 from functools import partial
 from picard import config, log
-from picard.metadata import Image
 from picard.util import mimetype, parse_amazon_url
 from picard.const import CAA_HOST, CAA_PORT
 from PyQt4.QtCore import QUrl, QObject

From 7de9477b7b58cd8c30ea032cb346c0f0010dda4c Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Mon, 5 May 2014 17:35:09 +0200
Subject: [PATCH 36/70] Add types_and_front() helper function

- it converts id3 type to CAA-like type list
- it returns a is_front boolean, emulating CAA flag
---
 picard/formats/id3.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/picard/formats/id3.py b/picard/formats/id3.py
index 5afb0738f..94c05f2f7 100644
--- a/picard/formats/id3.py
+++ b/picard/formats/id3.py
@@ -92,6 +92,12 @@ def image_type_as_id3_num(texttype):
     return __ID3_IMAGE_TYPE_MAP.get(texttype, 0)
 
 
+def types_and_front(id3type):
+    imgtype = image_type_from_id3_num(id3type)
+    is_front = imgtype == 'front'
+    return [unicode(imgtype)], is_front
+
+
 class ID3File(File):
 
     """Generic ID3-based file."""

From b79149f84307076807c8c27d098b779699a581fa Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Mon, 5 May 2014 17:43:40 +0200
Subject: [PATCH 37/70] Improve handling of multiple image types and is_front
 flag

- convert type-as-a-string to list of types
- display concatenated list of types in messages (ie. back,spine)
- pass is_front flag everywhere needed
- pass list of types instead of single string everywhere needed
- introduce maintype() method to get the first type if more than one is set
- in formats/*, use types_and_front() at read time to get properly formatted list of types (only one element for now, since no format seems to support multiple types)
- use Image.maintype() at write time (since no format supports multiple types for now)
---
 picard/coverart.py       | 22 ++++++++++++----------
 picard/formats/asf.py    |  7 ++++---
 picard/formats/id3.py    |  5 +++--
 picard/formats/vorbis.py | 11 +++++++----
 picard/metadata.py       | 27 ++++++++++++++++-----------
 picard/ui/coverartbox.py |  2 +-
 6 files changed, 43 insertions(+), 31 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 6091a85cd..2f99d06a8 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -80,7 +80,7 @@ class CoverArtImage:
     # consider all images as front if types aren't supported by provider
     is_front = True
 
-    def __init__(self, url, type='front', desc=''):
+    def __init__(self, url, types=[u'front'], desc=''):
         self.url = QUrl(url)
         path = str(self.url.encodedPath())
         if self.url.hasQuery():
@@ -88,7 +88,7 @@ class CoverArtImage:
         self.host = str(self.url.host())
         self.port = self.url.port(80)
         self.path = str(path)
-        self.type = type
+        self.types = types
         self.desc = desc
 
     def is_front_image(self):
@@ -96,7 +96,7 @@ class CoverArtImage:
         if self.is_front:
             return True
         # no caa front flag, use type instead
-        return (self.type == 'front')
+        return u'front' in self.types
 
     def __repr__(self):
         if self.desc:
@@ -194,7 +194,7 @@ class CoverArt:
             self.message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
                 {
-                    'type': coverartimage.type,
+                    'type': ','.join(coverartimage.types),
                     'albumid': self.album.id,
                     'host': coverartimage.host
                 }
@@ -205,15 +205,17 @@ class CoverArt:
                 self.metadata.make_and_add_image(
                     mime,
                     data,
-                    imagetype=coverartimage.type,
-                    comment=coverartimage.desc
+                    types=coverartimage.types,
+                    comment=coverartimage.desc,
+                    is_front=coverartimage.is_front
                 )
                 for track in self.album._new_tracks:
                     track.metadata.make_and_add_image(
                         mime,
                         data,
-                        imagetype=coverartimage.type,
-                        comment=coverartimage.desc
+                        types=coverartimage.types,
+                        comment=coverartimage.desc,
+                        is_front=coverartimage.is_front
                     )
             except (IOError, OSError) as e:
                 self.album.error_append(e.message)
@@ -272,7 +274,7 @@ class CoverArt:
             url = image["thumbnails"][thumbsize]
         coverartimage = CaaCoverArtImage(
             url,
-            type = image["types"][0],  # FIXME: we pass only 1 type
+            types = image["types"],
             desc = image["comment"],
         )
         coverartimage.is_front = bool(image['front'])  # front image indicator from CAA
@@ -317,7 +319,7 @@ class CoverArt:
         self.message(
             N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
             {
-                'type': coverartimage.type,
+                'type': ','.join(coverartimage.types),
                 'albumid': self.album.id,
                 'host': coverartimage.host
             }
diff --git a/picard/formats/asf.py b/picard/formats/asf.py
index 6b0eed69d..75e95d627 100644
--- a/picard/formats/asf.py
+++ b/picard/formats/asf.py
@@ -19,7 +19,7 @@
 
 from picard import config, log
 from picard.file import File
-from picard.formats.id3 import image_type_from_id3_num, image_type_as_id3_num
+from picard.formats.id3 import types_and_front, image_type_as_id3_num
 from picard.util import encode_filename
 from picard.metadata import Metadata, save_this_image_to_tags
 from mutagen.asf import ASF, ASFByteArrayAttribute
@@ -141,8 +141,9 @@ class ASFFile(File):
             if name == 'WM/Picture':
                 for image in values:
                     (mime, data, type, description) = unpack_image(image.value)
+                    types, is_front = types_and_front(type)
                     metadata.make_and_add_image(mime, data, comment=description,
-                                                imagetype=image_type_from_id3_num(type))
+                                                types=types, is_front=is_front)
                 continue
             elif name not in self.__RTRANS:
                 continue
@@ -168,7 +169,7 @@ class ASFFile(File):
                 if not save_this_image_to_tags(image):
                     continue
                 tag_data = pack_image(image.mimetype, image.data,
-                                      image_type_as_id3_num(image.imagetype),
+                                      image_type_as_id3_num(image.maintype()),
                                       image.description)
                 cover.append(ASFByteArrayAttribute(tag_data))
             if cover:
diff --git a/picard/formats/id3.py b/picard/formats/id3.py
index 94c05f2f7..7aacfc89f 100644
--- a/picard/formats/id3.py
+++ b/picard/formats/id3.py
@@ -261,8 +261,9 @@ class ID3File(File):
                 else:
                     log.error("Invalid %s value '%s' dropped in %r", frameid, frame.text[0], filename)
             elif frameid == 'APIC':
+                types, is_front = types_and_front(frame.type)
                 metadata.make_and_add_image(frame.mime, frame.data, comment=frame.desc,
-                                            imagetype=image_type_from_id3_num(frame.type))
+                                            types=types, is_front=is_front)
             elif frameid == 'POPM':
                 # Rating in ID3 ranges from 0 to 255, normalize this to the range 0 to 5
                 if frame.email == config.setting['rating_user_email']:
@@ -328,7 +329,7 @@ class ID3File(File):
                 counters[desc] += 1
                 tags.add(id3.APIC(encoding=0,
                                   mime=image.mimetype,
-                                  type=image_type_as_id3_num(image.imagetype),
+                                  type=image_type_as_id3_num(image.maintype()),
                                   desc=desctag,
                                   data=image.data))
 
diff --git a/picard/formats/vorbis.py b/picard/formats/vorbis.py
index bf9161988..1e42f9bca 100644
--- a/picard/formats/vorbis.py
+++ b/picard/formats/vorbis.py
@@ -32,7 +32,7 @@ except ImportError:
     with_opus = False
 from picard import config, log
 from picard.file import File
-from picard.formats.id3 import image_type_from_id3_num, image_type_as_id3_num
+from picard.formats.id3 import types_and_front, image_type_as_id3_num
 from picard.metadata import Metadata, save_this_image_to_tags
 from picard.util import encode_filename, sanitize_date
 
@@ -96,17 +96,20 @@ class VCommentFile(File):
                     name = "totaldiscs"
                 elif name == "metadata_block_picture":
                     image = mutagen.flac.Picture(base64.standard_b64decode(value))
+                    types, is_front = types_and_front(image.type)
                     metadata.make_and_add_image(image.mime, image.data,
                                                 comment=image.desc,
-                                                imagetype=image_type_from_id3_num(image.type))
+                                                types=types,
+                                                is_front=is_front)
                     continue
                 elif name in self.__translate:
                     name = self.__translate[name]
                 metadata.add(name, value)
         if self._File == mutagen.flac.FLAC:
             for image in file.pictures:
+                types, is_front = types_and_front(image.type)
                 metadata.make_and_add_image(image.mime, image.data, comment=image.desc,
-                                            imagetype=image_type_from_id3_num(image.type))
+                                            types=types, is_front=is_front)
         # Read the unofficial COVERART tags, for backward compatibillity only
         if not "metadata_block_picture" in file.tags:
             try:
@@ -173,7 +176,7 @@ class VCommentFile(File):
                 picture.data = image.data
                 picture.mime = image.mimetype
                 picture.desc = image.description
-                picture.type = image_type_as_id3_num(image.imagetype)
+                picture.type = image_type_as_id3_num(image.maintype())
                 if self._File == mutagen.flac.FLAC:
                     file.add_picture(picture)
                 else:
diff --git a/picard/metadata.py b/picard/metadata.py
index 78ed1d138..4b336f244 100644
--- a/picard/metadata.py
+++ b/picard/metadata.py
@@ -48,7 +48,7 @@ MULTI_VALUED_JOINER = '; '
 def save_this_image_to_tags(image):
     if not config.setting["save_only_front_images_to_tags"]:
         return True
-    return image.is_front_image
+    return image.is_front
 
 
 class Image(object):
@@ -57,8 +57,8 @@ class Image(object):
     an IOError or OSError due to the usage of tempfiles underneath.
     """
 
-    def __init__(self, data, mimetype="image/jpeg", imagetype="front",
-                 comment="", filename=None, datahash=""):
+    def __init__(self, data, mimetype="image/jpeg", types=[u"front"],
+                 comment="", filename=None, datahash="", is_front=True):
         self.description = comment
         (fd, self._tempfile_filename) = tempfile.mkstemp(prefix="picard")
         with fdopen(fd, "wb") as imagefile:
@@ -68,10 +68,13 @@ class Image(object):
         self.datalength = len(data)
         self.extension = mime.get_extension(mime, ".jpg")
         self.filename = filename
-        self.imagetype = imagetype
-        self.is_front_image = imagetype == "front"
+        self.types = types
+        self.is_front = is_front
         self.mimetype = mimetype
 
+    def maintype(self):
+        return self.types[0]
+
     def _make_image_filename(self, filename, dirname, metadata):
         if config.setting["ascii_filenames"]:
             if isinstance(filename, unicode):
@@ -100,8 +103,8 @@ class Image(object):
             log.debug("Using the custom file name %s", self.filename)
             filename = self.filename
         elif config.setting["caa_image_type_as_filename"]:
-            log.debug("Using image type %s", self.imagetype)
-            filename = self.imagetype
+            filename = self.maintype()
+            log.debug("Make filename from types: %r -> %r", self.types, filename)
         else:
             log.debug("Using default file name %s",
                       config.setting["cover_image_filename"])
@@ -170,7 +173,7 @@ class Metadata(dict):
         self.length = 0
 
     def make_and_add_image(self, mime, data, filename=None, comment="",
-                           imagetype="front"):
+                           types=[u"front"], is_front=True):
         """Build a new image object from ``data`` and adds it to this Metadata
         object. If an image with the same MD5 hash has already been added to
         any Metadata object, that file will be reused.
@@ -180,7 +183,8 @@ class Metadata(dict):
         data -- The image data
         filename -- The image filename, without an extension
         comment -- image description or comment, default to ''
-        imagetype -- main type as a string, default to 'front'
+        types -- list of types, default to [u'front']
+        is_front -- mark image as front image
         """
         m = md5()
         m.update(data)
@@ -188,8 +192,9 @@ class Metadata(dict):
         QObject.tagger.images.lock()
         image = QObject.tagger.images[datahash]
         if image is None:
-            image = Image(data, mime, imagetype, comment, filename,
-                          datahash=datahash)
+            image = Image(data, mime, types, comment, filename,
+                          datahash=datahash,
+                          is_front=is_front)
             QObject.tagger.images[datahash] = image
         QObject.tagger.images.unlock()
         self.images.append(image)
diff --git a/picard/ui/coverartbox.py b/picard/ui/coverartbox.py
index 9a2122f0c..f20aba56d 100644
--- a/picard/ui/coverartbox.py
+++ b/picard/ui/coverartbox.py
@@ -122,7 +122,7 @@ class CoverArtBox(QtGui.QGroupBox):
         data = None
         if metadata and metadata.images:
             for image in metadata.images:
-                if image.is_front_image:
+                if image.is_front:
                     data = image
                     break
             else:

From af405b1baa080bb94080babbb6bdb4fc1997e06d Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Mon, 5 May 2014 22:44:56 +0200
Subject: [PATCH 38/70] Improve debug messages

---
 picard/coverart.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 2f99d06a8..3cbebfaa0 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -167,8 +167,9 @@ class CoverArt:
                 important=False
             )
         else:
-            log.debug("There are no suitable images in the cover art archive for %s"
-                      % self.release.id)
+            if config.setting['ca_provider_use_caa']:
+                log.debug("There are no suitable images in the cover art archive for %s"
+                          % self.release.id)
             self._fill_from_relationships()
             self._walk()
 
@@ -260,7 +261,6 @@ class CoverArt:
                         self._append_caa_image(image)
 
         if error or not caa_front_found:
-            log.debug("Trying to get cover art from release relationships")
             self._fill_from_relationships()
         self._walk()
 
@@ -286,6 +286,7 @@ class CoverArt:
         use_amazon = config.setting['ca_provider_use_amazon']
         if not (use_whitelist or use_amazon):
             return
+        log.debug("Trying to get cover art from release relationships ...")
         try:
             if 'relation_list' in self.release.children:
                 for relation_list in self.release.relation_list:
@@ -295,6 +296,7 @@ class CoverArt:
                             if use_whitelist \
                                 and (relation.type == 'cover art link' or
                                      relation.type == 'has_cover_art_at'):
+                                log.debug("Found cover art link in whitelist")
                                 url = relation.target[0].text
                                 self._append_image(CoverArtImage(url))
                             elif use_amazon \
@@ -337,6 +339,7 @@ class CoverArt:
         amz = parse_amazon_url(relation.target[0].text)
         if amz is None:
             return
+        log.debug("Found ASIN relation : %s %s", amz['host'], amz['asin'])
         if amz['host'] in AMAZON_SERVER:
             serverInfo = AMAZON_SERVER[amz['host']]
         else:

From f2a0c1b2cdc43eb91141a0f2b1f6bcd76096b7bd Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Tue, 6 May 2014 16:44:18 +0200
Subject: [PATCH 39/70] Modify how we skip unneeded front images from non-CAA
 sources

Instead of looping and modify the try list, just check in _walk()
and continue.
Improve debugging messages (no more silent removal in debug mode).
---
 picard/coverart.py | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 3cbebfaa0..b1e53ea57 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -120,6 +120,7 @@ class CoverArt:
         self.release = release
         self.caa_types = map(unicode.lower, config.setting["caa_image_types"])
         self.len_caa_types = len(self.caa_types)
+        self.at_least_one_front_image = False
 
     def __repr__(self):
         return "CoverArt for %r" % (self.album)
@@ -226,11 +227,9 @@ class CoverArt:
                 return
 
         # If the image already was a front image, there might still be some
-        # other non-CAA front images in the try_list - remove them.
-        if coverartimage.is_front_image():
-            for item in self.try_list[:]:
-                if not item.support_types:
-                    self.try_list.remove(item)
+        # other non-CAA front images in the queue - ignore them.
+        if not self.at_least_one_front_image:
+            self.at_least_one_front_image = coverartimage.is_front_image()
         self._walk()
 
     def _caa_json_downloaded(self, data, http, error):
@@ -318,6 +317,14 @@ class CoverArt:
 
         # We still have some items to try!
         coverartimage = self.try_list.pop(0)
+        if not coverartimage.support_types and self.at_least_one_front_image:
+            # we already have one front image, no need to try other type-less
+            # sources
+            log.debug("Skipping cover art %r, one front image is already available",
+                      coverartimage)
+            self._walk()
+            return
+
         self.message(
             N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
             {

From 6d68b9f00b99c24c123c73427e6f69be6cde13de Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Tue, 6 May 2014 17:03:56 +0200
Subject: [PATCH 40/70] try_list -> queue + fix up few comments

---
 picard/coverart.py | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index b1e53ea57..4f9a1cb49 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -114,7 +114,7 @@ class CaaCoverArtImage(CoverArtImage):
 class CoverArt:
 
     def __init__(self, album, metadata, release):
-        self.try_list = []
+        self.queue = []
         self.album = album
         self.metadata = metadata
         self.release = release
@@ -264,7 +264,7 @@ class CoverArt:
         self._walk()
 
     def _append_caa_image(self, image):
-        """Adds URLs to `try_list` depending on the users CAA image size settings."""
+        """Queue images depending on the CAA image size settings."""
         imagesize = config.setting["caa_image_size"]
         thumbsize = _CAA_THUMBNAIL_SIZE_MAP.get(imagesize, None)
         if thumbsize is None:
@@ -280,7 +280,8 @@ class CoverArt:
         self._append_image(coverartimage)
 
     def _fill_from_relationships(self):
-        """Fills ``try_list`` by looking at the relationships in ``release``."""
+        """Queue images by looking at the release's relationships.
+        """
         use_whitelist = config.setting['ca_provider_use_whitelist']
         use_amazon = config.setting['ca_provider_use_amazon']
         if not (use_whitelist or use_amazon):
@@ -306,9 +307,10 @@ class CoverArt:
             self.album.error_append(traceback.format_exc())
 
     def _walk(self):
-        """Downloads each item in ``try_list``. If there are none left, loading of
-        ``album`` will be finalized."""
-        if not self.try_list:
+        """Downloads each item in queue.
+           If there are none left, loading of album will be finalized.
+        """
+        if not self.queue:
             self.album._finalize_loading(None)
             return
 
@@ -316,7 +318,7 @@ class CoverArt:
             return
 
         # We still have some items to try!
-        coverartimage = self.try_list.pop(0)
+        coverartimage = self.queue.pop(0)
         if not coverartimage.support_types and self.at_least_one_front_image:
             # we already have one front image, no need to try other type-less
             # sources
@@ -358,8 +360,8 @@ class CoverArt:
             self._append_image(CoverArtImage(url))
 
     def _append_image(self, coverartimage):
-        log.debug("Appending cover art image %r", coverartimage)
-        self.try_list.append(coverartimage)
+        log.debug("Queing image %r for download", coverartimage)
+        self.queue.append(coverartimage)
 
 
 def coverart(album, metadata, release):

From 8677a4aa3db20e72248f1e727fa11232095fe17f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Tue, 6 May 2014 17:18:53 +0200
Subject: [PATCH 41/70] Hide underlying queue/list using a bunch of new methods

- _append_image() -> _queue_put()
- new _queue_get()
- new _queue_empty()
- new _queue_new()
---
 picard/coverart.py | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 4f9a1cb49..fa175f88f 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -114,7 +114,7 @@ class CaaCoverArtImage(CoverArtImage):
 class CoverArt:
 
     def __init__(self, album, metadata, release):
-        self.queue = []
+        self._queue_new()
         self.album = album
         self.metadata = metadata
         self.release = release
@@ -277,7 +277,7 @@ class CoverArt:
             desc = image["comment"],
         )
         coverartimage.is_front = bool(image['front'])  # front image indicator from CAA
-        self._append_image(coverartimage)
+        self._queue_put(coverartimage)
 
     def _fill_from_relationships(self):
         """Queue images by looking at the release's relationships.
@@ -298,7 +298,7 @@ class CoverArt:
                                      relation.type == 'has_cover_art_at'):
                                 log.debug("Found cover art link in whitelist")
                                 url = relation.target[0].text
-                                self._append_image(CoverArtImage(url))
+                                self._queue_put(CoverArtImage(url))
                             elif use_amazon \
                                 and (relation.type == 'amazon asin' or
                                      relation.type == 'has_Amazon_ASIN'):
@@ -310,7 +310,7 @@ class CoverArt:
         """Downloads each item in queue.
            If there are none left, loading of album will be finalized.
         """
-        if not self.queue:
+        if self._queue_empty():
             self.album._finalize_loading(None)
             return
 
@@ -318,7 +318,7 @@ class CoverArt:
             return
 
         # We still have some items to try!
-        coverartimage = self.queue.pop(0)
+        coverartimage = self._queue_get()
         if not coverartimage.support_types and self.at_least_one_front_image:
             # we already have one front image, no need to try other type-less
             # sources
@@ -357,11 +357,24 @@ class CoverArt:
         for size in ('L', 'M'):
             path = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], size)
             url = "http://%s:%s" % (host, path)
-            self._append_image(CoverArtImage(url))
+            self._queue_put(CoverArtImage(url))
 
-    def _append_image(self, coverartimage):
+    def _queue_put(self, coverartimage):
+        "Add an image to queue"
         log.debug("Queing image %r for download", coverartimage)
-        self.queue.append(coverartimage)
+        self.__queue.append(coverartimage)
+
+    def _queue_get(self):
+        "Get next image and remove it from queue"
+        return self.__queue.pop(0)
+
+    def _queue_empty(self):
+        "Returns True if the queue is empty"
+        return not self.__queue
+
+    def _queue_new(self):
+        "Initialize the queue"
+        self.__queue = []
 
 
 def coverart(album, metadata, release):

From 80ed1d7265032e2bf721183cac2eb5b85dd3c202 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Wed, 7 May 2014 16:38:33 +0200
Subject: [PATCH 42/70] Do not hide errors that are actually errors

Use Album.error_append() instead of log.debug()
---
 picard/coverart.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index fa175f88f..6c10eae59 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -191,7 +191,7 @@ class CoverArt:
         if error:
             self._coverart_http_error(http)
         elif len(data) < 1000:
-            log.debug("Not enough data, skipping image")
+            self.album.error_append("Not enough data, skipping image %r" % coverartimage)
         else:
             self.message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
@@ -241,7 +241,7 @@ class CoverArt:
             try:
                 caa_data = json.loads(data)
             except ValueError:
-                log.debug("Invalid JSON: %s", http.url().toString())
+                self.album.error_append("Invalid JSON: %s", http.url().toString())
             else:
                 for image in caa_data["images"]:
                     if config.setting["caa_approved_only"] and not image["approved"]:

From a9199b4911e9d4568c8b660ffd19d1f37a66034f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Wed, 7 May 2014 16:43:17 +0200
Subject: [PATCH 43/70] _download() -> _xmlws_download()

This is a wrapper to the actual call to album.tagger.xmlws.download()
---
 picard/coverart.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 6c10eae59..373119391 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -159,7 +159,7 @@ class CoverArt:
                 and self.len_caa_types > 0:
             log.debug("There are suitable images in the cover art archive for %s"
                       % self.release.id)
-            self._download(
+            self._xmlws_download(
                 CAA_HOST,
                 CAA_PORT,
                 "/release/%s/" % self.metadata["musicbrainz_albumid"],
@@ -177,7 +177,7 @@ class CoverArt:
     def message(self, *args, **kwargs):
         QObject.tagger.window.set_statusbar_message(*args, **kwargs)
 
-    def _download(self, *args, **kwargs):
+    def _xmlws_download(self, *args, **kwargs):
         self.album._requests += 1
         self.album.tagger.xmlws.download(*args, **kwargs)
 
@@ -335,7 +335,7 @@ class CoverArt:
                 'host': coverartimage.host
             }
         )
-        self._download(
+        self._xmlws_download(
             coverartimage.host,
             coverartimage.port,
             coverartimage.path,

From 50db339a84e7731d80b14fda7f04433b06c3ff18 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Wed, 7 May 2014 16:54:30 +0200
Subject: [PATCH 44/70] _walk() -> _download_next_in_queue()

Better match what it does.
---
 picard/coverart.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 373119391..136e22d65 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -172,7 +172,7 @@ class CoverArt:
                 log.debug("There are no suitable images in the cover art archive for %s"
                           % self.release.id)
             self._fill_from_relationships()
-            self._walk()
+            self._download_next_in_queue()
 
     def message(self, *args, **kwargs):
         QObject.tagger.window.set_statusbar_message(*args, **kwargs)
@@ -230,7 +230,7 @@ class CoverArt:
         # other non-CAA front images in the queue - ignore them.
         if not self.at_least_one_front_image:
             self.at_least_one_front_image = coverartimage.is_front_image()
-        self._walk()
+        self._download_next_in_queue()
 
     def _caa_json_downloaded(self, data, http, error):
         self.album._requests -= 1
@@ -261,7 +261,7 @@ class CoverArt:
 
         if error or not caa_front_found:
             self._fill_from_relationships()
-        self._walk()
+        self._download_next_in_queue()
 
     def _append_caa_image(self, image):
         """Queue images depending on the CAA image size settings."""
@@ -306,7 +306,7 @@ class CoverArt:
         except AttributeError:
             self.album.error_append(traceback.format_exc())
 
-    def _walk(self):
+    def _download_next_in_queue(self):
         """Downloads each item in queue.
            If there are none left, loading of album will be finalized.
         """
@@ -324,7 +324,7 @@ class CoverArt:
             # sources
             log.debug("Skipping cover art %r, one front image is already available",
                       coverartimage)
-            self._walk()
+            self._download_next_in_queue()
             return
 
         self.message(

From e881e090037c93d51196dd23e1a111cca6de2bf5 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Wed, 7 May 2014 16:57:04 +0200
Subject: [PATCH 45/70] _fill_from_relationships() ->
 _queue_from_relationships()

---
 picard/coverart.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 136e22d65..890d99aee 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -171,7 +171,7 @@ class CoverArt:
             if config.setting['ca_provider_use_caa']:
                 log.debug("There are no suitable images in the cover art archive for %s"
                           % self.release.id)
-            self._fill_from_relationships()
+            self._queue_from_relationships()
             self._download_next_in_queue()
 
     def message(self, *args, **kwargs):
@@ -260,7 +260,7 @@ class CoverArt:
                         self._append_caa_image(image)
 
         if error or not caa_front_found:
-            self._fill_from_relationships()
+            self._queue_from_relationships()
         self._download_next_in_queue()
 
     def _append_caa_image(self, image):
@@ -279,7 +279,7 @@ class CoverArt:
         coverartimage.is_front = bool(image['front'])  # front image indicator from CAA
         self._queue_put(coverartimage)
 
-    def _fill_from_relationships(self):
+    def _queue_from_relationships(self):
         """Queue images by looking at the release's relationships.
         """
         use_whitelist = config.setting['ca_provider_use_whitelist']

From 1dddba902932a2b05a9305a2777abd82899367f8 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Wed, 7 May 2014 16:59:17 +0200
Subject: [PATCH 46/70] _append_caa_image() -> _queue_image_from_caa()

---
 picard/coverart.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 890d99aee..0f15fd669 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -257,13 +257,13 @@ class CoverArt:
                     if types:
                         if not caa_front_found:
                             caa_front_found = u'front' in types
-                        self._append_caa_image(image)
+                        self._queue_image_from_caa(image)
 
         if error or not caa_front_found:
             self._queue_from_relationships()
         self._download_next_in_queue()
 
-    def _append_caa_image(self, image):
+    def _queue_image_from_caa(self, image):
         """Queue images depending on the CAA image size settings."""
         imagesize = config.setting["caa_image_size"]
         thumbsize = _CAA_THUMBNAIL_SIZE_MAP.get(imagesize, None)

From d1e08001d5c1de975682b272a2080c34f4409b27 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Wed, 7 May 2014 17:15:26 +0200
Subject: [PATCH 47/70] Move part of code from retrieve() to new
 _has_caa_artwork()

---
 picard/coverart.py | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 0f15fd669..7d9ef1400 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -126,6 +126,28 @@ class CoverArt:
         return "CoverArt for %r" % (self.album)
 
     def retrieve(self):
+
+        if (config.setting['ca_provider_use_caa']
+            and self.len_caa_types > 0
+            and self._has_caa_artwork()):
+            log.debug("There are suitable images in the cover art archive for %s"
+                      % self.release.id)
+            self._xmlws_download(
+                CAA_HOST,
+                CAA_PORT,
+                "/release/%s/" % self.metadata["musicbrainz_albumid"],
+                self._caa_json_downloaded,
+                priority=True,
+                important=False
+            )
+        else:
+            if config.setting['ca_provider_use_caa']:
+                log.debug("There are no suitable images in the cover art archive for %s"
+                          % self.release.id)
+            self._queue_from_relationships()
+            self._download_next_in_queue()
+
+    def _has_caa_artwork(self):
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
         has_caa_artwork = False
@@ -155,24 +177,7 @@ class CoverArt:
                 back_in_caa = caa_node.back[0].text == 'true' and has_back
                 has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
 
-        if config.setting['ca_provider_use_caa'] and has_caa_artwork\
-                and self.len_caa_types > 0:
-            log.debug("There are suitable images in the cover art archive for %s"
-                      % self.release.id)
-            self._xmlws_download(
-                CAA_HOST,
-                CAA_PORT,
-                "/release/%s/" % self.metadata["musicbrainz_albumid"],
-                self._caa_json_downloaded,
-                priority=True,
-                important=False
-            )
-        else:
-            if config.setting['ca_provider_use_caa']:
-                log.debug("There are no suitable images in the cover art archive for %s"
-                          % self.release.id)
-            self._queue_from_relationships()
-            self._download_next_in_queue()
+        return has_caa_artwork
 
     def message(self, *args, **kwargs):
         QObject.tagger.window.set_statusbar_message(*args, **kwargs)

From 14aef163b6a419c3726279e498792f4178d9a362 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Wed, 7 May 2014 18:02:12 +0200
Subject: [PATCH 48/70] Add methods descriptions + minor tidy up

---
 picard/coverart.py | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 7d9ef1400..8a7816870 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -74,6 +74,7 @@ _CAA_THUMBNAIL_SIZE_MAP = {
     1: "large",
 }
 
+
 class CoverArtImage:
 
     support_types = False
@@ -126,10 +127,11 @@ class CoverArt:
         return "CoverArt for %r" % (self.album)
 
     def retrieve(self):
+        """Retrieve available cover art images for the release"""
 
         if (config.setting['ca_provider_use_caa']
-            and self.len_caa_types > 0
-            and self._has_caa_artwork()):
+                and self.len_caa_types > 0
+                and self._has_caa_artwork()):
             log.debug("There are suitable images in the cover art archive for %s"
                       % self.release.id)
             self._xmlws_download(
@@ -148,6 +150,7 @@ class CoverArt:
             self._download_next_in_queue()
 
     def _has_caa_artwork(self):
+        """Check if CAA artwork has to be downloaded"""
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
         has_caa_artwork = False
@@ -180,17 +183,21 @@ class CoverArt:
         return has_caa_artwork
 
     def message(self, *args, **kwargs):
+        """Display message to status bar"""
         QObject.tagger.window.set_statusbar_message(*args, **kwargs)
 
     def _xmlws_download(self, *args, **kwargs):
+        """xmlws.download wrapper"""
         self.album._requests += 1
         self.album.tagger.xmlws.download(*args, **kwargs)
 
     def _coverart_http_error(self, http):
+        """Append http error to album errors"""
         self.album.error_append(u'Coverart error: %s' %
                                 (unicode(http.errorString())))
 
     def _coverart_downloaded(self, coverartimage, data, http, error):
+        """Handle finished download, save it to metadata"""
         self.album._requests -= 1
 
         if error:
@@ -238,6 +245,7 @@ class CoverArt:
         self._download_next_in_queue()
 
     def _caa_json_downloaded(self, data, http, error):
+        """Parse CAA JSON file and queue CAA cover art images for download"""
         self.album._requests -= 1
         caa_front_found = False
         if error:
@@ -278,8 +286,8 @@ class CoverArt:
             url = image["thumbnails"][thumbsize]
         coverartimage = CaaCoverArtImage(
             url,
-            types = image["types"],
-            desc = image["comment"],
+            types=image["types"],
+            desc=image["comment"],
         )
         coverartimage.is_front = bool(image['front'])  # front image indicator from CAA
         self._queue_put(coverartimage)
@@ -350,6 +358,7 @@ class CoverArt:
         )
 
     def _process_asin_relation(self, relation):
+        """Queue cover art images from Amazon"""
         amz = parse_amazon_url(relation.target[0].text)
         if amz is None:
             return

From adc014eae1230d415d0ca4c1415ecf6bf362962e Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 07:53:08 +0200
Subject: [PATCH 49/70] Move _process_asin_relation() near its caller

---
 picard/coverart.py | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 8a7816870..c55b5488f 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -319,6 +319,22 @@ class CoverArt:
         except AttributeError:
             self.album.error_append(traceback.format_exc())
 
+    def _process_asin_relation(self, relation):
+        """Queue cover art images from Amazon"""
+        amz = parse_amazon_url(relation.target[0].text)
+        if amz is None:
+            return
+        log.debug("Found ASIN relation : %s %s", amz['host'], amz['asin'])
+        if amz['host'] in AMAZON_SERVER:
+            serverInfo = AMAZON_SERVER[amz['host']]
+        else:
+            serverInfo = AMAZON_SERVER['amazon.com']
+        host = serverInfo['server']
+        for size in ('L', 'M'):
+            path = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], size)
+            url = "http://%s:%s" % (host, path)
+            self._queue_put(CoverArtImage(url))
+
     def _download_next_in_queue(self):
         """Downloads each item in queue.
            If there are none left, loading of album will be finalized.
@@ -357,22 +373,6 @@ class CoverArt:
             important=False
         )
 
-    def _process_asin_relation(self, relation):
-        """Queue cover art images from Amazon"""
-        amz = parse_amazon_url(relation.target[0].text)
-        if amz is None:
-            return
-        log.debug("Found ASIN relation : %s %s", amz['host'], amz['asin'])
-        if amz['host'] in AMAZON_SERVER:
-            serverInfo = AMAZON_SERVER[amz['host']]
-        else:
-            serverInfo = AMAZON_SERVER['amazon.com']
-        host = serverInfo['server']
-        for size in ('L', 'M'):
-            path = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], size)
-            url = "http://%s:%s" % (host, path)
-            self._queue_put(CoverArtImage(url))
-
     def _queue_put(self, coverartimage):
         "Add an image to queue"
         log.debug("Queing image %r for download", coverartimage)

From 91f1baf8dd736a471459828c44df5b74204cf957 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 07:55:05 +0200
Subject: [PATCH 50/70] Move message() and _xmlws_download()

---
 picard/coverart.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index c55b5488f..90fbe6331 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -182,15 +182,6 @@ class CoverArt:
 
         return has_caa_artwork
 
-    def message(self, *args, **kwargs):
-        """Display message to status bar"""
-        QObject.tagger.window.set_statusbar_message(*args, **kwargs)
-
-    def _xmlws_download(self, *args, **kwargs):
-        """xmlws.download wrapper"""
-        self.album._requests += 1
-        self.album.tagger.xmlws.download(*args, **kwargs)
-
     def _coverart_http_error(self, http):
         """Append http error to album errors"""
         self.album.error_append(u'Coverart error: %s' %
@@ -390,6 +381,15 @@ class CoverArt:
         "Initialize the queue"
         self.__queue = []
 
+    def message(self, *args, **kwargs):
+        """Display message to status bar"""
+        QObject.tagger.window.set_statusbar_message(*args, **kwargs)
+
+    def _xmlws_download(self, *args, **kwargs):
+        """xmlws.download wrapper"""
+        self.album._requests += 1
+        self.album.tagger.xmlws.download(*args, **kwargs)
+
 
 def coverart(album, metadata, release):
     """ Gets all cover art URLs from the metadata and then attempts to

From 1fe8c708a43c9f43caf73f1bc26cea3dc81b75bd Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 07:56:46 +0200
Subject: [PATCH 51/70] message() -> _message()

---
 picard/coverart.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 90fbe6331..c89ef3a0d 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -196,7 +196,7 @@ class CoverArt:
         elif len(data) < 1000:
             self.album.error_append("Not enough data, skipping image %r" % coverartimage)
         else:
-            self.message(
+            self._message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
                 {
                     'type': ','.join(coverartimage.types),
@@ -347,7 +347,7 @@ class CoverArt:
             self._download_next_in_queue()
             return
 
-        self.message(
+        self._message(
             N_("Downloading cover art of type '%(type)s' for %(albumid)s from %(host)s ..."),
             {
                 'type': ','.join(coverartimage.types),
@@ -381,7 +381,7 @@ class CoverArt:
         "Initialize the queue"
         self.__queue = []
 
-    def message(self, *args, **kwargs):
+    def _message(self, *args, **kwargs):
         """Display message to status bar"""
         QObject.tagger.window.set_statusbar_message(*args, **kwargs)
 

From 2b94c71348dadab1a9434093e4fa7b55a5d2508f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 07:57:59 +0200
Subject: [PATCH 52/70] _process_asin_relation() -> _queue_from_asin_relation()

---
 picard/coverart.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index c89ef3a0d..5554bd3c1 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -306,11 +306,11 @@ class CoverArt:
                             elif use_amazon \
                                 and (relation.type == 'amazon asin' or
                                      relation.type == 'has_Amazon_ASIN'):
-                                self._process_asin_relation(relation)
+                                self._queue_from_asin_relation(relation)
         except AttributeError:
             self.album.error_append(traceback.format_exc())
 
-    def _process_asin_relation(self, relation):
+    def _queue_from_asin_relation(self, relation):
         """Queue cover art images from Amazon"""
         amz = parse_amazon_url(relation.target[0].text)
         if amz is None:

From 90faeb9fd473ffccd76154eee7f68793b7e44a13 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 08:02:06 +0200
Subject: [PATCH 53/70] Move code to new _queue_from_cover_art_relation()

Consistency.
---
 picard/coverart.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 5554bd3c1..742c75a6c 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -300,9 +300,7 @@ class CoverArt:
                             if use_whitelist \
                                 and (relation.type == 'cover art link' or
                                      relation.type == 'has_cover_art_at'):
-                                log.debug("Found cover art link in whitelist")
-                                url = relation.target[0].text
-                                self._queue_put(CoverArtImage(url))
+                                self._queue_from_cover_art_relation(relation)
                             elif use_amazon \
                                 and (relation.type == 'amazon asin' or
                                      relation.type == 'has_Amazon_ASIN'):
@@ -310,6 +308,12 @@ class CoverArt:
         except AttributeError:
             self.album.error_append(traceback.format_exc())
 
+    def _queue_from_cover_art_relation(self, relation):
+        """Queue from cover art relationships"""
+        log.debug("Found cover art link in whitelist")
+        url = relation.target[0].text
+        self._queue_put(CoverArtImage(url))
+
     def _queue_from_asin_relation(self, relation):
         """Queue cover art images from Amazon"""
         amz = parse_amazon_url(relation.target[0].text)

From f086ccb1e917bdc960e9f02ada3181c031b20714 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 08:04:26 +0200
Subject: [PATCH 54/70] _queue_image_from_caa() -> _queue_from_caa()

---
 picard/coverart.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 742c75a6c..1db103a9c 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -261,13 +261,13 @@ class CoverArt:
                     if types:
                         if not caa_front_found:
                             caa_front_found = u'front' in types
-                        self._queue_image_from_caa(image)
+                        self._queue_from_caa(image)
 
         if error or not caa_front_found:
             self._queue_from_relationships()
         self._download_next_in_queue()
 
-    def _queue_image_from_caa(self, image):
+    def _queue_from_caa(self, image):
         """Queue images depending on the CAA image size settings."""
         imagesize = config.setting["caa_image_size"]
         thumbsize = _CAA_THUMBNAIL_SIZE_MAP.get(imagesize, None)

From 5cddf673131a10689c0573ecc2b193097bdb1281 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 09:00:01 +0200
Subject: [PATCH 55/70] CoverArtImage.desc -> CoverArtImage.comment

---
 picard/coverart.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 1db103a9c..3fa4f4516 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -81,7 +81,7 @@ class CoverArtImage:
     # consider all images as front if types aren't supported by provider
     is_front = True
 
-    def __init__(self, url, types=[u'front'], desc=''):
+    def __init__(self, url, types=[u'front'], comment=''):
         self.url = QUrl(url)
         path = str(self.url.encodedPath())
         if self.url.hasQuery():
@@ -90,7 +90,7 @@ class CoverArtImage:
         self.port = self.url.port(80)
         self.path = str(path)
         self.types = types
-        self.desc = desc
+        self.comment = comment
 
     def is_front_image(self):
         # CAA has a flag for "front" image, use it in priority
@@ -100,8 +100,8 @@ class CoverArtImage:
         return u'front' in self.types
 
     def __repr__(self):
-        if self.desc:
-            return "types: %r (%r) from %s" % (self.types, self.desc, self.url.toString())
+        if self.comment:
+            return "types: %r (%r) from %s" % (self.types, self.comment, self.url.toString())
         else:
             return "types: %r from %s" % (self.types, self.url.toString())
 
@@ -211,7 +211,7 @@ class CoverArt:
                     mime,
                     data,
                     types=coverartimage.types,
-                    comment=coverartimage.desc,
+                    comment=coverartimage.comment,
                     is_front=coverartimage.is_front
                 )
                 for track in self.album._new_tracks:
@@ -219,7 +219,7 @@ class CoverArt:
                         mime,
                         data,
                         types=coverartimage.types,
-                        comment=coverartimage.desc,
+                        comment=coverartimage.comment,
                         is_front=coverartimage.is_front
                     )
             except (IOError, OSError) as e:
@@ -278,7 +278,7 @@ class CoverArt:
         coverartimage = CaaCoverArtImage(
             url,
             types=image["types"],
-            desc=image["comment"],
+            comment=image["comment"],
         )
         coverartimage.is_front = bool(image['front'])  # front image indicator from CAA
         self._queue_put(coverartimage)

From 919150d2a2415035bc93f4c02bd48a0386a280cc Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 09:31:24 +0200
Subject: [PATCH 56/70] CoverArtImage: make url optional at instanciation,
 improve __repr__()

---
 picard/coverart.py | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 3fa4f4516..bdb52245e 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -81,7 +81,15 @@ class CoverArtImage:
     # consider all images as front if types aren't supported by provider
     is_front = True
 
-    def __init__(self, url, types=[u'front'], comment=''):
+    def __init__(self, url=None, types=[u'front'], comment=''):
+        if url is not None:
+            self.parse_url(url)
+        else:
+            self.url = None
+        self.types = types
+        self.comment = comment
+
+    def parse_url(self, url):
         self.url = QUrl(url)
         path = str(self.url.encodedPath())
         if self.url.hasQuery():
@@ -89,8 +97,6 @@ class CoverArtImage:
         self.host = str(self.url.host())
         self.port = self.url.port(80)
         self.path = str(path)
-        self.types = types
-        self.comment = comment
 
     def is_front_image(self):
         # CAA has a flag for "front" image, use it in priority
@@ -100,10 +106,13 @@ class CoverArtImage:
         return u'front' in self.types
 
     def __repr__(self):
+        p = []
+        if self.url is not None:
+            p.append("url=%r" % self.url.toString())
+        p.append("types=%r" % self.types)
         if self.comment:
-            return "types: %r (%r) from %s" % (self.types, self.comment, self.url.toString())
-        else:
-            return "types: %r from %s" % (self.types, self.url.toString())
+            p.append("comment=%r" % self.comment)
+        return "%s(%s)" % (self.__class__.__name__, ", ".join(p))
 
 
 class CaaCoverArtImage(CoverArtImage):

From 4f3520e68855aca41b4d52221b1e79bcf22ee797 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 09:46:35 +0200
Subject: [PATCH 57/70] CoverArtImage: add __unicode__() and __str__(), improve
 error/debug messages

---
 picard/coverart.py | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index bdb52245e..11d59b892 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -114,6 +114,18 @@ class CoverArtImage:
             p.append("comment=%r" % self.comment)
         return "%s(%s)" % (self.__class__.__name__, ", ".join(p))
 
+    def __unicode__(self):
+        p = [u'Image']
+        if self.url is not None:
+            p.append(u"from %s" % self.url.toString())
+        p.append(u"of type %s" % u','.join(self.types))
+        if self.comment:
+            p.append(u"and comment '%s'" % self.comment)
+        return u' '.join(p)
+
+    def __str__(self):
+        return unicode(self).encode('utf-8')
+
 
 class CaaCoverArtImage(CoverArtImage):
 
@@ -203,7 +215,7 @@ class CoverArt:
         if error:
             self._coverart_http_error(http)
         elif len(data) < 1000:
-            self.album.error_append("Not enough data, skipping image %r" % coverartimage)
+            self.album.error_append("Not enough data, skipping %s" % coverartimage)
         else:
             self._message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
@@ -355,7 +367,7 @@ class CoverArt:
         if not coverartimage.support_types and self.at_least_one_front_image:
             # we already have one front image, no need to try other type-less
             # sources
-            log.debug("Skipping cover art %r, one front image is already available",
+            log.debug("Skipping %r, one front image is already available",
                       coverartimage)
             self._download_next_in_queue()
             return
@@ -379,7 +391,7 @@ class CoverArt:
 
     def _queue_put(self, coverartimage):
         "Add an image to queue"
-        log.debug("Queing image %r for download", coverartimage)
+        log.debug("Queing %r for download", coverartimage)
         self.__queue.append(coverartimage)
 
     def _queue_get(self):

From 118bfa8d5c558d985961c2b73077ce07d3d97ccc Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 10:16:37 +0200
Subject: [PATCH 58/70] Improve CAA debug messages

---
 picard/coverart.py | 67 +++++++++++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 11d59b892..c404ac55c 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -150,11 +150,7 @@ class CoverArt:
     def retrieve(self):
         """Retrieve available cover art images for the release"""
 
-        if (config.setting['ca_provider_use_caa']
-                and self.len_caa_types > 0
-                and self._has_caa_artwork()):
-            log.debug("There are suitable images in the cover art archive for %s"
-                      % self.release.id)
+        if self._has_caa_artwork():
             self._xmlws_download(
                 CAA_HOST,
                 CAA_PORT,
@@ -164,14 +160,18 @@ class CoverArt:
                 important=False
             )
         else:
-            if config.setting['ca_provider_use_caa']:
-                log.debug("There are no suitable images in the cover art archive for %s"
-                          % self.release.id)
             self._queue_from_relationships()
             self._download_next_in_queue()
 
     def _has_caa_artwork(self):
         """Check if CAA artwork has to be downloaded"""
+        if not config.setting['ca_provider_use_caa']:
+            log.debug("Cover Art Archive disabled by user")
+            return False
+        if not self.len_caa_types:
+            log.debug("User disabled all Cover Art Archive types")
+            return False
+
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
         has_caa_artwork = False
@@ -179,27 +179,40 @@ class CoverArt:
         if 'cover_art_archive' in self.release.children:
             caa_node = self.release.children['cover_art_archive'][0]
             has_caa_artwork = (caa_node.artwork[0].text == 'true')
-            has_front = 'front' in self.caa_types
-            has_back = 'back' in self.caa_types
 
-            if self.len_caa_types == 2 and (has_front or has_back):
-                # The OR cases are there to still download and process the CAA
-                # JSON file if front or back is enabled but not in the CAA and
-                # another type (that's neither front nor back) is enabled.
-                # For example, if both front and booklet are enabled and the
-                # CAA only has booklet images, the front element in the XML
-                # from the webservice will be false (thus front_in_caa is False
-                # as well) but it's still necessary to download the booklet
-                # images by using the fact that back is enabled but there are
-                # no back images in the CAA.
-                front_in_caa = caa_node.front[0].text == 'true' or not has_front
-                back_in_caa = caa_node.back[0].text == 'true' or not has_back
-                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
+        if not has_caa_artwork:
+            log.debug("There are no images in the Cover Art Archive for %s"
+                      % self.release.id)
+            return False
 
-            elif self.len_caa_types == 1 and (has_front or has_back):
-                front_in_caa = caa_node.front[0].text == 'true' and has_front
-                back_in_caa = caa_node.back[0].text == 'true' and has_back
-                has_caa_artwork = has_caa_artwork and (front_in_caa or back_in_caa)
+        has_front = 'front' in self.caa_types
+        has_back = 'back' in self.caa_types
+
+        if self.len_caa_types == 2 and (has_front or has_back):
+            # The OR cases are there to still download and process the CAA
+            # JSON file if front or back is enabled but not in the CAA and
+            # another type (that's neither front nor back) is enabled.
+            # For example, if both front and booklet are enabled and the
+            # CAA only has booklet images, the front element in the XML
+            # from the webservice will be false (thus front_in_caa is False
+            # as well) but it's still necessary to download the booklet
+            # images by using the fact that back is enabled but there are
+            # no back images in the CAA.
+            front_in_caa = caa_node.front[0].text == 'true' or not has_front
+            back_in_caa = caa_node.back[0].text == 'true' or not has_back
+            has_caa_artwork = front_in_caa or back_in_caa
+
+        elif self.len_caa_types == 1 and (has_front or has_back):
+            front_in_caa = caa_node.front[0].text == 'true' and has_front
+            back_in_caa = caa_node.back[0].text == 'true' and has_back
+            has_caa_artwork = front_in_caa or back_in_caa
+
+        if not has_caa_artwork:
+            log.debug("There are no suitable images in the Cover Art Archive for %s"
+                      % self.release.id)
+        else:
+            log.debug("There are suitable images in the Cover Art Archive for %s"
+                      % self.release.id)
 
         return has_caa_artwork
 

From 5ed2a5b81aa24623157f5c5efded0e3021e6639f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 May 2014 10:54:57 +0200
Subject: [PATCH 59/70] _download_next_in_queue(): fix up description

---
 picard/coverart.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index c404ac55c..0bdb38175 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -365,7 +365,7 @@ class CoverArt:
             self._queue_put(CoverArtImage(url))
 
     def _download_next_in_queue(self):
-        """Downloads each item in queue.
+        """Downloads next item in queue.
            If there are none left, loading of album will be finalized.
         """
         if self._queue_empty():

From e762388a94ce1027c924ef92f15769d3ed5e0492 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 May 2014 15:19:28 +0200
Subject: [PATCH 60/70] Add Copyright line in licence header

---
 picard/coverart.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/picard/coverart.py b/picard/coverart.py
index 0bdb38175..d15a4bb09 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -6,6 +6,7 @@
 # Copyright (C) 2007, 2010, 2011 Lukáš Lalinský
 # Copyright (C) 2011 Michael Wiencek
 # Copyright (C) 2011-2012 Wieland Hoffmann
+# Copyright (C) 2013-2014 Laurent Monin
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License

From 7a90e8896abed75f037fe6b39afae0d366dde29c Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 May 2014 15:21:10 +0200
Subject: [PATCH 61/70] CoverArtImage.parse_url(): tidy up

---
 picard/coverart.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index d15a4bb09..d455ec8ba 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -92,12 +92,11 @@ class CoverArtImage:
 
     def parse_url(self, url):
         self.url = QUrl(url)
-        path = str(self.url.encodedPath())
-        if self.url.hasQuery():
-            path += '?' + self.url.encodedQuery()
         self.host = str(self.url.host())
         self.port = self.url.port(80)
-        self.path = str(path)
+        self.path = str(self.url.encodedPath())
+        if self.url.hasQuery():
+            self.path += '?' + str(self.url.encodedQuery())
 
     def is_front_image(self):
         # CAA has a flag for "front" image, use it in priority

From b256b673cdda68cfa479fc523a94cc80c86659e1 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 May 2014 15:23:01 +0200
Subject: [PATCH 62/70] at_least_one_front_image -> front_image_found

---
 picard/coverart.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index d455ec8ba..88165a128 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -142,7 +142,7 @@ class CoverArt:
         self.release = release
         self.caa_types = map(unicode.lower, config.setting["caa_image_types"])
         self.len_caa_types = len(self.caa_types)
-        self.at_least_one_front_image = False
+        self.front_image_found = False
 
     def __repr__(self):
         return "CoverArt for %r" % (self.album)
@@ -265,8 +265,8 @@ class CoverArt:
 
         # If the image already was a front image, there might still be some
         # other non-CAA front images in the queue - ignore them.
-        if not self.at_least_one_front_image:
-            self.at_least_one_front_image = coverartimage.is_front_image()
+        if not self.front_image_found:
+            self.front_image_found = coverartimage.is_front_image()
         self._download_next_in_queue()
 
     def _caa_json_downloaded(self, data, http, error):
@@ -377,7 +377,7 @@ class CoverArt:
 
         # We still have some items to try!
         coverartimage = self._queue_get()
-        if not coverartimage.support_types and self.at_least_one_front_image:
+        if not coverartimage.support_types and self.front_image_found:
             # we already have one front image, no need to try other type-less
             # sources
             log.debug("Skipping %r, one front image is already available",

From af67fd22bd2edfda3b9698bd003474d039319d5b Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 May 2014 15:24:51 +0200
Subject: [PATCH 63/70] Wrap few long lines and remove spurious space

---
 picard/coverart.py | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 88165a128..7b2efe288 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -228,7 +228,8 @@ class CoverArt:
         if error:
             self._coverart_http_error(http)
         elif len(data) < 1000:
-            self.album.error_append("Not enough data, skipping %s" % coverartimage)
+            self.album.error_append("Not enough data, skipping %s" %
+                                    coverartimage)
         else:
             self._message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
@@ -279,7 +280,8 @@ class CoverArt:
             try:
                 caa_data = json.loads(data)
             except ValueError:
-                self.album.error_append("Invalid JSON: %s", http.url().toString())
+                self.album.error_append(
+                    "Invalid JSON: %s", http.url().toString())
             else:
                 for image in caa_data["images"]:
                     if config.setting["caa_approved_only"] and not image["approved"]:
@@ -291,7 +293,8 @@ class CoverArt:
                     else:
                         image["types"] = map(unicode.lower, image["types"])
                     # only keep enabled caa types
-                    types = set(image["types"]).intersection(set(self.caa_types))
+                    types = set(image["types"]).intersection(
+                        set(self.caa_types))
                     if types:
                         if not caa_front_found:
                             caa_front_found = u'front' in types
@@ -314,7 +317,8 @@ class CoverArt:
             types=image["types"],
             comment=image["comment"],
         )
-        coverartimage.is_front = bool(image['front'])  # front image indicator from CAA
+        # front image indicator from CAA
+        coverartimage.is_front = bool(image['front'])
         self._queue_put(coverartimage)
 
     def _queue_from_relationships(self):
@@ -430,7 +434,7 @@ class CoverArt:
 
 
 def coverart(album, metadata, release):
-    """ Gets all cover art URLs from the metadata and then attempts to
+    """Gets all cover art URLs from the metadata and then attempts to
     download the album art. """
 
     coverart = CoverArt(album, metadata, release)

From 75857294556171d6ae4594a29805ecbd33bc0370 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 May 2014 16:12:55 +0200
Subject: [PATCH 64/70] Use named place holders in AMAZON_IMAGE_PATH

---
 picard/coverart.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 7b2efe288..6f33cc47e 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -67,7 +67,7 @@ AMAZON_SERVER = {
     },
 }
 
-AMAZON_IMAGE_PATH = '/images/P/%s.%s.%sZZZZZZZ.jpg'
+AMAZON_IMAGE_PATH = '/images/P/%(asin)s.%(serverid)s.%(size)sZZZZZZZ.jpg'
 
 
 _CAA_THUMBNAIL_SIZE_MAP = {
@@ -364,7 +364,11 @@ class CoverArt:
             serverInfo = AMAZON_SERVER['amazon.com']
         host = serverInfo['server']
         for size in ('L', 'M'):
-            path = AMAZON_IMAGE_PATH % (amz['asin'], serverInfo['id'], size)
+            path = AMAZON_IMAGE_PATH % {
+                'asin': amz['asin'],
+                'serverid': serverInfo['id'],
+                'size': size
+            }
             url = "http://%s:%s" % (host, path)
             self._queue_put(CoverArtImage(url))
 

From 21590355e123fe6fe00ead60897d06047de65b06 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 May 2014 16:15:47 +0200
Subject: [PATCH 65/70] Fix up incorrect handling of missing image (data <
 1000) case

- just log a warning, it may happen ie. if large amazon's image request failed
- set front_image_found only if no error
---
 picard/coverart.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 6f33cc47e..e70d656c4 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -228,8 +228,7 @@ class CoverArt:
         if error:
             self._coverart_http_error(http)
         elif len(data) < 1000:
-            self.album.error_append("Not enough data, skipping %s" %
-                                    coverartimage)
+            log.warning("Not enough data, skipping %s" % coverartimage)
         else:
             self._message(
                 N_("Cover art of type '%(type)s' downloaded for %(albumid)s from %(host)s"),
@@ -257,6 +256,12 @@ class CoverArt:
                         comment=coverartimage.comment,
                         is_front=coverartimage.is_front
                     )
+                # If the image already was a front image,
+                # there might still be some other non-CAA front
+                # images in the queue - ignore them.
+                if not self.front_image_found:
+                    self.front_image_found = coverartimage.is_front_image()
+
             except (IOError, OSError) as e:
                 self.album.error_append(e.message)
                 self.album._finalize_loading(error=True)
@@ -264,10 +269,6 @@ class CoverArt:
                 # save them in the temporary folder, abort.
                 return
 
-        # If the image already was a front image, there might still be some
-        # other non-CAA front images in the queue - ignore them.
-        if not self.front_image_found:
-            self.front_image_found = coverartimage.is_front_image()
         self._download_next_in_queue()
 
     def _caa_json_downloaded(self, data, http, error):

From 5a398062c10055bf83ab282bffa6b88c8eca0f88 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 May 2014 17:58:51 +0200
Subject: [PATCH 66/70] Improve amazon sizes codes description

Source: http://aaugh.com/imageabuse.html (+ confirmed by my tests)
---
 picard/coverart.py | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index e70d656c4..6c8e38914 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -67,8 +67,22 @@ AMAZON_SERVER = {
     },
 }
 
-AMAZON_IMAGE_PATH = '/images/P/%(asin)s.%(serverid)s.%(size)sZZZZZZZ.jpg'
+AMAZON_IMAGE_PATH = '/images/P/%(asin)s.%(serverid)s.%(size)s.jpg'
 
+# First item in the list will be tried first
+AMAZON_SIZES = (
+    # huge size option is only available for items
+    # that have a ZOOMing picture on its amazon web page
+    # and it doesn't work for all of the domain names
+    #'_SCRM_',        # huge size
+    'LZZZZZZZ',      # large size, option format 1
+    #'_SCLZZZZZZZ_',  # large size, option format 3
+    'MZZZZZZZ',      # default image size, format 1
+    #'_SCMZZZZZZZ_',  # medium size, option format 3
+    #'TZZZZZZZ',      # medium image size, option format 1
+    #'_SCTZZZZZZZ_',  # small size, option format 3
+    #'THUMBZZZ',      # small size, option format 1
+)
 
 _CAA_THUMBNAIL_SIZE_MAP = {
     0: "small",
@@ -364,7 +378,7 @@ class CoverArt:
         else:
             serverInfo = AMAZON_SERVER['amazon.com']
         host = serverInfo['server']
-        for size in ('L', 'M'):
+        for size in AMAZON_SIZES:
             path = AMAZON_IMAGE_PATH % {
                 'asin': amz['asin'],
                 'serverid': serverInfo['id'],

From 807870274128986bde2ede2404f29f0aa8035878 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sat, 10 May 2014 20:53:30 +0200
Subject: [PATCH 67/70] has_front -> want_front, has_back -> want_back

Those are defined according to user config option for accepted caa types.
---
 picard/coverart.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 6c8e38914..0c40358f0 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -199,10 +199,10 @@ class CoverArt:
                       % self.release.id)
             return False
 
-        has_front = 'front' in self.caa_types
-        has_back = 'back' in self.caa_types
+        want_front = 'front' in self.caa_types
+        want_back = 'back' in self.caa_types
 
-        if self.len_caa_types == 2 and (has_front or has_back):
+        if self.len_caa_types == 2 and (want_front or want_back):
             # The OR cases are there to still download and process the CAA
             # JSON file if front or back is enabled but not in the CAA and
             # another type (that's neither front nor back) is enabled.
@@ -212,13 +212,13 @@ class CoverArt:
             # as well) but it's still necessary to download the booklet
             # images by using the fact that back is enabled but there are
             # no back images in the CAA.
-            front_in_caa = caa_node.front[0].text == 'true' or not has_front
-            back_in_caa = caa_node.back[0].text == 'true' or not has_back
+            front_in_caa = caa_node.front[0].text == 'true' or not want_front
+            back_in_caa = caa_node.back[0].text == 'true' or not want_back
             has_caa_artwork = front_in_caa or back_in_caa
 
-        elif self.len_caa_types == 1 and (has_front or has_back):
-            front_in_caa = caa_node.front[0].text == 'true' and has_front
-            back_in_caa = caa_node.back[0].text == 'true' and has_back
+        elif self.len_caa_types == 1 and (want_front or want_back):
+            front_in_caa = caa_node.front[0].text == 'true' and want_front
+            back_in_caa = caa_node.back[0].text == 'true' and want_back
             has_caa_artwork = front_in_caa or back_in_caa
 
         if not has_caa_artwork:

From 7b034087d707d38620b7ca22281c49a1f87bc847 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sat, 10 May 2014 20:59:52 +0200
Subject: [PATCH 68/70] Introduce caa_has_front and caa_has_back, reduce code
 redundancy

---
 picard/coverart.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 0c40358f0..245198a62 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -201,6 +201,8 @@ class CoverArt:
 
         want_front = 'front' in self.caa_types
         want_back = 'back' in self.caa_types
+        caa_has_front = caa_node.front[0].text == 'true'
+        caa_has_back = caa_node.back[0].text == 'true'
 
         if self.len_caa_types == 2 and (want_front or want_back):
             # The OR cases are there to still download and process the CAA
@@ -212,13 +214,13 @@ class CoverArt:
             # as well) but it's still necessary to download the booklet
             # images by using the fact that back is enabled but there are
             # no back images in the CAA.
-            front_in_caa = caa_node.front[0].text == 'true' or not want_front
-            back_in_caa = caa_node.back[0].text == 'true' or not want_back
+            front_in_caa = caa_has_front or not want_front
+            back_in_caa = caa_has_back or not want_back
             has_caa_artwork = front_in_caa or back_in_caa
 
         elif self.len_caa_types == 1 and (want_front or want_back):
-            front_in_caa = caa_node.front[0].text == 'true' and want_front
-            back_in_caa = caa_node.back[0].text == 'true' and want_back
+            front_in_caa = caa_has_front and want_front
+            back_in_caa = caa_has_back and want_back
             has_caa_artwork = front_in_caa or back_in_caa
 
         if not has_caa_artwork:

From ced1d2ac82dcdc908095c2a0bb6ac34f428e5b5f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sat, 10 May 2014 21:02:35 +0200
Subject: [PATCH 69/70] has_caa_artwork -> caa_has_suitable_artwork

It reflects much more its purpose as even if caa has artwork we may have this set to False
---
 picard/coverart.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 245198a62..5aab95636 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -164,7 +164,7 @@ class CoverArt:
     def retrieve(self):
         """Retrieve available cover art images for the release"""
 
-        if self._has_caa_artwork():
+        if self._caa_has_suitable_artwork():
             self._xmlws_download(
                 CAA_HOST,
                 CAA_PORT,
@@ -177,7 +177,7 @@ class CoverArt:
             self._queue_from_relationships()
             self._download_next_in_queue()
 
-    def _has_caa_artwork(self):
+    def _caa_has_suitable_artwork(self):
         """Check if CAA artwork has to be downloaded"""
         if not config.setting['ca_provider_use_caa']:
             log.debug("Cover Art Archive disabled by user")
@@ -188,13 +188,13 @@ class CoverArt:
 
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
-        has_caa_artwork = False
+        caa_has_suitable_artwork = False
 
         if 'cover_art_archive' in self.release.children:
             caa_node = self.release.children['cover_art_archive'][0]
-            has_caa_artwork = (caa_node.artwork[0].text == 'true')
+            caa_has_suitable_artwork = (caa_node.artwork[0].text == 'true')
 
-        if not has_caa_artwork:
+        if not caa_has_suitable_artwork:
             log.debug("There are no images in the Cover Art Archive for %s"
                       % self.release.id)
             return False
@@ -216,21 +216,21 @@ class CoverArt:
             # no back images in the CAA.
             front_in_caa = caa_has_front or not want_front
             back_in_caa = caa_has_back or not want_back
-            has_caa_artwork = front_in_caa or back_in_caa
+            caa_has_suitable_artwork = front_in_caa or back_in_caa
 
         elif self.len_caa_types == 1 and (want_front or want_back):
             front_in_caa = caa_has_front and want_front
             back_in_caa = caa_has_back and want_back
-            has_caa_artwork = front_in_caa or back_in_caa
+            caa_has_suitable_artwork = front_in_caa or back_in_caa
 
-        if not has_caa_artwork:
+        if not caa_has_suitable_artwork:
             log.debug("There are no suitable images in the Cover Art Archive for %s"
                       % self.release.id)
         else:
             log.debug("There are suitable images in the Cover Art Archive for %s"
                       % self.release.id)
 
-        return has_caa_artwork
+        return caa_has_suitable_artwork
 
     def _coverart_http_error(self, http):
         """Append http error to album errors"""

From 52afea16d8413b29970fd0e032f2b911f0a148ec Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sat, 10 May 2014 21:21:13 +0200
Subject: [PATCH 70/70] Differentiate case of no CAA info vs no cover art

---
 picard/coverart.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/picard/coverart.py b/picard/coverart.py
index 5aab95636..a3e81b439 100644
--- a/picard/coverart.py
+++ b/picard/coverart.py
@@ -188,11 +188,13 @@ class CoverArt:
 
         # MB web service indicates if CAA has artwork
         # http://tickets.musicbrainz.org/browse/MBS-4536
-        caa_has_suitable_artwork = False
+        if 'cover_art_archive' not in self.release.children:
+            log.debug("No Cover Art Archive information for %s"
+                      % self.release.id)
+            return False
 
-        if 'cover_art_archive' in self.release.children:
-            caa_node = self.release.children['cover_art_archive'][0]
-            caa_has_suitable_artwork = (caa_node.artwork[0].text == 'true')
+        caa_node = self.release.children['cover_art_archive'][0]
+        caa_has_suitable_artwork = caa_node.artwork[0].text == 'true'
 
         if not caa_has_suitable_artwork:
             log.debug("There are no images in the Cover Art Archive for %s"