Issue #2783715 by claudiu.cristea, chx, mikeryan, heddn, phenaproxima: Entity...

Issue #2783715 by claudiu.cristea, chx, mikeryan, heddn, phenaproxima: Entity destination ID schema should match entity ID definition

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();
 

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();
 

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();
+  }
+
 }

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.');
   }

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;
   }
 
   /**

core/modules/migrate/tests/modules/migrate_entity_test/migrate_entity_test.info.yml

+name: 'Migrate entity test'
+type: module
+description: 'Support module for entity destination test.'
+package: Testing
+version: VERSION
+core: 8.x

core/modules/migrate/tests/modules/migrate_entity_test/src/Entity/StringIdEntityTest.php

+<?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'),
+    ];
+  }
+
+}

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']);
+  }
+
 }

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($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';
+  }
+
 }