-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpKernel] RFC: Support UploadedFile when deserialize requests with #[MapRequestPayload] attribute #60440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
If I'm not wrong, to include this feature, we just need to change these lines: symfony/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php Lines 210 to 212 in 830918d
To: if ($request->request->count() || $request->files->count()) {
$data = $this->mergeParamsAndFiles($request->request->all(), $request->files->all());
return $this->serializer->denormalize($data, $type, 'csv', $attribute->serializationContext + self::CONTEXT_DENORMALIZE + ('form' === $format ? ['filter_bool' => true] : []));
} And add the // Extracted from \Symfony\Component\Form\Util\FormUtil::mergeParamsAndFiles
private function mergeParamsAndFiles(array $params, array $files): array
{
$isFilesList = array_is_list($files);
foreach ($params as $key => $value) {
if (\is_array($value) && \is_array($files[$key] ?? null)) {
$params[$key] = $this->mergeParamsAndFiles($value, $files[$key]);
unset($files[$key]);
}
}
if (!$isFilesList) {
return array_replace($params, $files);
}
foreach ($files as $value) {
$params[] = $value;
}
return $params;
} TestingThe following code is a passing unit tests for the proposed solution: public function testMapRequestPayloadWithUploadedFiles()
{
$serializer = new Serializer([
new ObjectNormalizer(propertyTypeExtractor: new PropertyInfoExtractor(typeExtractors: [new PhpDocExtractor()])),
new ArrayDenormalizer(),
]);
$validator = $this->createMock(ValidatorInterface::class);
$validator->expects($this->once())
->method('validate')
->willReturn(new ConstraintViolationList());
$resolver = new RequestPayloadValueResolver($serializer, $validator);
$argument = new ArgumentMetadata('valid', RequestPayloadWithUploadFile::class, false, false, null, false, [
MapRequestPayload::class => new MapRequestPayload(),
]);
$file1 = new UploadedFile(
self::FIXTURES_BASE_PATH.'/file-small.txt',
'file-small.txt',
'text/plain',
null,
true
);
$file2 = clone $file1;
$request = Request::create(
uri:'/',
method: 'POST',
parameters: [
'images' => [
['caption' => 'Image 1'],
['caption' => 'Image 2'],
],
],
files: [
'images' => [
['file' => $file1],
['file' => $file2],
]
],
server: ['CONTENT_TYPE' => 'multipart/form-data']
);
$kernel = $this->createMock(HttpKernelInterface::class);
$arguments = $resolver->resolve($request, $argument);
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);
$resolver->onKernelControllerArguments($event);
$arguments = $event->getArguments();
$this->assertInstanceOf(RequestPayloadWithUploadFile::class, $arguments[0]);
$this->assertTrue(is_array($arguments[0]->images));
$this->assertCount(2, $arguments[0]->images);
$this->assertSame('Image 1', $arguments[0]->images[0]->caption);
$this->assertInstanceOf(UploadedFile::class, $arguments[0]->images[0]->file);
$this->assertSame('Image 2', $arguments[0]->images[1]->caption);
$this->assertInstanceOf(UploadedFile::class, $arguments[0]->images[1]->file);
} |
Hello With code like this it works already You propose to directly map file to the native resolver I like this #[MapRequestPayload(resolver: DataUpdateDocumentResolver::class)] DataUpdateDocument $dataUpdateDocument The resolver create the object with request data as well as the file final readonly class DataUpdateDocument
{
public function __construct(
#[Assert\NotBlank]
public string $dataUuid,
#[Assert\NotNull]
#[Assert\File(
maxSize: '3M',
mimeTypes: [
'application/pdf',
'image/png',
'image/jpeg',
],
)]
public UploadedFile|File $file,
) {
}
}
`ˋ` |
@94noni, but this solution will require to create a resolver for each request in my app, right? Since this is a common scenario in many Symfony applications, I think it could be handled by Symfony's core. |
Description
Currently,
RequestPayloadValueResolver
only deserializes$request->request->all()
into the target class. This behavior requires uploaded files to be mapped separately using another argument with the#[MapUploadedFile]
attribute:This separation forces developers to manually split file handling from the main request payload, even when the uploaded files conceptually belong to the same data structure.
It would be much more ergonomic and intuitive if the uploaded file could be included directly within the request DTO. For example:
This would allow the controller signature to be simplified to:
While it might be possible to work around this with custom denormalizers (not yet tested), I believe this use case is common enough to warrant native support in Symfony core.
Proposal
Extend
RequestPayloadValueResolver
(and possibly the serializer) to support injectingUploadedFile
instances into DTOs during deserialization, allowing them to be mapped directly as properties.Proposal requirements
Benefits
Looking forward to hearing your thoughts and if this could be a candidate for inclusion in a future Symfony release.
Something to consider
Allowing
UploadedFile
to be included inMapRequestPayload
could set a precedent for supporting other request sources, such as query strings and headers. If that happens, we might need to define a clear priority order for how data from different parts of the request (body, query, headers, files) should be merged.The text was updated successfully, but these errors were encountered: