From eecd757072f482fe871c2af5866052e82953f9bb Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org> Date: Tue, 3 Apr 2018 12:40:04 +0100 Subject: [PATCH] Issue #2951242 by alexpott, amateescu, bmunslow, Berdir, greg606, donaldp: Allow BaseFieldDefinition::setInitialValueFromField() to set a default value - this fixes issues with block_content_update_8400() --- .../Core/Database/Driver/mysql/Schema.php | 10 ++- .../Core/Database/Driver/pgsql/Schema.php | 10 ++- .../Core/Database/Driver/sqlite/Schema.php | 22 ++++++- .../Sql/SqlContentEntityStorageSchema.php | 2 +- .../Drupal/Core/Field/BaseFieldDefinition.php | 16 ++++- .../block_content/block_content.install | 2 +- .../KernelTests/Core/Database/SchemaTest.php | 25 +++++++- .../Entity/EntityDefinitionUpdateTest.php | 63 ++++++++++++++++--- 8 files changed, 132 insertions(+), 18 deletions(-) diff --git a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php b/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php index 878607c55dc1..44b560c242cc 100644 --- a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php @@ -434,8 +434,16 @@ public function addField($table, $field, $spec, $keys_new = []) { ->execute(); } if (isset($spec['initial_from_field'])) { + if (isset($spec['initial'])) { + $expression = 'COALESCE(' . $spec['initial_from_field'] . ', :default_initial_value)'; + $arguments = [':default_initial_value' => $spec['initial']]; + } + else { + $expression = $spec['initial_from_field']; + $arguments = []; + } $this->connection->update($table) - ->expression($field, $spec['initial_from_field']) + ->expression($field, $expression, $arguments) ->execute(); } if ($fixnull) { diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php index 96dea950f034..58c658766138 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php @@ -568,8 +568,16 @@ public function addField($table, $field, $spec, $new_keys = []) { ->execute(); } if (isset($spec['initial_from_field'])) { + if (isset($spec['initial'])) { + $expression = 'COALESCE(' . $spec['initial_from_field'] . ', :default_initial_value)'; + $arguments = [':default_initial_value' => $spec['initial']]; + } + else { + $expression = $spec['initial_from_field']; + $arguments = []; + } $this->connection->update($table) - ->expression($field, $spec['initial_from_field']) + ->expression($field, $expression, $arguments) ->execute(); } if ($fixnull) { diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php index e11c6da34e49..8c0ce54dc4d9 100644 --- a/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php @@ -333,8 +333,16 @@ public function addField($table, $field, $specification, $keys_new = []) { ->execute(); } if (isset($specification['initial_from_field'])) { + if (isset($specification['initial'])) { + $expression = 'COALESCE(' . $specification['initial_from_field'] . ', :default_initial_value)'; + $arguments = [':default_initial_value' => $specification['initial']]; + } + else { + $expression = $specification['initial_from_field']; + $arguments = []; + } $this->connection->update($table) - ->expression($field, $specification['initial_from_field']) + ->expression($field, $expression, $arguments) ->execute(); } } @@ -358,9 +366,17 @@ public function addField($table, $field, $specification, $keys_new = []) { } elseif (isset($specification['initial_from_field'])) { // If we have a initial value, copy it over. + if (isset($specification['initial'])) { + $expression = 'COALESCE(' . $specification['initial_from_field'] . ', :default_initial_value)'; + $arguments = [':default_initial_value' => $specification['initial']]; + } + else { + $expression = $specification['initial_from_field']; + $arguments = []; + } $mapping[$field] = [ - 'expression' => $specification['initial_from_field'], - 'arguments' => [], + 'expression' => $expression, + 'arguments' => $arguments, ]; } else { diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php index 3349ebd30da4..9c123417cad5 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php @@ -1881,7 +1881,7 @@ protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $st if ($initial_value && isset($initial_value[$field_column_name])) { $schema['fields'][$schema_field_name]['initial'] = drupal_schema_get_field_value($column_schema, $initial_value[$field_column_name]); } - elseif (!empty($initial_value_from_field)) { + if (!empty($initial_value_from_field)) { $schema['fields'][$schema_field_name]['initial_from_field'] = $initial_value_from_field[$field_column_name]; } } diff --git a/core/lib/Drupal/Core/Field/BaseFieldDefinition.php b/core/lib/Drupal/Core/Field/BaseFieldDefinition.php index 4a8e4fa07deb..2aa402ffa034 100644 --- a/core/lib/Drupal/Core/Field/BaseFieldDefinition.php +++ b/core/lib/Drupal/Core/Field/BaseFieldDefinition.php @@ -588,12 +588,24 @@ public function getInitialValueFromField() { * * @param string $field_name * The name of the field that will be used for getting initial values. + * @param mixed $default_value + * (optional) The default value for the field, in case the inherited value + * is NULL. This can be either: + * - a literal, in which case it will be assigned to the first property of + * the first item; + * - a numerically indexed array of items, each item being a property/value + * array; + * - a non-numerically indexed array, in which case the array is assumed to + * be a property/value array and used as the first item; + * - an empty array for no initial value. + * If the field being added is required or an entity key, it is recommended + * to provide a default value. * * @return $this */ - public function setInitialValueFromField($field_name) { + public function setInitialValueFromField($field_name, $default_value = NULL) { $this->definition['initial_value_from_field'] = $field_name; - + $this->setInitialValue($default_value); return $this; } diff --git a/core/modules/block_content/block_content.install b/core/modules/block_content/block_content.install index c6556e719c27..e3da6bde881c 100644 --- a/core/modules/block_content/block_content.install +++ b/core/modules/block_content/block_content.install @@ -112,7 +112,7 @@ function block_content_update_8400() { $has_content_translation_status_field = \Drupal::moduleHandler()->moduleExists('content_translation') && $definition_update_manager->getFieldStorageDefinition('content_translation_status', 'block_content'); if ($has_content_translation_status_field) { - $status->setInitialValueFromField('content_translation_status'); + $status->setInitialValueFromField('content_translation_status', TRUE); } else { $status->setInitialValue(TRUE); diff --git a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php index e23909871580..60c3754ffe39 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php @@ -528,6 +528,11 @@ public function testSchemaAddField() { ['not null' => TRUE, 'initial' => 1], ['not null' => TRUE, 'initial' => 1, 'default' => 7], ['not null' => TRUE, 'initial_from_field' => 'serial_column'], + [ + 'not null' => TRUE, + 'initial_from_field' => 'test_nullable_field', + 'initial' => 100, + ], ]; foreach ($variations as $variation) { @@ -581,6 +586,7 @@ protected function assertFieldAdditionRemoval($field_spec) { $table_spec = [ 'fields' => [ 'serial_column' => ['type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE], + 'test_nullable_field' => ['type' => 'int', 'not null' => FALSE], 'test_field' => $field_spec, ], 'primary key' => ['serial_column'], @@ -599,6 +605,7 @@ protected function assertFieldAdditionRemoval($field_spec) { $table_spec = [ 'fields' => [ 'serial_column' => ['type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE], + 'test_nullable_field' => ['type' => 'int', 'not null' => FALSE], ], 'primary key' => ['serial_column'], ]; @@ -609,9 +616,15 @@ protected function assertFieldAdditionRemoval($field_spec) { for ($i = 0; $i < 3; $i++) { db_insert($table_name) ->useDefaults(['serial_column']) + ->fields(['test_nullable_field' => 100]) ->execute(); } + // Add another row with no value for the 'test_nullable_field' column. + db_insert($table_name) + ->useDefaults(['serial_column']) + ->execute(); + db_add_field($table_name, 'test_field', $field_spec); $this->pass(format_string('Column %column created.', ['%column' => 'test_field'])); @@ -645,7 +658,7 @@ protected function assertFieldCharacteristics($table_name, $field_name, $field_s } // Check that the initial value from another field has been registered. - if (isset($field_spec['initial_from_field'])) { + if (isset($field_spec['initial_from_field']) && !isset($field_spec['initial'])) { // There should be no row with a value different than // $field_spec['initial_from_field']. $count = db_select($table_name) @@ -656,6 +669,16 @@ protected function assertFieldCharacteristics($table_name, $field_name, $field_s ->fetchField(); $this->assertEqual($count, 0, 'Initial values from another field filled out.'); } + elseif (isset($field_spec['initial_from_field']) && isset($field_spec['initial'])) { + // There should be no row with a value different than '100'. + $count = db_select($table_name) + ->fields($table_name, ['serial_column']) + ->condition($field_name, 100, '<>') + ->countQuery() + ->execute() + ->fetchField(); + $this->assertEqual($count, 0, 'Initial values from another field or a default value filled out.'); + } // Check that the default value has been registered. if (isset($field_spec['default'])) { diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php index 348c33cbb83d..ff3cee571261 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php @@ -1049,31 +1049,78 @@ public function testInitialValue() { /** * Tests adding a base field with initial values inherited from another field. + * + * @dataProvider initialValueFromFieldTestCases */ - public function testInitialValueFromField() { + public function testInitialValueFromField($default_initial_value, $expected_value) { $storage = \Drupal::entityTypeManager()->getStorage('entity_test_update'); $db_schema = $this->database->schema(); // Create two entities before adding the base field. - /** @var \Drupal\entity_test\Entity\EntityTestUpdate $entity */ - $storage->create(['name' => 'First entity'])->save(); - $storage->create(['name' => 'Second entity'])->save(); + /** @var \Drupal\entity_test_update\Entity\EntityTestUpdate $entity */ + $storage->create([ + 'name' => 'First entity', + 'test_single_property' => 'test existing value', + ])->save(); + + // The second entity does not have any value for the 'test_single_property' + // field, allowing us to test the 'default_value' parameter of + // \Drupal\Core\Field\BaseFieldDefinition::setInitialValueFromField(). + $storage->create([ + 'name' => 'Second entity' + ])->save(); // Add a base field with an initial value inherited from another field. - $this->addBaseField(); - $storage_definition = BaseFieldDefinition::create('string') - ->setLabel(t('A new base field')) + $definitions['new_base_field'] = BaseFieldDefinition::create('string') + ->setName('new_base_field') + ->setLabel('A new base field') ->setInitialValueFromField('name'); + $definitions['another_base_field'] = BaseFieldDefinition::create('string') + ->setName('another_base_field') + ->setLabel('Another base field') + ->setInitialValueFromField('test_single_property', $default_initial_value); + + $this->state->set('entity_test_update.additional_base_field_definitions', $definitions); $this->assertFalse($db_schema->fieldExists('entity_test_update', 'new_base_field'), "New field 'new_base_field' does not exist before applying the update."); - $this->entityDefinitionUpdateManager->installFieldStorageDefinition('new_base_field', 'entity_test_update', 'entity_test', $storage_definition); + $this->assertFalse($db_schema->fieldExists('entity_test_update', 'another_base_field'), "New field 'another_base_field' does not exist before applying the update."); + $this->entityDefinitionUpdateManager->installFieldStorageDefinition('new_base_field', 'entity_test_update', 'entity_test', $definitions['new_base_field']); + $this->entityDefinitionUpdateManager->installFieldStorageDefinition('another_base_field', 'entity_test_update', 'entity_test', $definitions['another_base_field']); $this->assertTrue($db_schema->fieldExists('entity_test_update', 'new_base_field'), "New field 'new_base_field' has been created on the 'entity_test_update' table."); + $this->assertTrue($db_schema->fieldExists('entity_test_update', 'another_base_field'), "New field 'another_base_field' has been created on the 'entity_test_update' table."); // Check that the initial values have been applied. $storage = \Drupal::entityTypeManager()->getStorage('entity_test_update'); $entities = $storage->loadMultiple(); $this->assertEquals('First entity', $entities[1]->get('new_base_field')->value); $this->assertEquals('Second entity', $entities[2]->get('new_base_field')->value); + + $this->assertEquals('test existing value', $entities[1]->get('another_base_field')->value); + $this->assertEquals($expected_value, $entities[2]->get('another_base_field')->value); + } + + /** + * Test cases for ::testInitialValueFromField. + */ + public function initialValueFromFieldTestCases() { + return [ + 'literal value' => [ + 'test initial value', + 'test initial value', + ], + 'indexed array' => [ + ['value' => 'test initial value'], + 'test initial value', + ], + 'empty array' => [ + [], + NULL, + ], + 'null' => [ + NULL, + NULL, + ], + ]; } /** -- GitLab