Commit 83923134 authored by catch's avatar catch

Issue #2609590 by Lars Toomre: Correct incorrect use of 'id' string in migration system

parent da77debc
......@@ -173,13 +173,13 @@ public function buildDependencyMigration(array $migrations, array $dynamic_ids)
* Add one or more dependencies to a graph.
*
* @param array $graph
* The graph so far.
* The graph so far, passed by reference.
* @param int $id
* The migration id.
* The migration ID.
* @param string $dependency
* The dependency string.
* @param array $dynamic_ids
* The dynamic id mapping.
* The dynamic ID mapping.
*/
protected function addDependency(array &$graph, $id, $dependency, $dynamic_ids) {
$dependencies = isset($dynamic_ids[$dependency]) ? $dynamic_ids[$dependency] : array($dependency);
......
......@@ -25,7 +25,7 @@
interface MigrateDestinationInterface extends PluginInspectionInterface {
/**
* Get the destination ids.
* Get the destination IDs.
*
* To support MigrateIdMap maps, derived destination classes should return
* schema field definition(s) corresponding to the primary key of the
......@@ -33,7 +33,7 @@ interface MigrateDestinationInterface extends PluginInspectionInterface {
* key fields of the map table for a migration using this destination.
*
* @return array
* An array of ids.
* An array of IDs.
*/
public function getIds();
......@@ -47,7 +47,7 @@ public function getIds();
* can we avoid that?
*
* @param \Drupal\migrate\Entity\MigrationInterface $migration
* (optional) the migration containing this destination.
* (optional) The migration containing this destination.
*
* @return array
* - Keys: machine names of the fields
......@@ -64,10 +64,10 @@ public function fields(MigrationInterface $migration = NULL);
* @param \Drupal\migrate\Row $row
* The row object.
* @param array $old_destination_id_values
* The old destination ids.
* (optional) The old destination IDs. Defaults to an empty array.
*
* @return mixed
* The entity id or an indication of success.
* The entity ID or an indication of success.
*/
public function import(Row $row, array $old_destination_id_values = array());
......@@ -95,4 +95,5 @@ public function supportsRollback();
* item should be handled on rollback.
*/
public function rollbackAction();
}
......@@ -77,10 +77,10 @@ public static function create(ContainerInterface $container, array $configuratio
}
/**
* Finds the entity type from configuration or plugin id.
* Finds the entity type from configuration or plugin ID.
*
* @param string $plugin_id
* The plugin id.
* The plugin ID.
*
* @return string
* The entity type.
......@@ -103,10 +103,10 @@ public function fields(MigrationInterface $migration = NULL) {
* @param \Drupal\migrate\Row $row
* The row object.
* @param array $old_destination_id_values
* The old destination ids.
* The old destination IDs.
*
* @return \Drupal\Core\Entity\EntityInterface
* The entity we're importing into.
* The entity we are importing into.
*/
protected function getEntity(Row $row, array $old_destination_id_values) {
$entity_id = $old_destination_id_values ? reset($old_destination_id_values) : $this->getEntityId($row);
......@@ -125,12 +125,13 @@ protected function getEntity(Row $row, array $old_destination_id_values) {
}
/**
* Get the entity id of the row.
* Get the entity ID of the row.
*
* @param \Drupal\migrate\Row $row
* The row of data.
*
* @return string
* The entity id for the row we're importing.
* The entity ID for the row we are importing.
*/
protected function getEntityId(Row $row) {
return $row->getDestinationProperty($this->getKey('id'));
......
......@@ -18,7 +18,7 @@
*
* This class serves as the import class for most configuration entities.
* It can be necessary to provide a specific entity class if the configuration
* entity has a compound id (see EntityFieldEntity) or it has specific setter
* entity has a compound ID (see EntityFieldEntity) or it has specific setter
* methods (see EntityDateFormat). When implementing an entity destination for
* the latter case, make sure to add a test not only for importing but also
* for re-importing (if that is supported).
......@@ -39,14 +39,14 @@ public function import(Row $row, array $old_destination_id_values = array()) {
// Ids is keyed by the key name so grab the keys.
$id_keys = array_keys($ids);
if (!$row->getDestinationProperty($id_key)) {
// Set the id into the destination in for form "val1.val2.val3".
// Set the ID into the destination in for form "val1.val2.val3".
$row->setDestinationProperty($id_key, $this->generateId($row, $id_keys));
}
}
$entity = $this->getEntity($row, $old_destination_id_values);
$entity->save();
if (count($ids) > 1) {
// This can only be a config entity, content entities have their id key
// This can only be a config entity, content entities have their ID key
// and that's it.
$return = array();
foreach ($id_keys as $id_key) {
......@@ -105,15 +105,15 @@ protected function updateEntityProperty(EntityInterface $entity, array $parents,
}
/**
* Generate an entity id.
* Generate an entity ID.
*
* @param \Drupal\migrate\Row $row
* The current row.
* @param array $ids
* The destination ids.
* The destination IDs.
*
* @return string
* The generated entity id.
* The generated entity ID.
*/
protected function generateId(Row $row, array $ids) {
$id_values = array();
......
......@@ -102,10 +102,10 @@ public function import(Row $row, array $old_destination_id_values = array()) {
* @param \Drupal\Core\Entity\ContentEntityInterface $entity
* The content entity.
* @param array $old_destination_id_values
* An array of destination id values.
* (optional) An array of destination ID values. Defaults to an empty array.
*
* @return array
* An array containing the entity id.
* An array containing the entity ID.
*/
protected function save(ContentEntityInterface $entity, array $old_destination_id_values = array()) {
$entity->save();
......
......@@ -300,7 +300,7 @@ protected function ensureTables() {
$fields = $source_id_schema;
// Add destination identifiers to map table.
// TODO: How do we discover the destination schema?
// @todo How do we discover the destination schema?
$count = 1;
foreach ($this->migration->getDestinationPlugin()->getIds() as $id_definition) {
// Allow dest identifier fields to be NULL (for IGNORED/FAILED
......@@ -405,12 +405,13 @@ protected function ensureTables() {
}
/**
* Create schema from an id definition.
* 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
*
* @return array
*/
protected function getFieldSchema(array $id_definition) {
......
......@@ -115,7 +115,7 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
// Only keep the process necessary to produce the destination ID.
$process = $migration->get('process');
// We already have the source id values but need to key them for the Row
// We already have the source ID values but need to key them for the Row
// constructor.
$source_ids = $migration->getSourcePlugin()->getIds();
$values = array();
......
......@@ -51,7 +51,7 @@ public function setUp() {
$this->installConfig(['system']);
// A simple migration, which will generate a message to the id map because
// A simple migration, which will generate a message to the ID map because
// the concat plugin throws an exception if its source is not an array.
$config = [
'id' => 'sample_data',
......
......@@ -123,9 +123,9 @@ private function cleanupMigrateConnection() {
* Prepare any dependent migrations.
*
* @param array $id_mappings
* A list of id mappings keyed by migration ids. Each id mapping is a list
* of two arrays, the first are source ids and the second are destination
* ids.
* A list of ID mappings keyed by migration IDs. Each ID mapping is a list
* of two arrays, the first are source IDs and the second are destination
* IDs.
*/
protected function prepareMigrations(array $id_mappings) {
foreach ($id_mappings as $migration_id => $data) {
......
......@@ -56,7 +56,7 @@ public function setUp() {
* Saves a single ID mapping row in the database.
*
* @param array $map
* The row to save.
* The row to save.
*/
protected function saveMap(array $map) {
$table = 'migrate_map_sql_idmap_test';
......@@ -114,6 +114,12 @@ protected function getIdMap() {
/**
* Sets defaults for SQL ID map plugin tests.
*
* @return array
* An associative array with the following keys:
* - source_row_status
* - rollback_action
* - hash
*/
protected function idMapDefaults() {
$defaults = array(
......@@ -731,7 +737,7 @@ public function testPrepareUpdate() {
*/
public function testDestroy() {
$id_map = $this->getIdMap();
// Initialize the id map.
// Initialize the ID map.
$id_map->getDatabase();
$map_table_name = $id_map->mapTableName();
$message_table_name = $id_map->messageTableName();
......
......@@ -23,11 +23,13 @@ class SqlBaseTest extends UnitTestCase {
* @param bool $id_map_is_sql
* TRUE if we want getIdMap() to return an instance of Sql.
* @param bool $with_id_map
* TRUE if we want the id map to have a valid map of ids.
* TRUE if we want the ID map to have a valid map of IDs.
* @param array $source_options
* An array of connection options for the source connection.
* (optional) An array of connection options for the source connection.
* Defaults to an empty array.
* @param array $idmap_options
* An array of connection options for the id map connection.
* (optional) An array of connection options for the ID map connection.
* Defaults to an empty array.
*
* @dataProvider sqlBaseTestProvider
*/
......@@ -40,7 +42,7 @@ public function testMapJoinable($expected_result, $id_map_is_sql, $with_id_map,
->method('getConnectionOptions')
->willReturn($source_options);
// Setup the id map connection.
// Setup the ID map connection.
$idmap_connection = $this->getMockBuilder('Drupal\Core\Database\Connection')
->disableOriginalConstructor()
->getMock();
......@@ -85,14 +87,14 @@ public function testMapJoinable($expected_result, $id_map_is_sql, $with_id_map,
*/
public function sqlBaseTestProvider() {
return [
// Source ids are empty so mapJoinable() is false.
// Source IDs are empty so mapJoinable() is false.
[FALSE, FALSE, FALSE],
// Still false because getIdMap() is not a subclass of Sql.
[FALSE, FALSE, TRUE],
// Test mapJoinable() returns false when source and id connection options
// Test mapJoinable() returns false when source and ID connection options
// differ.
[FALSE, TRUE, TRUE, ['username' => 'different_from_map', 'password' => 'different_from_map'], ['username' => 'different_from_source', 'password' => 'different_from_source']],
// Returns true because source and id map connection options are the same.
// Returns true because source and ID map connection options are the same.
[TRUE, TRUE, TRUE, ['username' => 'same_value', 'password' => 'same_value'], ['username' => 'same_value', 'password' => 'same_value']],
];
}
......@@ -151,7 +153,10 @@ public function getIds() {
}
/**
* Allows us to set the ids during a test.
* Allows us to set the IDs during a test.
*
* @param array $ids
* An array of identifiers.
*/
public function setIds($ids) {
$this->ids = $ids;
......
......@@ -54,7 +54,7 @@ public function processFieldWidget(MigrationInterface $migration) {
* {@inheritdoc}
*/
public function getFieldWidgetMap() {
// By default use the plugin id for the widget types.
// By default, use the plugin ID for the widget types.
return [
$this->pluginId => $this->pluginId . '_default',
];
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment