From ff6e36843e66caf937cdd8708309c36337f9f496 Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Mon, 2 Sep 2024 17:59:37 +0200 Subject: [PATCH] Add resource link in path parameters When path_parameter contain something like `user_id` it is more efficient (and provides a UX with more control) to accept `user-id|user-name` from the user. This is achieved by introducing resource links in the path parameters. When link is present parameter is being processed differently. Modify Keystone schema generation to insert link to the user everywhere where `user_id` path parameter is present. Change-Id: I8bf3d14dc751bdb97111a63dddfd0b604cf06abc --- codegenerator/common/rust.py | 2 + codegenerator/model.py | 6 ++- codegenerator/openapi/keystone.py | 9 ++++ codegenerator/rust_cli.py | 15 +++++- .../templates/rust_cli/path_parameters.j2 | 26 +++++++++- codegenerator/templates/rust_macros.j2 | 51 ++++++++++++++++--- 6 files changed, 100 insertions(+), 9 deletions(-) diff --git a/codegenerator/common/rust.py b/codegenerator/common/rust.py index c841e83..dfd6dff 100644 --- a/codegenerator/common/rust.py +++ b/codegenerator/common/rust.py @@ -397,6 +397,7 @@ class RequestParameter(BaseModel): description: str | None = None is_required: bool = False is_flag: bool = False + resource_link: str | None = None setter_name: str | None = None setter_type: str | None = None @@ -1031,6 +1032,7 @@ class TypeManager: description=sanitize_rust_docstrings(parameter.description), is_required=parameter.is_required, is_flag=parameter.is_flag, + resource_link=parameter.resource_link, ) self.parameters[param.local_name] = param diff --git a/codegenerator/model.py b/codegenerator/model.py index 603b47f..b6ca12b 100644 --- a/codegenerator/model.py +++ b/codegenerator/model.py @@ -661,6 +661,7 @@ class RequestParameter(BaseModel): description: str | None = None is_required: bool = False is_flag: bool = False + resource_link: str | None = None class OpenAPISchemaParser(JsonSchemaParser): @@ -737,7 +738,9 @@ class OpenAPISchemaParser(JsonSchemaParser): is_flag: bool = False os_ext = schema.get("x-openstack", {}) if not isinstance(os_ext, dict): - raise RuntimeError(f"x-openstack must be a dictionary in {schema}") + raise RuntimeError( + f"x-openstack must be a dictionary inside {schema}" + ) if "is-flag" in os_ext: is_flag = os_ext["is-flag"] @@ -749,6 +752,7 @@ class OpenAPISchemaParser(JsonSchemaParser): description=schema.get("description"), is_required=schema.get("required", False), is_flag=is_flag, + resource_link=os_ext.get("resource_link", None), ) raise NotImplementedError("Parameter %s is not covered yet" % schema) diff --git a/codegenerator/openapi/keystone.py b/codegenerator/openapi/keystone.py index 3636fc7..0546e6e 100644 --- a/codegenerator/openapi/keystone.py +++ b/codegenerator/openapi/keystone.py @@ -224,6 +224,15 @@ class KeystoneGenerator(OpenStackServerSourceBase): ) # We can only assume the param type. For path it is logically a string only path_param.type_schema = TypeSchema(type="string") + # For non /users/{id} urls link user_id path attribute to the user resource + if path_param.name == "user_id" and path_resource_names != [ + "users" + ]: + if not path_param.openstack: + path_param.openstack = {} + path_param.openstack["resource_link"] = ( + "identity/v3/user.id" + ) openapi_spec.components.parameters[global_param_name] = ( path_param ) diff --git a/codegenerator/rust_cli.py b/codegenerator/rust_cli.py index 2e94ba4..16746e2 100644 --- a/codegenerator/rust_cli.py +++ b/codegenerator/rust_cli.py @@ -1069,6 +1069,7 @@ class RustCliGenerator(BaseGenerator): target_class_name = resource_name is_image_download: bool = False is_json_patch: bool = False + global_additional_imports: set[str] = set() # Collect all operation parameters for param in openapi_spec["paths"][path].get( @@ -1086,6 +1087,18 @@ class RustCliGenerator(BaseGenerator): # for i.e. routers/{router_id} we want local_name to be `id` and not `router_id` param_.name = "id" operation_params.append(param_) + if param_.resource_link: + link_res_name: str = param_.resource_link.split(".")[0] + global_additional_imports.add("tracing::warn") + global_additional_imports.add( + "openstack_sdk::api::find_by_name" + ) + global_additional_imports.add( + "openstack_sdk::api::QueryAsync" + ) + global_additional_imports.add( + f"openstack_sdk::api::{'::'.join(link_res_name.split('/'))}::find as find_{link_res_name.split('/')[-1]}" + ) # List of operation variants (based on the body) operation_variants = common.get_operation_variants( @@ -1108,7 +1121,7 @@ class RustCliGenerator(BaseGenerator): for operation_variant in operation_variants: logging.debug("Processing variant %s" % operation_variant) - additional_imports = set() + additional_imports = set(global_additional_imports) type_manager: common_rust.TypeManager = RequestTypeManager() response_type_manager: common_rust.TypeManager = ( ResponseTypeManager() diff --git a/codegenerator/templates/rust_cli/path_parameters.j2 b/codegenerator/templates/rust_cli/path_parameters.j2 index 0e65183..4e798a5 100644 --- a/codegenerator/templates/rust_cli/path_parameters.j2 +++ b/codegenerator/templates/rust_cli/path_parameters.j2 @@ -3,10 +3,34 @@ #[derive(Args)] struct PathParameters { {%- for param in type_manager.parameters.values() %} -{%- if param.location == "path"%} +{%- if param.location == "path" %} + {%- if not param.resource_link %} {{ macros.docstring(param.description, indent=4) }} {{ param.clap_macros }} {{ param.local_name }}: {{ param.type_hint }}, + {%- else %} + {% set res_name = param.resource_link.split(".")[0].split('/')[-1] %} + /// {{ res_name | title }} resource for which the operation should be performed. + #[command(flatten)] + {{ res_name }}: {{ res_name | title }}Input, + {%- endif %} {%- endif %} {%- endfor %} } + +{%- for (k, param) in type_manager.get_parameters("path") %} + {%- if param.resource_link %} + {% set res_name = param.resource_link.split(".")[0].split('/')[-1] %} +/// {{ res_name | title }} input select group +#[derive(Args)] +#[group(required = true, multiple = false)] +struct {{ res_name | title }}Input { + /// {{ res_name | title }} Name. + #[arg(long, help_heading = "Path parameters", value_name = "{{ res_name | upper }}_NAME")] + {{ res_name }}_name: Option, + /// {{ res_name | title }} ID. + #[arg(long, help_heading = "Path parameters", value_name = "{{ res_name | upper }}_ID")] + {{ res_name }}_id: Option, +} + {%- endif %} +{%- endfor %} diff --git a/codegenerator/templates/rust_macros.j2 b/codegenerator/templates/rust_macros.j2 index 5a9b1d2..701293d 100644 --- a/codegenerator/templates/rust_macros.j2 +++ b/codegenerator/templates/rust_macros.j2 @@ -381,27 +381,66 @@ Some({{ val }}) // Set path parameters {%- endif %} {%- for (k, v) in type_manager.get_parameters("path") %} -{%- if not v.is_required %} - {%- if k != "project_id" %} + {%- if v.resource_link %} + {%- set res_name = v.resource_link.split(".")[0].split('/')[-1] %} + + // Process path parameter `{{ k }}` + if let Some(id) = &self.path.{{ res_name }}.{{ res_name }}_id { + // {{ res_name }}_id is passed. No need to lookup + {{ builder }}.{{ v.remote_name }}(id); + } + else if let Some(name) = &self.path.user.user_name { + // {{ res_name }}_name is passed. Need to lookup resource + let mut find_builder = find_{{ res_name }}::Request::builder(); + warn!("Querying {{ res_name}} by name (because of `--{{res_name}}-name` parameter passed) may not be definite. This may fail in which case parameter `--{{res_name}}-id` should be used instead."); + + find_builder.id(name); + let find_ep = find_builder + .build() + .map_err(|x| OpenStackCliError::EndpointBuild(x.to_string()))?; + let find_data: serde_json::Value = find_by_name(find_ep).query_async(client).await?; + // Try to extract resource id + match find_data.get("id") { + Some(val) => match val.as_str() { + Some(id_str) => { + {{ builder }}.{{ v.remote_name }}(id_str.to_owned()); + } + None => { + return Err(OpenStackCliError::ResourceAttributeNotString( + serde_json::to_string(&val)?, + )) + } + }, + None => { + return Err(OpenStackCliError::ResourceAttributeMissing( + "id".to_string(), + )) + } + }; + } + {%- else %} + {%- if not v.is_required %} + {%- if k != "project_id" %} if let Some(val) = &self.path.{{ v.local_name }} { {{ builder }}.{{ v.local_name }}(val); } - {%- else %} + {%- else %} if let Some(val) = &self.path.{{ v.local_name }} { {{ builder }}.{{ v.local_name }}(val); } else { {{ builder }}.{{ v.local_name }}(client.get_current_project().expect("Project ID must be known").id); } - {%- endif %} + {%- endif %} {%- elif not find_mode and find_present and operation_type in ["show", "set", "download"] %} let resource_id = find_data["id"] .as_str() .expect("Resource ID is a string") .to_string(); {{ builder }}.{{ v.local_name }}(resource_id.clone()); -{%- else %} + {%- else %} {{ builder }}.{{ v.local_name }}(&self.path.{{ v.local_name }}); -{%- endif %} + {%- endif %} + {%- endif %} {%- endfor %} {%- endmacro %}