From 51a76efb662e24cb21cba116e908c259f7e37561 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Wed, 10 Aug 2016 21:42:48 +0100
Subject: [PATCH] Issue #2579343 by Lendude, drclaw, heddn, mikeryan: Migrate
 rollback does not rollback failed items

---
 .../modules/migrate/src/MigrateExecutable.php |  6 ++++
 .../src/Plugin/MigrateIdMapInterface.php      |  8 +++++
 .../migrate/src/Plugin/migrate/id_map/Sql.php | 20 ++++++++++++-
 .../tests/src/Kernel/MigrateRollbackTest.php  |  3 ++
 .../tests/src/Unit/MigrateSqlIdMapTest.php    | 29 +++++++++++++++++++
 5 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/core/modules/migrate/src/MigrateExecutable.php b/core/modules/migrate/src/MigrateExecutable.php
index bd0468be0bae..7368acd96441 100644
--- a/core/modules/migrate/src/MigrateExecutable.php
+++ b/core/modules/migrate/src/MigrateExecutable.php
@@ -329,6 +329,12 @@ class MigrateExecutable implements MigrateExecutableInterface {
         // We're now done with this row, so remove it from the map.
         $id_map->deleteDestination($destination_key);
       }
+      else {
+        // If there is no destination key the import probably failed and we can
+        // remove the row without further action.
+        $source_key = $id_map->currentSource();
+        $id_map->delete($source_key);
+      }
 
       // Check for memory exhaustion.
       if (($return = $this->checkStatus()) != MigrationInterface::RESULT_COMPLETED) {
diff --git a/core/modules/migrate/src/Plugin/MigrateIdMapInterface.php b/core/modules/migrate/src/Plugin/MigrateIdMapInterface.php
index d3bdb8923982..eafac2966241 100644
--- a/core/modules/migrate/src/Plugin/MigrateIdMapInterface.php
+++ b/core/modules/migrate/src/Plugin/MigrateIdMapInterface.php
@@ -243,6 +243,14 @@ interface MigrateIdMapInterface extends \Iterator, PluginInspectionInterface {
    */
   public function currentDestination();
 
+  /**
+   * Looks up the source identifier(s) currently being iterated.
+   *
+   * @return array
+   *   The source identifier values of the record, or NULL on failure.
+   */
+  public function currentSource();
+
   /**
    * Removes any persistent storage used by this map.
    *
diff --git a/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php b/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
index cf39275f8004..cbf4a9f46e43 100644
--- a/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
+++ b/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
@@ -845,7 +845,25 @@ class Sql extends PluginBase implements MigrateIdMapInterface, ContainerFactoryP
     if ($this->valid()) {
       $result = array();
       foreach ($this->destinationIdFields() as $destination_field_name => $idmap_field_name) {
-        $result[$destination_field_name] = $this->currentRow[$idmap_field_name];
+        if (!is_null($this->currentRow[$idmap_field_name])) {
+          $result[$destination_field_name] = $this->currentRow[$idmap_field_name];
+        }
+      }
+      return $result;
+    }
+    else {
+      return NULL;
+    }
+  }
+
+  /**
+   * @inheritdoc
+   */
+  public function currentSource() {
+    if ($this->valid()) {
+      $result = array();
+      foreach ($this->sourceIdFields() as $field_name => $source_id) {
+        $result[$field_name] = $this->currentKey[$source_id];
       }
       return $result;
     }
diff --git a/core/modules/migrate/tests/src/Kernel/MigrateRollbackTest.php b/core/modules/migrate/tests/src/Kernel/MigrateRollbackTest.php
index 9ec0c65e0d0b..4b78e6526443 100644
--- a/core/modules/migrate/tests/src/Kernel/MigrateRollbackTest.php
+++ b/core/modules/migrate/tests/src/Kernel/MigrateRollbackTest.php
@@ -128,6 +128,9 @@ class MigrateRollbackTest extends MigrateTestBase {
       $this->assertNotNull($map_row['destid1']);
     }
 
+    // Add a failed row to test if this can be rolled back without errors.
+    $this->mockFailure($term_migration, ['id' => '4', 'vocab' => '2', 'name' => 'FAIL']);
+
     // Rollback and verify the entities are gone.
     $term_executable->rollback();
     foreach ($term_data_rows as $row) {
diff --git a/core/modules/migrate/tests/src/Unit/MigrateSqlIdMapTest.php b/core/modules/migrate/tests/src/Unit/MigrateSqlIdMapTest.php
index a9d1313f5558..e38e971be3b6 100644
--- a/core/modules/migrate/tests/src/Unit/MigrateSqlIdMapTest.php
+++ b/core/modules/migrate/tests/src/Unit/MigrateSqlIdMapTest.php
@@ -640,6 +640,35 @@ class MigrateSqlIdMapTest extends MigrateTestCase {
     $this->assertSame(0, count($source_id));
   }
 
+  /**
+   * Tests currentDestination() and currentSource().
+   */
+  public function testCurrentDestinationAndSource() {
+    // Simple map with one source and one destination ID.
+    $id_map = $this->setupRows(['nid'], ['nid'], [
+      [1, 101],
+      [2, 102],
+      [3, 103],
+      // Mock a failed row by setting the destination ID to NULL.
+      [4, NULL],
+    ]);
+
+    // The rows are ordered by destination ID so the failed row should be first.
+    $id_map->rewind();
+    $this->assertEquals([], $id_map->currentDestination());
+    $this->assertEquals(['nid' => 4], $id_map->currentSource());
+    $id_map->next();
+    $this->assertEquals(['nid' => 101], $id_map->currentDestination());
+    $this->assertEquals(['nid' => 1], $id_map->currentSource());
+    $id_map->next();
+    $this->assertEquals(['nid' => 102], $id_map->currentDestination());
+    $this->assertEquals(['nid' => 2], $id_map->currentSource());
+    $id_map->next();
+    $this->assertEquals(['nid' => 103], $id_map->currentDestination());
+    $this->assertEquals(['nid' => 3], $id_map->currentSource());
+    $id_map->next();
+  }
+
   /**
    * Tests the imported count method.
    *