Issue #1969698 by xjm, damiankloip, tim.plunkett, yched: ConfigEntity::save()...

Issue #1969698 by xjm, damiankloip, tim.plunkett, yched: ConfigEntity::save() should disallow saving ID/UUID conflicts (Field UUID changes can badly corrupt field data).

core/lib/Drupal/Core/Config/ConfigDuplicateUUIDException.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Config\ConfigDuplicateUUIDException.
+ */
+
+namespace Drupal\Core\Config;
+
+/**
+ * Exception thrown when a config object UUID causes a conflict.
+ */
+class ConfigDuplicateUUIDException extends ConfigException {
+
+}

core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php

@@ -8,6 +8,8 @@
 namespace Drupal\Core\Config\Entity;
 
 use Drupal\Core\Entity\Entity;
+use Drupal\Core\Entity\EntityStorageControllerInterface;
+use Drupal\Core\Config\ConfigDuplicateUUIDException;
 
 /**
  * Defines a base configuration entity class.
@@ -160,4 +162,30 @@ public function getExportProperties() {
     }
     return $properties;
   }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function preSave(EntityStorageControllerInterface $storage_controller) {
+    parent::preSave($storage_controller);
+
+    // Ensure this entity's UUID does not exist with a different ID, regardless
+    // of whether it's new or updated.
+    $matching_entities = $storage_controller->getQuery()
+      ->condition('uuid', $this->uuid())
+      ->execute();
+    $matched_entity = reset($matching_entities);
+    if (!empty($matched_entity) && ($matched_entity != $this->id())) {
+      throw new ConfigDuplicateUUIDException(format_string('Attempt to save a configuration entity %id with UUID %uuid when this UUID is already used for %matched', array('%id' => $this->id(), '%uuid' => $this->uuid(), '%matched' => $matched_entity)));
+    }
+
+    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(format_string('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())));
+      }
+    }
+  }
+
 }

core/lib/Drupal/Core/Config/Entity/ConfigStorageController.php

@@ -14,6 +14,7 @@
 use Drupal\Core\Config\Config;
 use Drupal\Core\Config\ConfigFactory;
 use Drupal\Core\Config\StorageInterface;
+use Drupal\Core\Entity\Query\QueryFactory;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -61,6 +62,13 @@ class ConfigStorageController extends EntityStorageControllerBase {
    */
   protected $configStorage;
 
+  /**
+   * The entity query factory.
+   *
+   * @var \Drupal\Core\Entity\Query\QueryFactory
+   */
+  protected $entityQueryFactory;
+
   /**
    * Constructs a ConfigStorageController object.
    *
@@ -72,8 +80,10 @@ class ConfigStorageController extends EntityStorageControllerBase {
    *   The config factory service.
    * @param \Drupal\Core\Config\StorageInterface $config_storage
    *   The config storage service.
+   * @param \Drupal\Core\Entity\Query\QueryFactory $entity_query_factory
+   *   The entity query factory.
    */
-  public function __construct($entity_type, array $entity_info, ConfigFactory $config_factory, StorageInterface $config_storage) {
+  public function __construct($entity_type, array $entity_info, ConfigFactory $config_factory, StorageInterface $config_storage, QueryFactory $entity_query_factory) {
     parent::__construct($entity_type, $entity_info);
 
     $this->idKey = $this->entityInfo['entity_keys']['id'];
@@ -87,6 +97,7 @@ public function __construct($entity_type, array $entity_info, ConfigFactory $con
 
     $this->configFactory = $config_factory;
     $this->configStorage = $config_storage;
+    $this->entityQueryFactory = $entity_query_factory;
   }
 
   /**
@@ -97,7 +108,8 @@ public static function createInstance(ContainerInterface $container, $entity_typ
       $entity_type,
       $entity_info,
       $container->get('config.factory'),
-      $container->get('config.storage')
+      $container->get('config.storage'),
+      $container->get('entity.query')
     );
   }
 
@@ -167,6 +179,22 @@ public function loadByProperties(array $values = array()) {
     return $entities;
   }
 
+  /**
+   * Returns an entity query instance.
+   *
+   * @param string $conjunction
+   *   - AND: all of the conditions on the query need to match.
+   *   - OR: at least one of the conditions on the query need to match.
+   *
+   * @return \Drupal\Core\Entity\Query\QueryInterface
+   *   The query instance.
+   *
+   * @see \Drupal\Core\Entity\EntityStorageControllerInterface::getQueryServicename()
+   */
+  public function getQuery($conjunction = 'AND') {
+    return $this->entityQueryFactory->get($this->entityType, $conjunction);
+  }
+
   /**
    * Returns the config prefix used by the configuration entity type.
    *

core/modules/config/lib/Drupal/config/Tests/ConfigEntityStorageControllerTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\config\Tests\ConfigEntityStorageControllerTest.
+ */
+
+namespace Drupal\config\Tests;
+
+use Drupal\simpletest\DrupalUnitTestBase;
+use Drupal\Component\Uuid\Uuid;
+use Drupal\Core\Config\ConfigDuplicateUUIDException;
+
+/**
+ * Tests importing config entity data when the ID or UUID matches existing data.
+ */
+class ConfigEntityStorageControllerTest extends DrupalUnitTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('config_test');
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Configuration entity UUID conflict',
+      'description' => 'Tests staging and importing config entities with IDs and UUIDs that match existing config.',
+      'group' => 'Configuration',
+    );
+  }
+
+  /**
+   * Tests importing fields and instances with changed IDs or UUIDs.
+   */
+  public function testUUIDConflict() {
+    $entity_type = 'config_test';
+    $id = 'test_1';
+    // Load the original field and instance entities.
+    entity_create($entity_type, array('id' => $id))->save();
+    $entity = entity_load($entity_type, $id);
+
+    $original_properties = $entity->getExportProperties();
+
+    // Override with a new UUID and try to save.
+    $uuid = new Uuid();
+    $new_uuid = $uuid->generate();
+    $entity->set('uuid', $new_uuid);
+
+    try {
+      $entity->save();
+      $this->fail('Exception thrown when attempting to save a configuration entity with a UUID that does not match the existing UUID.');
+    }
+    catch (ConfigDuplicateUUIDException $e) {
+      $this->pass(format_string('Exception thrown when attempting to save a configuration entity with a UUID that does not match existing data: %e.', array('%e' => $e)));
+    }
+
+    // Ensure that the config entity was not corrupted.
+    $entity = entity_load('config_test', $entity->id(), TRUE);
+    $this->assertIdentical($entity->getExportProperties(), $original_properties);
+  }
+
+}

core/modules/config/lib/Drupal/config/Tests/ConfigEntityTest.php

@@ -154,6 +154,8 @@ function testCRUD() {
     $this->assertIdentical($same_id->label(), '');
     $this->assertNotEqual($same_id->uuid(), $config_test->uuid());
 
+    // Delete the overridden entity first.
+    $same_id->delete();
     // Revert to previous state.
     $config_test->save();
 

core/modules/field/lib/Drupal/field/FieldInstanceStorageController.php

@@ -9,6 +9,7 @@
 
 use Drupal\Core\Config\Config;
 use Drupal\Core\Config\Entity\ConfigStorageController;
+use Drupal\Core\Entity\Query\QueryFactory;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Drupal\Core\Config\ConfigFactory;
 use Drupal\Core\Config\StorageInterface;
@@ -65,8 +66,8 @@ class FieldInstanceStorageController extends ConfigStorageController {
    * @param \Drupal\Core\KeyValueStore\KeyValueStoreInterface $state
    *   The state key value store.
    */
-  public function __construct($entity_type, array $entity_info, ConfigFactory $config_factory, StorageInterface $config_storage, EntityManager $entity_manager, ModuleHandler $module_handler, KeyValueStoreInterface $state) {
-    parent::__construct($entity_type, $entity_info, $config_factory, $config_storage);
+  public function __construct($entity_type, array $entity_info, ConfigFactory $config_factory, StorageInterface $config_storage, QueryFactory $entity_query_factory, EntityManager $entity_manager, ModuleHandler $module_handler, KeyValueStoreInterface $state) {
+    parent::__construct($entity_type, $entity_info, $config_factory, $config_storage, $entity_query_factory);
     $this->entityManager = $entity_manager;
     $this->moduleHandler = $module_handler;
     $this->state = $state;
@@ -81,6 +82,7 @@ public static function createInstance(ContainerInterface $container, $entity_typ
       $entity_info,
       $container->get('config.factory'),
       $container->get('config.storage'),
+      $container->get('entity.query'),
       $container->get('plugin.manager.entity'),
       $container->get('module_handler'),
       $container->get('state')

core/modules/field/lib/Drupal/field/FieldStorageController.php

@@ -9,6 +9,7 @@
 
 use Drupal\Core\Config\Config;
 use Drupal\Core\Config\Entity\ConfigStorageController;
+use Drupal\Core\Entity\Query\QueryFactory;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Drupal\Core\Config\ConfigFactory;
 use Drupal\Core\Config\StorageInterface;
@@ -60,8 +61,9 @@ class FieldStorageController extends ConfigStorageController {
    * @param \Drupal\Core\KeyValueStore\KeyValueStoreInterface $state
    *   The state key value store.
    */
-  public function __construct($entity_type, array $entity_info, ConfigFactory $config_factory, StorageInterface $config_storage, EntityManager $entity_manager, ModuleHandler $module_handler, KeyValueStoreInterface $state) {
-    parent::__construct($entity_type, $entity_info, $config_factory, $config_storage);
+  public function __construct($entity_type, array $entity_info, ConfigFactory $config_factory, StorageInterface $config_storage, QueryFactory $entity_query_factory, EntityManager $entity_manager, ModuleHandler $module_handler, KeyValueStoreInterface $state) {
+    parent::__construct($entity_type, $entity_info, $config_factory, $config_storage, $entity_query_factory);
+
     $this->entityManager = $entity_manager;
     $this->moduleHandler = $module_handler;
     $this->state = $state;
@@ -76,6 +78,7 @@ public static function createInstance(ContainerInterface $container, $entity_typ
       $entity_info,
       $container->get('config.factory'),
       $container->get('config.storage'),
+      $container->get('entity.query'),
       $container->get('plugin.manager.entity'),
       $container->get('module_handler'),
       $container->get('state')

core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.php

@@ -58,6 +58,13 @@ class ImageStyle extends ConfigEntityBase implements ImageStyleInterface {
    */
   public $label;
 
+  /**
+   * The UUID for this entity.
+   *
+   * @var string
+   */
+  public $uuid;
+
   /**
    * The array of image effects for this image style.
    *

core/modules/user/tests/Drupal/user/Tests/Views/Argument/RolesRidTest.php

@@ -60,10 +60,14 @@ public function testTitleQuery() {
     $config_factory = $this->getConfigFactoryStub($config);
     $config_storage = $this->getConfigStorageStub($config);
 
+    $entity_query_factory = $this->getMockBuilder('Drupal\Core\Entity\Query\QueryFactory')
+      ->disableOriginalConstructor()
+      ->getMock();
+
     // Creates a stub role storage controller and replace the attachLoad()
     // method with an empty version, because attachLoad() calls
     // module_implements().
-    $role_storage_controller = $this->getMock('Drupal\user\RoleStorageController', array('attachLoad'), array('user_role', static::$entityInfo, $config_factory, $config_storage));
+    $role_storage_controller = $this->getMock('Drupal\user\RoleStorageController', array('attachLoad'), array('user_role', static::$entityInfo, $config_factory, $config_storage, $entity_query_factory));
 
 
     $entity_manager = $this->getMockBuilder('Drupal\Core\Entity\EntityManager')

core/modules/views/lib/Drupal/views/Tests/ViewStorageTest.php

@@ -179,6 +179,8 @@ protected function displayTests() {
     $executable->initDisplay();
     $this->assertTrue($executable->displayHandlers->get($new_id) instanceof Page, 'New page display "test" uses the right display plugin.');
 
+    // To save this with a new ID, we should use createDuplicate().
+    $view = $view->createDuplicate();
     $view->set('id', 'test_view_storage_new_new2');
     $view->save();
     $values = config('views.view.test_view_storage_new_new2')->get();

core/modules/views_ui/lib/Drupal/views_ui/ViewCloneFormController.php

@@ -74,8 +74,9 @@ protected function actions(array $form, array &$form_state) {
    * Overrides \Drupal\Core\Entity\EntityFormController::form().
    */
   public function submit(array $form, array &$form_state) {
-    $this->entity = parent::submit($form, $form_state);
-    $this->entity->setOriginalID(NULL);
+    $original = parent::submit($form, $form_state);
+    $this->entity = $original->createDuplicate();
+    $this->entity->set('id', $form_state['values']['id']);
     $this->entity->save();
 
     // Redirect the user to the view admin form.