From 2ee4a7fbd35d68f0af8a9c064b1381b661c70612 Mon Sep 17 00:00:00 2001 From: webchick <drupal@webchick.net> Date: Wed, 16 Sep 2015 13:53:46 -0700 Subject: [PATCH] Issue #2567571 by mikeryan: Manage per-row messages more rationally --- .../modules/migrate/src/MigrateExecutable.php | 38 +------------ .../src/MigrateExecutableInterface.php | 14 ----- .../migrate/source/SourcePluginBase.php | 19 +++---- .../tests/src/Unit/MigrateExecutableTest.php | 55 ------------------- 4 files changed, 8 insertions(+), 118 deletions(-) diff --git a/core/modules/migrate/src/MigrateExecutable.php b/core/modules/migrate/src/MigrateExecutable.php index 5e9f38cc2f9e..70c61d17aa14 100644 --- a/core/modules/migrate/src/MigrateExecutable.php +++ b/core/modules/migrate/src/MigrateExecutable.php @@ -41,17 +41,6 @@ class MigrateExecutable implements MigrateExecutableInterface { */ protected $sourceRowStatus; - /** - * The queued messages not yet saved. - * - * Each element in the array is an array with two keys: - * - 'message': The message string. - * - 'level': The level, a MigrationInterface::MESSAGE_* constant. - * - * @var array - */ - protected $queuedMessages = array(); - /** * The ratio of the memory limit at which an operation will be interrupted. * @@ -178,10 +167,6 @@ public function __construct(MigrationInterface $migration, MigrateMessageInterfa protected function getSource() { if (!isset($this->source)) { $this->source = $this->migration->getSourcePlugin(); - - // @TODO, find out how to remove this. - // @see https://www.drupal.org/node/2443617 - $this->source->migrateExecutable = $this; } return $this->source; } @@ -245,11 +230,7 @@ public function import() { $destination = $this->migration->getDestinationPlugin(); while ($source->valid()) { $row = $source->current(); - if ($this->sourceIdValues = $row->getSourceIdValues()) { - // Wipe old messages, and save any new messages. - $id_map->delete($this->sourceIdValues, TRUE); - $this->saveQueuedMessages(); - } + $this->sourceIdValues = $row->getSourceIdValues(); try { $this->processRow($row); @@ -399,23 +380,6 @@ public function saveMessage($message, $level = MigrationInterface::MESSAGE_ERROR $this->migration->getIdMap()->saveMessage($this->sourceIdValues, $message, $level); } - /** - * {@inheritdoc} - */ - public function queueMessage($message, $level = MigrationInterface::MESSAGE_ERROR) { - $this->queuedMessages[] = array('message' => $message, 'level' => $level); - } - - /** - * {@inheritdoc} - */ - public function saveQueuedMessages() { - foreach ($this->queuedMessages as $queued_message) { - $this->saveMessage($queued_message['message'], $queued_message['level']); - } - $this->queuedMessages = array(); - } - /** * Takes an Exception object and both saves and displays it. * diff --git a/core/modules/migrate/src/MigrateExecutableInterface.php b/core/modules/migrate/src/MigrateExecutableInterface.php index 415a53f85736..3edf6200559e 100644 --- a/core/modules/migrate/src/MigrateExecutableInterface.php +++ b/core/modules/migrate/src/MigrateExecutableInterface.php @@ -46,18 +46,4 @@ public function processRow(Row $row, array $process = NULL, $value = NULL); */ public function saveMessage($message, $level = MigrationInterface::MESSAGE_ERROR); - /** - * Queues messages to be later saved through the map class. - * - * @param string $message - * The message to record. - * @param int $level - * (optional) Message severity (defaults to MESSAGE_ERROR). - */ - public function queueMessage($message, $level = MigrationInterface::MESSAGE_ERROR); - - /** - * Saves any messages we've queued up to the message table. - */ - public function saveQueuedMessages(); } diff --git a/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php b/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php index bfb8e0ee18ea..bfa35996ba21 100644 --- a/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php +++ b/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php @@ -125,14 +125,6 @@ abstract class SourcePluginBase extends PluginBase implements MigrateSourceInter */ protected $iterator; - /** - * @TODO, find out how to remove this. - * @see https://www.drupal.org/node/2443617 - * - * @var MigrateExecutableInterface - */ - public $migrateExecutable; - /** * {@inheritdoc} */ @@ -199,11 +191,8 @@ public function prepareRow(Row $row) { if ($skip) { // Make sure we replace any previous messages for this item with any // new ones. - $id_map = $this->migration->getIdMap(); - $id_map->delete($this->currentSourceIds, TRUE); - $this->migrateExecutable->saveQueuedMessages(); if ($save_to_map) { - $id_map->saveIdMapping($row, array(), MigrateIdMapInterface::STATUS_IGNORED, $this->migrateExecutable->rollbackAction); + $this->migration->getIdMap()->saveIdMapping($row, array(), MigrateIdMapInterface::STATUS_IGNORED); $this->currentRow = NULL; $this->currentSourceIds = NULL; } @@ -307,6 +296,12 @@ public function next() { $row->setIdMap($id_map); } + // Clear any previous messages for this row before potentially adding + // new ones. + if (!empty($this->currentSourceIds)) { + $this->idMap->delete($this->currentSourceIds, TRUE); + } + // Preparing the row gives source plugins the chance to skip. if ($this->prepareRow($row) === FALSE) { continue; diff --git a/core/modules/migrate/tests/src/Unit/MigrateExecutableTest.php b/core/modules/migrate/tests/src/Unit/MigrateExecutableTest.php index 97d7662eca91..63d509955c60 100644 --- a/core/modules/migrate/tests/src/Unit/MigrateExecutableTest.php +++ b/core/modules/migrate/tests/src/Unit/MigrateExecutableTest.php @@ -199,10 +199,6 @@ public function testImportWithValidRowNoDestinationValues() { ->method('getDestinationPlugin') ->will($this->returnValue($destination)); - $this->idMap->expects($this->once()) - ->method('delete') - ->with(array('id' => 'test'), TRUE); - $this->idMap->expects($this->once()) ->method('saveIdMapping') ->with($row, array(), MigrateIdMapInterface::STATUS_FAILED, NULL); @@ -378,57 +374,6 @@ public function testImportWithValidRowWithException() { $this->assertSame(MigrationInterface::RESULT_COMPLETED, $this->executable->import()); } - /** - * Tests saving of queued messages. - */ - public function testSaveQueuedMessages() { - // Assert no queued messages before save. - $this->assertAttributeEquals(array(), 'queuedMessages', $this->executable); - // Set required source_id_values for MigrateIdMapInterface::saveMessage(). - $expected_messages[] = array('message' => 'message 1', 'level' => MigrationInterface::MESSAGE_ERROR); - $expected_messages[] = array('message' => 'message 2', 'level' => MigrationInterface::MESSAGE_WARNING); - $expected_messages[] = array('message' => 'message 3', 'level' => MigrationInterface::MESSAGE_INFORMATIONAL); - foreach ($expected_messages as $queued_message) { - $this->executable->queueMessage($queued_message['message'], $queued_message['level']); - } - $this->executable->setSourceIdValues(array()); - $this->assertAttributeEquals($expected_messages, 'queuedMessages', $this->executable); - // No asserts of saved messages since coverage exists - // in MigrateSqlIdMapTest::saveMessage(). - $this->executable->saveQueuedMessages(); - // Assert no queued messages after save. - $this->assertAttributeEquals(array(), 'queuedMessages', $this->executable); - } - - /** - * Tests the queuing of messages. - */ - public function testQueueMessage() { - // Assert no queued messages. - $expected_messages = array(); - $this->assertAttributeEquals(array(), 'queuedMessages', $this->executable); - // Assert a single (default level) queued message. - $expected_messages[] = array( - 'message' => 'message 1', - 'level' => MigrationInterface::MESSAGE_ERROR, - ); - $this->executable->queueMessage('message 1'); - $this->assertAttributeEquals($expected_messages, 'queuedMessages', $this->executable); - // Assert multiple queued messages. - $expected_messages[] = array( - 'message' => 'message 2', - 'level' => MigrationInterface::MESSAGE_WARNING, - ); - $this->executable->queueMessage('message 2', MigrationInterface::MESSAGE_WARNING); - $this->assertAttributeEquals($expected_messages, 'queuedMessages', $this->executable); - $expected_messages[] = array( - 'message' => 'message 3', - 'level' => MigrationInterface::MESSAGE_INFORMATIONAL, - ); - $this->executable->queueMessage('message 3', MigrationInterface::MESSAGE_INFORMATIONAL); - $this->assertAttributeEquals($expected_messages, 'queuedMessages', $this->executable); - } - /** * Tests the processRow method. */ -- GitLab