Issue #2211949 by chx, Berdir, fago, benjy: Support keeping new revision id for migrate.

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

@@ -168,10 +168,27 @@ abstract class ContentEntityBase extends Entity implements \IteratorAggregate, C
     return \Drupal::typedDataManager();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function postCreate(EntityStorageInterface $storage_controller) {
+    $this->newRevision = TRUE;
+  }
+
   /**
    * {@inheritdoc}
    */
   public function setNewRevision($value = TRUE) {
+
+    if (!$this->getEntityType()->hasKey('revision')) {
+      throw new \LogicException(String::format('Entity type @entity_type does support revisions.'));
+    }
+
+    if ($value && !$this->newRevision) {
+      // When saving a new revision, set any existing revision ID to NULL so as
+      // to ensure that a new revision will actually be created.
+      $this->set($this->getEntityType()->getKey('revision'), NULL);
+    }
     $this->newRevision = $value;
   }
 

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

@@ -568,7 +568,9 @@ class ContentEntityDatabaseStorage extends ContentEntityStorageBase {
         if ($this->revisionDataTable) {
           $this->savePropertyData($entity, 'revision_data_table');
         }
-        $entity->setNewRevision(FALSE);
+        if ($this->revisionTable) {
+          $entity->setNewRevision(FALSE);
+        }
         $this->invokeFieldMethod('update', $entity);
         $this->saveFieldItems($entity, TRUE);
         $this->resetCache(array($entity->id()));
@@ -595,8 +597,11 @@ class ContentEntityDatabaseStorage extends ContentEntityStorageBase {
           $this->savePropertyData($entity, 'revision_data_table');
         }
 
-
         $entity->enforceIsNew(FALSE);
+        if ($this->revisionTable) {
+          $entity->setNewRevision(FALSE);
+        }
+
         $this->invokeFieldMethod('insert', $entity);
         $this->saveFieldItems($entity, FALSE);
         // Reset general caches, but keep caches specific to certain entities.
@@ -738,12 +743,6 @@ class ContentEntityDatabaseStorage extends ContentEntityStorageBase {
   protected function saveRevision(EntityInterface $entity) {
     $record = $this->mapToStorageRecord($entity, 'revision_table');
 
-    // When saving a new revision, set any existing revision ID to NULL so as to
-    // ensure that a new revision will actually be created.
-    if ($entity->isNewRevision() && isset($record->{$this->revisionKey})) {
-      $record->{$this->revisionKey} = NULL;
-    }
-
     $entity->preSaveRevision($this, $record);
 
     if ($entity->isNewRevision()) {

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

@@ -28,6 +28,9 @@ interface RevisionableInterface {
    * @param bool $value
    *   (optional) Whether a new revision should be saved.
    *
+   * @throws \LogicException
+   *   Thrown if the entity does not support revisions.
+   *
    * @see \Drupal\Core\Entity\EntityInterface::isNewRevision()
    */
   public function setNewRevision($value = TRUE);

core/modules/field/lib/Drupal/field/Tests/FieldAttachOtherTest.php

@@ -38,6 +38,7 @@ class FieldAttachOtherTest extends FieldUnitTestBase {
 
   public function setUp() {
     parent::setUp();
+    $this->installSchema('entity_test', array('entity_test_rev', 'entity_test_rev_revision'));
     $this->createFieldWithInstance();
   }
 
@@ -197,8 +198,8 @@ class FieldAttachOtherTest extends FieldUnitTestBase {
     $this->assertFalse(\Drupal::cache('entity')->get($cid), 'Non-cached: no cache entry on insert and load');
 
     // Cacheable entity type.
-    $entity_type = 'entity_test_cache';
-    $this->createFieldWithInstance('_2', 'entity_test_cache');
+    $entity_type = 'entity_test_rev';
+    $this->createFieldWithInstance('_2', $entity_type);
     entity_info_cache_clear();
 
     $entity_init = entity_create($entity_type, array(

core/modules/migrate/lib/Drupal/migrate/Plugin/migrate/destination/EntityContentBase.php

@@ -109,7 +109,6 @@ class EntityContentBase extends Entity {
    *   An array containing the entity id.
    */
   protected function save(ContentEntityInterface $entity, array $old_destination_id_values = array()) {
-    $entity->keepNewRevisionId(TRUE);
     $entity->save();
     return array($entity->id());
   }

core/modules/migrate/lib/Drupal/migrate/Plugin/migrate/destination/EntityRevision.php

@@ -46,7 +46,6 @@ class EntityRevision extends EntityContentBase {
       $entity = $this->storage->load($entity_id);
       $entity->enforceIsNew(FALSE);
       $entity->setNewRevision(TRUE);
-      $entity->keepNewRevisionId(TRUE);
     }
     $this->updateEntity($entity, $row);
     $entity->isDefaultRevision(FALSE);

core/modules/node/lib/Drupal/node/NodeFormController.php

@@ -50,8 +50,6 @@ class NodeFormController extends ContentEntityFormController {
       // Remove the log message from the original node entity.
       $node->log = NULL;
     }
-    // Always use the default revision setting.
-    $node->setNewRevision(!empty($this->settings['options']['revision']));
   }
 
   /**
@@ -123,7 +121,7 @@ class NodeFormController extends ContentEntityFormController {
     $form['revision'] = array(
       '#type' => 'checkbox',
       '#title' => t('Create new revision'),
-      '#default_value' => $node->isNewRevision(),
+      '#default_value' => !empty($this->settings['options']['revision']),
       '#access' => $node->isNewRevision() || user_access('administer nodes'),
       '#group' => 'revision_information',
     );

core/modules/system/lib/Drupal/system/Tests/Entity/EntityQueryTest.php

@@ -447,7 +447,6 @@ class EntityQueryTest extends EntityUnitTestBase {
       'type' => $bundle,
     ));
     $entity->enforceIsNew();
-    $entity->setNewRevision();
     $entity->save();
 
     // As the single entity of this type we just saved does not have a value

core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php

@@ -8,6 +8,7 @@
 namespace Drupal\Tests\Core\Entity;
 
 use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\Field\FieldDefinition;
 use Drupal\Tests\UnitTestCase;
 use Drupal\Core\Language\Language;
 
@@ -60,6 +61,13 @@ class ContentEntityBaseUnitTest extends UnitTestCase {
    */
   protected $typedDataManager;
 
+  /**
+   * The field type manager used for testing.
+   *
+   * @var \Drupal\Core\Field\FieldTypePluginManager|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $fieldTypePluginManager;
+
   /**
    * The language manager.
    *
@@ -112,14 +120,6 @@ class ContentEntityBaseUnitTest extends UnitTestCase {
       ->method('getDefinition')
       ->with($this->entityTypeId)
       ->will($this->returnValue($this->entityType));
-    $this->entityManager->expects($this->any())
-      ->method('getFieldDefinitions')
-      ->with($this->entityTypeId, $this->bundle)
-      ->will($this->returnValue(array(
-        'id' => array(
-          'type' => 'integer_field',
-        ),
-      )));
 
     $this->uuid = $this->getMock('\Drupal\Component\Uuid\UuidInterface');
 
@@ -137,13 +137,36 @@ class ContentEntityBaseUnitTest extends UnitTestCase {
       ->with('en')
       ->will($this->returnValue($language));
 
+    $this->fieldTypePluginManager = $this->getMockBuilder('\Drupal\Core\Field\FieldTypePluginManager')
+      ->disableOriginalConstructor()
+      ->getMock();
+    $this->fieldTypePluginManager->expects($this->any())
+      ->method('getDefaultSettings')
+      ->will($this->returnValue(array()));
+    $this->fieldTypePluginManager->expects($this->any())
+      ->method('getDefaultInstanceSettings')
+      ->will($this->returnValue(array()));
+
     $container = new ContainerBuilder();
     $container->set('entity.manager', $this->entityManager);
     $container->set('uuid', $this->uuid);
     $container->set('typed_data_manager', $this->typedDataManager);
     $container->set('language_manager', $this->languageManager);
+    $container->set('plugin.manager.field.field_type', $this->fieldTypePluginManager);
     \Drupal::setContainer($container);
 
+    $field_definitions = array(
+      'id' => FieldDefinition::create('integer'),
+      'revision_id' => FieldDefinition::create('integer'),
+    );
+
+    $this->entityManager->expects($this->any())
+      ->method('getFieldDefinitions')
+      ->with($this->entityTypeId, $this->bundle)
+      ->will($this->returnValue(array(
+        'id' => FieldDefinition::create('integer'),
+        'revision_id' => FieldDefinition::create('integer'),
+      )));
     $this->entity = $this->getMockForAbstractClass('\Drupal\Core\Entity\ContentEntityBase', array($values, $this->entityTypeId, $this->bundle));
   }
 
@@ -162,6 +185,22 @@ class ContentEntityBaseUnitTest extends UnitTestCase {
       ->method('hasKey')
       ->with('revision')
       ->will($this->returnValue(TRUE));
+    $this->entityType->expects($this->at(2))
+      ->method('hasKey')
+      ->with('revision')
+      ->will($this->returnValue(TRUE));
+    $this->entityType->expects($this->at(3))
+      ->method('getKey')
+      ->with('revision')
+      ->will($this->returnValue('revision_id'));
+
+    $field_item_list = $this->getMockBuilder('\Drupal\Core\Field\FieldItemList')
+      ->disableOriginalConstructor()
+      ->getMock();
+    $this->typedDataManager->expects($this->any())
+      ->method('getPropertyInstance')
+      ->with($this->entity, 'revision_id', NULL)
+      ->will($this->returnValue($field_item_list));
 
     $this->assertFalse($this->entity->isNewRevision());
     $this->assertTrue($this->entity->isNewRevision());