From d038d9d8d149e33a621ff9904c8964eb61d22d75 Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Fri, 5 Jul 2024 20:17:07 +0200 Subject: [PATCH] Consume decorated Keystone methods We started decorating Keystone with schema validation similarly to other services. Since Keystone uses Flask and thus we do not use base processing split the processing into separate method and call it. Change-Id: I4e70b85bb16a40cb3dff0d5ddc6004aabe0d1c62 --- codegenerator/openapi/base.py | 210 +++++++++++------- codegenerator/openapi/keystone.py | 81 +++++-- .../application_credential.py | 2 +- 3 files changed, 187 insertions(+), 106 deletions(-) diff --git a/codegenerator/openapi/base.py b/codegenerator/openapi/base.py index 7357aab..f1cd292 100644 --- a/codegenerator/openapi/base.py +++ b/codegenerator/openapi/base.py @@ -647,88 +647,20 @@ class OpenStackServerSourceBase: if action_name: operation_name = action_name - # Unwrap operation decorators to access all properties - f = func - while hasattr(f, "__wrapped__"): - closure = inspect.getclosurevars(f) - closure_locals = closure.nonlocals - min_ver = closure_locals.get("min_version", start_version) - if min_ver and not isinstance(min_ver, str): - min_ver = min_ver.get_string() - max_ver = closure_locals.get("max_version", end_version) - if max_ver and not isinstance(max_ver, str): - max_ver = max_ver.get_string() - - if "errors" in closure_locals: - expected_errors = closure_locals["errors"] - if isinstance(expected_errors, list): - expected_errors = [ - str(x) - for x in filter( - lambda x: isinstance(x, int), expected_errors - ) - ] - elif isinstance(expected_errors, int): - expected_errors = [str(expected_errors)] - if "request_body_schema" in closure_locals or hasattr( - f, "_request_body_schema" - ): - # Body type is known through method decorator - obj = closure_locals.get( - "request_body_schema", - getattr(f, "_request_body_schema", {}), - ) - if obj.get("type") in ["object", "array"]: - # We only allow object and array bodies - # To prevent type name collision keep module name part of the name - typ_name = ( - "".join([x.title() for x in path_resource_names]) - + func.__name__.title() - + (f"_{min_ver.replace('.', '')}" if min_ver else "") - ) - comp_schema = openapi_spec.components.schemas.setdefault( - typ_name, - self._sanitize_schema( - copy.deepcopy(obj), - start_version=start_version, - end_version=end_version, - ), - ) - - if min_ver: - if not comp_schema.openstack: - comp_schema.openstack = {} - comp_schema.openstack["min-ver"] = min_ver - if max_ver: - if not comp_schema.openstack: - comp_schema.openstack = {} - comp_schema.openstack["max-ver"] = max_ver - if mode == "action": - if not comp_schema.openstack: - comp_schema.openstack = {} - comp_schema.openstack["action-name"] = action_name - - ref_name = f"#/components/schemas/{typ_name}" - body_schemas.append(ref_name) - if "response_body_schema" in closure_locals or hasattr( - f, "_response_body_schema" - ): - # Response type is known through method decorator - PERFECT - obj = closure_locals.get( - "response_body_schema", - getattr(f, "_response_body_schema", {}), - ) - ser_schema = obj - if "query_params_schema" in closure_locals or hasattr( - f, "_request_query_schema" - ): - obj = closure_locals.get( - "query_params_schema", - getattr(f, "_request_query_schema", {}), - ) - query_params_versions.append((obj, min_ver, max_ver)) - - f = f.__wrapped__ + ( + query_params_versions, + body_schemas, + response_body_schema, + expected_errors, + ) = self._process_decorators( + func, + path_resource_names, + openapi_spec, + mode, + start_version, + end_version, + action_name, + ) if hasattr(func, "_wsme_definition"): fdef = getattr(func, "_wsme_definition") @@ -786,6 +718,8 @@ class OpenStackServerSourceBase: operation_name, ) + if ser_schema and not response_body_schema: + response_body_schema = ser_schema responses_spec = operation_spec.responses for error in expected_errors: responses_spec.setdefault(str(error), dict(description="Error")) @@ -830,7 +764,7 @@ class OpenStackServerSourceBase: if not action_name else f"Response of the {operation_spec.operationId}:{action_name} action" ), # noqa - schema_def=ser_schema, + schema_def=response_body_schema, action_name=action_name, ) @@ -910,7 +844,8 @@ class OpenStackServerSourceBase: **copy.deepcopy(spec["items"]) ) else: - raise RuntimeError("Error") + param_attrs["schema"] = TypeSchema(**copy.deepcopy(spec)) + param_attrs["description"] = spec.get("description") if min_ver: os_ext = param_attrs.setdefault("x-openstack", {}) os_ext["min-ver"] = min_ver @@ -1218,6 +1153,113 @@ class OpenStackServerSourceBase: response_code = "200" return [response_code] + def _process_decorators( + self, + func, + path_resource_names: list[str], + openapi_spec, + mode: str, + start_version, + end_version, + action_name: str | None, + ): + """Extract schemas from the decorated method.""" + # Unwrap operation decorators to access all properties + expected_errors: list[str] = [] + body_schemas: list[str] = [] + query_params_versions: list[tuple] = [] + response_body_schema: dict | None = None + + f = func + while hasattr(f, "__wrapped__"): + closure = inspect.getclosurevars(f) + closure_locals = closure.nonlocals + min_ver = closure_locals.get("min_version", start_version) + if min_ver and not isinstance(min_ver, str): + min_ver = min_ver.get_string() + max_ver = closure_locals.get("max_version", end_version) + if max_ver and not isinstance(max_ver, str): + max_ver = max_ver.get_string() + + if "errors" in closure_locals: + expected_errors = closure_locals["errors"] + if isinstance(expected_errors, list): + expected_errors = [ + str(x) + for x in filter( + lambda x: isinstance(x, int), expected_errors + ) + ] + elif isinstance(expected_errors, int): + expected_errors = [str(expected_errors)] + if "request_body_schema" in closure_locals or hasattr( + f, "_request_body_schema" + ): + # Body type is known through method decorator + obj = closure_locals.get( + "request_body_schema", + getattr(f, "_request_body_schema", {}), + ) + if obj.get("type") in ["object", "array"]: + # We only allow object and array bodies + # To prevent type name collision keep module name part of the name + typ_name = ( + "".join([x.title() for x in path_resource_names]) + + func.__name__.title() + + (f"_{min_ver.replace('.', '')}" if min_ver else "") + ) + comp_schema = openapi_spec.components.schemas.setdefault( + typ_name, + self._sanitize_schema( + copy.deepcopy(obj), + start_version=start_version, + end_version=end_version, + ), + ) + + if min_ver: + if not comp_schema.openstack: + comp_schema.openstack = {} + comp_schema.openstack["min-ver"] = min_ver + if max_ver: + if not comp_schema.openstack: + comp_schema.openstack = {} + comp_schema.openstack["max-ver"] = max_ver + if mode == "action": + if not comp_schema.openstack: + comp_schema.openstack = {} + comp_schema.openstack["action-name"] = action_name + + ref_name = f"#/components/schemas/{typ_name}" + body_schemas.append(ref_name) + + if "response_body_schema" in closure_locals or hasattr( + f, "_response_body_schema" + ): + # Response type is known through method decorator - PERFECT + obj = closure_locals.get( + "response_body_schema", + getattr(f, "_response_body_schema", {}), + ) + response_body_schema = obj + if "query_params_schema" in closure_locals or hasattr( + f, "_request_query_schema" + ): + obj = closure_locals.get( + "query_params_schema", + getattr(f, "_request_query_schema", {}), + ) + query_params_versions.append((obj, min_ver, max_ver)) + + f = f.__wrapped__ + + return ( + query_params_versions, + body_schemas, + response_body_schema, + expected_errors, + ) + def _convert_wsme_to_jsonschema(body_spec): """Convert WSME type description to JsonSchema""" diff --git a/codegenerator/openapi/keystone.py b/codegenerator/openapi/keystone.py index 5cfd47f..6c7d36a 100644 --- a/codegenerator/openapi/keystone.py +++ b/codegenerator/openapi/keystone.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. # +import copy import inspect from multiprocessing import Process import logging @@ -35,6 +36,7 @@ from codegenerator.openapi.keystone_schemas import role from codegenerator.openapi.keystone_schemas import service from codegenerator.openapi.keystone_schemas import user from codegenerator.openapi.utils import merge_api_ref_doc +from codegenerator.openapi.utils import rst_to_md class KeystoneGenerator(OpenStackServerSourceBase): @@ -151,10 +153,6 @@ class KeystoneGenerator(OpenStackServerSourceBase): for route in self.router.iter_rules(): if route.rule.startswith("/static"): continue - # if not route.rule.startswith("/v3/domains"): - # continue - if "/credentials/OS-EC2" in route.rule: - continue self._process_route(route, openapi_spec) @@ -358,26 +356,62 @@ class KeystoneGenerator(OpenStackServerSourceBase): path, method, ) + doc = inspect.getdoc(func) + if doc and not operation_spec.description: + doc = rst_to_md(doc) + operation_spec.description = LiteralScalarString(doc) + + query_params_versions = [] + body_schemas = [] + expected_errors = ["404"] + response_code = None + start_version = None + end_version = None + deser_schema: dict = {} + ser_schema: dict = {} + + ( + query_params_versions, + body_schemas, + ser_schema, + expected_errors, + ) = self._process_decorators( + func, + path_resource_names, + openapi_spec, + method, + start_version, + end_version, + None, + ) + + if query_params_versions: + so = sorted( + query_params_versions, + key=lambda d: ( + tuple(map(int, d[1].split("."))) if d[1] else (0, 0) + ), + ) + for data, min_ver, max_ver in so: + self.process_query_parameters( + openapi_spec, + operation_spec, + path_resource_names, + data, + min_ver, + max_ver, + ) + if method in ["PUT", "POST", "PATCH"]: - # This is clearly a modification operation but we know nothing about request - schema_name = ( - "".join([x.title() for x in path_resource_names]) - + method.title() - + "Request" - ) - - (schema_ref, mime_type) = self._get_schema_ref( + self.process_body_parameters( openapi_spec, - schema_name, - description=f"Request of the {operation_spec.operationId} operation", + operation_spec, + path_resource_names, + body_schemas, + None, + method, ) - if schema_ref: - content = operation_spec.requestBody = {"content": {}} - content["content"][mime_type] = { - "schema": {"$ref": schema_ref} - } - responses_spec = operation_spec.responses # Errors for error in ["403", "404"]: @@ -420,6 +454,7 @@ class KeystoneGenerator(OpenStackServerSourceBase): openapi_spec, schema_name, description=f"Response of the {operation_spec.operationId} operation", + schema_def=ser_schema, ) if schema_ref: @@ -486,7 +521,11 @@ class KeystoneGenerator(OpenStackServerSourceBase): # Default (ref, mime_type) = super()._get_schema_ref( - openapi_spec, name, description, action_name=action_name + openapi_spec, + name, + description, + schema_def=schema_def, + action_name=action_name, ) return (ref, mime_type) diff --git a/codegenerator/openapi/keystone_schemas/application_credential.py b/codegenerator/openapi/keystone_schemas/application_credential.py index 2383ce3..f771ea4 100644 --- a/codegenerator/openapi/keystone_schemas/application_credential.py +++ b/codegenerator/openapi/keystone_schemas/application_credential.py @@ -170,7 +170,7 @@ def _get_schema_ref( TypeSchema(**APPLICATION_CREDENTIAL_CREATE_SCHEMA), ) ref = f"#/components/schemas/{name}" - elif name in "UsersApplication_CredentialsPostResponse": + elif name == "UsersApplication_CredentialsPostResponse": openapi_spec.components.schemas.setdefault( name, TypeSchema(**APPLICATION_CREDENTIAL_CREATE_RESPONSE_SCHEMA),