From fb736f73c9cb459a30b872157faf94a3d3efae3d Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 20 Mar 2020 20:33:28 +0100 Subject: [PATCH 1/8] WIP cache loaded files in reference context simple cache implementation does not yet normalize relative paths in URIs --- src/ReferenceContext.php | 77 ++++++++++++++++++++++++++++++++++- src/ReferenceContextCache.php | 38 +++++++++++++++++ src/spec/Reference.php | 52 +++++------------------ 3 files changed, 125 insertions(+), 42 deletions(-) create mode 100644 src/ReferenceContextCache.php diff --git a/src/ReferenceContext.php b/src/ReferenceContext.php index 7eaab54f..4b55eabf 100644 --- a/src/ReferenceContext.php +++ b/src/ReferenceContext.php @@ -7,7 +7,11 @@ namespace cebe\openapi; +use cebe\openapi\exceptions\IOException; use cebe\openapi\exceptions\UnresolvableReferenceException; +use cebe\openapi\json\JsonPointer; +use cebe\openapi\spec\Reference; +use Symfony\Component\Yaml\Yaml; /** * ReferenceContext represents a context in which references are resolved. @@ -27,17 +31,24 @@ class ReferenceContext * @var string */ private $_uri; + /** + * @var ReferenceContextCache + */ + private $_cache; + /** * ReferenceContext constructor. * @param SpecObjectInterface $base the base object of the spec. * @param string $uri the URI to the base object. + * @param ReferenceContextCache $cache cache instance for storing referenced file data. * @throws UnresolvableReferenceException in case an invalid or non-absolute URI is provided. */ - public function __construct(?SpecObjectInterface $base, string $uri) + public function __construct(?SpecObjectInterface $base, string $uri, $cache = null) { $this->_baseSpec = $base; $this->_uri = $this->normalizeUri($uri); + $this->_cache = $cache ?? new ReferenceContextCache(); } /** @@ -138,4 +149,68 @@ private function dirname($path) } return ''; } + + private $_fileCache; + + /** + * Fetch referenced file by URI. + * + * The current context will cache files by URI, so they are only loaded once. + * + * @throws IOException in case the file is not readable or fetching the file + * from a remote URL failed. + */ + public function fetchReferencedFile($uri) + { + $content = file_get_contents($uri); + if ($content === false) { + $e = new IOException("Failed to read file: '$uri'"); + $e->fileName = $uri; + throw $e; + } + // TODO lazy content detection, should be improved + if (strpos(ltrim($content), '{') === 0) { + return json_decode($content, true); + } else { + return Yaml::parse($content); + } + } + + /** + * Retrieve the referenced data via JSON pointer. + * + * This function caches referenced data to make sure references to the same + * data structures end up being the same object instance in PHP. + * + * @param string $uri + * @param JsonPointer $pointer + * @param array $data + * @param string|null $toType + * @return SpecObjectInterface|array + */ + public function resolveReferenceData($uri, JsonPointer $pointer, $data, $toType) + { + $ref = $uri . '#' . $pointer->getPointer(); + if ($this->_cache->has($ref, $toType)) { + return $this->_cache->get($ref, $toType); + } + + $referencedData = $pointer->evaluate($data); + + if ($referencedData === null) { + return null; + } + + // transitive reference + if (isset($referencedData['$ref'])) { + return (new Reference($referencedData, $toType))->resolve(new ReferenceContext(null, $uri)); + } + /** @var SpecObjectInterface|array $referencedObject */ + $referencedObject = $toType !== null ? new $toType($referencedData) : $referencedData; + + $this->_cache->set($ref, $toType, $referencedObject); + + return $referencedObject; + } + } diff --git a/src/ReferenceContextCache.php b/src/ReferenceContextCache.php new file mode 100644 index 00000000..74ed3028 --- /dev/null +++ b/src/ReferenceContextCache.php @@ -0,0 +1,38 @@ + and contributors + * @license https://github.com/cebe/php-openapi/blob/master/LICENSE + */ + +namespace cebe\openapi; + +/** + * ReferenceContextCache represents a cache storage for caching content of referenced files. + */ +class ReferenceContextCache +{ + private $_cache = []; + + + public function set($ref, $type, $data) + { + $this->_cache[$ref][$type ?? ''] = $data; + + // store fallback value for resolving with unknown type + if ($type !== null && !isset($this->_cache[$ref][''])) { + $this->_cache[$ref][''] = $data; + } + } + + public function get($ref, $type) + { + return $this->_cache[$ref][$type ?? ''] ?? null; + } + + public function has($ref, $type) + { + return isset($this->_cache[$ref]) && + array_key_exists($type ?? '', $this->_cache[$ref]); + } +} diff --git a/src/spec/Reference.php b/src/spec/Reference.php index 1eef2eca..6cebb759 100644 --- a/src/spec/Reference.php +++ b/src/spec/Reference.php @@ -205,20 +205,19 @@ public function resolve(ReferenceContext $context = null) // resolve in external document $file = $context->resolveRelativeUri($jsonReference->getDocumentUri()); - // TODO could be a good idea to cache loaded files in current context to avoid loading the same files over and over again - $referencedDocument = $this->fetchReferencedFile($file); - $referencedData = $jsonReference->getJsonPointer()->evaluate($referencedDocument); - - if ($referencedData === null) { - return null; + try { + $referencedDocument = $context->fetchReferencedFile($file); + } catch (\Throwable $e) { + $exception = new UnresolvableReferenceException( + "Failed to resolve Reference '$this->_ref' to $this->_to Object: " . $e->getMessage(), + $e->getCode(), + $e + ); + $exception->context = $this->getDocumentPosition(); + throw $exception; } - // transitive reference - if (isset($referencedData['$ref'])) { - return (new Reference($referencedData, $this->_to))->resolve(new ReferenceContext(null, $file)); - } - /** @var SpecObjectInterface|array $referencedObject */ - $referencedObject = $this->_to !== null ? new $this->_to($referencedData) : $referencedData; + $referencedObject = $context->resolveReferenceData($file, $jsonReference->getJsonPointer(), $referencedDocument, $this->_to); if ($jsonReference->getJsonPointer()->getPointer() === '') { $newContext = new ReferenceContext($referencedObject instanceof SpecObjectInterface ? $referencedObject : null, $file); @@ -258,35 +257,6 @@ public function resolve(ReferenceContext $context = null) } } - /** - * @throws UnresolvableReferenceException - */ - private function fetchReferencedFile($uri) - { - try { - $content = file_get_contents($uri); - if ($content === false) { - $e = new IOException("Failed to read file: '$uri'"); - $e->fileName = $uri; - throw $e; - } - // TODO lazy content detection, should probably be improved - if (strpos(ltrim($content), '{') === 0) { - return json_decode($content, true); - } else { - return Yaml::parse($content); - } - } catch (\Throwable $e) { - $exception = new UnresolvableReferenceException( - "Failed to resolve Reference '$this->_ref' to $this->_to Object: " . $e->getMessage(), - $e->getCode(), - $e - ); - $exception->context = $this->getDocumentPosition(); - throw $exception; - } - } - /** * Resolves all Reference Objects in this object and replaces them with their resolution. * @throws UnresolvableReferenceException From 518a3d3306760189f79a8d1e0de3b2ddd37d2c81 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Thu, 26 Mar 2020 10:46:03 +0100 Subject: [PATCH 2/8] added failing test --- src/ReferenceContext.php | 9 ++-- src/spec/Reference.php | 4 +- tests/spec/ReferenceTest.php | 44 +++++++++++++++++++ tests/spec/data/reference/models/Cat.yaml | 11 +++++ tests/spec/data/reference/models/Pet.yaml | 8 ++++ tests/spec/data/reference/openapi_models.yaml | 16 +++++++ 6 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 tests/spec/data/reference/models/Cat.yaml create mode 100644 tests/spec/data/reference/models/Pet.yaml create mode 100644 tests/spec/data/reference/openapi_models.yaml diff --git a/src/ReferenceContext.php b/src/ReferenceContext.php index 4b55eabf..0dc99ca9 100644 --- a/src/ReferenceContext.php +++ b/src/ReferenceContext.php @@ -51,6 +51,11 @@ public function __construct(?SpecObjectInterface $base, string $uri, $cache = nu $this->_cache = $cache ?? new ReferenceContextCache(); } + public function getCache(): ReferenceContextCache + { + return $this->_cache; + } + /** * @throws UnresolvableReferenceException in case an invalid or non-absolute URI is provided. */ @@ -150,8 +155,6 @@ private function dirname($path) return ''; } - private $_fileCache; - /** * Fetch referenced file by URI. * @@ -203,7 +206,7 @@ public function resolveReferenceData($uri, JsonPointer $pointer, $data, $toType) // transitive reference if (isset($referencedData['$ref'])) { - return (new Reference($referencedData, $toType))->resolve(new ReferenceContext(null, $uri)); + return (new Reference($referencedData, $toType))->resolve(new ReferenceContext(null, $uri, $this->_cache)); } /** @var SpecObjectInterface|array $referencedObject */ $referencedObject = $toType !== null ? new $toType($referencedData) : $referencedData; diff --git a/src/spec/Reference.php b/src/spec/Reference.php index 6cebb759..bd8262b2 100644 --- a/src/spec/Reference.php +++ b/src/spec/Reference.php @@ -220,7 +220,7 @@ public function resolve(ReferenceContext $context = null) $referencedObject = $context->resolveReferenceData($file, $jsonReference->getJsonPointer(), $referencedDocument, $this->_to); if ($jsonReference->getJsonPointer()->getPointer() === '') { - $newContext = new ReferenceContext($referencedObject instanceof SpecObjectInterface ? $referencedObject : null, $file); + $newContext = new ReferenceContext($referencedObject instanceof SpecObjectInterface ? $referencedObject : null, $file, $context->getCache()); if ($referencedObject instanceof DocumentContextInterface) { $referencedObject->setDocumentContext($referencedObject, $jsonReference->getJsonPointer()); } @@ -228,7 +228,7 @@ public function resolve(ReferenceContext $context = null) // 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. - $newContext = new ReferenceContext(null, $file); + $newContext = new ReferenceContext(null, $file, $context->getCache()); } $newContext->throwException = $context->throwException; if ($referencedObject instanceof SpecObjectInterface) { diff --git a/tests/spec/ReferenceTest.php b/tests/spec/ReferenceTest.php index 223c4eb9..0aee0f31 100644 --- a/tests/spec/ReferenceTest.php +++ b/tests/spec/ReferenceTest.php @@ -416,4 +416,48 @@ public function testTransitiveReferenceCyclic() $openapi->resolveReferences(new \cebe\openapi\ReferenceContext($openapi, 'file:///tmp/openapi.yaml')); } + + public function testResolveRelativePath() + { + $openapi = Reader::readFromYamlFile(__DIR__ . '/data/reference/openapi_models.yaml'); + + $yaml = \cebe\openapi\Writer::writeToYaml($openapi); + + $this->assertEquals( +<< Date: Wed, 8 Jul 2020 10:39:54 +0200 Subject: [PATCH 3/8] WIP --- bin/php-openapi | 1 + src/Reader.php | 24 ++++++--- src/ReferenceContext.php | 91 +++++++++++++++++++++++----------- src/ReferenceContextCache.php | 12 ++++- src/spec/Reference.php | 54 +++++++++++++++++++- tests/ReferenceContextTest.php | 2 +- tests/spec/PathTest.php | 3 +- tests/spec/ReferenceTest.php | 8 +-- 8 files changed, 154 insertions(+), 41 deletions(-) diff --git a/bin/php-openapi b/bin/php-openapi index 81728f12..98c31a8c 100755 --- a/bin/php-openapi +++ b/bin/php-openapi @@ -109,6 +109,7 @@ switch ($command) { $openApi = read_input($inputFile, $inputFormat); $referenceContext = new ReferenceContext($openApi, $inputFile ? realpath($inputFile) : ''); $referenceContext->throwException = false; + $referenceContext->mode = ReferenceContext:: $openApi->resolveReferences($referenceContext); $openApi->setDocumentContext($openApi, new \cebe\openapi\json\JsonPointer('')); diff --git a/src/Reader.php b/src/Reader.php index 08c2996c..143a7b7e 100644 --- a/src/Reader.php +++ b/src/Reader.php @@ -57,9 +57,11 @@ public static function readFromYaml(string $yaml, string $baseType = OpenApi::cl * @param string $baseType the base Type to instantiate. This must be an instance of [[SpecObjectInterface]]. * The default is [[OpenApi]] which is the base type of a OpenAPI specification file. * You may choose a different type if you instantiate objects from sub sections of a specification. - * @param bool $resolveReferences whether to automatically resolve references in the specification. + * @param bool|string $resolveReferences whether to automatically resolve references in the specification. * If `true`, all [[Reference]] objects will be replaced with their referenced spec objects by calling * [[SpecObjectInterface::resolveReferences()]]. + * Since version 1.5.0 this can be a string indicating the reference resolving mode: + * - TODO inline vs all * @return SpecObjectInterface|OpenApi the OpenApi object instance. * The type of the returned object depends on the `$baseType` argument. * @throws TypeErrorException in case invalid spec data is supplied. @@ -75,8 +77,12 @@ public static function readFromJsonFile(string $fileName, string $baseType = Ope throw $e; } $spec = static::readFromJson($fileContent, $baseType); - $spec->setReferenceContext(new ReferenceContext($spec, $fileName)); - if ($resolveReferences) { + $context = new ReferenceContext($spec, $fileName); + $spec->setReferenceContext($context); + if ($resolveReferences !== false) { + if (is_string($resolveReferences)) { + $context->mode = $resolveReferences; + } $spec->resolveReferences(); } return $spec; @@ -90,9 +96,11 @@ public static function readFromJsonFile(string $fileName, string $baseType = Ope * @param string $baseType the base Type to instantiate. This must be an instance of [[SpecObjectInterface]]. * The default is [[OpenApi]] which is the base type of a OpenAPI specification file. * You may choose a different type if you instantiate objects from sub sections of a specification. - * @param bool $resolveReferences whether to automatically resolve references in the specification. + * @param bool|string $resolveReferences whether to automatically resolve references in the specification. * If `true`, all [[Reference]] objects will be replaced with their referenced spec objects by calling * [[SpecObjectInterface::resolveReferences()]]. + * Since version 1.5.0 this can be a string indicating the reference resolving mode: + * - TODO inline vs all * @return SpecObjectInterface|OpenApi the OpenApi object instance. * The type of the returned object depends on the `$baseType` argument. * @throws TypeErrorException in case invalid spec data is supplied. @@ -108,8 +116,12 @@ public static function readFromYamlFile(string $fileName, string $baseType = Ope throw $e; } $spec = static::readFromYaml($fileContent, $baseType); - $spec->setReferenceContext(new ReferenceContext($spec, $fileName)); - if ($resolveReferences) { + $context = new ReferenceContext($spec, $fileName); + $spec->setReferenceContext($context); + if ($resolveReferences !== false) { + if (is_string($resolveReferences)) { + $context->mode = $resolveReferences; + } $spec->resolveReferences(); } return $spec; diff --git a/src/ReferenceContext.php b/src/ReferenceContext.php index 0dc99ca9..19773eed 100644 --- a/src/ReferenceContext.php +++ b/src/ReferenceContext.php @@ -18,11 +18,26 @@ */ class ReferenceContext { + /** + * only resolve external references. + * The result will be a single API description file with references + * inside of the file structure. + */ + const RESOLVE_MODE_INLINE = 'inline'; + /** + * resolve all references, except recursive ones. + */ + const RESOLVE_MODE_ALL = 'all'; + /** * @var bool whether to throw UnresolvableReferenceException in case a reference can not * be resolved. If `false` errors are added to the Reference Objects error list instead. */ public $throwException = true; + /** + * @var string + */ + public $mode = self::RESOLVE_MODE_ALL; /** * @var SpecObjectInterface */ @@ -49,6 +64,9 @@ public function __construct(?SpecObjectInterface $base, string $uri, $cache = nu $this->_baseSpec = $base; $this->_uri = $this->normalizeUri($uri); $this->_cache = $cache ?? new ReferenceContextCache(); + if ($cache === null) { + $this->_cache->set($uri, null, $base); + } } public function getCache(): ReferenceContextCache @@ -61,6 +79,7 @@ public function getCache(): ReferenceContextCache */ private function normalizeUri($uri) { + // TODO remove dots if (strpos($uri, '://') !== false) { return $uri; } @@ -101,41 +120,55 @@ public function resolveRelativeUri(string $uri): string if (strncmp($baseUri, 'file://', 7) === 0) { if (isset($parts['path'][0]) && $parts['path'][0] === '/') { // absolute path - return 'file://' . $parts['path']; - } - // convert absolute path on windows to a file:// URI. This is probably incomplete but should work with the majority of paths. - if (stripos(PHP_OS, 'WIN') === 0 && strncmp(substr($uri, 1), ':\\', 2) === 0) { - return "file:///" . strtr($uri, [' ' => '%20', '\\' => '/']); - } - - if (isset($parts['path'])) { + $absoluteUri = 'file://' . $this->reduceDots($parts['path']); + } elseif (stripos(PHP_OS, 'WIN') === 0 && strncmp(substr($uri, 1), ':\\', 2) === 0) { + // convert absolute path on windows to a file:// URI. This is probably incomplete but should work with the majority of paths. + $absoluteUri = "file:///" . strtr($uri, [' ' => '%20', '\\' => '/']); + } elseif (isset($parts['path'])) { // relative path - return $this->dirname($baseUri) . '/' . $parts['path']; + $absoluteUri = $this->reduceDots($this->dirname($baseUri) . '/' . $parts['path']); + } else { + throw new UnresolvableReferenceException("Invalid URI: '$uri'"); + } + } else { + $baseParts = parse_url($baseUri); + $absoluteUri = implode('', [ + $baseParts['scheme'], + '://', + isset($baseParts['username']) ? $baseParts['username'] . ( + isset($baseParts['password']) ? ':' . $baseParts['password'] : '' + ) . '@' : '', + $baseParts['host'] ?? '', + isset($baseParts['port']) ? ':' . $baseParts['port'] : '', + ]); + if (isset($parts['path'][0]) && $parts['path'][0] === '/') { + $absoluteUri .= $this->reduceDots($parts['path']); + } elseif (isset($parts['path'])) { + $absoluteUri .= $this->reduceDots(rtrim($this->dirname($baseParts['path'] ?? ''), '/') . '/' . $parts['path']); } - - throw new UnresolvableReferenceException("Invalid URI: '$uri'"); - } - - $baseParts = parse_url($baseUri); - $absoluteUri = implode('', [ - $baseParts['scheme'], - '://', - isset($baseParts['username']) ? $baseParts['username'] . ( - isset($baseParts['password']) ? ':' . $baseParts['password'] : '' - ) . '@' : '', - $baseParts['host'] ?? '', - isset($baseParts['port']) ? ':' . $baseParts['port'] : '', - ]); - if (isset($parts['path'][0]) && $parts['path'][0] === '/') { - $absoluteUri .= $parts['path']; - } elseif (isset($parts['path'])) { - $absoluteUri .= rtrim($this->dirname($baseParts['path'] ?? ''), '/') . '/' . $parts['path']; } return $absoluteUri . (isset($parts['query']) ? '?' . $parts['query'] : '') . (isset($parts['fragment']) ? '#' . $parts['fragment'] : ''); } + private function reduceDots($uri) + { + $parts = explode('/', $uri); + $c = count($parts); + for($i = 0; $i < $c; $i++) { + if ($parts[$i] === '.') { + unset($parts[$i]); + continue; + } + if ($i > 0 && $parts[$i] === '..' && $parts[$i-1] !== '..') { + unset($parts[$i-1]); + unset($parts[$i]); + } + } + return implode('/', $parts); + } + /** * Returns parent directory's path. * This method is similar to `dirname()` except that it will treat @@ -206,7 +239,9 @@ public function resolveReferenceData($uri, JsonPointer $pointer, $data, $toType) // transitive reference if (isset($referencedData['$ref'])) { - return (new Reference($referencedData, $toType))->resolve(new ReferenceContext(null, $uri, $this->_cache)); + $subContext = new ReferenceContext(null, $uri, $this->_cache); + $subContext->mode = $this->mode; + return (new Reference($referencedData, $toType))->resolve($subContext); } /** @var SpecObjectInterface|array $referencedObject */ $referencedObject = $toType !== null ? new $toType($referencedData) : $referencedData; diff --git a/src/ReferenceContextCache.php b/src/ReferenceContextCache.php index 74ed3028..37ec4e35 100644 --- a/src/ReferenceContextCache.php +++ b/src/ReferenceContextCache.php @@ -13,9 +13,10 @@ class ReferenceContextCache { private $_cache = []; +// private $_isbase = []; - public function set($ref, $type, $data) + public function set($ref, $type, $data)//, $isbase = false) { $this->_cache[$ref][$type ?? ''] = $data; @@ -23,8 +24,17 @@ public function set($ref, $type, $data) if ($type !== null && !isset($this->_cache[$ref][''])) { $this->_cache[$ref][''] = $data; } + +// if ($isbase) { +// $this->_isbase[$ref] = true; +// } } +// public function isBase($ref) +// { +// return $this->_isbase[$ref] ?? false; +// } + public function get($ref, $type) { return $this->_cache[$ref][$type ?? ''] ?? null; diff --git a/src/spec/Reference.php b/src/spec/Reference.php index bd8262b2..b44b8fed 100644 --- a/src/spec/Reference.php +++ b/src/spec/Reference.php @@ -58,6 +58,7 @@ public function __construct(array $data, string $to = null) ); } if (!is_string($data['$ref'])) { + print_r($data); throw new TypeErrorException( 'Unable to instantiate Reference Object, value of $ref must be a string.' ); @@ -168,6 +169,11 @@ public function resolve(ReferenceContext $context = null) } try { if ($jsonReference->getDocumentUri() === '') { + + if ($context->mode === ReferenceContext::RESOLVE_MODE_INLINE) { + return $this; + } + // resolve in current document $baseSpec = $context->getBaseSpec(); if ($baseSpec !== null) { @@ -205,6 +211,7 @@ public function resolve(ReferenceContext $context = null) // resolve in external document $file = $context->resolveRelativeUri($jsonReference->getDocumentUri()); +// echo $file . "\n"; try { $referencedDocument = $context->fetchReferencedFile($file); } catch (\Throwable $e) { @@ -216,10 +223,17 @@ public function resolve(ReferenceContext $context = null) $exception->context = $this->getDocumentPosition(); throw $exception; } + echo "\n\n" . $context->getUri() . "\n" . $file . "\n"; + echo json_encode($referencedDocument) . "\n"; + echo $context->mode; + $referencedDocument = $this->adjustRelativeReferences($referencedDocument, $file, null, $context); +// echo json_encode($referencedDocument, JSON_PRETTY_PRINT); $referencedObject = $context->resolveReferenceData($file, $jsonReference->getJsonPointer(), $referencedDocument, $this->_to); - if ($jsonReference->getJsonPointer()->getPointer() === '') { + if ($context->getUri() === $file) { + $newContext = $context; + } elseif ($jsonReference->getJsonPointer()->getPointer() === '') { $newContext = new ReferenceContext($referencedObject instanceof SpecObjectInterface ? $referencedObject : null, $file, $context->getCache()); if ($referencedObject instanceof DocumentContextInterface) { $referencedObject->setDocumentContext($referencedObject, $jsonReference->getJsonPointer()); @@ -231,6 +245,7 @@ public function resolve(ReferenceContext $context = null) $newContext = new ReferenceContext(null, $file, $context->getCache()); } $newContext->throwException = $context->throwException; + $newContext->mode = $context->mode; if ($referencedObject instanceof SpecObjectInterface) { $referencedObject->setReferenceContext($newContext); } @@ -257,6 +272,43 @@ public function resolve(ReferenceContext $context = null) } } + // adjust relative refernces inside of the file to match the context of the base file + private function adjustRelativeReferences($referencedDocument, $basePath, $baseDocument = null, $oContext = null) + { + $context = new ReferenceContext(null, $basePath); + if ($baseDocument === null) { + $baseDocument = $referencedDocument; + } + + foreach($referencedDocument as $key => $value) { + if ($key === '$ref' && is_string($value)) { + if (isset($value[0]) && $value[0] === '#') { + echo "resolbin....\n"; +// if ($oContext === null || $oContext->mode !== ReferenceContext::RESOLVE_MODE_INLINE) { + // direcly inline references in the same document, + // these are not going to be valid in the new context anymore + $referencedDocument = (new JsonPointer(substr($value, 1)))->evaluate($baseDocument); + break; +// } + } +// echo "\n\n$value\n"; + $referencedDocument[$key] = $context->resolveRelativeUri($value); + $parts = explode('#', $referencedDocument[$key], 2); + if ($parts[0] === $oContext->getUri()) { + $referencedDocument[$key] = '#' . ($parts[1] ?? ''); + } + echo $referencedDocument[$key]; +// echo "$referencedDocument[$key]\n"; + + continue; + } + if (is_array($value)) { + $referencedDocument[$key] = $this->adjustRelativeReferences($value, $basePath, $baseDocument, $oContext); + } + } + return $referencedDocument; + } + /** * Resolves all Reference Objects in this object and replaces them with their resolution. * @throws UnresolvableReferenceException diff --git a/tests/ReferenceContextTest.php b/tests/ReferenceContextTest.php index 4934da95..73014fc8 100644 --- a/tests/ReferenceContextTest.php +++ b/tests/ReferenceContextTest.php @@ -32,7 +32,7 @@ public function uriProvider() [ 'https://example.com/api/openapi.yaml', // base URI '../definitions.yaml', // referenced URI - 'https://example.com/api/../definitions.yaml', // expected result + 'https://example.com/definitions.yaml', // expected result ], [ '/var/www/openapi.yaml', // base URI diff --git a/tests/spec/PathTest.php b/tests/spec/PathTest.php index aa065a64..8368f784 100644 --- a/tests/spec/PathTest.php +++ b/tests/spec/PathTest.php @@ -162,7 +162,8 @@ public function testPathItemReference() $this->assertEquals('getBar', $ReferencedBarPath->get->operationId); $this->assertInstanceOf(Reference::class, $ReferencedBarPath->get->responses['200']); - $this->assertInstanceOf(Reference::class, $ReferencedBarPath->get->responses['404']); + $this->assertInstanceOf(Response::class, $ReferencedBarPath->get->responses['404']); + $this->assertEquals('non-existing resource', $barPath->get->responses['404']->description); /** @var $openapi OpenApi */ $openapi = Reader::readFromYamlFile($file, \cebe\openapi\spec\OpenApi::class, true); diff --git a/tests/spec/ReferenceTest.php b/tests/spec/ReferenceTest.php index 0aee0f31..56150d57 100644 --- a/tests/spec/ReferenceTest.php +++ b/tests/spec/ReferenceTest.php @@ -419,7 +419,7 @@ public function testTransitiveReferenceCyclic() public function testResolveRelativePath() { - $openapi = Reader::readFromYamlFile(__DIR__ . '/data/reference/openapi_models.yaml'); + $openapi = Reader::readFromYamlFile(__DIR__ . '/data/reference/openapi_models.yaml', OpenApi::class, \cebe\openapi\ReferenceContext::RESOLVE_MODE_INLINE); $yaml = \cebe\openapi\Writer::writeToYaml($openapi); @@ -444,7 +444,7 @@ public function testResolveRelativePath() type: integer format: int64 cat: - \$ref: '#/components/schemas/Cat' + \$ref: '#/components/schemas/Cat' description: 'A Pet' Cat: type: object @@ -455,9 +455,11 @@ public function testResolveRelativePath() name: type: string description: 'the cats name' + pet: + \$ref: '#/components/schemas/Pet' description: 'A Cat' YAML - , $yaml); + , $yaml, $yaml); } } From 23b9f33f11c7f41648038a1fd2d306ec9ef7245f Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 10 Jul 2020 21:22:56 +0200 Subject: [PATCH 4/8] refactor reference handling --- Makefile | 18 ++- bin/php-openapi | 9 +- composer.json | 2 +- phpunit.xml.dist | 3 + src/Reader.php | 3 + src/ReferenceContext.php | 148 ++++++++++-------- src/ReferenceContextCache.php | 12 +- src/spec/PathItem.php | 6 + src/spec/Reference.php | 87 +++++----- tests/ReferenceContextTest.php | 130 ++++++++++++++- tests/spec/PathTest.php | 8 +- tests/spec/ReferenceTest.php | 96 +++++++++++- tests/spec/data/reference/structure.yaml | 9 ++ tests/spec/data/reference/structure/paths.yml | 5 + .../data/reference/structure/paths/cat.yml | 4 + .../data/reference/structure/paths/pet.yml | 4 + 16 files changed, 401 insertions(+), 143 deletions(-) create mode 100644 tests/spec/data/reference/structure.yaml create mode 100644 tests/spec/data/reference/structure/paths.yml create mode 100644 tests/spec/data/reference/structure/paths/cat.yml create mode 100644 tests/spec/data/reference/structure/paths/pet.yml diff --git a/Makefile b/Makefile index 04daecd4..627ad781 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,10 @@ TESTCASE= +XDEBUG=0 PHPARGS=-dmemory_limit=64M -#PHPARGS=-dmemory_limit=64M -dzend_extension=xdebug.so -dxdebug.remote_enable=1 +XPHPARGS= +ifeq ($(XDEBUG),1) +XPHPARGS=-dzend_extension=xdebug.so -dxdebug.remote_enable=1 -dxdebug.remote_autostart=1 +endif all: @@ -17,14 +21,14 @@ install: yarn install test: - php $(PHPARGS) vendor/bin/phpunit --verbose $(TESTCASE) - php $(PHPARGS) bin/php-openapi validate tests/spec/data/recursion.json - php $(PHPARGS) bin/php-openapi validate tests/spec/data/recursion2.yaml + php $(PHPARGS) $(XPHPARGS) vendor/bin/phpunit --verbose $(TESTCASE) + php $(PHPARGS) $(XPHPARGS) bin/php-openapi validate tests/spec/data/recursion.json + php $(PHPARGS) $(XPHPARGS) bin/php-openapi validate tests/spec/data/recursion2.yaml lint: - php $(PHPARGS) bin/php-openapi validate tests/spec/data/reference/playlist.json - php $(PHPARGS) bin/php-openapi validate tests/spec/data/recursion.json - php $(PHPARGS) bin/php-openapi validate tests/spec/data/recursion2.yaml + php $(PHPARGS) $(XPHPARGS) bin/php-openapi validate tests/spec/data/reference/playlist.json + php $(PHPARGS) $(XPHPARGS) bin/php-openapi validate tests/spec/data/recursion.json + php $(PHPARGS) $(XPHPARGS) bin/php-openapi validate tests/spec/data/recursion2.yaml node_modules/.bin/speccy lint tests/spec/data/reference/playlist.json node_modules/.bin/speccy lint tests/spec/data/recursion.json diff --git a/bin/php-openapi b/bin/php-openapi index 98c31a8c..00730ef6 100755 --- a/bin/php-openapi +++ b/bin/php-openapi @@ -109,7 +109,9 @@ switch ($command) { $openApi = read_input($inputFile, $inputFormat); $referenceContext = new ReferenceContext($openApi, $inputFile ? realpath($inputFile) : ''); $referenceContext->throwException = false; - $referenceContext->mode = ReferenceContext:: + // TODO apply reference context mode +// $referenceContext->mode = ReferenceContext::RESOLVE_MODE_ALL; +// $referenceContext->mode = ReferenceContext::RESOLVE_MODE_INLINE; $openApi->resolveReferences($referenceContext); $openApi->setDocumentContext($openApi, new \cebe\openapi\json\JsonPointer('')); @@ -187,6 +189,11 @@ switch ($command) { $openApi = read_input($inputFile, $inputFormat); try { + // TODO apply reference context mode +// $referenceContext->mode = ReferenceContext::RESOLVE_MODE_ALL; +// $referenceContext->mode = ReferenceContext::RESOLVE_MODE_INLINE; + // set document context for correctly converting recursive references + $openApi->setDocumentContext($openApi, new \cebe\openapi\json\JsonPointer('')); $openApi->resolveReferences(); } catch (\cebe\openapi\exceptions\UnresolvableReferenceException $e) { error("[\e[33m{$e->context}\e[0m] " . $e->getMessage()); diff --git a/composer.json b/composer.json index d1c98fb2..174bd565 100644 --- a/composer.json +++ b/composer.json @@ -41,7 +41,7 @@ }, "extra": { "branch-alias": { - "dev-master": "1.4.x-dev" + "dev-master": "1.5.x-dev" } }, "bin": [ diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 667ac5f3..de50cde0 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -11,6 +11,9 @@ + + ./src + ./vendor ./tests diff --git a/src/Reader.php b/src/Reader.php index 143a7b7e..b52ebe13 100644 --- a/src/Reader.php +++ b/src/Reader.php @@ -10,6 +10,7 @@ use cebe\openapi\exceptions\IOException; use cebe\openapi\exceptions\TypeErrorException; use cebe\openapi\exceptions\UnresolvableReferenceException; +use cebe\openapi\json\JsonPointer; use cebe\openapi\spec\OpenApi; use Symfony\Component\Yaml\Yaml; @@ -83,6 +84,7 @@ public static function readFromJsonFile(string $fileName, string $baseType = Ope if (is_string($resolveReferences)) { $context->mode = $resolveReferences; } + $spec->setDocumentContext($spec, new JsonPointer('')); $spec->resolveReferences(); } return $spec; @@ -122,6 +124,7 @@ public static function readFromYamlFile(string $fileName, string $baseType = Ope if (is_string($resolveReferences)) { $context->mode = $resolveReferences; } + $spec->setDocumentContext($spec, new JsonPointer('')); $spec->resolveReferences(); } return $spec; diff --git a/src/ReferenceContext.php b/src/ReferenceContext.php index 19773eed..e78d3fdc 100644 --- a/src/ReferenceContext.php +++ b/src/ReferenceContext.php @@ -64,8 +64,8 @@ public function __construct(?SpecObjectInterface $base, string $uri, $cache = nu $this->_baseSpec = $base; $this->_uri = $this->normalizeUri($uri); $this->_cache = $cache ?? new ReferenceContextCache(); - if ($cache === null) { - $this->_cache->set($uri, null, $base); + if ($cache === null && $base !== null) { + $this->_cache->set($this->_uri, null, $base); } } @@ -79,82 +79,41 @@ public function getCache(): ReferenceContextCache */ private function normalizeUri($uri) { - // TODO remove dots if (strpos($uri, '://') !== false) { - return $uri; + $parts = parse_url($uri); + if (isset($parts['path'])) { + $parts['path'] = $this->reduceDots($parts['path']); + } + return $this->buildUri($parts); } if (strncmp($uri, '/', 1) === 0) { + $uri = $this->reduceDots($uri); return "file://$uri"; } if (stripos(PHP_OS, 'WIN') === 0 && strncmp(substr($uri, 1), ':\\', 2) === 0) { + $uri = $this->reduceDots($uri); return "file:///" . strtr($uri, [' ' => '%20', '\\' => '/']); } throw new UnresolvableReferenceException('Can not resolve references for a specification given as a relative path.'); } - public function getBaseSpec(): ?SpecObjectInterface - { - return $this->_baseSpec; - } - - public function getUri(): string - { - return $this->_uri; - } - - /** - * Resolve a relative URI to an absolute URI in the current context. - * @param string $uri - * @throws UnresolvableReferenceException - * @return string - */ - public function resolveRelativeUri(string $uri): string + private function buildUri($parts) { - $parts = parse_url($uri); - if (isset($parts['scheme'])) { - // absolute URL - return $uri; - } - - $baseUri = $this->getUri(); - if (strncmp($baseUri, 'file://', 7) === 0) { - if (isset($parts['path'][0]) && $parts['path'][0] === '/') { - // absolute path - $absoluteUri = 'file://' . $this->reduceDots($parts['path']); - } elseif (stripos(PHP_OS, 'WIN') === 0 && strncmp(substr($uri, 1), ':\\', 2) === 0) { - // convert absolute path on windows to a file:// URI. This is probably incomplete but should work with the majority of paths. - $absoluteUri = "file:///" . strtr($uri, [' ' => '%20', '\\' => '/']); - } elseif (isset($parts['path'])) { - // relative path - $absoluteUri = $this->reduceDots($this->dirname($baseUri) . '/' . $parts['path']); - } else { - throw new UnresolvableReferenceException("Invalid URI: '$uri'"); - } - } else { - $baseParts = parse_url($baseUri); - $absoluteUri = implode('', [ - $baseParts['scheme'], - '://', - isset($baseParts['username']) ? $baseParts['username'] . ( - isset($baseParts['password']) ? ':' . $baseParts['password'] : '' - ) . '@' : '', - $baseParts['host'] ?? '', - isset($baseParts['port']) ? ':' . $baseParts['port'] : '', - ]); - if (isset($parts['path'][0]) && $parts['path'][0] === '/') { - $absoluteUri .= $this->reduceDots($parts['path']); - } elseif (isset($parts['path'])) { - $absoluteUri .= $this->reduceDots(rtrim($this->dirname($baseParts['path'] ?? ''), '/') . '/' . $parts['path']); - } - } - return $absoluteUri - . (isset($parts['query']) ? '?' . $parts['query'] : '') - . (isset($parts['fragment']) ? '#' . $parts['fragment'] : ''); + $scheme = !empty($parts['scheme']) ? $parts['scheme'] . '://' : ''; + $host = $parts['host'] ?? ''; + $port = !empty($parts['port']) ? ':' . $parts['port'] : ''; + $user = $parts['user'] ?? ''; + $pass = !empty($parts['pass']) ? ':' . $parts['pass'] : ''; + $pass = ($user || $pass) ? "$pass@" : ''; + $path = $parts['path'] ?? ''; + $query = !empty($parts['query']) ? '?' . $parts['query'] : ''; + $fragment = !empty($parts['fragment']) ? '#' . $parts['fragment'] : ''; + return "$scheme$user$pass$host$port$path$query$fragment"; } - private function reduceDots($uri) + private function reduceDots($path) { - $parts = explode('/', $uri); + $parts = explode('/', ltrim($path, '/')); $c = count($parts); for($i = 0; $i < $c; $i++) { if ($parts[$i] === '.') { @@ -166,7 +125,7 @@ private function reduceDots($uri) unset($parts[$i]); } } - return implode('/', $parts); + return '/'.implode('/', $parts); } /** @@ -188,6 +147,57 @@ private function dirname($path) return ''; } + public function getBaseSpec(): ?SpecObjectInterface + { + return $this->_baseSpec; + } + + public function getUri(): string + { + return $this->_uri; + } + + /** + * Resolve a relative URI to an absolute URI in the current context. + * @param string $uri + * @throws UnresolvableReferenceException + * @return string + */ + public function resolveRelativeUri(string $uri): string + { + $parts = parse_url($uri); + // absolute URI, no need to combine with baseURI + if (isset($parts['scheme'])) { + if (isset($parts['path'])) { + $parts['path'] = $this->reduceDots($parts['path']); + } + return $this->buildUri($parts); + } + + // convert absolute path on windows to a file:// URI. This is probably incomplete but should work with the majority of paths. + if (stripos(PHP_OS, 'WIN') === 0 && strncmp(substr($uri, 1), ':\\', 2) === 0) { + // convert absolute path on windows to a file:// URI. This is probably incomplete but should work with the majority of paths. + $absoluteUri = "file:///" . strtr($uri, [' ' => '%20', '\\' => '/']); + return $absoluteUri + . (isset($parts['fragment']) ? '#' . $parts['fragment'] : ''); + } + + $baseUri = $this->getUri(); + $baseParts = parse_url($baseUri); + if (isset($parts['path'][0]) && $parts['path'][0] === '/') { + // absolute path + $baseParts['path'] = $this->reduceDots($parts['path']); + } elseif (isset($parts['path'])) { + // relative path + $baseParts['path'] = $this->reduceDots(rtrim($this->dirname($baseParts['path'] ?? ''), '/') . '/' . $parts['path']); + } else { + throw new UnresolvableReferenceException("Invalid URI: '$uri'"); + } + $baseParts['query'] = $parts['query'] ?? null; + $baseParts['fragment'] = $parts['fragment'] ?? null; + return $this->buildUri($baseParts); + } + /** * Fetch referenced file by URI. * @@ -239,12 +249,12 @@ public function resolveReferenceData($uri, JsonPointer $pointer, $data, $toType) // transitive reference if (isset($referencedData['$ref'])) { - $subContext = new ReferenceContext(null, $uri, $this->_cache); - $subContext->mode = $this->mode; - return (new Reference($referencedData, $toType))->resolve($subContext); + /** @var Reference $referencedObject */ + return new Reference($referencedData, $toType); + } else { + /** @var SpecObjectInterface|array $referencedObject */ + $referencedObject = $toType !== null ? new $toType($referencedData) : $referencedData; } - /** @var SpecObjectInterface|array $referencedObject */ - $referencedObject = $toType !== null ? new $toType($referencedData) : $referencedData; $this->_cache->set($ref, $toType, $referencedObject); diff --git a/src/ReferenceContextCache.php b/src/ReferenceContextCache.php index 37ec4e35..74ed3028 100644 --- a/src/ReferenceContextCache.php +++ b/src/ReferenceContextCache.php @@ -13,10 +13,9 @@ class ReferenceContextCache { private $_cache = []; -// private $_isbase = []; - public function set($ref, $type, $data)//, $isbase = false) + public function set($ref, $type, $data) { $this->_cache[$ref][$type ?? ''] = $data; @@ -24,17 +23,8 @@ public function set($ref, $type, $data)//, $isbase = false) if ($type !== null && !isset($this->_cache[$ref][''])) { $this->_cache[$ref][''] = $data; } - -// if ($isbase) { -// $this->_isbase[$ref] = true; -// } } -// public function isBase($ref) -// { -// return $this->_isbase[$ref] ?? false; -// } - public function get($ref, $type) { return $this->_cache[$ref][$type ?? ''] ?? null; diff --git a/src/spec/PathItem.php b/src/spec/PathItem.php index 236bda81..f93e43fc 100644 --- a/src/spec/PathItem.php +++ b/src/spec/PathItem.php @@ -91,6 +91,12 @@ public function getSerializableData() if ($this->_ref instanceof Reference) { $data->{'$ref'} = $this->_ref->getReference(); } + if (isset($data->servers) && empty($data->servers)) { + unset($data->servers); + } + if (isset($data->parameters) && empty($data->parameters)) { + unset($data->parameters); + } return $data; } diff --git a/src/spec/Reference.php b/src/spec/Reference.php index b44b8fed..5f65a7e1 100644 --- a/src/spec/Reference.php +++ b/src/spec/Reference.php @@ -182,21 +182,7 @@ public function resolve(ReferenceContext $context = null) $referencedObject = $jsonReference->getJsonPointer()->evaluate($baseSpec); // transitive reference if ($referencedObject instanceof Reference) { - if ($referencedObject->_to === null) { - $referencedObject->_to = $this->_to; - } - $referencedObject->setContext($context); - - if ($referencedObject === $this) { // catch recursion - throw new UnresolvableReferenceException('Cyclic reference detected on a Reference Object.'); - } - - $transitiveRefResult = $referencedObject->resolve(); - - if ($transitiveRefResult === $this) { // catch recursion - throw new UnresolvableReferenceException('Cyclic reference detected on a Reference Object.'); - } - return $transitiveRefResult; + $referencedObject = $this->resolveTransitiveReference($referencedObject, $context); } if ($referencedObject instanceof SpecObjectInterface) { $referencedObject->setReferenceContext($context); @@ -211,8 +197,8 @@ public function resolve(ReferenceContext $context = null) // resolve in external document $file = $context->resolveRelativeUri($jsonReference->getDocumentUri()); -// echo $file . "\n"; try { + // TODO cache here? $referencedDocument = $context->fetchReferencedFile($file); } catch (\Throwable $e) { $exception = new UnresolvableReferenceException( @@ -223,31 +209,27 @@ public function resolve(ReferenceContext $context = null) $exception->context = $this->getDocumentPosition(); throw $exception; } - echo "\n\n" . $context->getUri() . "\n" . $file . "\n"; - echo json_encode($referencedDocument) . "\n"; - echo $context->mode; - $referencedDocument = $this->adjustRelativeReferences($referencedDocument, $file, null, $context); -// echo json_encode($referencedDocument, JSON_PRETTY_PRINT); + $referencedDocument = $this->adjustRelativeReferences($referencedDocument, $file, null, $context); $referencedObject = $context->resolveReferenceData($file, $jsonReference->getJsonPointer(), $referencedDocument, $this->_to); - if ($context->getUri() === $file) { - $newContext = $context; - } elseif ($jsonReference->getJsonPointer()->getPointer() === '') { - $newContext = new ReferenceContext($referencedObject instanceof SpecObjectInterface ? $referencedObject : null, $file, $context->getCache()); - if ($referencedObject instanceof DocumentContextInterface) { - $referencedObject->setDocumentContext($referencedObject, $jsonReference->getJsonPointer()); + if ($referencedObject instanceof DocumentContextInterface) { + if ($referencedObject->getDocumentPosition() === null && $this->getDocumentPosition() !== null) { + $referencedObject->setDocumentContext($context->getBaseSpec(), $this->getDocumentPosition()); } - } 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. - $newContext = new ReferenceContext(null, $file, $context->getCache()); } - $newContext->throwException = $context->throwException; - $newContext->mode = $context->mode; - if ($referencedObject instanceof SpecObjectInterface) { - $referencedObject->setReferenceContext($newContext); + + // transitive reference + if ($referencedObject instanceof Reference) { + if ($context->mode === ReferenceContext::RESOLVE_MODE_INLINE && strncmp($referencedObject->getReference(), '#', 1) === 0) { + $referencedObject->setContext($context); + } else { + return $this->resolveTransitiveReference($referencedObject, $context); + } + } else { + if ($referencedObject instanceof SpecObjectInterface) { + $referencedObject->setReferenceContext($context); + } } return $referencedObject; @@ -272,6 +254,25 @@ public function resolve(ReferenceContext $context = null) } } + private function resolveTransitiveReference(Reference $referencedObject, ReferenceContext $context) + { + if ($referencedObject->_to === null) { + $referencedObject->_to = $this->_to; + } + $referencedObject->setContext($context); + + if ($referencedObject === $this) { // catch recursion + throw new UnresolvableReferenceException('Cyclic reference detected on a Reference Object.'); + } + + $transitiveRefResult = $referencedObject->resolve(); + + if ($transitiveRefResult === $this) { // catch recursion + throw new UnresolvableReferenceException('Cyclic reference detected on a Reference Object.'); + } + return $transitiveRefResult; + } + // adjust relative refernces inside of the file to match the context of the base file private function adjustRelativeReferences($referencedDocument, $basePath, $baseDocument = null, $oContext = null) { @@ -283,23 +284,15 @@ private function adjustRelativeReferences($referencedDocument, $basePath, $baseD foreach($referencedDocument as $key => $value) { if ($key === '$ref' && is_string($value)) { if (isset($value[0]) && $value[0] === '#') { - echo "resolbin....\n"; -// if ($oContext === null || $oContext->mode !== ReferenceContext::RESOLVE_MODE_INLINE) { - // direcly inline references in the same document, - // these are not going to be valid in the new context anymore - $referencedDocument = (new JsonPointer(substr($value, 1)))->evaluate($baseDocument); - break; -// } + // direcly inline references in the same document, + // these are not going to be valid in the new context anymore + return (new JsonPointer(substr($value, 1)))->evaluate($baseDocument); } -// echo "\n\n$value\n"; $referencedDocument[$key] = $context->resolveRelativeUri($value); $parts = explode('#', $referencedDocument[$key], 2); if ($parts[0] === $oContext->getUri()) { $referencedDocument[$key] = '#' . ($parts[1] ?? ''); } - echo $referencedDocument[$key]; -// echo "$referencedDocument[$key]\n"; - continue; } if (is_array($value)) { diff --git a/tests/ReferenceContextTest.php b/tests/ReferenceContextTest.php index 73014fc8..92d9d93e 100644 --- a/tests/ReferenceContextTest.php +++ b/tests/ReferenceContextTest.php @@ -6,7 +6,7 @@ class ReferenceContextTest extends \PHPUnit\Framework\TestCase { - public function uriProvider() + public function resolveUriProvider() { $data = [ [ @@ -14,37 +14,77 @@ public function uriProvider() 'definitions.yaml', // referenced URI 'https://example.com/definitions.yaml', // expected result ], + [ + 'https://example.com/openapi.yaml', // base URI + 'definitions.yaml#/components/Pet', // referenced URI + 'https://example.com/definitions.yaml#/components/Pet', // expected result + ], + [ 'https://example.com/openapi.yaml', // base URI '/definitions.yaml', // referenced URI 'https://example.com/definitions.yaml', // expected result ], + [ + 'https://example.com/openapi.yaml', // base URI + '/definitions.yaml#/components/Pet', // referenced URI + 'https://example.com/definitions.yaml#/components/Pet', // expected result + ], + [ 'https://example.com/api/openapi.yaml', // base URI 'definitions.yaml', // referenced URI 'https://example.com/api/definitions.yaml', // expected result ], + [ + 'https://example.com/api/openapi.yaml', // base URI + 'definitions.yaml#/components/Pet', // referenced URI + 'https://example.com/api/definitions.yaml#/components/Pet', // expected result + ], + [ 'https://example.com/api/openapi.yaml', // base URI '/definitions.yaml', // referenced URI 'https://example.com/definitions.yaml', // expected result ], + [ + 'https://example.com/api/openapi.yaml', // base URI + '/definitions.yaml#/components/Pet', // referenced URI + 'https://example.com/definitions.yaml#/components/Pet', // expected result + ], + [ 'https://example.com/api/openapi.yaml', // base URI '../definitions.yaml', // referenced URI 'https://example.com/definitions.yaml', // expected result ], + [ + 'https://example.com/api/openapi.yaml', // base URI + '../definitions.yaml#/components/Pet', // referenced URI + 'https://example.com/definitions.yaml#/components/Pet', // expected result + ], + [ '/var/www/openapi.yaml', // base URI 'definitions.yaml', // referenced URI 'file:///var/www/definitions.yaml', // expected result ], + [ + '/var/www/openapi.yaml', // base URI + 'definitions.yaml#/components/Pet', // referenced URI + 'file:///var/www/definitions.yaml#/components/Pet', // expected result + ], + [ '/var/www/openapi.yaml', // base URI '/var/definitions.yaml', // referenced URI 'file:///var/definitions.yaml', // expected result ], - + [ + '/var/www/openapi.yaml', // base URI + '/var/definitions.yaml#/components/Pet', // referenced URI + 'file:///var/definitions.yaml#/components/Pet', // expected result + ], ]; // absolute URLs should not be changed @@ -54,18 +94,29 @@ public function uriProvider() 'file:///var/www/definitions.yaml', 'file:///var/www/definitions.yaml', ]; + $data[] = [ + $url, + 'file:///var/www/definitions.yaml#/components/Pet', + 'file:///var/www/definitions.yaml#/components/Pet', + ]; + $data[] = [ $url, 'https://example.com/definitions.yaml', 'https://example.com/definitions.yaml', ]; + $data[] = [ + $url, + 'https://example.com/definitions.yaml#/components/Pet', + 'https://example.com/definitions.yaml#/components/Pet', + ]; } return $data; } /** - * @dataProvider uriProvider + * @dataProvider resolveUriProvider */ public function testResolveUri($baseUri, $referencedUri, $expected) { @@ -73,4 +124,77 @@ public function testResolveUri($baseUri, $referencedUri, $expected) $this->assertEquals($expected, $context->resolveRelativeUri($referencedUri)); } + public function normalizeUriProvider() + { + $data = [ + [ + 'https://example.com/openapi.yaml', + 'https://example.com/openapi.yaml', + ], + [ + 'https://example.com/openapi.yaml#/components/Pet', + 'https://example.com/openapi.yaml#/components/Pet', + ], + [ + 'https://example.com/./openapi.yaml', + 'https://example.com/openapi.yaml', + ], + [ + 'https://example.com/./openapi.yaml#/components/Pet', + 'https://example.com/openapi.yaml#/components/Pet', + ], + [ + 'https://example.com/api/../openapi.yaml', + 'https://example.com/openapi.yaml', + ], + [ + 'https://example.com/api/../openapi.yaml#/components/Pet', + 'https://example.com/openapi.yaml#/components/Pet', + ], + [ + 'https://example.com/../openapi.yaml', + 'https://example.com/../openapi.yaml', + ], + [ + 'https://example.com/../openapi.yaml#/components/Pet', + 'https://example.com/../openapi.yaml#/components/Pet', + ], + [ + '/definitions.yaml', + 'file:///definitions.yaml', + ], + [ + '/definitions.yaml#/components/Pet', + 'file:///definitions.yaml#/components/Pet', + ], + [ + '/var/www/definitions.yaml', + 'file:///var/www/definitions.yaml', + ], + [ + '/var/www/definitions.yaml#/components/Pet', + 'file:///var/www/definitions.yaml#/components/Pet', + ], + [ + '/var/www/api/../definitions.yaml', + 'file:///var/www/definitions.yaml', + ], + [ + '/var/www/api/../definitions.yaml#/components/Pet', + 'file:///var/www/definitions.yaml#/components/Pet', + ], + ]; + + return $data; + } + + /** + * @dataProvider normalizeUriProvider + */ + public function testNormalizeUri($uri, $expected) + { + $context = new ReferenceContext(null, $uri); + $this->assertEquals($expected, $context->getUri()); + } + } diff --git a/tests/spec/PathTest.php b/tests/spec/PathTest.php index 8368f784..cf50b19d 100644 --- a/tests/spec/PathTest.php +++ b/tests/spec/PathTest.php @@ -161,9 +161,13 @@ public function testPathItemReference() $this->assertInstanceOf(Operation::class, $ReferencedBarPath->get); $this->assertEquals('getBar', $ReferencedBarPath->get->operationId); - $this->assertInstanceOf(Reference::class, $ReferencedBarPath->get->responses['200']); + $this->assertInstanceOf(Reference::class, $reference200 = $ReferencedBarPath->get->responses['200']); $this->assertInstanceOf(Response::class, $ReferencedBarPath->get->responses['404']); - $this->assertEquals('non-existing resource', $barPath->get->responses['404']->description); + $this->assertEquals('non-existing resource', $ReferencedBarPath->get->responses['404']->description); + + $path200 = $reference200->resolve(); + $this->assertInstanceOf(Response::class, $path200); + $this->assertEquals('A bar', $path200->description); /** @var $openapi OpenApi */ $openapi = Reader::readFromYamlFile($file, \cebe\openapi\spec\OpenApi::class, true); diff --git a/tests/spec/ReferenceTest.php b/tests/spec/ReferenceTest.php index 56150d57..29928b4c 100644 --- a/tests/spec/ReferenceTest.php +++ b/tests/spec/ReferenceTest.php @@ -417,7 +417,35 @@ public function testTransitiveReferenceCyclic() $openapi->resolveReferences(new \cebe\openapi\ReferenceContext($openapi, 'file:///tmp/openapi.yaml')); } - public function testResolveRelativePath() + public function testTransitiveReferenceOverTwoFiles() + { + $openapi = Reader::readFromYamlFile(__DIR__ . '/data/reference/structure.yaml', OpenApi::class, \cebe\openapi\ReferenceContext::RESOLVE_MODE_INLINE); + + $yaml = \cebe\openapi\Writer::writeToYaml($openapi); + + $this->assertEquals( +<<assertEquals( +<< Date: Fri, 10 Jul 2020 21:32:30 +0200 Subject: [PATCH 5/8] cleanup --- src/spec/Reference.php | 1 - tests/spec/data/reference/structure/paths.yml | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/spec/Reference.php b/src/spec/Reference.php index 5f65a7e1..c604699f 100644 --- a/src/spec/Reference.php +++ b/src/spec/Reference.php @@ -58,7 +58,6 @@ public function __construct(array $data, string $to = null) ); } if (!is_string($data['$ref'])) { - print_r($data); throw new TypeErrorException( 'Unable to instantiate Reference Object, value of $ref must be a string.' ); diff --git a/tests/spec/data/reference/structure/paths.yml b/tests/spec/data/reference/structure/paths.yml index 636af21e..46f4d6b3 100644 --- a/tests/spec/data/reference/structure/paths.yml +++ b/tests/spec/data/reference/structure/paths.yml @@ -2,4 +2,4 @@ get: $ref: 'paths/pet.yml#/get' /cat: - $ref: 'paths/cat.yml' \ No newline at end of file + $ref: 'paths/cat.yml' From 76b1077e62da6dc1c4cc0b84a7bd171b90d82c7f Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Mon, 14 Dec 2020 23:23:01 +0100 Subject: [PATCH 6/8] Cache parsed file content --- src/ReferenceContext.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ReferenceContext.php b/src/ReferenceContext.php index e78d3fdc..a5f02d25 100644 --- a/src/ReferenceContext.php +++ b/src/ReferenceContext.php @@ -208,6 +208,10 @@ public function resolveRelativeUri(string $uri): string */ public function fetchReferencedFile($uri) { + if ($this->_cache->has('FILE_CONTENT://' . $uri, 'FILE_CONTENT')) { + return $this->_cache->get('FILE_CONTENT://' . $uri, 'FILE_CONTENT'); + } + $content = file_get_contents($uri); if ($content === false) { $e = new IOException("Failed to read file: '$uri'"); @@ -216,10 +220,12 @@ public function fetchReferencedFile($uri) } // TODO lazy content detection, should be improved if (strpos(ltrim($content), '{') === 0) { - return json_decode($content, true); + $parsedContent = json_decode($content, true); } else { - return Yaml::parse($content); + $parsedContent = Yaml::parse($content); } + $this->_cache->set('FILE_CONTENT://' . $uri, 'FILE_CONTENT', $parsedContent); + return $parsedContent; } /** From 10a51356cef42c43a65f4e0f1b07968f5352017b Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Mon, 14 Dec 2020 23:36:33 +0100 Subject: [PATCH 7/8] Apply suggestions from code review --- src/spec/Reference.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/spec/Reference.php b/src/spec/Reference.php index c604699f..a1e2b859 100644 --- a/src/spec/Reference.php +++ b/src/spec/Reference.php @@ -197,7 +197,6 @@ public function resolve(ReferenceContext $context = null) // resolve in external document $file = $context->resolveRelativeUri($jsonReference->getDocumentUri()); try { - // TODO cache here? $referencedDocument = $context->fetchReferencedFile($file); } catch (\Throwable $e) { $exception = new UnresolvableReferenceException( From 333aa5f44b0baeb94502cdcfa0053a07efc55533 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Mon, 14 Dec 2020 23:41:36 +0100 Subject: [PATCH 8/8] Apply suggestions from code review --- src/Reader.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Reader.php b/src/Reader.php index b52ebe13..7dc00a92 100644 --- a/src/Reader.php +++ b/src/Reader.php @@ -62,7 +62,8 @@ public static function readFromYaml(string $yaml, string $baseType = OpenApi::cl * If `true`, all [[Reference]] objects will be replaced with their referenced spec objects by calling * [[SpecObjectInterface::resolveReferences()]]. * Since version 1.5.0 this can be a string indicating the reference resolving mode: - * - TODO inline vs all + * - `inline` only resolve references to external files. + * - `all` resolve all references exceot recursive references. * @return SpecObjectInterface|OpenApi the OpenApi object instance. * The type of the returned object depends on the `$baseType` argument. * @throws TypeErrorException in case invalid spec data is supplied. @@ -102,7 +103,8 @@ public static function readFromJsonFile(string $fileName, string $baseType = Ope * If `true`, all [[Reference]] objects will be replaced with their referenced spec objects by calling * [[SpecObjectInterface::resolveReferences()]]. * Since version 1.5.0 this can be a string indicating the reference resolving mode: - * - TODO inline vs all + * - `inline` only resolve references to external files. + * - `all` resolve all references exceot recursive references. * @return SpecObjectInterface|OpenApi the OpenApi object instance. * The type of the returned object depends on the `$baseType` argument. * @throws TypeErrorException in case invalid spec data is supplied.