Field instance config files are not being deleted when fully uninstalling Comment

[indigoxela]

Description of the bug

The comment module does an incomplete cleanup, when uninstalled. This leads to PHP Notices on the config export page.

Steps To Reproduce

1. Turn on error display
2. Uninstall the comment module
3. Go to /admin/config/development/configuration/single/export

Actual behavior

Two PHP notices:

Notice: Trying to access array offset on value of type null in field_config_label_instance() (Zeile 435 von .../core/modules/field/field.module).
Notice: Trying to access array offset on value of type null in field_config_label_instance() (Zeile 435 von .../core/modules/field/field.module).

Expected behavior

No notices, the comment module should remove all related config.

Additional information

When the comment module gets uninstalled it does run field_delete_field('comment_body');, but two files stay untouched:

• field.instance.comment.comment_node_page.comment_body.json
• field.instance.comment.comment_node_post.comment_body.json

And these two files cause the php notices on the "Single export" configuration manager page. The Comment entity no longer exists, there's no instance label.

It may be worth checking, if this incomplete uninstall also affects other modules, for instance Taxonomy. Update: no, the Taxonomy module is not affected.

• Backdrop CMS version: latest stable, 1.21.1
• PHP version: 7.4

[argiepiano]

I have been able to reproduce this issue with fieldable custom entities as well.

1. I installed the module that creates a fieldable custom entity
2. I added a new Field API field. Config files for both the field info (field.field.[FIELD_NAME].json) and field instance (field.instance.[ENTITY_TYPE].[ENTITY_BUNDLE].[FIELD_NAME].json) are created
3. I created and saved an instance of the entity through the UI and then deleted it.
4. I uninstalled the module that created the custom entity. field_delete_field() was called at this time
5. When checking the config folder, I noticed that the field info configuration file (field.field.[FIELD_NAME].jason) was removed (as it should), but the field instance configuration file was not
6. Then, when visiting /admin/config/development/configuration/single/export I get the same notice you reported above

UDPDATE: during the uninstall process, in addition to field_delete_field() being invoked, field_delete_instance() is invoked as well. It's strange that the field instance json file is not deleted.

UPDATE 2: field_delete_instance() sets the deleted key to TRUE in the instance config file. However the field instance config file is never actually removed? I cleared the cache and run cron, the field instance config file is still there.

[klonos]

Would it then make sense to iterate through all entities, find all fields added to comments, and make sure that the instances that are not in use are deleted before completely uninstalling the Comment module? (not when disabling it I guess, cause people might reenable it).

PS: We are already warning about data loss when uninstalling the module:

[indigoxela]

I belief, the problem arises further down the road.

When uninstalling Comment, the module is already disabled, hence the Comment entity info is no longer available, which seems to influence the instance deletion. Might need a little more digging. Note that the data tables (field_something_data/revision) do get deleted as well, it's only the instance config file that stays there.

[klonos]

...it's only the instance config file that stays there.

That kinda rings a bell. I think that we had some issue re removing config files that are no longer owned by any active module. Couldn't find it though.

[echozone]

I get same Notice plus the second Notice, on config manger single export screen, but comment module is installed + I did not uninstall it. It's a D7 migrated site.

Notice: Trying to access array offset on value of type null in field_config_label_instance() (line 435 of __/core/modules/field/field.module). Notice: Trying to access array offset on value of type null in field_config_label_bundle() (line 446 of __/core/modules/field/field.module).

[argiepiano]

This bug appears ALSO when the Comment module is disabled but not fully uninstalled.

I believe one should not be able to export a field instance config file when the entity the field is attached to is not available. In this case, when Comment is disabled or uninstalled. Is this correct?

Additionally, we don't want to delete the field info and the instance info CMI files until the module is completely uninstalled, right? This gives the user the chance of re-enabling and having the data still intact.

If the answer is yes to both of the above, then the solution is to skip including the comment_body and other fields attached to comment within config_get_prefix_groups() if the entity info for comment is not available. This is a bit tricky to do, as config_get_prefix_groups() is really agnostic about the type of config file it's dealing with. So, this checking would only apply to config prefixes field.instance.

Do you think this is the right approach?

BTW, since the entity is not available, you get an incomplete label for the export. In the screenshot, it's missing the entity type (Comment) and the bundle (only machine name there).

[indigoxela]

To prevent an export of a field instance config, when the entity the field is attached to is not available, might solve at least a part of the problem.

The base problem - on uninstall config that should get deleted, doesn't get deleted - would still be unsolved, though. But let's fix what's fixable. If it is.

[argiepiano]

To prevent an export of a field instance config, when the entity the field is attached to is not available, might solve at least a part of the problem.

The base problem - on uninstall config that should get deleted, doesn't get deleted - would still be unsolved, though. But let's fix what's fixable. If it is.

Yes. It seems like we should split this issue into two: (1) one dealing with the fact that config export functionality is trying to access an unavailable entity/bundle (entities created by "disabled-only" modules, where the config file should not be deleted), and (2) another dealing with the fact that completely uninstalling comment doesn't remove field instance config files attached to comment entities.

How about we rename this to fit the first case and open a second issue to fit the second case?

[indigoxela]

How about we rename this to fit the first case and open a second issue to fit the second case?

Hm, this issue's description is clearly about the second case - the title isn't that specific.

How about opening a new issue for the config export problem and making this issue's title more specific, so it's clearer what it's about?

[argiepiano]

That's OK with me. Then this issue focuses on the specific notice that's thrown by the configuration exporter. I noticed however that the title still says "uninstalling". As I pointed out, the notice is also a problem when the module is "disabled". There is a difference between disabling and uninstalling. Can we change the title to include both cases?

I would still like to tackle the problem of the field instance config files not being deleted when completely uninstalling Comment. I'll open a separate issue for that.

[indigoxela]

tackle the problem of the field instance config files not being deleted when completely uninstalling Comment. I'll open a separate issue for that.

This IS the issue for that 😀 This comment tries to explain why this happens. But there's no (clean) solution for that yet. It also affects other modules, btw.

[argiepiano]

Well, if this is the issue for that, then the title should clearly indicate that. Something like

"Field instance config files are not being deleted when fully uninstalling Comment"

[indigoxela]

Issue title changed as suggested.

[yorkshire-pudding]

I can reproduce this issue. Also the one where it is just disabled.

[indigoxela]

The bug's still present, btw. 😉