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?
Comments
Comment #1
clemens.tolboomAdded small example.
Comment #2
dawehnerCurrently 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.
Comment #3
clemens.tolboom@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?
Comment #4
dawehnerThis issue sounds like a duplicate of #2105797: Add CompositeConstraintBase so that constraints involving multiple fields, such as CommentNameConstraint, can be discovered
Comment #5
fagoI 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.
Comment #6
clemens.tolboomI 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.
Comment #7
clemens.tolboomThis 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.
Comment #8
jcmiller09 CreditAttribution: jcmiller09 commentedAny plans to add a hook to alter the other rest endpoints? (register, login, logout, token, .etc)
Comment #9
clemens.tolboomClosed 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