From 499886deab9466d63cee14243d3d1562dc37390d Mon Sep 17 00:00:00 2001 From: Zarko Stankovic Date: Tue, 27 Feb 2018 15:42:40 +0100 Subject: [PATCH 1/3] Fix #1239: FosRestDescriber properly configures Parameter#pattern property. Minor bugfix that configures `Parameter#pattern` property instead of `Parameter#format`. Tests provided that prove this implementation works as expected. --- RouteDescriber/FosRestDescriber.php | 2 +- Tests/Functional/Controller/ApiController.php | 2 +- Tests/Functional/FunctionalTest.php | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/RouteDescriber/FosRestDescriber.php b/RouteDescriber/FosRestDescriber.php index 1a4615f..fef4b18 100644 --- a/RouteDescriber/FosRestDescriber.php +++ b/RouteDescriber/FosRestDescriber.php @@ -54,7 +54,7 @@ final class FosRestDescriber implements RouteDescriberInterface $normalizedRequirements = $this->normalizeRequirements($annotation->requirements); if (null !== $normalizedRequirements) { - $parameter->setFormat($normalizedRequirements); + $parameter->setPattern($normalizedRequirements); } } } diff --git a/Tests/Functional/Controller/ApiController.php b/Tests/Functional/Controller/ApiController.php index db401ee..8ec78ba 100644 --- a/Tests/Functional/Controller/ApiController.php +++ b/Tests/Functional/Controller/ApiController.php @@ -108,7 +108,7 @@ class ApiController /** * @Route("/fosrest.{_format}", methods={"POST"}) * @QueryParam(name="foo") - * @RequestParam(name="bar") + * @RequestParam(name="bar", requirements="\d+") */ public function fosrestAction() { diff --git a/Tests/Functional/FunctionalTest.php b/Tests/Functional/FunctionalTest.php index 38ff95e..2069ed3 100644 --- a/Tests/Functional/FunctionalTest.php +++ b/Tests/Functional/FunctionalTest.php @@ -125,6 +125,10 @@ class FunctionalTest extends WebTestCase $this->assertTrue($parameters->has('foo', 'query')); $this->assertTrue($parameters->has('bar', 'formData')); + $barParameter = $parameters->get('bar', 'formData'); + $this->assertNotNull($barParameter->getPattern()); + $this->assertEquals('\d+', $barParameter->getPattern()); + // The _format path attribute should be removed $this->assertFalse($parameters->has('_format', 'path')); } From 5a1dfa6eadaf94080317fe5d00033394e1f48160 Mon Sep 17 00:00:00 2001 From: Zarko Stankovic Date: Thu, 15 Mar 2018 19:09:51 +0100 Subject: [PATCH 2/3] Added support for "format" field for Constraint objects. --- RouteDescriber/FosRestDescriber.php | 21 ++++++++++++------- Tests/Functional/Controller/ApiController.php | 3 ++- Tests/Functional/FunctionalTest.php | 6 ++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/RouteDescriber/FosRestDescriber.php b/RouteDescriber/FosRestDescriber.php index fef4b18..3984332 100644 --- a/RouteDescriber/FosRestDescriber.php +++ b/RouteDescriber/FosRestDescriber.php @@ -54,7 +54,18 @@ final class FosRestDescriber implements RouteDescriberInterface $normalizedRequirements = $this->normalizeRequirements($annotation->requirements); if (null !== $normalizedRequirements) { - $parameter->setPattern($normalizedRequirements); + if ($normalizedRequirements instanceof Constraint) { + if ($normalizedRequirements instanceof Regex) { + $format = $normalizedRequirements->getHtmlPattern(); + } else { + $reflectionClass = new \ReflectionClass($normalizedRequirements); + $format = $reflectionClass->getShortName(); + } + + $parameter->setFormat($format); + } else { + $parameter->setPattern($normalizedRequirements); + } } } } @@ -71,13 +82,7 @@ final class FosRestDescriber implements RouteDescriberInterface } // if custom constraint if ($requirements instanceof Constraint) { - if ($requirements instanceof Regex) { - return $requirements->getHtmlPattern(); - } - - $reflectionClass = new \ReflectionClass($requirements); - - return $reflectionClass->getShortName(); + return $requirements; } } } diff --git a/Tests/Functional/Controller/ApiController.php b/Tests/Functional/Controller/ApiController.php index 8ec78ba..9b8a160 100644 --- a/Tests/Functional/Controller/ApiController.php +++ b/Tests/Functional/Controller/ApiController.php @@ -23,6 +23,7 @@ use Nelmio\ApiDocBundle\Tests\Functional\Form\DummyType; use Nelmio\ApiDocBundle\Tests\Functional\Form\UserType; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route; use Swagger\Annotations as SWG; +use Symfony\Component\Validator\Constraints\Regex; /** * @Route("/api") @@ -107,7 +108,7 @@ class ApiController /** * @Route("/fosrest.{_format}", methods={"POST"}) - * @QueryParam(name="foo") + * @QueryParam(name="foo", requirements=@Regex("/^\d+$/")) * @RequestParam(name="bar", requirements="\d+") */ public function fosrestAction() diff --git a/Tests/Functional/FunctionalTest.php b/Tests/Functional/FunctionalTest.php index 2069ed3..7684a83 100644 --- a/Tests/Functional/FunctionalTest.php +++ b/Tests/Functional/FunctionalTest.php @@ -125,9 +125,15 @@ class FunctionalTest extends WebTestCase $this->assertTrue($parameters->has('foo', 'query')); $this->assertTrue($parameters->has('bar', 'formData')); + $fooParameter = $parameters->get('foo', 'query'); + $this->assertNotNull($fooParameter->getFormat()); + $this->assertEquals('\d+', $fooParameter->getFormat()); + $this->assertNull($fooParameter->getPattern()); + $barParameter = $parameters->get('bar', 'formData'); $this->assertNotNull($barParameter->getPattern()); $this->assertEquals('\d+', $barParameter->getPattern()); + $this->assertNull($barParameter->getFormat()); // The _format path attribute should be removed $this->assertFalse($parameters->has('_format', 'path')); From e83856ad9a41cfa704d1b77c423a3703e885c51a Mon Sep 17 00:00:00 2001 From: Zarko Stankovic Date: Fri, 23 Mar 2018 19:46:54 +0100 Subject: [PATCH 3/3] Refactored code to achieve the logic we want for pattern and format. --- RouteDescriber/FosRestDescriber.php | 43 +++++++++++-------- Tests/Functional/Controller/ApiController.php | 2 + Tests/Functional/FunctionalTest.php | 12 ++++-- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/RouteDescriber/FosRestDescriber.php b/RouteDescriber/FosRestDescriber.php index 3984332..b4015df 100644 --- a/RouteDescriber/FosRestDescriber.php +++ b/RouteDescriber/FosRestDescriber.php @@ -52,37 +52,44 @@ final class FosRestDescriber implements RouteDescriberInterface $parameter->setDescription($annotation->description); } - $normalizedRequirements = $this->normalizeRequirements($annotation->requirements); - if (null !== $normalizedRequirements) { - if ($normalizedRequirements instanceof Constraint) { - if ($normalizedRequirements instanceof Regex) { - $format = $normalizedRequirements->getHtmlPattern(); - } else { - $reflectionClass = new \ReflectionClass($normalizedRequirements); - $format = $reflectionClass->getShortName(); - } + $pattern = $this->getPattern($annotation->requirements); + if (null !== $pattern) { + $parameter->setPattern($pattern); + } - $parameter->setFormat($format); - } else { - $parameter->setPattern($normalizedRequirements); - } + $format = $this->getFormat($annotation->requirements); + if (null !== $format) { + $parameter->setFormat($format); } } } } - private function normalizeRequirements($requirements) + private function getPattern($requirements) { - // if pattern if (is_array($requirements) && isset($requirements['rule'])) { return (string) $requirements['rule']; } + if (is_string($requirements)) { return $requirements; } - // if custom constraint - if ($requirements instanceof Constraint) { - return $requirements; + + if ($requirements instanceof Regex) { + return $requirements->getHtmlPattern(); } + + return null; + } + + private function getFormat($requirements) + { + if ($requirements instanceof Constraint && !$requirements instanceof Regex) { + $reflectionClass = new \ReflectionClass($requirements); + + return $reflectionClass->getShortName(); + } + + return null; } } diff --git a/Tests/Functional/Controller/ApiController.php b/Tests/Functional/Controller/ApiController.php index 9b8a160..4ce42f8 100644 --- a/Tests/Functional/Controller/ApiController.php +++ b/Tests/Functional/Controller/ApiController.php @@ -23,6 +23,7 @@ use Nelmio\ApiDocBundle\Tests\Functional\Form\DummyType; use Nelmio\ApiDocBundle\Tests\Functional\Form\UserType; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route; use Swagger\Annotations as SWG; +use Symfony\Component\Validator\Constraints\IsTrue; use Symfony\Component\Validator\Constraints\Regex; /** @@ -110,6 +111,7 @@ class ApiController * @Route("/fosrest.{_format}", methods={"POST"}) * @QueryParam(name="foo", requirements=@Regex("/^\d+$/")) * @RequestParam(name="bar", requirements="\d+") + * @RequestParam(name="baz", requirements=@IsTrue) */ public function fosrestAction() { diff --git a/Tests/Functional/FunctionalTest.php b/Tests/Functional/FunctionalTest.php index 7684a83..d003f3e 100644 --- a/Tests/Functional/FunctionalTest.php +++ b/Tests/Functional/FunctionalTest.php @@ -124,17 +124,23 @@ class FunctionalTest extends WebTestCase $parameters = $operation->getParameters(); $this->assertTrue($parameters->has('foo', 'query')); $this->assertTrue($parameters->has('bar', 'formData')); + $this->assertTrue($parameters->has('baz', 'formData')); $fooParameter = $parameters->get('foo', 'query'); - $this->assertNotNull($fooParameter->getFormat()); - $this->assertEquals('\d+', $fooParameter->getFormat()); - $this->assertNull($fooParameter->getPattern()); + $this->assertNotNull($fooParameter->getPattern()); + $this->assertEquals('\d+', $fooParameter->getPattern()); + $this->assertNull($fooParameter->getFormat()); $barParameter = $parameters->get('bar', 'formData'); $this->assertNotNull($barParameter->getPattern()); $this->assertEquals('\d+', $barParameter->getPattern()); $this->assertNull($barParameter->getFormat()); + $bazParameter = $parameters->get('baz', 'formData'); + $this->assertNotNull($bazParameter->getFormat()); + $this->assertEquals('IsTrue', $bazParameter->getFormat()); + $this->assertNull($bazParameter->getPattern()); + // The _format path attribute should be removed $this->assertFalse($parameters->has('_format', 'path')); }