From e0cd761f262e8005924ee88a459d9780f8ed5b60 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Tue, 3 Dec 2019 12:09:57 +0100 Subject: [PATCH 1/2] Fix bug with referencing non-object types a reference to an array was crashing with an invalid function call. Proper instanceof check has been added in Reference. also creating references in non-object types and Type::ANY is working correctly now. fixes #47 --- src/SpecBaseObject.php | 25 ++++++++++-- src/spec/Reference.php | 14 ++++--- tests/spec/ReferenceTest.php | 47 ++++++++++++++++++++++ tests/spec/data/reference/definitions.yaml | 5 +++ 4 files changed, 82 insertions(+), 9 deletions(-) diff --git a/src/SpecBaseObject.php b/src/SpecBaseObject.php index f811067a..aa231aff 100644 --- a/src/SpecBaseObject.php +++ b/src/SpecBaseObject.php @@ -73,6 +73,11 @@ public function __construct(array $data) $this->_errors[] = "property '$property' must be array, but " . gettype($data[$property]) . " given."; continue; } + if (isset($data[$property]['$ref'])) { + $this->_properties[$property] = new Reference($data[$property], null); + unset($data[$property]); + continue; + } switch (\count($type)) { case 1: // array @@ -83,8 +88,14 @@ public function __construct(array $data) $this->_errors[] = "property '$property' must be array of strings, but array has " . gettype($item) . " element."; } $this->_properties[$property][] = $item; - } elseif ($type[0] === Type::ANY || Type::isScalar($type[0])) { + } elseif (Type::isScalar($type[0])) { $this->_properties[$property][] = $item; + } elseif ($type[0] === Type::ANY) { + if (is_array($item) && isset($item['$ref'])) { + $this->_properties[$property][] = new Reference($item, null); + } else { + $this->_properties[$property][] = $item; + } } else { $this->_properties[$property][] = $this->instantiate($type[0], $item); } @@ -110,8 +121,14 @@ public function __construct(array $data) } break; } - } elseif ($type === Type::ANY || Type::isScalar($type)) { + } elseif (Type::isScalar($type)) { $this->_properties[$property] = $data[$property]; + } elseif ($type === Type::ANY) { + if (is_array($data[$property]) && isset($data[$property]['$ref'])) { + $this->_properties[$property] = new Reference($data[$property], null); + } else { + $this->_properties[$property] = $data[$property]; + } } else { $this->_properties[$property] = $this->instantiate($type, $data[$property]); } @@ -353,7 +370,7 @@ public function resolveReferences(ReferenceContext $context = null) if ($value instanceof Reference) { $referencedObject = $value->resolve($context); $this->_properties[$property] = $referencedObject; - if (!$referencedObject instanceof Reference && $referencedObject !== null) { + if (!$referencedObject instanceof Reference && $referencedObject instanceof SpecObjectInterface) { $referencedObject->resolveReferences(); } } elseif ($value instanceof SpecObjectInterface) { @@ -363,7 +380,7 @@ public function resolveReferences(ReferenceContext $context = null) if ($item instanceof Reference) { $referencedObject = $item->resolve($context); $this->_properties[$property][$k] = $referencedObject; - if (!$referencedObject instanceof Reference && $referencedObject !== null) { + if (!$referencedObject instanceof Reference && $referencedObject instanceof SpecObjectInterface) { $referencedObject->resolveReferences(); } } elseif ($item instanceof SpecObjectInterface) { diff --git a/src/spec/Reference.php b/src/spec/Reference.php index b2e30d23..1610162d 100644 --- a/src/spec/Reference.php +++ b/src/spec/Reference.php @@ -174,7 +174,9 @@ public function resolve(ReferenceContext $context = null) // TODO type error if resolved object does not match $this->_to ? /** @var $referencedObject SpecObjectInterface */ $referencedObject = $jsonReference->getJsonPointer()->evaluate($baseSpec); - $referencedObject->setReferenceContext($context); + if ($referencedObject instanceof SpecObjectInterface) { + $referencedObject->setReferenceContext($context); + } return $referencedObject; } else { // if current document was loaded via reference, it may be null, @@ -196,10 +198,10 @@ public function resolve(ReferenceContext $context = null) // transitive reference if (isset($referencedData['$ref'])) { return (new Reference($referencedData, $this->_to))->resolve(new ReferenceContext(null, $file)); - } else { - /** @var $referencedObject SpecObjectInterface */ - $referencedObject = new $this->_to($referencedData); } + /** @var $referencedObject SpecObjectInterface|array */ + $referencedObject = $this->_to !== null ? new $this->_to($referencedData) : $referencedData; + if ($jsonReference->getJsonPointer()->getPointer() === '') { $newContext = new ReferenceContext($referencedObject, $file); if ($referencedObject instanceof DocumentContextInterface) { @@ -212,7 +214,9 @@ public function resolve(ReferenceContext $context = null) $newContext = new ReferenceContext(null, $file); } $newContext->throwException = $context->throwException; - $referencedObject->setReferenceContext($newContext); + if ($referencedObject instanceof SpecObjectInterface) { + $referencedObject->setReferenceContext($newContext); + } return $referencedObject; } catch (NonexistentJsonPointerReferenceException $e) { diff --git a/tests/spec/ReferenceTest.php b/tests/spec/ReferenceTest.php index 16f33614..990fc9e7 100644 --- a/tests/spec/ReferenceTest.php +++ b/tests/spec/ReferenceTest.php @@ -257,4 +257,51 @@ public function testResolvePaths() $this->assertInstanceOf(RequestBody::class, $newPlaylistBody); $this->assertSame($playlistsBody, $newPlaylistBody); } + + public function testReferenceToArray() + { + $schema = <<<'YAML' +openapi: 3.0.0 +components: + schemas: + Pet: + type: object + properties: + id: + type: integer + typeA: + type: string + enum: + - "One" + - "Two" + typeB: + type: string + enum: + $ref: '#/components/schemas/Pet/properties/typeA/enum' + typeC: + type: string + enum: + - "Three" + - $ref: '#/components/schemas/Pet/properties/typeA/enum/1' + typeD: + type: string + enum: + $ref: 'definitions.yaml#/Dog/properties/typeD/enum' + typeE: + type: string + enum: + - $ref: 'definitions.yaml#/Dog/properties/typeD/enum/1' + - "Six" + +YAML; + $openapi = Reader::readFromYaml($schema); + $openapi->resolveReferences(new \cebe\openapi\ReferenceContext($openapi, 'file://' . __DIR__ . '/data/reference/definitions.yaml')); + + $this->assertTrue(isset($openapi->components->schemas['Pet'])); + $this->assertEquals(['One', 'Two'], $openapi->components->schemas['Pet']->properties['typeA']->enum); + $this->assertEquals(['One', 'Two'], $openapi->components->schemas['Pet']->properties['typeB']->enum); + $this->assertEquals(['Three', 'Two'], $openapi->components->schemas['Pet']->properties['typeC']->enum); + $this->assertEquals(['Four', 'Five'], $openapi->components->schemas['Pet']->properties['typeD']->enum); + $this->assertEquals(['Five', 'Six'], $openapi->components->schemas['Pet']->properties['typeE']->enum); + } } diff --git a/tests/spec/data/reference/definitions.yaml b/tests/spec/data/reference/definitions.yaml index 9e93f3df..8b255f14 100644 --- a/tests/spec/data/reference/definitions.yaml +++ b/tests/spec/data/reference/definitions.yaml @@ -11,3 +11,8 @@ Dog: type: string food: $ref: 'Food.yaml' + typeD: + type: string + enum: + - "Four" + - "Five" From 3de06bf41dff14683ece989ec2125bd2482e7f36 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Tue, 3 Dec 2019 13:12:18 +0100 Subject: [PATCH 2/2] only allow $ref in array, a map may have a $ref property --- src/SpecBaseObject.php | 41 +++++++++++++++++++------------------- tests/spec/OpenApiTest.php | 5 ++++- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/SpecBaseObject.php b/src/SpecBaseObject.php index aa231aff..ecc1c87a 100644 --- a/src/SpecBaseObject.php +++ b/src/SpecBaseObject.php @@ -73,31 +73,30 @@ public function __construct(array $data) $this->_errors[] = "property '$property' must be array, but " . gettype($data[$property]) . " given."; continue; } - if (isset($data[$property]['$ref'])) { - $this->_properties[$property] = new Reference($data[$property], null); - unset($data[$property]); - continue; - } switch (\count($type)) { case 1: - // array - $this->_properties[$property] = []; - foreach ($data[$property] as $item) { - if ($type[0] === Type::STRING) { - if (!is_string($item)) { - $this->_errors[] = "property '$property' must be array of strings, but array has " . gettype($item) . " element."; - } - $this->_properties[$property][] = $item; - } elseif (Type::isScalar($type[0])) { - $this->_properties[$property][] = $item; - } elseif ($type[0] === Type::ANY) { - if (is_array($item) && isset($item['$ref'])) { - $this->_properties[$property][] = new Reference($item, null); - } else { + if (isset($data[$property]['$ref'])) { + $this->_properties[$property] = new Reference($data[$property], null); + } else { + // array + $this->_properties[$property] = []; + foreach ($data[$property] as $item) { + if ($type[0] === Type::STRING) { + if (!is_string($item)) { + $this->_errors[] = "property '$property' must be array of strings, but array has " . gettype($item) . " element."; + } + $this->_properties[$property][] = $item; + } elseif (Type::isScalar($type[0])) { $this->_properties[$property][] = $item; + } elseif ($type[0] === Type::ANY) { + if (is_array($item) && isset($item['$ref'])) { + $this->_properties[$property][] = new Reference($item, null); + } else { + $this->_properties[$property][] = $item; + } + } else { + $this->_properties[$property][] = $this->instantiate($type[0], $item); } - } else { - $this->_properties[$property][] = $this->instantiate($type[0], $item); } } break; diff --git a/tests/spec/OpenApiTest.php b/tests/spec/OpenApiTest.php index 17cd8ed1..80951cb6 100644 --- a/tests/spec/OpenApiTest.php +++ b/tests/spec/OpenApiTest.php @@ -171,7 +171,10 @@ public function specProvider() $nexmoExamples ); foreach($all as $path) { - yield [substr($path, strlen(__DIR__ . '/../../vendor/')), basename($path)]; + yield [ + substr($path, strlen(__DIR__ . '/../../vendor/')), + basename(dirname($path, 2)) . DIRECTORY_SEPARATOR . basename(dirname($path, 1)) . DIRECTORY_SEPARATOR . basename($path) + ]; } }