Skip to content

[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

Open
tsantos84 opened this issue May 16, 2025 · 3 comments

Comments

@tsantos84
Copy link
Contributor

tsantos84 commented May 16, 2025

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:

public function __invoke(
    #[MapRequestPayload] MyRequest $myRequest,
    #[MapUploadedFile] UploadedFile $myFile
) {
}

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:

class MyRequest {
    public ?string $foo = null;
    public ?UploadedFile $file = null;
}

This would allow the controller signature to be simplified to:

public function __invoke(#[MapRequestPayload] MyRequest $myRequest) {
}

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 injecting UploadedFile instances into DTOs during deserialization, allowing them to be mapped directly as properties.

Proposal requirements

  • It should allow to add validation constraints to files as well.
  • It should handle multi-level data like Symfony Form does.

Benefits

  • Cleaner controller method signatures
  • Better encapsulation of request data
  • Less boilerplate and manual mapping
  • More aligned with how form handling works

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 in MapRequestPayload 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.

@tsantos84
Copy link
Contributor Author

tsantos84 commented May 16, 2025

If I'm not wrong, to include this feature, we just need to change these lines:

if ($data = $request->request->all()) {
return $this->serializer->denormalize($data, $type, 'csv', $attribute->serializationContext + self::CONTEXT_DENORMALIZE + ('form' === $format ? ['filter_bool' => true] : []));
}

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 mergeParamsAndFiles method to merge $request->request and $request->files into only one array.

// 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;
}

Testing

The 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);
}

@tsantos84 tsantos84 changed the title RFC: Support Uploaded Files in RequestPayloadValueResolver Mapping RFC: Support UploadedFile when deserialize requests with #[MapRequestPayload] attribute May 16, 2025
@tsantos84 tsantos84 changed the title RFC: Support UploadedFile when deserialize requests with #[MapRequestPayload] attribute [HttpKernel] RFC: Support UploadedFile when deserialize requests with #[MapRequestPayload] attribute May 16, 2025
@94noni
Copy link
Contributor

94noni commented May 18, 2025

Hello

With code like this it works already
Of course need spécial declaration on attribute for the underlying resolver

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,
    ) {
    }
}
`ˋ`

@tsantos84
Copy link
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants