From d934d53d28c3a0288497ae61964983a5cf7ed0b5 Mon Sep 17 00:00:00 2001 From: webchick Date: Tue, 27 Oct 2015 20:22:37 -0400 Subject: [PATCH] Issue #2598696 by mikeryan, effulgentsia, quietone: Rollback should not delete uid 1 --- .../modules/migrate/src/MigrateExecutable.php | 19 ++++------- .../Plugin/MigrateDestinationInterface.php | 12 +++++++ .../migrate/destination/DestinationBase.php | 34 +++++++++++++++++++ .../migrate/destination/EntityConfigBase.php | 5 ++- .../migrate/destination/EntityContentBase.php | 4 +++ .../migrate/src/Tests/MigrateRollbackTest.php | 16 ++++++--- .../src/Tests/Migrate/d6/MigrateUserTest.php | 24 ++++++++++++- 7 files changed, 94 insertions(+), 20 deletions(-) diff --git a/core/modules/migrate/src/MigrateExecutable.php b/core/modules/migrate/src/MigrateExecutable.php index 4755167220d..2749313d325 100644 --- a/core/modules/migrate/src/MigrateExecutable.php +++ b/core/modules/migrate/src/MigrateExecutable.php @@ -64,13 +64,6 @@ class MigrateExecutable implements MigrateExecutableInterface { */ protected $sourceIdValues; - /** - * The rollback action to be saved for the current row. - * - * @var int - */ - public $rollbackAction; - /** * An array of counts. Initially used for cache hit/miss tracking. * @@ -230,12 +223,12 @@ class MigrateExecutable implements MigrateExecutableInterface { $save = TRUE; } catch (MigrateException $e) { - $this->migration->getIdMap()->saveIdMapping($row, array(), $e->getStatus(), $this->rollbackAction); + $this->migration->getIdMap()->saveIdMapping($row, array(), $e->getStatus()); $this->saveMessage($e->getMessage(), $e->getLevel()); $save = FALSE; } catch (MigrateSkipRowException $e) { - $id_map->saveIdMapping($row, array(), MigrateIdMapInterface::STATUS_IGNORED, $this->rollbackAction); + $id_map->saveIdMapping($row, array(), MigrateIdMapInterface::STATUS_IGNORED); $save = FALSE; } @@ -247,11 +240,11 @@ class MigrateExecutable implements MigrateExecutableInterface { if ($destination_id_values) { // We do not save an idMap entry for config. if ($destination_id_values !== TRUE) { - $id_map->saveIdMapping($row, $destination_id_values, $this->sourceRowStatus, $this->rollbackAction); + $id_map->saveIdMapping($row, $destination_id_values, $this->sourceRowStatus, $destination->rollbackAction()); } } else { - $id_map->saveIdMapping($row, array(), MigrateIdMapInterface::STATUS_FAILED, $this->rollbackAction); + $id_map->saveIdMapping($row, array(), MigrateIdMapInterface::STATUS_FAILED); if (!$id_map->messageCount()) { $message = $this->t('New object was not saved, no error provided'); $this->saveMessage($message); @@ -260,11 +253,11 @@ class MigrateExecutable implements MigrateExecutableInterface { } } catch (MigrateException $e) { - $this->migration->getIdMap()->saveIdMapping($row, array(), $e->getStatus(), $this->rollbackAction); + $this->migration->getIdMap()->saveIdMapping($row, array(), $e->getStatus()); $this->saveMessage($e->getMessage(), $e->getLevel()); } catch (\Exception $e) { - $this->migration->getIdMap()->saveIdMapping($row, array(), MigrateIdMapInterface::STATUS_FAILED, $this->rollbackAction); + $this->migration->getIdMap()->saveIdMapping($row, array(), MigrateIdMapInterface::STATUS_FAILED); $this->handleException($e); } } diff --git a/core/modules/migrate/src/Plugin/MigrateDestinationInterface.php b/core/modules/migrate/src/Plugin/MigrateDestinationInterface.php index e19e24c5325..81348833c1e 100644 --- a/core/modules/migrate/src/Plugin/MigrateDestinationInterface.php +++ b/core/modules/migrate/src/Plugin/MigrateDestinationInterface.php @@ -80,7 +80,19 @@ interface MigrateDestinationInterface extends PluginInspectionInterface { public function rollback(array $destination_identifier); /** + * Whether the destination can be rolled back or not. + * * @return bool + * TRUE if rollback is supported, FALSE if not. */ public function supportsRollback(); + + /** + * The rollback action for the last imported item. + * + * @return int + * The MigrateIdMapInterface::ROLLBACK_ constant indicating how an imported + * item should be handled on rollback. + */ + public function rollbackAction(); } diff --git a/core/modules/migrate/src/Plugin/migrate/destination/DestinationBase.php b/core/modules/migrate/src/Plugin/migrate/destination/DestinationBase.php index 4f1d596dc99..93f3b83668a 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/DestinationBase.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/DestinationBase.php @@ -12,6 +12,7 @@ use Drupal\Core\Plugin\PluginBase; use Drupal\migrate\Entity\MigrationInterface; use Drupal\migrate\Exception\RequirementsException; use Drupal\migrate\Plugin\MigrateDestinationInterface; +use Drupal\migrate\Plugin\MigrateIdMapInterface; use Drupal\migrate\Plugin\RequirementsInterface; /** @@ -33,6 +34,13 @@ abstract class DestinationBase extends PluginBase implements MigrateDestinationI */ protected $supportsRollback = FALSE; + /** + * The rollback action to be saved for the last imported item. + * + * @var int + */ + protected $rollbackAction = MigrateIdMapInterface::ROLLBACK_DELETE; + /** * The migration. * @@ -57,6 +65,13 @@ abstract class DestinationBase extends PluginBase implements MigrateDestinationI $this->migration = $migration; } + /** + * {@inheritdoc} + */ + public function rollbackAction() { + return $this->rollbackAction; + } + /** * {@inheritdoc} */ @@ -79,4 +94,23 @@ abstract class DestinationBase extends PluginBase implements MigrateDestinationI public function supportsRollback() { return $this->supportsRollback; } + + /** + * For a destination item being updated, set the appropriate rollback action. + * + * @param array $id_map + * The map row data for the item. + */ + protected function setRollbackAction(array $id_map) { + // If the entity we're updating was previously migrated by us, preserve the + // existing rollback action. + if (isset($id_map['sourceid1'])) { + $this->rollbackAction = $id_map['rollback_action']; + } + // Otherwise, we're updating an entity which already existed on the + // destination and want to make sure we do not delete it on rollback. + else { + $this->rollbackAction = MigrateIdMapInterface::ROLLBACK_PRESERVE; + } + } } diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityConfigBase.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityConfigBase.php index 688feffbd25..5e5fbd54f01 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/EntityConfigBase.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityConfigBase.php @@ -10,6 +10,7 @@ namespace Drupal\migrate\Plugin\migrate\destination; use Drupal\Component\Utility\NestedArray; use Drupal\Core\Entity\EntityInterface; use Drupal\migrate\MigrateException; +use Drupal\migrate\Plugin\MigrateIdMapInterface; use Drupal\migrate\Row; /** @@ -31,6 +32,7 @@ class EntityConfigBase extends Entity { if ($row->isStub()) { throw new MigrateException('Config entities can not be stubbed.'); } + $this->rollbackAction = MigrateIdMapInterface::ROLLBACK_DELETE; $ids = $this->getIds(); $id_key = $this->getKey('id'); if (count($ids) > 1) { @@ -64,7 +66,6 @@ class EntityConfigBase extends Entity { return $ids; } - /** * Updates an entity with the contents of a row. * @@ -77,6 +78,8 @@ class EntityConfigBase extends Entity { foreach ($row->getRawDestination() as $property => $value) { $this->updateEntityProperty($entity, explode(Row::PROPERTY_SEPARATOR, $property), $value); } + + $this->setRollbackAction($row->getIdMap()); } /** diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php index f80034ce2c2..ae3030a11b2 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php @@ -13,6 +13,7 @@ use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\TypedData\TypedDataInterface; use Drupal\migrate\Entity\MigrationInterface; +use Drupal\migrate\Plugin\MigrateIdMapInterface; use Drupal\migrate\Row; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -71,6 +72,7 @@ class EntityContentBase extends Entity { * {@inheritdoc} */ public function import(Row $row, array $old_destination_id_values = array()) { + $this->rollbackAction = MigrateIdMapInterface::ROLLBACK_DELETE; $entity = $this->getEntity($row, $old_destination_id_values); return $this->save($entity, $old_destination_id_values); } @@ -126,6 +128,8 @@ class EntityContentBase extends Entity { $field->setValue($values); } } + + $this->setRollbackAction($row->getIdMap()); } } diff --git a/core/modules/migrate/src/Tests/MigrateRollbackTest.php b/core/modules/migrate/src/Tests/MigrateRollbackTest.php index 4fda48c7822..ec03bb1aef8 100644 --- a/core/modules/migrate/src/Tests/MigrateRollbackTest.php +++ b/core/modules/migrate/src/Tests/MigrateRollbackTest.php @@ -111,15 +111,21 @@ class MigrateRollbackTest extends MigrateTestBase { $this->assertTrue($term_migration->getDestinationPlugin()->supportsRollback()); + // Pre-create a term, to make sure it isn't deleted on rollback. + $preserved_term_ids[] = 1; + $new_term = Term::create(['tid' => 1, 'vid' => 1, 'name' => 'music']); + $new_term->save(); + // Import and validate term entities were created. $term_executable = new MigrateExecutable($term_migration, $this); $term_executable->import(); - // Mark one row to be preserved on rollback. - $preserved_term_id = 2; - $map_row = $term_id_map->getRowBySource(['id' => $preserved_term_id]); - $dummy_row = new Row(['id' => $preserved_term_id], $ids); + // Also explicitly mark one row to be preserved on rollback. + $preserved_term_ids[] = 2; + $map_row = $term_id_map->getRowBySource(['id' => 2]); + $dummy_row = new Row(['id' => 2], $ids); $term_id_map->saveIdMapping($dummy_row, [$map_row['destid1']], $map_row['source_row_status'], MigrateIdMapInterface::ROLLBACK_PRESERVE); + foreach ($term_data_rows as $row) { /** @var Term $term */ $term = Term::load($row['id']); @@ -132,7 +138,7 @@ class MigrateRollbackTest extends MigrateTestBase { $term_executable->rollback(); foreach ($term_data_rows as $row) { $term = Term::load($row['id']); - if ($row['id'] == $preserved_term_id) { + if (in_array($row['id'], $preserved_term_ids)) { $this->assertNotNull($term); } else { diff --git a/core/modules/user/src/Tests/Migrate/d6/MigrateUserTest.php b/core/modules/user/src/Tests/Migrate/d6/MigrateUserTest.php index d76e2145c94..ddb93cf4fb3 100644 --- a/core/modules/user/src/Tests/Migrate/d6/MigrateUserTest.php +++ b/core/modules/user/src/Tests/Migrate/d6/MigrateUserTest.php @@ -8,6 +8,7 @@ namespace Drupal\user\Tests\Migrate\d6; use Drupal\migrate\Entity\Migration; +use Drupal\migrate\MigrateExecutable; use Drupal\user\Entity\User; use Drupal\file\Entity\File; use Drupal\Core\Database\Database; @@ -29,6 +30,10 @@ class MigrateUserTest extends MigrateDrupal6TestBase { $this->installEntitySchema('file'); $this->installSchema('file', ['file_usage']); + $this->installEntitySchema('node'); + $this->installSchema('user', ['users_data']); + // Make sure uid 1 is created. + user_install(); $file = File::create(array( 'fid' => 2, @@ -68,7 +73,7 @@ class MigrateUserTest extends MigrateDrupal6TestBase { $users = Database::getConnection('default', 'migrate') ->select('users', 'u') ->fields('u') - ->condition('uid', 1, '>') + ->condition('uid', 0, '>') ->execute() ->fetchAll(); @@ -117,6 +122,23 @@ class MigrateUserTest extends MigrateDrupal6TestBase { // conform the Drupal >= 7. $this->assertTrue(\Drupal::service('password')->check($source->pass_plain, $user->getPassword())); } + // Rollback the migration and make sure everything is deleted but uid 1. + (new MigrateExecutable($this->migration, $this))->rollback(); + $users = Database::getConnection('default', 'migrate') + ->select('users', 'u') + ->fields('u', ['uid']) + ->condition('uid', 0, '>') + ->execute() + ->fetchCol(); + foreach ($users as $uid) { + $account = User::load($uid); + if ($uid == 1) { + $this->assertNotNull($account, 'User 1 was preserved after rollback'); + } + else { + $this->assertNull($account); + } + } } }