Merge pull request #39 from cebe/fix-resolving-references

Fix resolving recursive references
cebe · Oct 25, 2019 · ddfcbe0 · ddfcbe0
2 parents 6b8414c + e2d3cc6
commit ddfcbe0
Show file tree

Hide file tree

Showing 11 changed files with 135 additions and 11 deletions.
diff --git a/README.md b/README.md
@@ -194,13 +194,11 @@ $openapi->resolveReferences(
 );
 ```
 
-> **Note:** Resolving references currently does not deal with references in referenced files, you have to call it multiple times to resolve these.
-
 ### Validation
 
 The library provides simple validation operations, that check basic OpenAPI spec requirements.
 This is the same as "structural errors found while reading the API Description file" from the CLI tool.
-This validation does not include checking against the OpenAPI v3.0 JSON schema.
+This validation does not include checking against the OpenAPI v3.0 JSON schema, this is only implemented in the CLI.
 
 ```
 // return `true` in case no errors have been found, `false` in case of errors.

diff --git a/src/SpecBaseObject.php b/src/SpecBaseObject.php
@@ -343,21 +343,37 @@ public function __unset($name)
      */
     public function resolveReferences(ReferenceContext $context = null)
     {
+        // avoid recursion to get stuck in a loop
+        if ($this->_recursing) {
+            return;
+        }
+        $this->_recursing = true;
+
         foreach ($this->_properties as $property => $value) {
             if ($value instanceof Reference) {
-                $this->_properties[$property] = $value->resolve($context);
+                $referencedObject = $value->resolve($context);
+                $this->_properties[$property] = $referencedObject;
+                if (!$referencedObject instanceof Reference && $referencedObject !== null) {
+                    $referencedObject->resolveReferences();
+                }
             } elseif ($value instanceof SpecObjectInterface) {
                 $value->resolveReferences($context);
             } elseif (is_array($value)) {
                 foreach ($value as $k => $item) {
                     if ($item instanceof Reference) {
-                        $this->_properties[$property][$k] = $item->resolve($context);
+                        $referencedObject = $item->resolve($context);
+                        $this->_properties[$property][$k] = $referencedObject;
+                        if (!$referencedObject instanceof Reference && $referencedObject !== null) {
+                            $referencedObject->resolveReferences();
+                        }
                     } elseif ($item instanceof SpecObjectInterface) {
                         $item->resolveReferences($context);
                     }
                 }
             }
         }
+
+        $this->_recursing = false;
     }
 
     /**

diff --git a/src/spec/Operation.php b/src/spec/Operation.php
@@ -21,7 +21,7 @@
  * @property string $operationId
  * @property Parameter[]|Reference[] $parameters
  * @property RequestBody|Reference|null $requestBody
- * @property Responses|null $responses
+ * @property Responses|Response[]|null $responses
  * @property Callback[]|Reference[] $callbacks
  * @property bool $deprecated
  * @property SecurityRequirement[] $security

diff --git a/src/spec/PathItem.php b/src/spec/PathItem.php
@@ -159,6 +159,30 @@ public function resolveReferences(ReferenceContext $context = null)
                     $this->addError("Conflicting properties, property '$attribute' exists in local PathItem and also in the referenced one.");
                 }
                 $this->$attribute = $pathItem->$attribute;
+
+                // resolve references in all properties assinged from the reference
+                // use the referenced object context in this case
+                if ($this->$attribute instanceof Reference) {
+                    $referencedObject = $this->$attribute->resolve();
+                    $this->$attribute = $referencedObject;
+                    if (!$referencedObject instanceof Reference && $referencedObject !== null) {
+                        $referencedObject->resolveReferences();
+                    }
+                } elseif ($this->$attribute instanceof SpecObjectInterface) {
+                    $this->$attribute->resolveReferences();
+                } elseif (is_array($this->$attribute)) {
+                    foreach ($this->$attribute as $k => $item) {
+                        if ($item instanceof Reference) {
+                            $referencedObject = $item->resolve();
+                            $this->$attribute[$k] = $referencedObject;
+                            if (!$referencedObject instanceof Reference && $referencedObject !== null) {
+                                $referencedObject->resolveReferences();
+                            }
+                        } elseif ($item instanceof SpecObjectInterface) {
+                            $item->resolveReferences();
+                        }
+                    }
+                }
             }
         }
         parent::resolveReferences($context);

diff --git a/src/spec/Reference.php b/src/spec/Reference.php
@@ -146,6 +146,9 @@ public function getContext() : ?ReferenceContext
      * If not specified, `getContext()` will be called to determine the context, if
      * that does not return a context, the UnresolvableReferenceException will be thrown.
      * @return SpecObjectInterface the resolved spec type.
+     * You might want to call resolveReferences() on the resolved object to recursively resolve recursive references.
+     * This is not done automatically to avoid recursion to run into the same function again.
+     * If you call resolveReferences() make sure to replace the Reference with the resolved object first.
      * @throws UnresolvableReferenceException in case of errors.
      */
     public function resolve(ReferenceContext $context = null)
@@ -169,7 +172,10 @@ public function resolve(ReferenceContext $context = null)
                 $baseSpec = $context->getBaseSpec();
                 if ($baseSpec !== null) {
                     // TODO type error if resolved object does not match $this->_to ?
-                    return $jsonReference->getJsonPointer()->evaluate($baseSpec);
+                    /** @var $referencedObject SpecObjectInterface */
+                    $referencedObject = $jsonReference->getJsonPointer()->evaluate($baseSpec);
+                    $referencedObject->setReferenceContext($context);
+                    return $referencedObject;
                 } else {
                     // if current document was loaded via reference, it may be null,
                     // so we load current document by URI instead.
@@ -189,15 +195,19 @@ public function resolve(ReferenceContext $context = null)
             /** @var $referencedObject SpecObjectInterface */
             $referencedObject = new $this->_to($referencedData);
             if ($jsonReference->getJsonPointer()->getPointer() === '') {
-                $referencedObject->setReferenceContext(new ReferenceContext($referencedObject, $file));
+                $newContext = new ReferenceContext($referencedObject, $file);
+                $newContext->throwException = $context->throwException;
+                $referencedObject->setReferenceContext($newContext);
                 if ($referencedObject instanceof DocumentContextInterface) {
                     $referencedObject->setDocumentContext($referencedObject, $jsonReference->getJsonPointer());
                 }
             } else {
                 // resolving references recursively does not work the same if we have not referenced
                 // the whole document. We do not know the base type of the file at this point,
                 // so base document must be null.
-                $referencedObject->setReferenceContext(new ReferenceContext(null, $file));
+                $newContext = new ReferenceContext(null, $file);
+                $newContext->throwException = $context->throwException;
+                $referencedObject->setReferenceContext($newContext);
             }
 
             return $referencedObject;

diff --git a/src/spec/Responses.php b/src/spec/Responses.php
@@ -239,7 +239,11 @@ public function resolveReferences(ReferenceContext $context = null)
     {
         foreach ($this->_responses as $key => $response) {
             if ($response instanceof Reference) {
-                $this->_responses[$key] = $response->resolve($context);
+                $referencedObject = $response->resolve($context);
+                $this->_responses[$key] = $referencedObject;
+                if (!$referencedObject instanceof Reference && $referencedObject !== null) {
+                    $referencedObject->resolveReferences();
+                }
             } else {
                 $response->resolveReferences($context);
             }

diff --git a/tests/spec/ReferenceTest.php b/tests/spec/ReferenceTest.php
@@ -162,15 +162,43 @@ public function testResolveFile()
 
         // second level reference inside of definitions.yaml
         $this->assertArrayHasKey('food', $openapi->components->schemas['Dog']->properties);
-        $this->assertInstanceOf(Reference::class, $openapi->components->schemas['Dog']->properties['food']);
+        $this->assertInstanceOf(Schema::class, $openapi->components->schemas['Dog']->properties['food']);
+        $this->assertArrayHasKey('id', $openapi->components->schemas['Dog']->properties['food']->properties);
+        $this->assertArrayHasKey('name', $openapi->components->schemas['Dog']->properties['food']->properties);
+        $this->assertEquals(1, $openapi->components->schemas['Dog']->properties['food']->properties['id']->example);
+    }
+
+    public function testResolveFileInSubdir()
+    {
+        $file = __DIR__ . '/data/reference/subdir.yaml';
+        /** @var $openapi OpenApi */
+        $openapi = Reader::readFromYamlFile($file, OpenApi::class, false);
+
+        $result = $openapi->validate();
+        $this->assertEquals([], $openapi->getErrors());
+        $this->assertTrue($result);
+
+        $this->assertInstanceOf(Reference::class, $petItems = $openapi->components->schemas['Pet']);
+        $this->assertInstanceOf(Reference::class, $petItems = $openapi->components->schemas['Dog']);
 
         $openapi->resolveReferences(new \cebe\openapi\ReferenceContext($openapi, $file));
 
+        $this->assertInstanceOf(Schema::class, $petItems = $openapi->components->schemas['Pet']);
+        $this->assertInstanceOf(Schema::class, $petItems = $openapi->components->schemas['Dog']);
+        $this->assertArrayHasKey('id', $openapi->components->schemas['Pet']->properties);
+        $this->assertArrayHasKey('name', $openapi->components->schemas['Dog']->properties);
+
+        // second level reference inside of definitions.yaml
         $this->assertArrayHasKey('food', $openapi->components->schemas['Dog']->properties);
         $this->assertInstanceOf(Schema::class, $openapi->components->schemas['Dog']->properties['food']);
         $this->assertArrayHasKey('id', $openapi->components->schemas['Dog']->properties['food']->properties);
         $this->assertArrayHasKey('name', $openapi->components->schemas['Dog']->properties['food']->properties);
         $this->assertEquals(1, $openapi->components->schemas['Dog']->properties['food']->properties['id']->example);
+
+        $this->assertEquals('return a pet', $openapi->paths->getPath('/pets')->get->responses[200]->description);
+        $responseContent = $openapi->paths->getPath('/pets')->get->responses[200]->content['application/json'];
+        $this->assertInstanceOf(Schema::class, $responseContent->schema);
+        $this->assertEquals('A Pet', $responseContent->schema->description);
     }
 
     public function testResolveFileHttp()

diff --git a/tests/spec/data/reference/paths/pets.json b/tests/spec/data/reference/paths/pets.json
@@ -0,0 +1,14 @@
+{
+  "get": {
+    "responses": {
+      "200": {
+        "description": "return a pet",
+        "content": {
+          "application/json": {
+            "schema": {"$ref":  "../subdir/Pet.yaml"}
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/tests/spec/data/reference/subdir.yaml b/tests/spec/data/reference/subdir.yaml
@@ -0,0 +1,18 @@
+openapi: 3.0.0
+info:
+  title: Link Example
+  version: 1.0.0
+components:
+  schemas:
+    Pet:
+      $ref: 'subdir/Pet.yaml'
+    Dog:
+      $ref: 'subdir/Dog.yaml'
+paths:
+  '/pet':
+    get:
+      responses:
+        200:
+          description: return a pet
+  '/pets':
+    "$ref": "paths/pets.json"
diff --git a/tests/spec/data/reference/subdir/Dog.yaml b/tests/spec/data/reference/subdir/Dog.yaml
@@ -0,0 +1,6 @@
+type: object
+properties:
+  name:
+    type: string
+  food:
+    $ref: '../Food.yaml'
diff --git a/tests/spec/data/reference/subdir/Pet.yaml b/tests/spec/data/reference/subdir/Pet.yaml
@@ -0,0 +1,6 @@
+type: object
+description: "A Pet"
+properties:
+  id:
+    type: integer
+    format: int64