Skip to content

Commit 6608121

Browse files
xabbuhxabbuh
committed
feature #1460 Backport #1453 (Ener-Getick, xabbuh)
This PR was merged into the 1.8 branch. Discussion ---------- Backport #1453 Commits ------- e230461 deprecate the violation formatter 55fafa4 Replace the ViolationFormatter by a new exception
2 parents fc4f688 + e230461 commit 6608121

File tree

13 files changed

+199
-89
lines changed

13 files changed

+199
-89
lines changed

‎CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ CHANGELOG
44
1.8.0
55
-----
66

7+
* added a new `InvalidParameterException` as a specialization of the `BadRequestHttpException`
8+
9+
* deprecated the `FOS\RestBundle\Util\ViolationFormatter` class and the
10+
`FOS\RestBundle\Util\ViolationFormatterInterface`
11+
12+
* deprecated the `ViolationFormatterInterface` argument of the `ParamFetcher` class constructor
13+
714
* deprecated the `RedirectView` and `RouteRedirectView` classes, use `View::createRedirect()` and
815
`View::createRouteRedirect()` instead
916

‎Exception/InvalidParameterException.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the FOSRestBundle package.
5+
*
6+
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace FOS\RestBundle\Exception;
13+
14+
use FOS\RestBundle\Controller\Annotations\ParamInterface;
15+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
16+
use Symfony\Component\Validator\ConstraintViolationListInterface;
17+
18+
class InvalidParameterException extends BadRequestHttpException
19+
{
20+
private $parameter;
21+
private $violations;
22+
23+
public function getParameter()
24+
{
25+
return $this->parameter;
26+
}
27+
28+
public function getViolations()
29+
{
30+
return $this->violations;
31+
}
32+
33+
public static function withViolations(ParamInterface $parameter, ConstraintViolationListInterface $violations)
34+
{
35+
$message = '';
36+
37+
foreach ($violations as $key => $violation) {
38+
if ($key > 0) {
39+
$message .= "\n";
40+
}
41+
42+
$message .= sprintf(
43+
'Parameter "%s" of value "%s" violated a constraint "%s"',
44+
$parameter->getName(),
45+
$violation->getInvalidValue(),
46+
$violation->getMessage()
47+
);
48+
}
49+
50+
return self::withViolationsAndMessage($parameter, $violations, $message);
51+
}
52+
53+
/**
54+
* Do not use this method. It will be removed in 2.0.
55+
*
56+
* @param ParamInterface $parameter
57+
* @param ConstraintViolationListInterface $violations
58+
* @param string $message
59+
*
60+
* @return self
61+
*
62+
* @internal
63+
*/
64+
public static function withViolationsAndMessage(ParamInterface $parameter, ConstraintViolationListInterface $violations, $message)
65+
{
66+
$exception = new self($message);
67+
$exception->parameter = $parameter;
68+
$exception->violations = $violations;
69+
70+
return $exception;
71+
}
72+
}

‎Request/ParamFetcher.php‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use FOS\RestBundle\Controller\Annotations\Param;
1616
use FOS\RestBundle\Controller\Annotations\RequestParam;
1717
use FOS\RestBundle\Util\ViolationFormatterInterface;
18+
use FOS\RestBundle\Exception\InvalidParameterException;
1819
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
1920
use Symfony\Component\DependencyInjection\ContainerInterface;
2021
use Symfony\Component\HttpFoundation\Request;
@@ -39,10 +40,7 @@ class ParamFetcher implements ParamFetcherInterface, ContainerAwareInterface
3940
private $requestStack;
4041
private $validator;
4142
private $violationFormatter;
42-
/**
43-
* @var callable
44-
*/
45-
private $controller;
43+
4644
/**
4745
* @var ContainerInterface
4846
*
@@ -54,11 +52,10 @@ class ParamFetcher implements ParamFetcherInterface, ContainerAwareInterface
5452
* Initializes fetcher.
5553
*
5654
* @param ParamReader $paramReader
57-
* @param Request|RequestStack|null $request
55+
* @param Request|RequestStack|null $requestStack
5856
* @param ValidatorInterface|LegacyValidatorInterface $validator
59-
* @param ViolationFormatterInterface $violationFormatter
6057
*/
61-
public function __construct(ParamReader $paramReader, $requestStack, ViolationFormatterInterface $violationFormatter, $validator = null)
58+
public function __construct(ParamReader $paramReader, $requestStack, $validator = null)
6259
{
6360
if (null === $requestStack || $requestStack instanceof Request) {
6461
@trigger_error('Support of Symfony\Component\HttpFoundation\Request in FOS\RestBundle\Request\ParamFetcher is deprecated since version 1.7 and will be removed in 2.0. Use Symfony\Component\HttpFoundation\RequestStack instead.', E_USER_DEPRECATED);
@@ -67,7 +64,17 @@ public function __construct(ParamReader $paramReader, $requestStack, ViolationFo
6764
}
6865

6966
$this->requestStack = $requestStack;
70-
$this->violationFormatter = $violationFormatter;
67+
68+
if ($validator instanceof ViolationFormatterInterface) {
69+
$this->violationFormatter = $validator;
70+
$validator = func_num_args() >= 4 ? func_get_arg(3) : null;
71+
$triggerDeprecation = func_num_args() >= 5 ? func_get_arg(4) : true;
72+
73+
if ($triggerDeprecation) {
74+
@trigger_error(sprintf('Passing a ViolationFormatterInterface instance to the constructor of the %s class is deprecated since version 1.8 and will not be supported anymore in 2.0.', __CLASS__), E_USER_DEPRECATED);
75+
}
76+
}
77+
7178
$this->validator = $validator;
7279

7380
$this->parameterBag = new ParameterBag($paramReader);
@@ -260,12 +267,17 @@ public function cleanParamWithRequirements(Param $config, $param, $strict)
260267

261268
if (0 !== count($errors)) {
262269
if ($strict) {
270+
if (null === $this->violationFormatter) {
271+
throw InvalidParameterException::withViolations($config, $errors);
272+
}
273+
263274
if (is_array($config->requirements) && isset($config->requirements['error_message'])) {
264275
$errorMessage = $config->requirements['error_message'];
265276
} else {
266277
$errorMessage = $this->violationFormatter->formatList($config, $errors);
267278
}
268-
throw new BadRequestHttpException($errorMessage);
279+
280+
throw InvalidParameterException::withViolationsAndMessage($config, $errors, $errorMessage);
269281
}
270282

271283
return null === $default ? '' : $default;

‎Resources/config/request.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
<argument type="service" id="request_stack" on-invalid="null"/>
1919
<argument type="service" id="fos_rest.violation_formatter"/>
2020
<argument type="service" id="validator" on-invalid="null"/>
21+
<argument>false</argument>
2122
</service>
2223

2324
<service id="fos_rest.request.param_fetcher.reader" class="%fos_rest.request.param_fetcher.reader.class%">

‎Resources/config/util.xml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,16 @@
88
<parameter key="fos_rest.format_negotiator.class">FOS\RestBundle\Negotiation\FormatNegotiator</parameter>
99
<parameter key="fos_rest.inflector.class">FOS\RestBundle\Inflector\DoctrineInflector</parameter>
1010
<parameter key="fos_rest.request_matcher.class">Symfony\Component\HttpFoundation\RequestMatcher</parameter>
11-
<parameter key="fos_rest.violation_formatter.class">FOS\RestBundle\Validator\ViolationFormatter</parameter>
11+
<parameter key="fos_rest.violation_formatter.class">FOS\RestBundle\Util\ViolationFormatter</parameter>
1212
</parameters>
1313

1414
<services>
1515
<service id="fos_rest.format_negotiator" class="%fos_rest.format_negotiator.class%" />
1616
<service id="fos_rest.exception_format_negotiator" class="%fos_rest.format_negotiator.class%" />
1717
<service id="fos_rest.inflector.doctrine" class="%fos_rest.inflector.class%" />
1818
<service id="fos_rest.request_matcher" class="%fos_rest.request_matcher.class%" public="false" />
19-
<service id="fos_rest.violation_formatter" class="%fos_rest.violation_formatter.class%" />
19+
<service id="fos_rest.violation_formatter" class="%fos_rest.violation_formatter.class%">
20+
<argument>false</argument>
21+
</service>
2022
</services>
2123
</container>

‎Tests/Request/ParamFetcherTest.php‎

Lines changed: 61 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,6 @@ class ParamFetcherTest extends \PHPUnit_Framework_TestCase
4949
*/
5050
private $validatorMethod;
5151

52-
/**
53-
* @var \PHPUnit_Framework_MockObject_MockObject
54-
*/
55-
private $violationFormatter;
56-
5752
/**
5853
* @var \PHPUnit_Framework_MockObject_MockObject
5954
*/
@@ -155,8 +150,6 @@ public function setup()
155150
$this->validator = $this->getMock('Symfony\Component\Validator\ValidatorInterface');
156151
$this->validatorMethod = 'validateValue';
157152
}
158-
159-
$this->violationFormatter = $this->getMock('FOS\RestBundle\Util\ViolationFormatterInterface');
160153
}
161154

162155
/**
@@ -180,7 +173,7 @@ public function getParamFetcher($query = array(), $request = array(), $attribute
180173
$req = $requestStack;
181174
}
182175

183-
return new ParamFetcher($this->paramReader, $req, $this->violationFormatter, $this->validator);
176+
return new ParamFetcher($this->paramReader, $req, $this->validator);
184177
}
185178

186179
/**
@@ -507,7 +500,7 @@ public function testExceptionOnRequestWithoutController()
507500
$requestStack = new RequestStack();
508501
$requestStack->push(new Request());
509502

510-
$queryFetcher = new ParamFetcher($this->paramReader, $requestStack, $this->violationFormatter, $this->validator);
503+
$queryFetcher = new ParamFetcher($this->paramReader, $requestStack, $this->validator);
511504
$queryFetcher->get('none', '42');
512505
}
513506

@@ -553,11 +546,13 @@ public function testKeyPrecedenceOverName()
553546

554547
/**
555548
* Test an Exception is thrown in strict mode.
549+
*
550+
* @group legacy
556551
*/
557-
public function testConstraintThrowExceptionInStrictMode()
552+
public function testConstraintThrowsExceptionWithExpectedMessageInStrictModeIfViolationFormatterIsGiven()
558553
{
559554
if (!class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
560-
$this->markTestSkipped('RequestStack unvailable.');
555+
$this->markTestSkipped('RequestStack unavailable.');
561556
}
562557

563558
$errors = new ConstraintViolationList(array(
@@ -586,16 +581,56 @@ public function testConstraintThrowExceptionInStrictMode()
586581
->method('read')
587582
->will($this->returnValue(array('bizoo' => $param)));
588583

589-
$this->violationFormatter->expects($this->once())
584+
$violationFormatter = $this->getMock('FOS\RestBundle\Util\ViolationFormatterInterface');
585+
$violationFormatter->expects($this->once())
590586
->method('formatList')
591587
->will($this->returnValue('foobar'));
592588

593589
$this->setExpectedException(
594-
'\\Symfony\\Component\\HttpKernel\\Exception\\BadRequestHttpException',
590+
'\FOS\RestBundle\Exception\InvalidParameterException',
595591
'foobar'
596592
);
597593

598-
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->violationFormatter, $this->validator);
594+
$queryFetcher = new ParamFetcher($reader, $requestStack, $violationFormatter, $this->validator);
595+
$queryFetcher->setController($this->controller);
596+
$queryFetcher->get('bizoo');
597+
}
598+
599+
/**
600+
* @expectedException \FOS\RestBundle\Exception\InvalidParameterException
601+
*/
602+
public function testConstraintThrowsExceptionInStrictMode()
603+
{
604+
if (!class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
605+
$this->markTestSkipped('RequestStack unavailable.');
606+
}
607+
608+
$errors = new ConstraintViolationList(array(
609+
new ConstraintViolation('expected message 1', null, array(), null, null, null),
610+
new ConstraintViolation('expected message 2', null, array(), null, null, null),
611+
));
612+
613+
$this->validator->expects($this->once())
614+
->method($this->validatorMethod)
615+
->with('foobar', $this->constraint)
616+
->will($this->returnValue($errors));
617+
618+
$param = new QueryParam();
619+
$param->name = 'bizoo';
620+
$param->strict = true;
621+
$param->requirements = $this->constraint;
622+
$param->description = 'A requirements param';
623+
624+
$reader = $this->getMockBuilder('FOS\RestBundle\Request\ParamReader')
625+
->disableOriginalConstructor()
626+
->getMock();
627+
$reader->expects($this->any())
628+
->method('read')
629+
->will($this->returnValue(array('bizoo' => $param)));
630+
$requestStack = new RequestStack();
631+
$requestStack->push(new Request(array('bizoo' => 'foobar'), array(), array('_controller' => __CLASS__.'::stubAction')));
632+
633+
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->validator);
599634
$queryFetcher->setController($this->controller);
600635
$queryFetcher->get('bizoo');
601636
}
@@ -636,7 +671,7 @@ public function testConstraintReturnDefaultInSafeMode()
636671
->method('read')
637672
->will($this->returnValue(array('bizoo' => $param)));
638673

639-
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->violationFormatter, $this->validator);
674+
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->validator);
640675
$queryFetcher->setController($this->controller);
641676
$this->assertEquals('expected', $queryFetcher->get('bizoo'));
642677
}
@@ -671,7 +706,7 @@ public function testConstraintOk()
671706
->method('read')
672707
->will($this->returnValue(array('bizoo' => $param)));
673708

674-
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->violationFormatter, $this->validator);
709+
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->validator);
675710
$queryFetcher->setController($this->controller);
676711
$this->assertEquals('foobar', $queryFetcher->get('bizoo'));
677712
}
@@ -706,7 +741,7 @@ public function testDeepArrayAllowedWithConstraint()
706741
->method('read')
707742
->will($this->returnValue(array('bizoo' => $param)));
708743

709-
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->violationFormatter, $this->validator);
744+
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->validator);
710745
$queryFetcher->setController($this->controller);
711746
$this->assertSame(array('foo' => array('b', 'a', 'r')), $queryFetcher->get('bizoo'));
712747
}
@@ -737,7 +772,7 @@ public function testNullValidatorWithRequirements()
737772
->method('read')
738773
->will($this->returnValue(array('bizoo' => $param)));
739774

740-
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->violationFormatter);
775+
$queryFetcher = new ParamFetcher($reader, $requestStack);
741776
$queryFetcher->setController($this->controller);
742777
$queryFetcher->get('bizoo');
743778
}
@@ -763,11 +798,15 @@ public function testNullValidatorWithoutRequirements()
763798
->method('read')
764799
->will($this->returnValue(array('bizoo' => $param)));
765800

766-
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->violationFormatter);
801+
$queryFetcher = new ParamFetcher($reader, $requestStack);
767802
$queryFetcher->setController($this->controller);
768803
$this->assertEquals('foobar', $queryFetcher->get('bizoo'));
769804
}
770805

806+
/**
807+
* @expectedException \FOS\RestBundle\Exception\InvalidParameterException
808+
* @expectedExceptionMessage Parameter "fero" of value "foobar" violated a constraint "variable must be an integer"
809+
*/
771810
public function testCustomErrorMessage()
772811
{
773812
if (!class_exists('Symfony\Component\HttpFoundation\RequestStack')) {
@@ -797,22 +836,16 @@ public function testCustomErrorMessage()
797836
));
798837

799838
$errors = new ConstraintViolationList(array(
800-
new ConstraintViolation($errorMessage, null, array(), null, null, null),
839+
new ConstraintViolation($errorMessage, null, array(), null, null, 'foobar'),
801840
));
802841

803842
$this->validator->expects($this->once())
804843
->method($this->validatorMethod)
805844
->with('foobar', $constraint)
806845
->will($this->returnValue($errors));
807846

808-
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->violationFormatter, $this->validator);
847+
$queryFetcher = new ParamFetcher($reader, $requestStack, $this->validator);
809848
$queryFetcher->setController($this->controller);
810-
811-
try {
812-
$queryFetcher->get('fero');
813-
$this->fail('Fetching get() in strict mode with no default value did not throw an exception');
814-
} catch (HttpException $httpException) {
815-
$this->assertEquals($errorMessage, $httpException->getMessage());
816-
}
849+
$queryFetcher->get('fero');
817850
}
818851
}

0 commit comments

Comments
 (0)