From 3348de12ea1d5d0b5a86bd98db7459a05a46b592 Mon Sep 17 00:00:00 2001
From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org>
Date: Fri, 28 Oct 2016 12:37:47 +0100
Subject: [PATCH] Issue #2783715 by claudiu.cristea, chx, mikeryan, heddn,
phenaproxima: Entity destination ID schema should match entity ID definition
---
.../Plugin/MigrateDestinationInterface.php | 56 +++++++++++++++++--
.../src/Plugin/MigrateSourceInterface.php | 51 ++++++++++++++++-
.../migrate/destination/EntityContentBase.php | 36 ++++++++++--
.../migrate/destination/EntityRevision.php | 3 +-
.../migrate/src/Plugin/migrate/id_map/Sql.php | 32 +++++++++--
.../migrate_entity_test.info.yml | 6 ++
.../src/Entity/StringIdEntityTest.php | 36 ++++++++++++
.../Kernel/MigrateEntityContentBaseTest.php | 48 ++++++++++++++++
.../destination/EntityContentBaseTest.php | 26 +++++++++
9 files changed, 272 insertions(+), 22 deletions(-)
create mode 100644 core/modules/migrate/tests/modules/migrate_entity_test/migrate_entity_test.info.yml
create mode 100644 core/modules/migrate/tests/modules/migrate_entity_test/src/Entity/StringIdEntityTest.php
diff --git a/core/modules/migrate/src/Plugin/MigrateDestinationInterface.php b/core/modules/migrate/src/Plugin/MigrateDestinationInterface.php
index 0f69308f9c6b..b6e86a09f857 100644
--- a/core/modules/migrate/src/Plugin/MigrateDestinationInterface.php
+++ b/core/modules/migrate/src/Plugin/MigrateDestinationInterface.php
@@ -21,15 +21,59 @@
interface MigrateDestinationInterface extends PluginInspectionInterface {
/**
- * Get the destination IDs.
+ * Gets the destination IDs.
*
* To support MigrateIdMap maps, derived destination classes should return
- * schema field definition(s) corresponding to the primary key of the
- * destination being implemented. These are used to construct the destination
- * key fields of the map table for a migration using this destination.
+ * field definition(s) corresponding to the primary key of the destination
+ * being implemented. These are used to construct the destination key fields
+ * of the map table for a migration using this destination.
*
- * @return array
- * An array of IDs.
+ * @return array[]
+ * An associative array of field definitions keyed by field ID. Values are
+ * associative arrays with a structure that contains the field type ('type'
+ * key). The other keys are the field storage settings as they are returned
+ * by FieldStorageDefinitionInterface::getSettings(). As an example, for a
+ * composite destination primary key that is defined by an integer and a
+ * string, the returned value might look like:
+ * @code
+ * return [
+ * 'id' => [
+ * 'type' => 'integer',
+ * 'unsigned' => FALSE,
+ * 'size' => 'big',
+ * ],
+ * 'version' => [
+ * 'type' => 'string',
+ * 'max_length' => 64,
+ * 'is_ascii' => TRUE,
+ * ],
+ * ];
+ * @endcode
+ * If 'type' points to a field plugin with multiple columns and needs to
+ * refer to a column different than 'value', the key of that column will be
+ * appended as a suffix to the plugin name, separated by dot ('.'). Example:
+ * @code
+ * return [
+ * 'format' => [
+ * 'type' => 'text.format',
+ * ],
+ * ];
+ * @endcode
+ * Additional custom keys/values, that are not part of field storage
+ * definition, can be passed in definitions:
+ * @code
+ * return [
+ * 'nid' => [
+ * 'type' => 'integer',
+ * 'custom_setting' => 'some_value',
+ * ],
+ * ];
+ * @endcode
+ *
+ * @see \Drupal\Core\Field\FieldStorageDefinitionInterface::getSettings()
+ * @see \Drupal\Core\Field\Plugin\Field\FieldType\IntegerItem
+ * @see \Drupal\Core\Field\Plugin\Field\FieldType\StringItem
+ * @see \Drupal\text\Plugin\Field\FieldType\TextItem
*/
public function getIds();
diff --git a/core/modules/migrate/src/Plugin/MigrateSourceInterface.php b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
index 32d2b983bd8c..ef1785e6fb00 100644
--- a/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
+++ b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
@@ -49,9 +49,54 @@ public function __toString();
* prepareRow() or hook_migrate_prepare_row() to rewrite NULL values to
* appropriate empty values (such as '' or 0).
*
- * @return array
- * Array keyed by source field name, with values being a schema array
- * describing the field (such as ['type' => 'string]).
+ * @return array[]
+ * An associative array of field definitions keyed by field ID. Values are
+ * associative arrays with a structure that contains the field type ('type'
+ * key). The other keys are the field storage settings as they are returned
+ * by FieldStorageDefinitionInterface::getSettings(). As an example, for a
+ * composite source primary key that is defined by an integer and a
+ * string, the returned value might look like:
+ * @code
+ * return [
+ * 'id' => [
+ * 'type' => 'integer',
+ * 'unsigned' => FALSE,
+ * 'size' => 'big',
+ * ],
+ * 'version' => [
+ * 'type' => 'string',
+ * 'max_length' => 64,
+ * 'is_ascii' => TRUE,
+ * ],
+ * ];
+ * @endcode
+ * If 'type' points to a field plugin with multiple columns and needs to
+ * refer to a column different than 'value', the key of that column will be
+ * appended as a suffix to the plugin name, separated by dot ('.'). Example:
+ * @code
+ * return [
+ * 'format' => [
+ * 'type' => 'text.format',
+ * ],
+ * ];
+ * @endcode
+ * Additional custom keys/values, that are not part of field storage
+ * definition, can be passed in definitions. The most common setting, passed
+ * along the ID definition, is 'alias' used by SqlBase source plugin:
+ * @code
+ * return [
+ * 'nid' => [
+ * 'type' => 'integer',
+ * 'alias' => 'n',
+ * ],
+ * ];
+ * @endcode
+ *
+ * @see \Drupal\Core\Field\FieldStorageDefinitionInterface::getSettings()
+ * @see \Drupal\Core\Field\Plugin\Field\FieldType\IntegerItem
+ * @see \Drupal\Core\Field\Plugin\Field\FieldType\StringItem
+ * @see \Drupal\text\Plugin\Field\FieldType\TextItem
+ * @see \Drupal\migrate\Plugin\migrate\source\SqlBase
*/
public function getIds();
diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php
index d3adce883ff0..9da3de6f9057 100644
--- a/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php
+++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php
@@ -125,15 +125,13 @@ protected function isTranslationDestination() {
*/
public function getIds() {
$id_key = $this->getKey('id');
- $ids[$id_key]['type'] = 'integer';
+ $ids[$id_key] = $this->getDefinitionFromEntity($id_key);
if ($this->isTranslationDestination()) {
- if ($key = $this->getKey('langcode')) {
- $ids[$key]['type'] = 'string';
- }
- else {
+ if (!$langcode_key = $this->getKey('langcode')) {
throw new MigrateException('This entity type does not support translation.');
}
+ $ids[$langcode_key] = $this->getDefinitionFromEntity($langcode_key);
}
return $ids;
@@ -263,4 +261,32 @@ public function rollback(array $destination_identifier) {
}
}
+ /**
+ * Gets the field definition from a specific entity base field.
+ *
+ * The method takes the field ID as an argument and returns the field storage
+ * definition to be used in getIds() by querying the destination entity base
+ * field definition.
+ *
+ * @param string $key
+ * The field ID key.
+ *
+ * @return array
+ * An associative array with a structure that contains the field type, keyed
+ * as 'type', together with field storage settings as they are returned by
+ * FieldStorageDefinitionInterface::getSettings().
+ *
+ * @see \Drupal\Core\Field\FieldStorageDefinitionInterface::getSettings()
+ */
+ protected function getDefinitionFromEntity($key) {
+ $entity_type_id = static::getEntityTypeId($this->getPluginId());
+ /** @var \Drupal\Core\Field\FieldStorageDefinitionInterface[] $definitions */
+ $definitions = $this->entityManager->getBaseFieldDefinitions($entity_type_id);
+ $field_definition = $definitions[$key];
+
+ return [
+ 'type' => $field_definition->getType(),
+ ] + $field_definition->getSettings();
+ }
+
}
diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php
index 62469e730512..381974291eeb 100644
--- a/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php
+++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php
@@ -73,8 +73,7 @@ protected function save(ContentEntityInterface $entity, array $old_destination_i
*/
public function getIds() {
if ($key = $this->getKey('revision')) {
- $ids[$key]['type'] = 'integer';
- return $ids;
+ return [$key => $this->getDefinitionFromEntity($key)];
}
throw new MigrateException('This entity type does not support revisions.');
}
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 cbf4a9f46e43..962e05b2362d 100644
--- a/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
+++ b/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php
@@ -441,20 +441,40 @@ protected function ensureTables() {
* Creates schema from an ID definition.
*
* @param array $id_definition
- * A field schema definition. Can be SQL schema or a type data
- * based schema. In the latter case, the value of type needs to be
- * $typed_data_type.$column.
+ * The definition of the field having the structure as the items returned by
+ * MigrateSourceInterface or MigrateDestinationInterface::getIds().
*
* @return array
- * The schema definition.
+ * The database schema definition.
+ *
+ * @see \Drupal\migrate\Plugin\MigrateSourceInterface::getIds()
+ * @see \Drupal\migrate\Plugin\MigrateDestinationInterface::getIds()
*/
protected function getFieldSchema(array $id_definition) {
$type_parts = explode('.', $id_definition['type']);
if (count($type_parts) == 1) {
$type_parts[] = 'value';
}
- $schema = BaseFieldDefinition::create($type_parts[0])->getColumns();
- return $schema[$type_parts[1]];
+ unset($id_definition['type']);
+
+ // Get the field storage definition.
+ $definition = BaseFieldDefinition::create($type_parts[0]);
+
+ // Get a list of setting keys belonging strictly to the field definition.
+ $default_field_settings = $definition->getSettings();
+ // Separate field definition settings from custom settings. Custom settings
+ // are settings passed in $id_definition that are not part of field storage
+ // definition settings.
+ $field_settings = array_intersect_key($id_definition, $default_field_settings);
+ $custom_settings = array_diff_key($id_definition, $default_field_settings);
+
+ // Resolve schema from field storage definition settings.
+ $schema = $definition
+ ->setSettings($field_settings)
+ ->getColumns()[$type_parts[1]];
+
+ // Merge back custom settings.
+ return $schema + $custom_settings;
}
/**
diff --git a/core/modules/migrate/tests/modules/migrate_entity_test/migrate_entity_test.info.yml b/core/modules/migrate/tests/modules/migrate_entity_test/migrate_entity_test.info.yml
new file mode 100644
index 000000000000..8f2da388cf31
--- /dev/null
+++ b/core/modules/migrate/tests/modules/migrate_entity_test/migrate_entity_test.info.yml
@@ -0,0 +1,6 @@
+name: 'Migrate entity test'
+type: module
+description: 'Support module for entity destination test.'
+package: Testing
+version: VERSION
+core: 8.x
diff --git a/core/modules/migrate/tests/modules/migrate_entity_test/src/Entity/StringIdEntityTest.php b/core/modules/migrate/tests/modules/migrate_entity_test/src/Entity/StringIdEntityTest.php
new file mode 100644
index 000000000000..060e355f8971
--- /dev/null
+++ b/core/modules/migrate/tests/modules/migrate_entity_test/src/Entity/StringIdEntityTest.php
@@ -0,0 +1,36 @@
+<?php
+
+namespace Drupal\migrate_entity_test\Entity;
+
+use Drupal\Core\Entity\ContentEntityBase;
+use Drupal\Core\Entity\EntityTypeInterface;
+use Drupal\Core\Field\BaseFieldDefinition;
+
+/**
+ * Defines a content entity type that has a string ID.
+ *
+ * @ContentEntityType(
+ * id = "migrate_string_id_entity_test",
+ * label = @Translation("String id entity test"),
+ * base_table = "migrate_entity_test_string_id",
+ * entity_keys = {
+ * "id" = "id",
+ * }
+ * )
+ */
+class StringIdEntityTest extends ContentEntityBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
+ return [
+ 'id' => BaseFieldDefinition::create('integer')
+ ->setSetting('size', 'big')
+ ->setLabel('ID'),
+ 'version' => BaseFieldDefinition::create('string')
+ ->setLabel('Version'),
+ ];
+ }
+
+}
diff --git a/core/modules/migrate/tests/src/Kernel/MigrateEntityContentBaseTest.php b/core/modules/migrate/tests/src/Kernel/MigrateEntityContentBaseTest.php
index 54ded16c5756..0ebc719ec90c 100644
--- a/core/modules/migrate/tests/src/Kernel/MigrateEntityContentBaseTest.php
+++ b/core/modules/migrate/tests/src/Kernel/MigrateEntityContentBaseTest.php
@@ -4,6 +4,8 @@
use Drupal\KernelTests\KernelTestBase;
use Drupal\language\Entity\ConfigurableLanguage;
+use Drupal\migrate\MigrateExecutable;
+use Drupal\migrate\MigrateMessage;
use Drupal\migrate\Plugin\migrate\destination\EntityContentBase;
use Drupal\migrate\Plugin\MigrateIdMapInterface;
use Drupal\migrate\Plugin\MigrationInterface;
@@ -156,4 +158,50 @@ public function testTranslated() {
$this->assertTranslations(4, 'fr');
}
+ /**
+ * Tests creation of ID columns table with definitions taken from entity type.
+ */
+ public function testEntityWithStringId() {
+ $this->enableModules(['migrate_entity_test']);
+ $this->installEntitySchema('migrate_string_id_entity_test');
+
+ $definition = [
+ 'source' => [
+ 'plugin' => 'embedded_data',
+ 'data_rows' => [
+ ['id' => 123, 'version' => 'foo'],
+ // This integer needs an 'int' schema with 'big' size. If 'destid1'
+ // is not correctly taking the definition from the destination entity
+ // type, the import will fail with a SQL exception.
+ ['id' => 123456789012, 'version' => 'bar'],
+ ],
+ 'ids' => [
+ 'id' => ['type' => 'integer', 'size' => 'big'],
+ 'version' => ['type' => 'string'],
+ ],
+ ],
+ 'process' => [
+ 'id' => 'id',
+ 'version' => 'version',
+ ],
+ 'destination' => [
+ 'plugin' => 'entity:migrate_string_id_entity_test',
+ ],
+ ];
+
+ $migration = \Drupal::service('plugin.manager.migration')->createStubMigration($definition);
+ $executable = new MigrateExecutable($migration, new MigrateMessage());
+ $result = $executable->import();
+ $this->assertEquals(MigrationInterface::RESULT_COMPLETED, $result);
+
+ /** @var \Drupal\migrate\Plugin\MigrateIdMapInterface $id_map_plugin */
+ $id_map_plugin = $migration->getIdMap();
+
+ // Check that the destination has been stored.
+ $map_row = $id_map_plugin->getRowBySource(['id' => 123, 'version' => 'foo']);
+ $this->assertEquals(123, $map_row['destid1']);
+ $map_row = $id_map_plugin->getRowBySource(['id' => 123456789012, 'version' => 'bar']);
+ $this->assertEquals(123456789012, $map_row['destid1']);
+ }
+
}
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 bbd134c179a7..0757e5cc6659 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
@@ -11,6 +11,7 @@
use Drupal\Core\Entity\ContentEntityType;
use Drupal\Core\Entity\EntityManagerInterface;
use Drupal\Core\Entity\EntityStorageInterface;
+use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Field\FieldTypePluginManagerInterface;
use Drupal\migrate\Plugin\MigrationInterface;
use Drupal\migrate\Plugin\migrate\destination\EntityContentBase;
@@ -109,6 +110,8 @@ public function testUntranslatable() {
$entity_type = $this->prophesize(ContentEntityType::class);
$entity_type->getKey('langcode')->willReturn('');
$entity_type->getKey('id')->willReturn('id');
+ $this->entityManager->getBaseFieldDefinitions('foo')
+ ->willReturn(['id' => BaseFieldDefinitionTest::create('integer')]);
$this->storage->getEntityType()->willReturn($entity_type->reveal());
@@ -144,4 +147,27 @@ protected function getEntity(Row $row, array $old_destination_id_values) {
return $this->entity;
}
+ public static function getEntityTypeId() {
+ 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';
+ }
+
}
--
GitLab