Merge pull request #121 from relaxnow/feature/issue-109-pr2

Issue-109: Fixed infinite recursion on JMS Types that reference themselves or their parents
This commit is contained in:
William DURAND 2013-03-16 18:50:45 +01:00
commit 3fe7e15f58
4 changed files with 279 additions and 129 deletions

View File

@ -32,8 +32,6 @@ class JmsMetadataParser implements ParserInterface
*/ */
private $commentExtractor; private $commentExtractor;
private $parsedClasses = array();
/** /**
* Constructor, requires JMS Metadata factory * Constructor, requires JMS Metadata factory
*/ */
@ -63,10 +61,23 @@ class JmsMetadataParser implements ParserInterface
*/ */
public function parse($input) public function parse($input)
{ {
$meta = $this->factory->getMetadataForClass($input); return $this->doParse($input);
}
/**
* Recursively parse all metadata for a class
*
* @param string $className Class to get all metadata for
* @param array $visited Classes we've already visited to prevent infinite recursion.
* @return array metadata for given class
* @throws \InvalidArgumentException
*/
protected function doParse($className, $visited = array())
{
$meta = $this->factory->getMetadataForClass($className);
if (null === $meta) { if (null === $meta) {
throw new \InvalidArgumentException(sprintf("No metadata found for class %s", $input)); throw new \InvalidArgumentException(sprintf("No metadata found for class %s", $className));
} }
$params = array(); $params = array();
@ -81,23 +92,22 @@ class JmsMetadataParser implements ParserInterface
$params[$name] = array( $params[$name] = array(
'dataType' => $dataType['normalized'], 'dataType' => $dataType['normalized'],
'required' => false, //TODO: can't think of a good way to specify this one, JMS doesn't have a setting for this 'required' => false, //TODO: can't think of a good way to specify this one, JMS doesn't have a setting for this
'description' => $this->getDescription($input, $item), 'description' => $this->getDescription($className, $item),
'readonly' => $item->readOnly 'readonly' => $item->readOnly
); );
// if class already parsed, continue, to avoid infinite recursion // if class already parsed, continue, to avoid infinite recursion
if (in_array($dataType['class'], $this->parsedClasses)) { if (in_array($dataType['class'], $visited)) {
continue; continue;
} }
// check for nested classes with JMS metadata // check for nested classes with JMS metadata
if ($dataType['class'] && null !== $this->factory->getMetadataForClass($dataType['class'])) { if ($dataType['class'] && null !== $this->factory->getMetadataForClass($dataType['class'])) {
$this->parsedClasses[] = $dataType['class']; $visited[] = $dataType['class'];
$params[$name]['children'] = $this->parse($dataType['class']); $params[$name]['children'] = $this->doParse($dataType['class'], $visited);
} }
} }
} }
$this->parsedClasses = array();
return $params; return $params;
} }

View File

@ -26,4 +26,14 @@ class JmsNested
*/ */
public $baz; public $baz;
/**
* @JMS\Type("Nelmio\ApiDocBundle\Tests\Fixtures\Model\JmsNested");
*/
public $circular;
/**
* @JMS\Type("Nelmio\ApiDocBundle\Tests\Fixtures\Model\JmsTest");
*/
public $parent;
} }

View File

@ -205,25 +205,53 @@ nested[baz][]:
With multiple lines. With multiple lines.
nestedArray[]: nested[circular]:
* type: array of objects (JmsNested) * type: object (JmsNested)
* required: false * required: false
* description: No description. * description: No description.
nestedArray[][bar]: nested[parent]:
* type: object (JmsTest)
* required: false
* description: No description.
nested[parent][foo]:
* type: string * type: string
* required: false * required: false
* description: No description. * description: No description.
nestedArray[][baz][]: nested[parent][number]:
* type: array of integers * type: double
* required: false * required: false
* description: Epic description. * description: No description.
With multiple lines. nested[parent][arr]:
* type: array
* required: false
* description: No description.
nested[parent][nested]:
* type: object (JmsNested)
* required: false
* description: No description.
nested[parent][nestedArray][]:
* type: array of objects (JmsNested)
* required: false
* description: No description.
nestedArray[]:
* type: array of objects (JmsNested)
* required: false
* description: No description.
### `GET` /jms-return-test ### ### `GET` /jms-return-test ###
@ -270,7 +298,7 @@ _This method is useful to test if the getDocComment works._
**id** **id**
- Requirement: \d+ - Requirement: \\d+
### `GET` /z-action-with-query-param ### ### `GET` /z-action-with-query-param ###
@ -280,7 +308,7 @@ _This method is useful to test if the getDocComment works._
page: page:
* Requirement: \d+ * Requirement: \\d+
* Description: Page of the overview. * Description: Page of the overview.

View File

@ -32,6 +32,7 @@ class SimpleFormatterTest extends WebTestCase
array ( array (
'method' => 'GET', 'method' => 'GET',
'uri' => '/tests.{_format}', 'uri' => '/tests.{_format}',
'description' => 'index action',
'filters' => 'filters' =>
array ( array (
'a' => 'a' =>
@ -48,9 +49,14 @@ class SimpleFormatterTest extends WebTestCase
), ),
), ),
), ),
'description' => 'index action', 'requirements' =>
'requirements' => array( array (
'_format' => array('dataType' => '', 'description' => '', 'requirement' => ''), '_format' =>
array (
'requirement' => '',
'dataType' => '',
'description' => '',
),
), ),
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
@ -59,6 +65,7 @@ class SimpleFormatterTest extends WebTestCase
array ( array (
'method' => 'GET', 'method' => 'GET',
'uri' => '/tests.{_format}', 'uri' => '/tests.{_format}',
'description' => 'index action',
'filters' => 'filters' =>
array ( array (
'a' => 'a' =>
@ -75,9 +82,14 @@ class SimpleFormatterTest extends WebTestCase
), ),
), ),
), ),
'description' => 'index action', 'requirements' =>
'requirements' => array( array (
'_format' => array('dataType' => '', 'description' => '', 'requirement' => ''), '_format' =>
array (
'requirement' => '',
'dataType' => '',
'description' => '',
),
), ),
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
@ -86,6 +98,7 @@ class SimpleFormatterTest extends WebTestCase
array ( array (
'method' => 'POST', 'method' => 'POST',
'uri' => '/tests.{_format}', 'uri' => '/tests.{_format}',
'description' => 'create test',
'parameters' => 'parameters' =>
array ( array (
'a' => 'a' =>
@ -93,26 +106,31 @@ class SimpleFormatterTest extends WebTestCase
'dataType' => 'string', 'dataType' => 'string',
'required' => true, 'required' => true,
'description' => 'A nice description', 'description' => 'A nice description',
'readonly' => false 'readonly' => false,
), ),
'b' => 'b' =>
array ( array (
'dataType' => 'string', 'dataType' => 'string',
'required' => false, 'required' => false,
'description' => '', 'description' => '',
'readonly' => false 'readonly' => false,
), ),
'c' => 'c' =>
array ( array (
'dataType' => 'boolean', 'dataType' => 'boolean',
'required' => true, 'required' => true,
'description' => '', 'description' => '',
'readonly' => false 'readonly' => false,
), ),
), ),
'description' => 'create test', 'requirements' =>
'requirements' => array( array (
'_format' => array('dataType' => '', 'description' => '', 'requirement' => ''), '_format' =>
array (
'requirement' => '',
'dataType' => '',
'description' => '',
),
), ),
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
@ -121,6 +139,7 @@ class SimpleFormatterTest extends WebTestCase
array ( array (
'method' => 'POST', 'method' => 'POST',
'uri' => '/tests.{_format}', 'uri' => '/tests.{_format}',
'description' => 'create test',
'parameters' => 'parameters' =>
array ( array (
'a' => 'a' =>
@ -128,26 +147,31 @@ class SimpleFormatterTest extends WebTestCase
'dataType' => 'string', 'dataType' => 'string',
'required' => true, 'required' => true,
'description' => 'A nice description', 'description' => 'A nice description',
'readonly' => false 'readonly' => false,
), ),
'b' => 'b' =>
array ( array (
'dataType' => 'string', 'dataType' => 'string',
'required' => false, 'required' => false,
'description' => '', 'description' => '',
'readonly' => false 'readonly' => false,
), ),
'c' => 'c' =>
array ( array (
'dataType' => 'boolean', 'dataType' => 'boolean',
'required' => true, 'required' => true,
'description' => '', 'description' => '',
'readonly' => false 'readonly' => false,
), ),
), ),
'description' => 'create test', 'requirements' =>
'requirements' => array( array (
'_format' => array('dataType' => '', 'description' => '', 'requirement' => ''), '_format' =>
array (
'requirement' => '',
'dataType' => '',
'description' => '',
),
), ),
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
@ -159,6 +183,7 @@ class SimpleFormatterTest extends WebTestCase
array ( array (
'method' => 'POST', 'method' => 'POST',
'uri' => '/another-post', 'uri' => '/another-post',
'description' => 'create another test',
'parameters' => 'parameters' =>
array ( array (
'a' => 'a' =>
@ -166,10 +191,9 @@ class SimpleFormatterTest extends WebTestCase
'dataType' => 'string', 'dataType' => 'string',
'required' => true, 'required' => true,
'description' => 'A nice description', 'description' => 'A nice description',
'readonly' => false 'readonly' => false,
), ),
), ),
'description' => 'create another test',
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
), ),
@ -185,11 +209,16 @@ class SimpleFormatterTest extends WebTestCase
array ( array (
'method' => 'ANY', 'method' => 'ANY',
'uri' => '/any/{foo}', 'uri' => '/any/{foo}',
'description' => 'Action without HTTP verb',
'requirements' => 'requirements' =>
array ( array (
'foo' => array('dataType' => '', 'description' => '', 'requirement' => ''), 'foo' =>
array (
'requirement' => '',
'dataType' => '',
'description' => '',
),
), ),
'description' => 'Action without HTTP verb',
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
), ),
@ -204,6 +233,7 @@ class SimpleFormatterTest extends WebTestCase
array( array(
'method' => 'POST', 'method' => 'POST',
'uri' => '/jms-input-test', 'uri' => '/jms-input-test',
'description' => 'Testing JMS',
'parameters' => 'parameters' =>
array ( array (
'foo' => 'foo' =>
@ -211,87 +241,129 @@ class SimpleFormatterTest extends WebTestCase
'dataType' => 'string', 'dataType' => 'string',
'required' => false, 'required' => false,
'description' => 'No description.', 'description' => 'No description.',
'readonly' => false 'readonly' => false,
), ),
'bar' => 'bar' =>
array ( array (
'dataType' => 'DateTime', 'dataType' => 'DateTime',
'required' => false, 'required' => false,
'description' => 'No description.', 'description' => 'No description.',
'readonly' => true 'readonly' => true,
), ),
'number' => 'number' =>
array ( array (
'dataType' => 'double', 'dataType' => 'double',
'required' => false, 'required' => false,
'description' => 'No description.', 'description' => 'No description.',
'readonly' => false 'readonly' => false,
), ),
'arr' => 'arr' =>
array ( array (
'dataType' => 'array', 'dataType' => 'array',
'required' => false, 'required' => false,
'description' => 'No description.', 'description' => 'No description.',
'readonly' => false 'readonly' => false,
), ),
'nested' => array( 'nested' =>
array (
'dataType' => 'object (JmsNested)', 'dataType' => 'object (JmsNested)',
'required' => false, 'required' => false,
'description' => 'No description.', 'description' => 'No description.',
'readonly' => false, 'readonly' => false,
'children' => array( 'children' =>
'foo' => array( array (
'foo' =>
array (
'dataType' => 'DateTime', 'dataType' => 'DateTime',
'required' => false, 'required' => false,
'description' => 'No description.', 'description' => 'No description.',
'readonly' => true, 'readonly' => true,
), ),
'bar' => array( 'bar' =>
array (
'dataType' => 'string', 'dataType' => 'string',
'required' => false, 'required' => false,
'description' => 'No description.', 'description' => 'No description.',
'readonly' => false, 'readonly' => false,
), ),
'baz' => array( 'baz' =>
array (
'dataType' => 'array of integers', 'dataType' => 'array of integers',
'required' => false, 'required' => false,
'description' => 'Epic description. 'description' => 'Epic description.
With multiple lines.', With multiple lines.',
'readonly' => false, 'readonly' => false,
)
)
), ),
'nestedArray' => array( 'circular' =>
array (
'dataType' => 'object (JmsNested)',
'required' => false,
'description' => 'No description.',
'readonly' => false,
),
'parent' =>
array (
'dataType' => 'object (JmsTest)',
'required' => false,
'description' => 'No description.',
'readonly' => false,
'children' =>
array (
'foo' =>
array (
'dataType' => 'string',
'required' => false,
'description' => 'No description.',
'readonly' => false,
),
'bar' =>
array (
'dataType' => 'DateTime',
'required' => false,
'description' => 'No description.',
'readonly' => true,
),
'number' =>
array (
'dataType' => 'double',
'required' => false,
'description' => 'No description.',
'readonly' => false,
),
'arr' =>
array (
'dataType' => 'array',
'required' => false,
'description' => 'No description.',
'readonly' => false,
),
'nested' =>
array (
'dataType' => 'object (JmsNested)',
'required' => false,
'description' => 'No description.',
'readonly' => false,
),
'nestedArray' =>
array (
'dataType' => 'array of objects (JmsNested)', 'dataType' => 'array of objects (JmsNested)',
'required' => false, 'required' => false,
'description' => 'No description.', 'description' => 'No description.',
'readonly' => false, 'readonly' => false,
'children' => array(
'foo' => array(
'dataType' => 'DateTime',
'required' => false,
'description' => 'No description.',
'readonly' => true,
), ),
'bar' => array( ),
'dataType' => 'string', ),
),
),
'nestedArray' =>
array (
'dataType' => 'array of objects (JmsNested)',
'required' => false, 'required' => false,
'description' => 'No description.', 'description' => 'No description.',
'readonly' => false, 'readonly' => false,
), ),
'baz' => array(
'dataType' => 'array of integers',
'required' => false,
'description' => 'Epic description.
With multiple lines.',
'readonly' => false,
)
)
), ),
),
'description' => 'Testing JMS',
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
), ),
@ -300,13 +372,15 @@ With multiple lines.',
'method' => 'GET', 'method' => 'GET',
'uri' => '/jms-return-test', 'uri' => '/jms-return-test',
'description' => 'Testing return', 'description' => 'Testing return',
'response' => array( 'response' =>
'a' => array( array (
'a' =>
array (
'dataType' => 'string', 'dataType' => 'string',
'required' => true, 'required' => true,
'description' => 'A nice description', 'description' => 'A nice description',
'readonly' => false 'readonly' => false,
) ),
), ),
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
@ -315,10 +389,23 @@ With multiple lines.',
array( array(
'method' => 'ANY', 'method' => 'ANY',
'uri' => '/my-commented/{id}/{page}', 'uri' => '/my-commented/{id}/{page}',
'description' => 'This method is useful to test if the getDocComment works.',
'documentation' => 'This method is useful to test if the getDocComment works.
And, it supports multilines until the first \'@\' char.',
'requirements' => 'requirements' =>
array ( array (
'id' => array('dataType' => 'int', 'description' => 'A nice comment', 'requirement' => ''), 'id' =>
'page' => array('dataType' => 'int', 'description' => '', 'requirement' => ''), array (
'dataType' => 'int',
'description' => 'A nice comment',
'requirement' => '',
),
'page' =>
array (
'dataType' => 'int',
'description' => '',
'requirement' => '',
),
), ),
'https' => false, 'https' => false,
'description' => 'This method is useful to test if the getDocComment works.', 'description' => 'This method is useful to test if the getDocComment works.',
@ -329,13 +416,13 @@ With multiple lines.',
array( array(
'method' => 'ANY', 'method' => 'ANY',
'uri' => '/secure-route', 'uri' => '/secure-route',
// 'description' => '[secureRouteAction description]', 'requirements' =>
// 'documentation' => '[secureRouteAction description]', array (
'requirements' => array( '_scheme' =>
'_scheme' => array( array (
'requirement' => 'https', 'requirement' => 'https',
'dataType' => null, 'dataType' => '',
'description' => null, 'description' => '',
), ),
), ),
'https' => true, 'https' => true,
@ -347,7 +434,12 @@ With multiple lines.',
'uri' => '/yet-another/{id}', 'uri' => '/yet-another/{id}',
'requirements' => 'requirements' =>
array ( array (
'id' => array('dataType' => '', 'description' => '', 'requirement' => '\d+') 'id' =>
array (
'requirement' => '\\d+',
'dataType' => '',
'description' => '',
),
), ),
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
@ -358,7 +450,11 @@ With multiple lines.',
'uri' => '/z-action-with-query-param', 'uri' => '/z-action-with-query-param',
'filters' => 'filters' =>
array ( array (
'page' => array('description' => 'Page of the overview.', 'requirement' => '\d+') 'page' =>
array (
'requirement' => '\\d+',
'description' => 'Page of the overview.',
),
), ),
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,
@ -369,7 +465,13 @@ With multiple lines.',
'uri' => '/z-action-with-request-param', 'uri' => '/z-action-with-request-param',
'parameters' => 'parameters' =>
array ( array (
'param1' => array('description' => 'Param1 description.', 'required' => true, 'dataType' => 'string', 'readonly' => false) 'param1' =>
array (
'required' => true,
'dataType' => 'string',
'description' => 'Param1 description.',
'readonly' => false,
),
), ),
'https' => false, 'https' => false,
'authentication' => false, 'authentication' => false,