Problem/Motivation

This:

  1. is blocked on #2862574: Add ability to track an entity object's dirty fields (and see if it has changed)
  2. was raised in #2821077-11: PATCHing entities validates the entire entity, also unmodified fields, so unmodified fields can throw validation errors
  3. blocks simplifying \Drupal\rest\Plugin\rest\resource\EntityResourceValidationTrait

If ContentEntityBase::validate() would only validate fields that were actually modified, then #2821077: PATCHing entities validates the entire entity, also unmodified fields, so unmodified fields can throw validation errors wouldn't have had to add \Drupal\rest\Plugin\rest\resource\EntityResourceValidationTrait::validateWithFilteredFields(), which is necessary to only validate fields that were actually modified by a PATCH request.

Proposed resolution

  1. Update ContentEntityBase::validate() to only validate dirty fields
  2. Remove EntityResourceValidationTrait::validateWithFilteredFields()

Remaining tasks

  1. Blocker: #2862574: Add ability to track an entity object's dirty fields (and see if it has changed).
  2. Discuss + implement.

User interface changes

None.

API changes

ContentEntityBase::validate() would only validate fields that were actually changed.

Data model changes

None.

Comments

Wim Leers created an issue. See original summary.

amateescu’s picture

amateescu commented

Wouldn't this open up a way to save invalid entities when only an entity field is changed and another field will be made invalid by this change, for example in a composite constraint?

amateescu’s picture

amateescu commented

In fact, we have a very nice API for filtering violations:

$violations = $entity->validate();
$violations->filterByFields($some_fields);

So what is this issue trying to solve?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#3

Yes, #2821077: PATCHing entities validates the entire entity, also unmodified fields, so unmodified fields can throw validation errors is using filterByFields(). The problem is that it puts the burden on the calling code to know which fields are being modified (i.e. the calling code must filter the violations, and pass in $some_fields).

But Entity/Field API in fact is able to figure this out for itself — it has all the necessary information!

It's just not tracking yet which fields have actually been modified and therefore need to be validated. So once it is tracking which fields have been modified AKA which fields are dirty (#2862574: Add ability to track an entity object's dirty fields (and see if it has changed)), we can make $entity->validate() automatically do filterbyFields($dirty_fields) internally.

That would remove this complexity from rest.module and jsonapi.module, as well as any custom code that is modifying entities.

#2:

Wouldn't this open up a way to save invalid entities when only an entity field is changed and another field will be made invalid by this change, for example in a composite constraint?

I thought about this too. But isn't that what \Drupal\Core\Entity\ContentEntityForm::validateForm() + \Drupal\Core\Entity\ContentEntityForm::getEditedFieldNames() is already doing anyway? Therefore it should be safe? (See #2821077-9: PATCHing entities validates the entire entity, also unmodified fields, so unmodified fields can throw validation errors by @tstoeckler.)

For example: node body uses a certain format that user A is not allowed to use, but when user A PATCHes only the node title, they still get a validation error.

dawehner’s picture

dawehner
Primary language German
commented

I personally believe there are different usecase. You might want to validate the entire entity vs. just the changed fields ... so we either keep the status quo or flip it around, so you can retrieve all the errors, if you still like.

amateescu’s picture

amateescu commented
Status: Postponed » Closed (works as designed)

Re #4:

The problem is that it puts the burden on the calling code to know which fields are being modified (i.e. the calling code must filter the violations, and pass in $some_fields).

The calling code already knows whether it should filter out violations for a subset of fields or not. Here are various examples:

API calls

$entity = $storage->create([...]);
$entity->save();

When we create and then save an entity "manually", we know that we're working with the full set of fields from that entity, so we need to validate it entirely.

Entity forms

Content entity forms know that they're (possibly) working with only a subset of fields (specified by the form display), so they need to filter out violations for fields that are not edited by the form.

REST POST requests

POST requests know that they're working with full entity objects, so, just like API calls, they need to validate the entity with its entire set of fields.

REST PATCH requests

PATCH requests by definition work with a subset of fields from an entity, so they know that they need to filter out violations to include only those that are relevant to the fields that are being changed, along with the "entity level" one which might validate one of the changed fields in combination with a field that was not changed.

That would remove this complexity from rest.module and jsonapi.module, as well as any custom code that is modifying entities.

As explained above, there is no complexity in rest.module and jsonapi.module, because their various request types should already know if they are working with a full entity or only a subset of fields from it.

In conclusion, this change would actually make the system more brittle IMO, because you'll never know what $entity->validate() decides to give you, a full set of violations or a filtered list of them.