2

In a fully decoupled drupal 9 project I have a custom entity type and added a unique constraint for several fields just as described here. This works well and adding a second entity with the same field values is not possible. However, I'm using JSONAPI POST requests to create the entities. I noticed that when issuing multiple POST requests with the exact same field values right after another, the validator method (using the entityTypeManager->getStorage(...)->getQuery(...)->condition(...)->execute() to check the DB) does not return other entities, as no duplicate entity exists yet. I.e. it happens so fast that multiple identical-valued entities are created at the exact same timestamp (The created values of the entities are identical)!

Bypassing the constraint is dangerous and must be prevented.

What can I do to solve this problem?

Update This is the function that is called inside the ConstraintValidator

public function validate($entity, Constraint $constraint)
{
  ...
  if (!$this->isUnique($entity))
    $this->context->addViolation($constraint->notUnique);
  ...
}
private function isUnique(CustomType $entity) {
  $date = $entity->get('date')->value;
  $type = $entity->bundle();
  $employee = $entity->get('employee')->target_id;
  $query = $this->entityTypeManager->getStorage('custom_type')->getQuery()
    ->condition('status', 1)
    ->condition('type', $type)
    ->condition('employee', $employee)
    ->condition('date', $date);

  if (!is_null($entity->id()))
    $query->condition('id', $entity->id(), '<>');

  $workIds = $query->execute();
  return empty($workIds);
}

I'm happy to find any flaws. So far this code works well in all other cases.

Update Drupal::lock()

I've implemented 2 event subscribers to add and release \Drupal::lock() as mentioned in the comments. Using xdebug I can confirm that the code is run, however, the lock does not seem to have any effect. The documentation for lock() is rather limited. Not sure what's wrong here.

<?php

namespace Drupal\custom_entities\EventSubscriber;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\HttpKernel\KernelEvents;

class JsonApiRequestDBLock implements EventSubscriberInterface {

  /**
   * Adds a lock for JSON:API requests.
   *
   * @param \Symfony\Component\HttpKernel\Event\RequestEvent $event
   *   The event to process.
   */
  public function onRequest(RequestEvent $event) {
    $request = $event->getRequest();
    if ($request->getRequestFormat() !== 'api_json') {
      return;
    }

    if ($request->attributes->get('_route') === 'jsonapi.custom_type--work.collection.post' &&
      $request->attributes->get('_controller') === 'jsonapi.entity_resource:createIndividual'
    ) {
      $lock = \Drupal::lock();
      $lock->acquire('custom_create_lock');
    }

  }

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents() {
    $events[KernelEvents::REQUEST][] = ['onRequest'];
    return $events;
  }

}

and release the lock after the response

<?php

namespace Drupal\custom_entities\EventSubscriber;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

class JsonApiResponseDBRelease implements EventSubscriberInterface {

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents() {
    $events[KernelEvents::RESPONSE][] = ['onResponse'];
    return $events;
  }


  /**
   * Release JSON:API responses.
   *
   * @param \Symfony\Component\HttpKernel\Event\ResponseEvent $event
   *   The event to process.
   */
  public function onResponse(ResponseEvent $event) {
    $response = $event->getResponse();
    if (strpos($response->headers->get('Content-Type'), 'application/vnd.api+json') === FALSE) {
      return;
    }
    $request = $event->getRequest();
    if ($request->attributes->get('_route') === 'jsonapi.custom_type--work.collection.post' &&
      $request->attributes->get('_controller') === 'jsonapi.entity_resource:createIndividual'
    ) {
      // Release the lock.
      $lock = \Drupal::lock();
      if (!$lock->lockMayBeAvailable('custom_create_lock'))
        $lock->release('custom_create_lock');
    }
  }

}

This was added to the services.yml

  # Event subscribers.
  custom_entities.jsonapi_db_lock.subscriber:
    class: Drupal\custom_entities\EventSubscriber\JsonApiRequestDBLock
    tags:
      - { name: event_subscriber }
  custom_entities.jsonapi_response_db_release.subscriber:
    class: Drupal\custom_entities\EventSubscriber\JsonApiResponseDBRelease
    tags:
      - { name: event_subscriber }
8
  • That doesn't seem like it should happen, as it would be almost impossible to save two entities at the exact same time, one has to happen before the other, so the second should get caught by the constraint. Are you sure your constraint code is correct?
    – Jaypan
    Dec 30 '21 at 16:55
  • I do agree and I'm rather confused at the moment. I added a \Drupal::logger statement inside the isUnique(), logging the current time. It gets called multiple times at the exact same second
    – theuni
    Dec 30 '21 at 17:29
  • The $this->entityTypeManager->getStorage('custom_type')->getQuery() line is missing a call to accessCheck(FALSE), which is necessary for the query to ignore any access permission the logged-in user may have for the queried entities.
    – apaderno
    Dec 30 '21 at 21:05
  • 1
    I don't think this is the issue here. It's about concurrent requests. As @Jaypan noted it's very unlikely that the same field data is submitted twice within a second or two. But if this can happen you need some kind of locking mechanism. It's probably not possible to make a database lock on the tables involved, so you need an independent locking mechanism like api.drupal.org/api/drupal/…
    – 4k4
    Jan 1 at 10:49
  • 1
    You need to apply the lock on the entire request, not only the constraint. Either by overriding the controller or by using event subscribers, KernelEvents::REQUEST to acquire the lock, KernelEvents::RESPONSE to release the lock and KernelEvents::EXCEPTION to handle exceptions raised because of the lock.
    – 4k4
    Jan 3 at 7:58
2

Without seeing all the code used for the custom_type entity, including the code for its handlers, and answering to why the code doesn't find duplicates, there are two "flaws" I can imagine possible in the shown code.

The first "flaw" is that the query searching for existing entities is more restrictive than it should. This means it checks for entity fields that should be irrelevant for the entities to be duplicates, for example:

  • The status field, assuming it's the status field used by the Drupal core entities
  • The date field, assuming it contains the creation date/timestamp

The only Drupal core entity that uses an entity validation to avoid duplicate entities are created is the path_alias entity, implemented by the PathAlias class. That entity has a status field, it supports revisions, but it doesn't have a field to store when it has been created, nor does it have an owner entity (like nodes do).
UniquePathAliasConstraintValidator is its entity constraint validator; the code for UniquePathAliasConstraintValidator::validate() is the following one.

  $path = $entity->getPath();
  $alias = $entity->getAlias();
  $langcode = $entity->language()->getId();
  $storage = $this->entityTypeManager->getStorage('path_alias');
  $query = $storage->getQuery()
    ->accessCheck(FALSE)
    ->condition('alias', $alias, '=')
    ->condition('langcode', $langcode, '=');
  if (!$entity->isNew()) {
    $query->condition('id', $entity->id(), '<>');
  }
  if ($path) {
    $query->condition('path', $path, '<>');
  }
  if ($result = $query->range(0, 1)->execute()) {
    $existing_alias_id = reset($result);
    $existing_alias = $storage->load($existing_alias_id);
    if ($existing_alias->getAlias() !== $alias) {
      $this->context->buildViolation($constraint->differentCapitalizationMessage, ['%alias' => $alias, '%stored_alias' => $existing_alias->getAlias()])
        ->addViolation();
    }
    else {
      $this->context->buildViolation($constraint->message, ['%alias' => $alias])
        ->addViolation();
    }
  }
}

Using that code as example, and using only the code that strictly checks for duplicates, in your case I would use the following code. (I am showing only the code for isUnique().)

private function isUnique(CustomType $entity) {
  $date = $entity->get('date')->value;
  $type = $entity->bundle();
  $employee = $entity->get('employee')->target_id;
  $query = $this->entityTypeManager->getStorage('custom_type')
    ->getQuery()
    ->accessCheck(FALSE)
    ->condition('type', $type)
    ->condition('employee', $employee)
    ->condition('date', $date);

  if (!$entity->isNew()) {
    $query->condition('id', $entity->id(), '<>');
  }

  $result = $query->range(0, 1)->execute();
  return empty($result);
}

I added the call to accessCheck(FALSE) because, without seeing the code used by the entity access handler, and without knowing if the entity has an owner entity, I cannot exclude the implementation of isUnique() isn't finding duplicates because the currently logged-in user doesn't have access to the duplicates (or any entity of that type). Without calling accessCheck(FALSE), the query will return the entities to which the currently logged-in user has access.
(The missing call to accessCheck(FALSE) is the other, possible, "flaw" I can see in the shown code.)

4
  • Thanks! I've tried this code, played around and tested many times. However, the actual issue remains, with 2 concurrent requests my validator does not prevent the duplicated entities. I'm digging deeper and keep this issue up to date.
    – theuni
    Jan 3 at 7:21
  • Is the date field used by your code like the created field used for nodes? If that is the case, I would not search for existing entities with the same value for date.
    – apaderno
    Jan 3 at 10:50
  • not quite sure what exactly you mean but the date is highly important for the uniqueness check
    – theuni
    Jan 5 at 9:58
  • I mean that if that field contains when the entity has been created, I would not include it in the query to find duplicates. Clearly, if the entity is for events, for example, and date is the event scheduled date, then I would include it.
    – apaderno
    Jan 5 at 11:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.