Problem/Motivation

Split off from #1740378: Implement renames in the import cycle.

Handle importing configuration entities with the same ID but a different UUID. This can occur when someone creates a view called views.view.content_list and then sync this to their production site. On the development site they then decide to the delete the view and create a new view called views.view.content_list.

Proposed resolution

The storage comparer needs to detect this situation and instead of adding the configuration name to the update changelist add it to both the delete and create changelists.

Remaining tasks

This patch is blocked on #2124535: Prevent secondary configuration creates and deletes from breaking the ConfigImporter since secondary deletes are occuring.

User interface changes

None

API changes

None

Files: 
CommentFileSizeAuthor
#10 2224873.10.patch6.93 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,737 pass(es). View
#10 9-10-interdiff.txt840 bytesalexpott
#9 2224873.9.patch6.93 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,594 pass(es), 1 fail(s), and 0 exception(s). View
#9 6-9-interdiff.txt1.4 KBalexpott
#6 5-6-interdiff.txt3.33 KBalexpott
#6 2224873.6.patch7.94 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,743 pass(es). View
#5 2224873.5.patch6.7 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,668 pass(es), 7 fail(s), and 0 exception(s). View

Comments

beejeebus’s picture

Issue tags: +beta blocker
tim.plunkett’s picture

This, from ConfigEntityBase::preSave()

    if (!$this->isNew()) {
      $original = $storage_controller->loadUnchanged($this->id());
      // Ensure that the UUID cannot be changed for an existing entity.
      if ($original && ($original->uuid() != $this->uuid())) {
        throw new ConfigDuplicateUUIDException(String::format('Attempt to save a configuration entity %id with UUID %uuid when this entity already exists with UUID %original_uuid', array('%id' => $this->id(), '%uuid' => $this->uuid(), '%original_uuid' => $original->uuid())));
      }
    }

That made some amount of sense to me. Now we want to blow away the original entity with something potentially completely different?

beejeebus’s picture

re #2, this will happen 'outside' the config entity system somewhat.

when computing the change list, we'll compare UUIDs, and if we have an entity with the same name but different UUIDs, we'll add the name to the delete and create lists. so, the code snippet in #2 will not need to change.

tim.plunkett’s picture

Title: handle a config entity with the same name but different UUID being imported » Handle a config entity with the same name but different UUID being imported
Priority: Normal » Critical

All other beta blockers are critical.

I don't fully understand this one yet.

alexpott’s picture

Issue summary: View changes
FileSize
6.7 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,668 pass(es), 7 fail(s), and 0 exception(s). View

This blocked on #2124535: Prevent secondary configuration creates and deletes from breaking the ConfigImporter as the test it adds nicely illustrates the problem we have. Once all the field instances are deleted the field system helpfully deletes the fields :) the config importer then tries to delete the field - which no longer exists.

alexpott’s picture

Status: Active » Needs review
FileSize
7.94 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,743 pass(es). View
3.33 KB

Fixed FieldInstanceConfig to not do secondary deletes if a config sync is taking place and improved the test.

beejeebus’s picture

this looks good to me.

i don't understand the views snippets though, otherwise i'd RTBC it.

swentel’s picture

  1. +++ b/core/lib/Drupal/Core/Config/StorageComparer.php
    @@ -170,11 +170,27 @@ protected function addChangelistCreate() {
    +          // not match. This means that the the entity has been recreated so
    

    'the the'

  2. +++ b/core/modules/field/field.module
    @@ -223,6 +223,15 @@ function field_entity_bundle_rename($entity_type, $bundle_old, $bundle_new) {
    +  $view_displays = entity_load_multiple_by_properties('entity_view_display', array('targetEntityType' => $entity_type, 'bundle' => $bundle));
    +  foreach ($view_displays as $view_display) {
    +    $view_display->delete();
    +  }
    +  $form_displays = entity_load_multiple_by_properties('entity_form_display', array('targetEntityType' => $entity_type, 'bundle' => $bundle));
    +  foreach ($form_displays as $form_display) {
    +    $form_display->delete();
    +  }
    

    entity_entity_bundle_delete() does this too ...

alexpott’s picture

FileSize
1.4 KB
6.93 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,594 pass(es), 1 fail(s), and 0 exception(s). View

Interestingly the order modules are enabled in DUBT tests matters! Changing the order to match how it would be in a real drupal install means the entity_entity_bundle_rename is fired before field_entity_bundle_rename and everything works as expected. Going to raise a follow up issue about DUBT module ordering and NodeType::postDelete removing field instances and entity displays. This needs to be done in preDelete since a field instance should never exist for a bundle that does not exist.

alexpott’s picture

FileSize
840 bytes
6.93 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,737 pass(es). View

Missed point 1. from @swentel's review

beejeebus’s picture

Status: Needs review » Reviewed & tested by the community

looks ready to me.

The last submitted patch, 9: 2224873.9.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 2224873.10.patch, failed testing.

The last submitted patch, 5: 2224873.5.patch, failed testing.

alexpott’s picture

The fail in Drupal\image\Tests\ImageStylesPathAndUrlTest is unrelated.

alexpott’s picture

Status: Needs work » Needs review

10: 2224873.10.patch queued for re-testing.

alexpott’s picture

Status: Needs review » Reviewed & tested by the community

The test fails were unrelated to the patch - putting back to rtbc as per #10

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

  • Commit 682a214 on 8.x by catch:
    Issue #2224873 by alexpott: Handle a config entity with the same name...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.