Ignore field definition removals/additions from fields with custom storage

Berdir,

Thanks for the info. I was worried about this until reading this thread and the related one you reference. So based on your comments I will simply ignore the error.

Thanks for all of your hard work on this critically important module.

+++ b/core/lib/Drupal/Core/Entity/EntityDefinitionUpdateManager.php
@@ -287,12 +287,21 @@ protected function getChangeList() {
+            // Ignore the deletion if the storage definition has custom storage.
...
+            // Ignore the deletion if the original storage definition had custom
+            // storage.

Would be nice to explain WHY we are doing things here.

Adding [in status reports], because the first 2 times I read this I didn't get it.

Ok, but I think this is about "ignoring the reporting of changes" to fields, for some reason when I first read it I thought that it was "blocking the changes".

The patch looks good to me, I guess we are only missing test coverage.

After update to Drupal 8.1 the same error present.
Will the decision of this problem?
I still do not install the module, waiting for solution of the problem for many months.
but it is very necessary in the work site.

So I've worked on the pathauto side of this today #2661178: Error: Mismatched entity and/or field definitions and whilst it is possible to do something about pathauto using install and uninstall hooks I think the solution here is way better.

Here's a test. Given the number of reports and what people are doing to get around it I think this is a major bug.

I think we might need a update function to remove the field definitions which have custom storage from the schema repository.

I think they are written - hence the error in pathauto about having to delete the fields... I'll work on this.

Here's tests for what happens when you install a new entity type with a custom storage field and an upgrade path.

@Berdir you still need the changes otherwise it'd always show up in the report page. But if you can change a definition without having to update the definitions to say that this is the definition that is now applying what is the point?

Sorry, lost track of this, will review it ASAP.

Adding the screenshot from #2661178: Error: Mismatched entity and/or field definitions to clarify what this bug is about. The message is displayed to site administrators.

My understanding is that users see this message under normal site operation with any field with custom storage (including those used by Pathauto) and there is no way to work around it in contrib, correct? However, it does not actually impact the functionality other than displaying a confusing error to admins and adding a little crufty stuff to stored configuration.

Thanks @Berdir.

The developers discussed this with the Entity and Field maintainers and decided to defer triage on it. While the bug is only an admin-facing error message, it is confusing a lot of people and causing a lot of duplicate bug reports, plus also leading people to some really strange/ill-advised workarounds. It's possible that #2539634: PathItem::delete() never runs because the path field type is a computed field in disguise might make this issue less problematic for pathauto, so if that is fixed, we can revisit this and consider downgrading to normal.

(Edit: wrong issue link)

I find it hard to understand the discussion of development in English.
so the question.
if I install the module now, whether the difficulty to correct this error in the future?
or an error will be eliminated by simply updating the module / core Drupal?
or should wait a little longer, if in the near future will fix this error?
(I do not use drush, but many of errors corrected using it).
thanks for the answer.

Isn't the bug in #2539634: PathItem::delete() never runs because the path field type is a computed field in disguise a manifestation of the issue reported here? Changing the path field from hasCustomStorage() to isComputed() won't change the underlying issue of custom storage/computed fields incorrectly showing up on the status report.

As mentioned in #24, people with knowledge of this issue will know to ignore it or to run drush entup, but people who don't know this will start Googling and god knows what they may end up doing in order to "fix" this.

I'd like to think of this issue as unrelated to #2539634: PathItem::delete() never runs because the path field type is a computed field in disguise and see it fixed in core. The other issue is about the way path.module implements its FieldItem and FieldItemList classes.

That being said, the patch in #17 looks great, with these tiny nitpicks:

1. +++ b/core/lib/Drupal/Core/Entity/EntityDefinitionUpdateManager.php
   @@ -281,12 +281,21 @@ protected function getChangeList() {
   +            // Ignore the deletion if the storage definition has custom storage.
   

   Should say creation?

2. +++ b/core/lib/Drupal/Core/Entity/EntityLastInstalledSchemaRepository.php
   @@ -66,6 +66,10 @@ public function getLastInstalledFieldStorageDefinitions($entity_type_id) {
   +    // Field storages with custom storage are ignored.
   

   Should say field definitions?

I just had a look to this again. Both #18 and #20 make sense to me, I'm wondering whether it would make sense to always keep custom storage definitions in the last installed repository but silently applying changes when they are detected in the proper contexts, e.g. when update.php is run.

+++ b/core/modules/system/src/Tests/Update/CustomStorageEntityDefinitionUpdateTest.php
@@ -0,0 +1,35 @@
+  public function testAllowedHtmlUpdate() {

Huh?

It's been a while since I had a look at this issue. Is there something specific that needs to be done for this to move forward again?

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Wondering if this is still an issue? Using pathauto with Drupal 10.1.x on a standard install I don't see any errors.
Tagging for issue summary update for steps to reproduce/updated screenshots.

There has been no follow up to the prompt 2 years ago for more information. Therefor, I am closing this is as outdated.

If you are experiencing this problem on a supported version of Drupal reopen the issue, by setting the status to 'Active', and provide complete steps to reproduce the issue (starting from "Install Drupal core").

Thanks!