Merge pull request #358 from nickgaya/collapse-required-properties

Collapse required properties from inherited models

bravado_core/marshal.py

@@ -11,6 +11,7 @@
 from bravado_core.model import Model
 from bravado_core.model import MODEL_MARKER
 from bravado_core.schema import collapsed_properties
+from bravado_core.schema import collapsed_required
 from bravado_core.schema import get_type_from_schema
 from bravado_core.schema import is_dict_like
 from bravado_core.schema import is_list_like
@@ -335,7 +336,7 @@ def _marshaling_method_object(swagger_spec, object_schema):
             )
 
     properties = collapsed_properties(object_schema, swagger_spec)
-    required_properties = set(object_schema.get('required', []))
+    required_properties = collapsed_required(object_schema, swagger_spec)
     properties_to_marshaling_function = {
         prop_name: _get_marshaling_method(
             swagger_spec=swagger_spec,

bravado_core/schema.py

@@ -188,6 +188,29 @@ def collapsed_properties(model_spec, swagger_spec):
     return properties
 
 
+def collapsed_required(model_spec, swagger_spec):
+    """
+    Processes model spec and outputs a set of required properties for the model.
+
+    This handles traversing any polymorphic models and the hierarchy
+    of properties properly.
+
+    :param model_spec: model specification (must be dereferenced already)
+    :type model_spec: dict
+    :param swagger_spec: :class:`bravado_core.spec.Spec`
+    :returns: Set of required properties.
+    """
+    required = set(model_spec.get('required', []))
+
+    if 'allOf' in model_spec:
+        deref = swagger_spec.deref
+        for item_spec in model_spec['allOf']:
+            item_spec = deref(item_spec)
+            required.update(collapsed_required(item_spec, swagger_spec))
+
+    return required
+
+
 def get_type_from_schema(swagger_spec, schema_object_spec):
     try:
         return schema_object_spec['type']

bravado_core/unmarshal.py

@@ -16,6 +16,7 @@
 from bravado_core.exception import SwaggerMappingError
 from bravado_core.model import MODEL_MARKER
 from bravado_core.schema import collapsed_properties
+from bravado_core.schema import collapsed_required
 from bravado_core.schema import get_type_from_schema
 from bravado_core.schema import is_dict_like
 from bravado_core.schema import is_list_like
@@ -337,7 +338,7 @@ def _unmarshaling_method_object(swagger_spec, object_schema, use_models=True):
             model_type = None
 
     properties = collapsed_properties(object_schema, swagger_spec)
-    required_properties = object_schema.get('required', [])
+    required_properties = collapsed_required(object_schema, swagger_spec)
     properties_to_unmarshaling_function = {
         prop_name: _get_unmarshaling_method(
             swagger_spec=swagger_spec,

tests/schema/collapsed_properties_test.py

@@ -1,79 +1,5 @@
 # -*- coding: utf-8 -*-
-import pytest
-
 from bravado_core.schema import collapsed_properties
-from bravado_core.spec import Spec
-
-
-@pytest.fixture
-def users_spec():
-    return {
-        "User": {
-            "properties": {
-                "id": {
-                    "type": "integer",
-                    "format": "int64",
-                },
-                "username": {
-                    "type": "string",
-                },
-                "email": {
-                    "type": "string",
-                },
-                "password": {
-                    "type": "string",
-                },
-            },
-        },
-        "VIP": {
-            "allOf": [
-                {
-                    "$ref": "#/definitions/User",
-                },
-                {
-                    "properties": {
-                        "vip_pass_no": {
-                            "type": "string",
-                        },
-                    },
-                },
-            ],
-        },
-        "Admin": {
-            "allOf": [
-                {
-                    "$ref": "#/definitions/User",
-                },
-                {
-                    "type": "object",
-                    "properties": {
-                        "permissions": {
-                            "type": "array",
-                            "items": {
-                                "type": "string",
-                            },
-                        },
-                    },
-                },
-            ],
-        },
-        "SuperUser": {
-            "allOf": [
-                {
-                    "$ref": "#/definitions/Admin",
-                },
-                {
-                    "$ref": "#/definitions/VIP",
-                },
-            ],
-        },
-    }
-
-
-@pytest.fixture
-def users_swagger_spec(minimal_swagger_dict, users_spec):
-    minimal_swagger_dict['definitions'] = users_spec
-    return Spec.from_dict(minimal_swagger_dict)
 
 
 def test_allOf(users_spec, users_swagger_spec):

tests/schema/collapsed_required_test.py

@@ -0,0 +1,14 @@
+# -*- coding: utf-8 -*-
+from bravado_core.schema import collapsed_required
+
+
+def test_allOf(users_spec, users_swagger_spec):
+    """Test allOf functionality, including:
+     - multiple levels of allOf
+     - multiple references within one allOf
+     - referencing the same model multiple times across the
+       allOf-hierarchy
+    """
+    superuser_spec = users_spec['SuperUser']
+    required = collapsed_required(superuser_spec, users_swagger_spec)
+    assert required == {'id', 'username', 'password', 'permissions'}

tests/schema/conftest.py

@@ -0,0 +1,77 @@
+# -*- coding: utf-8 -*-
+import pytest
+
+from bravado_core.spec import Spec
+
+
+@pytest.fixture
+def users_spec():
+    return {
+        "User": {
+            "properties": {
+                "id": {
+                    "type": "integer",
+                    "format": "int64",
+                },
+                "username": {
+                    "type": "string",
+                },
+                "email": {
+                    "type": "string",
+                },
+                "password": {
+                    "type": "string",
+                },
+            },
+            "required": ["id", "username", "password"],
+        },
+        "VIP": {
+            "allOf": [
+                {
+                    "$ref": "#/definitions/User",
+                },
+                {
+                    "properties": {
+                        "vip_pass_no": {
+                            "type": "string",
+                        },
+                    },
+                },
+            ],
+        },
+        "Admin": {
+            "allOf": [
+                {
+                    "$ref": "#/definitions/User",
+                },
+                {
+                    "type": "object",
+                    "properties": {
+                        "permissions": {
+                            "type": "array",
+                            "items": {
+                                "type": "string",
+                            },
+                        },
+                    },
+                    "required": ["permissions"],
+                },
+            ],
+        },
+        "SuperUser": {
+            "allOf": [
+                {
+                    "$ref": "#/definitions/Admin",
+                },
+                {
+                    "$ref": "#/definitions/VIP",
+                },
+            ],
+        },
+    }
+
+
+@pytest.fixture
+def users_swagger_spec(minimal_swagger_dict, users_spec):
+    minimal_swagger_dict['definitions'] = users_spec
+    return Spec.from_dict(minimal_swagger_dict)