diff --git a/core/modules/book/src/Plugin/migrate/destination/Book.php b/core/modules/book/src/Plugin/migrate/destination/Book.php index 3659b34f4f..0e065b6ec5 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 0cebbd07c8..b1f5c161cc 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 708addf03a..3330abeada 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 8d15cf4bc9..581ad9b4bb 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,18 @@ 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. + * @return \Drupal\Core\Entity\EntityInterface + * An updated entity from row values. This deprecated in Drupal 8.6.0 and + * will be removed in Drupal 9. See tbd. */ - 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; diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php index 52d0f4fe15..522651132a 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 0000000000..d15446fbad --- /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 9f43523560..d0fd987dc3 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/EntityContentBaseTest.php b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php similarity index 53% copy from core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php copy to core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php index 9f43523560..3fb4b7dbda 100644 --- a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php +++ b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php @@ -1,13 +1,7 @@ storage = $this->prophesize(EntityStorageInterface::class); $this->entityType = $this->prophesize(EntityTypeInterface::class); - $this->entityType->getPluralLabel()->willReturn('wonkiness'); + $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); } /** - * Test basic entity save. - * - * @covers ::import + * Tests that revision destination fails for unrevisionable entities. */ - public function testImport() { - $bundles = []; - $destination = new EntityTestDestination([], '', [], - $this->migration->reveal(), - $this->storage->reveal(), - $bundles, - $this->entityManager->reveal(), - $this->prophesize(FieldTypePluginManagerInterface::class)->reveal()); - $entity = $this->prophesize(ContentEntityInterface::class); - // Assert that save is called. - $entity->save() - ->shouldBeCalledTimes(1); - // Set an id for the entity - $entity->id() - ->willReturn(5); - $destination->setEntity($entity->reveal()); - // Ensure the id is saved entity id is returned from import. - $this->assertEquals([5], $destination->import(new Row())); - // Assert that import set the rollback action. - $this->assertEquals(MigrateIdMapInterface::ROLLBACK_DELETE, $destination->rollbackAction()); - } + public function testUnrevisionable() { + $this->entityType->getKey('id')->willReturn('id'); + $this->entityType->getKey('revision')->willReturn(''); + $this->entityManager->getBaseFieldDefinitions('foo') + ->willReturn([ + 'id' => BaseFieldDefinitionTest::create('integer'), + ]); - /** - * Test row skipping when we can't get an entity to save. - * - * @covers ::import - */ - public function testImportEntityLoadFailure() { - $bundles = []; - $destination = new EntityTestDestination([], '', [], + $destination = new EntityRevisionTestDestination( + [], + '', + [], $this->migration->reveal(), $this->storage->reveal(), - $bundles, + [], $this->entityManager->reveal(), - $this->prophesize(FieldTypePluginManagerInterface::class)->reveal()); - $destination->setEntity(FALSE); - $this->setExpectedException(MigrateException::class, 'Unable to get entity'); - $destination->import(new Row()); + $this->prophesize(FieldTypePluginManagerInterface::class)->reveal() + ); + $this->setExpectedException(MigrateException::class, 'The "foo" entity type does not support revisions.'); + $destination->getIds(); } /** - * Test that translation destination fails for untranslatable entities. + * Tests that translation destination fails for untranslatable entities. */ public function testUntranslatable() { - // An entity type without a language. - $this->entityType->getKey('langcode')->willReturn(''); $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')]); + ->willReturn([ + 'id' => BaseFieldDefinitionTest::create('integer'), + 'vid' => BaseFieldDefinitionTest::create('integer'), + ]); - $destination = new EntityTestDestination( + $destination = new EntityRevisionTestDestination( ['translations' => TRUE], '', [], @@ -129,18 +115,16 @@ 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(); } } /** - * Stub class for testing EntityContentBase methods. - * - * We want to test things without testing the base class implementations. + * Stub class for testing EntityRevision methods. */ -class EntityTestDestination extends EntityContentBase { +class EntityRevisionTestDestination extends EntityRevision { private $entity = NULL; diff --git a/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php b/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php index 8a6fe9a43d..73b809dd61 100644 --- a/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php +++ b/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php @@ -230,6 +230,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 6be7801b6e..db44c63450 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 ddd8da7956..7251bfb6a6 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')); }