Not properly clearing EntityFieldManager's fieldMap leads to fatals, often after migration or bundle creation

Interesting problem:

Looking at the code:

foreach ($this->entityFieldManager->getFieldMap() as $entity_type_id => $entity_field_map) {
      foreach ($entity_field_map as $field_name => $field_info) {
        // Skip fields without any formatters.
        $options = $this->formatterManager->getOptions($field_info['type']);
        if (empty($options)) {
          continue;
        }

        foreach ($field_info['bundles'] as $bundle) {
          $derivative = $base_plugin_definition;
          $field_definition = $this->entityFieldManager->getFieldDefinitions($entity_type_id, $bundle)[$field_name];

It seems like difference in what is being returned by \Drupal\Core\Entity\EntityFieldManagerInterface::getFieldMap()
and \Drupal\Core\Entity\EntityFieldManagerInterface::getFieldDefinitions()

$this->entityFieldManager->getFieldDefinitions($entity_type_id, $bundle)[$field_name]

Should work because getFieldMap should not return a field name for the bundle that doesn't exist on the bundle and that you can't get a definition for by calling getFieldDefinitions()

So maybe it is problem with \Drupal\Core\Entity\EntityFieldManager()

Interesting. I just was hitting issues with EntityFieldManager on #2959132: Taxonomy pages crash with layout_builder enabled. Could be related?
The main thing was that the cache was not being cleared correctly.

Assigning to @phenaproxima for help debugging the migrate test fails.

That was a tricky one, but I think I know what's causing this. The failing tests do not actually install the node entity tables, and the new cache tag causes Content Translation to attempt to create the content_translation_source field on the node data table. But since the table does not exist, it's failing.

Let's see what the attached patch does.

Also, 8.7.x, people!

Thanks for the help @phenaproxima!
Here's a generic test for this.
The FAIL-TOTALLY patch is the interdiff.

Also attached are two other FAIL patches, to prove that both the change in EntityBundleListener and EntityFieldManager are needed in conjunction.

Fixed an extra blank line, here's the interdiff

--- a/core/tests/Drupal/KernelTests/Core/Entity/EntityBundleListenerTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityBundleListenerTest.php
@@ -2,7 +2,6 @@
 
 namespace Drupal\KernelTests\Core\Entity;
 
-
 /**
  * @coversDefaultClass \Drupal\Core\Entity\EntityBundleListener
  *

Updated the IS.

Closing #2926139: EntityFieldManager's field map not invalidated as promised as a duplicate as this issue is more complete, but crediting those who worked over there on a fix.

+++ b/core/modules/language/tests/src/Kernel/Migrate/d6/MigrateLanguageContentSettingsTest.php
@@ -25,6 +25,13 @@ protected function setUp() {
     $this->installConfig(['node']);
+    // If the entity_field_info cache tag is invalidated, Content Translation
+    // will attempt to create the content_translation_source column in the node
+    // data table (usually node_field_data). However, if the node entity schema

If => When?

Until now, I guess that didn't happen, but afaik comments should avoid refering to how things used to work, because someone reading that likely doesn't have that context.

Also, reading this makes me wonder just how closely this is related to #2599228: Programmatically created translatable content type returns SQL error on content creation and whether that is adding workarounds when the real problem is actually this?

100% agreed on the "write comments for how things are".
In fact, I don't think a comment is needed here at all.

    $this->installConfig(['node']);
    $this->installEntitySchema('node');

Seeing that in a test doesn't make me question why the second line is there. It fits right in. And if more clarity is needed, there is git blame.

Nice.

We discussed in Slack whether or not it would make sense that the added call to clearCachedFieldDefinitions() needs a comment or not, @tim.plunkett argued that the identical call in onBundleDelete() doesn't have one either, which isn't wrong :)

Tim also confirmed that this indeed seems to fix the related issue I referenced above, which is a 3 year old major bug with 57 followers and this fix is way simpler than the workaround that was done there.

Hi, the patch in #17 don't fix my use case described in "Programmatically created translatable content type returns SQL error on content creation".

While install I get:

An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /core/install.php?rewrite=ok&profile=sdos_scity&langcode=es&id=5&op=do_nojs&op=do
StatusText: OK
ResponseText: Drupal\Core\Entity\EntityStorageException: Exception thrown while performing a schema update. SQLSTATE[42S22]: Column not found: 1054 Unknown column 'content_translation_source' in 'where clause': SELECT 1 AS expression
FROM
{node_field_data} t
WHERE content_translation_source IS NOT NULL
LIMIT 1 OFFSET 0; Array
(
)
in Drupal\Core\Entity\Sql\SqlContentEntityStorage->wrapSchemaException() (line 1481 of /var/www/html/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

Going to the error page the installation continue, when finish, I do a drush entity-updates and crash with:

developer@scityram:/var/www/html (develop)*$ drush entity-updates
The following updates are pending:

node entity type : 
El campo Origen de la traducción necesita ser actualizado.
El campo Traducción anticuada necesita ser actualizado.

 Do you wish to run all pending updates? (yes/no) [yes]:
 > 

 [error]  Drupal\Core\Entity\EntityStorageException: Exception thrown while performing a schema update. SQLSTATE[42S22]: Column not found: 1054 Unknown column 'content_translation_source' in 'where clause': SELECT 1 AS expression
FROM
{node_field_data} t
WHERE content_translation_source IS NOT NULL
LIMIT 1 OFFSET 0; Array
(
)
 in Drupal\Core\Entity\Sql\SqlContentEntityStorage->wrapSchemaException() (line 1481 of /var/www/html/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php). 
Array	Array
 [success] Cache rebuild complete.
 [success] Finished performing updates.

Neither did the patch over there from what I understood.

The other issue isn't closed as a duplicate, just postponed, because it makes sense to commit this first and then revisit if and what still needs to be done there.

Yes, I misunderstand. We'll stop core update while this issues are open.

Other possibility is that the problem came from "default content" module, it's uncanny but possible.

Thanks @Berdir :)

@tim.plunkett++
@Berdir++

Committed and pushed d3784a1300 to 8.7.x and e2ffcb5e03 to 8.6.x. Thanks!