Skip to content
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

Partial update and constraint validations #1637

Open
Muspi opened this issue Aug 31, 2020 · 6 comments
Open

Partial update and constraint validations #1637

Muspi opened this issue Aug 31, 2020 · 6 comments

Comments

@Muspi
Copy link

Muspi commented Aug 31, 2020

Hi,

I have an entity with many fields, and I have to allow a partial update (PATCH action) via API Platform of some fields.
Depending on front interface, any field of this entity will be updated or not.

I added some constraint on these fields, via Assert annotations.

Problem is when I execute my patch action, all constraints validation are executed, even if field is not present in my request.
Is there a solution to ignore constraints for non defined fields?

I would like to avoid create many validation groups, because it creates a constraint relation between front office/ back office.

Thanks for your help.

@GHuygen
Copy link

GHuygen commented Aug 31, 2020

In a project i'm working we handled it this way:

We created a class that creates groups for every attribute send in the request:

<?php

namespace App\Validator\Groups;

use ApiPlatform\Core\Api\FormatMatcher;
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface;
use ApiPlatform\Core\Util\RequestAttributesExtractor;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpKernel\Exception\UnsupportedMediaTypeHttpException;
use Symfony\Component\Serializer\SerializerInterface;

/**
 * Class RequestPayloadGroupGenerator
 * @package App\Validator\Groups
 *
 * Automatically generates validation groups based upon current request payload and resource class
 * which is useful for partial/patch updates.
 *
 * Naming convention for group is as follows:
 * [request_method]:[attribute_name]
 *
 * Only simple attributes are allowed, nested objects as well as collections are skipped.
 *
 * @example
 * When a patch Client's request with payload: { "iban": "123456", "note": "my note" } is sent,
 * then following validation groups are generated:
 * ["patch:iban", "patch:node"]
 *
 * Then on given Entity you can setup validation group like:
 * `[@]Assert\NotNull(groups={"Default", "patch:iban"})`,
 * so that only updated values are being validated.
 *
 */
class RequestPayloadGroupGenerator
{
    private $serializer;
    private $requestsStack;
    private $resourceMetadataFactory;
    private $serializerContextBuilder;

    public function __construct(
        SerializerInterface $serializer,
        RequestStack $requestStack,
        ResourceMetadataFactoryInterface $resourceMetadataFactory,
        SerializerContextBuilderInterface $serializerContextBuilder
    ) {
        $this->serializer = $serializer;
        $this->requestsStack = $requestStack;
        $this->resourceMetadataFactory = $resourceMetadataFactory;
        $this->serializerContextBuilder = $serializerContextBuilder;
    }

    public function __invoke(): array
    {
        $request = $this->requestsStack->getMasterRequest();
        $method = strtolower($request->getMethod());
        $groups = [];
        $attributes = RequestAttributesExtractor::extractAttributes($request);
        $context = $this->serializerContextBuilder->createFromRequest($request, false, $attributes);
        $formats = $this
            ->resourceMetadataFactory
            ->create($attributes['resource_class'])
            ->getOperationAttribute($attributes, 'input_formats', [], true);
        $format = $this->getFormat($request, $formats);
        $inputClass = $context['input']['class'] ?? $attributes['resource_class'];
        $payload = $this->serializer->deserialize($request->getContent(), $inputClass, $format, $context);

        $entityAttributes = (array) array_filter((array) $payload, function ($item) {
            return !(is_object($item) || is_null($item));
        });

        $groups[] = $method; // post, put or patch

        foreach ($entityAttributes as $name => $attribute) {
            $groups[] = sprintf(
                "%s:%s", // put:[attribute] or patch:[attribute], etc..
                $method,
                trim(str_replace($inputClass, '', $name))
            );
        }

        return $groups;
    }

    /**
     * Extracts the format from the Content-Type header and check that it is supported.
     *
     * @throws UnsupportedMediaTypeHttpException
     */
    private function getFormat(Request $request, array $formats): string
    {
        /**
         * @var string|null
         */
        $contentType = $request->headers->get('CONTENT_TYPE');
        if (null === $contentType) {
            throw new UnsupportedMediaTypeHttpException('The "Content-Type" header must exist.');
        }

        $formatMatcher = new FormatMatcher($formats);
        $format = $formatMatcher->getFormat($contentType);
        if (null === $format) {
            $supportedMimeTypes = [];
            foreach ($formats as $mimeTypes) {
                foreach ($mimeTypes as $mimeType) {
                    $supportedMimeTypes[] = $mimeType;
                }
            }

            throw new UnsupportedMediaTypeHttpException(sprintf('The content-type "%s" is not supported. Supported MIME types are "%s".', $contentType, implode('", "', $supportedMimeTypes)));
        }

        return $format;
    }
}

And then you can add the generated groups to the validator:

/**
 * @ApiResource(
 *     itemOperations={
 *         "patch"={
 *              "validation_groups"=RequestPayloadGroupGenerator::class
 *         }
 *     }
 * )
 */

But you still have to add every constraint to the expected validation group.

    /**
     * @Assert\Type(type="boolean", groups={"patch:hasAcceptedGdpr"})
     */
    private $hasAcceptedGdpr;

@Muspi
Copy link
Author

Muspi commented Aug 31, 2020

Thanks @GHuygen for these answer, it could work as I want.
I'm surprised there is no way to achieve this basically.

@dunglas, Is it because this is a bad practice?

@TekPike
Copy link

TekPike commented Oct 25, 2021

EDIT: I had a similar problem with the DateTime constraint.
Just replace Assert\Datetime() by Assert\Type(type: datetime) (cf: symfony/symfony#35378)

=============================================================================

The @GHuygen solution works, but this not a solution, this is DIY.

Patch requests are supposed to ignore unsent fields, and therefore not trigger their validation.
The problem you share is not a bad practice but a malfunction of ApiPlatform.
This need a fix.

@carlobeltrame
Copy link

The Symfony Validators on one field also have access to the values of all other fields. This is necessary for some stock validations (e.g. GreaterThan with propertyPath specified) as well as some custom validation use cases. Imagine a validator that checks that exactly one of two fields must be null. Or imagine a currency-specific amount validation, which should validate that an "amount" field is always rounded to the nearest smallest coin value, i.e. 0.01 or 0.05 or only whole numbers, depending on the currency. For this purpose, Symfony simply exposes all fields to the Validator, and Symfony has no way of knowing which validation constraints take into account which fields exactly.

So in the general case, validations don't just concern one field, and therefore we can't automatically do any better than running all validations even on partial PATCH. After all, changing just the "currency" field may make the "amount" field invalid implicitly. At least not as long as Symfony doesn't introduce something like "pure validations" which don't have access to any other fields, or any kind of dependency tracking for validations.

Personally I embrace the current status, because it gives me stronger guarantees that the objects saved into the database are always valid, and because the validation groups give me enough flexibility for any use case I have come across so far.

@vince83110
Copy link

I agree with @carlobeltrame , the Symfony validation system ensures the validity of your whole entity, not only your request. I can't see any business logic where you would accept to patch / update a non-valid entity.

I would just think about performances, but it's a small effort.

@stefanides
Copy link

I have met the similar situation, this also applies when creating/patching entity and e.g. (in some scenarios) selected fields are filled otherwise, like set user/owner/email etc. from logged in user. The workaround could be to turn off the validation on the API deserialization step (validate: false) and call it later, i.e. in prePersist()/preUpdate() like

namespace App\EntityListener;

use Doctrine\ORM\Event\LifecycleEventArgs;
use Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener;
use Symfony\Component\Validator\Validator\ValidatorInterface;
use App\Entity\User;
...

#[AsEntityListener(event: Events::prePersist,  method: 'prePersist', entity: User::class)]

class UserListener
{
    public function __construct(
        private ValidatorInterface $validator,
    )
    {}

    public function prePersist(User $user, LifecycleEventArgs $args)
    {
        $errors = $this->validator->validate($user);
        ...
    }
}

and format validation's result in ValidationException() as does e.g. ApiPlatform\Symfony\EventListener\DeserializeListener::onKernelRequest()

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

6 participants