BaseFieldOverride fails to take into account ContentEntityInterface::bundleFieldDefinitions() when invoking onFieldDefinitionUpdate()

Is there really an issue here ?

I mean, until some code decides to create a BFO for a base field, the definition that EM::getFieldDefinitions() hands out is the one that comes out of ET::bundleFieldDefinitions() (if present) or ET::baseFieldDefinitions().

Then, some code creates a BFO for that field, and saves it. It does so by doing :
EM::getFieldDefinitions($entity_type, $bundle)['field_foo']->getConfig($bundle);

This means that the BFO starts out with the values from ET::bundleFieldDefinitions(), as it should ?

Why is this Postponed? I'm assuming #1498720: [meta] Make the entity storage system handle changes in the entity and field schema definitions which is fixed.

It's not the first time that we run into this issue while doing a config-import. This time after module upgrades - last times I'm can't remember the exact situation but might also been after some upgrade.

This is our exception breaking the import (Drupal 8.6.3):

TypeError: Argument 2 passed to Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate() must implement interface Drupal\Core\Field\FieldDefinitionInterface, null given, called in /drupal/web/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php on line 207 in Drupal\Core\Entity\ContentEntityStorageBase->onFieldDefinitionUpdate() (line 424 of /drupal/web/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).

This block in web/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php is causing "the null":

    [...]
    if ($this->isNew()) {
      // @todo This assumes that the previous definition isn't some
      //   non-config-based override, but that might not be the case:
      //   https://www.drupal.org/node/2321071.
      $previous_definition = $this->getBaseFieldDefinition();
    }
    [...]

Here is a `var_export` dump of the BaseFieldOverride instance in question: https://0bin.net/paste/Z+rIGISbx1feTqCF#ms1LBhcaExJbNWOY5enxkfnunkJUtH1I...

:-(

Is this happening because during config-import the baseFieldDefinition is not in place when the baseFieldOverride gets imported? Hence this is very specific to the config-import situation?

For now - since we need to roll out into production - we just skip the field definition update event trigger in this edge case. Especially since we have not found one single implementation of a field definition update handler:
https://api.drupal.org/api/drupal/8.6.x/search/onFieldDefinitionUpdate
(also none in all the module we have in use for this project)

Found a version conflict in our config schemes!
That's why we came into this use-case.

So we fixed our project config-import by sanitizing our config YMLs.

Please don't follow my thoughts in #9 and #10 - they were not leading into the right corner

Having the same issue on config import during site install throw existing site configuration.
Can you guedressel please provide more information on how to sanitize the config yaml's?
Thanks in advance.

@akalam: For me, "sanitize" meant "delete the stored config files." I deleted all the files that ended in `scheduled_state` or `scheduled_date`.

I applied the patch in #10 and it helped, but it revealed another error, even after I sanitized the config (see above). Here's a new patch for postDelete() in the same file. I'm not sure if this patch should replace the one from #10, or supplement it. I'm going to try using both.

Unfortunately my last patch didn't fix the problem; undefined fields are causing problems in preSave(), postDelete(), and getBaseFieldDefinition(). Here's a new patch that combines all three fixes (two old, one new) into a single patch.

Just FYI, I met the following error when uninstalled the content_moderation module.

TypeError: Argument 2 passed to Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate() must implement interface Drupal\Core\Field\FieldDefinitionInterface, null given

During module uninstall, the base field override config was not removed for the moderation_state field. E.g. config/sync/core.base_field_override.node.page.moderation_state.yml

This had no effect on the existing environments. But the new site installation (using Drush 9 site-install with existing config) was failing with the mentioned error.

The fix was to remove the base field override files manually from the exported config.

(Unfortunately, I have no time to dig this case dipper)

Ran into this problem when doing config import when deleting X Paragraph types and all the fields on those paragraph types.

Seems that the bit 'Inform the system that the field definition is being updated back to its non-overridden status' runs into trouble if the system no longer contains the field it is trying to update.

Patch fixed the problem (thanks!) but it does seem to me that this fix might just be hiding an upstream error.

Also, this needs tests.

Re-rolled for 8.8.x. This didn't apply anymore.

Today I hit this issue again.
Just for the record: The exported configuration of our project was not importable for me after a team mate did some module upgrades. Removing the the core.base_file_override.[entity-type].[entity-bundle].* configs fixed the config import process for us (as it did when I posted #11)

I'm still not sure why this issue arises in the first place. Is it either because during the module upgrade no clean config export was committed into the project git repos or because the config discrepancies only show up if you do a config import on a different instance (read: computer) as the original upgrade ran. Or maybe it's something totally different...

¯\_(ツ)_/¯

Another day, same issue: This thing keeps annoying us.

Our newest finding is: BaseFieldOverride configurations do not get deleted when their base fields vanish.
In our current project we modified the Content Moderation settings what resulted in removal of the "moderation_state" field from various entity definitions (media and node bundles). But their core.base_file_override.[entity-type].[entity-bundle].* configurations stayed in the system.

Their config dependencies declare only the entity bundles, not the entity bundle's base fields it overrides. For example: core.base_file_override.node.page.moderation_state declares core.base_file_override.node.page as config dependency, nothing else.

What helped us today was again deleting obsolete configuration yaml files manually....

Handle empty return value of getBaseFieldDefinition() in other methods

Here I go again (typo fixed!)

Resolved some coding standard issues & added an interdiff .

I found a case when the FieldConfigBase::postSave was triggered and here, the BaseFieldOverride::getFieldStorageDefinition is called which returns NULL. So, I added a check in FieldConfigBase::postSave to check for this.

We hit the following error when uninstalled the metatag module.

TypeError: Argument 2 passed to Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate() must implement interface Drupal\Core\Field\FieldDefinitionInterface, null given

During module uninstall, the base field override config was not removed for the metatag field. E.g. config/sync/core.base_field_override.node.page.metatag.yml

The fix was to remove the base field override files manually from the exported config.

Here a reroll of the last patch for drupal 8.6.x

We just had this issue on a local project. Our edge case was enabling translations and exporting the config for the subset of translateable fields set at admin/config/regional/content-language.

The patch at #25 worked resolved the issue for us.

I'm not sure how RTBC works with patches for multiple versions on the same issue but with that patch being for 8.8.x and the issue being for the same version I'm assuming it's okay to RTBC.

Re #16:

Ran into this problem when doing config import when deleting X Paragraph types and all the fields on those paragraph types.

I am sorry, but I couldn't parse that sentence :). But it might not be important => see the next quotation.

Seems that the bit 'Inform the system that the field definition is being updated back to its non-overridden status' runs into trouble if the system no longer contains the field it is trying to update.

In order for this not to happen the override should be deleted when the field it overrides is deleted. We have an issue for that - #3043741: Delete base field overrides when removing entity fields they override..

Re #20, #21, #22

Our newest finding is: BaseFieldOverride configurations do not get deleted when their base fields vanish.

Handle empty return value of getBaseFieldDefinition() in other methods

So this is the reason for all the changes in the patch, where it is first checked whether the base field definition exists? Please let's revert them because #3043741: Delete base field overrides when removing entity fields they override. should prevent this from happening.

+++ b/core/lib/Drupal/Core/Field/FieldConfigBase.php
@@ -283,12 +283,18 @@ public function postCreate(EntityStorageInterface $storage) {
-    $this->entityManager()->clearCachedFieldDefinitions();
+    \Drupal::service('entity_field.manager')->clearCachedFieldDefinitions();
...
-    if ($this->entityManager()->hasHandler($entity_type, 'view_builder')) {
-      $this->entityManager()->getViewBuilder($entity_type)->resetCache();
+    if ($this->entityTypeManager()->hasHandler($entity_type, 'view_builder')) {
+      $this->entityTypeManager()->getViewBuilder($entity_type)->resetCache();
+      if ($field_storage_definition = $this->getFieldStorageDefinition()) {
+        $entity_type = $field_storage_definition->getTargetEntityTypeId();
+        if ($this->entityTypeManager()->hasHandler($entity_type, 'view_builder')) {
+          $this->entityTypeManager()->getViewBuilder($entity_type)->resetCache();
+        }
+      }

These changes seem also unneeded.

+++ b/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php
@@ -163,8 +170,10 @@ public function getUniqueIdentifier() {
-      $fields = $this->entityManager()->getBaseFieldDefinitions($this->entity_type);
...
+      $fields = \Drupal::service('entity_field.manager')->getBaseFieldDefinitions($this->entity_type);
+      if (!empty($fields[$this->getName()])) {
+        $this->baseFieldDefinition = $fields[$this->getName()];
+      }

I guess this should be the main change? However you don't retrieve the bundle field overrides here. For that we'll need something similar to \Drupal\Core\Entity\EntityFieldManager::buildBundleFieldDefinitions(), which doesn't load the base field overrides.

We also still need a test.

Please give credit to @effulgentsia for the amazing issue summary.

@hchonov thank you for your insightful review.
Pointing to #3043741: Delete base field overrides when removing entity fields they override. is the crucial piece of information in it for me.

When I find time I'll revise my patch but I guess it's actually useless since I always just responded to symptoms and never had a clue how to approach the actual issue as discussed in #3043741: Delete base field overrides when removing entity fields they override..

That being said I don't think we'll get a real solution to this very issue since I experienced code breaks in different situations once patch #22 begins to return NULL in various methods: Even if returning NULL seems to be a viable reaction to not being able to determine a clear result, the rest of Drupal's code does not expect non-results like NULL.
Hence this is either a wrong solution or it is an API breaking change which will need some serious testing and refactoring on many places in Drupal's code base.

Maybe it's better to let #3043741: Delete base field overrides when removing entity fields they override. settle and keep looking for other situations when this issue arises.

We still have an issue here to solve and it is the one described in the issue summary. And that problem is independent of the referenced issue about deleting the base field.

Same issue and fix it with 2321071-25.patch, thanks
PS: Hey Renrhaf ;)

I face an issue that seem to be caused by this. Sadly, the #37 patch does not fix it.

My use case is not that hard to reproduce:

1. Install Drupal using the standard profile
2. Enable Content Moderation and Content Translation
3. Enable translations on one of your Content Type but mark the moderation_state field as not translatable
4. Export configuration. You should get a config_override file for the moderation_state base field.
5. Install Drupal using the existing configuration

Expected result: Drupal is installed and your Content Type moderation_state field is not translatable
Current result: TypeError: Argument 2 passed to Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate() must implement interface Drupal\Core\Field\FieldDefinitionInterface, null given, called in core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php on line 205 in core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php on line 463 #0 core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php(205): Drupal\Core\Entity\ContentEntityStorageBase->onFieldDefinitionUpdate(Object(Drupal\Core\Field\Entity\BaseFieldOverride), NULL)

Status update: even if the patch above does not solve the case I described, it makes the install process go a little bit further.

I figured out that the patch had a bit been messed up between #25 and #28 so I repaired it.

This new patch should be considered as a reroll of #25.
It works for me and solves my case \o/

Also works for me and solves my case \o/

Perhaps this could be reviewed by an expert, as I never want to see this error again :-)

Patch #39 solved this issue for me as well.

I can also confirm and reproduce the issue DuaelFr explained in #39 when doing an install from existing config containing a moderation_state base field override.

I investigated a bit and what happens in my case is this when attempting to save the moderation_state base field override:
- It goes in \Drupal\Core\Field\Entity\BaseFieldOverride::preSave() which calls $previous_definition = $this->getBaseFieldDefinition();
- It then goes into \Drupal\content_moderation\EntityTypeInfo::entityBaseFieldInfo()
which contains:

  if (!$this->moderationInfo->isModeratedEntityType($entity_type)) {
    return [];
  }
  ... // base field definitions below.
  

- This condition passes and returns[] early because at that point, no workflow for $entity_type (node in my case) exists because it was not imported yet.

Importing the workflow config before the moderation_state base field override (by adding a config dependency) solves it.
But this may not be ideal.

The patch from #39 makes the installation work.
However I'm not sure we can safely return NULL for all those functions, especially because a lot of contribs may not be expecting NULL.
Although this may be an edge case only when installing the site.
I don't know how to properly resolve this so maybe someone else can jump in on the review.

I have the error described in #38 on site installation from the existing config. There is also content moderation in my project.
#39 solves the problem for me.
Let's RTBC?

This still needs tests, but let's RTBC for now so we can get feedback on #42 from the maintainers. It would be a waste of time to write tests if we don't have a good solution. They can set back to NW with feedback.

I'm not sure we can safely return NULL for all those functions, especially because a lot of contribs may not be expecting NULL. Although this may be an edge case only when installing the site.

#42 sound like there is a real missing config dependency that needs fixing and the patch here is a workaround that make the dependency a kind of soft dependency. In other words it sounds like a separate bug.

#45 Exactly

I believe some of the reports here may have been caused and fixed by #3043741: Delete base field overrides when removing entity fields they override.

The moderation_state issue is a different case, but not sure how to solve it because:
- workflows.workflow.editorial depends on node.type.article
- core.base_field_override.node.article.moderation_state depends on node.type.article

Should we add workflows.workflow.editorial as dependency to core.base_field_override.node.article.moderation_state ?
If so what is the best way? Maybe create a new class that extends \Drupal\Core\Field\Plugin\Field\FieldType\StringItem and implements calculateDependencies() ?

Something like this?
WIP, do not use it, there is no upgrade path yet to update the config.

I've also run into this issue.

My env: BLT(w/lightning)/D8.

After disabling lightning_scheduler and exporting the config, my Travis build fails with the following:

 [info] Undefined index: scheduled_transition_state BaseFieldOverride.php:165
TypeError: Argument 2 passed to Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate() must implement interface Drupal\Core\Field\FieldDefinitionInterface, null given, called in /home/travis/build/myprojWW/myproj-blt/docroot/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php on line 205 in /home/travis/build/myprojWW/myproj-blt/docroot/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php on line 463 #0 /home/travis/build/myprojWW/myproj-blt/docroot/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php(205): Drupal\Core\Entity\ContentEntityStorageBase->onFieldDefinitionUpdate(Object(Drupal\Core\Field\Entity\BaseFieldOverride), NULL)

So, far it looks like the patch #39 is working.

Watching...

@ejanus You probably have a remaining base field override for "scheduled_transition_state". Deleting it should fix your issue.
In your case #3043741: Delete base field overrides when removing entity fields they override. would have deleted it when uninstalling lightning_scheduler and avoided the issue.

For the moderation_state base_field_override explained in #38 to #47, the issue is still relevant but the issue description is not really appropriate for this. I created a new issue with the patch from #47: #3129874: The "moderation_state" base field overrides cause install from existing config to fail

Patch #39 also solved my issue. Thank

I was getting this error for the "moderation_state" field:

TypeError: Argument 2 passed to Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate() must implement interface Drupal\Core\Field\FieldDefinitionInterface, null given

and the patch in #39 solved it for me, thanks.

I must add that adding workflows.workflow.editorial as a dependency to core.base_field_override.node.article.moderation_state (as suggested in #46) did fix it as well.

I believe this issue should be closed as a duplicate of #3043741: Delete base field overrides when removing entity fields they override..
As for the moderation_state issue explained in #38 onwards, I opened a dedicated issue #3129874: The "moderation_state" base field overrides cause install from existing config to fail and proposed a patch there.

FWIW, I've been running into this problem while running an install from configuration, so I am not 100% convinced that #3043741: Delete base field overrides when removing entity fields they override. is able to prevent this from happing, actually. Unfortunately, this is horrible to debug as the problem only occurs well into the install process, which goes horribly slow with XDebug enabled.

@eelkebok could you identify the field override causing your issue and see if it is related to #3043741: Delete base field overrides when removing entity fields they override. (by checking if the field it overrides still exists)?
Thanks

Sorry, I meant to report back. It looks like it is related to the other issue; the override was for base field revision_log on media items. Not sure how we ended up with those, but the base field for revision logs on media items is called revision_log_message now (and the issue that changed it seems to be on the old media_entity contrib module; may be an issue with the distribution we've used).

I do wonder whether we can improve the error reporting here; in this case I've had to hack an echo into the core code to actually find out what base field was causing the problem. It still looks like we need to address the source - like #3043741: Delete base field overrides when removing entity fields they override. is doing - instead of ignoring the problem, but maybe we should also take into account the possibility of this situation cropping up.

Running into the same problem when installing from existing configuration. Patch from #39 resolves the issue.

Logging values at failure in preSave() method of BaseFieldOverride.php (line 205) yields the following:
default settings: Array()
$this->settings = Array()
$this->isNew() = 1
$this->getBaseFieldDefinition() = null
$this->getName() = moderation_state
$this->getLabel() = Moderation state
$this->getType() = string

Unfortunately, unable to do further troubleshooting at this time.

@James Marks, see #3129874: The "moderation_state" base field overrides cause install from existing config to fail which is the proper issue for moderation_state.

+++ b/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php
@@ -162,7 +169,9 @@ public function getUniqueIdentifier() {
+      if (!empty($fields[$this->getName()])) {
+        $this->baseFieldDefinition = $fields[$this->getName()];
+      }

This is really odd - a BaseFieldOverride without a base field to override is a strange concept to understand. Why does this happen and is it not an error condition.

Patch in #37 worked for me.
I was getting the error below when saving the configuration in the Content Traslation page (/admin/config/regional/content-language). It generated override files for fields of one of the entities provided by the Recurring Events module.
Then, after exporting the configuration and trying to import it in another environment, I got this:

TypeError: Argument 2 passed to Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate() must implement interface Drupal\Core\Field\FieldDefinitionInterface, null given, called in /app/web/core/lib/Drupal/Core/Field/Entity/Base  
  FieldOverride.php

Thanks for looking into this and providing the patch.

The patch at #37 has helped me on D9.5

I'd been trying to uninstall modules and getting lots WSOD/errors:

TypeError: Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate(): Argument #1 ($field_definition) must be of type Drupal\Core\Field\FieldDefinitionInterface, null given, called in web/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php on line 222 in Drupal\Core\Entity\ContentEntityStorageBase->onFieldDefinitionUpdate() (line 546 of web/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php)

#37 also worked for me on D9.5.8. Was getting the same error as #68.

Just stating that in my 9.5.10 install #37 didn't work but #39 did

Reroll of #39 for Drupal 10.1.x

Patch in #71 worked for me. I'm on Drupal 10.1.6, PHP 8.1.