Validation from DTO before resolver

[codedge]

API Platform version(s) affected: 3.3.3

Description
With the latest 3.3.3 release the validation within a DTO is not run before the data reaches the resolver. The change is, I guess this one:

• fix(graphql): resolver before validation: https://github.com/api-platform/core/pull/6363

What is the reasoning behind or how should this be handled now? I usually validate data before reaching the resolver.

[NicoHaase]

See https://github.com/api-platform/core/issues/6354 for my bug report about this

[soyuka]

I'm not sure about what should be done here, @NicoHaase does #6363 solves your issue? I've no strong opinion about this but I would like to keep the same behavior as in 3.2, this is an untested behavior so we didn't really planned anything. @codedge you're also resolving a mutation? Can we try to compare the use cases?

[NicoHaase]

@soyuka Yes, my issue was resolved - thanks for your quick response. On the other hand, I faced a different issue where validation was now run on deleting an entity.

@codedge can you share more details about your problem? How exactly do you validate the data?

[codedge]

Here are my details:

I have a custom resolver, that is filled with a custom DTO. Inside the DTO a run a validation of a property (GreaterThan(value: 0)). Before the change, the validation hit, before the data reached the resolver.

Entity

<?php

declare(strict_types=1);

namespace App\Entity;

#[
    Api\ApiResource(
        operations: [],
        graphQlOperations: [
            new GraphQl\Mutation(
                resolver: SampleResolver::class,
                input: MySampleDto::class,
                name: 'create',
            ),
        ]
    )
]
class SampleRequest implements CustomInterface
{

}

DTO

<?php

declare(strict_types=1);

namespace App\Dto;

use Symfony\Component\Validator\Constraints as Assert;

final class MySampleDto
{
    #[Assert\GreaterThan(value: 0)]
    public int $amount;
}

Resolver

final readonly class CreateBurnRequestResolver implements MutationResolverInterface
{
    public function __construct(
        private ManagerRegistry $managerRegistry,
    ) {}

    public function __invoke(?object $item, array $context): ?object
    {
        if (!$item instanceof MySampleDto) {
            return null;
        }

        // Not being executed if amount is 0
        ;
    }

[soyuka]

Yes, my issue was resolved - thanks for your quick response. On the other hand, I faced a different issue where validation was now run on deleting an entity.

This is wrong something must be missing to set canValidate with graphql I'll attempt a fix Friday.

About this issue I don't understand how on one case validation ran, on the other it did not. The only difference I see is that there's an input in this case?

We all agree that the resolver shouldn't be hit if validation fails?

[NicoHaase]

@soyuka sorry for confusing: the issue "validation on delete" happened on another entity type, and I'll try to craft a test case for it

[soyuka]

I'd like to fix this tomorrow would be awesome if I could have a test case tyvm!

[NicoHaase]

https://github.com/api-platform/core/compare/3.3...NicoHaase:core:fix-delete-validation contains a first way of testing this.

[codedge]

We all agree that the resolver shouldn't be hit if validation fails?

yes

[soyuka]

thanks, this makes more thanks sorry for the trouble, will be patched today.

[soyuka]

After looking into it it looks like in 3.2 the mutation resolver runs before validation:

https://github.com/api-platform/core/blob/9a8adfffd67f877f9c63d30be818903157d23b8c/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php#L82-L90

Validation only runs later at:

https://github.com/api-platform/core/blob/9a8adfffd67f877f9c63d30be818903157d23b8c/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php#L100

I'm confused I must have failed reproducing that behavior at afb739b392d337159f6475f7c3144cbac74b1067 when the backward compatibility is disabled.

I think that the best would be to add a note to the changelog and to the migration guides about this when EVENT_LISTENERS_BACKWARD_COMPATIBILITY_LAYER is on/off. IMO validation should be done on the user input.

I'll revert https://github.com/api-platform/core/pull/6363 and fix the delete mutation not needing validation.

[NicoHaase]

Please keep in mind that reverting #6363 will also re-open #6354, as the behaviour reported there (which was fixed with #6363) is then again broken. My problem with deleting entities (where the validation was run, but probably shouldn't) is not related to the mutation problem I've reported initially

[soyuka]

@NicoHaase the problem with valiadating a delete mutation is fixed right?

[NicoHaase]

Yes, that problem is fixed after updating to 3.3.5. Thank you for asking 🙌