Avoid PHP notice when comparing config without uuid

Applied the patch on a Drupal 8.5.5 running on php 7.0 applied with just some offsets, no problems. Afterwards no more UUID errors.
Code looks clean also.

That said I have absolutely no clue as to this is the right solution to solve the error.

At the very least this needs tests.

Also the patch in #3 is failing tests.

I think a more complete description of the problem and perhaps a test that causes the problem would be a good place to start since I have no idea why the config importer is dealing with configuration entities without a UUID.

@alexpott

I think a more complete description of the problem and perhaps a test that causes the problem would be a good place to start since I have no idea why the config importer is dealing with configuration entities without a UUID.

We hit the same error on a project built in the early stages of D8. Some config entries from the database were imported without an UUID, directly from the module's default config. Don't ask how they landed there without an UUID. Perhaps because the project used initially a very old version of config_sync module. I have no clear understanding. The fact is that the site is in production for > 2 years and the configs are there from a long time.

I think the solution from #2 makes sense, as it will replace the config from the database as if their UUID is different and would heal the database store.

To add some more context to this issue.

I experienced it on a project that was heavily using config_split that I inherited that was creating new splits from the config in an example split directory via a script. These example configs didn't have a uuid so we got the error message. I was able to run the import, which completed successfully and after page refresh the error message went away. Obviously, that's not a "fix" but there you go.

I have no idea why the original config didn't have a uuid.

FYI, admin_toolbar module's admin_toolbar.settings.yml exported has no uuid key.

Drupal core: 9.3.22
admin_toolbar:3.2.1

captcha.settings.yml exported from captcha as well

We are also experiencing this issue on our site which was built on Drupal 8.6 and now upgraded to Drupal 10.0.8. Some of the configs like admin toolbar, advagg, etc.do not have UUIDs, I also doubt it is happening due to those.

Changing to Major as it is a blocker during config export/import.

Webform also triggers this with its default contact form when installing from config.

Don't think a notice qualifies for major. If someone could help me recreate the fatal from the load failures addressed in #4 maybe we can bump it back but I suspect that might be a different maybe related issue. Until then it will also be difficult to create a realistic test for it.

Here's an updated patch for Drupal 10 with just the uuid handling (since I don't know how to recreate and test the load failure) and a test.

1. +++ b/core/tests/Drupal/Tests/Core/Config/StorageComparerTest.php
   @@ -284,6 +284,50 @@ public function testDifferentCollections() {
   +    $recreate_expected = [
   +      'field.storage.node.body',
   +      // TODO Should this be recreated?
   +      'views.view.test_view',
   +    ];
   

   This asserts the existing behavior with the undefined property evaluating to null. Is recreating actually correct though? That seems kinda odd.

2. +++ b/core/tests/Drupal/Tests/Core/Config/StorageComparerTest.php
   @@ -284,6 +284,50 @@ public function testDifferentCollections() {
   +    // TODO does asserting the order matter here?
   +    $this->assertEqualsCanonicalizing($recreate_expected, $this->storageComparer->getChangelist('create'));
   +    $this->assertEqualsCanonicalizing($recreate_expected, $this->storageComparer->getChangelist('delete'));
   

   I don't know if the order matters here. Or if we need to assert it since its probably better tested else where. I would appreciate someone more familiar with the methods chiming in here.

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

@neclimdul if webform is creating entities without a UUID then that's a bug in webform - no?

I got this doing a drush cex

A drush cr
- clear cache

appeared to resolve it

My Drupal version 10.0.9 (need to upgrade to latest)

this has come back

when developing drupal code to share with others, I would want to remove the uuid
https://drupal.stackexchange.com/questions/295320/best-practice-remove-u...

however, I believe in doing so, it causes the "Undefined index: uuid". This post appears to explain: https://drupal.stackexchange.com/questions/204859/site-uuid-in-source-st...

Re-rolled for a certain project but yes, it's the config that should be fixed.

yeah, I think the problem was more that the config to fix was the database store so fixing would require manual queries to serialized data structures. That or recreating the config by deleting and re-importing manually similar to proposed solution. The intention of the patch as claudiu put it was to let the config system detect and heal a bad database store.

Looking at the patch after many years, that's a bit difficult to see so I split the code out with a comment so hopefully that will make review and acceptance easier.

At this point I think config is at lot more stable and users that would have run into this probably already have and worked through but the patch should guard against future users getting into a bind.

Also updated title and IS to reflect what's being proposed.

Removing the test tag as it doesn't cause an error but does generate a warning

1) /var/www/html/core/lib/Drupal/Core/Config/StorageComparer.php:303
Undefined array key "uuid"

Can't run in the pipeline because of my role so did it locally.

Very neat approach setting it to NULL to fix itself. Comment is very nice too.

0 objections LGTM.

This does seem like a reasonable hardening but I don't think we should actually be setting the uuid in the target data here. This ends up changing the target data when it should not. Leaving a review and suggestion on the MR.

Review addressed

Feedback from @alexpott appears to be addressed

Updated the title since the MR does not actually repair storage anymore (at least in any different way than before).

If I am right, the issue description needs an update. If I am wrong, my apologies, please reset the title back. However, from what I can tell, the behavior of storage has not changed in the MR, other than an edge case no longer throwing a notice. I ran into this notice with config from the editoria11y contrib module, and the MR fixed it.

@john.oltman - well I guess you could consider it repairing when you run the import and it recreates the config with a uuid. But your new title is more accurate.

Committed and pushed c8bda45c62b to main and 23a4af7850e to 11.x. Thanks!

Committed 89b938b and pushed to 11.4.x. Thanks!