From a657f3aad29f89655a7c5acaa6e322502ce6c4a8 Mon Sep 17 00:00:00 2001 From: gtema Date: Thu, 8 Feb 2024 09:21:05 +0100 Subject: [PATCH 1/6] feat(volume): add volume metadata --- codegenerator/openapi/cinder.py | 22 +++++++++++++++++++ codegenerator/openapi/cinder_schemas.py | 12 ++++++++++ codegenerator/rust_cli.py | 7 +++++- .../templates/rust_cli/response_struct.j2 | 12 +++++----- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/codegenerator/openapi/cinder.py b/codegenerator/openapi/cinder.py index a2a2cc0..1d082d9 100644 --- a/codegenerator/openapi/cinder.py +++ b/codegenerator/openapi/cinder.py @@ -167,6 +167,7 @@ def _get_schema_ref( action_name=None, ): mime_type: str = "application/json" + # ### Volume if name == "VolumesListResponse": openapi_spec.components.schemas.setdefault( name, TypeSchema(**cinder_schemas.VOLUMES_SCHEMA) @@ -186,6 +187,27 @@ def _get_schema_ref( name, TypeSchema(**cinder_schemas.VOLUME_CONTAINER_SCHEMA) ) ref = f"#/components/schemas/{name}" + # ### Volume Metadata + elif name in [ + "VolumesMetadataListResponse", + "VolumesMetadataUpdate_All", + "VolumesMetadataUpdate_AllResponse", + "VolumesMetadataCreateResponse", + ]: + openapi_spec.components.schemas.setdefault( + name, TypeSchema(**cinder_schemas.METADATA_CONTAINER_SCHEMA) + ) + ref = f"#/components/schemas/{name}" + elif name in [ + "VolumesMetadataShowResponse", + "VolumesMetadataUpdate", + "VolumesMetadataUpdateResponse", + ]: + openapi_spec.components.schemas.setdefault( + name, TypeSchema(**cinder_schemas.METADATA_ITEM_SCHEMA) + ) + ref = f"#/components/schemas/{name}" + else: (ref, mime_type) = super()._get_schema_ref( openapi_spec, name, description, action_name=action_name diff --git a/codegenerator/openapi/cinder_schemas.py b/codegenerator/openapi/cinder_schemas.py index f9aba3c..82f572c 100644 --- a/codegenerator/openapi/cinder_schemas.py +++ b/codegenerator/openapi/cinder_schemas.py @@ -58,6 +58,18 @@ "description": "A metadata object. Contains one or more metadata key and value pairs that are associated with the resource.", } +METADATA_CONTAINER_SCHEMA: dict[str, Any] = { + "type": "object", + "description": "Metadata key and value pairs. The maximum size for each metadata key and value pair is 255 bytes.", + "properties": {"metadata": METADATA_SCHEMA}, +} + +METADATA_ITEM_SCHEMA: dict[str, Any] = { + "type": "object", + "description": "Metadata key and value pairs. The maximum size for each metadata key and value pair is 255 bytes.", + "properties": {"meta": {"maxProperties": 1, **METADATA_SCHEMA}}, +} + VOLUME_SHORT_SCHEMA: dict[str, Any] = { "type": "object", "description": "A volume object.", diff --git a/codegenerator/rust_cli.py b/codegenerator/rust_cli.py index dfb4752..a52628b 100644 --- a/codegenerator/rust_cli.py +++ b/codegenerator/rust_cli.py @@ -1065,7 +1065,12 @@ def generate( if not response_def: continue - if response_def.get("type", "object") == "object": + if response_def.get("type", "object") == "object" or ( + # BS metadata is defined with type: ["object", + # "null"] + isinstance(response_def.get("type"), list) + and "object" in response_def["type"] + ): (_, response_types) = openapi_parser.parse( response_def ) diff --git a/codegenerator/templates/rust_cli/response_struct.j2 b/codegenerator/templates/rust_cli/response_struct.j2 index df0431a..b3e20c2 100644 --- a/codegenerator/templates/rust_cli/response_struct.j2 +++ b/codegenerator/templates/rust_cli/response_struct.j2 @@ -57,18 +57,20 @@ {%- elif data_type.__class__.__name__ == "HashMapResponse" %} /// Response data as HashMap type #[derive(Deserialize, Serialize)] - struct ResponseData(HashMap); + struct ResponseData(HashMap); impl StructTable for ResponseData { - fn build(&self, _options: &OutputConfig) -> (Vec, - Vec>) { - let headers: Vec = Vec::from(["Name". - to_string(), "Value".to_string()]); + fn build(&self, _options: &OutputConfig) -> (Vec, Vec>) { + let headers: Vec = Vec::from(["Name".to_string(), "Value".to_string()]); let mut rows: Vec> = Vec::new(); rows.extend( self.0 .iter() + {%- if data_type.value_type.type_hint == "String" %} + .map(|(k, v)| Vec::from([k.clone(), v.clone()])), + {%- elif data_type.value_type.type_hint == "Value" %} .map(|(k, v)| Vec::from([k.clone(), serde_json::to_string(&v).expect("Is a valid data")])), + {%- endif %} ); (headers, rows) } From b66e16ff81370bcf75e8b618885b8c923ce5da79 Mon Sep 17 00:00:00 2001 From: gtema Date: Thu, 8 Feb 2024 11:56:14 +0100 Subject: [PATCH 2/6] feat: add helper to locate proper response in openapi --- codegenerator/common/__init__.py | 121 +++++++++++++++++++++++- codegenerator/tests/unit/test_common.py | 119 +++++++++++++++++++++++ 2 files changed, 235 insertions(+), 5 deletions(-) create mode 100644 codegenerator/tests/unit/test_common.py diff --git a/codegenerator/common/__init__.py b/codegenerator/common/__init__.py index e24e218..210db6e 100644 --- a/codegenerator/common/__init__.py +++ b/codegenerator/common/__init__.py @@ -93,7 +93,7 @@ def get_plural_form(resource: str) -> str: def find_resource_schema( schema: dict, parent: str | None = None, resource_name: str | None = None -): +) -> tuple[dict | None, str | None]: """Find the actual resource schema in the body schema Traverse through the body schema searching for an element that represent @@ -128,7 +128,16 @@ def find_resource_schema( and resource_name and parent == get_plural_form(resource_name) ): - return (schema["items"], parent) + items = schema["items"] + if ( + items.get("type") == "object" + and resource_name in items.get("properties", []) + and len(items.get("properties", []).keys()) == 1 + ): + # Most likely this is Keypair where we have keypairs.keypair.{} + return (items["properties"][resource_name], parent) + else: + return (items, parent) elif ( not parent and schema.get("items", {}).get("type") == "object" ): @@ -147,8 +156,8 @@ def find_resource_schema( else schema.get("properties", {}) ) if not parent and resource_name in props: - # we are at the top level and there is property with the resource - # name - it is what we are searching for + # we are at the top level and there is property with the + # resource name - it is what we are searching for el_type = props[resource_name]["type"] if el_type == "array": return (props[resource_name]["items"], resource_name) @@ -165,7 +174,12 @@ def find_resource_schema( keys = list(props.keys()) if len(keys) == 1: # there is only one field in the object - return (props[keys[0]], keys[0]) + if props[keys[0]].get("type") == "object": + # and it is itself an object + return (props[keys[0]], keys[0]) + else: + # only field is not an object + return (schema, None) else: return (schema, None) except Exception as ex: @@ -176,6 +190,103 @@ def find_resource_schema( return (None, None) +def find_response_schema( + responses: dict, response_key: str, action_name: str | None = None +): + """Locate response schema + + Some operations are having variety of possible responses (depending on + microversion, action, etc). Try to locate suitable response for the client. + + The function iterates over all defined responses and for 2** appies the + following logic: + + - if action_name is present AND oneOf is present AND action_name is in one + of the oneOf schemas -> return this schema + + - if action_name is not present AND oneOf is present AND response_key is in + one of the OneOf candidates' properties (this is an object) -> return it + + - action_name is not present AND oneOf is not present and (response_key or + plural of the response_key) in candidate -> return it + + :param dict responses: Dictionary with responses as defined in OpenAPI spec + :param str response_key: Response key to be searching in responses (when + aciton_name is not given) :param str action_name: Action name to be + searching response for + """ + for code, rspec in responses.items(): + if not code.startswith("2"): + continue + content = rspec.get("content", {}) + if "application/json" in content: + response_spec = content["application/json"] + schema = response_spec["schema"] + oneof = schema.get("oneOf") + discriminator = schema.get("x-openstack", {}).get("discriminator") + if oneof: + if not discriminator: + # Server create returns server or reservation info. For the + # cli it is not very helpful and we look for response + # candidate with the resource_name in the response + for candidate in oneof: + if ( + action_name + and candidate.get("x-openstack", {}).get( + "action-name" + ) + == action_name + ): + if response_key in candidate.get("properties", {}): + # If there is a object with resource_name in + # the props - this must be what we want to look + # at + return candidate["properties"][response_key] + else: + return candidate + elif ( + not action_name + and response_key + and candidate.get("type") == "object" + and response_key in candidate.get("properties", {}) + ): + # Actually for the sake of the CLI it may make + # sense to merge all candidates + return candidate["properties"][response_key] + else: + raise NotImplementedError + elif ( + not action_name + and schema + and ( + response_key in schema + or ( + schema.get("type") == "object" + and ( + response_key in schema.get("properties", []) + or get_plural_form(response_key) + in schema.get("properties", []) + ) + ) + ) + ): + return schema + if not action_name: + # Could not find anything with the given response_key. If there is any + # 200/204 response - return it + for code in ["200", "204"]: + if code in responses: + schema = ( + responses[code] + .get("content", {}) + .get("application/json", {}) + .get("schema") + ) + if schema and "type" in schema: + return schema + return None + + def get_resource_names_from_url(path: str): """Construct Resource name from the URL""" path_elements = list(filter(None, path.split("/"))) diff --git a/codegenerator/tests/unit/test_common.py b/codegenerator/tests/unit/test_common.py new file mode 100644 index 0000000..8aff4c6 --- /dev/null +++ b/codegenerator/tests/unit/test_common.py @@ -0,0 +1,119 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# +from unittest import TestCase + +from typing import Any + +from codegenerator import common + + +class TestFindResponseSchema(TestCase): + FOO = {"foo": {"type": "string"}} + + # def setUp(self): + # super().setUp() + # logging.basicConfig(level=logging.DEBUG) + + def test_find_with_single_candidate(self): + responses = { + "200": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": {**self.FOO}, + } + } + } + } + } + self.assertEqual( + responses["200"]["content"]["application/json"]["schema"], + common.find_response_schema(responses, "foo"), + ) + + def test_find_with_list(self): + responses = { + "200": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "foos": {"type": "array", "items": self.FOO} + }, + } + } + } + } + } + self.assertEqual( + responses["200"]["content"]["application/json"]["schema"], + common.find_response_schema(responses, "foo"), + ) + + def test_find_correct_action(self): + foo_action = { + "type": "string", + "x-openstack": {"action-name": "foo-action"}, + } + bar_action = { + "type": "string", + "x-openstack": {"action-name": "bar-action"}, + } + responses: dict[str, Any] = { + "200": { + "content": { + "application/json": { + "schema": {"type": "object", "properties": self.FOO} + } + } + }, + "204": { + "content": { + "application/json": { + "schema": {"oneOf": [foo_action, bar_action]} + } + } + }, + } + self.assertEqual( + foo_action, + common.find_response_schema(responses, "foo", "foo-action"), + ) + self.assertEqual( + bar_action, + common.find_response_schema(responses, "foo", "bar-action"), + ) + self.assertIsNone( + common.find_response_schema(responses, "foo", "baz-action"), + ) + self.assertEqual( + responses["200"]["content"]["application/json"]["schema"], + common.find_response_schema(responses, "foo"), + ) + + def test_no_candidates_returns_root(self): + responses = { + "200": { + "content": { + "application/json": { + "schema": self.FOO["foo"], + } + } + } + } + self.assertEqual( + responses["200"]["content"]["application/json"]["schema"], + common.find_response_schema(responses, "foo"), + ) From d680595c581ad221c5a22b78daec5402ca6b1f2d Mon Sep 17 00:00:00 2001 From: gtema Date: Thu, 8 Feb 2024 15:17:14 +0100 Subject: [PATCH 3/6] fix compute keypair responses fix structure of the keypair responses --- codegenerator/openapi/nova.py | 2 +- codegenerator/openapi/nova_schemas.py | 21 ++++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/codegenerator/openapi/nova.py b/codegenerator/openapi/nova.py index 0fe4f91..90786ec 100644 --- a/codegenerator/openapi/nova.py +++ b/codegenerator/openapi/nova.py @@ -491,7 +491,7 @@ def _get_schema_ref( ref = f"#/components/schemas/{name}" elif name == "Os_KeypairShowResponse": schema = openapi_spec.components.schemas.setdefault( - name, TypeSchema(**nova_schemas.KEYPAIR_SCHEMA) + name, TypeSchema(**nova_schemas.KEYPAIR_CONTAINER_SCHEMA) ) ref = f"#/components/schemas/{name}" elif name == "Os_KeypairsCreateResponse": diff --git a/codegenerator/openapi/nova_schemas.py b/codegenerator/openapi/nova_schemas.py index 3f1ee18..c14741c 100644 --- a/codegenerator/openapi/nova_schemas.py +++ b/codegenerator/openapi/nova_schemas.py @@ -780,7 +780,12 @@ "keypairs": { "type": "array", "description": "Array of Keypair objects", - "items": copy.deepcopy(KEYPAIR_SHORT_SCHEMA), + "items": { + "type": "object", + "properties": { + "keypair": copy.deepcopy(KEYPAIR_SHORT_SCHEMA), + }, + }, }, "keypairs_links": copy.deepcopy(LINKS_SCHEMA), }, @@ -818,8 +823,18 @@ }, } -KEYPAIR_CREATED_SCHEMA: dict[str, Any] = copy.deepcopy(KEYPAIR_SCHEMA) -KEYPAIR_CREATED_SCHEMA["properties"]["private_key"] = { +KEYPAIR_CONTAINER_SCHEMA: dict[str, Any] = { + "type": "object", + "description": "Keypair object", + "properties": {"keypair": KEYPAIR_SCHEMA}, +} + +KEYPAIR_CREATED_SCHEMA: dict[str, Any] = copy.deepcopy( + KEYPAIR_CONTAINER_SCHEMA +) +KEYPAIR_CREATED_SCHEMA["properties"]["keypair"]["properties"][ + "private_key" +] = { "type": "string", "description": "If you do not provide a public key on create, a new keypair will be built for you, and the private key will be returned during the initial create call. Make sure to save this, as there is no way to get this private key again in the future.", "x-openstack": {"max-ver": "2.91"}, From 6e4b64a46f06283cf598ad2ca88be33c291b6e94 Mon Sep 17 00:00:00 2001 From: gtema Date: Thu, 8 Feb 2024 15:46:03 +0100 Subject: [PATCH 4/6] fix cinder metadata(meta) schema Metadata is can not really be "none" --- codegenerator/openapi/cinder.py | 43 ++++++++++++++++- codegenerator/openapi/cinder_schemas.py | 63 ++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/codegenerator/openapi/cinder.py b/codegenerator/openapi/cinder.py index 1d082d9..1c3625b 100644 --- a/codegenerator/openapi/cinder.py +++ b/codegenerator/openapi/cinder.py @@ -193,6 +193,8 @@ def _get_schema_ref( "VolumesMetadataUpdate_All", "VolumesMetadataUpdate_AllResponse", "VolumesMetadataCreateResponse", + "VolumesActionOs-Set_Image_MetadataResponse", + "VolumesActionOs-Show_Image_MetadataResponse", ]: openapi_spec.components.schemas.setdefault( name, TypeSchema(**cinder_schemas.METADATA_CONTAINER_SCHEMA) @@ -207,7 +209,46 @@ def _get_schema_ref( name, TypeSchema(**cinder_schemas.METADATA_ITEM_SCHEMA) ) ref = f"#/components/schemas/{name}" - + # Volume Actions + elif name == "VolumesActionRevertResponse": + return (None, None) + elif name == "VolumesActionOs-Reset_StatusRequest": + openapi_spec.components.schemas.setdefault( + name, TypeSchema(**cinder_schemas.VOLUME_RESET_STATUS_SCHEMA) + ) + ref = f"#/components/schemas/{name}" + elif name in [ + "VolumesActionOs-Reset_StatusResponse", + "VolumesActionOs-Force_DeleteResponse", + "VolumesActionOs-Force_DetachResponse", + "VolumesActionOs-Migrate_VolumeResponse", + "VolumesActionOs-Migrate_Volume_CompletionResponse", + "VolumesActionOs-AttachResponse", + "VolumesActionOs-DetachResponse", + "VolumesActionOs-ReserveResponse", + "VolumesActionOs-UnreserveResponse", + "VolumesActionOs-Begin_DetachingResponse", + "VolumesActionOs-Roll_DetachingResponse", + "VolumesActionOs-Initialize_ConnectionResponse", + "VolumesActionOs-Terminate_ConnectionResponse", + "VolumesActionOs-ExtendResponse", + "VolumesActionOs-Update_Readonly_FlagResponse", + "VolumesActionOs-RetypeResponse", + "VolumesActionOs-Set_BootableResponse", + "VolumesActionOs-ReimageResponse", + "VolumesActionOs-Unset_Image_MetadataResponse", + "VolumesActionOs-UnmanageResponse", + ]: + return (None, None) + elif name == "VolumesActionOs-Volume_Upload_ImageResponse": + openapi_spec.components.schemas.setdefault( + name, + TypeSchema( + **cinder_schemas.VOLUME_UPLOAD_IMAGE_RESPONSE_SCHEMA + ), + ) + ref = f"#/components/schemas/{name}" + # Default else: (ref, mime_type) = super()._get_schema_ref( openapi_spec, name, description, action_name=action_name diff --git a/codegenerator/openapi/cinder_schemas.py b/codegenerator/openapi/cinder_schemas.py index 82f572c..46a722c 100644 --- a/codegenerator/openapi/cinder_schemas.py +++ b/codegenerator/openapi/cinder_schemas.py @@ -13,6 +13,8 @@ import copy from typing import Any +from cinder.api.schemas import admin_actions + # NOTE(gtema): This is a temporary location for schemas not currently defined # in Glance. Once everything is stabilized those must be moved directly to Glabne @@ -50,7 +52,7 @@ } METADATA_SCHEMA = { - "type": ["null", "object"], + "type": "object", "patternProperties": { "^[a-zA-Z0-9-_:. /]{1,255}$": {"type": "string", "maxLength": 255}, }, @@ -352,3 +354,62 @@ "x-openstack": {"min-ver": "3.65"}, }, } + +VOLUME_RESET_STATUS_SCHEMA: dict[str, Any] = admin_actions.reset + +VOLUME_UPLOAD_IMAGE_RESPONSE_SCHEMA: dict[str, Any] = { + "type": "object", + "properties": { + "container_format": { + "type": "string", + "description": "Container format for the new image. Default is bare.", + }, + "disk_format": { + "type": "string", + "description": "Disk format for the new image. Default is raw.", + }, + "display_description": { + "type": "string", + "description": "The volume description.", + }, + "id": { + "type": "string", + "format": "uuid", + "description": "The UUID of the volume.", + }, + "image_id": { + "type": "string", + "format": "uuid", + "description": "The uuid for the new image.", + }, + "image_name": { + "type": "string", + "description": "The name for the new image.", + }, + "protected": { + "type": "boolean", + "description": "Whether the new image is protected. Default=False.", + "x-openstack": {"min-ver": "3.1"}, + }, + "size": { + "type": "integer", + "format": "int64", + "description": "The size of the volume, in gibibytes (GiB).", + }, + "status": {"type": "integer", "description": "The volume status."}, + "updated_at": { + "type": "string", + "format": "date-time", + "description": "The date and time when the resource was updated.", + }, + "visibility": { + "type": "string", + "description": "The visibility property of the new image. Default is private.", + "x-openstack": {"min-ver": "3.1"}, + }, + "volume_type": { + "type": "string", + "description": "The associated volume type name for the volume.", + }, + }, +} From a8f0fa352187e83135cb5f03debbd68ecade9bc0 Mon Sep 17 00:00:00 2001 From: gtema Date: Thu, 8 Feb 2024 15:48:39 +0100 Subject: [PATCH 5/6] fix: further improve generator de-duplicate code and address few new clippy findings in generated code --- codegenerator/rust_cli.py | 103 +++++++----------- .../templates/rust_cli/response_struct.j2 | 8 +- .../templates/rust_cli/set_body_parameters.j2 | 2 +- codegenerator/templates/rust_macros.j2 | 2 +- 4 files changed, 47 insertions(+), 68 deletions(-) diff --git a/codegenerator/rust_cli.py b/codegenerator/rust_cli.py index a52628b..3b83c10 100644 --- a/codegenerator/rust_cli.py +++ b/codegenerator/rust_cli.py @@ -264,6 +264,18 @@ def get_structable_macros( class StructResponse(common_rust.Struct): field_type_class_: Type[common_rust.StructField] = StructFieldResponse + @property + def imports(self): + imports: set[str] = set(["serde::Deserialize"]) + for field in self.fields.values(): + imports.update(field.data_type.imports) + # In difference to the SDK and Input we do not currently handle + # additional_fields of the struct in response + # if self.additional_fields_type: + # imports.add("std::collections::BTreeMap") + # imports.update(self.additional_fields_type.imports) + return imports + class TupleStruct(common_rust.Struct): """Rust tuple struct without named fields""" @@ -655,7 +667,7 @@ def _get_array_type(self, type_model: model.Array) -> common_rust.Array: type_model.item_type, simplified_data_type, ) - self.ignored_models.append(only_field) + self.ignored_models.append(type_model.item_type) item_type = simplified_data_type elif isinstance(item_type, DictionaryInput): # Array of Freestyle objects in CLI can be only represented as @@ -1007,64 +1019,22 @@ def generate( # Process response information # # Prepare information about response if method.upper() != "HEAD": - response_def = None - for code, rspec in spec["responses"].items(): - if not code.startswith("2"): - continue - content = rspec.get("content", {}) - if "application/json" in content: - response_spec = content["application/json"] - oneof = response_spec["schema"].get("oneOf") - discriminator = ( - response_spec["schema"] - .get("x-openstack", {}) - .get("discriminator") - ) - if oneof: - if not discriminator: - # Server returns server or reservation info. For the cli it is not very helpful and we look for response candidate with the resource_name in the response - for candidate in oneof: - if ( - args.operation_type == "action" - and candidate.get( - "x-openstack", {} - ).get("action-name") - == args.operation_name - ): - if resource_name in candidate.get( - "properties", {} - ): - # If there is a object with - # resource_name in the props - - # this must be what we want to - # look at - response_def = candidate[ - "properties" - ][resource_name] - else: - response_def = candidate - break - elif ( - resource_name - and candidate.get("type") == "object" - and resource_name - in candidate.get("properties", {}) - ): - # Actually for the sake of the CLI it may make sense to merge all candidates - response_def = candidate["properties"][ - resource_name - ] - else: - raise NotImplementedError - else: - response_def, _ = common.find_resource_schema( - response_spec["schema"], - None, - args.response_key or resource_name, - ) - if not response_def: - continue + response = common.find_response_schema( + spec["responses"], + args.response_key or resource_name, + args.operation_name + if args.operation_type == "action" + else None, + ) + + if response: + response_def, _ = common.find_resource_schema( + response, + None, + args.response_key or resource_name, + ) + if response_def: if response_def.get("type", "object") == "object" or ( # BS metadata is defined with type: ["object", # "null"] @@ -1077,8 +1047,8 @@ def generate( response_type_manager.set_models(response_types) if method == "patch" and not request_types: # image patch is a jsonpatch based operation - # where there is no request. For it we need - # to look at the response and get writable + # where there is no request. For it we need to + # look at the response and get writable # parameters as a base is_json_patch = True if not args.find_implemented_by_sdk: @@ -1114,9 +1084,16 @@ def generate( response_type_manager.refs[ model.Reference(name="Body", type=TupleStruct) ] = tuple_struct - response_props = response_spec["schema"].get( - "properties", {} - ) + elif ( + response_def["type"] == "array" + and "items" in response_def + ): + (_, response_types) = openapi_parser.parse( + response_def["items"] + ) + response_type_manager.set_models(response_types) + + response_props = response.get("properties", {}) if ( response_props and response_props[ diff --git a/codegenerator/templates/rust_cli/response_struct.j2 b/codegenerator/templates/rust_cli/response_struct.j2 index b3e20c2..065b348 100644 --- a/codegenerator/templates/rust_cli/response_struct.j2 +++ b/codegenerator/templates/rust_cli/response_struct.j2 @@ -66,10 +66,12 @@ rows.extend( self.0 .iter() - {%- if data_type.value_type.type_hint == "String" %} - .map(|(k, v)| Vec::from([k.clone(), v.clone()])), - {%- elif data_type.value_type.type_hint == "Value" %} + {%- if data_type.value_type.type_hint == "Value" %} .map(|(k, v)| Vec::from([k.clone(), serde_json::to_string(&v).expect("Is a valid data")])), + {%- elif data_type.value_type.type_hint == "String" %} + .map(|(k, v)| Vec::from([k.clone(), v.clone()])), + {%- else %} + .map(|(k, v)| Vec::from([k.clone(), v.to_string()])), {%- endif %} ); (headers, rows) diff --git a/codegenerator/templates/rust_cli/set_body_parameters.j2 b/codegenerator/templates/rust_cli/set_body_parameters.j2 index dd00b89..2fba9bd 100644 --- a/codegenerator/templates/rust_cli/set_body_parameters.j2 +++ b/codegenerator/templates/rust_cli/set_body_parameters.j2 @@ -35,7 +35,7 @@ {{ builder_name }}.{{ v.remote_name }}(secret.to_string()); } {%- else %} - {{ macros.set_request_data_from_input(builder_name, v, "args." + v.local_name) }} + {{ macros.set_request_data_from_input(builder_name, v, "&args." + v.local_name) }} {%- endif %} {% endfor %} diff --git a/codegenerator/templates/rust_macros.j2 b/codegenerator/templates/rust_macros.j2 index c941853..d7ccc14 100644 --- a/codegenerator/templates/rust_macros.j2 +++ b/codegenerator/templates/rust_macros.j2 @@ -135,7 +135,7 @@ Some({{ val }}) {%- elif param.type_hint in ["i32", "i64", "f32", "f64", "bool"] %} {{ dst_var }}.{{ param.remote_name }}({{ val_var | replace("&", "" )}}); {%- elif param.data_type.__class__.__name__ in ["ArrayInput"] %} - {{ sdk_plain_array_setter(param, val_var, dst_var) }} + {{ sdk_plain_array_setter(param, val_var.replace("&", ""), dst_var) }} {%- elif param.data_type.__class__.__name__ in ["JsonValue"] %} {{ dst_var }}.{{ param.remote_name }}({{ val_var | replace("&", "" )}}.clone()); {%- elif param.data_type.__class__.__name__ == "DictionaryInput" %} From 11ce53b20d109239ffbafd11c8894ba85ae54ab6 Mon Sep 17 00:00:00 2001 From: gtema Date: Thu, 8 Feb 2024 18:05:46 +0100 Subject: [PATCH 6/6] chore: improve unused imports handling --- codegenerator/common/rust.py | 32 ++++++++++++++++---- codegenerator/rust_cli.py | 5 +++ codegenerator/templates/rust_sdk/find.rs.j2 | 10 +++--- codegenerator/templates/rust_sdk/impl.rs.j2 | 16 +++++----- codegenerator/templates/rust_sdk/subtypes.j2 | 24 ++++++++++++++- 5 files changed, 67 insertions(+), 20 deletions(-) diff --git a/codegenerator/common/rust.py b/codegenerator/common/rust.py index 86a1f2a..dffc54c 100644 --- a/codegenerator/common/rust.py +++ b/codegenerator/common/rust.py @@ -252,9 +252,19 @@ def type_hint(self): @property def imports(self): - imports: set[str] = set(["serde::Deserialize"]) - for field in self.fields.values(): - imports.update(field.data_type.imports) + imports: set[str] = set([]) + field_types = [x.data_type for x in self.fields.values()] + if len(field_types) > 1 or ( + len(field_types) == 1 + and not isinstance(field_types[0], Null) + and not isinstance(field_types[0], Dictionary) + and not isinstance(field_types[0], Array) + ): + # We use structure only if it is not consisting from only Null + imports.add("serde::Deserialize") + imports.add("serde::Serialize") + for field_type in field_types: + imports.update(field_type.imports) if self.additional_fields_type: imports.add("std::collections::BTreeMap") imports.update(self.additional_fields_type.imports) @@ -305,6 +315,8 @@ def type_hint(self): @property def imports(self): imports: set[str] = set() + imports.add("serde::Deserialize") + imports.add("serde::Serialize") for kind in self.kinds.values(): imports.update(kind.data_type.imports) return imports @@ -325,7 +337,7 @@ def clap_macros(self) -> set[str]: class StringEnum(BaseCompoundType): base_type: str = "enum" variants: dict[str, set[str]] = {} - imports: set[str] = set([]) + imports: set[str] = set(["serde::Deserialize", "serde::Serialize"]) lifetimes: set[str] = set() derive_container_macros: str = ( "#[derive(Debug, Deserialize, Clone, Serialize)]" @@ -765,6 +777,9 @@ def _simplify_oneof_combinations(self, type_model, kinds): elif string_klass in kinds_classes and dict_klass in kinds_classes: # oneOf [string, dummy object] => JsonValue # Simple string can be easily represented by JsonValue + for c in kinds: + # Discard dict + self.ignored_models.append(c["model"]) kinds.clear() jsonval_klass = self.primitive_type_mapping[model.PrimitiveAny] kinds.append({"local": jsonval_klass(), "class": jsonval_klass}) @@ -880,8 +895,11 @@ def get_root_data_type(self): def get_imports(self): """Get complete set of additional imports required by all models in scope""" imports: set[str] = set() - for item in self.refs.values(): - imports.update(item.imports) + imports.update(self.get_root_data_type().imports) + for subt in self.get_subtypes(): + imports.update(subt.imports) + # for item in self.refs.values(): + # imports.update(item.imports) for param in self.parameters.values(): imports.update(param.data_type.imports) return imports @@ -904,6 +922,8 @@ def subtype_requires_private_builders(self, subtype) -> bool: for field in subtype.fields.values(): if "private" in field.builder_macros: return True + if isinstance(subtype, Struct) and subtype.additional_fields_type: + return True return False def set_parameters(self, parameters: list[model.RequestParameter]) -> None: diff --git a/codegenerator/rust_cli.py b/codegenerator/rust_cli.py index 3b83c10..6b49446 100644 --- a/codegenerator/rust_cli.py +++ b/codegenerator/rust_cli.py @@ -1152,6 +1152,7 @@ def generate( ) if args.operation_type == "download": additional_imports.add("crate::common::download_file") + if args.operation_type == "upload": additional_imports.add( "crate::common::build_upload_asyncread" @@ -1212,11 +1213,15 @@ def generate( ] ) ) + # Discard unnecessry imports + additional_imports.discard("http::Response") + additional_imports.discard("bytes::Bytes") additional_imports.update(type_manager.get_imports()) additional_imports.update(response_type_manager.get_imports()) # Deserialize is already in template since it is uncoditionally required additional_imports.discard("serde::Deserialize") + additional_imports.discard("serde::Serialize") command_description: str = spec.get("description") command_summary: str = spec.get("summary") diff --git a/codegenerator/templates/rust_sdk/find.rs.j2 b/codegenerator/templates/rust_sdk/find.rs.j2 index cb739d7..4c47859 100644 --- a/codegenerator/templates/rust_sdk/find.rs.j2 +++ b/codegenerator/templates/rust_sdk/find.rs.j2 @@ -17,15 +17,13 @@ {% import 'rust_macros.j2' as macros with context -%} use derive_builder::Builder; use http::{HeaderMap, HeaderName, HeaderValue}; -use serde::de::DeserializeOwned; -use tracing::trace; -use crate::api::common::CommaSeparatedList; use crate::api::find::Findable; use crate::api::rest_endpoint_prelude::*; -use crate::api::ParamValue; - -use crate::api::{ApiError, Client, Pageable, Query, RestClient}; +{%- if not name_filter_supported %} +use crate::api::{ApiError, RestClient}; +use tracing::trace; +{%- endif %} use crate::api::{{ mod_path | join("::") }}::{ get as Get, diff --git a/codegenerator/templates/rust_sdk/impl.rs.j2 b/codegenerator/templates/rust_sdk/impl.rs.j2 index 1dd0dd6..dd91c67 100644 --- a/codegenerator/templates/rust_sdk/impl.rs.j2 +++ b/codegenerator/templates/rust_sdk/impl.rs.j2 @@ -20,7 +20,6 @@ use derive_builder::Builder; use http::{HeaderMap, HeaderName, HeaderValue}; use crate::api::rest_endpoint_prelude::*; -use serde::Serialize; {% for mod in type_manager.get_imports() | sort %} use {{ mod }}; @@ -30,7 +29,7 @@ use {{ mod }}; use json_patch::Patch; {%- endif %} -{%- if operation_type == "list" %} +{%- if operation_type == "list" and "limit" in type_manager.parameters.keys() or "marker" in type_manager.parameters.keys() %} use crate::api::Pageable; {%- endif %} @@ -266,14 +265,17 @@ impl{{ type_manager.get_request_static_lifetimes(request) }} Pageable for Reques #[cfg(test)] mod tests { + #![allow(unused_imports)] use super::*; - use crate::api::{self, Query, RawQuery}; +{%- if method.upper() == "HEAD" %} + use crate::api::RawQuery; +{%- else %} + use crate::api::Query; + use serde_json::json; +{%- endif %} use crate::types::ServiceType; use crate::test::client::MockServerClient; use http::{HeaderName, HeaderValue}; - use serde::Deserialize; - use serde::Serialize; - use serde_json::json; {%- if is_json_patch %} use serde_json::from_value; use json_patch::Patch; @@ -370,7 +372,7 @@ mod tests { .header("not_foo", "not_bar") .build() .unwrap(); - {%- if method.upper() != "HEAD" %} +{%- if method.upper() != "HEAD" %} let _: serde_json::Value = endpoint.query(&client).unwrap(); {%- else %} let _ = endpoint.raw_query(&client).unwrap(); diff --git a/codegenerator/templates/rust_sdk/subtypes.j2 b/codegenerator/templates/rust_sdk/subtypes.j2 index af780ce..65940fe 100644 --- a/codegenerator/templates/rust_sdk/subtypes.j2 +++ b/codegenerator/templates/rust_sdk/subtypes.j2 @@ -32,6 +32,12 @@ pub {{ subtype.base_type }} {{ subtype.name }}{{ ("<" + ",".join(subtype.lifetim {{ k }}, {%- endfor %} {%- endif %} + + {%- if subtype.base_type == "struct" and subtype.additional_fields_type %} + + #[builder(setter(name = "_properties"), default, private)] + _properties: BTreeMap, {{ subtype.additional_fields_type.type_hint }}>, + {%- endif %} } {% if type_manager.subtype_requires_private_builders(subtype) %} @@ -42,6 +48,22 @@ impl{{ ("<" + ",".join(subtype.lifetimes) + ">") if subtype.lifetimes else ""}} {{ macros.sdk_builder_setter(field)}} {%- endif %} {%- endfor %} + + {% if subtype.additional_fields_type is defined and subtype.additional_fields_type %} + pub fn properties(&mut self, iter: I) -> &mut Self + where + I: Iterator, + K: Into>, + V: Into<{{ subtype.additional_fields_type.type_hint }}>, + { + self._properties + .get_or_insert_with(BTreeMap::new) + .extend(iter.map(|(k, v)| (k.into(), v.into()))); + self + } + + {%- endif %} + } -{%- endif %} +{% endif %} {%- endfor %}