Enhance flattening

bravado_core/model.py

@@ -584,7 +584,7 @@ def create_model_type(swagger_spec, model_name, model_spec, bases=(Model,), json
     inherits_from = []
     if 'allOf' in model_spec:
         for schema in model_spec['allOf']:
-            inherited_name = swagger_spec.deref(schema).get(MODEL_MARKER, None)
+            inherited_name = _get_model_name(swagger_spec.deref(schema))
             if inherited_name:
                 inherits_from.append(inherited_name)
 
@@ -808,7 +808,7 @@ def _call_post_process_spec(spec_dict):
         if uri == spec.origin_url or re.match(r'http://json-schema.org/draft-\d+/schema', uri)
     }
     additional_uri = _get_unprocessed_uri(spec, processed_uris)
-    while additional_uri:
+    while additional_uri is not None:
         # Post process each referenced specs to identify models in definitions of linked files
         with spec.resolver.in_scope(additional_uri):
             _call_post_process_spec(

bravado_core/spec_flattening.py

@@ -10,6 +10,7 @@
 from six import iterkeys
 from six import itervalues
 from six.moves.urllib.parse import ParseResult
+from six.moves.urllib.parse import urldefrag
 from six.moves.urllib.parse import urlparse
 from six.moves.urllib.parse import urlunparse
 from swagger_spec_validator.ref_validators import in_scope
@@ -316,20 +317,59 @@ def model_discovery(self):
             },
         ))
 
-    def add_original_models_into_known_mappings(self):
+    def include_discriminated_models(self):
+        """
+        This function ensures that discriminated models, present on the original Spec object
+        but not directly referenced by the flattened schema (because there is no direct $ref
+        attribute) are included in the final flattened specs
+
+        NOTE: The method re-run model_discovery in case additional models have been added to
+              the flattened models
+        """
         flatten_models = {
             # schema objects might not have a "type" set so they won't be tagged as models
             definition.get(MODEL_MARKER)
             for definition in itervalues(self.known_mappings['definitions'])
         }
 
-        for model_name, model_type in iteritems(self.swagger_spec.definitions):
-            if model_name in flatten_models:
-                continue
-            model_url = urlparse(model_type._json_reference)
-            self.known_mappings['definitions'][model_url] = self.descend(
-                value=model_type._model_spec,
-            )
+        def unflattened_models():
+            for m_name, m_type in iteritems(self.swagger_spec.definitions):
+                if m_name not in flatten_models:
+                    yield m_name, m_type
+
+        def register_unflattened_models():
+            """
+            :return: True if new models have been added
+            """
+            initial_number_of_models = len(self.known_mappings['definitions'])
+
+            modified = True
+            while modified:
+                modified = False
+                for model_name, model_type in unflattened_models():
+                    if any(
+                        parent in flatten_models
+                        for parent in model_type._inherits_from
+                    ):
+                        model_url = urlparse(model_type._json_reference)
+                        flatten_models.add(model_name)
+                        self.known_mappings['definitions'][model_url] = self.descend(
+                            value=model_type._model_spec,
+                        )
+                        modified = True
+
+            return len(self.known_mappings['definitions']) != initial_number_of_models
+
+        while register_unflattened_models():
+            self.model_discovery()
+
+    def include_root_definition(self):
+        self.known_mappings['definitions'].update({
+            urlparse(v._json_reference): self.descend(value=v._model_spec)
+            for v in itervalues(self.swagger_spec.definitions)
+            # urldefrag(url)[0] returns the url without the fragment, it is guaranteed to be present
+            if urldefrag(v._json_reference)[0] == self.swagger_spec.origin_url
+        })
 
     @cached_property
     def resolved_specs(self):
@@ -339,10 +379,14 @@ def resolved_specs(self):
         # Perform model discovery of the newly identified definitions
         self.model_discovery()
 
+        # Ensure that all the root definitions, even if not referenced, are not lost due to flattening.
+        self.include_root_definition()
+
         # Add the identified models that are not available on the know_mappings definitions
-        # This could happen in case of models that have been discovered because on
-        # '#/definitions' of a referenced file, but not directly referenced by the specs
-        self.add_original_models_into_known_mappings()
+        # but that are related, via polymorphism (discriminator), to flattened models
+        # This could happen in case discriminated models are not directly referenced by the specs
+        # but is fair to assume that they should be on the final artifact due to polymorphism
+        self.include_discriminated_models()
 
         for mapping_key, mappings in iteritems(self.known_mappings):
             self.warn_if_uri_clash_on_same_marshaled_representation(mappings)

test-data/2.0/multi-file-multi-directory-spec/flattened-multi-file-multi-directory-spec.json

@@ -59,28 +59,21 @@
           "$ref": "#/definitions/ErrorResponse"
         },
         {
-          "$ref": "#/definitions/endpoint_v1_HTTP_FORBIDDEN_action_part"
+          "properties": {
+            "action": {
+              "description": "name of the forbidden action",
+              "type": "string"
+            }
+          },
+          "required": [
+            "action"
+          ],
+          "type": "object",
+          "x-model": "endpoint_v1_HTTP_FORBIDDEN_action_part"
         }
       ],
       "type": "object",
       "x-model": "endpoint_v1_HTTP_FORBIDDEN"
-    },
-    "endpoint_v1_HTTP_FORBIDDEN_action_part": {
-      "properties": {
-        "action": {
-          "description": "name of the forbidden action",
-          "type": "string"
-        }
-      },
-      "required": [
-        "action"
-      ],
-      "type": "object",
-      "x-model": "endpoint_v1_HTTP_FORBIDDEN_action_part"
-    },
-    "endpoint_v1_HTTP_OK": {
-      "type": "object",
-      "x-model": "endpoint_v1_HTTP_OK"
     }
   },
   "info": {
@@ -126,7 +119,8 @@
     "lfile:endpoint..v1..responses.yaml|..200": {
       "description": "HTTP/200",
       "schema": {
-        "$ref": "#/definitions/endpoint_v1_HTTP_OK"
+        "type": "object",
+        "x-model": "endpoint_v1_HTTP_OK"
       }
     },
     "lfile:endpoint..v1..responses.yaml|..403": {

test-data/2.0/multi-file-recursive/aux.json

@@ -1,17 +1,50 @@
 {
     "definitions": {
+        "mobile_ping": {
+            "allOf": [
+                {"$ref":  "#/definitions/ping"},
+                {
+                    "properties": {
+                        "text": {
+                            "type": "string"
+                        }
+                    }
+                }
+            ],
+            "type": "object",
+            "x-model": "mobile_ping"
+        },
+        "phone_ping": {
+            "allOf": [
+                {"$ref":  "#/definitions/ping"},
+                {
+                    "properties": {
+                        "duration": {
+                            "type": "number"
+                        }
+                    }
+                }
+            ],
+            "type": "object",
+            "x-model": "phone_ping"
+        },
         "ping": {
             "properties": {
                 "message": {
                     "type": "string"
                 },
                 "pong": {
                     "$ref": "swagger.json#/definitions/pong"
+                },
+                "type": {
+                    "type": "string"
                 }
             },
             "required": [
-                "message"
+                "message",
+                "type"
             ],
+            "discriminator": "type",
             "type": "object",
             "x-model": "ping"
         },

test-data/2.0/multi-file-recursive/flattened-multi-file-recursive-spec.json

@@ -1,8 +1,20 @@
 {
   "definitions": {
-    "inline_model": {
+    "mobile_ping": {
+      "allOf": [
+        {
+          "$ref": "#/definitions/ping"
+        },
+        {
+          "properties": {
+            "text": {
+              "type": "string"
+            }
+          }
+        }
+      ],
       "type": "object",
-      "x-model": "inline_model"
+      "x-model": "mobile_ping"
     },
     "model_with_allOf_recursive": {
       "allOf": [
@@ -23,39 +35,42 @@
       ],
       "x-model": "model_with_allOf_recursive"
     },
-    "not_referenced_models_in_not_directly_linked_file": {
-      "properties": {
-        "inline_model": {
-          "$ref": "#/definitions/inline_model"
-        }
-      },
-      "type": "object",
-      "x-model": "not_referenced_models_in_not_directly_linked_file"
-    },
     "not_used": {
       "type": "object",
       "x-model": "not_used"
     },
-    "not_used_remote_reference": {
-      "properties": {
-        "random_number": {
-          "$ref": "#/definitions/random_integer"
+    "phone_ping": {
+      "allOf": [
+        {
+          "$ref": "#/definitions/ping"
+        },
+        {
+          "properties": {
+            "duration": {
+              "type": "number"
+            }
+          }
         }
-      },
+      ],
       "type": "object",
-      "x-model": "not_used_remote_reference"
+      "x-model": "phone_ping"
     },
     "ping": {
+      "discriminator": "type",
       "properties": {
         "message": {
           "type": "string"
         },
         "pong": {
           "$ref": "#/definitions/pong"
+        },
+        "type": {
+          "type": "string"
         }
       },
       "required": [
-        "message"
+        "message",
+        "type"
       ],
       "type": "object",
       "x-model": "ping"
@@ -74,21 +89,6 @@
       ],
       "type": "object",
       "x-model": "pong"
-    },
-    "random_integer": {
-      "properties": {
-        "max-value": {
-          "type": "integer"
-        },
-        "min-value": {
-          "type": "integer"
-        },
-        "value": {
-          "type": "integer"
-        }
-      },
-      "type": "object",
-      "x-model": "random_integer"
     }
   },
   "info": {

tests/model/create_model_type_test.py

@@ -3,6 +3,7 @@
 import pytest
 
 from bravado_core.model import create_model_type
+from bravado_core.schema import is_ref
 
 
 def test_pet_model(empty_swagger_spec, pet_spec):
@@ -70,3 +71,42 @@ def test_deprecated_marshal_and_unmarshal(petstore_spec):
     assert unmarshalled_marshalled_model.id == pet_id
     assert unmarshalled_marshalled_model.name == pet_name
     assert unmarshalled_marshalled_model.photoUrls == pet_photo_urls
+
+
+@pytest.mark.parametrize(
+    'deref_value, expected_inherits',
+    [
+        [{'type': 'object'}, []],
+        [{'type': 'object', 'x-model': 'GenericPet'}, ['GenericPet']],
+        [{'type': 'object', 'title': 'GenericPet'}, ['GenericPet']],
+        [{'type': 'object', 'x-model': 'GenericPet', 'title': 'overridden'}, ['GenericPet']],
+    ],
+)
+def test_create_model_type_properly_extracts_model_name(deref_value, expected_inherits):
+    swagger_spec = mock.Mock(
+        name='swagger-spec',
+        deref=lambda schema: deref_value if is_ref(schema) else schema,
+    )
+    model_type = create_model_type(
+        swagger_spec=swagger_spec,
+        model_name='Dog',
+        model_spec={
+            'type': 'object',
+            'title': 'Dog',
+            'allOf': [
+                {
+                    '$ref': '#/definitions/GenericPet'
+                },
+                {
+                    'properties': {
+                        'birth_date': {
+                            'type': 'string',
+                            'format': 'date',
+                        },
+                    },
+                    'required': ['birth_date'],
+                },
+            ],
+        }
+    )
+    assert model_type._inherits_from == expected_inherits