diff --git a/Resources/config/fos_rest.xml b/Resources/config/fos_rest.xml index ede8766..dbf8f2f 100644 --- a/Resources/config/fos_rest.xml +++ b/Resources/config/fos_rest.xml @@ -7,7 +7,7 @@ - + diff --git a/Resources/config/php_doc.xml b/Resources/config/php_doc.xml index 47b2624..2adc859 100644 --- a/Resources/config/php_doc.xml +++ b/Resources/config/php_doc.xml @@ -5,7 +5,7 @@ - + diff --git a/RouteDescriber/RouteMetadataDescriber.php b/RouteDescriber/RouteMetadataDescriber.php index ed67dd2..a226a32 100644 --- a/RouteDescriber/RouteMetadataDescriber.php +++ b/RouteDescriber/RouteMetadataDescriber.php @@ -11,9 +11,15 @@ namespace Nelmio\ApiDocBundle\RouteDescriber; +use EXSyst\Component\Swagger\Operation; +use EXSyst\Component\Swagger\Parameter; use EXSyst\Component\Swagger\Swagger; +use LogicException; use Symfony\Component\Routing\Route; +/** + * Should be last route describer executed to make sure all params are set. + */ final class RouteMetadataDescriber implements RouteDescriberInterface { use RouteDescriberTrait; @@ -25,6 +31,7 @@ final class RouteMetadataDescriber implements RouteDescriberInterface $requirements = $route->getRequirements(); $compiledRoute = $route->compile(); + $existingParams = $this->getRefParams($api, $operation); // Don't include host requirements foreach ($compiledRoute->getPathVariables() as $pathVariable) { @@ -32,6 +39,16 @@ final class RouteMetadataDescriber implements RouteDescriberInterface continue; } + $paramId = $pathVariable.'/path'; + $parameter = $existingParams[$paramId] ?? null; + if (null !== $parameter) { + if (!$parameter->getRequired()) { + throw new LogicException(\sprintf('Global parameter "%s" is used as part of route "%s" and must be set as "required"', $pathVariable, $route->getPath())); + } + + continue; + } + $parameter = $operation->getParameters()->get($pathVariable, 'path'); $parameter->setRequired(true); @@ -39,10 +56,43 @@ final class RouteMetadataDescriber implements RouteDescriberInterface $parameter->setType('string'); } - if (isset($requirements[$pathVariable])) { + if (isset($requirements[$pathVariable]) && null === $parameter->getPattern()) { $parameter->setPattern($requirements[$pathVariable]); } } } } + + /** + * The '$ref' parameters need special handling, since their objects are missing 'name' and 'in'. + * + * @return Parameter[] existing $ref parameters + */ + private function getRefParams(Swagger $api, Operation $operation): array + { + /** @var Parameter[] $globalParams */ + $globalParams = $api->getParameters(); + $existingParams = []; + + foreach ($operation->getParameters() as $id => $parameter) { + $ref = $parameter->getRef(); + if (null === $ref) { + // we only concern ourselves with '$ref' parameters, so continue the loop + continue; + } + + $ref = \mb_substr($ref, 13); // trim the '#/parameters/' part of ref + if (!isset($globalParams[$ref])) { + // this shouldn't happen during proper configs, but in case of bad config, just ignore it here + continue; + } + + $refParameter = $globalParams[$ref]; + + // param ids are in form {name}/{in} + $existingParams[\sprintf('%s/%s', $refParameter->getName(), $refParameter->getIn())] = $refParameter; + } + + return $existingParams; + } } diff --git a/Tests/Describer/AbstractDescriberTest.php b/Tests/Describer/AbstractDescriberTest.php index b1cca2d..f73dcb3 100644 --- a/Tests/Describer/AbstractDescriberTest.php +++ b/Tests/Describer/AbstractDescriberTest.php @@ -12,10 +12,12 @@ namespace Nelmio\ApiDocBundle\Tests\Describer; use EXSyst\Component\Swagger\Swagger; +use Nelmio\ApiDocBundle\Describer\DescriberInterface; use PHPUnit\Framework\TestCase; abstract class AbstractDescriberTest extends TestCase { + /** @var DescriberInterface */ protected $describer; protected function getSwaggerDoc(): Swagger diff --git a/Tests/Functional/Controller/TestController.php b/Tests/Functional/Controller/TestController.php index f7cf30d..8b2aaba 100644 --- a/Tests/Functional/Controller/TestController.php +++ b/Tests/Functional/Controller/TestController.php @@ -29,4 +29,16 @@ class TestController public function testAction() { } + + /** + * @SWG\Parameter(ref="#/parameters/test"), + * @SWG\Response( + * response="200", + * description="Test Ref" + * ) + * @Route("/test/{id}", methods={"GET"}) + */ + public function testRefAction() + { + } } diff --git a/Tests/Functional/FunctionalTest.php b/Tests/Functional/FunctionalTest.php index 0c7fc20..138aaa7 100644 --- a/Tests/Functional/FunctionalTest.php +++ b/Tests/Functional/FunctionalTest.php @@ -402,4 +402,9 @@ class FunctionalTest extends WebTestCase $this->assertSame('This is post', $postOperation->getDescription()); $this->assertSame('Worked well!', $postOperation->getResponses()->get(200)->getDescription()); } + + public function testNoDuplicatedParameters() + { + $this->assertFalse($this->getOperation('/api/article/{id}', 'get')->getParameters()->has('id', 'path')); + } } diff --git a/Tests/Functional/SwaggerUiTest.php b/Tests/Functional/SwaggerUiTest.php index cd29e7a..0ed8ce5 100644 --- a/Tests/Functional/SwaggerUiTest.php +++ b/Tests/Functional/SwaggerUiTest.php @@ -59,6 +59,10 @@ class SwaggerUiTest extends WebTestCase '/test/test/' => ['get' => [ 'responses' => ['200' => ['description' => 'Test']], ]], + '/test/test/{id}' => ['get' => [ + 'responses' => ['200' => ['description' => 'Test Ref']], + 'parameters' => [['$ref' => '#/parameters/test']], + ]], ]; $expected['definitions'] = [ 'Dummy' => $expected['definitions']['Dummy'],