Skip to content

#[MapRequestPayload] does not work if a DTO contains **only** nested DTOs #50708

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.

Sign up for GitHub

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

Jump to bottom
Closed
someniatko opened this issue Jun 19, 2023 · 7 comments
Closed

#[MapRequestPayload] does not work if a DTO contains **only** nested DTOs #50708

someniatko opened this issue Jun 19, 2023 · 7 comments
Labels
Bug HttpKernel Status: Needs Review

Comments

@someniatko
Copy link

someniatko commented Jun 19, 2023 •
edited
Loading

Symfony version(s) affected

6.3.0+

Description

If you use #MapRequestPayload attribute on a DTO whose only field is a nested DTO, Symfony crashes on the container compilation stage.

How to reproduce

(Also see https://github.com/someniatko/symfony-50708-reproducer)

Consider a following situation:

class ParentDto
{
    public function __construct(public NestedDto $nested) {}
}

class NestedDto
{
    public function __construct(public string $value) {}
}

class Controller extends AbstractController
{
    #[Route(path: [ '/path' ], methods: 'POST')]
    public function index(#[MapRequestPayload] ParentDto $dto): Response
    {
        return $this->json($dto);
    }
}

Then execute php bin/console cache:clear for any environment.

Expected: Cache clears successfully

Actual:

In DefinitionErrorExceptionPass.php line 51:
                                                                                                                                                                
  Cannot autowire service "App\Dto\NestedDto": argument "$value" of method "__construct()" is type-hinted "string", you should configure its value explicitly.

It took me almost four hours to pinpoint the issue, because I thought maybe some of the bundles I use are the culprit, or my config is incorrect, and also because, and it's the most "funny" part, this will work fine:

class ParentDto
{
    public function __construct(public string $additionalValue, public NestedDto $nested) {}
}

class NestedDto
{
    public function __construct(public string $value) {}
}

Possible Solution

No response

Additional Context

No response
@someniatko someniatko added the Bug label Jun 19, 2023
@HypeMC
Copy link
Contributor

HypeMC commented Jun 19, 2023

The problem is that you haven't excluded your DTOs from being automatically loaded by the container. You can either add an exclude option to your services.yaml or use the new #[Exclude] attribute.

@ro0NL
Copy link
Contributor

ro0NL commented Jun 19, 2023

unreferenced services that are broken due autodiscovery should not be an issue, but im afraid controller.service_arguments detects it as a service and thus creates a reference

@ghost
Copy link

ghost commented Jun 19, 2023

The problem is that you haven't excluded your DTOs from being automatically loaded by the container. You can either add an exclude option to your services.yaml or use the new #[Exclude] attribute.

I think argument with this attribute should not be considered by DI as a service.

@MatTheCat
Copy link
Contributor

A reference is created by the RegisterControllerArgumentLocatorsPass which feeds the ServiceValueResolver its locator. ParentDto invalid behavior is RUNTIME_EXCEPTION_ON_INVALID_REFERENCE because of that (see #26627) but its $nested argument’s is EXCEPTION_ON_INVALID_REFERENCE, so the DefinitionErrorExceptionPass will throw the error coming from the AutoWirePass.

Adding $additionalValue to ParentDto’s constructor will error the service, which means the exception would only be thrown if it is accessed.

Excluding the DTOs from the container would indeed work, but it feels like the compilation should not fail anyways 🤔

@nicolas-grekas
Copy link
Member

The hints shared in this discussion are good ones, but I didn't manage to reproduce the issue by just creating the 3 classes in the description.
Please provide a reproducer.

@someniatko
Copy link
Author

someniatko commented Jun 22, 2023 •
edited
Loading

https://github.com/someniatko/symfony-50708-reproducer

UPD: I've excluded irrelevant bundles from the reproducer

@nicolas-grekas nicolas-grekas mentioned this issue Jun 23, 2023
Merged
@someniatko
Copy link
Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpKernel Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@nicolas-grekas @ro0NL @MatTheCat @xabbuh @HypeMC @carsonbot @someniatko