Merge pull request #246 from idesoto-rover/fix-additional-properties-validation

#245 fix validation for additionalProperties

Author: sondrelg

CONTRIBUTING.md

@@ -3,9 +3,10 @@
 This package is open to contributions. To contribute, please follow these steps:
 
 1. Fork the upstream drf-openapi-tester repository into a personal account.
-2. Install [poetry](https://python-poetry.org/), and install dev dependencies using ``poetry install``
-3. Install [pre-commit](https://pre-commit.com/) (for project linting) by running ``pre-commit install``
+2. Install [poetry](https://python-poetry.org/), and install dev dependencies using `poetry install`
+3. Install [pre-commit](https://pre-commit.com/) (for project linting) by running `pre-commit install`
 4. Create a new branch for your changes, and make sure to add tests!
-5. Push the topic branch to your personal fork
-6. Run "pre-commit run --all-files" locally to ensure proper linting
-6. Create a pull request to the drf-openapi-tester repository with an explanation of your changes
+5. Run `poetry run pytest` to ensure all tests are passing
+6. Run `pre-commit run --all-files` locally to ensure proper linting
+7. Push the topic branch to your personal fork
+8. Create a pull request to the drf-openapi-tester repository with an explanation of your changes

openapi_tester/schema_tester.py

@@ -6,6 +6,7 @@
 from django.core.exceptions import ImproperlyConfigured
 from rest_framework.response import Response
 
+from openapi_tester import OpenAPISchemaError
 from openapi_tester import type_declarations as td
 from openapi_tester.constants import (
     INIT_ERROR,
@@ -295,8 +296,9 @@ def test_openapi_object(
         required_keys = [key for key in schema_section.get("required", []) if key not in write_only_properties]
         response_keys = data.keys()
         additional_properties: Optional[Union[bool, dict]] = schema_section.get("additionalProperties")
-        if not properties and isinstance(additional_properties, dict):
-            properties = additional_properties
+        additional_properties_allowed = additional_properties is not None
+        if additional_properties_allowed and not isinstance(additional_properties, (bool, dict)):
+            raise OpenAPISchemaError("Invalid additionalProperties type")
         for key in properties.keys():
             self.test_key_casing(key, case_tester, ignore_case)
             if key in required_keys and key not in response_keys:
@@ -307,9 +309,7 @@ def test_openapi_object(
                 )
         for key in response_keys:
             self.test_key_casing(key, case_tester, ignore_case)
-            key_in_additional_properties = isinstance(additional_properties, dict) and key in additional_properties
-            additional_properties_allowed = additional_properties is True
-            if key not in properties and not key_in_additional_properties and not additional_properties_allowed:
+            if key not in properties and not additional_properties_allowed:
                 raise DocumentationError(
                     f"{VALIDATE_EXCESS_RESPONSE_KEY_ERROR.format(excess_key=key)}\n\nReference: {reference}.object:key:"
                     f"{key}\n\nHint: Remove the key from your API response, or include it in your OpenAPI docs"
@@ -321,16 +321,22 @@ def test_openapi_object(
                     f'"WriteOnly" restriction'
                 )
         for key, value in data.items():
-            if key not in properties and additional_properties_allowed:
-                # Avoid KeyError below
-                continue
-            self.test_schema_section(
-                schema_section=properties[key],
-                data=value,
-                reference=f"{reference}.object:key:{key}",
-                case_tester=case_tester,
-                ignore_case=ignore_case,
-            )
+            if key in properties:
+                self.test_schema_section(
+                    schema_section=properties[key],
+                    data=value,
+                    reference=f"{reference}.object:key:{key}",
+                    case_tester=case_tester,
+                    ignore_case=ignore_case,
+                )
+            elif isinstance(additional_properties, dict):
+                self.test_schema_section(
+                    schema_section=additional_properties,
+                    data=value,
+                    reference=f"{reference}.object:key:{key}",
+                    case_tester=case_tester,
+                    ignore_case=ignore_case,
+                )
 
     def test_openapi_array(self, schema_section: dict, data: dict, reference: str, **kwargs: Any) -> None:
         for datum in data:

tests/test_validators.py

@@ -16,6 +16,7 @@
     VALIDATE_MINIMUM_ERROR,
     VALIDATE_MINIMUM_NUMBER_OF_PROPERTIES_ERROR,
     VALIDATE_MULTIPLE_OF_ERROR,
+    VALIDATE_TYPE_ERROR,
 )
 from openapi_tester.validators import VALIDATOR_MAP
 from tests import (
@@ -141,12 +142,68 @@ def test_additional_properties_allowed():
     tester.test_schema_section(schema, {"oneKey": "test", "twoKey": "test2"})
 
 
+def test_additional_properties_specified_as_empty_object_allowed():
+    schema = {"type": "object", "additionalProperties": {}, "properties": {"oneKey": {"type": "string"}}}
+    tester.test_schema_section(schema, {"oneKey": "test", "twoKey": "test2"})
+
+
 def test_additional_properties_not_allowed_by_default():
     schema = {"type": "object", "properties": {"oneKey": {"type": "string"}}}
     with pytest.raises(DocumentationError, match=VALIDATE_EXCESS_RESPONSE_KEY_ERROR[:90]):
         tester.test_schema_section(schema, {"oneKey": "test", "twoKey": "test2"})
 
 
+def test_string_dictionary_specified_as_additional_properties_allowed():
+    schema = {"type": "object", "additionalProperties": {"type": "string"}, "properties": {"key_1": {"type": "string"}}}
+    tester.test_schema_section(schema, {"key_1": "value_1", "key_2": "value_2", "key_3": "value_3"})
+
+
+def test_string_dictionary_with_non_string_value_fails_validation():
+    schema = {"type": "object", "additionalProperties": {"type": "string"}, "properties": {"key_1": {"type": "string"}}}
+    expected_error_message = VALIDATE_TYPE_ERROR.format(article="a", type="string", received=123)
+    with pytest.raises(DocumentationError, match=expected_error_message):
+        tester.test_schema_section(schema, {"key_1": "value_1", "key_2": 123, "key_3": "value_3"})
+
+
+def test_object_dictionary_specified_as_additional_properties_allowed():
+    schema = {
+        "type": "object",
+        "properties": {"key_1": {"type": "string"}},
+        "additionalProperties": {
+            "type": "object",
+            "properties": {"key_2": {"type": "string"}, "key_3": {"type": "number"}},
+        },
+    }
+    tester.test_schema_section(
+        schema,
+        {
+            "key_1": "value_1",
+            "some_extra_key": {"key_2": "value_2", "key_3": 123},
+            "another_extra_key": {"key_2": "value_4", "key_3": 246},
+        },
+    )
+
+
+def test_additional_properties_schema_not_validated_in_main_properties():
+    schema = {
+        "type": "object",
+        "properties": {"key_1": {"type": "string"}},
+        "additionalProperties": {
+            "type": "object",
+            "properties": {"key_2": {"type": "string"}, "key_3": {"type": "number"}},
+        },
+    }
+    expected_error_message = VALIDATE_TYPE_ERROR.format(article="an", type="object", received='"value_2"')
+    with pytest.raises(DocumentationError, match=expected_error_message):
+        tester.test_schema_section(schema, {"key_1": "value_1", "key_2": "value_2", "key_3": 123})
+
+
+def test_invalid_additional_properties_raises_schema_error():
+    schema = {"type": "object", "properties": {"key_1": {"type": "string"}}, "additionalProperties": 123}
+    with pytest.raises(OpenAPISchemaError, match="Invalid additionalProperties type"):
+        tester.test_schema_section(schema, {"key_1": "value_1", "key_2": "value_2"})
+
+
 def test_pattern_validation():
     """The a regex pattern can be passed to describe how a string should look"""
     schema = {"type": "string", "pattern": r"^\d{3}-\d{2}-\d{4}$"}