Issue #2226027 by tim.plunkett: ConfigEntityBase::preSave() tries to load the...

Issue #2226027 by tim.plunkett: ConfigEntityBase::preSave() tries to load the original entity but instead loads itself.

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

@@ -278,8 +278,9 @@ public function preSave(EntityStorageInterface $storage) {
       throw new ConfigDuplicateUUIDException(String::format('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 entity is not new, load the original entity for comparison.
     if (!$this->isNew()) {
-      $original = $storage->loadUnchanged($this->id());
+      $original = $storage->loadUnchanged($this->getOriginalId());
       // 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())));

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

@@ -18,6 +18,7 @@
 use Drupal\Core\Entity\EntityStorageException;
 use Drupal\Core\Entity\Query\QueryFactory;
 use Drupal\Component\Uuid\UuidInterface;
+use Drupal\Core\Language\LanguageManagerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -65,6 +66,13 @@ class ConfigEntityStorage extends EntityStorageBase implements ConfigEntityStora
    */
   protected $configStorage;
 
+  /**
+   * The language manager.
+   *
+   * @var \Drupal\Core\Language\LanguageManagerInterface
+   */
+  protected $languageManager;
+
   /**
    * Constructs a ConfigEntityStorage object.
    *
@@ -76,8 +84,10 @@ class ConfigEntityStorage extends EntityStorageBase implements ConfigEntityStora
    *   The config storage service.
    * @param \Drupal\Component\Uuid\UuidInterface $uuid_service
    *   The UUID service.
+   * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager
+   *   The language manager.
    */
-  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service) {
+  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service, LanguageManagerInterface $language_manager) {
     parent::__construct($entity_type);
 
     $this->idKey = $this->entityType->getKey('id');
@@ -86,6 +96,7 @@ public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInter
     $this->configFactory = $config_factory;
     $this->configStorage = $config_storage;
     $this->uuidService = $uuid_service;
+    $this->languageManager = $language_manager;
   }
 
   /**
@@ -96,7 +107,8 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
       $entity_type,
       $container->get('config.factory'),
       $container->get('config.storage'),
-      $container->get('uuid')
+      $container->get('uuid'),
+      $container->get('language_manager')
     );
   }
 
@@ -228,7 +240,7 @@ public function create(array $values = array()) {
     $class::preCreate($this, $values);
 
     // Set default language to site default if not provided.
-    $values += array('langcode' => language_default()->id);
+    $values += array('langcode' => $this->languageManager->getDefaultLanguage()->id);
 
     $entity = new $class($values, $this->entityTypeId);
     // Mark this entity as new, so isNew() returns TRUE. This does not check
@@ -331,9 +343,6 @@ public function save(EntityInterface $entity) {
       $config->save();
       $entity->postSave($this, TRUE);
       $this->invokeHook('update', $entity);
-
-      // Immediately update the original ID.
-      $entity->setOriginalId($entity->id());
     }
     else {
       $return = SAVED_NEW;
@@ -343,6 +352,11 @@ public function save(EntityInterface $entity) {
       $this->invokeHook('insert', $entity);
     }
 
+    // After saving, this is now the "original entity", and subsequent saves
+    // will be updates instead of inserts, and updates must always be able to
+    // correctly identify the original entity.
+    $entity->setOriginalId($entity->id());
+
     unset($entity->original);
 
     return $return;

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

@@ -314,7 +314,9 @@ public function delete() {
   public function createDuplicate() {
     $duplicate = clone $this;
     $entity_type = $this->getEntityType();
+    // Reset the entity ID and indicate that this is a new entity.
     $duplicate->{$entity_type->getKey('id')} = NULL;
+    $duplicate->enforceIsNew();
 
     // Check if the entity type supports UUIDs and generate a new one if so.
     if ($entity_type->hasKey('uuid')) {

core/modules/breakpoint/lib/Drupal/breakpoint/Entity/Breakpoint.php

@@ -105,6 +105,18 @@ class Breakpoint extends ConfigEntityBase implements BreakpointInterface {
    */
   public $multipliers = array();
 
+  /**
+   * {@inheritdoc}
+   */
+  public function id() {
+    // If no ID is specified, build one from the properties that uniquely define
+    // this breakpoint.
+    if (!isset($this->id)) {
+      $this->id = $this->sourceType . '.' . $this->source . '.' . $this->name;
+    }
+    return $this->id;
+  }
+
   /**
    * Overrides Drupal\config\ConfigEntityBase::save().
    */
@@ -114,13 +126,6 @@ public function save() {
       throw new InvalidBreakpointException('Invalid data detected.');
     }
 
-    // Build an id if none is set.
-    // Since a particular name can be used by multiple theme/modules we need
-    // to make a unique id.
-    if (empty($this->id)) {
-      $this->id = $this->sourceType . '.' . $this->source . '.' . $this->name;
-    }
-
     // Set the label if none is set.
     if (empty($this->label)) {
       $this->label = $this->name;

core/modules/breakpoint/lib/Drupal/breakpoint/Entity/BreakpointGroup.php

@@ -104,10 +104,19 @@ public function save() {
     if (!$this->isValid()) {
       throw new InvalidBreakpointException('Invalid data detected.');
     }
-    if (empty($this->id)) {
+    parent::save();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function id() {
+    // If no ID is specified, build one from the properties that uniquely define
+    // this breakpoint group.
+    if (!isset($this->id)) {
       $this->id = $this->sourceType . '.' . $this->source . '.' . $this->name;
     }
-    parent::save();
+    return $this->id;
   }
 
   /**

core/modules/breakpoint/lib/Drupal/breakpoint/Tests/BreakpointAPITest.php

@@ -46,7 +46,7 @@ public function testConfigName() {
     $this->assertTrue($exception, 'breakpoint_config_name: An exception is thrown when an invalid sourceType is entered.');
 
     // Try an invalid source.
-    $breakpoint->id = '';
+    $breakpoint = $breakpoint->createDuplicate();
     $breakpoint->sourceType = Breakpoint::SOURCE_TYPE_USER_DEFINED;
     $breakpoint->source = 'custom*_module source';
 
@@ -60,7 +60,7 @@ public function testConfigName() {
     $this->assertTrue($exception, 'breakpoint_config_name: An exception is thrown when an invalid source is entered.');
 
     // Try an invalid name (make sure there is at least once capital letter).
-    $breakpoint->id = '';
+    $breakpoint = $breakpoint->createDuplicate();
     $breakpoint->source = 'custom_module';
     $breakpoint->name = drupal_ucfirst($this->randomName());
 
@@ -74,7 +74,7 @@ public function testConfigName() {
     $this->assertTrue($exception, 'breakpoint_config_name: An exception is thrown when an invalid name is entered.');
 
     // Try a valid breakpoint.
-    $breakpoint->id = '';
+    $breakpoint = $breakpoint->createDuplicate();
     $breakpoint->name = drupal_strtolower($this->randomName());
     $breakpoint->mediaQuery = 'all';
 

core/modules/breakpoint/lib/Drupal/breakpoint/Tests/BreakpointGroupAPITest.php

@@ -49,6 +49,7 @@ public function testConfigName() {
     $this->assertTrue($exception, 'An exception is thrown when an invalid sourceType is entered.');
 
     // Try an invalid source.
+    $breakpoint_group = $breakpoint_group->createDuplicate();
     $breakpoint_group->name = '';
     $breakpoint_group->sourceType = Breakpoint::SOURCE_TYPE_USER_DEFINED;
     $breakpoint_group->source = 'custom*_module source';
@@ -63,6 +64,7 @@ public function testConfigName() {
     $this->assertTrue($exception, 'An exception is thrown when an invalid source is entered.');
 
     // Try a valid breakpoint_group.
+    $breakpoint_group = $breakpoint_group->createDuplicate();
     $breakpoint_group->name = 'test';
     $breakpoint_group->source = 'custom_module_source';
 

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

@@ -13,6 +13,7 @@
 use Drupal\Core\Entity\EntityManagerInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Entity\Query\QueryFactory;
+use Drupal\Core\Language\LanguageManagerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Config\StorageInterface;
@@ -56,6 +57,8 @@ class FieldConfigStorage extends ConfigEntityStorage {
    *   The config storage service.
    * @param \Drupal\Component\Uuid\UuidInterface $uuid_service
    *   The UUID service.
+   * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager
+   *   The language manager.
    * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
    *   The entity manager.
    * @param \Drupal\Core\Extension\ModuleHandler $module_handler
@@ -63,8 +66,8 @@ class FieldConfigStorage extends ConfigEntityStorage {
    * @param \Drupal\Core\KeyValueStore\StateInterface $state
    *   The state key value store.
    */
-  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service, EntityManagerInterface $entity_manager, ModuleHandler $module_handler, StateInterface $state) {
-    parent::__construct($entity_type, $config_factory, $config_storage, $uuid_service);
+  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service, LanguageManagerInterface $language_manager, EntityManagerInterface $entity_manager, ModuleHandler $module_handler, StateInterface $state) {
+    parent::__construct($entity_type, $config_factory, $config_storage, $uuid_service, $language_manager);
     $this->entityManager = $entity_manager;
     $this->moduleHandler = $module_handler;
     $this->state = $state;
@@ -79,6 +82,7 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
       $container->get('config.factory'),
       $container->get('config.storage'),
       $container->get('uuid'),
+      $container->get('language_manager'),
       $container->get('entity.manager'),
       $container->get('module_handler'),
       $container->get('state')

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

@@ -12,6 +12,7 @@
 use Drupal\Core\Entity\EntityManagerInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Entity\Query\QueryFactory;
+use Drupal\Core\Language\LanguageManagerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Component\Uuid\UuidInterface;
@@ -54,13 +55,15 @@ class FieldInstanceConfigStorage extends ConfigEntityStorage {
    *   The config storage service.
    * @param \Drupal\Component\Uuid\UuidInterface $uuid_service
    *   The UUID service.
+   * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager
+   *   The language manager.
    * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
    *   The entity manager.
    * @param \Drupal\Core\KeyValueStore\StateInterface $state
    *   The state key value store.
    */
-  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service, EntityManagerInterface $entity_manager, StateInterface $state) {
-    parent::__construct($entity_type, $config_factory, $config_storage, $uuid_service);
+  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service, LanguageManagerInterface $language_manager, EntityManagerInterface $entity_manager, StateInterface $state) {
+    parent::__construct($entity_type, $config_factory, $config_storage, $uuid_service, $language_manager);
     $this->entityManager = $entity_manager;
     $this->state = $state;
   }
@@ -74,6 +77,7 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
       $container->get('config.factory'),
       $container->get('config.storage'),
       $container->get('uuid'),
+      $container->get('language_manager'),
       $container->get('entity.manager'),
       $container->get('state')
     );

core/modules/filter/lib/Drupal/filter/Tests/FilterCrudTest.php

@@ -41,9 +41,10 @@ function testTextFormatCrud() {
     $this->verifyTextFormat($format);
 
     // Add another text format specifying all possible properties.
-    $format = entity_create('filter_format');
-    $format->format = 'custom_format';
-    $format->name = 'Custom format';
+    $format = entity_create('filter_format', array(
+      'format' => 'custom_format',
+      'name' => 'Custom format',
+    ));
     $format->setFilterConfig('filter_url', array(
       'status' => 1,
       'settings' => array(

core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityBaseUnitTest.php

@@ -369,6 +369,8 @@ public function testCreateDuplicate() {
     $duplicate = $this->entity->createDuplicate();
     $this->assertInstanceOf('\Drupal\Core\Entity\Entity', $duplicate);
     $this->assertNotSame($this->entity, $duplicate);
+    $this->assertFalse($this->entity->isNew());
+    $this->assertTrue($duplicate->isNew());
     $this->assertNull($duplicate->id());
     $this->assertNull($duplicate->getOriginalId());
     $this->assertNotEquals($this->entity->uuid(), $duplicate->uuid());