Skip to content

Commit e230461

Browse files
xabbuhxabbuh
committed
deprecate the violation formatter
1 parent 55fafa4 commit e230461

File tree

13 files changed

+164
-97
lines changed

13 files changed

+164
-97
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: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
namespace FOS\RestBundle\Exception;
1313

14-
use FOS\RestBundle\Controller\Annotations\Param;
14+
use FOS\RestBundle\Controller\Annotations\ParamInterface;
1515
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
1616
use Symfony\Component\Validator\ConstraintViolationListInterface;
1717

@@ -20,33 +20,53 @@ class InvalidParameterException extends BadRequestHttpException
2020
private $parameter;
2121
private $violations;
2222

23-
public function __construct(Param $parameter, ConstraintViolationListInterface $violations)
23+
public function getParameter()
2424
{
25-
$this->parameter = $parameter;
26-
$this->violations = $violations;
25+
return $this->parameter;
26+
}
2727

28+
public function getViolations()
29+
{
30+
return $this->violations;
31+
}
32+
33+
public static function withViolations(ParamInterface $parameter, ConstraintViolationListInterface $violations)
34+
{
2835
$message = '';
36+
2937
foreach ($violations as $key => $violation) {
3038
if ($key > 0) {
3139
$message .= "\n";
3240
}
41+
3342
$message .= sprintf(
3443
'Parameter "%s" of value "%s" violated a constraint "%s"',
35-
$parameter->name,
44+
$parameter->getName(),
3645
$violation->getInvalidValue(),
3746
$violation->getMessage()
3847
);
3948
}
40-
parent::__construct($message);
41-
}
4249

43-
public function getParameter()
44-
{
45-
return $this->parameter;
50+
return self::withViolationsAndMessage($parameter, $violations, $message);
4651
}
4752

48-
public function getViolations()
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)
4965
{
50-
return $this->violations;
66+
$exception = new self($message);
67+
$exception->parameter = $parameter;
68+
$exception->violations = $violations;
69+
70+
return $exception;
5171
}
5272
}

‎Request/ParamFetcher.php‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class ParamFetcher implements ParamFetcherInterface, ContainerAwareInterface
3939
private $parameterBag;
4040
private $requestStack;
4141
private $validator;
42+
private $violationFormatter;
4243

4344
/**
4445
* @var ContainerInterface
@@ -51,11 +52,10 @@ class ParamFetcher implements ParamFetcherInterface, ContainerAwareInterface
5152
* Initializes fetcher.
5253
*
5354
* @param ParamReader $paramReader
54-
* @param Request|RequestStack|null $request
55+
* @param Request|RequestStack|null $requestStack
5556
* @param ValidatorInterface|LegacyValidatorInterface $validator
56-
* @param ViolationFormatterInterface $violationFormatter
5757
*/
58-
public function __construct(ParamReader $paramReader, $requestStack, ViolationFormatterInterface $violationFormatter, $validator = null)
58+
public function __construct(ParamReader $paramReader, $requestStack, $validator = null)
5959
{
6060
if (null === $requestStack || $requestStack instanceof Request) {
6161
@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);
@@ -64,6 +64,17 @@ public function __construct(ParamReader $paramReader, $requestStack, ViolationFo
6464
}
6565

6666
$this->requestStack = $requestStack;
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+
6778
$this->validator = $validator;
6879

6980
$this->parameterBag = new ParameterBag($paramReader);
@@ -256,7 +267,17 @@ public function cleanParamWithRequirements(Param $config, $param, $strict)
256267

257268
if (0 !== count($errors)) {
258269
if ($strict) {
259-
throw new InvalidParameterException($config, $errors);
270+
if (null === $this->violationFormatter) {
271+
throw InvalidParameterException::withViolations($config, $errors);
272+
}
273+
274+
if (is_array($config->requirements) && isset($config->requirements['error_message'])) {
275+
$errorMessage = $config->requirements['error_message'];
276+
} else {
277+
$errorMessage = $this->violationFormatter->formatList($config, $errors);
278+
}
279+
280+
throw InvalidParameterException::withViolationsAndMessage($config, $errors, $errorMessage);
260281
}
261282

262283
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: 62 additions & 29 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',
595-
"Parameter \"bizoo\" of value \"\" violated a constraint \"expected message 1\"\nParameter \"bizoo\" of value \"\" violated a constraint \"expected message 2\""
590+
'\FOS\RestBundle\Exception\InvalidParameterException',
591+
'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('Parameter "fero" of value "" violated a constraint "variable must be an integer"', $httpException->getMessage());
816-
}
849+
$queryFetcher->get('fero');
817850
}
818851
}

‎Tests/Util/ViolationFormatterTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
use FOS\RestBundle\Controller\Annotations\RequestParam;
1515
use FOS\RestBundle\Controller\Annotations\QueryParam;
16-
use FOS\RestBundle\Validator\ViolationFormatter;
16+
use FOS\RestBundle\Util\ViolationFormatter;
1717
use Symfony\Component\Validator\ConstraintViolation;
1818
use Symfony\Component\Validator\ConstraintViolationList;
1919

@@ -41,7 +41,7 @@ public function testViolationIsWellFormatted()
4141
$param = new QueryParam();
4242
$param->name = 'foo';
4343

44-
$formatter = new ViolationFormatter();
44+
$formatter = new ViolationFormatter(false);
4545
$this->assertEquals(
4646
"Query parameter foo value 'bar' violated a constraint (expected message)",
4747
$formatter->format($param, $violation)
@@ -58,7 +58,7 @@ public function testViolationListIsWellFormatted()
5858
$param = new RequestParam();
5959
$param->name = 'foo';
6060

61-
$formatter = new ViolationFormatter();
61+
$formatter = new ViolationFormatter(false);
6262
$this->assertEquals(
6363
"Request parameter foo value 'bar' violated a constraint (expected message 1)"
6464
."\nRequest parameter foo value 'bar' violated a constraint (expected message 2)",

0 commit comments

Comments
 (0)