From daeda0a90d7cc4fff0b13414724df833efeabb17 Mon Sep 17 00:00:00 2001 From: Guilhem N Date: Wed, 7 Sep 2016 16:38:45 +0200 Subject: [PATCH 1/9] [ParamFetcher] Fix default resolving --- Request/ParamFetcher.php | 9 ++++----- .../Controller/ParamFetcherController.php | 6 +++--- Tests/Functional/app/ParamFetcher/config.yml | 5 +++-- Tests/Request/ParamFetcherTest.php | 16 ++++++---------- 4 files changed, 16 insertions(+), 20 deletions(-) diff --git a/Request/ParamFetcher.php b/Request/ParamFetcher.php index ea026df5d..24840362b 100644 --- a/Request/ParamFetcher.php +++ b/Request/ParamFetcher.php @@ -101,17 +101,19 @@ public function get($name, $strict = null) /** @var ParamInterface $param */ $param = $params[$name]; $default = $param->getDefault(); + $default = $this->resolveValue($this->container, $default); $strict = (null !== $strict ? $strict : $param->isStrict()); $paramValue = $param->getValue($this->getRequest(), $default); - return $this->cleanParamWithRequirements($param, $paramValue, $strict); + return $this->cleanParamWithRequirements($param, $paramValue, $strict, $default); } /** * @param ParamInterface $param * @param mixed $paramValue * @param bool $strict + * @param mixed $default * * @throws BadRequestHttpException * @throws \RuntimeException @@ -120,11 +122,8 @@ public function get($name, $strict = null) * * @internal */ - protected function cleanParamWithRequirements(ParamInterface $param, $paramValue, $strict) + protected function cleanParamWithRequirements(ParamInterface $param, $paramValue, $strict, $default) { - $default = $param->getDefault(); - $default = $this->resolveValue($this->container, $default); - $this->checkNotIncompatibleParams($param); if ($default !== null && $default === $paramValue) { return $paramValue; diff --git a/Tests/Functional/Bundle/TestBundle/Controller/ParamFetcherController.php b/Tests/Functional/Bundle/TestBundle/Controller/ParamFetcherController.php index ccbbdd407..3358d5625 100644 --- a/Tests/Functional/Bundle/TestBundle/Controller/ParamFetcherController.php +++ b/Tests/Functional/Bundle/TestBundle/Controller/ParamFetcherController.php @@ -24,8 +24,8 @@ class ParamFetcherController extends FOSRestController { /** * @RequestParam(name="raw", requirements=@IdenticalTo({"foo"="raw", "bar"="foo"}), default="invalid") - * @RequestParam(name="map", map=true, requirements=@IdenticalTo({"foo"="map", "foobar"="foo"}), default="%invalid% %%") - * @RequestParam(name="bar", map=true, requirements="%foo% foo", strict=true) + * @RequestParam(name="map", map=true, requirements=@IdenticalTo({"foo"="map", "foobar"="foo"}), default="%invalid2% %%") + * @RequestParam(name="bar", map=true, requirements="%bar% foo", strict=true) */ public function paramsAction(ParamFetcherInterface $fetcher) { @@ -34,7 +34,7 @@ public function paramsAction(ParamFetcherInterface $fetcher) /** * @QueryParam(name="foo", default="invalid") - * @RequestParam(name="bar", default="foo") + * @RequestParam(name="bar", default="%foo%") */ public function testAction(Request $request, ParamFetcherInterface $fetcher) { diff --git a/Tests/Functional/app/ParamFetcher/config.yml b/Tests/Functional/app/ParamFetcher/config.yml index 8cb4b91d4..77993d2ee 100644 --- a/Tests/Functional/app/ParamFetcher/config.yml +++ b/Tests/Functional/app/ParamFetcher/config.yml @@ -2,8 +2,9 @@ imports: - { resource: ../config/default.yml } parameters: - foo: bar - invalid: invalid2 + bar: bar + foo: foo + invalid2: invalid2 framework: serializer: diff --git a/Tests/Request/ParamFetcherTest.php b/Tests/Request/ParamFetcherTest.php index 0aa651eb2..3cd524aa7 100644 --- a/Tests/Request/ParamFetcherTest.php +++ b/Tests/Request/ParamFetcherTest.php @@ -138,10 +138,6 @@ public function testDefaultReplacement() public function testReturnBeforeGettingConstraints() { $param = $this->getMockBuilder(\FOS\RestBundle\Controller\Annotations\ParamInterface::class)->getMock(); - $param - ->expects($this->once()) - ->method('getDefault') - ->willReturn('default'); $param ->expects($this->never()) ->method('getConstraints'); @@ -150,7 +146,7 @@ public function testReturnBeforeGettingConstraints() $this->assertEquals( 'default', - $method->invokeArgs($fetcher, array($param, 'default', null)) + $method->invokeArgs($fetcher, array($param, 'default', null, 'default')) ); } @@ -161,7 +157,7 @@ public function testReturnWhenEmptyConstraints() $this->assertEquals( 'value', - $method->invokeArgs($fetcher, array($param, 'value', null)) + $method->invokeArgs($fetcher, array($param, 'value', null, null)) ); } @@ -198,7 +194,7 @@ public function testNoValidationErrors() ->with('value', array('constraint')) ->willReturn(array()); - $this->assertEquals('value', $method->invokeArgs($fetcher, array($param, 'value', null))); + $this->assertEquals('value', $method->invokeArgs($fetcher, array($param, 'value', null, null))); } public function testValidationErrors() @@ -218,7 +214,7 @@ public function testValidationErrors() ->with('value', ['constraint']) ->willReturn($errors); - $this->assertEquals('default', $method->invokeArgs($fetcher, array($param, 'value', false))); + $this->assertEquals('default', $method->invokeArgs($fetcher, array($param, 'value', false, 'default'))); } public function testValidationException() @@ -238,7 +234,7 @@ public function testValidationException() ->willReturn($errors); try { - $method->invokeArgs($fetcher, array($param, 'value', true)); + $method->invokeArgs($fetcher, array($param, 'value', true, 'default')); $this->fail(sprintf('An exception must be thrown in %s', __METHOD__)); } catch (InvalidParameterException $exception) { $this->assertSame($param, $exception->getParameter()); @@ -272,7 +268,7 @@ public function testValidationErrorsInStrictMode() ->with('value', array('constraint')) ->willReturn($errors); - $method->invokeArgs($fetcher, array($param, 'value', true)); + $method->invokeArgs($fetcher, array($param, 'value', true, null)); } protected function getFetcherToCheckValidation($param, array $constructionArguments = null) From c5f39e5f8bb1771c5b13676dff605551efdcd7e4 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Tue, 11 Oct 2016 21:28:47 +0200 Subject: [PATCH 2/9] drop the build job using Symfony 3.0 Symfony 3.0 is not forward-compatible with Twig 2.0 (it references Twig extensions by their name instead of the extension's FQCN) and won't be updated anymore as it already reached end of maintenance. This currently lets our Travis CI builds fail. --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 94c38873b..05be3715e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,8 +24,6 @@ matrix: env: COMPOSER_FLAGS="--prefer-lowest" SYMFONY_DEPRECATIONS_HELPER=weak - php: 7.0 env: SYMFONY_VERSION='2.8.*' - - php: 7.0 - env: SYMFONY_VERSION='3.0.*' - php: 7.0 env: SYMFONY_VERSION='3.1.*' From a236fd4044d759c75c81b11a403134eafcdf5eba Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Tue, 11 Oct 2016 21:12:07 +0200 Subject: [PATCH 3/9] explicitly list all required packages Instead of relying on the Symfony FrameworkBundle to pull in all our needed dependencies, we should explicitly list all of them. This will ensure that nothing breaks when the FrameworkBundle stops depending on some optional dependencies (which was done recently). --- composer.json | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 5f1d8ff5b..53b4ff768 100644 --- a/composer.json +++ b/composer.json @@ -23,9 +23,17 @@ "require": { "php": "^5.5.9|~7.0", "psr/log": "^1.0", - "symfony/framework-bundle": "^2.7|^3.0", + "symfony/config": "^2.7|^3.0", + "symfony/debug": "^2.7|^3.0", + "symfony/dependency-injection": "^2.7|^3.0", + "symfony/event-dispatcher": "^2.7|^3.0", "symfony/finder": "^2.7|^3.0", + "symfony/framework-bundle": "^2.7|^3.0", + "symfony/http-foundation": "^2.7|^3.0", + "symfony/http-kernel": "^2.7|^3.0", "symfony/routing": "^2.7|^3.0", + "symfony/security-core": "^2.7|^3.0", + "symfony/templating": "^2.7|^3.0", "doctrine/inflector": "^1.0", "willdurand/negotiation": "^2.0", "willdurand/jsonp-callback-validator": "^1.0" From f7464fd35ad31d163370c6339661b328bfdda837 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Thu, 20 Oct 2016 19:45:12 +0200 Subject: [PATCH 4/9] Ignoring PSR7 request typehint in route loader (#1597) --- Routing/Loader/Reader/RestActionReader.php | 2 + .../Controller/TypeHintedController.php | 41 +++++++++++++++++++ Tests/Routing/Loader/RestRouteLoaderTest.php | 18 ++++++++ composer.json | 3 +- 4 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 Tests/Fixtures/Controller/TypeHintedController.php diff --git a/Routing/Loader/Reader/RestActionReader.php b/Routing/Loader/Reader/RestActionReader.php index 5486c3dde..491b76e1e 100644 --- a/Routing/Loader/Reader/RestActionReader.php +++ b/Routing/Loader/Reader/RestActionReader.php @@ -16,6 +16,7 @@ use FOS\RestBundle\Inflector\InflectorInterface; use FOS\RestBundle\Request\ParamReaderInterface; use FOS\RestBundle\Routing\RestRouteCollection; +use Psr\Http\Message\MessageInterface; use Symfony\Component\Routing\Route; /** @@ -476,6 +477,7 @@ private function getMethodArguments(\ReflectionMethod $method) \FOS\RestBundle\Request\ParamFetcherInterface::class, \Symfony\Component\Validator\ConstraintViolationListInterface::class, \Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter::class, + MessageInterface::class, ]; $arguments = []; diff --git a/Tests/Fixtures/Controller/TypeHintedController.php b/Tests/Fixtures/Controller/TypeHintedController.php new file mode 100644 index 000000000..1b1cc7c7e --- /dev/null +++ b/Tests/Fixtures/Controller/TypeHintedController.php @@ -0,0 +1,41 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\RestBundle\Tests\Fixtures\Controller; + +use FOS\RestBundle\Controller\Annotations as Rest; +use FOS\RestBundle\Controller\FOSRestController; +use FOS\RestBundle\Routing\ClassResourceInterface; +use Psr\Http\Message\MessageInterface; +use Psr\Http\Message\ServerRequestInterface; +use Symfony\Component\HttpFoundation\Request; + +/** + * @Rest\RouteResource("Article") + */ +class TypeHintedController implements ClassResourceInterface +{ + public function cgetAction(Request $request) + { + } + + public function cpostAction(MessageInterface $request) + { + } + + public function getAction(Request $request, $id) + { + } + + public function postAction(ServerRequestInterface $request, $id) + { + } +} diff --git a/Tests/Routing/Loader/RestRouteLoaderTest.php b/Tests/Routing/Loader/RestRouteLoaderTest.php index 23a756009..186e8c527 100644 --- a/Tests/Routing/Loader/RestRouteLoaderTest.php +++ b/Tests/Routing/Loader/RestRouteLoaderTest.php @@ -342,6 +342,24 @@ public function testNameMethodPrefixIsPrependingCorrectly() $this->assertNotNull($collection->get('post_users_bar'), 'route for "post_users_bar" does not exist'); } + /** + * RestActionReader::getMethodArguments should ignore certain types of + * parameters. + */ + public function testRequestTypeHintsIgnoredCorrectly() + { + $collection = $this->loadFromControllerFixture('TypeHintedController'); + + $this->assertNotNull($collection->get('get_articles'), 'route for "get_articles" does not exist'); + $this->assertEquals('/articles.{_format}', $collection->get('get_articles')->getPath()); + $this->assertNotNull($collection->get('post_articles'), 'route for "post_articles" does not exist'); + $this->assertEquals('/articles.{_format}', $collection->get('post_articles')->getPath()); + $this->assertNotNull($collection->get('get_article'), 'route for "get_article" does not exist'); + $this->assertEquals('/articles/{id}.{_format}', $collection->get('get_article')->getPath()); + $this->assertNotNull($collection->get('post_article'), 'route for "post_article" does not exist'); + $this->assertEquals('/articles/{id}.{_format}', $collection->get('post_article')->getPath()); + } + /** * Load routes collection from fixture class under Tests\Fixtures directory. * diff --git a/composer.json b/composer.json index 53b4ff768..c5a90e20c 100644 --- a/composer.json +++ b/composer.json @@ -54,7 +54,8 @@ "symfony/expression-language": "~2.7|^3.0", "symfony/css-selector": "^2.7|^3.0", "phpoption/phpoption": "^1.1", - "jms/serializer-bundle": "^1.0" + "jms/serializer-bundle": "^1.0", + "psr/http-message": "^1.0" }, "suggest": { From d849eb3c4e1f73ee256ea3897e48fe0a425e027d Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Thu, 20 Oct 2016 19:45:56 +0200 Subject: [PATCH 5/9] tag attribute depends on the Form component version (#1598) --- DependencyInjection/FOSRestExtension.php | 13 ++++++++++++- Resources/config/forms.xml | 2 -- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/DependencyInjection/FOSRestExtension.php b/DependencyInjection/FOSRestExtension.php index 17b8de982..b47757322 100644 --- a/DependencyInjection/FOSRestExtension.php +++ b/DependencyInjection/FOSRestExtension.php @@ -16,6 +16,8 @@ use Symfony\Component\DependencyInjection\DefinitionDecorator; use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\Extension\Core\Type\FormType; use Symfony\Component\HttpKernel\DependencyInjection\Extension; use Symfony\Component\HttpFoundation\Response; @@ -88,7 +90,16 @@ private function loadForm(array $config, XmlFileLoader $loader, ContainerBuilder { if (!empty($config['disable_csrf_role'])) { $loader->load('forms.xml'); - $container->getDefinition('fos_rest.form.extension.csrf_disable')->replaceArgument(1, $config['disable_csrf_role']); + + $definition = $container->getDefinition('fos_rest.form.extension.csrf_disable'); + $definition->replaceArgument(1, $config['disable_csrf_role']); + + // BC for Symfony < 2.8: the extended_type attribute is used on higher versions + if (!method_exists(AbstractType::class, 'getBlockPrefix')) { + $definition->addTag('form.type_extension', ['alias' => 'form']); + } else { + $definition->addTag('form.type_extension', ['extended_type' => FormType::class]); + } } } diff --git a/Resources/config/forms.xml b/Resources/config/forms.xml index fd42a663c..aede2bf94 100644 --- a/Resources/config/forms.xml +++ b/Resources/config/forms.xml @@ -6,8 +6,6 @@ - - From 9663a24e5f6244ad2e38264e1668a210752bdcec Mon Sep 17 00:00:00 2001 From: Guilhem N Date: Thu, 20 Oct 2016 22:27:54 +0200 Subject: [PATCH 6/9] Remove a contradictory doc block --- Controller/ExceptionController.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Controller/ExceptionController.php b/Controller/ExceptionController.php index 76980462b..6a5ada2cd 100644 --- a/Controller/ExceptionController.php +++ b/Controller/ExceptionController.php @@ -114,10 +114,7 @@ protected function getStatusCode(\Exception $exception) } /** - * Determines the parameters to pass to the view layer. - * - * Overwrite it in a custom ExceptionController class to add additionally parameters - * that should be passed to the view layer. + * Determines the template parameters to pass to the view layer. * * @param string $currentContent * @param int $code From 12081a7402d72f3b7a81184fd15f724c6d3da993 Mon Sep 17 00:00:00 2001 From: Guilhem N Date: Sat, 22 Oct 2016 11:40:42 +0200 Subject: [PATCH 7/9] Add an upgrade warning (#1599) * Upgrade warning * Typo --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 5c9453c6c..234b41a5b 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,11 @@ applications with Symfony. Features include: [![Latest Stable Version](https://poser.pugx.org/FriendsOfSymfony/rest-bundle/v/stable.svg)](https://packagist.org/packages/FriendsOfSymfony/rest-bundle) [![SensioLabsInsight](https://insight.sensiolabs.com/projects/0be23389-2e85-49cf-b333-caaa36d11c62/mini.png)](https://insight.sensiolabs.com/projects/0be23389-2e85-49cf-b333-caaa36d11c62) +Note +---- + +FOSRestBundle 1.x is no longer maintained, 1.8 only receives security fixes. Please upgrade to FOSRestBundle 2.x as soon as possible. + Documentation ------------- From 1f422125b9746ded4ea1037acb65be5f5e7cb9ec Mon Sep 17 00:00:00 2001 From: Toby Griffiths Date: Mon, 31 Oct 2016 09:39:23 +0000 Subject: [PATCH 8/9] Minor typo correction --- Resources/doc/2-the-view-layer.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Resources/doc/2-the-view-layer.rst b/Resources/doc/2-the-view-layer.rst index a01564d13..35bd1bdb2 100644 --- a/Resources/doc/2-the-view-layer.rst +++ b/Resources/doc/2-the-view-layer.rst @@ -185,7 +185,7 @@ Then: leads to a "validation failed" response. - In a rendered template, the form is passed as 'form' and ``createView()`` is called automatically. -- ``$form->getData()`` is passed into the view as template as ``'data'`` if the +- ``$form->getData()`` is passed into the view template as ``'data'`` if the form is the only view data. - An invalid form will be wrapped into an exception. From cdc6095109f60a3366943c0a47c71147eda974e9 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Mon, 7 Nov 2016 18:59:45 +0100 Subject: [PATCH 9/9] the Regex constraint is resolvable (#1591) * the Regex constraint is resolvable * Add a test case * Update ParamFetcherController.php --- .../Controller/ParamFetcherController.php | 8 ++++---- Tests/Functional/ParamFetcherTest.php | 18 ++---------------- Validator/Constraints/Regex.php | 2 +- 3 files changed, 7 insertions(+), 21 deletions(-) diff --git a/Tests/Functional/Bundle/TestBundle/Controller/ParamFetcherController.php b/Tests/Functional/Bundle/TestBundle/Controller/ParamFetcherController.php index 3358d5625..8ae718d8f 100644 --- a/Tests/Functional/Bundle/TestBundle/Controller/ParamFetcherController.php +++ b/Tests/Functional/Bundle/TestBundle/Controller/ParamFetcherController.php @@ -23,13 +23,13 @@ class ParamFetcherController extends FOSRestController { /** - * @RequestParam(name="raw", requirements=@IdenticalTo({"foo"="raw", "bar"="foo"}), default="invalid") - * @RequestParam(name="map", map=true, requirements=@IdenticalTo({"foo"="map", "foobar"="foo"}), default="%invalid2% %%") - * @RequestParam(name="bar", map=true, requirements="%bar% foo", strict=true) + * @RequestParam(name="raw", requirements=@IdenticalTo({"foo"="raw", "bar"="foo"}), default="invalid", strict=false) + * @RequestParam(name="map", map=true, requirements=@IdenticalTo({"foo"="map", "foobar"="foo"}), default="%invalid2% %%", strict=false) + * @RequestParam(name="bar", nullable=true, requirements="%bar%\ foo") */ public function paramsAction(ParamFetcherInterface $fetcher) { - return new JsonResponse($fetcher->all(false)); + return new JsonResponse($fetcher->all()); } /** diff --git a/Tests/Functional/ParamFetcherTest.php b/Tests/Functional/ParamFetcherTest.php index 7628c384d..53114fab0 100644 --- a/Tests/Functional/ParamFetcherTest.php +++ b/Tests/Functional/ParamFetcherTest.php @@ -50,23 +50,9 @@ public function testValidMapParameter() 'foo' => $this->validMap, 'bar' => $this->validMap, ]; - $this->client->request('POST', '/params', ['raw' => 'bar', 'map' => $map]); + $this->client->request('POST', '/params', ['raw' => 'bar', 'map' => $map, 'bar' => 'bar foo']); - $this->assertEquals(['raw' => 'invalid', 'map' => $map, 'bar' => null], $this->getData()); - } - - public function testFooParameter() - { - $value = ['bar foo', 'bar foo']; - $this->client->request('POST', '/params', ['foo' => $value]); - - $map = array( - 'foo' => $this->validMap, - 'bar' => $this->validMap, - ); - $this->client->request('POST', '/params', array('raw' => 'bar', 'map' => $map)); - - $this->assertEquals(array('raw' => 'invalid', 'map' => $map, 'bar' => null), $this->getData()); + $this->assertEquals(['raw' => 'invalid', 'map' => $map, 'bar' => 'bar foo'], $this->getData()); } public function testWithSubRequests() diff --git a/Validator/Constraints/Regex.php b/Validator/Constraints/Regex.php index 8c2c0dda0..377e02345 100644 --- a/Validator/Constraints/Regex.php +++ b/Validator/Constraints/Regex.php @@ -20,7 +20,7 @@ * * @author Ener-Getick */ -class Regex extends BaseRegex +class Regex extends BaseRegex implements ResolvableConstraintInterface { use ResolverTrait;