Applying entity schema updates fail when both entity type and base field definitions change at the same time

Plain re-roll for #4.

1. +++ b/core/lib/Drupal/Core/Entity/EntityDefinitionUpdateManager.php
   @@ -108,14 +115,17 @@ public function getChangeSummary() {
   +      // definition was not already recreated, because any any new field storage
   

   two any

2. +++ b/core/lib/Drupal/Core/Entity/EntityTypeListenerInterface.php
   @@ -27,6 +27,11 @@ public function onEntityTypeCreate(EntityTypeInterface $entity_type);
   +   *   Returns EntityDefinitionUpdateManager::DEFINITION_UPDATED if the storage
   ...
   +   *   was deleted and recreated or NULL if no action was taken.
   ...
      public function onEntityTypeUpdate(EntityTypeInterface $entity_type, EntityTypeInterface $original);
    
   

   While the general architecture of the solution would be fine to me, the implementation is a bit problematic as the schema handler currently implements a listener concept. It's weird for a listener to provide additional information via return values. In particular if the return values stem from an "unrelated" different interface :)

   Also, we've the current problem/glitch that we do not invoke onFieldStorageDefinitionCreate() when creating the entity type either - so I'm wondering whether shouldn't just always compile a complete $change_list and pass it own to all listeners. So listeners can decide whether to act based upon the complete change_list? That would allow us to account for any other change combination issues we might run into in future as well.

   Another option would be to just override the onEntityUpdate() method in the EntityStorageSchemaInterface ?

Discussed with plach, yched and berdir on how we could solve that.

Came up with a possible way with plach: It's the same class that consumes both events, so we could statically keep the events processed before and then skip the field event once the entity event was there before. I could see this a bit troublesome only if on a single page load multiple changelists are compiled instead of a single one - what probably mostly happens in tests, but might happen due to custom update scripts also. To mitigate we'd have to provide changelist ids, set them on event objects and make sure listeners receive the events.

But then, plach came up with another way simpler solution: Just move around the around ordering and apply field updates first - so let's try that.

Tried re-ordering things and took over the test case - looks like it doesn't solve the problem, not sure why yet.

silly me. this should work.

+++ b/core/lib/Drupal/Core/Entity/EntityDefinitionUpdateManager.php
@@ -135,6 +127,12 @@ public function applyUpdates() {
+      // Process entity type definition changes.
+      if (!empty($change_list['entity_type']) && $change_list['entity_type'] == static::DEFINITION_UPDATED) {
+        $entity_type = $this->entityManager->getDefinition($entity_type_id);
+        $original = $this->entityManager->getLastInstalledDefinition($entity_type_id);
+        $this->entityManager->onEntityTypeUpdate($entity_type, $original);
+      }
     }

This fix was originally introduced in #1916790: Convert translation metadata into regular entity fields. There I added also a comment:

// Process entity type definition changes after storage definitions ones,
// as in some conditions a change in the field storage definition can also
// be detected as an entity type definition change. This avoids the change
// to be applied twice (resulting in an unrecoverable error).

Thanks. Refactored the comment a bit to take account of this issue and updated patch.

+++ b/core/modules/system/src/Tests/Entity/EntityDefinitionUpdateTest.php
@@ -478,6 +478,25 @@ public function testDefinitionEvents() {
+    // Entity type updates creates base fields as well, thus make sure doing

Typo, should be "create". Aside from that this looks good to me.

Thanks, fixed.

Let's get this in.

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 4738109 and pushed to 8.0.x. Thanks!