From c7acf9ea5b1dfd8fb4607836b4aa5efa169e05c4 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Thu, 8 Jun 2023 11:50:43 +0200 Subject: [PATCH] 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()