Problem/Motivation

It is currently possible to override of the uuid key in configuration entities. However this is problematic because the configuration importer needs to detect configuration renames, recreates and deletes using the uuid key and this is hardcoded because the module that supplies the configuration entity type might not yet be installed.

Proposed resolution

Ensure that the uuidKey is uuid.

Remaining tasks

Write patch with tests

User interface changes

None

API changes

Restrict uuidKey to only being uuid.

Data model changes

None

Why is this an rc target?

If a configuration entity used an uuid key other than uuid it would break ConfigImporter.

CommentFileSizeAuthor
#28 2604450-28-test.patch3.84 KBNikhil_110
#28 2604450-28.patch5.17 KBNikhil_110
#24 interdiff_23-24.txt1.37 KBNikhil_110
#24 interdiff_23-24-test.txt1.37 KBNikhil_110
#24 2604450-24.patch5.33 KBNikhil_110
#24 2604450-test-24.patch4 KBNikhil_110
#23 2604450-23.patch4.56 KBNikhil_110
#23 2604450-23-test.patch3.22 KBNikhil_110
#5 2604450-5.patch4.37 KBalexpott
#5 2-5-interdiff.txt665 bytesalexpott
#2 2604450-2.patch4.43 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Active » Needs review
FileSize
2604450-2.patch4.43 KB

I think this is the simplest thing to do.

At the moment the UUID key is basically enforced through the constructor of the ConfigEntityType constructor but this patch hardens and tests the implementation.

Status: Needs review » Needs work

The last submitted patch, 2: 2604450-2.patch, failed testing.

The last submitted patch, 2: 2604450-2.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs work » Needs review
FileSize
2-5-interdiff.txt665 bytes
2604450-5.patch4.37 KB

Doh...

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Needs review » Needs work

Ran these as test-only locally, they did not fail.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented

See #2052083: EntityType::getKey() should be used instead of hardcoding id, status, weight, etc., is this actually a bug these days?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Priority: Major » Normal

\Drupal\Core\Entity\Entity::uuid() still hardcodes uuid and #2052083: EntityType::getKey() should be used instead of hardcoding id, status, weight, etc. won't change that, so why do we have \Drupal\Core\Entity\EntityStorageBase::$uuidKey at all? Unless the entity class also overrides that method...

Shouldn't we just hardcode this in all entities?

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue tags: -rc target triage

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Issue tags: +Needs reroll, +Bug Smash Initiative

Is this still relevant? Can we get a failing test from the test changes above?

Nikhil_110’s picture

Nikhil_110 CreditAttribution: Nikhil_110 at Srijan | A Material+ Company commented
FileSize
2604450-23-test.patch3.22 KB
2604450-23.patch4.56 KB

Re-roll patch and added tests, please review.

Nikhil_110’s picture

Nikhil_110 CreditAttribution: Nikhil_110 at Srijan | A Material+ Company commented
Status: Needs work » Needs review
FileSize
2604450-test-24.patch4 KB
2604450-24.patch5.33 KB
interdiff_23-24-test.txt1.37 KB
interdiff_23-24.txt1.37 KB

Fixed Issue #23 Please review

The last submitted patch, 24: 2604450-test-24.patch, failed testing. View results

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

Seems reroll showed the issue does exist. Ran the tests for 11.x and all green there.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Version: 9.5.x-dev » 11.x-dev
Status: Reviewed & tested by the community » Needs work

Thanks for breathing life into this long stale issue!
Couple of minor observations/questions but this looks close.

I also think there's a risk of disruption here, so moving this to 11.x so it will be 10.2 only (next minor release).

Can we also get a change notice and release notes snippet?

Thanks!

  1. +++ b/core/modules/config/tests/config_test/src/Entity/ConfigTest.php
@@ -26,7 +26,8 @@
+ *     "status" = "status",
+ *     "uuid" = "fake_key_to_ensure_this_does_not_work"

    Are we using this in any of the tests? 

  2. +++ b/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php
@@ -36,6 +36,13 @@
+  protected $entityType;

@@ -78,6 +85,12 @@ class ConfigEntityStorageTest extends UnitTestCase {
+  protected $memoryCache;

    I don't think we need class properties if we're only using them in one test case, let's just make these inline variables

Nikhil_110’s picture

Nikhil_110 CreditAttribution: Nikhil_110 at Srijan | A Material+ Company commented
FileSize
2604450-28.patch5.17 KB
2604450-28-test.patch3.84 KB

Patch added against #27 Point - 2 and still Pending 1 point so need work.