Issue #2571499 by benjy, mikeryan, phenaproxima: idMap source and destination...

Issue #2571499 by benjy, mikeryan, phenaproxima: idMap source and destination id filtering requires keys

core/modules/migrate/src/MigrateExecutable.php

@@ -327,7 +327,7 @@ public function rollback() {
     $destination = $this->migration->getDestinationPlugin();
 
     // Loop through each row in the map, and try to roll it back.
-    foreach ($id_map as $serialized_key => $map_row) {
+    foreach ($id_map as $map_row) {
       $destination_key = $id_map->currentDestination();
       if ($destination_key) {
         $this->getEventDispatcher()
@@ -336,7 +336,7 @@ public function rollback() {
         $this->getEventDispatcher()
           ->dispatch(MigrateEvents::POST_ROW_DELETE, new MigrateRowDeleteEvent($this->migration, $destination_key));
         // We're now done with this row, so remove it from the map.
-        $id_map->delete(unserialize($serialized_key));
+        $id_map->deleteDestination($destination_key);
       }
 
       // Check for memory exhaustion.

core/modules/migrate/src/Plugin/MigrateIdMapInterface.php

@@ -57,7 +57,7 @@ public function saveIdMapping(Row $row, array $destination_id_values, $status =
    * Saves a message related to a source record in the migration message table.
    *
    * @param array $source_id_values
-   *   The source identifier values of the record in error.
+   *   The source identifier keyed values of the record, e.g. ['nid' => 5].
    * @param string $message
    *   The message to record.
    * @param int $level
@@ -69,7 +69,7 @@ public function saveMessage(array $source_id_values, $message, $level = Migratio
    * Retrieves an iterator over messages relate to source records.
    *
    * @param array $source_id_values
-   *   (optional) The source identifier values of a specific record to retrieve.
+   *   (optional) The source identifier keyed values of the record, e.g. ['nid' => 5].
    *   If empty, all messages are retrieved.
    * @param int $level
    *   (optional) Message severity. If NULL, retrieve messages of all severities.
@@ -134,7 +134,7 @@ public function messageCount();
    * Deletes the map and message entries for a given source record.
    *
    * @param array $source_id_values
-   *   The source identifier values of the record to delete.
+   *   The source identifier keyed values of the record, e.g. ['nid' => 5].
    * @param bool $messages_only
    *   TRUE to only delete the migrate messages.
    */
@@ -144,19 +144,10 @@ public function delete(array $source_id_values, $messages_only = FALSE);
    * Deletes the map and message table entries for a given destination row.
    *
    * @param array $destination_id_values
-   *   The destination identifier values we should do the deletes for.
+   *   The destination identifier key value pairs we should do the deletes for.
    */
   public function deleteDestination(array $destination_id_values);
 
-  /**
-   * Deletes the map and message entries for a set of given source records.
-   *
-   * @param array $source_id_values
-   *   The identifier values of the sources we should do the deletes for. Each
-   *   array member is an array of identifier values for one source row.
-   */
-  public function deleteBulk(array $source_id_values);
-
   /**
    * Clears all messages from the map.
    */
@@ -166,7 +157,7 @@ public function clearMessages();
    * Retrieves a row from the map table based on source identifier values.
    *
    * @param array $source_id_values
-   *   The source identifier values of the record to retrieve.
+   *   The source identifier keyed values of the record, e.g. ['nid' => 5].
    *
    * @return array
    *   The raw row data as an associative array.
@@ -177,7 +168,7 @@ public function getRowBySource(array $source_id_values);
    * Retrieves a row by the destination identifiers.
    *
    * @param array $destination_id_values
-   *   The destination identifier values of the record to retrieve.
+   *   The destination identifier keyed values of the record, e.g. ['nid' => 5].
    *
    * @return array
    *   The row(s) of data.
@@ -202,10 +193,11 @@ public function getRowsNeedingUpdate($count);
    * (possibly multi-field) source identifier value mapped to it.
    *
    * @param array $destination_id_values
-   *   The destination identifier values of the record.
+   *   The destination identifier keyed values of the record, e.g. ['nid' => 5].
    *
    * @return array
-   *   The source identifier values of the record, or NULL on failure.
+   *   The source identifier keyed values of the record, e.g. ['nid' => 5], or
+   *   an empty array on failure.
    */
   public function lookupSourceID(array $destination_id_values);
 
@@ -216,7 +208,7 @@ public function lookupSourceID(array $destination_id_values);
    * (possibly multi-field) destination identifier value it is mapped to.
    *
    * @param array $source_id_values
-   *   The source identifier values of the record.
+   *   The source identifier keyed values of the record, e.g. ['nid' => 5].
    *
    * @return array
    *   The destination identifier values of the record, or NULL on failure.

core/modules/migrate/src/Plugin/migrate/id_map/Sql.php

@@ -8,8 +8,6 @@
 namespace Drupal\migrate\Plugin\migrate\id_map;
 
 use Drupal\Component\Utility\Unicode;
-use Drupal\Core\Database\Connection;
-use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Field\BaseFieldDefinition;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\Plugin\PluginBase;
@@ -428,8 +426,8 @@ protected function getFieldSchema(array $id_definition) {
   public function getRowBySource(array $source_id_values) {
     $query = $this->getDatabase()->select($this->mapTableName(), 'map')
               ->fields('map');
-    foreach ($this->sourceIdFields() as $source_id) {
-      $query = $query->condition("map.$source_id", array_shift($source_id_values), '=');
+    foreach ($this->sourceIdFields() as $field_name => $source_id) {
+      $query->condition("map.$source_id", $source_id_values[$field_name], '=');
     }
     $result = $query->execute();
     return $result->fetchAssoc();
@@ -441,8 +439,8 @@ public function getRowBySource(array $source_id_values) {
   public function getRowByDestination(array $destination_id_values) {
     $query = $this->getDatabase()->select($this->mapTableName(), 'map')
               ->fields('map');
-    foreach ($this->destinationIdFields() as $destination_id) {
-      $query = $query->condition("map.$destination_id", array_shift($destination_id_values), '=');
+    foreach ($this->destinationIdFields() as $field_name => $destination_id) {
+      $query->condition("map.$destination_id", $destination_id_values[$field_name], '=');
     }
     $result = $query->execute();
     return $result->fetchAssoc();
@@ -467,28 +465,36 @@ public function getRowsNeedingUpdate($count) {
   /**
    * {@inheritdoc}
    */
-  public function lookupSourceID(array $destination_id) {
-    $query = $this->getDatabase()->select($this->mapTableName(), 'map')
-              ->fields('map', $this->sourceIdFields());
-    foreach ($this->destinationIdFields() as $key_name) {
-      $query = $query->condition("map.$key_name", array_shift($destination_id), '=');
+  public function lookupSourceID(array $destination_id_values) {
+    $source_id_fields = $this->sourceIdFields();
+    $query = $this->getDatabase()->select($this->mapTableName(), 'map');
+    foreach ($source_id_fields as $source_field_name => $idmap_field_name) {
+      $query->addField('map', $idmap_field_name, $source_field_name);
+    }
+    foreach ($this->destinationIdFields() as $field_name => $destination_id) {
+      $query->condition("map.$destination_id", $destination_id_values[$field_name], '=');
     }
     $result = $query->execute();
-    $source_id = $result->fetchAssoc();
-    return array_values($source_id ?: array());
+    return $result->fetchAssoc() ?: [];
   }
 
   /**
    * {@inheritdoc}
    */
-  public function lookupDestinationId(array $source_id) {
-    if (empty($source_id)) {
+  public function lookupDestinationId(array $source_id_values) {
+    if (empty($source_id_values)) {
       return array();
     }
     $query = $this->getDatabase()->select($this->mapTableName(), 'map')
               ->fields('map', $this->destinationIdFields());
-    foreach ($this->sourceIdFields() as $key_name) {
-      $query = $query->condition("map.$key_name", array_shift($source_id), '=');
+
+    // When looking up the destination ID we require an array with both the
+    // source key and value, e.g. ['nid' => 41]. However, the Migration process
+    // plugin doesn't currently have a way to get the source key so we presume
+    // the values have been passed through in the correct order.
+    $have_keys = !isset($source_id_values[0]);
+    foreach ($this->sourceIdFields() as $field_name => $source_id) {
+      $query->condition("map.$source_id", $have_keys ? $source_id_values[$field_name] : array_shift($source_id_values), '=');
     }
     $result = $query->execute();
     $destination_id = $result->fetchAssoc();
@@ -544,13 +550,12 @@ public function saveIdMapping(Row $row, array $destination_id_values, $source_ro
    * {@inheritdoc}
    */
   public function saveMessage(array $source_id_values, $message, $level = MigrationInterface::MESSAGE_ERROR) {
-    $count = 1;
-    foreach ($source_id_values as $id_value) {
-      $fields['sourceid' . $count++] = $id_value;
+    foreach ($this->sourceIdFields() as $field_name => $source_id) {
       // If any key value is not set, we can't save.
-      if (!isset($id_value)) {
+      if (!isset($source_id_values[$field_name])) {
         return;
       }
+      $fields[$source_id] = $source_id_values[$field_name];
     }
     $fields['level'] = $level;
     $fields['message'] = $message;
@@ -565,10 +570,12 @@ public function saveMessage(array $source_id_values, $message, $level = Migratio
   public function getMessageIterator(array $source_id_values = [], $level = NULL) {
     $query = $this->getDatabase()->select($this->messageTableName(), 'msg')
       ->fields('msg');
-    $count = 1;
-    foreach ($source_id_values as $id_value) {
-      $query->condition('sourceid' . $count++, $id_value);
+    foreach ($this->sourceIdFields() as $field_name => $source_id) {
+      if (isset($source_id_values[$field_name])) {
+        $query->condition($source_id, $source_id_values[$field_name]);
+      }
     }
+
     if ($level) {
       $query->condition('level', $level);
     }
@@ -655,13 +662,11 @@ public function delete(array $source_id_values, $messages_only = FALSE) {
       $map_query = $this->getDatabase()->delete($this->mapTableName());
     }
     $message_query = $this->getDatabase()->delete($this->messageTableName());
-    $count = 1;
-    foreach ($source_id_values as $id_value) {
+    foreach ($this->sourceIdFields() as $field_name => $source_id) {
       if (!$messages_only) {
-        $map_query->condition('sourceid' . $count, $id_value);
+        $map_query->condition($source_id, $source_id_values[$field_name]);
       }
-      $message_query->condition('sourceid' . $count, $id_value);
-      $count++;
+      $message_query->condition($source_id, $source_id_values[$field_name]);
     }
 
     if (!$messages_only) {
@@ -675,22 +680,20 @@ public function delete(array $source_id_values, $messages_only = FALSE) {
   /**
    * {@inheritdoc}
    */
-  public function deleteDestination(array $destination_id) {
+  public function deleteDestination(array $destination_id_values) {
     $map_query = $this->getDatabase()->delete($this->mapTableName());
     $message_query = $this->getDatabase()->delete($this->messageTableName());
-    $source_id = $this->lookupSourceID($destination_id);
-    if (!empty($source_id)) {
-      $count = 1;
-      foreach ($destination_id as $key_value) {
-        $map_query->condition('destid' . $count, $key_value);
-        $count++;
+    $source_id_values = $this->lookupSourceID($destination_id_values);
+    if (!empty($source_id_values)) {
+      foreach ($this->destinationIdFields() as $field_name => $destination_id) {
+        $map_query->condition($destination_id, $destination_id_values[$field_name]);
       }
       // Notify anyone listening of the map row we're about to delete.
-      $this->eventDispatcher->dispatch(MigrateEvents::MAP_DELETE, new MigrateMapDeleteEvent($this, $source_id));
+      $this->eventDispatcher->dispatch(MigrateEvents::MAP_DELETE, new MigrateMapDeleteEvent($this, $source_id_values));
       $map_query->execute();
       $count = 1;
-      foreach ($source_id as $key_value) {
-        $message_query->condition('sourceid' . $count, $key_value);
+      foreach ($this->sourceIdFields() as $field_name => $source_id) {
+        $message_query->condition($source_id, $source_id_values[$field_name]);
         $count++;
       }
       $message_query->execute();
@@ -700,54 +703,18 @@ public function deleteDestination(array $destination_id) {
   /**
    * {@inheritdoc}
    */
-  public function setUpdate(array $source_id) {
-    if (empty($source_id)) {
+  public function setUpdate(array $source_id_values) {
+    if (empty($source_id_values)) {
       throw new MigrateException('No source identifiers provided to update.');
     }
     $query = $this->getDatabase()
       ->update($this->mapTableName())
       ->fields(array('source_row_status' => MigrateIdMapInterface::STATUS_NEEDS_UPDATE));
-    $count = 1;
-    foreach ($source_id as $key_value) {
-      $query->condition('sourceid' . $count++, $key_value);
-    }
-    $query->execute();
-  }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function deleteBulk(array $source_id_values) {
-    // If we have a single-column key, we can shortcut it.
-    if (count($this->migration->getSourcePlugin()->getIds()) == 1) {
-      $sourceids = array();
-      foreach ($source_id_values as $source_id) {
-        // Notify anyone listening of the map rows we're about to delete.
-        $this->eventDispatcher->dispatch(MigrateEvents::MAP_DELETE, new MigrateMapDeleteEvent($this, $source_id));
-        $sourceids[] = $source_id;
-      }
-      $this->getDatabase()->delete($this->mapTableName())
-        ->condition('sourceid1', $sourceids, 'IN')
-        ->execute();
-      $this->getDatabase()->delete($this->messageTableName())
-        ->condition('sourceid1', $sourceids, 'IN')
-        ->execute();
-    }
-    else {
-      foreach ($source_id_values as $source_id) {
-        // Notify anyone listening of the map rows we're deleting.
-        $this->eventDispatcher->dispatch(MigrateEvents::MAP_DELETE, new MigrateMapDeleteEvent($this, $source_id));
-        $map_query = $this->getDatabase()->delete($this->mapTableName());
-        $message_query = $this->getDatabase()->delete($this->messageTableName());
-        $count = 1;
-        foreach ($source_id as $key_value) {
-          $map_query->condition('sourceid' . $count, $key_value);
-          $message_query->condition('sourceid' . $count++, $key_value);
-        }
-        $map_query->execute();
-        $message_query->execute();
-      }
+    foreach ($this->sourceIdFields() as $field_name => $source_id) {
+      $query->condition($source_id, $source_id_values[$field_name]);
     }
+    $query->execute();
   }
 
   /**
@@ -811,8 +778,8 @@ public function key() {
   public function currentDestination() {
     if ($this->valid()) {
       $result = array();
-      foreach ($this->destinationIdFields() as $field_name) {
-        $result[$field_name] = $this->currentRow[$field_name];
+      foreach ($this->destinationIdFields() as $destination_field_name => $idmap_field_name) {
+        $result[$destination_field_name] = $this->currentRow[$idmap_field_name];
       }
       return $result;
     }

core/modules/migrate/src/Plugin/migrate/process/Migration.php

@@ -114,6 +114,7 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
       $destination_plugin = $migration->getDestinationPlugin(TRUE);
       // Only keep the process necessary to produce the destination ID.
       $process = $migration->get('process');
+
       // We already have the source id values but need to key them for the Row
       // constructor.
       $source_ids = $migration->getSourcePlugin()->getIds();

core/modules/migrate/src/Tests/MigrateRollbackTest.php

@@ -75,7 +75,7 @@ public function testRollback() {
       /** @var Vocabulary $vocabulary */
       $vocabulary = Vocabulary::load($row['id']);
       $this->assertTrue($vocabulary);
-      $map_row = $vocabulary_id_map->getRowBySource([$row['id']]);
+      $map_row = $vocabulary_id_map->getRowBySource(['id' => $row['id']]);
       $this->assertNotNull($map_row['destid1']);
     }
 
@@ -116,7 +116,7 @@ public function testRollback() {
       /** @var Term $term */
       $term = Term::load($row['id']);
       $this->assertTrue($term);
-      $map_row = $term_id_map->getRowBySource([$row['id']]);
+      $map_row = $term_id_map->getRowBySource(['id' => $row['id']]);
       $this->assertNotNull($map_row['destid1']);
     }
 
@@ -125,14 +125,14 @@ public function testRollback() {
     foreach ($term_data_rows as $row) {
       $term = Term::load($row['id']);
       $this->assertNull($term);
-      $map_row = $term_id_map->getRowBySource([$row['id']]);
+      $map_row = $term_id_map->getRowBySource(['id' => $row['id']]);
       $this->assertFalse($map_row);
     }
     $vocabulary_executable->rollback();
     foreach ($vocabulary_data_rows as $row) {
       $term = Vocabulary::load($row['id']);
       $this->assertNull($term);
-      $map_row = $vocabulary_id_map->getRowBySource([$row['id']]);
+      $map_row = $vocabulary_id_map->getRowBySource(['id' => $row['id']]);
       $this->assertFalse($map_row);
     }
 

core/modules/migrate/src/Tests/MigrateSkipRowTest.php

@@ -63,10 +63,10 @@ public function testPrepareRowSkip() {
 
     $id_map_plugin = $migration->getIdMap();
     // The first row is recorded in the map as ignored.
-    $map_row = $id_map_plugin->getRowBySource([1]);
+    $map_row = $id_map_plugin->getRowBySource(['id' => 1]);
     $this->assertEqual(MigrateIdMapInterface::STATUS_IGNORED, $map_row['source_row_status']);
     // The second row is not recorded in the map.
-    $map_row = $id_map_plugin->getRowBySource([2]);
+    $map_row = $id_map_plugin->getRowBySource(['id' => 2]);
     $this->assertFalse($map_row);
 
   }

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

@@ -191,7 +191,7 @@ public function testClearMessages() {
 
     // Insert 4 message for later delete.
     foreach ($expected_results as $key => $expected_result) {
-      $id_map->saveMessage([$key], $message);
+      $id_map->saveMessage(['source_id_property' => $key], $message);
     }
 
     // Truncate and check that 4 messages were deleted.
@@ -240,7 +240,7 @@ public function testGetRowsNeedingUpdate() {
 
     // Assert the row matches its original source.
     $source_id = $expected_results[MigrateIdMapInterface::STATUS_NEEDS_UPDATE]['sourceid1'];
-    $test_row = $id_map->getRowBySource([$source_id]);
+    $test_row = $id_map->getRowBySource(['source_id_property' => $source_id]);
     // $row_needing_update is an array of objects returned from the database,
     // but $test_row is an array, so the cast is necessary.
     $this->assertSame($test_row, (array) $row_needing_update[0]);
@@ -268,7 +268,7 @@ public function testMessageCount() {
     foreach ($expected_results as $key => $expected_result) {
       $count = $id_map->messageCount();
       $this->assertEquals($expected_result, $count);
-      $id_map->saveMessage([$key], $message);
+      $id_map->saveMessage(['source_id_property' => $key], $message);
     }
   }
 
@@ -286,7 +286,7 @@ public function testMessageSave() {
     $id_map = $this->getIdMap();
 
     foreach ($expected_results as $key => $expected_result) {
-      $id_map->saveMessage([$key], $message, $expected_result['level']);
+      $id_map->saveMessage(['source_id_property' => $key], $message, $expected_result['level']);
     }
 
     foreach ($id_map->getMessageIterator() as $message_row) {
@@ -297,8 +297,8 @@ public function testMessageSave() {
 
     // Insert with default level.
     $message_default = 'Hello world default.';
-    $id_map->saveMessage([5], $message_default);
-    $messages = $id_map->getMessageIterator([5]);
+    $id_map->saveMessage(['source_id_property' => 5], $message_default);
+    $messages = $id_map->getMessageIterator(['source_id_property' => 5]);
     $count = 0;
     foreach ($messages as $key => $message_row) {
       $count = 1;
@@ -334,11 +334,11 @@ public function testGetRowBySource() {
       'destid1' => 'destination_id_value_2',
     ] + $this->idMapDefaults();
     $this->saveMap($row);
-    $source_id_values = [$row['sourceid1'], $row['sourceid2']];
+    $source_id_values = ['source_id_property' => $row['sourceid1'], 'sourceid2' => $row['sourceid2']];
     $id_map = $this->getIdMap();
     $result_row = $id_map->getRowBySource($source_id_values);
     $this->assertSame($row, $result_row);
-    $source_id_values = ['missing_value_1', 'missing_value_2'];
+    $source_id_values = ['source_id_property' => 'missing_value_1', 'sourceid2' => 'missing_value_2'];
     $result_row = $id_map->getRowBySource($source_id_values);
     $this->assertFalse($result_row);
   }
@@ -416,12 +416,12 @@ public function testGetRowByDestination() {
       'destid1' => 'destination_id_value_2',
     ] + $this->idMapDefaults();
     $this->saveMap($row);
-    $dest_id_values = [$row['destid1']];
+    $dest_id_values = ['destination_id_property' => $row['destid1']];
     $id_map = $this->getIdMap();
     $result_row = $id_map->getRowByDestination($dest_id_values);
     $this->assertSame($row, $result_row);
     // This value does not exist.
-    $dest_id_values = ['invalid_destination_id_property'];
+    $dest_id_values = ['destination_id_property' => 'invalid_destination_id_property'];
     $id_map = $this->getIdMap();
     $result_row = $id_map->getRowByDestination($dest_id_values);
     $this->assertFalse($result_row);
@@ -463,15 +463,15 @@ public function testLookupSourceIDMapping($num_source_fields, $num_destination_f
     $expected_result = [];
     for ($i = 1; $i <= $num_source_fields; $i++) {
       $row["sourceid$i"] = "source_id_value_$i";
-      $expected_result[] = "source_id_value_$i";
+      $expected_result["source_id_property_$i"] = "source_id_value_$i";
       $this->sourceIds["source_id_property_$i"] = [];
     }
     $destination_id_values = [];
     $nonexistent_id_values = [];
     for ($i = 1; $i <= $num_destination_fields; $i++) {
       $row["destid$i"] = "destination_id_value_$i";
-      $destination_id_values[] = "destination_id_value_$i";
-      $nonexistent_id_values[] = "nonexistent_destination_id_value_$i";
+      $destination_id_values["destination_id_property_$i"] = "destination_id_value_$i";
+      $nonexistent_id_values["destination_id_property_$i"] = "nonexistent_destination_id_value_$i";
       $this->destinationIds["destination_id_property_$i"] = [];
     }
     $this->saveMap($row);
@@ -662,7 +662,7 @@ public function testSetUpdate() {
     $this->queryResultTest($this->getIdMapContents(), $expected_results);
     // Mark each row as STATUS_NEEDS_UPDATE.
     foreach ($row_statuses as $status) {
-      $id_map->setUpdate(['source_value_' . $status]);
+      $id_map->setUpdate(['source_id_property' => 'source_value_' . $status]);
     }
     // Update expected results.
     foreach ($expected_results as $key => $value) {
@@ -737,7 +737,7 @@ public function testDestroy() {
     $message_table_name = $id_map->messageTableName();
     $row = new Row(['source_id_property' => 'source_value'], ['source_id_property' => []]);
     $id_map->saveIdMapping($row, ['destination_id_property' => 2]);
-    $id_map->saveMessage(['source_value'], 'A message');
+    $id_map->saveMessage(['source_id_property' => 'source_value'], 'A message');
     $this->assertTrue($this->database->schema()->tableExists($map_table_name),
                       "$map_table_name exists");
     $this->assertTrue($this->database->schema()->tableExists($message_table_name),