From ac524832e99b63edbad2f0984a6ced56d9dae49e Mon Sep 17 00:00:00 2001 From: Jianjian Huo Date: Mon, 14 Aug 2023 20:57:33 -0700 Subject: [PATCH] direct_client: support extra request params for direct_get_container. While the existing named kwargs (marker, limit, prefix and etc) are common parameters among different use cases of direct_get_container, a new 'extra_params' kwarg is added to support additional private parameters for each individual use case. Co-Authored-By: Alistair Coles Change-Id: If1d355fbc5d292cee4f44c0e5dc52e5df9ae990b --- swift/common/direct_client.py | 52 ++++++++--- test/unit/common/test_direct_client.py | 116 ++++++++++++++++++++++--- 2 files changed, 142 insertions(+), 26 deletions(-) diff --git a/swift/common/direct_client.py b/swift/common/direct_client.py index deb9fbe9ee..f2097d9fe0 100644 --- a/swift/common/direct_client.py +++ b/swift/common/direct_client.py @@ -156,29 +156,46 @@ def _get_direct_account_container(path, stype, node, part, marker=None, limit=None, prefix=None, delimiter=None, conn_timeout=5, response_timeout=15, - end_marker=None, reverse=None, headers=None): - """Base class for get direct account and container. + end_marker=None, reverse=None, headers=None, + extra_params=None): + """Base function for get direct account and container. - Do not use directly use the get_direct_account or - get_direct_container instead. + Do not use directly use the direct_get_account or + direct_get_container instead. """ if headers is None: headers = {} - params = ['format=json'] + params = {'format': 'json'} + if extra_params: + for key, value in extra_params.items(): + if value is not None: + params[key] = value if marker: - params.append('marker=%s' % quote(marker)) + if 'marker' in params: + raise TypeError('duplicate values for keyword arg: marker') + params['marker'] = quote(marker) if limit: - params.append('limit=%d' % limit) + if 'limit' in params: + raise TypeError('duplicate values for keyword arg: limit') + params['limit'] = '%d' % limit if prefix: - params.append('prefix=%s' % quote(prefix)) + if 'prefix' in params: + raise TypeError('duplicate values for keyword arg: prefix') + params['prefix'] = quote(prefix) if delimiter: - params.append('delimiter=%s' % quote(delimiter)) + if 'delimiter' in params: + raise TypeError('duplicate values for keyword arg: delimiter') + params['delimiter'] = quote(delimiter) if end_marker: - params.append('end_marker=%s' % quote(end_marker)) + if 'end_marker' in params: + raise TypeError('duplicate values for keyword arg: end_marker') + params['end_marker'] = quote(end_marker) if reverse: - params.append('reverse=%s' % quote(reverse)) - qs = '&'.join(params) + if 'reverse' in params: + raise TypeError('duplicate values for keyword arg: reverse') + params['reverse'] = quote(reverse) + qs = '&'.join('%s=%s' % (k, v) for k, v in params.items()) ip, port = get_ip_port(node, headers) with Timeout(conn_timeout): @@ -293,7 +310,7 @@ def direct_head_container(node, part, account, container, conn_timeout=5, def direct_get_container(node, part, account, container, marker=None, limit=None, prefix=None, delimiter=None, conn_timeout=5, response_timeout=15, end_marker=None, - reverse=None, headers=None): + reverse=None, headers=None, extra_params=None): """ Get container listings directly from the container server. @@ -310,6 +327,12 @@ def direct_get_container(node, part, account, container, marker=None, :param end_marker: end_marker query :param reverse: reverse the returned listing :param headers: headers to be included in the request + :param extra_params: a dict of extra parameters to be included in the + request. It can be used to pass additional parameters, e.g, + {'states':'updating'} can be used with shard_range/namespace listing. + It can also be used to pass the existing keyword args, like 'marker' or + 'limit', but if the same parameter appears twice in both keyword arg + (not None) and extra_params, this function will raise TypeError. :returns: a tuple of (response headers, a list of objects) The response headers will be a HeaderKeyDict. """ @@ -322,7 +345,8 @@ def direct_get_container(node, part, account, container, marker=None, reverse=reverse, conn_timeout=conn_timeout, response_timeout=response_timeout, - headers=headers) + headers=headers, + extra_params=extra_params) def direct_delete_container(node, part, account, container, conn_timeout=5, diff --git a/test/unit/common/test_direct_client.py b/test/unit/common/test_direct_client.py index b1d02c6102..5d1393bf29 100644 --- a/test/unit/common/test_direct_client.py +++ b/test/unit/common/test_direct_client.py @@ -224,12 +224,12 @@ class TestDirectClient(unittest.TestCase): self.user_agent) self.assertEqual(resp_headers, stub_headers) self.assertEqual(json.loads(body), resp) - self.assertIn('format=json', conn.query_string) - for k, v in req_params.items(): - if v is None: - self.assertNotIn('&%s' % k, conn.query_string) - else: - self.assertIn('&%s=%s' % (k, v), conn.query_string) + actual_params = conn.query_string.split('&') + exp_params = ['%s=%s' % (k, v) + for k, v in req_params.items() + if v is not None] + exp_params.append('format=json') + self.assertEqual(sorted(actual_params), sorted(exp_params)) except AssertionError as err: self.fail('Failed with params %s: %s' % (req_params, err)) @@ -443,12 +443,12 @@ class TestDirectClient(unittest.TestCase): self.user_agent) self.assertEqual(headers, resp_headers) self.assertEqual(json.loads(body), resp) - self.assertIn('format=json', conn.query_string) - for k, v in req_params.items(): - if v is None: - self.assertNotIn('&%s' % k, conn.query_string) - else: - self.assertIn('&%s=%s' % (k, v), conn.query_string) + actual_params = conn.query_string.split('&') + exp_params = ['%s=%s' % (k, v) + for k, v in req_params.items() + if v is not None] + exp_params.append('format=json') + self.assertEqual(sorted(actual_params), sorted(exp_params)) except AssertionError as err: self.fail('Failed with params %s: %s' % (req_params, err)) @@ -475,6 +475,98 @@ class TestDirectClient(unittest.TestCase): self.assertEqual(headers, resp_headers) self.assertEqual([], resp) + def test_direct_get_container_with_extra_params(self): + def do_test(req_params, expected_params): + headers = HeaderKeyDict({'key': 'value'}) + body = (b'[{"hash": "8f4e3", "last_modified": "317260", ' + b'"bytes": 209}]') + + with mocked_http_conn(200, headers, body) as conn: + resp_headers, resp = direct_client.direct_get_container( + self.node, self.part, self.account, self.container, + **req_params) + + self.assertEqual(conn.method, 'GET') + self.assertEqual(conn.path, self.container_path) + self.assertEqual( + conn.req_headers['user-agent'], self.user_agent) + self.assertEqual(headers, resp_headers) + self.assertEqual(json.loads(body), resp) + actual_params = conn.query_string.split('&') + exp_params = ['%s=%s' % (k, v) + for k, v in expected_params.items()] + exp_params.append('format=json') + self.assertEqual(sorted(actual_params), sorted(exp_params)) + + req_params = dict(marker='my-marker', prefix='my-prefix', + delimiter='my-delimiter', + limit=10, end_marker='my-endmarker', reverse='on', + extra_params={'states': 'updating', + 'test': 'okay'}) + expected_params = dict(marker='my-marker', prefix='my-prefix', + delimiter='my-delimiter', limit=10, + end_marker='my-endmarker', reverse='on', + states='updating', test='okay') + do_test(req_params, expected_params) + + req_params = dict(marker='my-marker', prefix='my-prefix', + delimiter='my-delimiter', + limit=10, end_marker='my-endmarker', reverse='on', + extra_params={'states': None}) + expected_params = dict(marker='my-marker', prefix='my-prefix', + delimiter='my-delimiter', limit=10, + end_marker='my-endmarker', reverse='on') + + req_params = dict(marker='my-marker', prefix='my-prefix', + delimiter='my-delimiter', + limit=10, end_marker='my-endmarker', reverse='on', + extra_params={}) + expected_params = dict(marker='my-marker', prefix='my-prefix', + delimiter='my-delimiter', limit=10, + end_marker='my-endmarker', reverse='on') + do_test(req_params, expected_params) + + req_params = dict(marker='my-marker', prefix='my-prefix', + delimiter='my-delimiter', + limit=10, end_marker='my-endmarker', reverse='on', + extra_params={'states': 'updating', + 'marker': 'others'}) + with self.assertRaises(TypeError) as cm: + do_test(req_params, expected_params={}) + self.assertIn('duplicate values for keyword arg: marker', + str(cm.exception)) + + req_params = dict(marker='my-marker', prefix='my-prefix', + delimiter='my-delimiter', + limit=10, end_marker='my-endmarker', reverse='on', + extra_params={'prefix': 'others'}) + with self.assertRaises(TypeError) as cm: + do_test(req_params, expected_params=None) + self.assertIn('duplicate values for keyword arg: prefix', + str(cm.exception)) + + req_params = dict(marker='my-marker', delimiter='my-delimiter', + limit=10, end_marker='my-endmarker', reverse='on', + extra_params={'prefix': 'others'}) + expected_params = dict(marker='my-marker', prefix='others', + delimiter='my-delimiter', limit=10, + end_marker='my-endmarker', reverse='on') + do_test(req_params, expected_params) + + req_params = dict(marker='my-marker', prefix=None, + delimiter='my-delimiter', + limit=10, end_marker='my-endmarker', reverse='on', + extra_params={'prefix': 'others'}) + expected_params = dict(marker='my-marker', prefix='others', + delimiter='my-delimiter', limit=10, + end_marker='my-endmarker', reverse='on') + do_test(req_params, expected_params) + + req_params = dict(extra_params={'limit': 10, 'empty': '', + 'test': True, 'zero': 0}) + expected_params = dict(limit='10', empty='', test='True', zero='0') + do_test(req_params, expected_params) + def test_direct_delete_container(self): with mocked_http_conn(200) as conn: direct_client.direct_delete_container(