Problem/Motivation

I want to validate my business rules when submitting nodes through either form edit of REST POST/PATCH

When using REST hook_node_validate is not invoked as that is form tied.

Using hook_entity_presave seems the only possible hook to validate a REST submission. But that would lead to weird code checking whether submission is through REST or Form API throwing exception.

core/modules/node/node.api.php:410
function hook_node_validate(\Drupal\node\NodeInterface $node, $form, \Drupal\Core\Form\FormStateInterface $form_state) {

core/modules/system/entity.api.php:894
function hook_entity_presave(Drupal\Core\Entity\EntityInterface $entity) {

Validation is done in core/modules/rest/src/Plugin/rest/resource/EntityResource.php using \Drupal\rest\Plugin\rest\resource\EntityResource::validate which is not part of \Drupal\Core\Entity\EntityInterface but of core/lib/Drupal/Core/Entity/ContentEntityBase.php:22

This is probably strongly related to #1696648: [META] Untie content entity validation from form validation

Example

The example below only accepts only article titles less then 4 chars long leading to schizo code using the form validation to tell the form user and REST PATCH validation. It needs hook_node_validate AND hook_entity_presave() which is weird and inconsistent.

/**
 * Implements hook_node_submit().
 *
 * Article titles may no exceed 3 chars.
 */
function example_rest_validate_node_validate(\Drupal\node\NodeInterface $node, $form, \Drupal\Core\Form\FormStateInterface $form_state) {
  $node_type = $node->type->target_id;
  if ($node_type == 'article') {
    if (strlen($node->getTitle()) > 3) {
     $form_state->setErrorByName('title', t('Title must contain 3 or less characters.'));
    }
  }
}

/**
 * Implements hook_ENTITY_presave().
 *
 * Article titles may no exceed 3 chars.
 * Only trigger on node:article:PATCH method as Form validation
 * is done through example_rest_validate_node_submit()
 *
 * @see example_rest_validate_node_submit()
 */
function example_rest_validate_node_presave(Drupal\Core\Entity\EntityInterface $entity) {
  $method = \Drupal::request()->getMethod();
  if ($method == 'PATCH') {
    $entity_type = $entity->getEntityTypeId();
    if ($entity_type == 'node') {
      $entity_bundle = $entity->type->target_id;
      if ($entity_bundle == 'article') {
        if (strlen($entity->getTitle()) > 3) {
          // $form_state->setErrorByName('title', t('Title must contain 3 or less characters.'));
          // really throw an exception?
          throw new HttpException(422, t('Title must contain 3 or less characters.'));
        }
      }
    }
  }
}

This could be rewritten by a helper function accepting an $entity listing validation errors same as ContentEntityBase::validate() does but the need for two interception functions to either set form errors of throw an exception feels bad.

Proposed resolution

Remaining tasks

  • How to validate transparently using either Form API or REST?
  • How to report validation errors through REST?

User interface changes

API changes

Comments

clemens.tolboom’s picture

Issue summary: View changes
Issue tags: +API, +entity API

Added small example.

dawehner’s picture

Currently we validate purely on a typed data level. Ideally I guess, we would have a generic validator which fires the hook and you add more
violations manually in there.

clemens.tolboom’s picture

@dawehner thanks for the prompt reply.

Adding a generic validator sound like a great solution but:

- how does that solve the hook_node_validate() versus entity_presave() dilemma?
- the new hook_entity_ENTITY_TYPE_validate would add a new API feature

Shall I try a patch or is there another place for that?

dawehner’s picture

fago’s picture

I think for cases like that we need to have a way to add entity-level validation constraints - right now, we miss the place for adding them. I don't think there should be a hook or event aggregating the constraints, as constraints shouldn't be dynamically generated. If you need custom logic that's not part of an existing constraint, it should be added as a new constraint which is added non-dynamically.

See #2395831-4: Entity forms skip validation of fields that are not in the EntityFormDisplay for a proposal which contains adding entity-level constraints.

clemens.tolboom’s picture

Issue summary: View changes
Status: Closed (duplicate) » Active

I reopen this as it is still not clear to me hoe the DX would fall into place. I will close it again after the sprint.

As a contrib/module developer the different interfaces hook_node_validate and hook_ENTITY_presave is a unresolvable hurdleregarding validation error messages.

Using hook_entity_base_field_info_alter as suggested in #2105797-12: Add CompositeConstraintBase so that constraints involving multiple fields, such as CommentNameConstraint, can be discovered is not targeting my 'article' target but all bundles.

If both #2105797: Add CompositeConstraintBase so that constraints involving multiple fields, such as CommentNameConstraint, can be discovered and #2395831: Entity forms skip validation of fields that are not in the EntityFormDisplay land I would expect a Node API change of which I see no evidence.

clemens.tolboom’s picture

Status: Active » Closed (duplicate)

This is resolved through 8.0.0-beta7

CR https://www.drupal.org/node/2420295 [#2420295] hook_node_validate() and hook_node_submit() have been removed

@dawehner you were right about duplicate but I needed some more time to understand.

jcmiller09’s picture

Any plans to add a hook to alter the other rest endpoints? (register, login, logout, token, .etc)

clemens.tolboom’s picture

Closed issues (for 6 years) are not the place to ask for features. I guess one of https://www.drupal.org/community/contributor-guide/reference-information... are a good starting point