From b353426fc70f8e0e23b647bd6599ec9237b0be97 Mon Sep 17 00:00:00 2001 From: Ruben Fonseca Date: Thu, 21 Mar 2024 14:49:47 +0100 Subject: [PATCH 1/3] fix(event_handler): always add 422 response to the schema --- .../event_handler/api_gateway.py | 44 +++++++++++-------- .../event_handler/openapi/types.py | 2 +- .../event_handler/test_openapi_responses.py | 4 +- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index fe51d68dab9..3139995e456 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -503,8 +503,23 @@ def _get_openapi_path( if request_body_oai: operation["requestBody"] = request_body_oai + # Validation failure response (422) will always be part of the schema + default_responses: Dict[int, OpenAPIResponse] = { + 422: { + "description": "Validation Error", + "content": { + "application/json": { + "schema": {"$ref": COMPONENT_REF_PREFIX + "HTTPValidationError"}, + }, + }, + }, + } + # Add the response to the OpenAPI operation if self.responses: + # Merge default responses with user responses + self.responses = {**default_responses, **self.responses} + for status_code in list(self.responses): response = self.responses[status_code] @@ -552,8 +567,7 @@ def _get_openapi_path( operation["responses"] = self.responses else: # Set the default 200 response - responses = operation.setdefault("responses", {}) - success_response = responses.setdefault(200, {}) + success_response = default_responses.setdefault(200, {}) success_response["description"] = self.response_description or _DEFAULT_OPENAPI_RESPONSE_DESCRIPTION success_response["content"] = {"application/json": {"schema": {}}} json_response = success_response["content"].setdefault("application/json", {}) @@ -567,24 +581,16 @@ def _get_openapi_path( ), ) - # Add validation failure response (422) - operation["responses"][422] = { - "description": "Validation Error", - "content": { - "application/json": { - "schema": {"$ref": COMPONENT_REF_PREFIX + "HTTPValidationError"}, - }, - }, - } + operation["responses"] = default_responses - # Add the validation error schema to the definitions, but only if it hasn't been added yet - if "ValidationError" not in definitions: - definitions.update( - { - "ValidationError": validation_error_definition, - "HTTPValidationError": validation_error_response_definition, - }, - ) + # Add the validation error schema to the definitions, but only if it hasn't been added yet + if "ValidationError" not in definitions: + definitions.update( + { + "ValidationError": validation_error_definition, + "HTTPValidationError": validation_error_response_definition, + }, + ) path[self.method.lower()] = operation diff --git a/aws_lambda_powertools/event_handler/openapi/types.py b/aws_lambda_powertools/event_handler/openapi/types.py index beafa0e566c..5a99ee76e98 100644 --- a/aws_lambda_powertools/event_handler/openapi/types.py +++ b/aws_lambda_powertools/event_handler/openapi/types.py @@ -28,7 +28,7 @@ "type": "array", "items": {"anyOf": [{"type": "string"}, {"type": "integer"}]}, }, - "msg": {"title": "Message", "type": "string"}, + # For security reasons, we hide **msg** details (don't leak Python, Pydantic or filenames) "type": {"title": "Error Type", "type": "string"}, }, "required": ["loc", "msg", "type"], diff --git a/tests/functional/event_handler/test_openapi_responses.py b/tests/functional/event_handler/test_openapi_responses.py index be5d9bca288..1162564c036 100644 --- a/tests/functional/event_handler/test_openapi_responses.py +++ b/tests/functional/event_handler/test_openapi_responses.py @@ -50,8 +50,8 @@ def handler(): assert 202 in responses.keys() assert responses[202].description == "Custom response" - assert 200 not in responses.keys() - assert 422 not in responses.keys() + assert 200 not in responses.keys() # 200 was not added due to custom responses + assert 422 in responses.keys() # 422 is always added due to potential data validation errors def test_openapi_200_custom_schema(): From b05a78bc7e03472e70c852b70799a31c7f3fb661 Mon Sep 17 00:00:00 2001 From: Ruben Fonseca Date: Thu, 21 Mar 2024 15:10:18 +0100 Subject: [PATCH 2/3] fix: refactor --- .../event_handler/api_gateway.py | 33 ++++++++----------- .../event_handler/test_openapi_params.py | 2 +- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index 3139995e456..29601247b48 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -504,7 +504,7 @@ def _get_openapi_path( operation["requestBody"] = request_body_oai # Validation failure response (422) will always be part of the schema - default_responses: Dict[int, OpenAPIResponse] = { + operation_responses: Dict[int, OpenAPIResponse] = { 422: { "description": "Validation Error", "content": { @@ -517,9 +517,6 @@ def _get_openapi_path( # Add the response to the OpenAPI operation if self.responses: - # Merge default responses with user responses - self.responses = {**default_responses, **self.responses} - for status_code in list(self.responses): response = self.responses[status_code] @@ -564,24 +561,24 @@ def _get_openapi_path( response["content"][content_type] = new_payload - operation["responses"] = self.responses + # Merge the user provided response with the default responses + operation_responses[status_code] = response else: # Set the default 200 response - success_response = default_responses.setdefault(200, {}) - success_response["description"] = self.response_description or _DEFAULT_OPENAPI_RESPONSE_DESCRIPTION - success_response["content"] = {"application/json": {"schema": {}}} - json_response = success_response["content"].setdefault("application/json", {}) + response_schema = self._openapi_operation_return( + param=dependant.return_param, + model_name_map=model_name_map, + field_mapping=field_mapping, + ) # Add the response schema to the OpenAPI 200 response - json_response.update( - self._openapi_operation_return( - param=dependant.return_param, - model_name_map=model_name_map, - field_mapping=field_mapping, - ), - ) + operation_responses[200] = { + "description": self.response_description or _DEFAULT_OPENAPI_RESPONSE_DESCRIPTION, + "content": {"application/json": response_schema}, + } - operation["responses"] = default_responses + operation["responses"] = operation_responses + path[self.method.lower()] = operation # Add the validation error schema to the definitions, but only if it hasn't been added yet if "ValidationError" not in definitions: @@ -592,8 +589,6 @@ def _get_openapi_path( }, ) - path[self.method.lower()] = operation - # Generate the response schema return path, definitions diff --git a/tests/functional/event_handler/test_openapi_params.py b/tests/functional/event_handler/test_openapi_params.py index 38b0cbed307..2ac9c036f3f 100644 --- a/tests/functional/event_handler/test_openapi_params.py +++ b/tests/functional/event_handler/test_openapi_params.py @@ -52,7 +52,7 @@ def handler(): assert JSON_CONTENT_TYPE in response.content json_response = response.content[JSON_CONTENT_TYPE] - assert json_response.schema_ == Schema() + assert json_response.schema_ is None assert not json_response.examples assert not json_response.encoding From d966fdf7efad93d03feba773d00dbc2161b489ae Mon Sep 17 00:00:00 2001 From: Ruben Fonseca Date: Thu, 21 Mar 2024 16:08:05 +0100 Subject: [PATCH 3/3] chore: add more tests --- .../event_handler/test_openapi_responses.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/functional/event_handler/test_openapi_responses.py b/tests/functional/event_handler/test_openapi_responses.py index 1162564c036..21a71d7dee3 100644 --- a/tests/functional/event_handler/test_openapi_responses.py +++ b/tests/functional/event_handler/test_openapi_responses.py @@ -54,6 +54,32 @@ def handler(): assert 422 in responses.keys() # 422 is always added due to potential data validation errors +def test_openapi_422_default_response(): + app = APIGatewayRestResolver(enable_validation=True) + + @app.get("/") + def handler(): + return {"message": "hello world"} + + schema = app.get_openapi_schema() + responses = schema.paths["/"].get.responses + assert 422 in responses.keys() + assert responses[422].description == "Validation Error" + + +def test_openapi_422_custom_response(): + app = APIGatewayRestResolver(enable_validation=True) + + @app.get("/", responses={422: {"description": "Custom validation response"}}) + def handler(): + return {"message": "hello world"} + + schema = app.get_openapi_schema() + responses = schema.paths["/"].get.responses + assert 422 in responses.keys() + assert responses[422].description == "Custom validation response" + + def test_openapi_200_custom_schema(): app = APIGatewayRestResolver(enable_validation=True)