From 07a7ea84a2664f87f5fd2b2349f9c3dfc931e5b1 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 13:45:37 +0200
Subject: [PATCH 01/33] WebService: add *_url() methods accepting url= (and
 others) parameters

Those are meant to ease the transition from equivalent methods using host,port,etc...
---
 picard/webservice/__init__.py | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index ccde659e8..203e08086 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -629,6 +629,34 @@ class WebService(QtCore.QObject):
         )
         return self.add_request(request)
 
+    def get_url(self, **kwargs):
+        kwargs['method'] = 'GET'
+        kwargs['parse_response_type'] = kwargs.get('parse_response_type', DEFAULT_RESPONSE_PARSER_TYPE)
+        return self.add_request(WSRequest(**kwargs))
+
+    def post_url(self, **kwargs):
+        kwargs['method'] = 'POST'
+        kwargs['parse_response_type'] = kwargs.get('parse_response_type', DEFAULT_RESPONSE_PARSER_TYPE)
+        kwargs['mblogin'] = kwargs.get('mblogin', True)
+        log.debug("POST-DATA %r", kwargs['data'])
+        return self.add_request(WSRequest(**kwargs))
+
+    def put_url(self, **kwargs):
+        kwargs['method'] = 'PUT'
+        kwargs['priority'] = kwargs.get('priority', True)
+        kwargs['mblogin'] = kwargs.get('mblogin', True)
+        return self.add_request(WSRequest(**kwargs))
+
+    def delete_url(self, **kwargs):
+        kwargs['method'] = 'DELETE'
+        kwargs['priority'] = kwargs.get('priority', True)
+        kwargs['mblogin'] = kwargs.get('mblogin', True)
+        return self.add_request(WSRequest(**kwargs))
+
+    def download_url(self, **kwargs):
+        kwargs['method'] = 'GET'
+        return self.add_request(WSRequest(**kwargs))
+
     def stop(self):
         for reply in list(self._active_requests):
             reply.abort()

From 04fb24deb08236d7ad4103dda2b28ca9ae552ae9 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 13:56:04 +0200
Subject: [PATCH 02/33] Add tests for WebService.*_url() new methods

---
 test/test_webservice.py | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/test/test_webservice.py b/test/test_webservice.py
index 63b311f5b..bd79fe698 100644
--- a/test/test_webservice.py
+++ b/test/test_webservice.py
@@ -100,6 +100,30 @@ class WebServiceTest(PicardTestCase):
         self.assertIn("GET", get_wsreq(mock_add_task).method)
         self.assertEqual(5, mock_add_task.call_count)
 
+    @patch.object(WebService, 'add_task')
+    def test_webservice_url_method_calls(self, mock_add_task):
+        url = "http://abc.xyz"
+        handler = dummy_handler
+        data = None
+
+        def get_wsreq(mock_add_task):
+            return mock_add_task.call_args[0][1]
+
+        self.ws.get_url(url=url, handler=handler)
+        self.assertEqual(1, mock_add_task.call_count)
+        self.assertEqual('abc.xyz', get_wsreq(mock_add_task).host)
+        self.assertEqual(80, get_wsreq(mock_add_task).port)
+        self.assertIn("GET", get_wsreq(mock_add_task).method)
+        self.ws.post_url(url=url, data=data, handler=handler)
+        self.assertIn("POST", get_wsreq(mock_add_task).method)
+        self.ws.put_url(url=url, data=data, handler=handler)
+        self.assertIn("PUT", get_wsreq(mock_add_task).method)
+        self.ws.delete_url(url=url, handler=handler)
+        self.assertIn("DELETE", get_wsreq(mock_add_task).method)
+        self.ws.download_url(url=url, handler=handler)
+        self.assertIn("GET", get_wsreq(mock_add_task).method)
+        self.assertEqual(5, mock_add_task.call_count)
+
 
 class WebServiceTaskTest(PicardTestCase):
 

From a0213bd8174abb1b08f066754931891b6b5bb87a Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 13:46:53 +0200
Subject: [PATCH 03/33] Use WebService.get_url() and WebService.get_download(),
 introduce CAA_URL constant

Note: I removed a call to CaaThumbnailCoverArtImage() that doesn't look to make sense
---
 picard/const/__init__.py        |  1 +
 picard/ui/searchdialog/album.py | 26 ++++++++------------------
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/picard/const/__init__.py b/picard/const/__init__.py
index ae8814395..3aa43cb5d 100644
--- a/picard/const/__init__.py
+++ b/picard/const/__init__.py
@@ -65,6 +65,7 @@ MUSICBRAINZ_OAUTH_CLIENT_SECRET = 'xIsvXbIuntaLuRRhzuazOA'
 # Cover art archive URL and port
 CAA_HOST = "coverartarchive.org"
 CAA_PORT = 443
+CAA_URL = 'https://coverartarchive.org'
 
 # Prepare documentation URLs
 if PICARD_VERSION.identifier == 'final':
diff --git a/picard/ui/searchdialog/album.py b/picard/ui/searchdialog/album.py
index b99721927..338fb02ce 100644
--- a/picard/ui/searchdialog/album.py
+++ b/picard/ui/searchdialog/album.py
@@ -35,11 +35,7 @@ from picard.config import (
     Option,
     get_config,
 )
-from picard.const import (
-    CAA_HOST,
-    CAA_PORT,
-)
-from picard.coverart.image import CaaThumbnailCoverArtImage
+from picard.const import CAA_URL
 from picard.mbjson import (
     countries_from_node,
     media_formats_from_node,
@@ -248,12 +244,10 @@ class AlbumSearchDialog(SearchDialog):
         if not cell.is_visible():
             return
         cell.fetched = True
-        caa_path = "/release/%s" % cell.release["musicbrainz_albumid"]
-        cell.fetch_task = self.tagger.webservice.get(
-            CAA_HOST,
-            CAA_PORT,
-            caa_path,
-            partial(self._caa_json_downloaded, cell)
+        mbid = cell.release["musicbrainz_albumid"]
+        cell.fetch_task = self.tagger.webservice.get_url(
+            url=f'{CAA_URL}/release/{mbid}',
+            handler=partial(self._caa_json_downloaded, cell),
         )
 
     def _caa_json_downloaded(self, cover_cell, data, http, error):
@@ -275,13 +269,9 @@ class AlbumSearchDialog(SearchDialog):
                     break
 
             if front:
-                url = front["thumbnails"]["small"]
-                coverartimage = CaaThumbnailCoverArtImage(url=url)
-                cover_cell.fetch_task = self.tagger.webservice.download(
-                    coverartimage.host,
-                    coverartimage.port,
-                    coverartimage.path,
-                    partial(self._cover_downloaded, cover_cell)
+                cover_cell.fetch_task = self.tagger.webservice.download_url(
+                    url=front["thumbnails"]["small"],
+                    handler=partial(self._cover_downloaded, cover_cell)
                 )
             else:
                 cover_cell.not_found()

From 572648d11010c7708ecd1993c52d2b296256b04b Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 14:10:34 +0200
Subject: [PATCH 04/33] oauth: use WebService *_url() methods

---
 picard/oauth.py | 51 ++++++++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/picard/oauth.py b/picard/oauth.py
index 68442f761..971a0f9f6 100644
--- a/picard/oauth.py
+++ b/picard/oauth.py
@@ -150,6 +150,12 @@ class OAuthManager(object):
                 self.forget_access_token()
                 self.refresh_access_token(callback)
 
+    def url(self, path=None, params=None):
+        return build_qurl(
+            self.host, self.port, path=path,
+            queryargs=params
+        )
+
     def get_authorization_url(self, scopes):
         params = {
             "response_type": "code",
@@ -157,11 +163,7 @@ class OAuthManager(object):
             "redirect_uri": "urn:ietf:wg:oauth:2.0:oob",
             "scope": scopes,
         }
-        url = build_qurl(
-            self.host, self.port, path="/oauth2/authorize",
-            queryargs=params
-        )
-        return bytes(url.toEncoded()).decode()
+        return bytes(self.url(path="/oauth2/authorize", params=params).toEncoded()).decode()
 
     def set_refresh_token(self, refresh_token, scopes):
         log.debug("OAuth: got refresh_token %s with scopes %s", refresh_token, scopes)
@@ -179,18 +181,20 @@ class OAuthManager(object):
 
     def refresh_access_token(self, callback):
         log.debug("OAuth: refreshing access_token with a refresh_token %s", self.refresh_token)
-        path = "/oauth2/token"
         params = {
             "grant_type": "refresh_token",
             "refresh_token": self.refresh_token,
             "client_id": MUSICBRAINZ_OAUTH_CLIENT_ID,
             "client_secret": MUSICBRAINZ_OAUTH_CLIENT_SECRET,
         }
-        self.webservice.post(
-            self.host, self.port, path, self._query_data(params),
-            partial(self.on_refresh_access_token_finished, callback),
-            mblogin=True, priority=True, important=True,
-            request_mimetype="application/x-www-form-urlencoded"
+        self.webservice.post_url(
+            url=self.url(path="/oauth2/token"),
+            data=self._query_data(params),
+            handler=partial(self.on_refresh_access_token_finished, callback),
+            mblogin=True,
+            priority=True,
+            important=True,
+            request_mimetype="application/x-www-form-urlencoded",
         )
 
     def on_refresh_access_token_finished(self, callback, data, http, error):
@@ -212,7 +216,6 @@ class OAuthManager(object):
 
     def exchange_authorization_code(self, authorization_code, scopes, callback):
         log.debug("OAuth: exchanging authorization_code %s for an access_token", authorization_code)
-        path = "/oauth2/token"
         params = {
             "grant_type": "authorization_code",
             "code": authorization_code,
@@ -220,11 +223,14 @@ class OAuthManager(object):
             "client_secret": MUSICBRAINZ_OAUTH_CLIENT_SECRET,
             "redirect_uri": "urn:ietf:wg:oauth:2.0:oob",
         }
-        self.webservice.post(
-            self.host, self.port, path, self._query_data(params),
-            partial(self.on_exchange_authorization_code_finished, scopes, callback),
-            mblogin=True, priority=True, important=True,
-            request_mimetype="application/x-www-form-urlencoded"
+        self.webservice.post_url(
+            url=self.url(path="/oauth2/token"),
+            data=self._query_data(params),
+            handler=partial(self.on_exchange_authorization_code_finished, scopes, callback),
+            mblogin=True,
+            priority=True,
+            important=True,
+            request_mimetype="application/x-www-form-urlencoded",
         )
 
     def on_exchange_authorization_code_finished(self, scopes, callback, data, http, error):
@@ -246,11 +252,12 @@ class OAuthManager(object):
 
     def fetch_username(self, callback):
         log.debug("OAuth: fetching username")
-        path = "/oauth2/userinfo"
-        self.webservice.get(
-            self.host, self.port, path,
-            partial(self.on_fetch_username_finished, callback),
-            mblogin=True, priority=True, important=True
+        self.webservice.get_url(
+            url=self.url(path="/oauth2/userinfo"),
+            handler=partial(self.on_fetch_username_finished, callback),
+            mblogin=True,
+            priority=True,
+            important=True,
         )
 
     def on_fetch_username_finished(self, callback, data, http, error):

From 76139d946d10ffaf4233bda81b764656fdcd4730 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 17:18:42 +0200
Subject: [PATCH 05/33] CoverArt: use WebService.download_url()

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

diff --git a/picard/coverart/__init__.py b/picard/coverart/__init__.py
index e94a6c19c..a6f3883f4 100644
--- a/picard/coverart/__init__.py
+++ b/picard/coverart/__init__.py
@@ -195,14 +195,10 @@ class CoverArt:
             echo=None
         )
         log.debug("Downloading %r", coverartimage)
-        self.album.tagger.webservice.download(
-            coverartimage.host,
-            coverartimage.port,
-            coverartimage.path,
-            partial(self._coverart_downloaded, coverartimage),
-            queryargs=coverartimage.queryargs,
+        self.album.tagger.webservice.download_url(
+            url=coverartimage.url,
+            handler=partial(self._coverart_downloaded, coverartimage),
             priority=True,
-            important=False
         )
         self.album._requests += 1
 

From d4e4641837c8ec4da45034345824acd259bd3b8d Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 18:10:22 +0200
Subject: [PATCH 06/33] WSRequest: (re-)introduce queryargs optional parameter
 to ease adding query params

---
 picard/webservice/__init__.py | 10 ++++++++++
 test/test_webservice.py       | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index 203e08086..dce872384 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -115,6 +115,7 @@ class WSRequest(QNetworkRequest):
         important=False,
         request_mimetype=None,
         url=None,
+        queryargs=None,
     ):
         """
         Args:
@@ -134,6 +135,7 @@ class WSRequest(QNetworkRequest):
             important: Indicates that this is an important request.
             request_mimetype: Set the Content-Type header.
             url: URL passed as a string or as a QUrl to use for this request
+            queryargs: dictionary of keys and values to add to the url
         """
         # mandatory parameters
         self.method = method
@@ -149,6 +151,14 @@ class WSRequest(QNetworkRequest):
 
         if not isinstance(url, QUrl):
             url = QUrl(url)
+
+        if queryargs is not None:
+            query = QtCore.QUrlQuery(url)
+            for k, v in queryargs.items():
+                # FIXME: check if encoding is correct
+                query.addQueryItem(k, str(v))
+            url.setQuery(query)
+
         super().__init__(url)
 
         # optional parameters
diff --git a/test/test_webservice.py b/test/test_webservice.py
index bd79fe698..384925c2a 100644
--- a/test/test_webservice.py
+++ b/test/test_webservice.py
@@ -521,3 +521,13 @@ class WSRequestTest(PicardTestCase):
         self.assertTrue(TEMP_ERRORS_RETRIES > 1)
         self.assertEqual(request.mark_for_retry(), 1)
         self.assertFalse(request.max_retries_reached())
+
+    def test_queryargs(self):
+        request = WSRequest(
+            url='http://example.org/path?a=1',
+            method='GET',
+            handler=dummy_handler,
+            queryargs={'a': 2, 'b': 'x%20x', 'c': '1+2', 'd': '&', 'e': '?'},
+        )
+        # FIXME: check encoding
+        self.assertEqual(request.url().toString(), 'http://example.org/path?a=1&a=2&b=x x&c=1+2&d=%26&e=?')

From cef706de081a32492396baafa910393b8cb18689 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 18:25:45 +0200
Subject: [PATCH 07/33] Plugins API: use WebService.get_url() and URLs instead
 of host,port,etc

---
 picard/const/__init__.py     | 13 ++++++-------
 picard/pluginmanager.py      | 10 ++++------
 picard/ui/options/plugins.py | 10 ++++------
 picard/util/checkupdate.py   | 15 ++++++---------
 4 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/picard/const/__init__.py b/picard/const/__init__.py
index 3aa43cb5d..f1d510ec0 100644
--- a/picard/const/__init__.py
+++ b/picard/const/__init__.py
@@ -145,14 +145,13 @@ MUSICBRAINZ_SERVERS = [
 ]
 
 # Plugins and Release Versions API
+PLUGINS_API_BASE_URL = 'https://picard.musicbrainz.org/api/v2/'
 PLUGINS_API = {
-    'host': 'picard.musicbrainz.org',
-    'port': 443,
-    'endpoint': {
-        'plugins': '/api/v2/plugins/',
-        'download': '/api/v2/download/',
-        'releases': '/api/v2/releases',
-    }
+    'urls': {
+        'plugins': PLUGINS_API_BASE_URL + 'plugins/',
+        'download': PLUGINS_API_BASE_URL + 'download/',
+        'releases': PLUGINS_API_BASE_URL + 'releases',
+    },
 }
 
 # Default query limit
diff --git a/picard/pluginmanager.py b/picard/pluginmanager.py
index 75e9e57c8..fc8899fa4 100644
--- a/picard/pluginmanager.py
+++ b/picard/pluginmanager.py
@@ -414,13 +414,11 @@ class PluginManager(QtCore.QObject):
                 self.plugin_updated.emit(plugin_name, False)
 
     def query_available_plugins(self, callback=None):
-        self.tagger.webservice.get(
-            PLUGINS_API['host'],
-            PLUGINS_API['port'],
-            PLUGINS_API['endpoint']['plugins'],
-            partial(self._plugins_json_loaded, callback=callback),
+        self.tagger.webservice.get_url(
+            url=PLUGINS_API['urls']['plugins'],
+            handler=partial(self._plugins_json_loaded, callback=callback),
             priority=True,
-            important=True
+            important=True,
         )
 
     def is_available(self, plugin_name):
diff --git a/picard/ui/options/plugins.py b/picard/ui/options/plugins.py
index 773d36dd5..122559a00 100644
--- a/picard/ui/options/plugins.py
+++ b/picard/ui/options/plugins.py
@@ -657,15 +657,13 @@ class PluginsOptionsPage(OptionsPage):
     def download_plugin(self, item, update=False):
         plugin = item.plugin
 
-        self.tagger.webservice.get(
-            PLUGINS_API['host'],
-            PLUGINS_API['port'],
-            PLUGINS_API['endpoint']['download'],
-            partial(self.download_handler, update, plugin=plugin),
+        self.tagger.webservice.get_url(
+            url=PLUGINS_API['urls']['download'],
+            handler=partial(self.download_handler, update, plugin=plugin),
             parse_response_type=None,
             priority=True,
             important=True,
-            queryargs={"id": plugin.module_name, "version": plugin.version.to_string(short=True)}
+            queryargs={"id": plugin.module_name, "version": plugin.version.to_string(short=True)},
         )
 
     def download_handler(self, update, response, reply, error, plugin):
diff --git a/picard/util/checkupdate.py b/picard/util/checkupdate.py
index cbd8e451a..38d3a840f 100644
--- a/picard/util/checkupdate.py
+++ b/picard/util/checkupdate.py
@@ -87,12 +87,10 @@ class UpdateCheckManager(QtCore.QObject):
 
     def _query_available_updates(self, callback=None):
         """Gets list of releases from specified website api."""
-        log.debug("Getting Picard release information from %s", PLUGINS_API['host'])
-        self.tagger.webservice.get(
-            PLUGINS_API['host'],
-            PLUGINS_API['port'],
-            PLUGINS_API['endpoint']['releases'],
-            partial(self._releases_json_loaded, callback=callback),
+        log.debug("Getting Picard release information from %s", PLUGINS_API['urls']['releases'])
+        self.tagger.webservice.get_url(
+            url=PLUGINS_API['urls']['releases'],
+            handler=partial(self._releases_json_loaded, callback=callback),
             priority=True,
             important=True
         )
@@ -105,9 +103,8 @@ class UpdateCheckManager(QtCore.QObject):
                 QMessageBox.information(
                     self._parent,
                     _("Picard Update"),
-                    _("Unable to retrieve the latest version information from the website.\n(https://{url}{endpoint})").format(
-                        url=PLUGINS_API['host'],
-                        endpoint=PLUGINS_API['endpoint']['releases'],
+                    _("Unable to retrieve the latest version information from the website.\n({url})").format(
+                        url=PLUGINS_API['urls']['releases'],
                     ),
                     QMessageBox.StandardButton.Ok, QMessageBox.StandardButton.Ok)
         else:

From 0eff0e1562240c2b2029202c0e2878f2fa76b4d9 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 21:19:06 +0200
Subject: [PATCH 08/33] CoverArtProviderCaa: use WebService.get_url()

---
 picard/coverart/providers/caa.py | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/picard/coverart/providers/caa.py b/picard/coverart/providers/caa.py
index d65da13fd..2248618a4 100644
--- a/picard/coverart/providers/caa.py
+++ b/picard/coverart/providers/caa.py
@@ -55,6 +55,7 @@ from picard.config import (
 from picard.const import (
     CAA_HOST,
     CAA_PORT,
+    CAA_URL,
 )
 from picard.coverart.image import (
     CaaCoverArtImage,
@@ -581,14 +582,12 @@ class CoverArtProviderCaa(CoverArtProvider):
         return "/release/%s/" % self.metadata["musicbrainz_albumid"]
 
     def queue_images(self):
-        self.album.tagger.webservice.get(
-            CAA_HOST,
-            CAA_PORT,
-            self._caa_path,
-            self._caa_json_downloaded,
+        self.album.tagger.webservice.get_url(
+            url=CAA_URL + self._caa_path,
+            handler=self._caa_json_downloaded,
             priority=True,
             important=False,
-            cacheloadcontrol=QNetworkRequest.CacheLoadControl.PreferNetwork
+            cacheloadcontrol=QNetworkRequest.CacheLoadControl.PreferNetwork,
         )
         self.album._requests += 1
         # we will call next_in_queue() after json parsing

From f7151e17f7f5e237025105a71b765d94a9acea3f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 21:34:26 +0200
Subject: [PATCH 09/33] Move code returning port from QUrl outside WSRequest

- it will be needed elsewhere
- add matching tests
---
 picard/webservice/__init__.py | 13 ++++++++-----
 test/test_webservice.py       | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index dce872384..0f5efa46e 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -91,6 +91,13 @@ class UnknownResponseParserError(Exception):
         super().__init__(message)
 
 
+def port_from_qurl(qurl):
+    """Returns QUrl port or default ports (443 for https, 80 for http)"""
+    if qurl.scheme() == 'https':
+        return qurl.port(443)
+    return qurl.port(80)
+
+
 class WSRequest(QNetworkRequest):
     """Represents a single HTTP request."""
     _access_token = None
@@ -214,11 +221,7 @@ class WSRequest(QNetworkRequest):
 
     @property
     def port(self):
-        """Returns QUrl port or default ports (443 for https, 80 for http)"""
-        url = self.url()
-        if url.scheme() == 'https':
-            return url.port(443)
-        return url.port(80)
+        return port_from_qurl(self.url())
 
     @property
     def path(self):
diff --git a/test/test_webservice.py b/test/test_webservice.py
index 384925c2a..4bd5f3e17 100644
--- a/test/test_webservice.py
+++ b/test/test_webservice.py
@@ -44,6 +44,7 @@ from picard.webservice import (
     UnknownResponseParserError,
     WebService,
     WSRequest,
+    port_from_qurl,
     ratecontrol,
 )
 
@@ -531,3 +532,22 @@ class WSRequestTest(PicardTestCase):
         )
         # FIXME: check encoding
         self.assertEqual(request.url().toString(), 'http://example.org/path?a=1&a=2&b=x x&c=1+2&d=%26&e=?')
+
+
+class MiscWebServiceTest(PicardTestCase):
+
+    def test_port_from_qurl_http(self):
+        self.assertEqual(port_from_qurl(QUrl('http://example.org')), 80)
+
+    def test_port_from_qurl_http_other(self):
+        self.assertEqual(port_from_qurl(QUrl('http://example.org:666')), 666)
+
+    def test_port_from_qurl_https(self):
+        self.assertEqual(port_from_qurl(QUrl('https://example.org')), 443)
+
+    def test_port_from_qurl_https_other(self):
+        self.assertEqual(port_from_qurl(QUrl('https://example.org:666')), 666)
+
+    def test_port_from_qurl_exception(self):
+        with self.assertRaises(AttributeError):
+            port_from_qurl('xxx')

From d10397d30aa46ae5d3df0aa1eac4fc736492b88a Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 21:47:31 +0200
Subject: [PATCH 10/33] Introduce hostkey_from_url()

It calculates (host, port) from passed URL.
With matching tests.
---
 picard/webservice/__init__.py | 7 +++++++
 test/test_webservice.py       | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index 0f5efa46e..fd43304ec 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -98,6 +98,13 @@ def port_from_qurl(qurl):
     return qurl.port(80)
 
 
+def hostkey_from_url(url):
+    """Returns (host, port) from passed url (as string or QUrl)"""
+    if not isinstance(url, QUrl):
+        url = QUrl(url)
+    return (url.host(), port_from_qurl(url))
+
+
 class WSRequest(QNetworkRequest):
     """Represents a single HTTP request."""
     _access_token = None
diff --git a/test/test_webservice.py b/test/test_webservice.py
index 4bd5f3e17..447bffc70 100644
--- a/test/test_webservice.py
+++ b/test/test_webservice.py
@@ -44,6 +44,7 @@ from picard.webservice import (
     UnknownResponseParserError,
     WebService,
     WSRequest,
+    hostkey_from_url,
     port_from_qurl,
     ratecontrol,
 )
@@ -551,3 +552,9 @@ class MiscWebServiceTest(PicardTestCase):
     def test_port_from_qurl_exception(self):
         with self.assertRaises(AttributeError):
             port_from_qurl('xxx')
+
+    def test_hostkey_from_qurl_http(self):
+        self.assertEqual(hostkey_from_url(QUrl('http://example.org')), ('example.org', 80))
+
+    def test_hostkey_from_url_https_other(self):
+        self.assertEqual(hostkey_from_url('https://example.org:666'), ('example.org', 666))

From 8b243e72b647dd6f489561ceb87b6c962a5c3ccf Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Sun, 4 Jun 2023 21:54:46 +0200
Subject: [PATCH 11/33] Get rid of CAA_HOST / CAA_PORT, replaced by CAA_URL

---
 picard/const/__init__.py         |  4 +---
 picard/coverart/providers/caa.py | 15 +++++++--------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/picard/const/__init__.py b/picard/const/__init__.py
index f1d510ec0..d6c12a9f5 100644
--- a/picard/const/__init__.py
+++ b/picard/const/__init__.py
@@ -62,9 +62,7 @@ DEFAULT_FPCALC_THREADS = 2
 MUSICBRAINZ_OAUTH_CLIENT_ID = 'ACa9wsDX19cLp-AeEP-vVw'
 MUSICBRAINZ_OAUTH_CLIENT_SECRET = 'xIsvXbIuntaLuRRhzuazOA'
 
-# Cover art archive URL and port
-CAA_HOST = "coverartarchive.org"
-CAA_PORT = 443
+# Cover art archive URL
 CAA_URL = 'https://coverartarchive.org'
 
 # Prepare documentation URLs
diff --git a/picard/coverart/providers/caa.py b/picard/coverart/providers/caa.py
index 2248618a4..e1bad91d7 100644
--- a/picard/coverart/providers/caa.py
+++ b/picard/coverart/providers/caa.py
@@ -52,11 +52,7 @@ from picard.config import (
     ListOption,
     get_config,
 )
-from picard.const import (
-    CAA_HOST,
-    CAA_PORT,
-    CAA_URL,
-)
+from picard.const import CAA_URL
 from picard.coverart.image import (
     CaaCoverArtImage,
     CaaThumbnailCoverArtImage,
@@ -69,7 +65,10 @@ from picard.coverart.utils import (
     CAA_TYPES,
     translate_caa_type,
 )
-from picard.webservice import ratecontrol
+from picard.webservice import (
+    hostkey_from_url,
+    ratecontrol,
+)
 
 from picard.ui import PicardDialog
 from picard.ui.ui_provider_options_caa import Ui_CaaOptions
@@ -93,8 +92,8 @@ _CAA_IMAGE_SIZE_DEFAULT = 500
 _CAA_IMAGE_TYPE_DEFAULT_INCLUDE = ['front']
 _CAA_IMAGE_TYPE_DEFAULT_EXCLUDE = ['matrix/runout', 'raw/unedited', 'watermark']
 
-ratecontrol.set_minimum_delay((CAA_HOST, CAA_PORT), 0)
-ratecontrol.set_minimum_delay(('archive.org', 443), 0)
+ratecontrol.set_minimum_delay(hostkey_from_url(CAA_URL), 0)
+ratecontrol.set_minimum_delay(hostkey_from_url('https://archive.org'), 0)
 
 
 def caa_url_fallback_list(desired_size, thumbnails):

From 2e54681fd66627bbcefe056425f01ad43f99f332 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Mon, 5 Jun 2023 09:05:33 +0200
Subject: [PATCH 12/33] Introduce host_port_to_url()

This method will help with the move, since we have in many places just host & port (including in Options dialog).

Basically it does its best to convert it to an URL.
---
 picard/webservice/__init__.py | 22 ++++++++++++++++++++++
 test/test_webservice.py       | 16 ++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index fd43304ec..1a277ccba 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -105,6 +105,28 @@ def hostkey_from_url(url):
     return (url.host(), port_from_qurl(url))
 
 
+def host_port_to_url(host, port, path=None, scheme=None, as_string=False):
+    """Convert host & port (with optional path and scheme) to an URL"""
+    url = QUrl()
+    if scheme is None:
+        if port == 443:
+            scheme = 'https'
+        else:
+            scheme = 'http'
+    url.setScheme(scheme)
+
+    if ((scheme == 'https' and port != 443)
+            or (scheme == 'http' and port != 80)):
+        url.setPort(port)
+
+    url.setHost(host)
+
+    if path is not None:
+        url.setPath(path)
+
+    return url.toString() if as_string else url
+
+
 class WSRequest(QNetworkRequest):
     """Represents a single HTTP request."""
     _access_token = None
diff --git a/test/test_webservice.py b/test/test_webservice.py
index 447bffc70..ff0bf55ea 100644
--- a/test/test_webservice.py
+++ b/test/test_webservice.py
@@ -44,6 +44,7 @@ from picard.webservice import (
     UnknownResponseParserError,
     WebService,
     WSRequest,
+    host_port_to_url,
     hostkey_from_url,
     port_from_qurl,
     ratecontrol,
@@ -558,3 +559,18 @@ class MiscWebServiceTest(PicardTestCase):
 
     def test_hostkey_from_url_https_other(self):
         self.assertEqual(hostkey_from_url('https://example.org:666'), ('example.org', 666))
+
+    def test_host_port_to_url_http_80(self):
+        self.assertEqual(host_port_to_url('example.org', 80, as_string=True), 'http://example.org')
+
+    def test_host_port_to_url_http_80_qurl(self):
+        self.assertEqual(host_port_to_url('example.org', 80).toString(), 'http://example.org')
+
+    def test_host_port_to_url_https_443(self):
+        self.assertEqual(host_port_to_url('example.org', 443, as_string=True), 'https://example.org')
+
+    def test_host_port_to_url_https_scheme_80(self):
+        self.assertEqual(host_port_to_url('example.org', 80, scheme='https', as_string=True), 'https://example.org:80')
+
+    def test_host_port_to_url_http_666_with_path(self):
+        self.assertEqual(host_port_to_url('example.org', 666, path='/abc', as_string=True), 'http://example.org:666/abc')

From 6583ba7524cb88c972ae46bd4ee27151f4b0dca8 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Mon, 5 Jun 2023 09:42:43 +0200
Subject: [PATCH 13/33] Convert APIHelper

---
 picard/webservice/api_helpers.py | 53 ++++++++++++++++----------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 88c4977ee..73e7c5788 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -35,7 +35,7 @@ from picard.const import (
 )
 from picard.webservice import (
     CLIENT_STRING,
-    DEFAULT_RESPONSE_PARSER_TYPE,
+    host_port_to_url,
     ratecontrol,
 )
 
@@ -78,35 +78,36 @@ class APIHelper(object):
     def port(self):
         return self._port
 
-    def get(self, path_list, handler, priority=False, important=False, mblogin=False,
-            cacheloadcontrol=None, refresh=False, queryargs=None, parse_response_type=DEFAULT_RESPONSE_PARSER_TYPE):
+    def url_from_path_list(self, path_list):
         path = self.api_path + "/".join(path_list)
-        return self._webservice.get(self.host, self.port, path, handler,
-                                    priority=priority, important=important, mblogin=mblogin,
-                                    refresh=refresh, queryargs=queryargs, parse_response_type=parse_response_type)
+        return host_port_to_url(self.host, self.port, path=path)
 
-    def post(self, path_list, data, handler, priority=False, important=False,
-             mblogin=True, queryargs=None, parse_response_type=DEFAULT_RESPONSE_PARSER_TYPE,
-             request_mimetype=None):
-        path = self.api_path + "/".join(path_list)
-        return self._webservice.post(self.host, self.port, path, data, handler,
-                                     priority=priority, important=important, mblogin=mblogin,
-                                     queryargs=queryargs, parse_response_type=parse_response_type,
-                                     request_mimetype=request_mimetype)
+    def get(self, path_list, handler, **kwargs):
+        kwargs['url'] = self.url_from_path_list(path_list)
+        kwargs['handler'] = handler
+        return self._webservice.get_url(**kwargs)
 
-    def put(self, path_list, data, handler, priority=True, important=False,
-            mblogin=True, queryargs=None, request_mimetype=None):
-        path = self.api_path + "/".join(path_list)
-        return self._webservice.put(self.host, self.port, path, data, handler,
-                                    priority=priority, important=important, mblogin=mblogin,
-                                    queryargs=queryargs, request_mimetype=request_mimetype)
+    def post(self, path_list, data, handler, **kwargs):
+        kwargs['url'] = self.url_from_path_list(path_list)
+        kwargs['handler'] = handler
+        kwargs['data'] = data
+        kwargs['mblogin'] = kwargs.get('mblogin', True)
+        return self._webservice.post_url(**kwargs)
 
-    def delete(self, path_list, handler, priority=True, important=False,
-               mblogin=True, queryargs=None):
-        path = self.api_path + "/".join(path_list)
-        return self._webservice.delete(self.host, self.port, path, handler,
-                                       priority=priority, important=important, mblogin=mblogin,
-                                       queryargs=queryargs)
+    def put(self, path_list, data, handler, **kwargs):
+        kwargs['url'] = self.url_from_path_list(path_list)
+        kwargs['handler'] = handler
+        kwargs['data'] = data
+        kwargs['priority'] = kwargs.get('priority', True)
+        kwargs['mblogin'] = kwargs.get('mblogin', True)
+        return self._webservice.put_url(**kwargs)
+
+    def delete(self, path_list, handler, **kwargs):
+        kwargs['url'] = self.url_from_path_list(path_list)
+        kwargs['handler'] = handler
+        kwargs['priority'] = kwargs.get('priority', True)
+        kwargs['mblogin'] = kwargs.get('mblogin', True)
+        return self._webservice.delete_url(**kwargs)
 
 
 class MBAPIHelper(APIHelper):

From 884b7e1990c5e9ee322393523be6f91e5dab3d8f Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Mon, 5 Jun 2023 10:28:19 +0200
Subject: [PATCH 14/33] Convert MBAPIHelper

---
 picard/webservice/api_helpers.py | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 73e7c5788..593c67eea 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -125,26 +125,18 @@ class MBAPIHelper(APIHelper):
         config = get_config()
         return config.setting['server_port']
 
-    def _get_by_id(self, entitytype, entityid, handler, inc=None, queryargs=None,
-                   priority=False, important=False, mblogin=False, refresh=False):
+    def _get_by_id(self, entitytype, entityid, handler, inc=None, **kwargs):
         path_list = (entitytype, entityid)
-        if queryargs is None:
-            queryargs = {}
         if inc:
-            queryargs["inc"] = "+".join(inc)
-        return self.get(path_list, handler,
-                        priority=priority, important=important, mblogin=mblogin,
-                        refresh=refresh, queryargs=queryargs)
+            kwargs['queryargs'] = kwargs.get('queryargs', {})
+            kwargs['queryargs']["inc"] = "+".join(sorted(set(inc)))
+        return self.get(path_list, handler, **kwargs)
 
-    def get_release_by_id(self, releaseid, handler, inc=None,
-                          priority=False, important=False, mblogin=False, refresh=False):
-        return self._get_by_id('release', releaseid, handler, inc,
-                               priority=priority, important=important, mblogin=mblogin, refresh=refresh)
+    def get_release_by_id(self, releaseid, handler, inc=None, **kwargs):
+        return self._get_by_id('release', releaseid, handler, inc, **kwargs)
 
-    def get_track_by_id(self, trackid, handler, inc=None,
-                        priority=False, important=False, mblogin=False, refresh=False):
-        return self._get_by_id('recording', trackid, handler, inc,
-                               priority=priority, important=important, mblogin=mblogin, refresh=refresh)
+    def get_track_by_id(self, trackid, handler, inc=None, **kwargs):
+        return self._get_by_id('recording', trackid, handler, inc, **kwargs)
 
     def lookup_discid(self, discid, handler, priority=True, important=True, refresh=False):
         inc = ('artist-credits', 'labels')

From a39932c264c3f280e8250baba799eb43f293193b Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Tue, 6 Jun 2023 09:00:23 +0200
Subject: [PATCH 15/33] Introduce APIHelper url property and replace host &
 port properties

- __init__() calls in subclasses are not needed anymore
- base_path class property defines the url path prefix
---
 picard/webservice/api_helpers.py | 44 ++++++++++++--------------------
 1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 593c67eea..5b53d82c9 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -59,11 +59,9 @@ def _wrap_xml_metadata(data):
 
 
 class APIHelper(object):
+    base_path = "/"
 
-    def __init__(self, host, port, api_path, webservice):
-        self._host = host
-        self._port = port
-        self.api_path = api_path
+    def __init__(self, webservice):
         self._webservice = webservice
 
     @property
@@ -71,16 +69,13 @@ class APIHelper(object):
         return self._webservice
 
     @property
-    def host(self):
-        return self._host
-
-    @property
-    def port(self):
-        return self._port
+    def url(self):
+        raise NotImplementedError
 
     def url_from_path_list(self, path_list):
-        path = self.api_path + "/".join(path_list)
-        return host_port_to_url(self.host, self.port, path=path)
+        url = self.url
+        url.setPath("/".join([self.base_path] + list(path_list)))
+        return url
 
     def get(self, path_list, handler, **kwargs):
         kwargs['url'] = self.url_from_path_list(path_list)
@@ -111,19 +106,14 @@ class APIHelper(object):
 
 
 class MBAPIHelper(APIHelper):
-
-    def __init__(self, webservice):
-        super().__init__(None, None, "/ws/2/", webservice)
+    base_path = '/ws/2'
 
     @property
-    def host(self):
+    def url(self):
         config = get_config()
-        return config.setting['server_host']
-
-    @property
-    def port(self):
-        config = get_config()
-        return config.setting['server_port']
+        host = config.setting['server_host']
+        port = config.setting['server_port']
+        return host_port_to_url(host, port)
 
     def _get_by_id(self, entitytype, entityid, handler, inc=None, **kwargs):
         path_list = (entitytype, entityid)
@@ -258,15 +248,13 @@ class MBAPIHelper(APIHelper):
 
 class AcoustIdAPIHelper(APIHelper):
 
-    acoustid_host = ACOUSTID_HOST
-    acoustid_port = ACOUSTID_PORT
+    base_path = '/v2'
     client_key = ACOUSTID_KEY
     client_version = PICARD_VERSION_STR
 
-    def __init__(self, webservice):
-        super().__init__(
-            self.acoustid_host, self.acoustid_port, '/v2/', webservice
-        )
+    @property
+    def url(self):
+        return host_port_to_url(ACOUSTID_HOST, ACOUSTID_PORT)
 
     def _encode_acoustid_args(self, args):
         filters = []

From 886cc15ef380a25fd8b5e20be80bb5f9332d18d4 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Tue, 6 Jun 2023 13:56:29 +0200
Subject: [PATCH 16/33] Drop ACOUSTID_HOST, ACOUSTID_PORT in favor of new
 ACOUSTID_URL

---
 picard/const/__init__.py         | 3 +--
 picard/webservice/api_helpers.py | 8 ++++----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/picard/const/__init__.py b/picard/const/__init__.py
index d6c12a9f5..9aec88bcc 100644
--- a/picard/const/__init__.py
+++ b/picard/const/__init__.py
@@ -53,8 +53,7 @@ CACHE_SIZE_IN_BYTES = 100*1000*1000
 
 # AcoustID client API key
 ACOUSTID_KEY = 'v8pQ6oyB'
-ACOUSTID_HOST = 'api.acoustid.org'
-ACOUSTID_PORT = 443
+ACOUSTID_URL = 'https://api.acoustid.org'
 FPCALC_NAMES = ['fpcalc', 'pyfpcalc']
 DEFAULT_FPCALC_THREADS = 2
 
diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 5b53d82c9..9c01d63b4 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -29,18 +29,18 @@ from PyQt5.QtCore import QUrl
 from picard import PICARD_VERSION_STR
 from picard.config import get_config
 from picard.const import (
-    ACOUSTID_HOST,
     ACOUSTID_KEY,
-    ACOUSTID_PORT,
+    ACOUSTID_URL,
 )
 from picard.webservice import (
     CLIENT_STRING,
     host_port_to_url,
+    hostkey_from_url,
     ratecontrol,
 )
 
 
-ratecontrol.set_minimum_delay((ACOUSTID_HOST, ACOUSTID_PORT), 333)
+ratecontrol.set_minimum_delay(hostkey_from_url(ACOUSTID_URL), 333)
 
 
 def escape_lucene_query(text):
@@ -254,7 +254,7 @@ class AcoustIdAPIHelper(APIHelper):
 
     @property
     def url(self):
-        return host_port_to_url(ACOUSTID_HOST, ACOUSTID_PORT)
+        return QUrl(ACOUSTID_URL)
 
     def _encode_acoustid_args(self, args):
         filters = []

From 16ff3c5c978d2ff872267c93bb80b10e29fe08f0 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Tue, 6 Jun 2023 14:39:31 +0200
Subject: [PATCH 17/33] Move few functions to new webservice.utils and
 introduce set_minimum_delay_for_url()

- it limits the need to import hostkey_from_url()
- it reduces the size of webservice.py
- it reduces dependencies between files
---
 picard/coverart/providers/caa.py |  9 ++---
 picard/webservice/__init__.py    | 37 +-----------------
 picard/webservice/api_helpers.py |  5 +--
 picard/webservice/ratecontrol.py |  9 +++++
 picard/webservice/utils.py       | 67 ++++++++++++++++++++++++++++++++
 test/test_webservice.py          |  6 ++-
 6 files changed, 86 insertions(+), 47 deletions(-)
 create mode 100644 picard/webservice/utils.py

diff --git a/picard/coverart/providers/caa.py b/picard/coverart/providers/caa.py
index e1bad91d7..1ddf938d9 100644
--- a/picard/coverart/providers/caa.py
+++ b/picard/coverart/providers/caa.py
@@ -65,10 +65,7 @@ from picard.coverart.utils import (
     CAA_TYPES,
     translate_caa_type,
 )
-from picard.webservice import (
-    hostkey_from_url,
-    ratecontrol,
-)
+from picard.webservice import ratecontrol
 
 from picard.ui import PicardDialog
 from picard.ui.ui_provider_options_caa import Ui_CaaOptions
@@ -92,8 +89,8 @@ _CAA_IMAGE_SIZE_DEFAULT = 500
 _CAA_IMAGE_TYPE_DEFAULT_INCLUDE = ['front']
 _CAA_IMAGE_TYPE_DEFAULT_EXCLUDE = ['matrix/runout', 'raw/unedited', 'watermark']
 
-ratecontrol.set_minimum_delay(hostkey_from_url(CAA_URL), 0)
-ratecontrol.set_minimum_delay(hostkey_from_url('https://archive.org'), 0)
+ratecontrol.set_minimum_delay_for_url(CAA_URL, 0)
+ratecontrol.set_minimum_delay_for_url('https://archive.org', 0)
 
 
 def caa_url_fallback_list(desired_size, thumbnails):
diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index 1a277ccba..6fad8f9e7 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -64,6 +64,7 @@ from picard.util import (
 )
 from picard.util.xml import parse_xml
 from picard.webservice import ratecontrol
+from picard.webservice.utils import port_from_qurl
 
 
 COUNT_REQUESTS_DELAY_MS = 250
@@ -91,42 +92,6 @@ class UnknownResponseParserError(Exception):
         super().__init__(message)
 
 
-def port_from_qurl(qurl):
-    """Returns QUrl port or default ports (443 for https, 80 for http)"""
-    if qurl.scheme() == 'https':
-        return qurl.port(443)
-    return qurl.port(80)
-
-
-def hostkey_from_url(url):
-    """Returns (host, port) from passed url (as string or QUrl)"""
-    if not isinstance(url, QUrl):
-        url = QUrl(url)
-    return (url.host(), port_from_qurl(url))
-
-
-def host_port_to_url(host, port, path=None, scheme=None, as_string=False):
-    """Convert host & port (with optional path and scheme) to an URL"""
-    url = QUrl()
-    if scheme is None:
-        if port == 443:
-            scheme = 'https'
-        else:
-            scheme = 'http'
-    url.setScheme(scheme)
-
-    if ((scheme == 'https' and port != 443)
-            or (scheme == 'http' and port != 80)):
-        url.setPort(port)
-
-    url.setHost(host)
-
-    if path is not None:
-        url.setPath(path)
-
-    return url.toString() if as_string else url
-
-
 class WSRequest(QNetworkRequest):
     """Represents a single HTTP request."""
     _access_token = None
diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 9c01d63b4..ff502a137 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -34,13 +34,12 @@ from picard.const import (
 )
 from picard.webservice import (
     CLIENT_STRING,
-    host_port_to_url,
-    hostkey_from_url,
     ratecontrol,
 )
+from picard.webservice.utils import host_port_to_url
 
 
-ratecontrol.set_minimum_delay(hostkey_from_url(ACOUSTID_URL), 333)
+ratecontrol.set_minimum_delay_for_url(ACOUSTID_URL, 333)
 
 
 def escape_lucene_query(text):
diff --git a/picard/webservice/ratecontrol.py b/picard/webservice/ratecontrol.py
index 0235bb024..8e9248270 100644
--- a/picard/webservice/ratecontrol.py
+++ b/picard/webservice/ratecontrol.py
@@ -29,6 +29,7 @@ import sys
 import time
 
 from picard import log
+from picard.webservice.utils import hostkey_from_url
 
 
 # ============================================================================
@@ -85,6 +86,14 @@ def set_minimum_delay(hostkey, delay_ms):
     REQUEST_DELAY_MINIMUM[hostkey] = delay_ms
 
 
+def set_minimum_delay_for_url(url, delay_ms):
+    """Set the minimun delay between requests
+            url will be converted to an unique key (host, port)
+            delay_ms is the delay in milliseconds
+    """
+    set_minimum_delay(hostkey_from_url(url), delay_ms)
+
+
 def current_delay(hostkey):
     """Returns the current delay (adaptive) between requests for this hostkey
             hostkey is an unique key, for example (host, port)
diff --git a/picard/webservice/utils.py b/picard/webservice/utils.py
new file mode 100644
index 000000000..272208da0
--- /dev/null
+++ b/picard/webservice/utils.py
@@ -0,0 +1,67 @@
+# -*- coding: utf-8 -*-
+#
+# Picard, the next-generation MusicBrainz tagger
+#
+# Copyright (C) 2007 Lukáš Lalinský
+# Copyright (C) 2009 Carlin Mangar
+# Copyright (C) 2017 Sambhav Kothari
+# Copyright (C) 2018-2022 Philipp Wolfer
+# Copyright (C) 2018-2023 Laurent Monin
+# Copyright (C) 2021 Tche333
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+
+"""
+Asynchronous web service utilities.
+"""
+
+from PyQt5.QtCore import QUrl
+
+
+def port_from_qurl(qurl):
+    """Returns QUrl port or default ports (443 for https, 80 for http)"""
+    if qurl.scheme() == 'https':
+        return qurl.port(443)
+    return qurl.port(80)
+
+
+def hostkey_from_url(url):
+    """Returns (host, port) from passed url (as string or QUrl)"""
+    if not isinstance(url, QUrl):
+        url = QUrl(url)
+    return (url.host(), port_from_qurl(url))
+
+
+def host_port_to_url(host, port, path=None, scheme=None, as_string=False):
+    """Convert host & port (with optional path and scheme) to an URL"""
+    url = QUrl()
+    if scheme is None:
+        if port == 443:
+            scheme = 'https'
+        else:
+            scheme = 'http'
+    url.setScheme(scheme)
+
+    if ((scheme == 'https' and port != 443)
+            or (scheme == 'http' and port != 80)):
+        url.setPort(port)
+
+    url.setHost(host)
+
+    if path is not None:
+        url.setPath(path)
+
+    return url.toString() if as_string else url
diff --git a/test/test_webservice.py b/test/test_webservice.py
index ff0bf55ea..66113f775 100644
--- a/test/test_webservice.py
+++ b/test/test_webservice.py
@@ -44,10 +44,12 @@ from picard.webservice import (
     UnknownResponseParserError,
     WebService,
     WSRequest,
+    ratecontrol,
+)
+from picard.webservice.utils import (
     host_port_to_url,
     hostkey_from_url,
     port_from_qurl,
-    ratecontrol,
 )
 
 
@@ -536,7 +538,7 @@ class WSRequestTest(PicardTestCase):
         self.assertEqual(request.url().toString(), 'http://example.org/path?a=1&a=2&b=x x&c=1+2&d=%26&e=?')
 
 
-class MiscWebServiceTest(PicardTestCase):
+class WebServiceUtilsTest(PicardTestCase):
 
     def test_port_from_qurl_http(self):
         self.assertEqual(port_from_qurl(QUrl('http://example.org')), 80)

From f876fc0403922537dec79de877c3febac1c57be4 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Wed, 7 Jun 2023 22:03:51 +0200
Subject: [PATCH 18/33] CoverArtBox: simplify code using
 WebService.download_url()

---
 picard/ui/coverartbox.py | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/picard/ui/coverartbox.py b/picard/ui/coverartbox.py
index 893b30c58..54f390d73 100644
--- a/picard/ui/coverartbox.py
+++ b/picard/ui/coverartbox.py
@@ -465,20 +465,12 @@ class CoverArtBox(QtWidgets.QGroupBox):
             self.load_remote_image(url, fallback_data)
 
         if url.scheme() in {'http', 'https'}:
-            path = url.path()
-            if url.hasQuery():
-                query = QtCore.QUrlQuery(url.query())
-                queryargs = dict(query.queryItems())
-            else:
-                queryargs = {}
-            if url.scheme() == 'https':
-                port = 443
-            else:
-                port = 80
-            self.tagger.webservice.get(url.host(), url.port(port), path,
-                                       partial(self.on_remote_image_fetched, url, fallback_data=fallback_data),
-                                       parse_response_type=None, queryargs=queryargs,
-                                       priority=True, important=True)
+            self.tagger.webservice.download_url(
+                url=url,
+                handler=partial(self.on_remote_image_fetched, url, fallback_data=fallback_data),
+                priority=True,
+                important=True,
+            )
         elif url.scheme() == 'file':
             path = normpath(url.toLocalFile().rstrip("\0"))
             if path and os.path.exists(path):

From 60778cd9feae20583a2a4f6ec1e8083e11996e1c Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 00:12:33 +0200
Subject: [PATCH 19/33] Mark WebService.(get|post|put|delete|download)()
 methods as deprecated

- log a warning
---
 picard/webservice/__init__.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index 6fad8f9e7..8d3472f5d 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -567,6 +567,7 @@ class WebService(QtCore.QObject):
     def get(self, host, port, path, handler, parse_response_type=DEFAULT_RESPONSE_PARSER_TYPE,
             priority=False, important=False, mblogin=False, cacheloadcontrol=None, refresh=False,
             queryargs=None):
+        log.warning("This method is deprecated, use WebService.get_url() instead")
         request = WSRequest(
             method='GET',
             url=build_qurl(host, port, path=path, queryargs=queryargs),
@@ -582,6 +583,7 @@ class WebService(QtCore.QObject):
 
     def post(self, host, port, path, data, handler, parse_response_type=DEFAULT_RESPONSE_PARSER_TYPE,
              priority=False, important=False, mblogin=True, queryargs=None, request_mimetype=None):
+        log.warning("This method is deprecated, use WebService.post_url() instead")
         request = WSRequest(
             method='POST',
             url=build_qurl(host, port, path=path, queryargs=queryargs),
@@ -598,6 +600,7 @@ class WebService(QtCore.QObject):
 
     def put(self, host, port, path, data, handler, priority=True, important=False, mblogin=True,
             queryargs=None, request_mimetype=None):
+        log.warning("This method is deprecated, use WebService.put_url() instead")
         request = WSRequest(
             method='PUT',
             url=build_qurl(host, port, path=path, queryargs=queryargs),
@@ -612,6 +615,7 @@ class WebService(QtCore.QObject):
 
     def delete(self, host, port, path, handler, priority=True, important=False, mblogin=True,
                queryargs=None):
+        log.warning("This method is deprecated, use WebService.delete_url() instead")
         request = WSRequest(
             method='DELETE',
             url=build_qurl(host, port, path=path, queryargs=queryargs),
@@ -625,6 +629,7 @@ class WebService(QtCore.QObject):
     def download(self, host, port, path, handler, priority=False,
                  important=False, cacheloadcontrol=None, refresh=False,
                  queryargs=None):
+        log.warning("This method is deprecated, use WebService.download_url() instead")
         request = WSRequest(
             method='GET',
             url=build_qurl(host, port, path=path, queryargs=queryargs),

From dffb7cd7db5102545a686003b42409570ed3ff6c Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 11:59:09 +0200
Subject: [PATCH 20/33] WSRequest: make it clear queryargs should consist of
 encoded arguments

---
 picard/webservice/__init__.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index 8d3472f5d..8fe2dcd08 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -136,7 +136,7 @@ class WSRequest(QNetworkRequest):
             important: Indicates that this is an important request.
             request_mimetype: Set the Content-Type header.
             url: URL passed as a string or as a QUrl to use for this request
-            queryargs: dictionary of keys and values to add to the url
+            queryargs: Encoded query arguments, a dictionary mapping field names to values
         """
         # mandatory parameters
         self.method = method
@@ -156,7 +156,6 @@ class WSRequest(QNetworkRequest):
         if queryargs is not None:
             query = QtCore.QUrlQuery(url)
             for k, v in queryargs.items():
-                # FIXME: check if encoding is correct
                 query.addQueryItem(k, str(v))
             url.setQuery(query)
 

From 944a92eecdeb418c6aa5f260ccf3aec4ae38a6e4 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 12:41:58 +0200
Subject: [PATCH 21/33] Add an helper method to encode query arguments:
 encoded_queryargs()

---
 picard/util/__init__.py | 11 +++++++++++
 test/test_utils.py      | 10 ++++++++++
 2 files changed, 21 insertions(+)

diff --git a/picard/util/__init__.py b/picard/util/__init__.py
index d4b8662e3..579018937 100644
--- a/picard/util/__init__.py
+++ b/picard/util/__init__.py
@@ -676,6 +676,17 @@ def album_artist_from_path(filename, album, artist):
     return album, artist
 
 
+def encoded_queryargs(queryargs):
+    """
+    Percent-encode all values from passed dictionary
+    Keys are left unmodified
+    """
+    return {
+        name: bytes(QtCore.QUrl.toPercentEncoding(str(value))).decode()
+        for name, value in queryargs.items()
+    }
+
+
 def build_qurl(host, port=80, path=None, queryargs=None):
     """
     Builds and returns a QUrl object from `host`, `port` and `path` and
diff --git a/test/test_utils.py b/test/test_utils.py
index 3b7efc385..0077e878a 100644
--- a/test/test_utils.py
+++ b/test/test_utils.py
@@ -58,6 +58,7 @@ from picard.util import (
     any_exception_isinstance,
     build_qurl,
     detect_unicode_encoding,
+    encoded_queryargs,
     extract_year_from_date,
     find_best_match,
     is_absolute_path,
@@ -755,6 +756,15 @@ class BuildQUrlTest(PicardTestCase):
             self.assertEqual(expected, build_qurl(host, port=443).toDisplayString())
             self.assertEqual(expected, build_qurl(host, port=8080).toDisplayString())
 
+    def test_encoded_queryargs(self):
+        query = encoded_queryargs({'foo': ' %20&;', 'bar': '=%+?abc'})
+        self.assertEqual('%20%2520%26%3B', query['foo'])
+        self.assertEqual('%3D%25%2B%3Fabc', query['bar'])
+        # spaces are decoded in displayed string
+        expected = 'http://example.com?foo= %2520%26%3B&bar=%3D%25%2B%3Fabc'
+        result = build_qurl('example.com', queryargs=query).toDisplayString()
+        self.assertEqual(expected, result)
+
 
 class NormpathTest(PicardTestCase):
 

From 59550aa366a85afbd969631b5e253da9aeed226a Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 12:58:38 +0200
Subject: [PATCH 22/33] Simplify code using encoded_queryargs()

---
 picard/webservice/api_helpers.py | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index ff502a137..53890d2fd 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -32,6 +32,7 @@ from picard.const import (
     ACOUSTID_KEY,
     ACOUSTID_URL,
 )
+from picard.util import encoded_queryargs
 from picard.webservice import (
     CLIENT_STRING,
     ratecontrol,
@@ -133,11 +134,11 @@ class MBAPIHelper(APIHelper):
                                priority=priority, important=important, refresh=refresh)
 
     def _find(self, entitytype, handler, **kwargs):
-        filters = []
+        filters = {}
 
         limit = kwargs.pop("limit")
         if limit:
-            filters.append(("limit", limit))
+            filters['limit'] = limit
 
         is_search = kwargs.pop("search", False)
         if is_search:
@@ -147,20 +148,15 @@ class MBAPIHelper(APIHelper):
                 query = kwargs["query"]
             else:
                 query = escape_lucene_query(kwargs["query"]).strip().lower()
-                filters.append(("dismax", 'true'))
+                filters['dismax'] = 'true'
         else:
             query = build_lucene_query(kwargs)
 
         if query:
-            filters.append(("query", query))
-
-        queryargs = {
-            name: bytes(QUrl.toPercentEncoding(str(value))).decode()
-            for name, value in filters
-        }
+            filters['query'] = query
 
         path_list = (entitytype, )
-        return self.get(path_list, handler, queryargs=queryargs,
+        return self.get(path_list, handler, queryargs=encoded_queryargs(filters),
                         priority=True, important=True, mblogin=False,
                         refresh=False)
 
@@ -256,14 +252,10 @@ class AcoustIdAPIHelper(APIHelper):
         return QUrl(ACOUSTID_URL)
 
     def _encode_acoustid_args(self, args):
-        filters = []
         args['client'] = self.client_key
         args['clientversion'] = self.client_version
         args['format'] = 'json'
-        for name, value in args.items():
-            value = bytes(QUrl.toPercentEncoding(value)).decode()
-            filters.append('%s=%s' % (name, value))
-        return '&'.join(filters)
+        return '&'.join((k + '=' + v for k, v in encoded_queryargs(args).items()))
 
     def query_acoustid(self, handler, **args):
         path_list = ('lookup', )

From c7acf9ea5b1dfd8fb4607836b4aa5efa169e05c4 Mon Sep 17 00:00:00 2001
From: Philipp Wolfer <ph.wolfer@gmail.com>
Date: Thu, 8 Jun 2023 11:50:43 +0200
Subject: [PATCH 23/33] Use base_url in APIHelper instead of separate url and
 base_path properties

---
 picard/webservice/api_helpers.py | 34 +++++++-------
 test/test_api_helpers.py         | 76 ++++++++++++++++----------------
 2 files changed, 53 insertions(+), 57 deletions(-)

diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 53890d2fd..3450f8b59 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -3,8 +3,8 @@
 # Picard, the next-generation MusicBrainz tagger
 #
 # Copyright (C) 2017 Sambhav Kothari
-# Copyright (C) 2018, 2020-2021 Laurent Monin
-# Copyright (C) 2018-2022 Philipp Wolfer
+# Copyright (C) 2018, 2020-2021, 2023 Laurent Monin
+# Copyright (C) 2018-2023 Philipp Wolfer
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -59,22 +59,22 @@ def _wrap_xml_metadata(data):
 
 
 class APIHelper(object):
-    base_path = "/"
 
-    def __init__(self, webservice):
+    def __init__(self, webservice, base_url=None):
         self._webservice = webservice
+        if not base_url:
+            raise ValueError("base_url is required")
+        elif not isinstance(base_url, QUrl):
+            base_url = QUrl(base_url)
+        self._base_url = base_url
 
     @property
     def webservice(self):
         return self._webservice
 
-    @property
-    def url(self):
-        raise NotImplementedError
-
     def url_from_path_list(self, path_list):
-        url = self.url
-        url.setPath("/".join([self.base_path] + list(path_list)))
+        url = QUrl(self._base_url)
+        url.setPath("/".join([self._base_url.path()] + list(path_list)))
         return url
 
     def get(self, path_list, handler, **kwargs):
@@ -106,14 +106,14 @@ class APIHelper(object):
 
 
 class MBAPIHelper(APIHelper):
-    base_path = '/ws/2'
 
-    @property
-    def url(self):
+    def __init__(self, webservice):
         config = get_config()
         host = config.setting['server_host']
         port = config.setting['server_port']
-        return host_port_to_url(host, port)
+        base_url = host_port_to_url(host, port)
+        base_url.setPath('/ws/2')
+        super().__init__(webservice, base_url=base_url)
 
     def _get_by_id(self, entitytype, entityid, handler, inc=None, **kwargs):
         path_list = (entitytype, entityid)
@@ -243,13 +243,11 @@ class MBAPIHelper(APIHelper):
 
 class AcoustIdAPIHelper(APIHelper):
 
-    base_path = '/v2'
     client_key = ACOUSTID_KEY
     client_version = PICARD_VERSION_STR
 
-    @property
-    def url(self):
-        return QUrl(ACOUSTID_URL)
+    def __init__(self, webservice):
+        super().__init__(webservice, base_url=ACOUSTID_URL + '/v2')
 
     def _encode_acoustid_args(self, args):
         args['client'] = self.client_key
diff --git a/test/test_api_helpers.py b/test/test_api_helpers.py
index 45848282e..e727c07a3 100644
--- a/test/test_api_helpers.py
+++ b/test/test_api_helpers.py
@@ -4,8 +4,8 @@
 #
 # Copyright (C) 2017 Sambhav Kothari
 # Copyright (C) 2018 Wieland Hoffmann
-# Copyright (C) 2018, 2020-2021 Laurent Monin
-# Copyright (C) 2019-2022 Philipp Wolfer
+# Copyright (C) 2018, 2020-2021, 2023 Laurent Monin
+# Copyright (C) 2019-2023 Philipp Wolfer
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -24,6 +24,8 @@
 
 from unittest.mock import MagicMock
 
+from PyQt5.QtCore import QUrl
+
 from test.picardtestcase import PicardTestCase
 
 from picard.acoustid.manager import Submission
@@ -42,35 +44,31 @@ class APITest(PicardTestCase):
 
     def setUp(self):
         super().setUp()
-        self.host = "abc.com"
-        self.port = 80
-        self.api_path = "/v1/"
+        base_url = "http://abc.com/v1/"
         self.path_list = ['test', 'more', 'test']
-        self.complete_path = "/v1/test/more/test"
+        self.complete_url = QUrl(base_url + "/test/more/test")
         self.ws = MagicMock(auto_spec=WebService)
-        self.api = APIHelper(self.host, self.port, self.api_path, self.ws)
+        self.api = APIHelper(self.ws, base_url=base_url)
 
     def _test_ws_function_args(self, ws_function):
         self.assertGreater(ws_function.call_count, 0)
-        self.assertEqual(ws_function.call_args[0][0], self.host)
-        self.assertEqual(ws_function.call_args[0][1], self.port)
-        self.assertEqual(ws_function.call_args[0][2], self.complete_path)
+        self.assertEqual(ws_function.call_args[1]['url'], self.complete_url)
 
     def test_get(self):
         self.api.get(self.path_list, None)
-        self._test_ws_function_args(self.ws.get)
+        self._test_ws_function_args(self.ws.get_url)
 
     def test_post(self):
         self.api.post(self.path_list, None, None)
-        self._test_ws_function_args(self.ws.post)
+        self._test_ws_function_args(self.ws.post_url)
 
     def test_put(self):
         self.api.put(self.path_list, None, None)
-        self._test_ws_function_args(self.ws.put)
+        self._test_ws_function_args(self.ws.put_url)
 
     def test_delete(self):
         self.api.delete(self.path_list, None)
-        self._test_ws_function_args(self.ws.delete)
+        self._test_ws_function_args(self.ws.delete_url)
 
 
 class MBAPITest(PicardTestCase):
@@ -84,15 +82,15 @@ class MBAPITest(PicardTestCase):
 
     def _test_ws_function_args(self, ws_function):
         self.assertGreater(ws_function.call_count, 0)
-        self.assertEqual(ws_function.call_args[0][0], self.config['server_host'])
-        self.assertEqual(ws_function.call_args[0][1], self.config['server_port'])
-        self.assertIn("/ws/2/", ws_function.call_args[0][2])
+        url = ws_function.call_args[1]['url']
+        self.assertTrue(url.toString().startswith("https://mb.org/"))
+        self.assertTrue(url.path().startswith("/ws/2/"))
 
     def assertInPath(self, ws_function, path):
-        self.assertIn(path, ws_function.call_args[0][2])
+        self.assertIn(path, ws_function.call_args[1]['url'].path())
 
     def assertNotInPath(self, ws_function, path):
-        self.assertNotIn(path, ws_function.call_args[0][2])
+        self.assertNotIn(path, ws_function.call_args[1]['url'].path())
 
     def assertInQuery(self, ws_function, argname, value=None):
         query_args = ws_function.call_args[1]['queryargs']
@@ -100,48 +98,48 @@ class MBAPITest(PicardTestCase):
         self.assertEqual(value, query_args[argname])
 
     def _test_inc_args(self, ws_function, arg_list):
-        self.assertInQuery(self.ws.get, 'inc', "+".join(arg_list))
+        self.assertInQuery(self.ws.get_url, 'inc', "+".join(arg_list))
 
     def test_get_release(self):
         inc_args_list = ['test']
         self.api.get_release_by_id("1", None, inc=inc_args_list)
-        self._test_ws_function_args(self.ws.get)
-        self.assertInPath(self.ws.get, "/release/1")
-        self._test_inc_args(self.ws.get, inc_args_list)
+        self._test_ws_function_args(self.ws.get_url)
+        self.assertInPath(self.ws.get_url, "/release/1")
+        self._test_inc_args(self.ws.get_url, inc_args_list)
 
     def test_get_track(self):
         inc_args_list = ['test']
         self.api.get_track_by_id("1", None, inc=inc_args_list)
-        self._test_ws_function_args(self.ws.get)
-        self.assertInPath(self.ws.get, "/recording/1")
-        self._test_inc_args(self.ws.get, inc_args_list)
+        self._test_ws_function_args(self.ws.get_url)
+        self.assertInPath(self.ws.get_url, "/recording/1")
+        self._test_inc_args(self.ws.get_url, inc_args_list)
 
     def test_get_collection(self):
         inc_args_list = ["releases", "artist-credits", "media"]
         self.api.get_collection("1", None)
-        self._test_ws_function_args(self.ws.get)
-        self.assertInPath(self.ws.get, "collection")
-        self.assertInPath(self.ws.get, "1/releases")
-        self._test_inc_args(self.ws.get, inc_args_list)
+        self._test_ws_function_args(self.ws.get_url)
+        self.assertInPath(self.ws.get_url, "collection")
+        self.assertInPath(self.ws.get_url, "1/releases")
+        self._test_inc_args(self.ws.get_url, inc_args_list)
 
     def test_get_collection_list(self):
         self.api.get_collection_list(None)
-        self._test_ws_function_args(self.ws.get)
-        self.assertInPath(self.ws.get, "collection")
-        self.assertNotInPath(self.ws.get, "releases")
+        self._test_ws_function_args(self.ws.get_url)
+        self.assertInPath(self.ws.get_url, "collection")
+        self.assertNotInPath(self.ws.get_url, "releases")
 
     def test_put_collection(self):
         self.api.put_to_collection("1", ["1", "2", "3"], None)
-        self._test_ws_function_args(self.ws.put)
-        self.assertInPath(self.ws.put, "collection/1/releases/1;2;3")
+        self._test_ws_function_args(self.ws.put_url)
+        self.assertInPath(self.ws.put_url, "collection/1/releases/1;2;3")
 
     def test_delete_collection(self):
         self.api.delete_from_collection("1", ["1", "2", "3", "4"] * 200, None)
         collection_string = ";".join(["1", "2", "3", "4"] * 100)
-        self._test_ws_function_args(self.ws.delete)
-        self.assertInPath(self.ws.delete, "collection/1/releases/" + collection_string)
-        self.assertNotInPath(self.ws.delete, collection_string + ";" + collection_string)
-        self.assertEqual(self.ws.delete.call_count, 2)
+        self._test_ws_function_args(self.ws.delete_url)
+        self.assertInPath(self.ws.delete_url, "collection/1/releases/" + collection_string)
+        self.assertNotInPath(self.ws.delete_url, collection_string + ";" + collection_string)
+        self.assertEqual(self.ws.delete_url.call_count, 2)
 
     def test_xml_ratings_empty(self):
         ratings = dict()

From ab7ab8a431b221bf81382f823639930016a6fbe1 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 13:59:15 +0200
Subject: [PATCH 24/33] WSRequest: add unencoded_queryargs parameters

It works the same as queryargs, but it encodes values
---
 picard/webservice/__init__.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index 8fe2dcd08..854698ce5 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -60,6 +60,7 @@ from picard.oauth import OAuthManager
 from picard.util import (
     build_qurl,
     bytes2human,
+    encoded_queryargs,
     parse_json,
 )
 from picard.util.xml import parse_xml
@@ -117,6 +118,7 @@ class WSRequest(QNetworkRequest):
         request_mimetype=None,
         url=None,
         queryargs=None,
+        unencoded_queryargs=None,
     ):
         """
         Args:
@@ -137,6 +139,7 @@ class WSRequest(QNetworkRequest):
             request_mimetype: Set the Content-Type header.
             url: URL passed as a string or as a QUrl to use for this request
             queryargs: Encoded query arguments, a dictionary mapping field names to values
+            unencoded_queryargs: Unencoded query arguments, a dictionary mapping field names to values
         """
         # mandatory parameters
         self.method = method
@@ -153,7 +156,11 @@ class WSRequest(QNetworkRequest):
         if not isinstance(url, QUrl):
             url = QUrl(url)
 
-        if queryargs is not None:
+        if queryargs is not None or unencoded_queryargs is not None:
+            if queryargs is None:
+                queryargs = {}
+            if unencoded_queryargs:
+                queryargs.update(encoded_queryargs(unencoded_queryargs))
             query = QtCore.QUrlQuery(url)
             for k, v in queryargs.items():
                 query.addQueryItem(k, str(v))

From efde79a5e19f51718226e44d3f0649df1bc12fcd Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 13:59:56 +0200
Subject: [PATCH 25/33] API Helpers: use unencoded_queryargs

---
 picard/webservice/__init__.py    |  4 +---
 picard/webservice/api_helpers.py | 16 ++++++++--------
 test/test_api_helpers.py         |  6 +++---
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index 854698ce5..75ab1a961 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -76,9 +76,7 @@ USER_AGENT_STRING = '%s-%s/%s (%s;%s-%s)' % (PICARD_ORG_NAME, PICARD_APP_NAME,
                                              platform.platform(),
                                              platform.python_implementation(),
                                              platform.python_version())
-CLIENT_STRING = bytes(QUrl.toPercentEncoding('%s %s-%s' % (PICARD_ORG_NAME,
-                                                           PICARD_APP_NAME,
-                                                           PICARD_VERSION_STR))).decode()
+CLIENT_STRING = '%s %s-%s' % (PICARD_ORG_NAME, PICARD_APP_NAME, PICARD_VERSION_STR)
 
 
 DEFAULT_RESPONSE_PARSER_TYPE = "json"
diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 3450f8b59..03de41868 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -118,8 +118,8 @@ class MBAPIHelper(APIHelper):
     def _get_by_id(self, entitytype, entityid, handler, inc=None, **kwargs):
         path_list = (entitytype, entityid)
         if inc:
-            kwargs['queryargs'] = kwargs.get('queryargs', {})
-            kwargs['queryargs']["inc"] = "+".join(sorted(set(inc)))
+            kwargs['unencoded_queryargs'] = kwargs.get('queryargs', {})
+            kwargs['unencoded_queryargs']["inc"] = "+".join(sorted(set(inc)))
         return self.get(path_list, handler, **kwargs)
 
     def get_release_by_id(self, releaseid, handler, inc=None, **kwargs):
@@ -156,7 +156,7 @@ class MBAPIHelper(APIHelper):
             filters['query'] = query
 
         path_list = (entitytype, )
-        return self.get(path_list, handler, queryargs=encoded_queryargs(filters),
+        return self.get(path_list, handler, unencoded_queryargs=filters,
                         priority=True, important=True, mblogin=False,
                         refresh=False)
 
@@ -175,7 +175,7 @@ class MBAPIHelper(APIHelper):
             queryargs = {}
         if inc:
             queryargs["inc"] = "+".join(inc)
-        return self.get(path_list, handler, queryargs=queryargs,
+        return self.get(path_list, handler, unencoded_queryargs=queryargs,
                         priority=True, important=True, mblogin=mblogin,
                         refresh=False)
 
@@ -199,7 +199,7 @@ class MBAPIHelper(APIHelper):
         params = {"client": CLIENT_STRING}
         data = self._xml_ratings(ratings)
         return self.post(path_list, data, handler, priority=True,
-                         queryargs=params, parse_response_type="xml",
+                         unencoded_queryargs=params, parse_response_type="xml",
                          request_mimetype="application/xml; charset=utf-8")
 
     def get_collection(self, collection_id, handler, limit=100, offset=0):
@@ -215,7 +215,7 @@ class MBAPIHelper(APIHelper):
             path_list = ('collection', )
             queryargs = None
         return self.get(tuple(path_list), handler, priority=True, important=True,
-                        mblogin=True, queryargs=queryargs)
+                        mblogin=True, unencoded_queryargs=queryargs)
 
     def get_collection_list(self, handler):
         return self.get_collection(None, handler)
@@ -233,12 +233,12 @@ class MBAPIHelper(APIHelper):
     def put_to_collection(self, collection_id, releases, handler):
         for path_list in self._collection_request(collection_id, releases):
             self.put(path_list, "", handler,
-                     queryargs=self._get_client_queryarg())
+                     unencoded_queryargs=self._get_client_queryarg())
 
     def delete_from_collection(self, collection_id, releases, handler):
         for path_list in self._collection_request(collection_id, releases):
             self.delete(path_list, handler,
-                        queryargs=self._get_client_queryarg())
+                        unencoded_queryargs=self._get_client_queryarg())
 
 
 class AcoustIdAPIHelper(APIHelper):
diff --git a/test/test_api_helpers.py b/test/test_api_helpers.py
index e727c07a3..be0beb849 100644
--- a/test/test_api_helpers.py
+++ b/test/test_api_helpers.py
@@ -93,9 +93,9 @@ class MBAPITest(PicardTestCase):
         self.assertNotIn(path, ws_function.call_args[1]['url'].path())
 
     def assertInQuery(self, ws_function, argname, value=None):
-        query_args = ws_function.call_args[1]['queryargs']
-        self.assertIn(argname, query_args)
-        self.assertEqual(value, query_args[argname])
+        unencoded_query_args = ws_function.call_args[1]['unencoded_queryargs']
+        self.assertIn(argname, unencoded_query_args)
+        self.assertEqual(value, unencoded_query_args[argname])
 
     def _test_inc_args(self, ws_function, arg_list):
         self.assertInQuery(self.ws.get_url, 'inc', "+".join(arg_list))

From 3f7b70d1befe770f6d75495343ab0131a49f03fc Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 15:36:25 +0200
Subject: [PATCH 26/33] CoverArtImage: remove unneeded parse_url()

- there's no point for it now, just ensure we have a QUrl
- instead of CoverArtImage.host, use CoverArtImage.url.host()
---
 picard/coverart/__init__.py |  4 ++--
 picard/coverart/image.py    | 17 ++++-------------
 2 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/picard/coverart/__init__.py b/picard/coverart/__init__.py
index a6f3883f4..243f17dc9 100644
--- a/picard/coverart/__init__.py
+++ b/picard/coverart/__init__.py
@@ -104,7 +104,7 @@ class CoverArt:
                 {
                     'type': coverartimage.types_as_string(),
                     'albumid': self.album.id,
-                    'host': coverartimage.host
+                    'host': coverartimage.url.host(),
                 },
                 echo=None
             )
@@ -190,7 +190,7 @@ class CoverArt:
             {
                 'type': coverartimage.types_as_string(),
                 'albumid': self.album.id,
-                'host': coverartimage.host
+                'host': coverartimage.url.host(),
             },
             echo=None
         )
diff --git a/picard/coverart/image.py b/picard/coverart/image.py
index ad0cec4ea..fcad8f3c4 100644
--- a/picard/coverart/image.py
+++ b/picard/coverart/image.py
@@ -37,7 +37,6 @@ from PyQt5.QtCore import (
     QMutex,
     QObject,
     QUrl,
-    QUrlQuery,
 )
 
 from picard import log
@@ -160,7 +159,10 @@ class CoverArtImage:
         else:
             self.types = types
         if url is not None:
-            self.parse_url(url)
+            if not isinstance(url, QUrl):
+                self.url = QUrl(url)
+            else:
+                self.url = url
         else:
             self.url = None
         self.comment = comment
@@ -178,17 +180,6 @@ class CoverArtImage:
         if data is not None:
             self.set_data(data)
 
-    def parse_url(self, url):
-        self.url = QUrl(url)
-        self.host = self.url.host()
-        self.port = self.url.port(443 if self.url.scheme() == 'https' else 80)
-        self.path = self.url.path(QUrl.ComponentFormattingOption.FullyEncoded)
-        if self.url.hasQuery():
-            query = QUrlQuery(self.url.query())
-            self.queryargs = dict(query.queryItems())
-        else:
-            self.queryargs = None
-
     @property
     def source(self):
         if self.url is not None:

From cfbdb1fb48d7e108fd6be44fca30a264350c6127 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 15:51:40 +0200
Subject: [PATCH 27/33] PluginsOptionsPage: use unencoded_queryargs (as they
 are unencoded)

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

diff --git a/picard/ui/options/plugins.py b/picard/ui/options/plugins.py
index 122559a00..131666645 100644
--- a/picard/ui/options/plugins.py
+++ b/picard/ui/options/plugins.py
@@ -663,7 +663,7 @@ class PluginsOptionsPage(OptionsPage):
             parse_response_type=None,
             priority=True,
             important=True,
-            queryargs={"id": plugin.module_name, "version": plugin.version.to_string(short=True)},
+            unencoded_queryargs={"id": plugin.module_name, "version": plugin.version.to_string(short=True)},
         )
 
     def download_handler(self, update, response, reply, error, plugin):

From 58f6b8dc7b7520289c1ad52c80c8ffecca05b9c7 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 16:27:02 +0200
Subject: [PATCH 28/33] Get rid of APIHelper path list, just pass the path
 string to append

---
 picard/webservice/api_helpers.py | 54 ++++++++++++++------------------
 test/test_api_helpers.py         | 20 ++++++------
 2 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 03de41868..7b01912c9 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -72,33 +72,33 @@ class APIHelper(object):
     def webservice(self):
         return self._webservice
 
-    def url_from_path_list(self, path_list):
+    def url_from_path(self, path):
         url = QUrl(self._base_url)
-        url.setPath("/".join([self._base_url.path()] + list(path_list)))
+        url.setPath(self._base_url.path() + path)
         return url
 
-    def get(self, path_list, handler, **kwargs):
-        kwargs['url'] = self.url_from_path_list(path_list)
+    def get(self, path, handler, **kwargs):
+        kwargs['url'] = self.url_from_path(path)
         kwargs['handler'] = handler
         return self._webservice.get_url(**kwargs)
 
-    def post(self, path_list, data, handler, **kwargs):
-        kwargs['url'] = self.url_from_path_list(path_list)
+    def post(self, path, data, handler, **kwargs):
+        kwargs['url'] = self.url_from_path(path)
         kwargs['handler'] = handler
         kwargs['data'] = data
         kwargs['mblogin'] = kwargs.get('mblogin', True)
         return self._webservice.post_url(**kwargs)
 
-    def put(self, path_list, data, handler, **kwargs):
-        kwargs['url'] = self.url_from_path_list(path_list)
+    def put(self, path, data, handler, **kwargs):
+        kwargs['url'] = self.url_from_path(path)
         kwargs['handler'] = handler
         kwargs['data'] = data
         kwargs['priority'] = kwargs.get('priority', True)
         kwargs['mblogin'] = kwargs.get('mblogin', True)
         return self._webservice.put_url(**kwargs)
 
-    def delete(self, path_list, handler, **kwargs):
-        kwargs['url'] = self.url_from_path_list(path_list)
+    def delete(self, path, handler, **kwargs):
+        kwargs['url'] = self.url_from_path(path)
         kwargs['handler'] = handler
         kwargs['priority'] = kwargs.get('priority', True)
         kwargs['mblogin'] = kwargs.get('mblogin', True)
@@ -116,11 +116,10 @@ class MBAPIHelper(APIHelper):
         super().__init__(webservice, base_url=base_url)
 
     def _get_by_id(self, entitytype, entityid, handler, inc=None, **kwargs):
-        path_list = (entitytype, entityid)
         if inc:
             kwargs['unencoded_queryargs'] = kwargs.get('queryargs', {})
             kwargs['unencoded_queryargs']["inc"] = "+".join(sorted(set(inc)))
-        return self.get(path_list, handler, **kwargs)
+        return self.get(f"/{entitytype}/{entityid}", handler, **kwargs)
 
     def get_release_by_id(self, releaseid, handler, inc=None, **kwargs):
         return self._get_by_id('release', releaseid, handler, inc, **kwargs)
@@ -155,8 +154,7 @@ class MBAPIHelper(APIHelper):
         if query:
             filters['query'] = query
 
-        path_list = (entitytype, )
-        return self.get(path_list, handler, unencoded_queryargs=filters,
+        return self.get(f"/{entitytype}", handler, unencoded_queryargs=filters,
                         priority=True, important=True, mblogin=False,
                         refresh=False)
 
@@ -170,12 +168,11 @@ class MBAPIHelper(APIHelper):
         return self._find('artist', handler, **kwargs)
 
     def _browse(self, entitytype, handler, inc=None, queryargs=None, mblogin=False):
-        path_list = (entitytype, )
         if queryargs is None:
             queryargs = {}
         if inc:
             queryargs["inc"] = "+".join(inc)
-        return self.get(path_list, handler, unencoded_queryargs=queryargs,
+        return self.get(f"/{entitytype}", handler, unencoded_queryargs=queryargs,
                         priority=True, important=True, mblogin=mblogin,
                         refresh=False)
 
@@ -195,26 +192,25 @@ class MBAPIHelper(APIHelper):
         return _wrap_xml_metadata('<recording-list>%s</recording-list>' % recordings)
 
     def submit_ratings(self, ratings, handler):
-        path_list = ('rating', )
         params = {"client": CLIENT_STRING}
         data = self._xml_ratings(ratings)
-        return self.post(path_list, data, handler, priority=True,
+        return self.post("/rating", data, handler, priority=True,
                          unencoded_queryargs=params, parse_response_type="xml",
                          request_mimetype="application/xml; charset=utf-8")
 
     def get_collection(self, collection_id, handler, limit=100, offset=0):
         if collection_id is not None:
             inc = ("releases", "artist-credits", "media")
-            path_list = ('collection', collection_id, "releases")
+            path = f"/collection/{collection_id}/releases"
             queryargs = {
                 "inc": "+".join(inc),
                 "limit": limit,
                 "offset": offset,
             }
         else:
-            path_list = ('collection', )
+            path = '/collection'
             queryargs = None
-        return self.get(tuple(path_list), handler, priority=True, important=True,
+        return self.get(path, handler, priority=True, important=True,
                         mblogin=True, unencoded_queryargs=queryargs)
 
     def get_collection_list(self, handler):
@@ -224,20 +220,20 @@ class MBAPIHelper(APIHelper):
     def _collection_request(collection_id, releases, batchsize=400):
         for i in range(0, len(releases), batchsize):
             ids = ";".join(releases[i:i+batchsize])
-            yield ("collection", collection_id, "releases", ids)
+            yield f"/collection/{collection_id}/releases/{ids}"
 
     @staticmethod
     def _get_client_queryarg():
         return {"client": CLIENT_STRING}
 
     def put_to_collection(self, collection_id, releases, handler):
-        for path_list in self._collection_request(collection_id, releases):
-            self.put(path_list, "", handler,
+        for path in self._collection_request(collection_id, releases):
+            self.put(path, "", handler,
                      unencoded_queryargs=self._get_client_queryarg())
 
     def delete_from_collection(self, collection_id, releases, handler):
-        for path_list in self._collection_request(collection_id, releases):
-            self.delete(path_list, handler,
+        for path in self._collection_request(collection_id, releases):
+            self.delete(path, handler,
                         unencoded_queryargs=self._get_client_queryarg())
 
 
@@ -256,10 +252,9 @@ class AcoustIdAPIHelper(APIHelper):
         return '&'.join((k + '=' + v for k, v in encoded_queryargs(args).items()))
 
     def query_acoustid(self, handler, **args):
-        path_list = ('lookup', )
         body = self._encode_acoustid_args(args)
         return self.post(
-            path_list, body, handler, priority=False, important=False,
+            "/lookup", body, handler, priority=False, important=False,
             mblogin=False, request_mimetype="application/x-www-form-urlencoded"
         )
 
@@ -274,10 +269,9 @@ class AcoustIdAPIHelper(APIHelper):
         return args
 
     def submit_acoustid_fingerprints(self, submissions, handler):
-        path_list = ('submit', )
         args = self._submissions_to_args(submissions)
         body = self._encode_acoustid_args(args)
         return self.post(
-            path_list, body, handler, priority=True, important=False,
+            "/submit", body, handler, priority=True, important=False,
             mblogin=False, request_mimetype="application/x-www-form-urlencoded"
         )
diff --git a/test/test_api_helpers.py b/test/test_api_helpers.py
index be0beb849..ff6a56a94 100644
--- a/test/test_api_helpers.py
+++ b/test/test_api_helpers.py
@@ -44,9 +44,9 @@ class APITest(PicardTestCase):
 
     def setUp(self):
         super().setUp()
-        base_url = "http://abc.com/v1/"
-        self.path_list = ['test', 'more', 'test']
-        self.complete_url = QUrl(base_url + "/test/more/test")
+        base_url = "http://abc.com/v1"
+        self.path = "/test/more/test"
+        self.complete_url = QUrl(base_url + self.path)
         self.ws = MagicMock(auto_spec=WebService)
         self.api = APIHelper(self.ws, base_url=base_url)
 
@@ -55,19 +55,19 @@ class APITest(PicardTestCase):
         self.assertEqual(ws_function.call_args[1]['url'], self.complete_url)
 
     def test_get(self):
-        self.api.get(self.path_list, None)
+        self.api.get(self.path, None)
         self._test_ws_function_args(self.ws.get_url)
 
     def test_post(self):
-        self.api.post(self.path_list, None, None)
+        self.api.post(self.path, None, None)
         self._test_ws_function_args(self.ws.post_url)
 
     def test_put(self):
-        self.api.put(self.path_list, None, None)
+        self.api.put(self.path, None, None)
         self._test_ws_function_args(self.ws.put_url)
 
     def test_delete(self):
-        self.api.delete(self.path_list, None)
+        self.api.delete(self.path, None)
         self._test_ws_function_args(self.ws.delete_url)
 
 
@@ -204,11 +204,11 @@ class MBAPITest(PicardTestCase):
         releases = tuple("r"+str(i) for i in range(13))
         generator = self.api._collection_request("test", releases, batchsize=5)
         batch = next(generator)
-        self.assertEqual(batch, ('collection', 'test', 'releases', 'r0;r1;r2;r3;r4'))
+        self.assertEqual(batch, '/collection/test/releases/r0;r1;r2;r3;r4')
         batch = next(generator)
-        self.assertEqual(batch, ('collection', 'test', 'releases', 'r5;r6;r7;r8;r9'))
+        self.assertEqual(batch, '/collection/test/releases/r5;r6;r7;r8;r9')
         batch = next(generator)
-        self.assertEqual(batch, ('collection', 'test', 'releases', 'r10;r11;r12'))
+        self.assertEqual(batch, '/collection/test/releases/r10;r11;r12')
         with self.assertRaises(StopIteration):
             next(generator)
 

From 9a0bedff20d266f6e32b9b2db460775e38456bb6 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Thu, 8 Jun 2023 22:38:53 +0200
Subject: [PATCH 29/33] Keep MBAPIHelper base_url dynamic

---
 picard/webservice/api_helpers.py | 35 +++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 7b01912c9..2849280cc 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -59,22 +59,32 @@ def _wrap_xml_metadata(data):
 
 
 class APIHelper(object):
+    _base_url = None
 
     def __init__(self, webservice, base_url=None):
         self._webservice = webservice
-        if not base_url:
-            raise ValueError("base_url is required")
-        elif not isinstance(base_url, QUrl):
-            base_url = QUrl(base_url)
-        self._base_url = base_url
+        if base_url is not None:
+            self.base_url = base_url
+
+    @property
+    def base_url(self):
+        if self._base_url is None:
+            raise ValueError("base_url undefined")
+        return self._base_url
+
+    @base_url.setter
+    def base_url(self, url):
+        if not isinstance(url, QUrl):
+            url = QUrl(url)
+        self._base_url = url
 
     @property
     def webservice(self):
         return self._webservice
 
     def url_from_path(self, path):
-        url = QUrl(self._base_url)
-        url.setPath(self._base_url.path() + path)
+        url = QUrl(self.base_url)
+        url.setPath(url.path() + path)
         return url
 
     def get(self, path, handler, **kwargs):
@@ -106,14 +116,15 @@ class APIHelper(object):
 
 
 class MBAPIHelper(APIHelper):
-
-    def __init__(self, webservice):
+    @property
+    def base_url(self):
+        # we have to keep it dynamic since host/port can be changed via options
         config = get_config()
         host = config.setting['server_host']
         port = config.setting['server_port']
-        base_url = host_port_to_url(host, port)
-        base_url.setPath('/ws/2')
-        super().__init__(webservice, base_url=base_url)
+        self._base_url = host_port_to_url(host, port)
+        self._base_url.setPath('/ws/2')
+        return self._base_url
 
     def _get_by_id(self, entitytype, entityid, handler, inc=None, **kwargs):
         if inc:

From 5395cbfc3d07fb8041662dcffb263da6c4ac34d2 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 Jun 2023 08:50:34 +0200
Subject: [PATCH 30/33] MBAPIHelper: introduce _make_inc_arg() to build inc
 parameter value

- it produces a more stable output (elements are sorted)
- it gets rid of empty elements (preventing a++b case)
- it accepts any iterable
- add a test for it

I also removed 2 useless conversion to tuple() (it can use a set() anyway)
---
 picard/album.py                  |  2 +-
 picard/track.py                  |  2 +-
 picard/webservice/api_helpers.py | 16 +++++++++++++---
 test/test_api_helpers.py         |  7 ++++++-
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/picard/album.py b/picard/album.py
index b7fa244c5..90c8f306f 100644
--- a/picard/album.py
+++ b/picard/album.py
@@ -628,7 +628,7 @@ class Album(DataObject, Item):
         self.load_task = self.tagger.mb_api.get_release_by_id(
             self.id,
             self._release_request_finished,
-            inc=tuple(inc),
+            inc=inc,
             mblogin=require_authentication,
             priority=priority,
             refresh=refresh
diff --git a/picard/track.py b/picard/track.py
index a6ffacd3a..148e802a1 100644
--- a/picard/track.py
+++ b/picard/track.py
@@ -408,7 +408,7 @@ class NonAlbumTrack(Track):
         self.tagger.mb_api.get_track_by_id(
             self.id,
             self._recording_request_finished,
-            inc=tuple(inc),
+            inc=inc,
             mblogin=require_authentication,
             priority=priority,
             refresh=refresh
diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 2849280cc..7c16e84c3 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -129,7 +129,7 @@ class MBAPIHelper(APIHelper):
     def _get_by_id(self, entitytype, entityid, handler, inc=None, **kwargs):
         if inc:
             kwargs['unencoded_queryargs'] = kwargs.get('queryargs', {})
-            kwargs['unencoded_queryargs']["inc"] = "+".join(sorted(set(inc)))
+            kwargs['unencoded_queryargs']['inc'] = self._make_inc_arg(inc)
         return self.get(f"/{entitytype}/{entityid}", handler, **kwargs)
 
     def get_release_by_id(self, releaseid, handler, inc=None, **kwargs):
@@ -178,11 +178,21 @@ class MBAPIHelper(APIHelper):
     def find_artists(self, handler, **kwargs):
         return self._find('artist', handler, **kwargs)
 
+    @staticmethod
+    def _make_inc_arg(inc):
+        """
+        Convert an iterable to a string to be passed as inc paramater to MB
+
+        It drops non-unique and empty elements, and sort them before joining
+        them as a '+'-separated string
+        """
+        return '+'.join(sorted(set(str(e) for e in inc if e)))
+
     def _browse(self, entitytype, handler, inc=None, queryargs=None, mblogin=False):
         if queryargs is None:
             queryargs = {}
         if inc:
-            queryargs["inc"] = "+".join(inc)
+            queryargs["inc"] = self._make_inc_arg(inc)
         return self.get(f"/{entitytype}", handler, unencoded_queryargs=queryargs,
                         priority=True, important=True, mblogin=mblogin,
                         refresh=False)
@@ -214,7 +224,7 @@ class MBAPIHelper(APIHelper):
             inc = ("releases", "artist-credits", "media")
             path = f"/collection/{collection_id}/releases"
             queryargs = {
-                "inc": "+".join(inc),
+                "inc": self._make_inc_arg(inc),
                 "limit": limit,
                 "offset": offset,
             }
diff --git a/test/test_api_helpers.py b/test/test_api_helpers.py
index ff6a56a94..748003d81 100644
--- a/test/test_api_helpers.py
+++ b/test/test_api_helpers.py
@@ -98,7 +98,7 @@ class MBAPITest(PicardTestCase):
         self.assertEqual(value, unencoded_query_args[argname])
 
     def _test_inc_args(self, ws_function, arg_list):
-        self.assertInQuery(self.ws.get_url, 'inc', "+".join(arg_list))
+        self.assertInQuery(self.ws.get_url, 'inc', self.api._make_inc_arg(arg_list))
 
     def test_get_release(self):
         inc_args_list = ['test']
@@ -212,6 +212,11 @@ class MBAPITest(PicardTestCase):
         with self.assertRaises(StopIteration):
             next(generator)
 
+    def test_make_inc_arg(self):
+        result = self.api._make_inc_arg(['b', 'a', '', 1, (), 0])
+        expected = '1+a+b'
+        self.assertEqual(result, expected)
+
 
 class AcoustdIdAPITest(PicardTestCase):
 

From 5d486c94fc0a9190307ab3d7adab6671b14595ca Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 Jun 2023 10:20:20 +0200
Subject: [PATCH 31/33] WebService: add more tests for queryargs and
 unencoded_queryargs

---
 test/test_webservice.py | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/test/test_webservice.py b/test/test_webservice.py
index 66113f775..211e207b7 100644
--- a/test/test_webservice.py
+++ b/test/test_webservice.py
@@ -534,8 +534,29 @@ class WSRequestTest(PicardTestCase):
             handler=dummy_handler,
             queryargs={'a': 2, 'b': 'x%20x', 'c': '1+2', 'd': '&', 'e': '?'},
         )
-        # FIXME: check encoding
-        self.assertEqual(request.url().toString(), 'http://example.org/path?a=1&a=2&b=x x&c=1+2&d=%26&e=?')
+        expected = 'http://example.org/path?a=1&a=2&b=x x&c=1+2&d=%26&e=?'
+        self.assertEqual(request.url().toString(), expected)
+
+    def test_unencoded_queryargs(self):
+        request = WSRequest(
+            url='http://example.org/path?a=1',
+            method='GET',
+            handler=dummy_handler,
+            unencoded_queryargs={'a': 2, 'b': 'x%20x', 'c': '1+2', 'd': '&', 'e': '?'},
+        )
+        expected = 'http://example.org/path?a=1&a=2&b=x%2520x&c=1%2B2&d=%26&e=%3F'
+        self.assertEqual(request.url().toString(), expected)
+
+    def test_mixed_queryargs(self):
+        request = WSRequest(
+            url='http://example.org/path?a=1',
+            method='GET',
+            handler=dummy_handler,
+            queryargs={'a': '2&', 'b': '1&', 'c': '&'},
+            unencoded_queryargs={'a': '1&', 'b': '2&', 'd': '&'},
+        )
+        expected = 'http://example.org/path?a=1&a=1%26&b=2%26&c=%26&d=%26'
+        self.assertEqual(request.url().toString(), expected)
 
 
 class WebServiceUtilsTest(PicardTestCase):

From 20fbea645456297d55d19405a8bea661908adf9c Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 Jun 2023 11:18:25 +0200
Subject: [PATCH 32/33] WebService: rework (mostly debug) logging of URLs

- ensure UserInfo is removed everywhere
- use QUrl.toDisplayString()
- keep spaces encoded (`%20`) in order to avoid breaking URLs (useful for terminal allowing to follow links)
- avoid having a ':' displayed just after URL (it also breaks links as : is legit in URLs)
- drop http_response_safe_url() in favor of more generic display_url()
- use constant variable naming scheme
---
 picard/webservice/__init__.py | 36 ++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/picard/webservice/__init__.py b/picard/webservice/__init__.py
index 75ab1a961..e0d6f4109 100644
--- a/picard/webservice/__init__.py
+++ b/picard/webservice/__init__.py
@@ -351,8 +351,8 @@ class WebService(QtCore.QObject):
         return reply.attribute(QNetworkRequest.Attribute.HttpReasonPhraseAttribute)
 
     @staticmethod
-    def http_response_safe_url(reply):
-        return reply.request().url().toString(QUrl.UrlFormattingOption.RemoveUserInfo)
+    def display_url(url):
+        return url.toDisplayString(QUrl.UrlFormattingOption.RemoveUserInfo | QUrl.ComponentFormattingOption.EncodeSpaces)
 
     def _network_accessible_changed(self, accessible):
         # Qt's network accessibility check sometimes fails, e.g. with VPNs on Windows.
@@ -465,13 +465,16 @@ class WebService(QtCore.QObject):
     def _handle_redirect(self, reply, request, redirect):
         error = int(reply.error())
         # merge with base url (to cover the possibility of the URL being relative)
-        redirect_qurl = request.url().resolved(redirect)
-        if not WebService.urls_equivalent(redirect_qurl, reply.request().url()):
-            log.debug("Redirect to %s requested", redirect_qurl.toString(QUrl.UrlFormattingOption.RemoveUserInfo))
+        redirect_url = request.url().resolved(redirect)
+        reply_url = reply.request().url()
+        display_redirect_url = self.display_url(redirect_url)
+        display_reply_url = self.display_url(reply_url)
+        if not WebService.urls_equivalent(redirect_url, reply_url):
+            log.debug("Redirect to %s requested", display_redirect_url)
 
             redirect_request = WSRequest(
                 method='GET',
-                url=redirect_qurl,
+                url=redirect_url,
                 handler=request.handler,
                 parse_response_type=request.parse_response_type,
                 priority=True,
@@ -488,9 +491,7 @@ class WebService(QtCore.QObject):
 
             self.add_request(redirect_request)
         else:
-            log.error("Redirect loop: %s",
-                      self.http_response_safe_url(reply)
-                      )
+            log.error("Redirect loop: %s", display_reply_url)
             request.handler(reply.readAll(), reply, error)
 
     def _handle_reply(self, reply, request):
@@ -504,11 +505,11 @@ class WebService(QtCore.QObject):
         error = int(reply.error())
         handler = request.handler
         response_code = self.http_response_code(reply)
-        url = self.http_response_safe_url(reply)
+        display_reply_url = self.display_url(reply.request().url())
         if error:
             errstr = reply.errorString()
-            log.error("Network request error for %s: %s (QT code %d, HTTP code %d)",
-                      url, errstr, error, response_code)
+            log.error("Network request error for %s -> %s (QT code %d, HTTP code %d)",
+                      display_reply_url, errstr, error, response_code)
             if (not request.max_retries_reached()
                 and (response_code == 503
                      or response_code == 429
@@ -519,7 +520,7 @@ class WebService(QtCore.QObject):
                      )):
                 slow_down = True
                 retries = request.mark_for_retry()
-                log.debug("Retrying %s (#%d)", url, retries)
+                log.debug("Retrying %s (#%d)", display_reply_url, retries)
                 self.add_request(request)
 
             elif handler is not None:
@@ -531,8 +532,8 @@ class WebService(QtCore.QObject):
             redirect = reply.attribute(QNetworkRequest.Attribute.RedirectionTargetAttribute)
             from_cache = reply.attribute(QNetworkRequest.Attribute.SourceIsFromCacheAttribute)
             cached = ' (CACHED)' if from_cache else ''
-            log.debug("Received reply for %s: HTTP %d (%s) %s",
-                      url,
+            log.debug("Received reply for %s -> HTTP %d (%s) %s",
+                      display_reply_url,
                       response_code,
                       self.http_response_phrase(reply),
                       cached
@@ -546,7 +547,7 @@ class WebService(QtCore.QObject):
                         document = request.response_parser(reply)
                         log.debug("Response received: %s", document)
                     except Exception as e:
-                        log.error("Unable to parse the response for %s: %s", url, e)
+                        log.error("Unable to parse the response for %s -> %s", display_reply_url, e)
                         document = reply.readAll()
                         error = e
                     finally:
@@ -560,7 +561,8 @@ class WebService(QtCore.QObject):
         try:
             request = self._active_requests.pop(reply)
         except KeyError:
-            log.error("Request not found for %s", self.http_response_safe_url(reply))
+            display_reply_url = self.display_url(reply.request().url())
+            log.error("Request not found for %s", display_reply_url)
             return
         try:
             self._handle_reply(reply, request)

From db4d9cfa0ad29fe947862669dfd1c4a340aa4f19 Mon Sep 17 00:00:00 2001
From: Laurent Monin <github@norz.org>
Date: Fri, 9 Jun 2023 16:08:36 +0200
Subject: [PATCH 33/33] Move /v2 to ACOUSTID_URL

---
 picard/const/__init__.py         | 2 +-
 picard/webservice/api_helpers.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/picard/const/__init__.py b/picard/const/__init__.py
index 9aec88bcc..3db7c0399 100644
--- a/picard/const/__init__.py
+++ b/picard/const/__init__.py
@@ -53,7 +53,7 @@ CACHE_SIZE_IN_BYTES = 100*1000*1000
 
 # AcoustID client API key
 ACOUSTID_KEY = 'v8pQ6oyB'
-ACOUSTID_URL = 'https://api.acoustid.org'
+ACOUSTID_URL = 'https://api.acoustid.org/v2'
 FPCALC_NAMES = ['fpcalc', 'pyfpcalc']
 DEFAULT_FPCALC_THREADS = 2
 
diff --git a/picard/webservice/api_helpers.py b/picard/webservice/api_helpers.py
index 7c16e84c3..1284d3dff 100644
--- a/picard/webservice/api_helpers.py
+++ b/picard/webservice/api_helpers.py
@@ -264,7 +264,7 @@ class AcoustIdAPIHelper(APIHelper):
     client_version = PICARD_VERSION_STR
 
     def __init__(self, webservice):
-        super().__init__(webservice, base_url=ACOUSTID_URL + '/v2')
+        super().__init__(webservice, base_url=ACOUSTID_URL)
 
     def _encode_acoustid_args(self, args):
         args['client'] = self.client_key