[PP-1] Normalizers must set $entity->_restSubmittedFields for entity POSTing/PATCHing to work

Problem/Motivation

In #2451397: Entity denormalization fails to retrieve bundle → Drupal\serialization\Normalizer\EntityNormalizer::denormalize() had to add _restSubmittedFields to the entity object so that Drupal\rest\Plugin\rest\resource\EntityResource::post() and ::patch() could work at all.

That work-around sets a new, undefined, public-but-for-internal-use-only property on Entity objects:

$entity->_restSubmittedFields

The reason that work-around was introduced: field-level access checking must occur, but only for those fields that are being edited (POSTed or PATCHed, respectively). Doing field access checking on all fields regardless of the field data that is actually being provided in the request body would result in incorrect REST API behavior. (For example: an optional field that is not being created or updated would still be validated, and might result in a validation error. We ran into this exact problem at #2405091: Cannot create user entities - {"error":"Access denied on creating field pass"} → , which had a work-around added for it.)

To know which fields to check access for, i.e. the fields that were actually sent (in the request body), we need to pass information from an earlier point in the call stack to a later point in the call stack. When the entity is being denormalized (i.e. from array-of-sent-data to Entity PHP object), we know which fields are present (in that array), and we need to pass that to the REST resource plugin.
In pseudocode:

\Drupal\rest\RequestHandler::handle($request) {
  $method = $request->getMethod();

  $entity_data = $serializer->decode($request->getBody())
  // In here we must set $entity->_restSubmittedFields…
  $entity = $serializer->denormalize($entity_data);

  // So that the method on the plugin that we call here knows which fields were actually sent.
  return $entity_rest_resource_plugin->$method($entity);
}

Consequences

Unfortunately, this has some negative consequences:

1. Code smell: Key Drupal 8 functionality (CRUD on entities via the REST API) depends on a hack that is introducing global state.
2. Contributed project soft blocker: each new serialization format's denormalizer must duplicate this hack, or otherwise that format won't work with the REST module. This required work-around means denormalizers MUST be aware of the REST module. (You can see this in core in the HAL module.)

Root cause

The root cause actually does not lie in the Serialization or REST modules. It lies in the Entity/Field/Typed Data API. Of course, the Entity/Field/Typed Data API was originally not designed with a REST API in mind. So the problem is that WSCCI did not pay the full integration cost. IOW: this is WSCCI Initiative technical debt from 2015 that we never paid off. It was rushed in because apparently in 2015, it was still impossible to POST or PATCH entities.

The capability that is missing in the Entity/Field/Typed Data API which REST functionality needs is tracking of changed aka "dirty" fields:

• For POST requests, we need to know the changes compared to the initial/default field values
• For PATCH requests, we need to know the changes compared to the stored field values.

Proposed resolution

Either:

Remaining tasks

• Land #2862574: Add ability to track an entity object's dirty fields (and see if it has changed) → .
• Get patch to green. The patch should only change REST-related code.
• Review
• Commit

User interface changes

None.

API changes

None.

Data model changes

None.

Summary of the discussion

A summary is helpful because this issue is quite confusing:

• @alexpott proposed solution 1 in #1.
• Immediately after solution 1 was proposed, starting in #4, an alternative approach was explored: solving the root cause, by adding dirty tracking to ContentEntityBase.
• Over the course of 2015, it was discussed more, as there were similar needs relating to updating the changed timestamp for translatable entities when translations change. But then that was solved in a different way. Comments #7–#11.
• But then another Entity API bug became a blocker. (#11).
• A bunch of straight rerolls without digging deeper: #14 – #19.)
• @yched re-stated the problem in a clear way in #21.
• @yched and @mkalkbrenner questioning the reliability of Typed Data's onChange() API in #22–#29.
• Everything until here is from April 2015 to October 2015.
• The next activity is in May 2016 — i.e. long after D8 was released.
• @fago points out in #34–#35 yet different problems. Not only was Typed Data not designed with certain things in mind, nor was ContentEntityBase::values. But he dispels part of what @yched said.
  @fago generally likes the approach in the patch so far. He confirms that ::onChange() was designed to allow for determining changes, but that it was never implemented.
• Silence. Until February 2017. In #38, Wim Leers points out that thanks to #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method → (which happened during the period of silence), we now have test coverage to ensure we don't introduce BC breaks unknowingly. And he points out a related issue: #2824851: EntityResource::patch() makes an incorrect assumption about entity keys, hence results in incorrect behavior → .
• From #39 to #49 he tries to push the patch forward, given @fago's feedback.
• Entity API maintainer @tstoeckler asked in #51 for the non-REST changes to happen in a separate issue.
• Wim Leers continued his work in #52–#71.
• Serialization module maintainer @damiankloip reviewed it in #72 + #73, was overall +1
• Wim Leers addressed the review in #74– #79.
• Entity API maintainer @tstoeckler again reviewing this in #80, Wim Leers addressing it in #81 and then moving all Entity API changes to a new issue: #2862574: Add ability to track an entity object's dirty fields (and see if it has changed) → .
• A hunk of this patch landed in #2768651: Let TimestampItem (de)normalize to/from RFC3339 timestamps, not UNIX timestamps, for better DX → , so Wim Leers started rerolling this in #83–#93, not realizing this was actually still blocked on #2862574: Add ability to track an entity object's dirty fields (and see if it has changed) → .