Skip to content

Commit e45b579

Browse files
RenrhafRenrhaf
authored
fix(validator): skip ValidateProcessor when ObjectMapper is not used (#7848)
Fixes #7846
1 parent 0d465ea commit e45b579

File tree

7 files changed

+212
-2
lines changed

7 files changed

+212
-2
lines changed

‎src/Symfony/EventListener/ValidateProcessorListener.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ public function onKernelView(ViewEvent $event): void
4545
return;
4646
}
4747

48+
// Only validate at the processor level when ObjectMapper is used (canMap() is true).
49+
// Without ObjectMapper, the validate listener already handles validation,
50+
// so running it again here would cause duplicate validation.
51+
if (!$operation->canMap()) {
52+
return;
53+
}
54+
4855
if (null === $operation->canWrite()) {
4956
$operation = $operation->withWrite(!$request->isMethodSafe());
5057
}

‎src/Symfony/Validator/State/ValidateProcessor.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
4444
return $this->decorated ? $this->decorated->process($data, $operation, $uriVariables, $context) : $data;
4545
}
4646

47+
// Only validate at the processor level when ObjectMapper is used (canMap() is true).
48+
// Without ObjectMapper, ValidateProvider already handles validation in the provider chain,
49+
// so running it again here would cause duplicate external API calls and DB queries.
50+
if (!$operation->canMap()) {
51+
return $this->decorated ? $this->decorated->process($data, $operation, $uriVariables, $context) : $data;
52+
}
53+
4754
$this->validator->validate($data, $operation->getValidationContext() ?? []);
4855

4956
return $this->decorated ? $this->decorated->process($data, $operation, $uriVariables, $context) : $data;

‎tests/Fixtures/TestBundle/ApiResource/ValidateOnce.php‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.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+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource;
15+
16+
use ApiPlatform\Metadata\ApiProperty;
17+
use ApiPlatform\Metadata\Get;
18+
use ApiPlatform\Metadata\Post;
19+
use ApiPlatform\Tests\Fixtures\TestBundle\Validator\CountableConstraint;
20+
21+
/**
22+
* Resource without ObjectMapper to test that validation runs only once.
23+
*
24+
* The CountableConstraint validator increments a static counter on each call.
25+
* Without the fix, ValidateProvider + ValidateProcessor both trigger validation,
26+
* causing the counter to reach 2 instead of 1.
27+
*/
28+
#[Post(
29+
shortName: 'ValidateOnce',
30+
uriTemplate: '/validate_once',
31+
provider: [self::class, 'provide'],
32+
processor: [self::class, 'process'],
33+
)]
34+
#[Get(
35+
shortName: 'ValidateOnce',
36+
uriTemplate: '/validate_once/{id}',
37+
provider: [self::class, 'provide'],
38+
)]
39+
class ValidateOnce
40+
{
41+
#[ApiProperty(identifier: true)]
42+
public ?int $id = null;
43+
44+
#[CountableConstraint]
45+
public string $name = '';
46+
47+
public static function provide(): self
48+
{
49+
$s = new self();
50+
$s->id = 1;
51+
$s->name = 'test';
52+
53+
return $s;
54+
}
55+
56+
public static function process(self $data): self
57+
{
58+
$data->id = 1;
59+
60+
return $data;
61+
}
62+
}

‎tests/Fixtures/TestBundle/Validator/CountableConstraint.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.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+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\Validator;
15+
16+
use Symfony\Component\Validator\Constraint;
17+
18+
/**
19+
* A constraint that counts how many times its validator is invoked.
20+
* Used to verify that validation is not run more than once per request.
21+
*/
22+
#[\Attribute(\Attribute::TARGET_PROPERTY)]
23+
class CountableConstraint extends Constraint
24+
{
25+
}

‎tests/Fixtures/TestBundle/Validator/CountableConstraintValidator.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.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+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\Validator;
15+
16+
use Symfony\Component\Validator\Constraint;
17+
use Symfony\Component\Validator\ConstraintValidator;
18+
19+
/**
20+
* Validator that counts invocations via a static counter.
21+
* Reset the counter before each test with CountableConstraintValidator::$count = 0.
22+
*/
23+
class CountableConstraintValidator extends ConstraintValidator
24+
{
25+
public static int $count = 0;
26+
27+
public function validate(mixed $value, Constraint $constraint): void
28+
{
29+
++self::$count;
30+
}
31+
}

‎tests/Functional/ValidateProcessorTest.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.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+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Functional;
15+
16+
use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
17+
use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\ValidateOnce;
18+
use ApiPlatform\Tests\Fixtures\TestBundle\Validator\CountableConstraintValidator;
19+
use ApiPlatform\Tests\SetupClassResourcesTrait;
20+
21+
/**
22+
* Tests that validation is not run twice for resources without ObjectMapper.
23+
*
24+
* ValidateProvider (provider chain) and ValidateProcessor (processor chain) should
25+
* not both validate the same data. ValidateProcessor should only run when
26+
* ObjectMapper is used (canMap() is true).
27+
*/
28+
class ValidateProcessorTest extends ApiTestCase
29+
{
30+
use SetupClassResourcesTrait;
31+
32+
protected static ?bool $alwaysBootKernel = false;
33+
34+
/**
35+
* @return class-string[]
36+
*/
37+
public static function getResources(): array
38+
{
39+
return [
40+
ValidateOnce::class,
41+
];
42+
}
43+
44+
public function testValidationRunsOnlyOnceWithoutObjectMapper(): void
45+
{
46+
CountableConstraintValidator::$count = 0;
47+
48+
self::createClient()->request('POST', '/validate_once', [
49+
'json' => ['name' => 'test'],
50+
]);
51+
52+
$this->assertResponseIsSuccessful();
53+
$this->assertSame(1, CountableConstraintValidator::$count, 'Validation should run exactly once for resources without ObjectMapper.');
54+
}
55+
}

‎tests/Symfony/Validator/State/ValidateProcessorTest.php‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function testValidate(): void
3131
$validator = $this->createMock(ValidatorInterface::class);
3232
$validator->expects($this->once())->method('validate')->with($obj, $validationContext);
3333
$processor = new ValidateProcessor($decorated, $validator);
34-
$processor->process($obj, new Post(validationContext: $validationContext));
34+
$processor->process($obj, new Post(map: true, validationContext: $validationContext));
3535
}
3636

3737
public function testNoValidate(): void
@@ -75,7 +75,7 @@ public function testPassesValidationContext(): void
7575
$validator = $this->createMock(ValidatorInterface::class);
7676
$validator->expects($this->once())->method('validate')->with($obj, $validationContext);
7777
$processor = new ValidateProcessor($decorated, $validator);
78-
$processor->process($obj, new Post(validationContext: $validationContext));
78+
$processor->process($obj, new Post(map: true, validationContext: $validationContext));
7979
}
8080

8181
public function testDecoratorChainContinues(): void
@@ -111,4 +111,27 @@ public function testSkipsWhenCanWriteIsFalse(): void
111111
// Delete operations typically have write: false
112112
$processor->process($obj, new Post(write: false));
113113
}
114+
115+
public function testSkipsWhenCanMapIsNull(): void
116+
{
117+
$obj = new \stdClass();
118+
$decorated = $this->createMock(ProcessorInterface::class);
119+
$decorated->expects($this->once())->method('process')->with($obj)->willReturn($obj);
120+
$validator = $this->createMock(ValidatorInterface::class);
121+
$validator->expects($this->never())->method('validate');
122+
$processor = new ValidateProcessor($decorated, $validator);
123+
// Default: map is null (no ObjectMapper), ValidateProvider already validated
124+
$processor->process($obj, new Post());
125+
}
126+
127+
public function testSkipsWhenCanMapIsFalse(): void
128+
{
129+
$obj = new \stdClass();
130+
$decorated = $this->createMock(ProcessorInterface::class);
131+
$decorated->expects($this->once())->method('process')->with($obj)->willReturn($obj);
132+
$validator = $this->createMock(ValidatorInterface::class);
133+
$validator->expects($this->never())->method('validate');
134+
$processor = new ValidateProcessor($decorated, $validator);
135+
$processor->process($obj, new Post(map: false));
136+
}
114137
}

0 commit comments

Comments
 (0)