Support optimistic locking

[backbone87]

Basically the optimistic locking support is almost there. You can add a version field with getter/setter to your entity and you can add it to de/normalization. the only thing missing is to make it required.

a quick fix would be to add a listener that sets the version field to null after ReadListener, but before the DenormalizeListener, so if you dont provide a version, then doctrine will fail to update and throws an optimistic lock exception. Furthermore the lock exception can be set to 400 response code. And you can add a validation constraint to the version to have nice error messages.

[soyuka]

You could use a priority of 3 on a kernel.request event to do so, no?

  #14     ApiPlatform\Core\EventListener\ReadListener::onKernelRequest()                                    4
  #15     ApiPlatform\Core\EventListener\DeserializeListener::onKernelRequest()                             2

[backbone87]

Well, ofc i can create the listener myself (currently i use another way to ensure the version gets set). the question is, if optimistic locking is something to provide out of the box and if so, what is the best way to implement it.

[dunglas]

I'll be glad to merge such feature (as long as it is optional).

[backbone87]

i can make a PR, but i want to discuss different approaches before. the problem is described in the OP: How to make sure the version is submitted?

edit: using a listener to force set the version of the fetched entity to null seems more like a hack to me.

[markusgillmeister]

Is there any progress on this feature? Cause I'm very interested in providing "optimistic locking" in my api. And its quite easy with doctrine's "version" annotation. The only problem is that API Platform should enforce the api user to send the version field within the PUT-Request (and not use it's old value).

[teohhanhui]

IMO we should have another validation listener before deserialize. It makes sense to use a different validation group.

[teohhanhui]

But perhaps it is better if we can specify such constraints in the Symfony Serializer?

[OskarStark]

Any news here? I am facing the same issue.

@teohhanhui could you elaborate what you mean by changing the priority?

[teohhanhui]

@teohhanhui could you elaborate what you mean by changing the priority?

I didn't mention anything about priority?

[OskarStark]

Ah sorry, I mixed things up!
I meant @soyuka 😄

[teohhanhui]

I think the gist of it is, you could use a custom event listener to enforce that the version property must always be sent.

[OskarStark]

Ok, but when I GET a response containing version=1 and you mean I should increase it every time by 1 and then send it back (if PUT, POST)?

[bastnic]

FYI, we did it on one of our API. We did manually with a Doctrine\Common\EventSubscriber but there is a way to do it directly with doctrine: https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/reference/transactions-and-concurrency.html.

on each PUT / PATCH requests, we increment the version and send it back. The field is mandatory and if you send a version lower than the one in database, we throw a 409 http error.

A small warning, we use Knp\DoctrineBehaviors\Model\Translatable and we got some trouble where changing just a translation didn't update the version of the parent object. (and so, it didn't triggered the varnish cache purge :'(). To fix that we changed a bit out way to do it to finish to that.
The tricky part is 1/ getting the parent 2/ change the version only once (if you change multiple translations, it's only one version increase) 3/ mark the parent entity as changed with recomputeSingleEntityChangeSet.
You will see that all entities do not need to be versionned, and that we use an interface for that.

I would be glad to have you opinion on that piece of code. It's do the job but it's quite naive for now.

class VersionSubscriber implements EventSubscriber
{
    /** @var array */
    protected $isProcessed = [];

    public function getSubscribedEvents()
    {
        return [
            Events::onFlush,
        ];
    }

    public function onFlush(OnFlushEventArgs $args)
    {
        $em = $args->getEntityManager();

        foreach ($em->getUnitOfWork()->getScheduledEntityUpdates() as $entity) {
            // get an entity (itself or its Translatable && Versionable parent
            if (!$entity instanceof VersionInterface) {
                if (in_array(Translation::class, array_keys(class_uses($entity)))) {
                    $entity = $entity->getTranslatable();
                }

                if (!$entity instanceof VersionInterface) {
                    continue;
                }
            }

            // avoid setting twice a version if multiple translations/relations are added
            if (isset($this->isProcessed[spl_object_hash($entity)])) {
                return;
            }

            $this->checkVersion($em, $entity);
            $this->updateVersion($entity);

            $classMetadata = $em->getClassMetadata(get_class($entity));
            $em->getUnitOfWork()->recomputeSingleEntityChangeSet($classMetadata, $entity);
        }
    }

    protected function checkVersion(EntityManagerInterface $entityManager, VersionInterface $entity)
    {
        $changeSet = $entityManager->getUnitOfWork()->getEntityChangeSet($entity);

        if (array_key_exists('version', $changeSet)) {
            throw new VersionConflictException(Response::HTTP_CONFLICT, sprintf('Version conflict. Please verify your version (current: "%s")', $changeSet['version'][0]));
        }
    }

    protected function updateVersion(VersionInterface $entity)
    {
        $entity->setVersion($entity->getVersion() + 1);

        $this->isProcessed[spl_object_hash($entity)] = true;
    }
}

[teohhanhui]

Ok, but when I GET a response containing version=1 and you mean I should increase it every time by 1 and then send it back (if PUT, POST)?

No, actually, you don't increment the version when using Doctrine ORM's optimistic locking: https://www.doctrine-project.org/projects/doctrine-orm/en/current/reference/transactions-and-concurrency.html#optimistic-locking

Doctrine has integrated support for automatic optimistic locking via a version field. In this approach any entity that should be protected against concurrent modifications during long-running business transactions gets a version field that is either a simple number (mapping type: integer) or a timestamp (mapping type: datetime). When changes to such an entity are persisted at the end of a long-running conversation the version of the entity is compared to the version in the database and if they don't match, an OptimisticLockException is thrown, indicating that the entity has been modified by someone else already.

You should send back the same version value as what you've received from the GET request.