Several post-update functions try to update config entity types without checking if they exist

Nice sleuthing @phenaproxima.

I'm not sure that ConfigEntityUpdater should check this because it's likely to hide other problems - for example an incorrect entity type ID. So I think we should do \Drupal::moduleHandler()->moduleExists() checks in the update hook.

So +1 on expanding the scope.

There's also:
content_moderation_post_update_views_field_plugin_id() - needs a views module check
views_post_update_exposed_filter_blocks_label_display() - needs a block module check

I think we should add some documentation to \Drupal\Core\Config\Entity\ConfigEntityUpdater() that it is the responsibility of the update hook to check that the entity type provider is installed. We could even add this to the example code.

My first thought when reading this patch is that the moduleExists check is not exactly what we care about. More correct would be

if (!\Drupal::entityTypeManager()->getDefinition('view', FALSE)) {
 return;
}

Which then begs the question, why not move that check into ConfigEntityUpdater itself?

re #14 my reasoning for going this way is because making the change in ConfigEntityUpdater makes the code susceptible to silent failure when you get the string identifier incorrect. #14 is the exact opposite of the direction already taken in the class which does...

      $entity_type = $this->entityTypeManager->getDefinition($entity_type_id);
      if (!($entity_type instanceof ConfigEntityTypeInterface)) {
        throw new \InvalidArgumentException("The provided entity type ID '$entity_type_id' is not a configuration entity type");
      }

This one is tricky... because both ways are can cause some annoyance. But I think failing when you get an ID incorrect is better.

+++ b/core/modules/block_content/tests/src/Functional/Update/BlockContentReusableUpdateTest.php
@@ -150,4 +150,12 @@ public function testReusableFieldAddition() {
+  /**
+   * Tests that the update is bypassed if Views is not installed.
+   */
+  public function testReusableFieldAdditionWithoutViews() {
+    $this->container->get('module_installer')->uninstall(['views']);
+    $this->runUpdates();
+  }

How does this test that updates are bypassed?

+++ b/core/modules/block_content/tests/src/Functional/Update/BlockContentReusableUpdateTest.php
@@ -150,4 +150,12 @@ public function testReusableFieldAddition() {
+   * Tests that the update is bypassed if Views is not installed.
+   */
+  public function testReusableFieldAdditionWithoutViews() {
+    $this->container->get('module_installer')->uninstall(['views']);
+    $this->runUpdates();

The only feedback I have is that this comment (and the others in similar places) is not really accurate. We're not testing that the update function is bypassed, we're just testing that it doesn't fail if the module/entity type is not available :)

At least to me, "bypassed" means that we'd have an assertion to check that the function was not executed at all, which is not the case here.

As for the module vs. entity type check discussion from #14 / #17, I can't say I have any preference, so I'll trust @alexpott's feeling :)

Much better! :D

Committed 24d99ff and pushed to 8.8.x. Thanks!

Doesn't apply cleanly to 8.7.x

Committed 7ee5e1d and pushed to 8.7.x. Thanks!