From 21f825b6129dcde91761462e26fd9b7e73b6c036 Mon Sep 17 00:00:00 2001 From: Matias Cardenas Date: Thu, 17 Oct 2024 16:41:12 +0200 Subject: [PATCH] feat: adding validation for readOnly properties in requests --- openapi_tester/constants.py | 1 + openapi_tester/schema_tester.py | 23 +++++- openapi_tester/utils.py | 21 +++++ .../schemas/openapi_v3_reference_schema.yaml | 1 + tests/test_clients.py | 14 ++++ tests/test_openapi_object.py | 39 +++++++++- tests/test_utils.py | 78 ++++++++++++++++++- 7 files changed, 168 insertions(+), 9 deletions(-) diff --git a/openapi_tester/constants.py b/openapi_tester/constants.py index 83fcf71b..7a567f78 100644 --- a/openapi_tester/constants.py +++ b/openapi_tester/constants.py @@ -50,6 +50,7 @@ 'but is missing from the schema definition: "{excess_key}"' ) VALIDATE_WRITE_ONLY_RESPONSE_KEY_ERROR = 'The following property was found in the response, but is documented as being "writeOnly": "{write_only_key}"' +VALIDATE_READ_ONLY_RESPONSE_KEY_ERROR = 'The following property was found in the request, but is documented as being "readOnly": "{read_only_key}"' VALIDATE_ONE_OF_ERROR = "Expected data to match one and only one of the oneOf schema types; found {matches} matches" VALIDATE_ANY_OF_ERROR = "Expected data to match one or more of the documented anyOf schema types, but found no matches" UNDOCUMENTED_SCHEMA_SECTION_ERROR = ( diff --git a/openapi_tester/schema_tester.py b/openapi_tester/schema_tester.py index 886dc60b..ed7ee968 100644 --- a/openapi_tester/schema_tester.py +++ b/openapi_tester/schema_tester.py @@ -21,6 +21,7 @@ VALIDATE_MISSING_KEY_ERROR, VALIDATE_NONE_ERROR, VALIDATE_ONE_OF_ERROR, + VALIDATE_READ_ONLY_RESPONSE_KEY_ERROR, VALIDATE_WRITE_ONLY_RESPONSE_KEY_ERROR, ) from openapi_tester.exceptions import ( @@ -35,6 +36,7 @@ UrlStaticSchemaLoader, ) from openapi_tester.utils import ( + get_required_keys, lazy_combinations, normalize_schema_section, serialize_schema_section_data, @@ -537,11 +539,15 @@ def test_openapi_object( write_only_properties = [ key for key in properties.keys() if properties[key].get("writeOnly") ] - required_keys = [ - key - for key in schema_section.get("required", []) - if key not in write_only_properties + read_only_properties = [ + key for key in properties.keys() if properties[key].get("readOnly") ] + required_keys = get_required_keys( + schema_section=schema_section, + http_message=test_config.http_message, + write_only_props=write_only_properties, + read_only_props=read_only_properties, + ) request_response_keys = data.keys() additional_properties: bool | dict | None = schema_section.get( "additionalProperties" @@ -584,6 +590,15 @@ def test_openapi_object( f"\n\nHint: Remove the key from your API {test_config.http_message}, or" ' remove the "WriteOnly" restriction' ) + if key in read_only_properties and test_config.http_message == "request": + raise DocumentationError( + f"{VALIDATE_READ_ONLY_RESPONSE_KEY_ERROR.format(read_only_key=key)}\n\nReference:" + f"\n\n{test_config.reference} > {key}" + f"\n\n{test_config.http_message.capitalize()} body:\n {serialize_schema_section_data(data=data)}" + f"\nSchema section:\n {serialize_schema_section_data(data=properties)}" + f"\n\nHint: Remove the key from your API {test_config.http_message}, or" + ' remove the "ReadOnly" restriction' + ) for key, value in data.items(): if key in properties: drill_down_test_config = copy(test_config) diff --git a/openapi_tester/utils.py b/openapi_tester/utils.py index 34f2fc4e..2b3c0209 100644 --- a/openapi_tester/utils.py +++ b/openapi_tester/utils.py @@ -81,3 +81,24 @@ def wrapper(*args, content_type="application/json", **kwargs): return func(*args, **kwargs) return wrapper + + +def get_required_keys( + schema_section: dict, + http_message: str, + write_only_props: list[str], + read_only_props: list[str], +) -> list[str]: + if http_message == "request": + return [ + key + for key in schema_section.get("required", []) + if key not in read_only_props + ] + if http_message == "response": + return [ + key + for key in schema_section.get("required", []) + if key not in write_only_props + ] + return [] diff --git a/tests/schemas/openapi_v3_reference_schema.yaml b/tests/schemas/openapi_v3_reference_schema.yaml index 1bdad3d6..06477929 100644 --- a/tests/schemas/openapi_v3_reference_schema.yaml +++ b/tests/schemas/openapi_v3_reference_schema.yaml @@ -146,6 +146,7 @@ components: id: type: integer format: int64 + readOnly: true NewPet: type: object diff --git a/tests/test_clients.py b/tests/test_clients.py index f0a07db8..28576dab 100644 --- a/tests/test_clients.py +++ b/tests/test_clients.py @@ -108,6 +108,20 @@ def test_request_with_write_only_field(pets_api_schema: "Path"): ) +def test_request_with_read_only_field(pets_api_schema: "Path"): + """Ensure validation doesn't raise exception when request a write-only field is + included in the request, and not in the response.""" + schema_tester = SchemaTester(schema_file_path=str(pets_api_schema)) + openapi_client = OpenAPIClient(schema_tester=schema_tester) + + with pytest.raises(DocumentationError): + openapi_client.post( + path="/api/pets", + data={"id": 1, "name": "doggie", "tag": "Bulldog"}, + content_type="application/json", + ) + + def test_response_with_write_only_field(pets_api_schema: "Path"): """Ensure validation raises exception when response includes a write-only field.""" schema_tester = SchemaTester(schema_file_path=str(pets_api_schema)) diff --git a/tests/test_openapi_object.py b/tests/test_openapi_object.py index 4db2269c..1ca95113 100644 --- a/tests/test_openapi_object.py +++ b/tests/test_openapi_object.py @@ -28,13 +28,13 @@ def test_missing_schema_key_error(): "\n\nReference:" "\n\nPOST /endpoint > response > two" '\n\nResponse body:\n {\n "one": 1,\n "two": 2\n}' - '\n\nSchema section:\n {\n "one": {\n "type": "int"\n }\n}' + '\n\nSchema section:\n {\n "one": {\n "type": "integer"\n }\n}' "\n\nHint: Remove the key from your API response, or include it in your OpenAPI docs" ) tester = SchemaTester() with pytest.raises(DocumentationError, match=expected_error_message): tester.test_openapi_object( - {"required": ["one"], "properties": {"one": {"type": "int"}}}, + {"required": ["one"], "properties": {"one": {"type": "integer"}}}, {"one": 1, "two": 2}, OpenAPITestConfig(reference="POST /endpoint > response"), ) @@ -46,18 +46,49 @@ def test_key_in_write_only_properties_error(): "\n\nReference:" "\n\nPOST /endpoint > response > one" '\n\nResponse body:\n {\n "one": 1\n}' - '\nSchema section:\n {\n "one": {\n "type": "int",\n "writeOnly": true\n }\n}' + '\nSchema section:\n {\n "one": {\n "type": "integer",\n "writeOnly": true\n }\n}' '\n\nHint: Remove the key from your API response, or remove the "WriteOnly" restriction' ) tester = SchemaTester() with pytest.raises(DocumentationError, match=expected_error_message): tester.test_openapi_object( - {"properties": {"one": {"type": "int", "writeOnly": True}}}, + {"properties": {"one": {"type": "integer", "writeOnly": True}}}, {"one": 1}, OpenAPITestConfig(reference="POST /endpoint > response"), ) +def test_key_in_read_only_properties_error(): + expected_error_message = ( + 'The following property was found in the request, but is documented as being "readOnly": "one"' + "\n\nReference:" + "\n\nPOST /endpoint > request > one" + '\n\nRequest body:\n {\n "one": 1\n}' + '\nSchema section:\n {\n "one": {\n "type": "integer",\n "readOnly": true\n }\n}' + '\n\nHint: Remove the key from your API request, or remove the "ReadOnly" restriction' + ) + tester = SchemaTester() + with pytest.raises(DocumentationError, match=expected_error_message): + tester.test_openapi_object( + {"properties": {"one": {"type": "integer", "readOnly": True}}}, + {"one": 1}, + OpenAPITestConfig( + reference="POST /endpoint > request", http_message="request" + ), + ) + + +def test_key_in_read_only_properties_response_does_not_raise_error(): + tester = SchemaTester() + tester.test_openapi_object( + {"properties": {"one": {"type": "integer", "readOnly": True}}}, + {"one": 1}, + OpenAPITestConfig( + reference="POST /endpoint > response", http_message="response" + ), + ) + + def test_date_serialization(): tester = SchemaTester() tester.test_openapi_object( diff --git a/tests/test_utils.py b/tests/test_utils.py index 602bc7c4..c6d11595 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,4 +1,8 @@ -from openapi_tester.utils import merge_objects, serialize_schema_section_data +from openapi_tester.utils import ( + get_required_keys, + merge_objects, + serialize_schema_section_data, +) from tests.utils import sort_object object_1 = { @@ -61,3 +65,75 @@ def test_serialize_schema_section_data(): " }" "\n}" ) + + +def test_get_required_keys(): + # given + schema_section = { + "type": "object", + "required": ["key1", "key2"], + "properties": {"key1": {"type": "string"}, "key2": {"type": "string"}}, + } + read_only_props = [] + write_only_props = [] + http_message = "response" + + # when + required_keys = get_required_keys( + schema_section=schema_section, + http_message=http_message, + read_only_props=read_only_props, + write_only_props=write_only_props, + ) + + # then + assert required_keys == ["key1", "key2"] + + +def test_get_required_keys_request_with_read_only_field(): + # given + schema_section = { + "type": "object", + "required": ["key1", "key2"], + "properties": {"key1": {"type": "string"}, "key2": {"type": "string"}}, + "readOnly": ["key2"], + } + read_only_props = ["key2"] + write_only_props = [] + + http_message = "request" + + # when + required_keys = get_required_keys( + schema_section=schema_section, + http_message=http_message, + read_only_props=read_only_props, + write_only_props=write_only_props, + ) + + # then + assert required_keys == ["key1"] + + +def test_get_required_keys_response_with_write_only_field(): + # given + schema_section = { + "type": "object", + "required": ["key1", "key2"], + "properties": {"key1": {"type": "string"}, "key2": {"type": "string"}}, + "writeOnly": ["key2"], + } + write_only_props = ["key2"] + read_only_props = [] + http_message = "response" + + # when + required_keys = get_required_keys( + schema_section=schema_section, + http_message=http_message, + write_only_props=write_only_props, + read_only_props=read_only_props, + ) + + # then + assert required_keys == ["key1"]