Issue #2711353 by rakesh.gectcr, Jo Fitzgerald, quietone, chx, Yogesh Pawar,...

Issue #2711353 by rakesh.gectcr, Jo Fitzgerald, quietone, chx, Yogesh Pawar, Pavan B S, biguzis, phenaproxima, mikeryan, vasi, iMiksu, Berdir, dawehner, benjy: Migrate never unsets existing data for content entitites

core/modules/migrate/src/MigrateExecutable.php

@@ -386,9 +386,14 @@ public function processRow(Row $row, array $process = NULL, $value = NULL) {
           $multiple = $plugin->multiple();
         }
       }
-      // No plugins or no value means do not set.
-      if ($plugins && !is_null($value)) {
-        $row->setDestinationProperty($destination, $value);
+      // Ensure all values, including nulls, are migrated.
+      if ($plugins) {
+        if (isset($value)) {
+          $row->setDestinationProperty($destination, $value);
+        }
+        else {
+          $row->setEmptyDestinationProperty($destination);
+        }
       }
       // Reset the value.
       $value = NULL;

core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php

@@ -149,6 +149,7 @@ public function getIds() {
    *   An updated entity, or NULL if it's the same as the one passed in.
    */
   protected function updateEntity(EntityInterface $entity, Row $row) {
+    $empty_destinations = $row->getEmptyDestinationProperties();
     // By default, an update will be preserved.
     $rollback_action = MigrateIdMapInterface::ROLLBACK_PRESERVE;
 
@@ -171,6 +172,7 @@ protected function updateEntity(EntityInterface $entity, Row $row) {
     // clone the row with an empty set of destination values, and re-add only
     // the specified properties.
     if (isset($this->configuration['overwrite_properties'])) {
+      $empty_destinations = array_intersect($empty_destinations, $this->configuration['overwrite_properties']);
       $clone = $row->cloneWithoutDestination();
       foreach ($this->configuration['overwrite_properties'] as $property) {
         $clone->setDestinationProperty($property, $row->getDestinationProperty($property));
@@ -184,6 +186,9 @@ protected function updateEntity(EntityInterface $entity, Row $row) {
         $field->setValue($values);
       }
     }
+    foreach ($empty_destinations as $field_name) {
+      $entity->$field_name = NULL;
+    }
 
     $this->setRollbackAction($row->getIdMap(), $rollback_action);
 

core/modules/migrate/src/Row.php

@@ -77,6 +77,13 @@ class Row {
    */
   protected $isStub = FALSE;
 
+  /**
+   * The empty destination properties.
+   *
+   * @var array
+   */
+  protected $emptyDestinationProperties = [];
+
   /**
    * Constructs a \Drupal\Migrate\Row object.
    *
@@ -229,6 +236,26 @@ public function removeDestinationProperty($property) {
     NestedArray::unsetValue($this->destination, explode(static::PROPERTY_SEPARATOR, $property));
   }
 
+  /**
+   * Sets a destination to be empty.
+   *
+   * @param string $property
+   *   The destination property.
+   */
+  public function setEmptyDestinationProperty($property) {
+    $this->emptyDestinationProperties[] = $property;
+  }
+
+  /**
+   * Gets the empty destination properties.
+   *
+   * @return array
+   *   An array of destination properties.
+   */
+  public function getEmptyDestinationProperties() {
+    return $this->emptyDestinationProperties;
+  }
+
   /**
    * Returns the whole destination array.
    *

core/modules/migrate/tests/src/Kernel/MigrateEntityContentBaseTest.php

@@ -9,6 +9,7 @@
 use Drupal\migrate\Plugin\MigrateIdMapInterface;
 use Drupal\migrate\Plugin\MigrationInterface;
 use Drupal\migrate\Row;
+use Drupal\migrate_entity_test\Entity\StringIdEntityTest;
 
 /**
  * Tests the EntityContentBase destination.
@@ -203,4 +204,65 @@ public function testEntityWithStringId() {
     $this->assertEquals(123456789012, $map_row['destid1']);
   }
 
+  /**
+   * Tests empty destinations.
+   */
+  public function testEmptyDestinations() {
+    $this->enableModules(['migrate_entity_test']);
+    $this->installEntitySchema('migrate_string_id_entity_test');
+
+    $definition = [
+      'source' => [
+        'plugin' => 'embedded_data',
+        'data_rows' => [
+          ['id' => 123, 'version' => 'foo'],
+          // This integer needs an 'int' schema with 'big' size. If 'destid1'
+          // is not correctly taking the definition from the destination entity
+          // type, the import will fail with an SQL exception.
+          ['id' => 123456789012, 'version' => 'bar'],
+        ],
+        'ids' => [
+          'id' => ['type' => 'integer', 'size' => 'big'],
+          'version' => ['type' => 'string'],
+        ],
+        'constants' => ['null' => NULL],
+      ],
+      'process' => [
+        'id' => 'id',
+        'version' => 'version',
+      ],
+      'destination' => [
+        'plugin' => 'entity:migrate_string_id_entity_test',
+      ],
+    ];
+
+    $migration = \Drupal::service('plugin.manager.migration')
+      ->createStubMigration($definition);
+    $executable = new MigrateExecutable($migration);
+    $executable->import();
+
+    /** @var \Drupal\migrate_entity_test\Entity\StringIdEntityTest $entity */
+    $entity = StringIdEntityTest::load('123');
+    $this->assertSame('foo', $entity->version->value);
+    $entity = StringIdEntityTest::load('123456789012');
+    $this->assertSame('bar', $entity->version->value);
+
+    // Rerun the migration forcing the version to NULL.
+    $definition['process'] = [
+      'id' => 'id',
+      'version' => 'constants/null',
+    ];
+
+    $migration = \Drupal::service('plugin.manager.migration')
+      ->createStubMigration($definition);
+    $executable = new MigrateExecutable($migration);
+    $executable->import();
+
+    /** @var \Drupal\migrate_entity_test\Entity\StringIdEntityTest $entity */
+    $entity = StringIdEntityTest::load('123');
+    $this->assertNull($entity->version->value);
+    $entity = StringIdEntityTest::load('123456789012');
+    $this->assertNull($entity->version->value);
+  }
+
 }

core/modules/migrate/tests/src/Unit/MigrateExecutableTest.php

@@ -439,6 +439,33 @@ public function testProcessRowPipelineException() {
     $this->executable->processRow($row);
   }
 
+  /**
+   * Tests the processRow method.
+   */
+  public function testProcessRowEmptyDestination() {
+    $expected = [
+      'test' => 'test destination',
+      'test1' => 'test1 destination',
+      'test2' => NULL,
+    ];
+    $row = new Row();
+    $plugins = [];
+    foreach ($expected as $key => $value) {
+      $plugin = $this->prophesize(MigrateProcessInterface::class);
+      $plugin->getPluginDefinition()->willReturn([]);
+      $plugin->transform(NULL, $this->executable, $row, $key)->willReturn($value);
+      $plugin->multiple()->willReturn(TRUE);
+      $plugins[$key][0] = $plugin->reveal();
+    }
+    $this->migration->method('getProcessPlugins')->willReturn($plugins);
+    $this->executable->processRow($row);
+    foreach ($expected as $key => $value) {
+      $this->assertSame($value, $row->getDestinationProperty($key));
+    }
+    $this->assertCount(2, $row->getDestination());
+    $this->assertSame(['test2'], $row->getEmptyDestinationProperties());
+  }
+
   /**
    * Returns a mock migration source instance.
    *