Skip to content

Commit ccb4d16

Browse files
KocKoc
committed
Code review fixes
1 parent 65778a8 commit ccb4d16

File tree

5 files changed

+43
-52
lines changed

5 files changed

+43
-52
lines changed

‎src/Symfony/Component/HttpKernel/Attribute/MapQueryString.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* @author Konstantin Myakshin <molodchick@gmail.com>
2020
*/
2121
#[\Attribute(\Attribute::TARGET_PARAMETER)]
22-
final class MapQueryString extends ValueResolver
22+
class MapQueryString extends ValueResolver
2323
{
2424
public function __construct(
2525
public readonly array $context = [],

‎src/Symfony/Component/HttpKernel/Attribute/MapRequestPayload.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* @author Konstantin Myakshin <molodchick@gmail.com>
2020
*/
2121
#[\Attribute(\Attribute::TARGET_PARAMETER)]
22-
final class MapRequestPayload extends ValueResolver
22+
class MapRequestPayload extends ValueResolver
2323
{
2424
public function __construct(
2525
public readonly array $context = [],

‎src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php

+16-10
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
2323
use Symfony\Component\Serializer\Exception\PartialDenormalizationException;
2424
use Symfony\Component\Serializer\Exception\UnsupportedFormatException;
25-
use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;
2625
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
2726
use Symfony\Component\Serializer\SerializerInterface;
2827
use Symfony\Component\Validator\ConstraintViolationInterface;
@@ -34,15 +33,20 @@
3433
*/
3534
final class RequestPayloadValueResolver implements ValueResolverInterface
3635
{
37-
private const DEFAULT_FORMAT = 'json';
38-
36+
/**
37+
* @see \Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT
38+
* @see DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS
39+
*/
3940
private const CONTEXT_DENORMALIZE = [
40-
AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true,
41-
DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS => true,
41+
'disable_type_enforcement' => true,
42+
'collect_denormalization_errors' => true,
4243
];
4344

45+
/**
46+
* @see DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS
47+
*/
4448
private const CONTEXT_DESERIALIZE = [
45-
DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS => true,
49+
'collect_denormalization_errors' => true,
4650
];
4751

4852
public function __construct(
@@ -59,7 +63,7 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable
5963
];
6064

6165
foreach ($payloadMappers as $mappingAttribute => [$payloadMapper, $validationFailedCode]) {
62-
if (!$attributes = $argument->getAttributesOfType($mappingAttribute)) {
66+
if (!$attributes = $argument->getAttributesOfType($mappingAttribute, ArgumentMetadata::IS_INSTANCEOF)) {
6367
continue;
6468
}
6569

@@ -91,20 +95,22 @@ private function mapQueryString(Request $request, string $type, MapQueryString $
9195
return null;
9296
}
9397

94-
return $this->serializer->denormalize($data, $type, self::DEFAULT_FORMAT, self::CONTEXT_DENORMALIZE + $attribute->context);
98+
return $this->serializer->denormalize($data, $type, 'json', self::CONTEXT_DENORMALIZE + $attribute->context);
9599
}
96100

97101
private function mapRequestPayload(Request $request, string $type, MapRequestPayload $attribute): ?object
98102
{
99103
if ($data = $request->request->all()) {
100-
return $this->serializer->denormalize($data, $type, self::DEFAULT_FORMAT, self::CONTEXT_DENORMALIZE + $attribute->context);
104+
return $this->serializer->denormalize($data, $type, 'json', self::CONTEXT_DENORMALIZE + $attribute->context);
101105
}
102106

103107
if ('' === $data = $request->getContent()) {
104108
return null;
105109
}
106110

107-
$format = $request->getRequestFormat(self::DEFAULT_FORMAT);
111+
if (null === $format = $request->getContentTypeFormat()) {
112+
throw new HttpException(Response::HTTP_UNSUPPORTED_MEDIA_TYPE, 'Unsupported format.');
113+
}
108114

109115
try {
110116
return $this->serializer->deserialize($data, $type, $format, self::CONTEXT_DESERIALIZE + $attribute->context);

‎src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestPayloadValueResolverTest.php

+24-39
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\HttpKernel\Exception\HttpException;
2121
use Symfony\Component\Serializer\Encoder\JsonEncoder;
2222
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
23+
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
2324
use Symfony\Component\Serializer\Serializer;
2425
use Symfony\Component\Serializer\SerializerInterface;
2526
use Symfony\Component\Validator\ConstraintViolation;
@@ -39,7 +40,7 @@ public function testNotTypedArgument()
3940
$argument = new ArgumentMetadata('notTyped', null, false, false, null, false, [
4041
MapRequestPayload::class => new MapRequestPayload(),
4142
]);
42-
$request = Request::create('/', 'POST', server: ['HTTP_ACCEPT' => 'application/json']);
43+
$request = Request::create('/', 'POST', server: ['HTTP_CONTENT_TYPE' => 'application/json']);
4344

4445
$this->expectException(\LogicException::class);
4546
$this->expectExceptionMessage('Could not resolve the "$notTyped" controller argument: argument should be typed.');
@@ -49,18 +50,26 @@ public function testNotTypedArgument()
4950

5051
public function testValidationNotPassed()
5152
{
52-
$serializer = new Serializer([new DummyDenormalizer()], ['json' => new JsonEncoder()]);
53+
$content = '{"price": 50}';
54+
$payload = new RequestPayload(50);
55+
$serializer = $this->createMock(DummySerializerDenormalizerInterface::class);
56+
$serializer->expects($this->once())
57+
->method('deserialize')
58+
->with($content)
59+
->willReturn($payload);
60+
5361
$validator = $this->createMock(ValidatorInterface::class);
5462
$validator->expects($this->once())
5563
->method('validate')
64+
->with($payload)
5665
->willReturn(new ConstraintViolationList([new ConstraintViolation('Test', null, [], '', null, '')]));
5766

5867
$resolver = new RequestPayloadValueResolver($serializer, $validator);
5968

60-
$argument = new ArgumentMetadata('invalid', \stdClass::class, false, false, null, false, [
69+
$argument = new ArgumentMetadata('invalid', RequestPayload::class, false, false, null, false, [
6170
MapRequestPayload::class => new MapRequestPayload(),
6271
]);
63-
$request = Request::create('/', 'POST', server: ['HTTP_ACCEPT' => 'application/json'], content: '["abc"]');
72+
$request = Request::create('/', 'POST', server: ['HTTP_CONTENT_TYPE' => 'application/json'], content: $content);
6473

6574
try {
6675
$resolver->resolve($request, $argument);
@@ -93,7 +102,7 @@ public function testUnsupportedMedia()
93102

94103
public function testRequestContentValidationPassed()
95104
{
96-
$content = '{"quantity": 50}';
105+
$content = '{"price": 50}';
97106
$payload = new RequestPayload(50);
98107
$serializer = $this->createMock(DummySerializerDenormalizerInterface::class);
99108
$serializer->expects($this->once())
@@ -108,7 +117,7 @@ public function testRequestContentValidationPassed()
108117

109118
$resolver = new RequestPayloadValueResolver($serializer, $validator);
110119

111-
$argument = new ArgumentMetadata('valid', \stdClass::class, false, false, null, false, [
120+
$argument = new ArgumentMetadata('valid', RequestPayload::class, false, false, null, false, [
112121
MapRequestPayload::class => new MapRequestPayload(),
113122
]);
114123
$request = Request::create('/', 'POST', content: $content);
@@ -119,12 +128,9 @@ public function testRequestContentValidationPassed()
119128
public function testQueryStringValidationPassed()
120129
{
121130
$payload = new RequestPayload(50);
122-
$query = ['quantity' => 50];
123-
$serializer = $this->createMock(Serializer::class);
124-
$serializer->expects($this->once())
125-
->method('denormalize')
126-
->with($query)
127-
->willReturn($payload);
131+
$query = ['price' => '50'];
132+
133+
$serializer = new Serializer([new ObjectNormalizer()], ['json' => new JsonEncoder()]);
128134

129135
$validator = $this->createMock(ValidatorInterface::class);
130136
$validator->expects($this->once())
@@ -133,7 +139,7 @@ public function testQueryStringValidationPassed()
133139

134140
$resolver = new RequestPayloadValueResolver($serializer, $validator);
135141

136-
$argument = new ArgumentMetadata('valid', \stdClass::class, false, false, null, false, [
142+
$argument = new ArgumentMetadata('valid', RequestPayload::class, false, false, null, false, [
137143
MapQueryString::class => new MapQueryString(),
138144
]);
139145
$request = Request::create('/', 'GET', $query);
@@ -143,13 +149,10 @@ public function testQueryStringValidationPassed()
143149

144150
public function testRequestInputValidationPassed()
145151
{
146-
$input = ['quantity' => 50];
152+
$input = ['price' => '50'];
147153
$payload = new RequestPayload(50);
148-
$serializer = $this->createMock(Serializer::class);
149-
$serializer->expects($this->once())
150-
->method('denormalize')
151-
->with($input)
152-
->willReturn($payload);
154+
155+
$serializer = new Serializer([new ObjectNormalizer()], ['json' => new JsonEncoder()]);
153156

154157
$validator = $this->createMock(ValidatorInterface::class);
155158
$validator->expects($this->once())
@@ -158,7 +161,7 @@ public function testRequestInputValidationPassed()
158161

159162
$resolver = new RequestPayloadValueResolver($serializer, $validator);
160163

161-
$argument = new ArgumentMetadata('valid', \stdClass::class, false, false, null, false, [
164+
$argument = new ArgumentMetadata('valid', RequestPayload::class, false, false, null, false, [
162165
MapRequestPayload::class => new MapRequestPayload(),
163166
]);
164167
$request = Request::create('/', 'POST', $input);
@@ -171,27 +174,9 @@ interface DummySerializerDenormalizerInterface extends SerializerInterface, Deno
171174
{
172175
}
173176

174-
class DummyDenormalizer implements DenormalizerInterface
175-
{
176-
public function denormalize(mixed $data, string $type, string $format = null, array $context = []): mixed
177-
{
178-
return new \stdClass();
179-
}
180-
181-
public function getSupportedTypes(): array
182-
{
183-
return ['*' => false];
184-
}
185-
186-
public function supportsDenormalization(mixed $data, string $type, string $format = null, array $context = []): bool
187-
{
188-
return true;
189-
}
190-
}
191-
192177
class RequestPayload
193178
{
194-
public function __construct(public readonly int $quantity)
179+
public function __construct(public readonly float $price)
195180
{
196181
}
197182
}

‎src/Symfony/Component/HttpKernel/composer.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"symfony/deprecation-contracts": "^2.5|^3",
2121
"symfony/error-handler": "^6.3",
2222
"symfony/event-dispatcher": "^5.4|^6.0",
23-
"symfony/http-foundation": "^5.4.21|^6.2.7",
23+
"symfony/http-foundation": "^6.2.7",
2424
"symfony/polyfill-ctype": "^1.8",
2525
"psr/log": "^1|^2|^3"
2626
},

0 commit comments

Comments
 (0)