From 68c0a254492c8b4ce8e98e09f08186ee7623e755 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Fri, 16 Aug 2019 08:49:13 +1000
Subject: [PATCH] Issue #3054167 by christinlepson, shubham.prakash, Charlie
 ChX Negyesi, init90, benjifisher, heddn, alexpott: The getMessageIterator()
 method does not return an instance of Iterator

---
 .../Kernel/Migrate/d6/MigrateFieldTest.php    |  7 ++---
 .../Migrate/d7/MigrateFieldInstanceTest.php   |  3 +--
 .../Kernel/Migrate/d7/MigrateFieldTest.php    |  3 +--
 .../Migrate/d6/MigrateImageCacheTest.php      | 13 +++-------
 .../Migrate/d6/MigrateDefaultLanguageTest.php |  4 +--
 .../Migrate/d7/MigrateDefaultLanguageTest.php |  4 +--
 .../d7/MigrateLanguageContentSettingsTest.php |  2 +-
 .../src/Plugin/MigrateIdMapInterface.php      | 26 +++++++++++++++++++
 .../src/Plugin/migrate/id_map/NullIdMap.php   | 10 ++++++-
 .../migrate/src/Plugin/migrate/id_map/Sql.php | 10 ++++++-
 .../process/DownloadFunctionalTest.php        |  2 +-
 .../tests/src/Kernel/MigrateMessageTest.php   |  6 ++---
 .../tests/src/Kernel/MigrateSkipRowTest.php   |  4 +--
 .../tests/src/Unit/MigrateNullIdMapTest.php   | 26 +++++++++++++++++++
 .../tests/src/Unit/MigrateSqlIdMapTest.php    | 17 +++++++++---
 15 files changed, 103 insertions(+), 34 deletions(-)
 create mode 100644 core/modules/migrate/tests/src/Unit/MigrateNullIdMapTest.php

diff --git a/core/modules/field/tests/src/Kernel/Migrate/d6/MigrateFieldTest.php b/core/modules/field/tests/src/Kernel/Migrate/d6/MigrateFieldTest.php
index 6173a3a73470..01d16807c63d 100644
--- a/core/modules/field/tests/src/Kernel/Migrate/d6/MigrateFieldTest.php
+++ b/core/modules/field/tests/src/Kernel/Migrate/d6/MigrateFieldTest.php
@@ -120,12 +120,9 @@ public function testFields() {
     $this->assertSame($migration->getSourcePlugin()->count(), $migration->getIdMap()->processedCount());
 
     // Check that we've reported on a conflict in widget_types.
-    $messages = [];
-    foreach ($migration->getIdMap()->getMessageIterator() as $message_row) {
-      $messages[] = $message_row->message;
-    }
+    $messages = iterator_to_array($migration->getIdMap()->getMessages());
     $this->assertCount(1, $messages);
-    $this->assertSame($messages[0], 'Widget types optionwidgets_onoff, text_textfield are used in Drupal 6 field instances: widget type optionwidgets_onoff applied to the Drupal 8 base field');
+    $this->assertSame($messages[0]->message, 'Widget types optionwidgets_onoff, text_textfield are used in Drupal 6 field instances: widget type optionwidgets_onoff applied to the Drupal 8 base field');
   }
 
 }
diff --git a/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldInstanceTest.php b/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldInstanceTest.php
index ed70c74415e3..3b32918e55c7 100644
--- a/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldInstanceTest.php
+++ b/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldInstanceTest.php
@@ -196,10 +196,9 @@ public function testTextFieldInstances() {
     // For each text field instances that were skipped, there should be a log
     // message with the required steps to fix this.
     $migration = $this->getMigration('d7_field_instance');
-    $messages = $migration->getIdMap()->getMessageIterator()->fetchAll();
     $errors = array_map(function ($message) {
       return $message->message;
-    }, $messages);
+    }, iterator_to_array($migration->getIdMap()->getMessages()));
     $this->assertCount(8, $errors);
     sort($errors);
     $message = 'Can\'t migrate source field field_text_long_plain_filtered configured with both plain text and filtered text processing. See https://www.drupal.org/docs/8/upgrade/known-issues-when-upgrading-from-drupal-6-or-7-to-drupal-8#plain-text';
diff --git a/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php b/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php
index 14c6c3176779..2fa3af38eff2 100644
--- a/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php
+++ b/core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php
@@ -173,10 +173,9 @@ public function testTextFields() {
     // For each text field bases that were skipped, there should be a log
     // message with the required steps to fix this.
     $migration = $this->getMigration('d7_field');
-    $messages = $migration->getIdMap()->getMessageIterator()->fetchAll();
     $errors = array_map(function ($message) {
       return $message->message;
-    }, $messages);
+    }, iterator_to_array($migration->getIdMap()->getMessages()));
     sort($errors);
     $this->assertCount(4, $errors);
     $this->assertEquals($errors[0], 'Can\'t migrate source field field_text_long_plain_filtered configured with both plain text and filtered text processing. See https://www.drupal.org/docs/8/upgrade/known-issues-when-upgrading-from-drupal-6-or-7-to-drupal-8#plain-text');
diff --git a/core/modules/image/tests/src/Kernel/Migrate/d6/MigrateImageCacheTest.php b/core/modules/image/tests/src/Kernel/Migrate/d6/MigrateImageCacheTest.php
index f609cd7da204..de7a276050f6 100644
--- a/core/modules/image/tests/src/Kernel/Migrate/d6/MigrateImageCacheTest.php
+++ b/core/modules/image/tests/src/Kernel/Migrate/d6/MigrateImageCacheTest.php
@@ -105,15 +105,10 @@ public function testMissingEffectPlugin() {
 
     $this->startCollectingMessages();
     $this->executeMigration('d6_imagecache_presets');
-    $messages = $this->migration->getIdMap()->getMessageIterator();
-    $count = 0;
-    foreach ($messages as $message) {
-      $count++;
-      $this->assertContains('The "image_deprecated_scale" plugin does not exist.', $message->message);
-      $this->assertEqual($message->level, MigrationInterface::MESSAGE_ERROR);
-    }
-    // There should be only the one message.
-    $this->assertEqual($count, 1);
+    $messages = iterator_to_array($this->migration->getIdMap()->getMessages());
+    $this->assertCount(1, $messages);
+    $this->assertContains('The "image_deprecated_scale" plugin does not exist.', $messages[0]->message);
+    $this->assertEqual($messages[0]->level, MigrationInterface::MESSAGE_ERROR);
   }
 
   /**
diff --git a/core/modules/language/tests/src/Kernel/Migrate/d6/MigrateDefaultLanguageTest.php b/core/modules/language/tests/src/Kernel/Migrate/d6/MigrateDefaultLanguageTest.php
index e76b4553ee03..16f723dfc5b1 100644
--- a/core/modules/language/tests/src/Kernel/Migrate/d6/MigrateDefaultLanguageTest.php
+++ b/core/modules/language/tests/src/Kernel/Migrate/d6/MigrateDefaultLanguageTest.php
@@ -41,7 +41,7 @@ public function testMigrationWithNonExistentLanguage() {
     $this->executeMigrations(['language', 'default_language']);
 
     // Tests the migration log contains an error message.
-    $messages = $this->migration->getIdMap()->getMessageIterator();
+    $messages = $this->migration->getIdMap()->getMessages();
     $count = 0;
     foreach ($messages as $message) {
       $count++;
@@ -62,7 +62,7 @@ public function testMigrationWithUnsetVariable() {
     $this->startCollectingMessages();
     $this->executeMigrations(['language', 'default_language']);
 
-    $messages = $this->migration->getIdMap()->getMessageIterator()->fetchAll();
+    $messages = $this->migration->getIdMap()->getMessages()->fetchAll();
     // Make sure there's no migration exceptions.
     $this->assertEmpty($messages);
     // Make sure the default langcode is 'en', as it was the default on D6 & D7.
diff --git a/core/modules/language/tests/src/Kernel/Migrate/d7/MigrateDefaultLanguageTest.php b/core/modules/language/tests/src/Kernel/Migrate/d7/MigrateDefaultLanguageTest.php
index 2a11b2562831..9a1998b7c06b 100644
--- a/core/modules/language/tests/src/Kernel/Migrate/d7/MigrateDefaultLanguageTest.php
+++ b/core/modules/language/tests/src/Kernel/Migrate/d7/MigrateDefaultLanguageTest.php
@@ -41,7 +41,7 @@ public function testMigrationWithNonExistentLanguage() {
     $this->executeMigrations(['language', 'default_language']);
 
     // Tests the migration log contains an error message.
-    $messages = $this->migration->getIdMap()->getMessageIterator();
+    $messages = $this->migration->getIdMap()->getMessages();
     $count = 0;
     foreach ($messages as $message) {
       $count++;
@@ -62,7 +62,7 @@ public function testMigrationWithUnsetVariable() {
     $this->startCollectingMessages();
     $this->executeMigrations(['language', 'default_language']);
 
-    $messages = $this->migration->getIdMap()->getMessageIterator()->fetchAll();
+    $messages = $this->migration->getIdMap()->getMessages()->fetchAll();
     // Make sure there's no migration exceptions.
     $this->assertEmpty($messages);
     // Make sure the default langcode is 'en', as it was the default on D6 & D7.
diff --git a/core/modules/language/tests/src/Kernel/Migrate/d7/MigrateLanguageContentSettingsTest.php b/core/modules/language/tests/src/Kernel/Migrate/d7/MigrateLanguageContentSettingsTest.php
index 7adc1f8b55a9..1e05e9193dcf 100644
--- a/core/modules/language/tests/src/Kernel/Migrate/d7/MigrateLanguageContentSettingsTest.php
+++ b/core/modules/language/tests/src/Kernel/Migrate/d7/MigrateLanguageContentSettingsTest.php
@@ -53,7 +53,7 @@ public function testLanguageContent() {
     $this->assertSame($config->getDefaultLangcode(), 'site_default');
 
     // Make sure there's no migration exceptions.
-    $messages = $this->migration->getIdMap()->getMessageIterator()->fetchAll();
+    $messages = $this->migration->getIdMap()->getMessages()->fetchAll();
     $this->assertEmpty($messages);
 
     // Assert that a content type translatable with entity_translation is still
diff --git a/core/modules/migrate/src/Plugin/MigrateIdMapInterface.php b/core/modules/migrate/src/Plugin/MigrateIdMapInterface.php
index 3482e4d3b8d4..0783f3acad61 100644
--- a/core/modules/migrate/src/Plugin/MigrateIdMapInterface.php
+++ b/core/modules/migrate/src/Plugin/MigrateIdMapInterface.php
@@ -62,6 +62,27 @@ public function saveIdMapping(Row $row, array $destination_id_values, $status =
    */
   public function saveMessage(array $source_id_values, $message, $level = MigrationInterface::MESSAGE_ERROR);
 
+  /**
+   * Retrieves a traversable object of messages related to source records.
+   *
+   * @param array $source_id_values
+   *   (optional) The source identifier keyed values of the record, e.g.
+   *   ['nid' => 5]. If empty (the default), all messages are retrieved.
+   * @param int $level
+   *   (optional) Message severity. If NULL (the default), retrieve messages of
+   *   all severities.
+   *
+   * @return \Traversable
+   *   Retrieves a traversable object of message objects of unspecified class.
+   *   Each object has the following public properties:
+   *   - source_row_hash: the hash of the entire serialized source row data.
+   *   - message: the text of the message.
+   *   - level: one of MigrationInterface::MESSAGE_ERROR,
+   *   MigrationInterface::MESSAGE_WARNING, MigrationInterface::MESSAGE_NOTICE,
+   *   MigrationInterface::MESSAGE_INFORMATIONAL.
+   */
+  public function getMessages(array $source_id_values = [], $level = NULL);
+
   /**
    * Retrieves an iterator over messages relate to source records.
    *
@@ -74,6 +95,11 @@ public function saveMessage(array $source_id_values, $message, $level = Migratio
    *
    * @return \Iterator
    *   Retrieves an iterator over the message rows.
+   *
+   * @deprecated in drupal:8.8.0 and is removed from drupal:9.0.0.
+   *   Use \Drupal\migrate\Plugin\MigrateIdMapInterface::getMessages() instead.
+   *
+   * @see https://www.drupal.org/node/3060969
    */
   public function getMessageIterator(array $source_id_values = [], $level = NULL);
 
diff --git a/core/modules/migrate/src/Plugin/migrate/id_map/NullIdMap.php b/core/modules/migrate/src/Plugin/migrate/id_map/NullIdMap.php
index 7b792cb3c516..0699703ecc55 100644
--- a/core/modules/migrate/src/Plugin/migrate/id_map/NullIdMap.php
+++ b/core/modules/migrate/src/Plugin/migrate/id_map/NullIdMap.php
@@ -84,10 +84,18 @@ public function saveMessage(array $source_id_values, $message, $level = Migratio
   /**
    * {@inheritdoc}
    */
-  public function getMessageIterator(array $source_id_values = [], $level = NULL) {
+  public function getMessages(array $source_id_values = [], $level = NULL) {
     return new \ArrayIterator([]);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getMessageIterator(array $source_id_values = [], $level = NULL) {
+    @trigger_error('getMessageIterator() is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use getMessages() instead. See https://www.drupal.org/node/3060969', E_USER_DEPRECATED);
+    return $this->getMessages($source_id_values, $level);
+  }
+
   /**
    * {@inheritdoc}
    */
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 4ae4686ef77b..03f631fb2628 100644
--- a/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
+++ b/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
@@ -676,7 +676,7 @@ public function saveMessage(array $source_id_values, $message, $level = Migratio
   /**
    * {@inheritdoc}
    */
-  public function getMessageIterator(array $source_id_values = [], $level = NULL) {
+  public function getMessages(array $source_id_values = [], $level = NULL) {
     $query = $this->getDatabase()->select($this->messageTableName(), 'msg');
     $condition = sprintf('msg.%s = map.%s', $this::SOURCE_IDS_HASH, $this::SOURCE_IDS_HASH);
     $query->addJoin('LEFT', $this->mapTableName(), 'map', $condition);
@@ -698,6 +698,14 @@ public function getMessageIterator(array $source_id_values = [], $level = NULL)
     return $query->execute();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getMessageIterator(array $source_id_values = [], $level = NULL) {
+    @trigger_error('getMessageIterator() is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use getMessages() instead. See https://www.drupal.org/node/3060969', E_USER_DEPRECATED);
+    return $this->getMessages($source_id_values, $level);
+  }
+
   /**
    * {@inheritdoc}
    */
diff --git a/core/modules/migrate/tests/src/Functional/process/DownloadFunctionalTest.php b/core/modules/migrate/tests/src/Functional/process/DownloadFunctionalTest.php
index 5e890a0e6a07..28824fb094cd 100644
--- a/core/modules/migrate/tests/src/Functional/process/DownloadFunctionalTest.php
+++ b/core/modules/migrate/tests/src/Functional/process/DownloadFunctionalTest.php
@@ -65,7 +65,7 @@ public function testExceptionThrow() {
     $this->assertNull($map_row['destid1']);
 
     // Check that a message with the thrown exception has been logged.
-    $messages = $id_map_plugin->getMessageIterator(['url' => $invalid_url])->fetchAll();
+    $messages = $id_map_plugin->getMessages(['url' => $invalid_url])->fetchAll();
     $this->assertCount(1, $messages);
     $message = reset($messages);
     $this->assertEquals("Cannot read from non-readable stream ($invalid_url)", $message->message);
diff --git a/core/modules/migrate/tests/src/Kernel/MigrateMessageTest.php b/core/modules/migrate/tests/src/Kernel/MigrateMessageTest.php
index 839923285279..a4cebef64d7d 100644
--- a/core/modules/migrate/tests/src/Kernel/MigrateMessageTest.php
+++ b/core/modules/migrate/tests/src/Kernel/MigrateMessageTest.php
@@ -98,12 +98,12 @@ public function testMessagesTeed() {
   }
 
   /**
-   * Tests the return value of getMessageIterator().
+   * Tests the return value of getMessages().
    *
    * This method returns an iterator of StdClass objects. Check that these
    * objects have the expected keys.
    */
-  public function testGetMessageIterator() {
+  public function testGetMessages() {
     $expected_message = (object) [
       'src_name' => 'source_message',
       'dest_config_name' => NULL,
@@ -115,7 +115,7 @@ public function testGetMessageIterator() {
     $executable = new MigrateExecutable($this->migration, $this);
     $executable->import();
     $count = 0;
-    foreach ($this->migration->getIdMap()->getMessageIterator() as $message) {
+    foreach ($this->migration->getIdMap()->getMessages() as $message) {
       ++$count;
       $this->assertEqual($message, $expected_message);
     }
diff --git a/core/modules/migrate/tests/src/Kernel/MigrateSkipRowTest.php b/core/modules/migrate/tests/src/Kernel/MigrateSkipRowTest.php
index 8c5e01e51347..dd42f252fd09 100644
--- a/core/modules/migrate/tests/src/Kernel/MigrateSkipRowTest.php
+++ b/core/modules/migrate/tests/src/Kernel/MigrateSkipRowTest.php
@@ -59,14 +59,14 @@ public function testPrepareRowSkip() {
     $map_row = $id_map_plugin->getRowBySource(['id' => 1]);
     $this->assertEqual(MigrateIdMapInterface::STATUS_IGNORED, $map_row['source_row_status']);
     // Check that no message has been logged for the first exception.
-    $messages = $id_map_plugin->getMessageIterator(['id' => 1])->fetchAll();
+    $messages = $id_map_plugin->getMessages(['id' => 1])->fetchAll();
     $this->assertEmpty($messages);
 
     // The second row is not recorded in the map.
     $map_row = $id_map_plugin->getRowBySource(['id' => 2]);
     $this->assertFalse($map_row);
     // Check that the correct message has been logged for the second exception.
-    $messages = $id_map_plugin->getMessageIterator(['id' => 2])->fetchAll();
+    $messages = $id_map_plugin->getMessages(['id' => 2])->fetchAll();
     $this->assertCount(1, $messages);
     $message = reset($messages);
     $this->assertEquals('skip_and_dont_record message', $message->message);
diff --git a/core/modules/migrate/tests/src/Unit/MigrateNullIdMapTest.php b/core/modules/migrate/tests/src/Unit/MigrateNullIdMapTest.php
new file mode 100644
index 000000000000..eba4bbb4c27e
--- /dev/null
+++ b/core/modules/migrate/tests/src/Unit/MigrateNullIdMapTest.php
@@ -0,0 +1,26 @@
+<?php
+
+namespace Drupal\Tests\migrate\Unit;
+
+use Drupal\migrate\Plugin\migrate\id_map\NullIdMap;
+
+/**
+ * Tests the NULL ID map plugin.
+ *
+ * @group migrate
+ */
+class MigrateNullIdMapTest extends MigrateTestCase {
+
+  /**
+   * Tests the NULL ID map get message iterator method.
+   *
+   * @group legacy
+   *
+   * @expectedDeprecation getMessageIterator() is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use getMessages() instead. See https://www.drupal.org/node/3060969
+   */
+  public function testGetMessageIterator() {
+    $id_map = new NullIdMap([], 'null', NULL);
+    $id_map->getMessageIterator();
+  }
+
+}
diff --git a/core/modules/migrate/tests/src/Unit/MigrateSqlIdMapTest.php b/core/modules/migrate/tests/src/Unit/MigrateSqlIdMapTest.php
index 249c26ee8f4f..098603ebcc77 100644
--- a/core/modules/migrate/tests/src/Unit/MigrateSqlIdMapTest.php
+++ b/core/modules/migrate/tests/src/Unit/MigrateSqlIdMapTest.php
@@ -312,7 +312,7 @@ public function testMessageSave() {
       $id_map->saveMessage(['source_id_property' => $key], $message, $original_value['level']);
     }
 
-    foreach ($id_map->getMessageIterator() as $message_row) {
+    foreach ($id_map->getMessages() as $message_row) {
       $key = $message_row->source_ids_hash;
       $this->assertEquals($expected_results[$key]['message'], $message_row->message);
       $this->assertEquals($expected_results[$key]['level'], $message_row->level);
@@ -321,7 +321,7 @@ public function testMessageSave() {
     // Insert with default level.
     $message_default = 'Hello world default.';
     $id_map->saveMessage(['source_id_property' => 5], $message_default);
-    $messages = $id_map->getMessageIterator(['source_id_property' => 5]);
+    $messages = $id_map->getMessages(['source_id_property' => 5]);
     $count = 0;
     foreach ($messages as $key => $message_row) {
       $count = 1;
@@ -331,7 +331,7 @@ public function testMessageSave() {
     $this->assertEquals($count, 1);
 
     // Retrieve messages with a specific level.
-    $messages = $id_map->getMessageIterator([], MigrationInterface::MESSAGE_WARNING);
+    $messages = $id_map->getMessages([], MigrationInterface::MESSAGE_WARNING);
     $count = 0;
     foreach ($messages as $key => $message_row) {
       $count = 1;
@@ -340,6 +340,17 @@ public function testMessageSave() {
     $this->assertEquals($count, 1);
   }
 
+  /**
+   * Tests the SQL ID map get message iterator method.
+   *
+   * @group legacy
+   *
+   * @expectedDeprecation getMessageIterator() is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use getMessages() instead. See https://www.drupal.org/node/3060969
+   */
+  public function testGetMessageIterator() {
+    $this->getIdMap()->getMessageIterator();
+  }
+
   /**
    * Tests the getRowBySource method.
    */
-- 
GitLab