From a3644ebd6395fb767c5e8bec2aced93ca7f5f8df Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Fri, 20 Nov 2020 16:55:12 +1300 Subject: [PATCH] Improve object_to_dict arguments As a follow-up to the review feedback in[1], type specific fields arguments are removed and the type is inferred from the versioned object fields. Story: 1651346 Task: 10551 [1] https://review.opendev.org/751160 Change-Id: I89a65214ab7d550d0b4a327dd033c27399ae13bf --- ironic/api/controllers/v1/allocation.py | 13 +++- ironic/api/controllers/v1/bios.py | 2 +- ironic/api/controllers/v1/conductor.py | 2 +- ironic/api/controllers/v1/node.py | 11 +-- ironic/api/controllers/v1/utils.py | 67 +++++++++---------- .../unit/api/controllers/v1/test_utils.py | 23 ++++--- 6 files changed, 61 insertions(+), 57 deletions(-) diff --git a/ironic/api/controllers/v1/allocation.py b/ironic/api/controllers/v1/allocation.py index fb876c3811..037e2c643a 100644 --- a/ironic/api/controllers/v1/allocation.py +++ b/ironic/api/controllers/v1/allocation.py @@ -73,9 +73,16 @@ def convert_with_links(rpc_allocation, fields=None, sanitize=True): allocation = api_utils.object_to_dict( rpc_allocation, link_resource='allocations', - fields=('extra', 'name', 'state', 'last_error', 'resource_class', - 'owner'), - list_fields=('candidate_nodes', 'traits') + fields=( + 'candidate_nodes', + 'extra', + 'last_error', + 'name', + 'owner', + 'resource_class', + 'state', + 'traits' + ) ) try: api_utils.populate_node_uuid(rpc_allocation, allocation) diff --git a/ironic/api/controllers/v1/bios.py b/ironic/api/controllers/v1/bios.py index 9475f766e8..be6743d703 100644 --- a/ironic/api/controllers/v1/bios.py +++ b/ironic/api/controllers/v1/bios.py @@ -31,7 +31,7 @@ def convert_with_links(rpc_bios, node_uuid): """Build a dict containing a bios setting value.""" bios = api_utils.object_to_dict( rpc_bios, - uuid=False, + include_uuid=False, fields=('name', 'value'), link_resource='nodes', link_resource_args="%s/bios/%s" % (node_uuid, rpc_bios.name), diff --git a/ironic/api/controllers/v1/conductor.py b/ironic/api/controllers/v1/conductor.py index 839eef5761..c6e55a38fd 100644 --- a/ironic/api/controllers/v1/conductor.py +++ b/ironic/api/controllers/v1/conductor.py @@ -36,7 +36,7 @@ DEFAULT_RETURN_FIELDS = ['hostname', 'conductor_group', 'alive'] def convert_with_links(rpc_conductor, fields=None, sanitize=True): conductor = api_utils.object_to_dict( rpc_conductor, - uuid=False, + include_uuid=False, fields=('hostname', 'conductor_group', 'drivers'), link_resource='conductors', link_resource_args=rpc_conductor.hostname diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 8340152a4d..d07561f9ca 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1137,6 +1137,7 @@ def node_convert_with_links(rpc_node, fields=None, sanitize=True): 'boot_interface', 'clean_step', 'conductor_group', + 'console_enabled', 'console_interface', 'deploy_interface', 'deploy_step', @@ -1146,11 +1147,14 @@ def node_convert_with_links(rpc_node, fields=None, sanitize=True): 'driver_internal_info', 'extra', 'fault', + 'inspection_finished_at', + 'inspection_started_at', 'inspect_interface', 'instance_info', 'instance_uuid', 'last_error', 'lessee', + 'maintenance', 'maintenance_reason', 'management_interface', 'name', @@ -1160,13 +1164,16 @@ def node_convert_with_links(rpc_node, fields=None, sanitize=True): 'power_interface', 'power_state', 'properties', + 'protected', 'protected_reason', 'provision_state', + 'provision_updated_at', 'raid_config', 'raid_interface', 'rescue_interface', 'reservation', 'resource_class', + 'retired', 'retired_reason', 'storage_interface', 'target_power_state', @@ -1174,10 +1181,6 @@ def node_convert_with_links(rpc_node, fields=None, sanitize=True): 'target_raid_config', 'vendor_interface' ), - boolean_fields=('console_enabled', 'maintenance', 'protected', - 'retired'), - date_fields=('inspection_finished_at', 'inspection_started_at', - 'provision_updated_at'), ) node['traits'] = rpc_node.traits.get_trait_names() diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 474987f9a3..1d1e0aa246 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -38,6 +38,7 @@ from ironic.common import policy from ironic.common import states from ironic.common import utils from ironic import objects +from ironic.objects import fields as ofields CONF = cfg.CONF @@ -152,18 +153,18 @@ LOCAL_LINK_VALIDATOR = args.and_valid( LOCAL_LINK_SMART_NIC_VALIDATOR = args.schema(LOCAL_LINK_SMART_NIC_SCHEMA) -def object_to_dict(obj, created_at=True, updated_at=True, uuid=True, - link_resource=None, link_resource_args=None, fields=None, - list_fields=None, date_fields=None, boolean_fields=None): +def object_to_dict(obj, include_created_at=True, include_updated_at=True, + include_uuid=True, link_resource=None, + link_resource_args=None, fields=None): """Helper function to convert RPC objects to REST API dicts. :param obj: RPC object to convert to a dict - :param created_at: + :param include_created_at: Whether to include standard base class attribute created_at - :param updated_at: + :param include_updated_at: Whether to include standard base class attribute updated_at - :param uuid: + :param include_uuid: Whether to include standard base class attribute uuid :param link_resource: When specified, generate a ``links`` value with a ``self`` and @@ -172,47 +173,39 @@ def object_to_dict(obj, created_at=True, updated_at=True, uuid=True, Resource arguments to be added to generated links. When not specified, the object ``uuid`` will be used. :param fields: - Dict values to populate directly from object attributes - :param list_fields: - Dict values to populate from object attributes where an empty list is - the default for empty attributes - :param date_fields: - Dict values to populate from object attributes as ISO 8601 dates, - or None if the value is None - :param boolean_fields: - Dict values to populate from object attributes as boolean values - or False if the value is empty + Key names for dict values to populate directly from object attributes :returns: A dict containing values from the object """ url = api.request.public_url to_dict = {} - if uuid: - to_dict['uuid'] = obj.uuid + all_fields = [] - if created_at: - to_dict['created_at'] = (obj.created_at - and obj.created_at.isoformat() or None) - if updated_at: - to_dict['updated_at'] = (obj.updated_at - and obj.updated_at.isoformat() or None) + if include_uuid: + all_fields.append('uuid') + if include_created_at: + all_fields.append('created_at') + if include_updated_at: + all_fields.append('updated_at') if fields: - for field in fields: - to_dict[field] = getattr(obj, field) + all_fields.extend(fields) - if list_fields: - for field in list_fields: - to_dict[field] = getattr(obj, field) or [] + for field in all_fields: + value = to_dict[field] = getattr(obj, field) + empty_value = None + if isinstance(obj.fields[field], ofields.ListOfStringsField): + empty_value = [] + elif isinstance(obj.fields[field], ofields.FlexibleDictField): + empty_value = {} + elif isinstance(obj.fields[field], ofields.DateTimeField): + if value: + value = value.isoformat() - if date_fields: - for field in date_fields: - date = getattr(obj, field) - to_dict[field] = date and date.isoformat() or None - - if boolean_fields: - for field in boolean_fields: - to_dict[field] = getattr(obj, field) or False + if value is not None: + to_dict[field] = value + else: + to_dict[field] = empty_value if link_resource: if not link_resource_args: diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index 0d9b489fe9..031042f7f9 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -1596,8 +1596,7 @@ class TestObjectToDict(base.TestCase): created_at=datetime.datetime(2000, 1, 1, 0, 0), updated_at=datetime.datetime(2001, 1, 1, 0, 0), inspection_started_at=datetime.datetime(2002, 1, 1, 0, 0), - console_enabled=True, - tags=['one', 'two', 'three']) + console_enabled=True) p = mock.patch.object(api, 'request', autospec=False) mock_req = p.start() @@ -1614,9 +1613,9 @@ class TestObjectToDict(base.TestCase): def test_no_base_attributes(self): self.assertEqual({}, utils.object_to_dict( self.node, - created_at=False, - updated_at=False, - uuid=False) + include_created_at=False, + include_updated_at=False, + include_uuid=False) ) def test_fields(self): @@ -1628,16 +1627,18 @@ class TestObjectToDict(base.TestCase): 'inspection_finished_at': None, 'inspection_started_at': '2002-01-01T00:00:00+00:00', 'maintenance': False, - 'tags': ['one', 'two', 'three'], - 'traits': [], 'updated_at': '2001-01-01T00:00:00+00:00', 'uuid': '1be26c0b-03f2-4d2e-ae87-c02d7f33c123' }, utils.object_to_dict( self.node, - fields=['conductor_group', 'driver'], - boolean_fields=['maintenance', 'console_enabled'], - date_fields=['inspection_started_at', 'inspection_finished_at'], - list_fields=['tags', 'traits']) + fields=[ + 'conductor_group', + 'console_enabled', + 'driver', + 'inspection_finished_at', + 'inspection_started_at', + 'maintenance', + ]) ) def test_links(self):