From 5527731e17188ef4635fbc0949009434840a4892 Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Fri, 8 Nov 2024 20:57:47 +0100 Subject: [PATCH] Fix codegeneration for placement esoterics Some of the placement schemas are too special and user unfriendly that codegeneration currently fails for those. And since we could not even test generated code complilation we simply skipped such cases from enabling. Now try to fix codegeneration and see how it goes. Change-Id: I6c74ace134b2c8cb7017f1adacf2a38469fe5777 --- codegenerator/common/__init__.py | 4 ++ codegenerator/common/rust.py | 2 + codegenerator/metadata.py | 5 ++ codegenerator/model.py | 12 +++- codegenerator/openapi/base.py | 2 - .../openapi/placement_schemas/allocation.py | 6 +- .../placement_schemas/resource_class.py | 23 +++---- .../openapi/placement_schemas/usage.py | 3 + codegenerator/rust_cli.py | 36 +++++++--- .../templates/rust_cli/set_body_parameters.j2 | 14 ++++ codegenerator/templates/rust_macros.j2 | 65 ++++++++++++++----- codegenerator/templates/rust_sdk/impl.rs.j2 | 8 +++ metadata/placement_metadata.yaml | 2 +- 13 files changed, 137 insertions(+), 45 deletions(-) diff --git a/codegenerator/common/__init__.py b/codegenerator/common/__init__.py index 36f34c7..7851746 100644 --- a/codegenerator/common/__init__.py +++ b/codegenerator/common/__init__.py @@ -413,6 +413,10 @@ def get_operation_variants(spec: dict, operation_name: str): raise RuntimeError( f"Cannot find body specification for action {operation_name}" ) + else: + raise RuntimeError( + f"Unsupported discriminator {discriminator}" + ) else: operation_variants.append( {"body": json_body_schema, "mime_type": mime_type} diff --git a/codegenerator/common/rust.py b/codegenerator/common/rust.py index 8099df5..8128b90 100644 --- a/codegenerator/common/rust.py +++ b/codegenerator/common/rust.py @@ -513,6 +513,8 @@ class TypeManager: x.capitalize() for x in re.split(common.SPLIT_NAME_RE, model_ref.name) ) + if name[0].isdigit(): + return "x" + name return name def _get_adt_by_reference(self, model_ref): diff --git a/codegenerator/metadata.py b/codegenerator/metadata.py index 04bad38..6a77f71 100644 --- a/codegenerator/metadata.py +++ b/codegenerator/metadata.py @@ -1239,4 +1239,9 @@ def post_process_placement_operation( operation.operation_type = "list_from_struct" operation.targets["rust-cli"].response_key = "usages" operation.targets["rust-sdk"].response_key = "usages" + if operation_name == "delete_all": + operation.targets["rust-cli"].cli_full_command = operation.targets[ + "rust-cli" + ].cli_full_command.replace("delete-all", "purge") + return operation diff --git a/codegenerator/model.py b/codegenerator/model.py index ed6a7c3..9a4b3cd 100644 --- a/codegenerator/model.py +++ b/codegenerator/model.py @@ -17,6 +17,7 @@ import copy import hashlib import json import logging +import string from typing import Any from typing import Type import typing as ty @@ -393,11 +394,19 @@ class JsonSchemaParser: if pattern_properties: # `"type": "object", "pattern_properties": {...}}` + for key_pattern, value_type in pattern_properties.items(): type_kind: PrimitiveType | ADT = self.parse_schema( value_type, results, - name=name, + name=(name or "") + + ( + key_pattern.translate( + str.maketrans(dict.fromkeys(string.punctuation)) + ) + if len(pattern_properties.keys()) > 1 + else "Item" + ), min_ver=min_ver, max_ver=max_ver, ignore_read_only=ignore_read_only, @@ -498,6 +507,7 @@ class JsonSchemaParser: else: obj.reference.name = new_name results.append(obj) + return obj def parse_oneOf( diff --git a/codegenerator/openapi/base.py b/codegenerator/openapi/base.py index c1ab2db..de20908 100644 --- a/codegenerator/openapi/base.py +++ b/codegenerator/openapi/base.py @@ -642,7 +642,6 @@ class OpenStackServerSourceBase: end_version, action_name, ) - print(f"Bodies: {body_schemas} {mode}") if hasattr(func, "_wsme_definition"): fdef = getattr(func, "_wsme_definition") @@ -878,7 +877,6 @@ class OpenStackServerSourceBase: action_name, ): # Body is not expected, exit (unless we are in the "action") - print(f"mode={mode}") if body_schemas is None or (body_schemas == [] and mode != "action"): return mime_type: str | None = "application/json" diff --git a/codegenerator/openapi/placement_schemas/allocation.py b/codegenerator/openapi/placement_schemas/allocation.py index 6c38b9e..7138097 100644 --- a/codegenerator/openapi/placement_schemas/allocation.py +++ b/codegenerator/openapi/placement_schemas/allocation.py @@ -65,11 +65,7 @@ ALLOCATION_POST_REQUEST_SCHEMA: dict[str, Any] = { "x-openstack": {"min-ver": "1.34", "max-ver": "1.37"}, }, { - **allocation.ALLOCATION_SCHEMA_V1_38, - "x-openstack": {"min-ver": "1.38"}, - }, - { - **allocation.ALLOCATION_SCHEMA_V1_38, + **allocation.POST_ALLOCATIONS_V1_38, "x-openstack": {"min-ver": "1.38"}, }, ], diff --git a/codegenerator/openapi/placement_schemas/resource_class.py b/codegenerator/openapi/placement_schemas/resource_class.py index 3ed357b..948c405 100644 --- a/codegenerator/openapi/placement_schemas/resource_class.py +++ b/codegenerator/openapi/placement_schemas/resource_class.py @@ -53,18 +53,19 @@ RESOURCE_CLASSES_SCHEMA: dict[str, Any] = { RESOURCE_CLASS_UPDATE_REQUEST_SCHEMA: dict[str, Any] = { "oneOf": [ - { - "type": "object", - "properties": { - "name": { - "type": "string", - "description": "The name of one resource class.", - } - }, - "x-openstack": {"min-ver": "1.7"}, - } + # { + # "type": "object", + # "properties": { + # "name": { + # "type": "string", + # "description": "The name of one resource class.", + # } + # }, + # "x-openstack": {"min-ver": "1.2", "max-ver": "1.6"}, + # }, + {"type": "null", "x-openstack": {"min-ver": "1.7"}} ], - "discriminator": "microversion", + "x-openstack": {"discriminator": "microversion"}, } diff --git a/codegenerator/openapi/placement_schemas/usage.py b/codegenerator/openapi/placement_schemas/usage.py index 5800a47..7b88712 100644 --- a/codegenerator/openapi/placement_schemas/usage.py +++ b/codegenerator/openapi/placement_schemas/usage.py @@ -45,14 +45,17 @@ USAGE_LIST_PARAMETERS: dict[str, Any] = { "project_id": { "in": "query", "name": "project_id", + "required": True, "description": "The uuid of a project.", "schema": {"type": "string", "format": "uuid"}, + "x-openstack": {"resource_link": "identity/v3/project.id"}, }, "user_id": { "in": "query", "name": "user_id", "description": "The uuid of a user.", "schema": {"type": "string", "format": "uuid"}, + "x-openstack": {"resource_link": "identity/v3/user.id"}, }, "consumer_type": { "in": "query", diff --git a/codegenerator/rust_cli.py b/codegenerator/rust_cli.py index 267ce7a..bc95c04 100644 --- a/codegenerator/rust_cli.py +++ b/codegenerator/rust_cli.py @@ -602,15 +602,35 @@ class RequestTypeManager(common_rust.TypeManager): original_data_type=original_data_type, item_type=String(), ) - elif isinstance(type_model, model.Dictionary) and isinstance( - type_model.value_type, model.Dictionary - ): - original_data_type = self.convert_model(type_model.value_type) - typ = JsonValue( - original_data_type=DictionaryInput( - value_type=original_data_type + elif isinstance(type_model, model.Dictionary): + if isinstance(type_model.value_type, model.Dictionary): + original_data_type = self.convert_model(type_model.value_type) + typ = JsonValue( + original_data_type=DictionaryInput( + value_type=original_data_type + ) ) - ) + else: + if isinstance(type_model.value_type, model.Struct): + # Placement has dict of structs of dicts of structs ... + # Only simplify the top level stuff + original_data_type = self.convert_model( + type_model.value_type + ) + typ = DictionaryInput( + description=type_model.value_type.description, + value_type=JsonValue( + original_data_type=original_data_type + ), + ) + if not model_ref: + model_ref = model.Reference( + name="Body", type=typ.__class__ + ) + if type_model.value_type.reference: + self.ignored_models.append( + type_model.value_type.reference + ) if typ: if model_ref: diff --git a/codegenerator/templates/rust_cli/set_body_parameters.j2 b/codegenerator/templates/rust_cli/set_body_parameters.j2 index fbae51d..78183dd 100644 --- a/codegenerator/templates/rust_cli/set_body_parameters.j2 +++ b/codegenerator/templates/rust_cli/set_body_parameters.j2 @@ -71,7 +71,21 @@ .map(|(k, v)| (k, v.as_ref().map(Into::into))), ); {%- else %} + {%- set original_type = root.value_type.original_data_type %} + {%- if root.value_type.__class__.__name__ == "JsonValue" and original_type.__class__.__name__ == "StructInput" %} + {#- Placement hacks #} + ep_builder.properties( + properties + .into_iter() + .map(|(k, v)| { + serde_json::from_value(v.to_owned()).map(|v: {{ sdk_mod_path[-1] }}::{{ original_type.name }}| (k, v)) + }) + .collect::, _>>()? + .into_iter(), + ); + {%- else %} ep_builder.properties(properties.iter().cloned()); + {%- endif %} {%- endif %} } {%- endif %} diff --git a/codegenerator/templates/rust_macros.j2 b/codegenerator/templates/rust_macros.j2 index e1fa861..c68033b 100644 --- a/codegenerator/templates/rust_macros.j2 +++ b/codegenerator/templates/rust_macros.j2 @@ -72,17 +72,26 @@ Option {%- set dt = field.data_type if not is_opt else field.data_type.item_type %} {%- set val_type = field.data_type.value_type %} {{ docstring(field.description, indent=4) }} - pub fn {{ field.local_name }}(&mut self, iter: I) -> &mut Self + pub fn {{ field.local_name }}(&mut self, iter: I) -> &mut Self where I: Iterator, K: Into>, - {%- if val_type.__class__.__name__ != "BTreeMap" %} - V: Into<{{ dt.value_type.type_hint }}>, - {% else %} - V: Iterator, - K1: Into>, - V1: Into<{{ val_type.value_type.type_hint }}>, - {% endif%} + {%- if val_type.__class__.__name__ == "BTreeMap" %} + V: Iterator, + K1: Into>, + V1: Into<{{ val_type.value_type.type_hint }}>, + {%- elif val_type.__class__.__name__ == "Array" %} + V: IntoIterator, + V1: Into<{{ val_type.item_type.type_hint }}>, + {% else %} + V: Into<{{ dt.value_type.type_hint }}>, + {% endif%} { self.{{ field.local_name }} {%- if field.is_optional %} @@ -94,14 +103,16 @@ Option .get_or_insert_with(BTreeMap::new) .extend(iter.map(|(k, v)| ( k.into(), - {%- if val_type.__class__.__name__ != "BTreeMap" %} - v.into() - {%- else %} + {%- if val_type.__class__.__name__ == "BTreeMap" %} BTreeMap::from_iter( v.into_iter() .map(|(k1, v1)| {(k1.into(), v1.into())}) ) - {%- endif %} + {%- elif val_type.__class__.__name__ == "Array" %} + v.into_iter().map(Into::into).collect() + {%- else %} + v.into() + {%- endif %} ))); self } @@ -145,12 +156,17 @@ Some({{ val }}) {#- Macros to render setting Request data from CLI input #} {%- macro set_request_data_from_input(manager, dst_var, param, val_var) %} {%- set is_nullable = param.is_nullable if param.is_nullable is defined else False %} - {%- if param.type_hint in ["Option>", "Option>", "Option>"] %} {{ dst_var }}.{{ param.remote_name }}({{ "*" + val_var }}); {%- elif param.type_hint in ["Option", "Option", "Option", "Option", "Option"] %} + {%- if param.is_optional is defined and not param.is_optional %} + if let Some(val) = {{ val_var }} { + {{ dst_var }}.{{ param.remote_name }}(*val); + } + {%- else %} {{ dst_var }}.{{ param.remote_name }}({{ "*" + val_var }}); + {%- endif %} {%- elif param.type_hint in ["i32", "i64", "f32", "f64", "bool"] %} {{ dst_var }}.{{ param.remote_name }}({{ val_var | replace("&", "" )}}); @@ -274,10 +290,15 @@ Some({{ val }}) {%- if param.data_type.value_type.__class__.__name__ == "JsonValue" and original_type.__class__.__name__ == "StructInput" %} {% set builder_name = param.local_name + "_builder" %} {{ dst_var}}.{{ param.remote_name }}( + {%- if val_var.startswith("&self") %} + {{ val_var | replace("&", "") }} + {%- else %} {{ val_var }} - .into_iter() + {%- endif %} + .iter() .map(|(k, v)| { - serde_json::from_value(v.to_owned()).map(|v: {{ sdk_mod_path[-1] }}::{{ original_type.name }}| (k, v)) + {#- sdk uses Struct while cli uses StructInput -> thus replace #} + serde_json::from_value(v.to_owned()).map(|v: {{ sdk_mod_path[-1] }}::{{ original_type.name | replace("StructInput", "Struct") }}| (k, v)) }) .collect::, _>>()? .into_iter(), @@ -285,6 +306,16 @@ Some({{ val }}) {%- elif param.data_type.value_type.__class__.__name__ == "Option" %} {{ dst_var }}.{{ param.remote_name }}({{ val_var | replace("&", "") }}.iter().cloned().map(|(k, v)| (k, v.map(Into::into)))); + {%- elif param.data_type.value_type.__class__.__name__ == "JsonValue" and original_type.__class__.__name__ == "ArrayInput" %} + {{ dst_var }}.{{ param.remote_name }}( + {{ val_var }}.iter() + .map(|(k, v)| { + serde_json::from_value::>(v.to_owned()) + .map(|v| (k, v.into_iter())) + }) + .collect::, _>>()? + .into_iter(), + ); {%- else %} {{ dst_var }}.{{ param.remote_name }}({{ val_var | replace("&", "") }}.iter().cloned()); {%- endif %} @@ -322,7 +353,7 @@ Some({{ val }}) serde_json::from_value::<{{ sdk_mod_path[-1] }}::{{ original_type.name }}>(v.to_owned())) .collect::>(); {{ dst_var }}.{{ param.remote_name }}({{ builder_name }}); - {%- elif param.data_type.item_type.__class__.__name__ == "String" and original_item_type.__class__.__name__ == "StructInput" %} + {%- elif param.data_type.item_type.__class__.__name__ == "String" and original_item_type.__class__.__name__ == "StructInput" %} {#- Single field structure replaced with only string #} {%- set original_type = param.data_type.item_type.original_data_type %} {%- set original_field = original_type.fields[param.data_type.item_type.original_data_type.fields.keys()|list|first] %} @@ -335,7 +366,7 @@ Some({{ val }}) ) .collect(); {{ dst_var }}.{{ param.remote_name }}({{ builder_name }}); - {%- elif param.data_type.item_type.__class__.__name__ == "String" and original_type.__class__.__name__ == "ArrayInput" %} + {%- elif param.data_type.item_type.__class__.__name__ == "String" and original_type.__class__.__name__ == "ArrayInput" %} {#- Single field structure replaced with only string #} {{ dst_var }}.{{ param.remote_name }}( val.iter() diff --git a/codegenerator/templates/rust_sdk/impl.rs.j2 b/codegenerator/templates/rust_sdk/impl.rs.j2 index 8084c1f..80453f4 100644 --- a/codegenerator/templates/rust_sdk/impl.rs.j2 +++ b/codegenerator/templates/rust_sdk/impl.rs.j2 @@ -174,7 +174,11 @@ impl{{ type_manager.get_request_static_lifetimes(request) }} RestEndpoint for Re self.{{ param.local_name }}.as_ref() ); {%- elif not param.type_hint.startswith("BTreeSet") %} + {%- if param.is_required %} + params.push( + {%- else %} params.push_opt( + {%- endif %} "{{ param.remote_name }}", self.{{ param.local_name}} {%- if "Cow<" in param.type_hint %} @@ -232,7 +236,11 @@ impl{{ type_manager.get_request_static_lifetimes(request) }} RestEndpoint for Re let mut params = JsonBodyParams::default(); for (key, val) in &self._properties { + {%- if request.value_type.base_type is defined and request.value_type.base_type == "struct" %} + params.push(key.clone(), serde_json::to_value(val)?); + {%- else %} params.push(key.clone(), serde_json::Value::from(val.clone())); + {%- endif %} } params.into_body() diff --git a/metadata/placement_metadata.yaml b/metadata/placement_metadata.yaml index 06279da..328e7bb 100644 --- a/metadata/placement_metadata.yaml +++ b/metadata/placement_metadata.yaml @@ -164,7 +164,7 @@ resources: rust-cli: module_name: delete_all sdk_mod_name: delete_all - cli_full_command: resource-provider inventory delete-all + cli_full_command: resource-provider inventory purge show: operation_id: resource_providers/uuid/inventories/resource_class:get operation_type: show