Skip to content

Commit

Permalink
Fix recursion issue with reading references which are included multip…
Browse files Browse the repository at this point in the history
…le times

fixes #44
  • Loading branch information
cebe committed Dec 16, 2019
1 parent 0158fec commit 497683f
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 18 deletions.
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
TESTCASE=
PHPARGS=
#PHPARGS=-dzend_extension=xdebug.so -dxdebug.remote_enable=1
PHPARGS=-dmemory_limit=64M
#PHPARGS=-dmemory_limit=64M -dzend_extension=xdebug.so -dxdebug.remote_enable=1

all:

Expand All @@ -18,10 +18,13 @@ install:

test:
php $(PHPARGS) vendor/bin/phpunit $(TESTCASE)
php $(PHPARGS) bin/php-openapi validate tests/spec/data/recursion.json
php $(PHPARGS) 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
node_modules/.bin/speccy lint tests/spec/data/reference/playlist.json
node_modules/.bin/speccy lint tests/spec/data/recursion.json

Expand Down
46 changes: 30 additions & 16 deletions src/SpecBaseObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ abstract class SpecBaseObject implements SpecObjectInterface, DocumentContextInt
{
private $_properties = [];
private $_errors = [];
private $_recursing = false;

private $_recursingSerializableData = false;
private $_recursingValidate = false;
private $_recursingErrors = false;
private $_recursingReferences = false;
private $_recursingReferenceContext = false;
private $_recursingDocumentContext = false;

private $_baseDocument;
private $_jsonPointer;
Expand Down Expand Up @@ -168,11 +174,11 @@ protected function instantiate($type, $data)
*/
public function getSerializableData()
{
if ($this->_recursing) {
if ($this->_recursingSerializableData) {
// return a reference
return (object) ['$ref' => JsonReference::createFromUri('', $this->getDocumentPosition())->getReference()];
}
$this->_recursing = true;
$this->_recursingSerializableData = true;

$data = $this->_properties;
foreach ($data as $k => $v) {
Expand All @@ -195,7 +201,7 @@ public function getSerializableData()
}
}

$this->_recursing = false;
$this->_recursingSerializableData = false;

return (object) $data;
}
Expand All @@ -208,10 +214,10 @@ public function getSerializableData()
public function validate(): bool
{
// avoid recursion to get stuck in a loop
if ($this->_recursing) {
if ($this->_recursingValidate) {
return true;
}
$this->_recursing = true;
$this->_recursingValidate = true;
$valid = true;
foreach ($this->_properties as $v) {
if ($v instanceof SpecObjectInterface) {
Expand All @@ -228,7 +234,7 @@ public function validate(): bool
}
}
}
$this->_recursing = false;
$this->_recursingValidate = false;

$this->performValidation();

Expand All @@ -246,10 +252,10 @@ public function validate(): bool
public function getErrors(): array
{
// avoid recursion to get stuck in a loop
if ($this->_recursing) {
if ($this->_recursingErrors) {
return [];
}
$this->_recursing = true;
$this->_recursingErrors = true;

if (($pos = $this->getDocumentPosition()) !== null) {
$errors = [
Expand All @@ -272,7 +278,7 @@ public function getErrors(): array
}
}

$this->_recursing = false;
$this->_recursingErrors = false;

return array_merge(...$errors);
}
Expand Down Expand Up @@ -360,10 +366,10 @@ public function __unset($name)
public function resolveReferences(ReferenceContext $context = null)
{
// avoid recursion to get stuck in a loop
if ($this->_recursing) {
if ($this->_recursingReferences) {
return;
}
$this->_recursing = true;
$this->_recursingReferences = true;

foreach ($this->_properties as $property => $value) {
if ($value instanceof Reference) {
Expand All @@ -389,14 +395,20 @@ public function resolveReferences(ReferenceContext $context = null)
}
}

$this->_recursing = false;
$this->_recursingReferences = false;
}

/**
* Set context for all Reference Objects in this object.
*/
public function setReferenceContext(ReferenceContext $context)
{
// avoid recursion to get stuck in a loop
if ($this->_recursingReferenceContext) {
return;
}
$this->_recursingReferenceContext = true;

foreach ($this->_properties as $property => $value) {
if ($value instanceof Reference) {
$value->setContext($context);
Expand All @@ -412,6 +424,8 @@ public function setReferenceContext(ReferenceContext $context)
}
}
}

$this->_recursingReferenceContext = false;
}

/**
Expand All @@ -428,10 +442,10 @@ public function setDocumentContext(SpecObjectInterface $baseDocument, JsonPointe
$this->_jsonPointer = $jsonPointer;

// avoid recursion to get stuck in a loop
if ($this->_recursing) {
if ($this->_recursingDocumentContext) {
return;
}
$this->_recursing = true;
$this->_recursingDocumentContext = true;

foreach ($this->_properties as $property => $value) {
if ($value instanceof DocumentContextInterface) {
Expand All @@ -445,7 +459,7 @@ public function setDocumentContext(SpecObjectInterface $baseDocument, JsonPointe
}
}

$this->_recursing = false;
$this->_recursingDocumentContext = false;
}

/**
Expand Down
44 changes: 44 additions & 0 deletions tests/spec/data/recursion2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
openapi: 3.0.2
info:
title: My API
version: "123"
components:
schemas:
SomeResponse:
type: object
properties:
name:
type: string
description: Name of SomeResponse
recursive:
$ref: '#/components/schemas/RecursiveItem'

AnotherResponse:
type: object
properties:
uuid:
type: string
format: uuid
description: UUID of AnotherResponse
recursive:
$ref: '#/components/schemas/RecursiveItem'


RecursiveItem:
type: object
properties:
children:
type: array
items:
oneOf:
- $ref: '#/components/schemas/RecursiveItem'
- $ref: '#/components/schemas/SomeResponse'


paths:
'/':
get:
description: default
responses:
200:
description: ok

0 comments on commit 497683f

Please sign in to comment.