diff --git a/core/modules/book/src/Plugin/migrate/destination/Book.php b/core/modules/book/src/Plugin/migrate/destination/Book.php index 3659b34..0e065b6 100644 --- a/core/modules/book/src/Plugin/migrate/destination/Book.php +++ b/core/modules/book/src/Plugin/migrate/destination/Book.php @@ -24,7 +24,7 @@ protected static function getEntityTypeId($plugin_id) { /** * {@inheritdoc} */ - protected function updateEntity(EntityInterface $entity, Row $row) { + protected function updateEntity(EntityInterface &$entity, Row $row) { $entity->book = $row->getDestinationProperty('book'); } diff --git a/core/modules/migrate/src/Plugin/migrate/destination/Entity.php b/core/modules/migrate/src/Plugin/migrate/destination/Entity.php index 0cebbd0..b1f5c16 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/Entity.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/Entity.php @@ -170,7 +170,11 @@ protected function getEntity(Row $row, array $old_destination_id_values) { $entity_id = reset($old_destination_id_values) ?: $this->getEntityId($row); if (!empty($entity_id) && ($entity = $this->storage->load($entity_id))) { // Allow updateEntity() to change the entity. - $entity = $this->updateEntity($entity, $row) ?: $entity; + $result = $this->updateEntity($entity, $row); + if ($result !== NULL) { + trigger_error(__CLASS__ . '::updateEntity() should not return an entity. See https://www.drupal.org/project/drupal/issues/2921661'); + $entity = $result; + } } else { // Attempt to ensure we always have a bundle. diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityConfigBase.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityConfigBase.php index af9f35c..985460f 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/EntityConfigBase.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityConfigBase.php @@ -186,12 +186,14 @@ public function getIds() { /** * Updates an entity with the contents of a row. * + * Note that the entity passed in is updated. + * * @param \Drupal\Core\Entity\EntityInterface $entity * The entity to update. * @param \Drupal\migrate\Row $row * The row object to update from. */ - protected function updateEntity(EntityInterface $entity, Row $row) { + protected function updateEntity(EntityInterface &$entity, Row $row) { // This is a translation if the language in the active config does not // match the language of this row. $translation = FALSE; diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php index 8d15cf4..82d3cb9 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php @@ -148,7 +148,7 @@ public function import(Row $row, array $old_destination_id_values = []) { } $ids = $this->save($entity, $old_destination_id_values); - if (!empty($this->configuration['translations'])) { + if ($this->isTranslationDestination()) { $ids[] = $entity->language()->getId(); } return $ids; @@ -181,12 +181,15 @@ public function isTranslationDestination() { * {@inheritdoc} */ public function getIds() { + $ids = []; + $id_key = $this->getKey('id'); $ids[$id_key] = $this->getDefinitionFromEntity($id_key); if ($this->isTranslationDestination()) { - if (!$langcode_key = $this->getKey('langcode')) { - throw new MigrateException('This entity type does not support translation.'); + $langcode_key = $this->getKey('langcode'); + if (!$langcode_key) { + throw new MigrateException(sprintf('The "%s" entity type does not support translations.', $this->storage->getEntityTypeId())); } $ids[$langcode_key] = $this->getDefinitionFromEntity($langcode_key); } @@ -197,15 +200,16 @@ public function getIds() { /** * Updates an entity with the new values from row. * + * Note that the entity passed in will also be updated. + * * @param \Drupal\Core\Entity\EntityInterface $entity * The entity to update. * @param \Drupal\migrate\Row $row * The row object to update from. * - * @return \Drupal\Core\Entity\EntityInterface|null - * An updated entity, or NULL if it's the same as the one passed in. + * @see https://www.drupal.org/node/2960492 */ - protected function updateEntity(EntityInterface $entity, Row $row) { + protected function updateEntity(EntityInterface &$entity, Row $row) { $empty_destinations = $row->getEmptyDestinationProperties(); // By default, an update will be preserved. $rollback_action = MigrateIdMapInterface::ROLLBACK_PRESERVE; @@ -248,9 +252,6 @@ protected function updateEntity(EntityInterface $entity, Row $row) { } $this->setRollbackAction($row->getIdMap(), $rollback_action); - - // We might have a different (translated) entity, so return it. - return $entity; } /** diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php index 52d0f4f..5226511 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php @@ -160,7 +160,13 @@ protected function getEntity(Row $row, array $old_destination_id_values) { $entity->enforceIsNew(FALSE); $entity->setNewRevision(TRUE); } - $this->updateEntity($entity, $row); + // We need to update the entity, so that the destination row IDs are + // correct. + $result = $this->updateEntity($entity, $row); + if ($result !== NULL) { + trigger_error(__CLASS__ . '::updateEntity() should not return an entity. See https://www.drupal.org/project/drupal/issues/2921661'); + $entity = $result; + } $entity->isDefaultRevision(FALSE); return $entity; } @@ -177,10 +183,23 @@ protected function save(ContentEntityInterface $entity, array $old_destination_i * {@inheritdoc} */ public function getIds() { - if ($key = $this->getKey('revision')) { - return [$key => $this->getDefinitionFromEntity($key)]; + $ids = []; + + $revision_key = $this->getKey('revision'); + if (!$revision_key) { + throw new MigrateException(sprintf('The "%s" entity type does not support revisions.', $this->storage->getEntityTypeId())); + } + $ids[$revision_key] = $this->getDefinitionFromEntity($revision_key); + + if ($this->isTranslationDestination()) { + $langcode_key = $this->getKey('langcode'); + if (!$langcode_key) { + throw new MigrateException(sprintf('The "%s" entity type does not support translations.', $this->storage->getEntityTypeId())); + } + $ids[$langcode_key] = $this->getDefinitionFromEntity($langcode_key); } - throw new MigrateException('This entity type does not support revisions.'); + + return $ids; } /** diff --git a/core/modules/migrate/tests/src/Kernel/Plugin/EntityRevisionTest.php b/core/modules/migrate/tests/src/Kernel/Plugin/EntityRevisionTest.php new file mode 100644 index 0000000..d15446f --- /dev/null +++ b/core/modules/migrate/tests/src/Kernel/Plugin/EntityRevisionTest.php @@ -0,0 +1,131 @@ +installConfig('node'); + $this->installSchema('node', ['node_access']); + $this->installEntitySchema('node'); + $this->installEntitySchema('user'); + } + + /** + * Tests that EntityRevision correctly handles revision translations. + */ + public function testRevisionTranslation() { + ConfigurableLanguage::createFromLangcode('fr')->save(); + + /** @var \Drupal\node\NodeInterface $node */ + $node = Node::create([ + 'type' => $this->createContentType()->id(), + 'title' => 'Default 1', + ]); + $node->addTranslation('fr', [ + 'title' => 'French 1', + ]); + $node->save(); + $node->setNewRevision(); + $node->setTitle('Default 2'); + $node->getTranslation('fr')->setTitle('French 2'); + $node->save(); + + $migration = [ + 'source' => [ + 'plugin' => 'embedded_data', + 'data_rows' => [ + [ + 'nid' => $node->id(), + 'vid' => $node->getRevisionId(), + 'langcode' => 'fr', + 'title' => 'Titre nouveau, tabarnak!', + ], + ], + 'ids' => [ + 'nid' => [ + 'type' => 'integer', + ], + 'vid' => [ + 'type' => 'integer', + ], + 'langcode' => [ + 'type' => 'string', + ], + ], + ], + 'process' => [ + 'nid' => 'nid', + 'vid' => 'vid', + 'langcode' => 'langcode', + 'title' => 'title', + ], + 'destination' => [ + 'plugin' => 'entity_revision:node', + 'translations' => TRUE, + ], + ]; + + /** @var \Drupal\migrate\Plugin\MigrationInterface $migration */ + $migration = $this->container + ->get('plugin.manager.migration') + ->createStubMigration($migration); + + $this->executeMigration($migration); + + // The entity_revision destination uses the revision ID and langcode as its + // keys (the langcode is only used if the destination is configured for + // translation), so we should be able to look up the source IDs by revision + // ID and langcode. + $source_ids = $migration->getIdMap()->lookupSourceID([ + 'vid' => $node->getRevisionId(), + 'langcode' => 'fr', + ]); + $this->assertNotEmpty($source_ids); + $this->assertSame($node->id(), $source_ids['nid']); + $this->assertSame($node->getRevisionId(), $source_ids['vid']); + $this->assertSame('fr', $source_ids['langcode']); + + // Confirm the french revision was used in the migration, instead of the + // default revision. + /** @var \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager */ + $entity_type_manager = \Drupal::entityTypeManager(); + $revision = $entity_type_manager->getStorage('node')->loadRevision(1); + $this->assertSame('Default 1', $revision->label()); + $this->assertSame('French 1', $revision->getTranslation('fr')->label()); + $revision = $entity_type_manager->getStorage('node')->loadRevision(2); + $this->assertSame('Default 2', $revision->label()); + $this->assertSame('Titre nouveau, tabarnak!', $revision->getTranslation('fr')->label()); + } + +} diff --git a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php index 9f43523..d0fd987 100644 --- a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php +++ b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php @@ -60,6 +60,7 @@ protected function setUp() { $this->entityType = $this->prophesize(EntityTypeInterface::class); $this->entityType->getPluralLabel()->willReturn('wonkiness'); $this->storage->getEntityType()->willReturn($this->entityType->reveal()); + $this->storage->getEntityTypeId()->willReturn('foo'); $this->entityManager = $this->prophesize(EntityManagerInterface::class); } @@ -129,7 +130,7 @@ public function testUntranslatable() { $this->entityManager->reveal(), $this->prophesize(FieldTypePluginManagerInterface::class)->reveal() ); - $this->setExpectedException(MigrateException::class, 'This entity type does not support translation'); + $this->setExpectedException(MigrateException::class, 'The "foo" entity type does not support translations.'); $destination->getIds(); } diff --git a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php new file mode 100644 index 0000000..3fb4b7d --- /dev/null +++ b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php @@ -0,0 +1,162 @@ +migration = $this->prophesize(MigrationInterface::class); + $this->storage = $this->prophesize(EntityStorageInterface::class); + + $this->entityType = $this->prophesize(EntityTypeInterface::class); + $this->entityType->getSingularLabel()->willReturn('foo'); + $this->entityType->getPluralLabel()->willReturn('bar'); + $this->storage->getEntityType()->willReturn($this->entityType->reveal()); + $this->storage->getEntityTypeId()->willReturn('foo'); + + $this->entityManager = $this->prophesize(EntityManagerInterface::class); + } + + /** + * Tests that revision destination fails for unrevisionable entities. + */ + public function testUnrevisionable() { + $this->entityType->getKey('id')->willReturn('id'); + $this->entityType->getKey('revision')->willReturn(''); + $this->entityManager->getBaseFieldDefinitions('foo') + ->willReturn([ + 'id' => BaseFieldDefinitionTest::create('integer'), + ]); + + $destination = new EntityRevisionTestDestination( + [], + '', + [], + $this->migration->reveal(), + $this->storage->reveal(), + [], + $this->entityManager->reveal(), + $this->prophesize(FieldTypePluginManagerInterface::class)->reveal() + ); + $this->setExpectedException(MigrateException::class, 'The "foo" entity type does not support revisions.'); + $destination->getIds(); + } + + /** + * Tests that translation destination fails for untranslatable entities. + */ + public function testUntranslatable() { + $this->entityType->getKey('id')->willReturn('id'); + $this->entityType->getKey('revision')->willReturn('vid'); + $this->entityType->getKey('langcode')->willReturn(''); + $this->entityManager->getBaseFieldDefinitions('foo') + ->willReturn([ + 'id' => BaseFieldDefinitionTest::create('integer'), + 'vid' => BaseFieldDefinitionTest::create('integer'), + ]); + + $destination = new EntityRevisionTestDestination( + ['translations' => TRUE], + '', + [], + $this->migration->reveal(), + $this->storage->reveal(), + [], + $this->entityManager->reveal(), + $this->prophesize(FieldTypePluginManagerInterface::class)->reveal() + ); + $this->setExpectedException(MigrateException::class, 'The "foo" entity type does not support translations.'); + $destination->getIds(); + } + +} + +/** + * Stub class for testing EntityRevision methods. + */ +class EntityRevisionTestDestination extends EntityRevision { + + private $entity = NULL; + + public function setEntity($entity) { + $this->entity = $entity; + } + + protected function getEntity(Row $row, array $old_destination_id_values) { + return $this->entity; + } + + public static function getEntityTypeId($plugin_id) { + return 'foo'; + } + +} + +/** + * Stub class for BaseFieldDefinition. + */ +class BaseFieldDefinitionTest extends BaseFieldDefinition { + + public static function create($type) { + return new static([]); + } + + public function getSettings() { + return []; + } + + public function getType() { + return 'integer'; + } + +} diff --git a/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php b/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php index 7c33191..fe8cd7d 100644 --- a/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php +++ b/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php @@ -229,6 +229,6 @@ public function save(ContentEntityInterface $entity, array $old_destination_id_v * workings of its implementation which would trickle into mock assertions. An * empty implementation avoids this. */ - protected function updateEntity(EntityInterface $entity, Row $row) {} + protected function updateEntity(EntityInterface &$entity, Row $row) {} } diff --git a/core/modules/migrate_drupal/tests/src/Kernel/d6/EntityContentBaseTest.php b/core/modules/migrate_drupal/tests/src/Kernel/d6/EntityContentBaseTest.php index 6be7801..db44c63 100644 --- a/core/modules/migrate_drupal/tests/src/Kernel/d6/EntityContentBaseTest.php +++ b/core/modules/migrate_drupal/tests/src/Kernel/d6/EntityContentBaseTest.php @@ -115,7 +115,7 @@ public function testUntranslatable() { // Match the expected message. Can't use default argument types, because // we need to convert to string from TranslatableMarkup. $argument = Argument::that(function ($msg) { - return strpos((string) $msg, "This entity type does not support translation") !== FALSE; + return strpos((string) $msg, htmlentities('The "no_language_entity_test" entity type does not support translations.')) !== FALSE; }); $message->display($argument, Argument::any()) ->shouldBeCalled(); diff --git a/core/modules/search/src/Plugin/migrate/destination/EntitySearchPage.php b/core/modules/search/src/Plugin/migrate/destination/EntitySearchPage.php index ddd8da7..7251bfb 100644 --- a/core/modules/search/src/Plugin/migrate/destination/EntitySearchPage.php +++ b/core/modules/search/src/Plugin/migrate/destination/EntitySearchPage.php @@ -21,7 +21,7 @@ class EntitySearchPage extends EntityConfigBase { * @param \Drupal\migrate\Row $row * The row object to update from. */ - protected function updateEntity(EntityInterface $entity, Row $row) { + protected function updateEntity(EntityInterface &$entity, Row $row) { $entity->setPlugin($row->getDestinationProperty('plugin')); $entity->getPlugin()->setConfiguration($row->getDestinationProperty('configuration')); }