Support more standard way of passing lists via query strings

Currently, arguments like "fields", "shared" or the new "device_types"
only accept comma-separated strings. While there is no single standard,
the most common approach is to repeat the arguments, i.e.

 NOT /nodes?fields=uuid,name
 BUT /nodes?fields=uuid&fields=name

Unfortunately, at least GopherCloud already relies [1] on the more common
(but not currently working in Ironic) behavior. Let's make it work.

[1] 8455d01343/openstack/baremetal/v1/nodes/testing/requests_test.go (L87)

Change-Id: Ia780b10986929d79dc4f334d278bcb00a9984fd0
This commit is contained in:
Dmitry Tantsur 2024-02-29 17:04:46 +01:00
parent 78b6f00af8
commit 934658dab4
No known key found for this signature in database
GPG Key ID: 315B2AF9FD216C60
3 changed files with 55 additions and 1 deletions

View File

@ -128,9 +128,12 @@ def string_list(name, value):
same order, or None if value is None same order, or None if value is None
:raises: InvalidParameterValue if the value is not a string :raises: InvalidParameterValue if the value is not a string
""" """
value = string(name, value)
if value is None: if value is None:
return return
if isinstance(value, list):
return [string(name, item) for item in value]
value = string(name, value)
items = [] items = []
for v in str(value).split(','): for v in str(value).split(','):
v_norm = v.strip().lower() v_norm = v.strip().lower()

View File

@ -523,6 +523,15 @@ class TestListNodes(test_api_base.BaseApiTest):
# We always append "links" # We always append "links"
self.assertCountEqual(['extra', 'instance_info', 'links'], data) self.assertCountEqual(['extra', 'instance_info', 'links'], data)
def test_get_one_custom_fields_as_list(self):
node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)
data = self.get_json(
'/nodes/%s?fields=extra&fields=instance_info' % node.uuid,
headers={api_base.Version.string: str(api_v1.max_version())})
# We always append "links"
self.assertCountEqual(['extra', 'instance_info', 'links'], data)
def test_get_collection_custom_fields(self): def test_get_collection_custom_fields(self):
fields = 'uuid,instance_info' fields = 'uuid,instance_info'
for i in range(3): for i in range(3):
@ -539,6 +548,21 @@ class TestListNodes(test_api_base.BaseApiTest):
# We always append "links" # We always append "links"
self.assertCountEqual(['uuid', 'instance_info', 'links'], node) self.assertCountEqual(['uuid', 'instance_info', 'links'], node)
def test_get_collection_custom_fields_as_list(self):
for i in range(3):
obj_utils.create_test_node(self.context,
uuid=uuidutils.generate_uuid(),
instance_uuid=uuidutils.generate_uuid())
data = self.get_json(
'/nodes?fields=uuid&fields=instance_info',
headers={api_base.Version.string: str(api_v1.max_version())})
self.assertEqual(3, len(data['nodes']))
for node in data['nodes']:
# We always append "links"
self.assertCountEqual(['uuid', 'instance_info', 'links'], node)
def test_get_collection_fields_for_nova(self): def test_get_collection_fields_for_nova(self):
# Unit test which explicitly attempts to request traits along with # Unit test which explicitly attempts to request traits along with
# all columns which nova would normally request as part of it's sync # all columns which nova would normally request as part of it's sync
@ -8301,6 +8325,13 @@ class TestNodeShardGets(test_api_base.BaseApiTest):
'/nodes?shard=foo,bar', headers=self.headers) '/nodes?shard=foo,bar', headers=self.headers)
self.assertEqual(2, len(result['nodes'])) self.assertEqual(2, len(result['nodes']))
def test_filtering_by_multi_shard_as_list(self):
obj_utils.create_test_node(
self.context, uuid=uuid.uuid4(), shard='bar')
result = self.get_json(
'/nodes?shard=foo&shard=bar', headers=self.headers)
self.assertEqual(2, len(result['nodes']))
def test_filtering_by_shard_detail_fails_wrong_version(self): def test_filtering_by_shard_detail_fails_wrong_version(self):
headers = {api_base.Version.string: '1.80'} headers = {api_base.Version.string: '1.80'}
@ -8763,6 +8794,19 @@ class TestNodeVmedia(test_api_base.BaseApiTest):
mock.ANY, mock.ANY, self.node.uuid, mock.ANY, mock.ANY, self.node.uuid,
device_types=[boot_devices.CDROM], topic='test-topic') device_types=[boot_devices.CDROM], topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'detach_virtual_media',
autospec=True)
def test_detach_several_via_argument(self, mock_detach):
ret = self.delete('/nodes/%s/vmedia?device_types=%s&device_types=%s'
% (self.node.uuid,
boot_devices.CDROM, boot_devices.DISK),
headers={api_base.Version.string: self.version})
self.assertEqual(http_client.NO_CONTENT, ret.status_int)
mock_detach.assert_called_once_with(
mock.ANY, mock.ANY, self.node.uuid,
device_types=[boot_devices.CDROM, boot_devices.DISK],
topic='test-topic')
def test_detach_wrong_device_types(self): def test_detach_wrong_device_types(self):
ret = self.delete('/nodes/%s/vmedia?device_types=cdrom,cat' ret = self.delete('/nodes/%s/vmedia?device_types=cdrom,cat'
% self.node.uuid, % self.node.uuid,

View File

@ -0,0 +1,7 @@
---
fixes:
- |
Query parameters in the API that expect lists now accept repeated arguments
(``param=value1&param=value2``) in addition to comma-separated strings
(``param=value1,value2``). The former seems to be more common and is
actually (incorrectly) used in GopherCloud.