From 61e631feab1a8f1dbef7f907191317e4a767fb3e Mon Sep 17 00:00:00 2001 From: Igor Davydenko Date: Mon, 27 Jan 2020 14:24:00 +0200 Subject: [PATCH] feature: Properly handle response data validation errors. Previously validation error on response resulted in 500 Server Error responses, this commit fixes it and retursn 422 Unprocessable Entity responses with data similar to Request Validation Error. Issue: #14 --- poetry.lock | 27 ++++++++---------- rororo/openapi/exceptions.py | 14 ++++++++++ rororo/openapi/validators.py | 4 ++- tests/openapi.json | 17 +++++++++++ tests/openapi.yaml | 11 ++++++++ tests/test_openapi.py | 48 ++++++++++++++++++++++++++++++++ tests/test_openapi_exceptions.py | 27 +++++++++--------- 7 files changed, 118 insertions(+), 30 deletions(-) diff --git a/poetry.lock b/poetry.lock index 0e7c7f4..c0e2641 100644 --- a/poetry.lock +++ b/poetry.lock @@ -210,7 +210,7 @@ python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" version = "1.4.3" [[package]] -category = "main" +category = "dev" description = "More routines for operating on iterables, beyond itertools" name = "more-itertools" optional = false @@ -263,7 +263,7 @@ description = "Core utilities for Python packages" name = "packaging" optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" -version = "20.0" +version = "20.1" [package.dependencies] pyparsing = ">=2.0.2" @@ -440,8 +440,8 @@ category = "dev" description = "HTTP library with thread-safe connection pooling, file post, and more." name = "urllib3" optional = false -python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, <4" -version = "1.25.7" +python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, <4" +version = "1.25.8" [package.extras] brotli = ["brotlipy (>=0.6.0)"] @@ -475,14 +475,11 @@ marker = "python_version < \"3.8\"" name = "zipp" optional = false python-versions = ">=3.6" -version = "2.0.0" - -[package.dependencies] -more-itertools = "*" +version = "2.1.0" [package.extras] docs = ["sphinx", "jaraco.packaging (>=3.2)", "rst.linker (>=1.9)"] -testing = ["pathlib2", "contextlib2", "unittest2"] +testing = ["jaraco.itertools"] [metadata] content-hash = "4233331dddbe4f846b342cc24c16f87b9d3cc66a0aa7db1be1daadfff38bcc36" @@ -647,8 +644,8 @@ openapi-spec-validator = [ {file = "openapi_spec_validator-0.2.8-py3-none-any.whl", hash = "sha256:0caacd9829e9e3051e830165367bf58d436d9487b29a09220fa7edb9f47ff81b"}, ] packaging = [ - {file = "packaging-20.0-py2.py3-none-any.whl", hash = "sha256:aec3fdbb8bc9e4bb65f0634b9f551ced63983a529d6a8931817d52fdd0816ddb"}, - {file = "packaging-20.0.tar.gz", hash = "sha256:fe1d8331dfa7cc0a883b49d75fc76380b2ab2734b220fbb87d774e4fd4b851f8"}, + {file = "packaging-20.1-py2.py3-none-any.whl", hash = "sha256:170748228214b70b672c581a3dd610ee51f733018650740e98c7df862a583f73"}, + {file = "packaging-20.1.tar.gz", hash = "sha256:e665345f9eef0c621aa0bf2f8d78cf6d21904eef16a93f020240b704a57f1334"}, ] pluggy = [ {file = "pluggy-0.13.1-py2.py3-none-any.whl", hash = "sha256:966c145cd83c96502c3c3868f50408687b38434af77734af1e9ca461a4081d2d"}, @@ -715,8 +712,8 @@ typing-extensions = [ {file = "typing_extensions-3.7.4.1.tar.gz", hash = "sha256:091ecc894d5e908ac75209f10d5b4f118fbdb2eb1ede6a63544054bb1edb41f2"}, ] urllib3 = [ - {file = "urllib3-1.25.7-py2.py3-none-any.whl", hash = "sha256:a8a318824cc77d1fd4b2bec2ded92646630d7fe8619497b142c84a9e6f5a7293"}, - {file = "urllib3-1.25.7.tar.gz", hash = "sha256:f3c5fd51747d450d4dcf6f923c81f78f811aab8205fda64b0aba34a4e48b0745"}, + {file = "urllib3-1.25.8-py2.py3-none-any.whl", hash = "sha256:2f3db8b19923a873b3e5256dc9c2dedfa883e33d87c690d9c7913e1f40673cdc"}, + {file = "urllib3-1.25.8.tar.gz", hash = "sha256:87716c2d2a7121198ebcb7ce7cccf6ce5e9ba539041cfbaeecfb641dc0bf6acc"}, ] wcwidth = [ {file = "wcwidth-0.1.8-py2.py3-none-any.whl", hash = "sha256:8fd29383f539be45b20bd4df0dc29c20ba48654a41e661925e612311e9f3c603"}, @@ -741,6 +738,6 @@ yarl = [ {file = "yarl-1.4.2.tar.gz", hash = "sha256:58cd9c469eced558cd81aa3f484b2924e8897049e06889e8ff2510435b7ef74b"}, ] zipp = [ - {file = "zipp-2.0.0-py3-none-any.whl", hash = "sha256:57147f6b0403b59f33fd357f169f860e031303415aeb7d04ede4839d23905ab8"}, - {file = "zipp-2.0.0.tar.gz", hash = "sha256:7ae5ccaca427bafa9760ac3cd8f8c244bfc259794b5b6bb9db4dda2241575d09"}, + {file = "zipp-2.1.0-py3-none-any.whl", hash = "sha256:ccc94ed0909b58ffe34430ea5451f07bc0c76467d7081619a454bf5c98b89e28"}, + {file = "zipp-2.1.0.tar.gz", hash = "sha256:feae2f18633c32fc71f2de629bfb3bd3c9325cd4419642b1f1da42ee488d9b98"}, ] diff --git a/rororo/openapi/exceptions.py b/rororo/openapi/exceptions.py index f134520..41ca25d 100644 --- a/rororo/openapi/exceptions.py +++ b/rororo/openapi/exceptions.py @@ -212,6 +212,20 @@ def from_request_errors( # type: ignore errors=parameters + body, ) + @classmethod + def from_response_errors( # type: ignore + cls, errors: List[OpenAPIMappingError] + ) -> "ValidationError": + result = [] + + for err in errors: + if isinstance(err, OpenAPIMediaTypeError): + details = get_media_type_error_details(["response"], err) + if details: + result.append(details) + + return cls(message="Response data validation error", errors=result) + def ensure_loc(loc: List[PathItem]) -> List[PathItem]: return [item for item in loc if item != ""] diff --git a/rororo/openapi/validators.py b/rororo/openapi/validators.py index f314784..1cac4c3 100644 --- a/rororo/openapi/validators.py +++ b/rororo/openapi/validators.py @@ -77,5 +77,7 @@ def validate_response_data( validator = ResponseValidator(spec, custom_formatters=CUSTOM_FORMATTERS) result = validator.validate(core_request, core_response) - result.raise_for_errors() + if result.errors: + raise ValidationError.from_response_errors(result.errors) + return result.data diff --git a/tests/openapi.json b/tests/openapi.json index efeab63..65f99d4 100644 --- a/tests/openapi.json +++ b/tests/openapi.json @@ -126,6 +126,23 @@ } } }, + "/invalid-response": { + "get": { + "operationId": "retrieve_invalid_response", + "responses": { + "200": { + "description": "Expected response.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NestedObject" + } + } + } + } + } + } + }, "/nested-object": { "post": { "operationId": "retrieve_nested_object_from_request_body", diff --git a/tests/openapi.yaml b/tests/openapi.yaml index 977be55..262a6ef 100644 --- a/tests/openapi.yaml +++ b/tests/openapi.yaml @@ -86,6 +86,17 @@ paths: "204": description: "Empty response." + "/invalid-response": + get: + operationId: "retrieve_invalid_response" + responses: + "200": + description: "Expected response." + content: + application/json: + schema: + $ref: "#/components/schemas/NestedObject" + "/nested-object": post: operationId: "retrieve_nested_object_from_request_body" diff --git a/tests/test_openapi.py b/tests/test_openapi.py index 1010916..45b0bb0 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -87,6 +87,11 @@ async def retrieve_empty(request: web.Request) -> web.Response: return web.Response(status=204) +@operations.register +async def retrieve_invalid_response(request: web.Request) -> web.Response: + return web.json_response({}) + + @operations.register async def retrieve_nested_object_from_request_body( request: web.Request, @@ -477,3 +482,46 @@ async def test_validate_empty_response(aiohttp_client, schema_path): client = await aiohttp_client(app) response = await client.get("/api/empty") assert response.status == 204 + + +@pytest.mark.parametrize( + "schema_path, is_validate_response, expected_status", + ( + (OPENAPI_JSON_PATH, False, 200), + (OPENAPI_JSON_PATH, True, 422), + (OPENAPI_YAML_PATH, False, 200), + (OPENAPI_JSON_PATH, True, 422), + ), +) +async def test_validate_response( + aiohttp_client, schema_path, is_validate_response, expected_status +): + app = setup_openapi( + web.Application(), + schema_path, + operations, + server_url="/api", + is_validate_response=is_validate_response, + ) + + client = await aiohttp_client(app) + response = await client.get("/api/invalid-response") + assert response.status == expected_status + + +@pytest.mark.parametrize("schema_path", (OPENAPI_JSON_PATH, OPENAPI_YAML_PATH)) +async def test_validate_response_error(aiohttp_client, schema_path): + app = setup_openapi( + web.Application(), + schema_path, + operations, + server_url="/api", + is_validate_response=True, + ) + + client = await aiohttp_client(app) + response = await client.get("/api/invalid-response") + assert response.status == 422 + assert await response.json() == { + "detail": [{"loc": ["response", "uid"], "message": "Field required"}] + } diff --git a/tests/test_openapi_exceptions.py b/tests/test_openapi_exceptions.py index a5977c5..2b4a0ae 100644 --- a/tests/test_openapi_exceptions.py +++ b/tests/test_openapi_exceptions.py @@ -92,19 +92,18 @@ def test_validation_error_from_dict_value_error(): ) -def test_validation_error_from_dummy_mapping_error(): - err = ValidationError.from_request_errors([OpenAPIMappingError()]) - assert err.errors == [] - assert err.data is None - - -def test_validation_error_from_dummy_media_type_error(): - err = ValidationError.from_request_errors([OpenAPIMediaTypeError()]) - assert err.errors == [] - assert err.data is None - - -def test_validation_error_from_dummy_operation_error(): - err = ValidationError.from_request_errors([OpenAPIParameterError()]) +@pytest.mark.parametrize( + "method, error", + ( + (ValidationError.from_request_errors, OpenAPIMappingError()), + (ValidationError.from_response_errors, OpenAPIMappingError()), + (ValidationError.from_request_errors, OpenAPIMediaTypeError()), + (ValidationError.from_response_errors, OpenAPIMediaTypeError()), + (ValidationError.from_request_errors, OpenAPIParameterError()), + (ValidationError.from_response_errors, OpenAPIParameterError()), + ), +) +def test_validation_error_from_dummy_error(method, error): + err = method([error]) assert err.errors == [] assert err.data is None