Merge pull request #126 from jbryan/better_reference_handling

Added support for circular references and additionalProperties

connexion/decorators/response.py

@@ -45,10 +45,11 @@ class ResponseValidator(BaseDecorator):
         if not response_definitions:
             return True
         response_definition = response_definitions.get(status_code, {})
+        response_definition = self.operation.resolve_reference(response_definition)
         # TODO handle default response definitions
 
         if response_definition and response_definition.get("schema"):
-            schema = self.operation.resolve_reference(response_definition.get("schema"))
+            schema = response_definition.get("schema")
             v = RequestBodyValidator(schema)
             error = v.validate_schema(data, schema)
             if error:

connexion/operation.py

@@ -123,44 +123,77 @@ class Operation:
 
     def resolve_reference(self, schema):
         schema = deepcopy(schema)  # avoid changing the original schema
-        # find the object we need to resolve/update
+        self.check_references(schema)
+
+        # find the object we need to resolve/update if this is not a proper SchemaObject
+        # e.g a response or parameter object
         for obj in schema, schema.get('items'):
             reference = obj and obj.get('$ref')  # type: str
             if reference:
                 break
         if reference:
-            if not reference.startswith('#/'):
-                raise InvalidSpecification(
-                    "{method} {path}  '$ref' needs to start with '#/'".format(**vars(self)))
-            path = reference.split('/')
-            definition_type = path[1]
-            try:
-                definitions = self.definitions_map[definition_type]
-            except KeyError:
-                raise InvalidSpecification(
-                    "{method} {path}  '$ref' needs to point to definitions or parameters".format(**vars(self)))
-            definition_name = path[-1]
-            try:
-                # Get sub definition
-                definition = deepcopy(definitions[definition_name])
-            except KeyError:
-                raise InvalidSpecification("{method} {path} Definition '{definition_name}' not found".format(
-                    definition_name=definition_name, method=self.method, path=self.path))
-
-            # resolve object properties too
-            for prop, prop_spec in definition.get('properties', {}).items():
-                resolved = self.resolve_reference(prop_spec.get('schema', {}))
-                if not resolved:
-                    resolved = self.resolve_reference(prop_spec)
-
-                if resolved:
-                    definition['properties'][prop] = resolved
-
+            definition = deepcopy(self._retrieve_reference(reference))
             # Update schema
             obj.update(definition)
             del obj['$ref']
+
+        # if there is a schema object on this param or response, then we just
+        # need to include the defs and it can be validated by jsonschema
+        if 'schema' in schema:
+            schema['schema']['definitions'] = self.definitions
+            return schema
+
         return schema
 
+    def check_references(self, schema):
+        """
+        Searches the keys and values of a schema object for json references.
+        If it finds one, it attempts to locate it and will thrown an exception
+        if the reference can't be found in the definitions dictionary.
+
+        :param schema: The schema object to check
+        :type schema: dict
+        :raises InvalidSpecification: raised when a reference isn't found
+        """
+
+        stack = [schema]
+        visited = set()
+        while stack:
+            schema = stack.pop()
+            for k, v in schema.items():
+                if k == "$ref":
+                    if v in visited:
+                        continue
+                    visited.add(v)
+                    stack.append(self._retrieve_reference(v))
+                elif isinstance(v, (list, tuple)):
+                    for item in v:
+                        if hasattr(item, "items"):
+                            stack.append(item)
+                elif hasattr(v, "items"):
+                    stack.append(v)
+
+    def _retrieve_reference(self, reference):
+        if not reference.startswith('#/'):
+            raise InvalidSpecification(
+                "{method} {path}  '$ref' needs to start with '#/'".format(**vars(self)))
+        path = reference.split('/')
+        definition_type = path[1]
+        try:
+            definitions = self.definitions_map[definition_type]
+        except KeyError:
+            raise InvalidSpecification(
+                "{method} {path}  '$ref' needs to point to definitions or parameters".format(**vars(self)))
+        definition_name = path[-1]
+        try:
+            # Get sub definition
+            definition = deepcopy(definitions[definition_name])
+        except KeyError:
+            raise InvalidSpecification("{method} {path} Definition '{definition_name}' not found".format(
+                definition_name=definition_name, method=self.method, path=self.path))
+
+        return definition
+
     def get_mimetype(self):
         if produces_json(self.produces):  # endpoint will return json
             try:
@@ -202,9 +235,6 @@ class Operation:
 
         body_parameters = body_parameters[0] if body_parameters else {}
         schema = body_parameters.get('schema')  # type: dict
-
-        if schema:
-            schema = self.resolve_reference(schema)
         return schema
 
     @property
@@ -215,15 +245,7 @@ class Operation:
         :rtype: types.FunctionType
         """
 
-        parameters = []
-        for param in self.parameters:  # resolve references
-            param = param.copy()
-            schema = param.get('schema')
-            if schema:
-                schema = self.resolve_reference(schema)
-            param['schema'] = schema
-            parameters.append(param)
-        function = parameter_to_arg(parameters, self.__undecorated_function)
+        function = parameter_to_arg(self.parameters, self.__undecorated_function)
 
         if self.validate_responses:
             logger.debug('... Response validation enabled.')

tests/fakeapi/api.yaml

@@ -352,6 +352,44 @@ paths:
           description: goodbye response
           schema:
             type: string
+  /test_schema_map:
+    post:
+      summary: Returns empty response
+      description: Returns empty response
+      operationId: fakeapi.hello.schema_map
+      parameters:
+        - name: new_stack
+          required: true
+          in: body
+          schema:
+            type: object
+            additionalProperties:
+              $ref: '#/definitions/new_stack'
+      produces:
+        - application/json
+      responses:
+        200:
+          description: goodbye response
+          schema:
+            type: string
+  /test_schema_recursive:
+    post:
+      summary: Returns empty response
+      description: Returns empty response
+      operationId: fakeapi.hello.schema_recursive
+      parameters:
+        - name: new_stack
+          required: true
+          in: body
+          schema:
+            $ref: '#/definitions/simple_tree'
+      produces:
+        - application/json
+      responses:
+        200:
+          description: goodbye response
+          schema:
+            type: string
   /test_schema_format:
     post:
       summary: Returns empty response
@@ -681,3 +719,12 @@ definitions:
         description: Docker image version to deploy
     required:
       - image_version
+  simple_tree:
+    type: object
+    properties:
+      children:
+        type: array
+        items: 
+          $ref: "#/definitions/simple_tree"
+        description: Docker image version to deploy
+    additionalProperties: false

tests/fakeapi/hello.py

@@ -160,6 +160,11 @@ def schema_query(image_version=None):
 def schema_list():
     return ''
 
+def schema_map():
+    return ''
+
+def schema_recursive():
+    return ''
 
 def schema_format():
     return ''

tests/test_app.py

@@ -397,6 +397,78 @@ def test_schema_list(app):
     assert wrong_items_response['title'] == 'Bad Request'
     assert wrong_items_response['detail'].startswith("42 is not of type 'string'")
 
+def test_schema_map(app):
+    app_client = app.app.test_client()
+    headers = {'Content-type': 'application/json'}
+
+    valid_object = {
+        "foo": {
+            "image_version": "string"
+        },
+        "bar": {
+            "image_version": "string"
+        }
+    }
+
+    invalid_object = { 
+        "foo": 42
+    }
+
+    wrong_type = app_client.post('/v1.0/test_schema_map', headers=headers, data=json.dumps(42))  # type: flask.Response
+    assert wrong_type.status_code == 400
+    assert wrong_type.content_type == 'application/problem+json'
+    wrong_type_response = json.loads(wrong_type.data.decode())  # type: dict
+    assert wrong_type_response['title'] == 'Bad Request'
+    assert wrong_type_response['detail'].startswith("42 is not of type 'object'")
+
+    wrong_items = app_client.post('/v1.0/test_schema_map', headers=headers,
+                                  data=json.dumps(invalid_object))  # type: flask.Response
+    assert wrong_items.status_code == 400
+    assert wrong_items.content_type == 'application/problem+json'
+    wrong_items_response = json.loads(wrong_items.data.decode())  # type: dict
+    assert wrong_items_response['title'] == 'Bad Request'
+    assert wrong_items_response['detail'].startswith("42 is not of type 'object'")
+
+    right_type = app_client.post('/v1.0/test_schema_map', headers=headers,
+                                  data=json.dumps(valid_object))  # type: flask.Response
+    assert right_type.status_code == 200
+
+def test_schema_recursive(app):
+    app_client = app.app.test_client()
+    headers = {'Content-type': 'application/json'}
+
+    valid_object = {
+        "children": [
+            {"children": []},
+            {"children": [
+                {"children": []},
+            ]},
+            {"children": []},
+        ]
+    }
+
+    invalid_object = { 
+        "children": [42]
+    }
+
+    wrong_type = app_client.post('/v1.0/test_schema_recursive', headers=headers, data=json.dumps(42))  # type: flask.Response
+    assert wrong_type.status_code == 400
+    assert wrong_type.content_type == 'application/problem+json'
+    wrong_type_response = json.loads(wrong_type.data.decode())  # type: dict
+    assert wrong_type_response['title'] == 'Bad Request'
+    assert wrong_type_response['detail'].startswith("42 is not of type 'object'")
+
+    wrong_items = app_client.post('/v1.0/test_schema_recursive', headers=headers,
+                                  data=json.dumps(invalid_object))  # type: flask.Response
+    assert wrong_items.status_code == 400
+    assert wrong_items.content_type == 'application/problem+json'
+    wrong_items_response = json.loads(wrong_items.data.decode())  # type: dict
+    assert wrong_items_response['title'] == 'Bad Request'
+    assert wrong_items_response['detail'].startswith("42 is not of type 'object'")
+
+    right_type = app_client.post('/v1.0/test_schema_recursive', headers=headers,
+                                  data=json.dumps(valid_object))  # type: flask.Response
+    assert right_type.status_code == 200
 
 def test_schema_format(app):
     app_client = app.app.test_client()

tests/test_operation.py

@@ -255,7 +255,12 @@ def test_operation():
     assert operation.method == 'GET'
     assert operation.produces == ['application/json']
     assert operation.security == [{'oauth': ['uid']}]
-    assert operation.body_schema == DEFINITIONS['new_stack']
+
+    expected_body_schema = {
+        '$ref': '#/definitions/new_stack',
+        'definitions': DEFINITIONS
+    }
+    assert operation.body_schema == expected_body_schema
 
 
 def test_operation_array():
@@ -277,7 +282,12 @@ def test_operation_array():
     assert operation.method == 'GET'
     assert operation.produces == ['application/json']
     assert operation.security == [{'oauth': ['uid']}]
-    assert operation.body_schema == {'type': 'array', 'items': DEFINITIONS['new_stack']}
+    expected_body_schema = {
+        'type': 'array', 
+        'items': {'$ref': '#/definitions/new_stack'},
+        'definitions': DEFINITIONS
+    }
+    assert operation.body_schema == expected_body_schema
 
 
 def test_operation_composed_definition():
@@ -299,22 +309,24 @@ def test_operation_composed_definition():
     assert operation.method == 'GET'
     assert operation.produces == ['application/json']
     assert operation.security == [{'oauth': ['uid']}]
-    definition = deepcopy(DEFINITIONS['composed'])
-    definition['properties']['test'] = DEFINITIONS['new_stack']
-    assert operation.body_schema == definition
+    expected_body_schema = {
+        '$ref': '#/definitions/composed',
+        'definitions': DEFINITIONS
+    }
+    assert operation.body_schema == expected_body_schema
 
 
 def test_non_existent_reference():
-    operation = Operation(method='GET',
-                          path='endpoint',
-                          operation=OPERATION1,
-                          app_produces=['application/json'],
-                          app_security=[],
-                          security_definitions={},
-                          definitions={},
-                          parameter_definitions={},
-                          resolver=Resolver())
     with pytest.raises(InvalidSpecification) as exc_info:  # type: py.code.ExceptionInfo
+        operation = Operation(method='GET',
+                            path='endpoint',
+                            operation=OPERATION1,
+                            app_produces=['application/json'],
+                            app_security=[],
+                            security_definitions={},
+                            definitions={},
+                            parameter_definitions={},
+                            resolver=Resolver())
         schema = operation.body_schema
 
     exception = exc_info.value
@@ -323,16 +335,16 @@ def test_non_existent_reference():
 
 
 def test_multi_body():
-    operation = Operation(method='GET',
-                          path='endpoint',
-                          operation=OPERATION2,
-                          app_produces=['application/json'],
-                          app_security=[],
-                          security_definitions={},
-                          definitions=DEFINITIONS,
-                          parameter_definitions=PARAMETER_DEFINITIONS,
-                          resolver=Resolver())
     with pytest.raises(InvalidSpecification) as exc_info:  # type: py.code.ExceptionInfo
+        operation = Operation(method='GET',
+                            path='endpoint',
+                            operation=OPERATION2,
+                            app_produces=['application/json'],
+                            app_security=[],
+                            security_definitions={},
+                            definitions=DEFINITIONS,
+                            parameter_definitions=PARAMETER_DEFINITIONS,
+                            resolver=Resolver())
         schema = operation.body_schema
 
     exception = exc_info.value
@@ -341,16 +353,16 @@ def test_multi_body():
 
 
 def test_invalid_reference():
-    operation = Operation(method='GET',
-                          path='endpoint',
-                          operation=OPERATION3,
-                          app_produces=['application/json'],
-                          app_security=[],
-                          security_definitions={},
-                          definitions=DEFINITIONS,
-                          parameter_definitions=PARAMETER_DEFINITIONS,
-                          resolver=Resolver())
     with pytest.raises(InvalidSpecification) as exc_info:  # type: py.code.ExceptionInfo
+        operation = Operation(method='GET',
+                            path='endpoint',
+                            operation=OPERATION3,
+                            app_produces=['application/json'],
+                            app_security=[],
+                            security_definitions={},
+                            definitions=DEFINITIONS,
+                            parameter_definitions=PARAMETER_DEFINITIONS,
+                            resolver=Resolver())
         schema = operation.body_schema
 
     exception = exc_info.value
@@ -374,7 +386,12 @@ def test_no_token_info():
     assert operation.method == 'GET'
     assert operation.produces == ['application/json']
     assert operation.security == [{'oauth': ['uid']}]
-    assert operation.body_schema == DEFINITIONS['new_stack']
+
+    expected_body_schema = {
+        '$ref': '#/definitions/new_stack',
+        'definitions': DEFINITIONS
+    }
+    assert operation.body_schema == expected_body_schema
 
 
 def test_parameter_reference():
@@ -403,7 +420,7 @@ def test_resolve_invalid_reference():
 def test_bad_default():
     with pytest.raises(InvalidSpecification) as exc_info:
         Operation(method='GET', path='endpoint', operation=OPERATION6, app_produces=['application/json'],
-                  app_security=[], security_definitions={}, definitions={}, parameter_definitions=PARAMETER_DEFINITIONS,
+                  app_security=[], security_definitions={}, definitions=DEFINITIONS, parameter_definitions=PARAMETER_DEFINITIONS,
                   resolver=Resolver())
     exception = exc_info.value
     assert str(exception) == "<InvalidSpecification: The parameter 'stack_version' has a default value which " \