Skip to content

Commit

Permalink
minor #20945 [Serializer] Fix MaxDepth annotation exceptions (ogizanagi)
Browse files Browse the repository at this point in the history
This PR was merged into the 3.1 branch.

Discussion
----------

[Serializer] Fix MaxDepth annotation exceptions

| Q             | A
| ------------- | ---
| Branch?       | 3.1
| Bug fix?      | yesish (rather improving existing DX, the exception message is not always the most relevant)
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

I'd suggest to slightly change the `MaxDepth` value checks, as for instance setting a 0 value throws the following exception:

> Parameter of annotation "Symfony\Component\Serializer\Annotation\MaxDepth" cannot be empty.

where we should rather expect:

> Parameter of annotation "Symfony\Component\Serializer\Annotation\MaxDepth" must be a positive integer.

IMHO we could even keep the last one only, even if no value has been provided.

Commits
-------

999f769 [Serializer] Fix MaxDepth annotation exceptions
  • Loading branch information
fabpot committed Dec 17, 2016
2 parents 05e83f5 + 999f769 commit 978a13b
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 9 deletions.
4 changes: 2 additions & 2 deletions src/Symfony/Component/Serializer/Annotation/MaxDepth.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ class MaxDepth

public function __construct(array $data)
{
if (!isset($data['value']) || !$data['value']) {
throw new InvalidArgumentException(sprintf('Parameter of annotation "%s" cannot be empty.', get_class($this)));
if (!isset($data['value'])) {
throw new InvalidArgumentException(sprintf('Parameter of annotation "%s" should be set.', get_class($this)));
}

if (!is_int($data['value']) || $data['value'] <= 0) {
Expand Down
20 changes: 13 additions & 7 deletions src/Symfony/Component/Serializer/Tests/Annotation/MaxDepthTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,32 @@ class MaxDepthTest extends \PHPUnit_Framework_TestCase
{
/**
* @expectedException \Symfony\Component\Serializer\Exception\InvalidArgumentException
* @expectedExceptionMessage Parameter of annotation "Symfony\Component\Serializer\Annotation\MaxDepth" should be set.
*/
public function testNotSetMaxDepthParameter()
{
new MaxDepth(array());
}

/**
* @expectedException \Symfony\Component\Serializer\Exception\InvalidArgumentException
*/
public function testEmptyMaxDepthParameter()
public function provideInvalidValues()
{
new MaxDepth(array('value' => ''));
return array(
array(''),
array('foo'),
array('1'),
array(0),
);
}

/**
* @dataProvider provideInvalidValues
*
* @expectedException \Symfony\Component\Serializer\Exception\InvalidArgumentException
* @expectedExceptionMessage Parameter of annotation "Symfony\Component\Serializer\Annotation\MaxDepth" must be a positive integer.
*/
public function testNotAnIntMaxDepthParameter()
public function testNotAnIntMaxDepthParameter($value)
{
new MaxDepth(array('value' => 'foo'));
new MaxDepth(array('value' => $value));
}

public function testMaxDepthParameters()
Expand Down

0 comments on commit 978a13b

Please sign in to comment.