Inconsistent form build between initial form generation and the first ajax request

The thing is:

Does that only happen with AJAX requests?

e.g. turn off Javascript in Chrome dev tools, do the same scenarios.

I would say this is a limitation of FAPI itself.

Discussed in IRC:

We really do not want to bring back the ability to store form_state on GET requests as that can easily kill a site both with writes on GET requests, have problematic cache consistency issues (form state expired, but page still in Varnish), and cache_form / KV store table growing each time a form is visited (can get HUGE).

Ideal solution: Use revisions and store the revision_id + a signed hash.

But because not everything supports revisions and not even EntityChangedInterface, the original entities need to be stored for saving (pseudo code):

public hook_entity_update($entity, $type) {
  $this->key_value->store('entity_versions:' . $this->getHash($entity->original), $entity->original);
}

Then use the hash of the entity in the form building process and load this exact version from key value store instead of from entity cache.

In case the same entity changes between loads, you could also store the hash in a field of the entity itself and use the same hash for form building and saving to K/V when it is gonna be outdated.

The advantage is that your conflict management will work without any problems and you will store only as many copies of the entities that you need, especially if you have some nice garbage collection using K/V expirable.

Good points on the approach. One additional challenge is to see who is still using a certain hash of the form. I would suggest a LRU approach for it and writing a TS per entity loaded during the form building to a table during KernelEvents::Terminate.

Overall I think a hash of the serialized entity would work well.

It might be possible to store this hash within the entity itself (unset $entity->hash before recomputing the hash), then the overhead is minimal.

You could store within the entity references, the hash to load as well.

I don't have concerns about the user manipulating the entity hash, but if we have we can sign the value with a : and a HashBase64 that uses the Drupal Private Key.

The entity being cached in FormState by \Drupal\Core\Entity\EntityForm and that calling \Drupal\Core\Entity\EntityForm::prepareEntity() has lots of confusing edge cases and AFAICT hasn't worked like it originally did for years. It was never cleaned up, so now we're left with this confusing system.

#14 In the ideal case any write requests during GET requests should be avoided. However as this would just create a persistent cache for this entity object, it should be fine to create this cache when actually storing the entity hash in the form's hidden field.

There still would need to be a strategy to expire those items though.

And it would need to be tested that the entity's hash / cache id would really only be dependent on the state at saving, not on any dynamic post load additions as indeed what killed the sites was one form_cache entry per view, in other words: Too many variations.

I just ran into a very weird issue which sounds like it could be the source for this as well. In my case, I debugged submitting entity changes with an ajaxif-ed button. The changes to $this->entity in the form are lost in the ajax callback somehow. I figured, the form object's entity is somehow re-loaded during serialization in
\Drupal\Core\Form\FormCache::setCache()
>$this->keyValueExpirableFactory->get('form')->setWithExpire($form_build_id, $form, $expire);

Generally this is related to the issue of ContentEntityForm putting the form object into the form array in
$form['#entity_builders']['update_form_langcode'] = [$this, 'updateFormLangcode'];
This might be related to trigger the serialization issues, but generally this is no biggie as the form object is serialized with the form state as part of the build_info arguments anyway. Nevertheless it helps to complicate things.

Actually, in my case the changes lost were in referenced entities (parapraphs) which contained the changes. So maybe there is just a serialization problem around that.

This issue also happening with Inline Entity Form or Paragraph (or other similar entity reference field widget) and Translations enabled, could be reproduced easily:
1. Create node type with translations and with entity reference field which also translatable and set inline entity form widget to it.
2. Add node English version with some entity in referenced field.
3. Add French translation and save.
4. Edit French version and click to Edit on referenced content (don't save yet).
5. Edit English version, click to edit referenced content, and Save.
6. Go back to tab with from step 4 and click to Save.

It will show a validation error that entity is outdated, but this doesn't happens when you didn't touch referenced field inline form (or any other form rebuild or ajax). For sure it is inconsistent behavior for ajaxed form and not ajaxed. The root of the issue lays in FormCache which stores `$this->entity` in case of EntityForm and later it trying to use that outdated entity object.

Right solution as I think should be next: instead of caching old entity on form rebuild, it should take the latest entity version and re-apply all values that was submitted in form. It should be strong concept of applying field values one by one to original entity, and it should help for normal concurrent edit and identifying conflicts during form submit, and if there no conflicts it should be saved normally. Like auto-merges in Git.

Here a dirty example of what I meaning:

diff --git a/lib/Drupal/Core/Form/FormBuilder.php b/lib/Drupal/Core/Form/FormBuilder.php
--- a/lib/Drupal/Core/Form/FormBuilder.php
+++ b/lib/Drupal/Core/Form/FormBuilder.php	(date 1642003564406)
@@ -10,6 +10,8 @@
 use Drupal\Core\Access\AccessResultInterface;
 use Drupal\Core\Access\CsrfTokenGenerator;
 use Drupal\Core\DependencyInjection\ClassResolverInterface;
+use Drupal\Core\Entity\EntityFormInterface;
+use Drupal\Core\Entity\TranslatableInterface;
 use Drupal\Core\EventSubscriber\MainContentViewSubscriber;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Form\Exception\BrokenPostRequestException;
@@ -262,6 +263,16 @@
     $check_cache = isset($input['form_id']) && $input['form_id'] == $form_id && !empty($input['form_build_id']);
     if ($check_cache) {
       $form = $this->getCache($input['form_build_id'], $form_state);
+      $form_obj = $form_state->getFormObject();
+      if ($form && $form_arg instanceof EntityFormInterface && $form_obj instanceof EntityFormInterface) {
+        $route_entity = $form_arg->getEntity();
+        $form_entity = $form_obj->getEntity();
+        if ($route_entity instanceof TranslatableInterface && $form_entity instanceof TranslatableInterface && $route_entity->hasTranslation($form_entity->language()->getId())) {
+          $route_entity = $route_entity->getTranslation($form_entity->language()->getId());
+        }
+        $form_obj->setEntity($route_entity);
+        $form_obj->buildEntity($form, $form_state);
+      }
     }

     // If the previous bit of code didn't result in a populated $form object, we

Just got an idea with more elegant way instead of patching core: add `EntityFormCache` service as wrapper to `@form_cache` service and override `getCache()` method.

I don't think it's good idea to "poison" form builder with entity namespace dependency

+use Drupal\Core\Entity

+1 issue for concurrent edits.

My entity is neither translatable nor revisionable, but fields that are not exposed to form display at all are overwritten in case ajax (file uploads, multiple values fields etc.) is triggered. This can be huge issue that can result in data loss.

I also think entity shouldn't be cached at all or at least we should create partial save that will save only fields on form display, this can also boost performance in some drupal projects.

Let's say products stock is managed by external service, it will be overwritten if someone is editing product even if stock is not exposed to form edit.