Fixing Subschema Required Properties Validation

amcmanigal · amcmanigal · commit 56be4b10eb6d · 2018-04-04T10:15:45.000-04:00
Currently if valid swagger syntax is used for model composition an
error will be thrown due to the lack of a type property. This was
corrected by making object the default type.

schema_type = schema_deref.get('type', 'object')

I changed the swagger definition to test for this. Now PetCreate is a
composite of PetCreatePartOne and PetCreatePartTwo. However, this
caused `test_post_pets_empty_body` to fail, which turned out to be a
bug in the required properties.

In `_unmarshal_object` the `get_all_properties` method is called to get
all properties from the subschemas. However, this is not done for
required properties, meaning that only top level required properties
will be correctly validated. I have added a
`get_all_required_properties’ to fix this.

This caused `test_get_pets` to fail. In this case the bug allowed an
incorrect test case to be introduced. Pet requires `id`, but it also
requires name because it inherits from PetCreate. I have fixed this
test case by adding the missing required property.

After these changes `test_get_pet_not_found` failed due to a string
formatting error (double quotes vs single quotes). I fixed this by
switching to dictionary comparisons.
diff --git a/.gitignore b/.gitignore
@@ -99,3 +99,6 @@ ENV/
 
 # mypy
 .mypy_cache/
+
+# Jetbrains project files
+.idea/
diff --git a/openapi_core/schemas.py b/openapi_core/schemas.py
@@ -63,6 +63,15 @@ def get_all_properties(self):
 
         return properties
 
+    def get_all_required_properties(self):
+        required = self.required.copy()
+
+        for subschema in self.all_of:
+            subschema_req = subschema.get_all_required_properties()
+            required += subschema_req
+
+        return required
+
     def get_cast_mapping(self):
         mapping = DEFAULT_CAST_CALLABLE_GETTER.copy()
         mapping.update({
@@ -121,6 +130,7 @@ def _unmarshal_object(self, value):
             value = loads(value)
 
         all_properties = self.get_all_properties()
+        all_required_properties = self.get_all_required_properties()
         all_properties_keys = all_properties.keys()
         value_keys = value.keys()
 
@@ -135,7 +145,7 @@ def _unmarshal_object(self, value):
             try:
                 prop_value = value[prop_name]
             except KeyError:
-                if prop_name in self.required:
+                if prop_name in all_required_properties:
                     raise MissingProperty(
                         "Missing schema property {0}".format(prop_name))
                 if not prop.nullable and not prop.default:
@@ -167,7 +177,7 @@ def __init__(self, dereferencer):
     def create(self, schema_spec):
         schema_deref = self.dereferencer.dereference(schema_spec)
 
-        schema_type = schema_deref.get('type')
+        schema_type = schema_deref.get('type', 'object')
         schema_format = schema_deref.get('format')
         model = schema_deref.get('x-model', None)
         required = schema_deref.get('required', False)
diff --git a/tests/integration/data/v3.0/petstore.yaml b/tests/integration/data/v3.0/petstore.yaml
@@ -144,8 +144,13 @@ components:
           type: integer
           format: int64
     PetCreate:
-      type: object
       x-model: PetCreate
+      allOf:
+        - $ref: "#/components/schemas/PetCreatePartOne"
+        - $ref: "#/components/schemas/PetCreatePartTwo"
+    PetCreatePartOne:
+      type: object
+      x-model: PetCreatePartOne
       required:
         - name
       properties:
@@ -155,6 +160,10 @@ components:
           $ref: "#/components/schemas/Tag"
         address:
           $ref: "#/components/schemas/Address"
+    PetCreatePartTwo:
+      type: object
+      x-model: PetCreatePartTwo
+      properties:
         position:
           $ref: "#/components/schemas/Position"
         healthy:
diff --git a/tests/integration/test_petstore.py b/tests/integration/test_petstore.py
@@ -654,4 +654,4 @@ def test_get_pet_not_found(self, spec, response_validator):
         response_result = response_validator.validate(request, response)
 
         assert response_result.errors == []
-        assert response_result.data == data
+        assert response_result.data == data_json
diff --git a/tests/integration/test_validators.py b/tests/integration/test_validators.py
@@ -245,6 +245,7 @@ def test_get_pets(self, validator):
             'data': [
                 {
                     'id': 1,
+                    'name': 'Sparky'
                 },
             ],
         }

Original file line number	Diff line number	Diff line change
`@@ -245,6 +245,7 @@ def test_get_pets(self, validator):`
`245`	`245`	`'data': [`
`246`	`246`	`{`
`247`	`247`	`'id': 1,`
	`248`	`+ 'name': 'Sparky'`
`248`	`249`	`},`
`249`	`250`	`],`
`250`	`251`	`}`