diff --git a/core/includes/menu.inc b/core/includes/menu.inc index 385ef48f27e8..46cedbcf1c92 100644 --- a/core/includes/menu.inc +++ b/core/includes/menu.inc @@ -823,6 +823,9 @@ function menu_link_rebuild_defaults() { $link['customized'] = $existing_item->customized; $link['updated'] = $existing_item->updated; $menu_link = $menu_link_storage->createFromDefaultLink($link); + // @todo Do not create a new entity in order to update it, see + // https://drupal.org/node/2241865 + $menu_link->setOriginalId($existing_item->mlid); // Convert the existing item to a typed object. /** @var \Drupal\menu_link\MenuLinkInterface $existing_item */ diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php index ec90b7e04a4a..76ba2ae103ae 100644 --- a/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php @@ -114,7 +114,7 @@ abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface public function setOriginalId($id) { $this->originalId = $id; - return $this; + return parent::setOriginalId($id); } /** diff --git a/core/lib/Drupal/Core/Entity/ContentEntityBase.php b/core/lib/Drupal/Core/Entity/ContentEntityBase.php index f1cf552c2983..6354ea9d2966 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityBase.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityBase.php @@ -925,6 +925,7 @@ abstract class ContentEntityBase extends Entity implements \IteratorAggregate, C $duplicate = clone $this; $entity_type = $this->getEntityType(); $duplicate->{$entity_type->getKey('id')}->value = NULL; + $duplicate->enforceIsNew(); // Check if the entity type supports UUIDs and generate a new one if so. if ($entity_type->hasKey('uuid')) { diff --git a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php index 0c2bfe49f884..cf43ee66f915 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php @@ -67,6 +67,7 @@ abstract class ContentEntityStorageBase extends EntityStorageBase implements Fie $bundle = $values[$this->bundleKey]; } $entity = new $entity_class(array(), $this->entityTypeId, $bundle); + $entity->enforceIsNew(); foreach ($entity as $name => $field) { if (isset($values[$name])) { diff --git a/core/lib/Drupal/Core/Entity/Entity.php b/core/lib/Drupal/Core/Entity/Entity.php index ed95b23127a1..a3f1ef7c8693 100644 --- a/core/lib/Drupal/Core/Entity/Entity.php +++ b/core/lib/Drupal/Core/Entity/Entity.php @@ -502,6 +502,12 @@ abstract class Entity extends DependencySerialization implements EntityInterface */ public function setOriginalId($id) { // By default, entities do not support renames and do not have original IDs. + // If the specified ID is anything except NULL, this should mark this entity + // as no longer new. + if ($id !== NULL) { + $this->enforceIsNew(FALSE); + } + return $this; } diff --git a/core/lib/Drupal/Core/Entity/EntityDatabaseStorage.php b/core/lib/Drupal/Core/Entity/EntityDatabaseStorage.php index c25ff56750ed..180dfefc84e4 100644 --- a/core/lib/Drupal/Core/Entity/EntityDatabaseStorage.php +++ b/core/lib/Drupal/Core/Entity/EntityDatabaseStorage.php @@ -238,6 +238,7 @@ class EntityDatabaseStorage extends EntityStorageBase { $entity_class::preCreate($this, $values); $entity = new $entity_class($values, $this->entityTypeId); + $entity->enforceIsNew(); // Assign a new UUID if there is none yet. if ($this->uuidKey && !isset($entity->{$this->uuidKey})) { diff --git a/core/modules/comment/comment.module b/core/modules/comment/comment.module index ba4bb479da72..539cce8f2a49 100644 --- a/core/modules/comment/comment.module +++ b/core/modules/comment/comment.module @@ -1281,7 +1281,11 @@ function comment_prepare_author(CommentInterface $comment) { // The account has been pre-loaded by CommentViewBuilder::buildContent(). $account = $comment->getOwner(); if (empty($account->uid->value)) { + // @todo Avoid creating a new entity by just creating a new instance + // directly, see https://drupal.org/node/1867228. $account = entity_create('user', array('uid' => 0, 'name' => $comment->getAuthorName(), 'homepage' => $comment->getHomepage())); + // The anonymous user is not a new account, do not treat it as one. + $account->enforceIsNew(FALSE); } return $account; } diff --git a/core/modules/field/lib/Drupal/field/Tests/FieldAttachOtherTest.php b/core/modules/field/lib/Drupal/field/Tests/FieldAttachOtherTest.php index 9e4212660a93..39f6e283a46e 100644 --- a/core/modules/field/lib/Drupal/field/Tests/FieldAttachOtherTest.php +++ b/core/modules/field/lib/Drupal/field/Tests/FieldAttachOtherTest.php @@ -226,10 +226,6 @@ class FieldAttachOtherTest extends FieldUnitTestBase { // Update with different values, and check that the cache entry is wiped. $values = $this->_generateTestFieldValues($this->field_2->getCardinality()); - $entity = entity_create($entity_type, array( - 'type' => $entity_type, - 'id' => $entity->id(), - )); $entity->{$this->field_name_2} = $values; $entity->save(); $this->assertFalse(\Drupal::cache('entity')->get($cid), 'Cached: no cache entry on update'); @@ -241,10 +237,6 @@ class FieldAttachOtherTest extends FieldUnitTestBase { $this->assertEqual($cache->data[$langcode][$this->field_name_2], $values, 'Cached: correct cache entry on load'); // Create a new revision, and check that the cache entry is wiped. - $entity = entity_create($entity_type, array( - 'type' => $entity_type, - 'id' => $entity->id(), - )); $values = $this->_generateTestFieldValues($this->field_2->getCardinality()); $entity->{$this->field_name_2} = $values; $entity->setNewRevision(); diff --git a/core/modules/field/lib/Drupal/field/Tests/FieldAttachStorageTest.php b/core/modules/field/lib/Drupal/field/Tests/FieldAttachStorageTest.php index efb6ae80a6b7..a80df81e4ff0 100644 --- a/core/modules/field/lib/Drupal/field/Tests/FieldAttachStorageTest.php +++ b/core/modules/field/lib/Drupal/field/Tests/FieldAttachStorageTest.php @@ -190,6 +190,9 @@ class FieldAttachStorageTest extends FieldUnitTestBase { $entity = $this->entitySaveReload($entity); $this->assertTrue($entity->{$this->field_name}->isEmpty(), 'Insert: NULL field results in no value saved'); + // All saves after this point should be updates, not inserts. + $entity_init->enforceIsNew(FALSE); + // Add some real data. $entity = clone($entity_init); $values = $this->_generateTestFieldValues(1); diff --git a/core/modules/file/file.module b/core/modules/file/file.module index df600f04cac0..9fb3c60dd7c9 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -173,11 +173,14 @@ function file_copy(File $source, $destination = NULL, $replace = FILE_EXISTS_REN $file->setFileUri($uri); $file->setFilename(drupal_basename($uri)); // If we are replacing an existing file re-use its database record. + // @todo Do not create a new entity in order to update it, see + // https://drupal.org/node/2241865 if ($replace == FILE_EXISTS_REPLACE) { $existing_files = entity_load_multiple_by_properties('file', array('uri' => $uri)); if (count($existing_files)) { $existing = reset($existing_files); $file->fid = $existing->id(); + $file->setOriginalId($existing->id()); $file->setFilename($existing->getFilename()); } } @@ -254,6 +257,7 @@ function file_move(File $source, $destination = NULL, $replace = FILE_EXISTS_REN $existing = reset($existing_files); $delete_source = TRUE; $file->fid = $existing->id(); + $file->uuid = $existing->uuid(); } } // If we are renaming around an existing file (rather than a directory), @@ -520,11 +524,14 @@ function file_save_data($data, $destination = NULL, $replace = FILE_EXISTS_RENAM 'status' => FILE_STATUS_PERMANENT, )); // If we are replacing an existing file re-use its database record. + // @todo Do not create a new entity in order to update it, see + // https://drupal.org/node/2241865 if ($replace == FILE_EXISTS_REPLACE) { $existing_files = entity_load_multiple_by_properties('file', array('uri' => $uri)); if (count($existing_files)) { $existing = reset($existing_files); $file->fid = $existing->id(); + $file->setOriginalId($existing->id()); $file->setFilename($existing->getFilename()); } } @@ -946,11 +953,14 @@ function file_save_upload($form_field_name, $validators = array(), $destination drupal_chmod($file->getFileUri()); // If we are replacing an existing file re-use its database record. + // @todo Do not create a new entity in order to update it, see + // https://drupal.org/node/2241865 if ($replace == FILE_EXISTS_REPLACE) { $existing_files = entity_load_multiple_by_properties('file', array('uri' => $file->getFileUri())); if (count($existing_files)) { $existing = reset($existing_files); $file->fid = $existing->id(); + $file->setOriginalId($existing->id()); } } diff --git a/core/modules/file/lib/Drupal/file/Tests/FileManagedTestBase.php b/core/modules/file/lib/Drupal/file/Tests/FileManagedTestBase.php index c77769133909..91edf6b30d1a 100644 --- a/core/modules/file/lib/Drupal/file/Tests/FileManagedTestBase.php +++ b/core/modules/file/lib/Drupal/file/Tests/FileManagedTestBase.php @@ -150,22 +150,19 @@ abstract class FileManagedTestBase extends WebTestBase { * File entity. */ function createFile($filepath = NULL, $contents = NULL, $scheme = NULL) { - $file = new \stdClass(); - $file->uri = $this->createUri($filepath, $contents, $scheme); - $file->filename = drupal_basename($file->uri); - $file->filemime = 'text/plain'; - $file->uid = 1; - $file->created = REQUEST_TIME; - $file->changed = REQUEST_TIME; - $file->filesize = filesize($file->uri); - $file->status = 0; + // Don't count hook invocations caused by creating the file. + \Drupal::state()->set('file_test.count_hook_invocations', FALSE); + $file = entity_create('file', array( + 'uri' => $this->createUri($filepath, $contents, $scheme), + 'uid' => 1, + )); + $file->save(); // Write the record directly rather than using the API so we don't invoke // the hooks. - $file = (array) $file; - $file['fid'] = db_insert('file_managed') - ->fields($file) - ->execute(); - return entity_create('file', $file); + $this->assertTrue($file->id() > 0, 'The file was added to the database.', 'Create test file'); + + \Drupal::state()->set('file_test.count_hook_invocations', TRUE); + return $file; } /** diff --git a/core/modules/file/lib/Drupal/file/Tests/FileManagedUnitTestBase.php b/core/modules/file/lib/Drupal/file/Tests/FileManagedUnitTestBase.php index c9ec86d203bb..8c83ac7ea43e 100644 --- a/core/modules/file/lib/Drupal/file/Tests/FileManagedUnitTestBase.php +++ b/core/modules/file/lib/Drupal/file/Tests/FileManagedUnitTestBase.php @@ -161,20 +161,19 @@ abstract class FileManagedUnitTestBase extends DrupalUnitTestBase { * File entity. */ function createFile($filepath = NULL, $contents = NULL, $scheme = NULL) { - $file = new \stdClass(); - $file->uri = $this->createUri($filepath, $contents, $scheme); - $file->filename = drupal_basename($file->uri); - $file->filemime = 'text/plain'; - $file->uid = 1; - $file->created = REQUEST_TIME; - $file->changed = REQUEST_TIME; - $file->filesize = filesize($file->uri); - $file->status = 0; + // Don't count hook invocations caused by creating the file. + \Drupal::state()->set('file_test.count_hook_invocations', FALSE); + $file = entity_create('file', array( + 'uri' => $this->createUri($filepath, $contents, $scheme), + 'uid' => 1, + )); + $file->save(); // Write the record directly rather than using the API so we don't invoke // the hooks. - $this->assertNotIdentical(drupal_write_record('file_managed', $file), FALSE, 'The file was added to the database.', 'Create test file'); + $this->assertTrue($file->id() > 0, 'The file was added to the database.', 'Create test file'); - return entity_create('file', (array) $file); + \Drupal::state()->set('file_test.count_hook_invocations', TRUE); + return $file; } /** diff --git a/core/modules/file/tests/file_test/file_test.module b/core/modules/file/tests/file_test/file_test.module index e5a10a04a41b..5fcd15fd9157 100644 --- a/core/modules/file/tests/file_test/file_test.module +++ b/core/modules/file/tests/file_test/file_test.module @@ -108,9 +108,11 @@ function file_test_get_all_calls() { * @see file_test_reset() */ function _file_test_log_call($op, $args) { - $results = \Drupal::state()->get('file_test.results') ?: array(); - $results[$op][] = $args; - \Drupal::state()->set('file_test.results', $results); + if (\Drupal::state()->get('file_test.count_hook_invocations', TRUE)) { + $results = \Drupal::state()->get('file_test.results') ?: array(); + $results[$op][] = $args; + \Drupal::state()->set('file_test.results', $results); + } } /** diff --git a/core/modules/menu_link/lib/Drupal/menu_link/Entity/MenuLink.php b/core/modules/menu_link/lib/Drupal/menu_link/Entity/MenuLink.php index b500b484fa57..e979dbbde8ef 100644 --- a/core/modules/menu_link/lib/Drupal/menu_link/Entity/MenuLink.php +++ b/core/modules/menu_link/lib/Drupal/menu_link/Entity/MenuLink.php @@ -369,7 +369,10 @@ class MenuLink extends Entity implements \ArrayAccess, MenuLinkInterface { $original['machine_name'] = $this->machine_name; /** @var \Drupal\menu_link\MenuLinkStorageInterface $storage */ $storage = \Drupal::entityManager()->getStorage($this->entityTypeId); + // @todo Do not create a new entity in order to update it, see + // https://drupal.org/node/2241865 $new_link = $storage->createFromDefaultLink($original); + $new_link->setOriginalId($this->id()); // Allow the menu to be determined by the parent if (!empty($new_link['parent']) && !empty($all_links[$new_link['parent']])) { // Walk up the tree to find the menu name. diff --git a/core/modules/menu_ui/menu_ui.module b/core/modules/menu_ui/menu_ui.module index 3bd9b9fa1816..d890237bf78a 100644 --- a/core/modules/menu_ui/menu_ui.module +++ b/core/modules/menu_ui/menu_ui.module @@ -564,7 +564,14 @@ function menu_ui_form_node_form_alter(&$form, $form_state) { */ function menu_ui_node_submit(EntityInterface $node, $form, $form_state) { if (!empty($form_state['values']['menu'])) { + $original_menu_id = !empty($node->menu) ? $node->menu->id() : NULL; $node->menu = entity_create('menu_link', $form_state['values']['menu']); + // @todo Do not create a new entity in order to update it, see + // https://drupal.org/node/2241865 + // If this menu had a previous menu link associated, mark it as not new. + if ($original_menu_id) { + $node->menu->setOriginalId($original_menu_id); + } // Decompose the selected menu parent option into 'menu_name' and 'plid', if // the form used the default parent selection widget. if (!empty($form_state['values']['menu']['parent'])) { diff --git a/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php b/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php index caf022674fc7..20f4a09b6846 100644 --- a/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php +++ b/core/tests/Drupal/Tests/Core/Config/Entity/ConfigEntityStorageTest.php @@ -515,8 +515,8 @@ class ConfigEntityStorageTest extends UnitTestCase { ->will($this->returnValue(array('baz'))); $entity = $this->getMockEntity(array('id' => 'foo')); - $entity->enforceIsNew(); $entity->setOriginalId('baz'); + $entity->enforceIsNew(); $this->entityStorage->save($entity); } diff --git a/core/tests/Drupal/Tests/Core/Entity/ContentEntityDatabaseStorageTest.php b/core/tests/Drupal/Tests/Core/Entity/ContentEntityDatabaseStorageTest.php index e803a3d19ee0..41358a6c4d3f 100644 --- a/core/tests/Drupal/Tests/Core/Entity/ContentEntityDatabaseStorageTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/ContentEntityDatabaseStorageTest.php @@ -15,7 +15,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; /** * Tests the fieldable database storage. * - * @see \Drupal\Core\Entity\ContentEntityDatabaseStorage + * @coversDefaultClass \Drupal\Core\Entity\ContentEntityDatabaseStorage * * @group Drupal * @group Entity @@ -36,7 +36,7 @@ class ContentEntityDatabaseStorageTest extends UnitTestCase { /** * Tests field SQL schema generation for an entity with a string identifier. * - * @see \Drupal\Core\Entity\Controller\ContentEntityDatabaseStorage::_fieldSqlSchema() + * @covers ::_fieldSqlSchema() */ public function testFieldSqlSchemaForEntityWithStringIdentifier() { $field_type_manager = $this->getMock('Drupal\Core\Field\FieldTypePluginManagerInterface'); @@ -112,4 +112,61 @@ class ContentEntityDatabaseStorageTest extends UnitTestCase { $this->assertEquals($schema['test_entity__test_field']['fields']['revision_id']['type'], 'varchar'); } + /** + * @covers ::create() + */ + public function testCreate() { + $language_manager = $this->getMock('Drupal\Core\Language\LanguageManagerInterface'); + $module_handler = $this->getMock('Drupal\Core\Extension\ModuleHandlerInterface'); + $entity_manager = $this->getMock('Drupal\Core\Entity\EntityManagerInterface'); + // @todo Add field definitions to test default values of fields. + $entity_manager->expects($this->atLeastOnce()) + ->method('getFieldDefinitions') + ->will($this->returnValue(array())); + + $container = new ContainerBuilder(); + $container->set('language_manager', $language_manager); + $container->set('entity.manager', $entity_manager); + $container->set('module_handler', $module_handler); + \Drupal::setContainer($container); + + $entity = $this->getMockForAbstractClass('Drupal\Core\Entity\ContentEntityBase', array(), '', FALSE, TRUE, TRUE, array('id')); + $entity_type = $this->getMock('Drupal\Core\Entity\EntityTypeInterface'); + $entity_type->expects($this->atLeastOnce()) + ->method('id') + ->will($this->returnValue('test_entity_type')); + $entity_type->expects($this->atLeastOnce()) + ->method('getClass') + ->will($this->returnValue(get_class($entity))); + $entity_type->expects($this->atLeastOnce()) + ->method('getKeys') + ->will($this->returnValue(array('id' => 'id'))); + $entity_type->expects($this->atLeastOnce()) + ->method('hasKey') + ->will($this->returnCallback(function ($key) { + return $key == 'id'; + })); + $entity_manager->expects($this->atLeastOnce()) + ->method('getDefinition') + ->with('test_entity_type') + ->will($this->returnValue($entity_type)); + + $connection = $this->getMockBuilder('Drupal\Core\Database\Connection') + ->disableOriginalConstructor() + ->getMock(); + $field_info = $this->getMockBuilder('\Drupal\field\FieldInfo') + ->disableOriginalConstructor() + ->getMock(); + $entity_storage = new ContentEntityDatabaseStorage($entity_type, $connection, $field_info); + + $entity = $entity_storage->create(); + $entity->expects($this->atLeastOnce()) + ->method('id') + ->will($this->returnValue('foo')); + + $this->assertInstanceOf('Drupal\Core\Entity\EntityInterface', $entity); + $this->assertSame('foo', $entity->id()); + $this->assertTrue($entity->isNew()); + } + }