diff --git a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php b/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php index c952630..c673009 100644 --- a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php @@ -395,14 +395,22 @@ public function addField($table, $field, $spec, $keys_new = array()) { throw new SchemaObjectExistsException(t("Cannot add field @table.@field: field already exists.", array('@field' => $field, '@table' => $table))); } + // Fields that are part of a PRIMARY KEY can not be added as NOT NULL. + $is_primary_key = isset($keys_new['primary key']) && in_array($field, $keys_new['primary key'], TRUE); + $fixnull = FALSE; - if (!empty($spec['not null']) && !isset($spec['default'])) { + if (!empty($spec['not null']) && !isset($spec['default']) && !$is_primary_key) { $fixnull = TRUE; $spec['not null'] = FALSE; } $query = 'ALTER TABLE {' . $table . '} ADD '; $query .= $this->createFieldSql($field, $this->processField($spec)); if ($keys_sql = $this->createKeysSql($keys_new)) { + // Make sure to drop the existing primary key before adding a new one. + if (isset($keys_new['primary key']) && $this->indexExists($table, 'PRIMARY')) { + $query .= ', DROP PRIMARY KEY'; + } + $query .= ', ADD ' . implode(', ADD ', $keys_sql); } $this->connection->query($query); diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php index 4a5f404..ac9c2e5 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php @@ -543,8 +543,11 @@ public function addField($table, $field, $spec, $new_keys = array()) { throw new SchemaObjectExistsException(t("Cannot add field @table.@field: field already exists.", array('@field' => $field, '@table' => $table))); } + // Fields that are part of a PRIMARY KEY can not be added as NOT NULL. + $is_primary_key = isset($keys_new['primary key']) && in_array($field, $keys_new['primary key'], TRUE); + $fixnull = FALSE; - if (!empty($spec['not null']) && !isset($spec['default'])) { + if (!empty($spec['not null']) && !isset($spec['default']) && !$is_primary_key) { $fixnull = TRUE; $spec['not null'] = FALSE; } diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php index 96cea9f..4e5555c 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php @@ -1018,9 +1018,7 @@ protected function addTableDefaults(&$schema) { * @return array * A partial schema array for the base table. */ - protected function processBaseTable(ContentEntityTypeInterface $entity_type, array &$schema) { - $this->processIdentifierSchema($schema, $entity_type->getKey('id')); - } + protected function processBaseTable(ContentEntityTypeInterface $entity_type, array &$schema) { } /** * Processes the gathered schema for a base table. @@ -1033,9 +1031,7 @@ protected function processBaseTable(ContentEntityTypeInterface $entity_type, arr * @return array * A partial schema array for the base table. */ - protected function processRevisionTable(ContentEntityTypeInterface $entity_type, array &$schema) { - $this->processIdentifierSchema($schema, $entity_type->getKey('revision')); - } + protected function processRevisionTable(ContentEntityTypeInterface $entity_type, array &$schema) { } /** * Processes the gathered schema for a base table. @@ -1157,10 +1153,19 @@ protected function createSharedTableSchema(FieldStorageDefinitionInterface $stor $schema[$table_name] = $this->getSharedTableFieldSchema($storage_definition, $table_name, $column_names); if (!$only_save) { foreach ($schema[$table_name]['fields'] as $name => $specifier) { + // Check if the field is part of the primary keys and pass along + // this information when adding the field. + // @see \Drupal\Core\Database\Schema::addField() + $keys_new = []; + $entity_schema = $this->getEntitySchema($this->entityType); + if (isset($entity_schema[$table_name]['primary key']) && in_array($created_field_name, $entity_schema[$table_name]['primary key'], TRUE)) { + $keys_new = ['primary key' => $entity_schema[$table_name]['primary key']]; + } + // Check if the field exists because it might already have been // created as part of the earlier entity type update event. if (!$schema_handler->fieldExists($table_name, $name)) { - $schema_handler->addField($table_name, $name, $specifier); + $schema_handler->addField($table_name, $name, $specifier, $keys_new); } } if (!empty($schema[$table_name]['indexes'])) { @@ -1650,6 +1655,15 @@ protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $st $schema['foreign keys'] = $this->getFieldForeignKeys($field_name, $field_schema, $column_mapping); } + // Process the 'id' and 'revision' entity keys for the base and revision + // tables. + if ($field_name === $this->entityType->getKey('id') && $table_name === $this->storage->getBaseTable()) { + $this->processIdentifierSchema($schema, $this->entityType->getKey('id')); + } + if ($field_name === $this->entityType->getKey('revision') && $table_name === $this->storage->getRevisionTable()) { + $this->processIdentifierSchema($schema, $this->entityType->getKey('revision')); + } + return $schema; } @@ -1977,6 +1991,15 @@ protected function hasColumnChanges(FieldStorageDefinitionInterface $storage_def $definition_spec = array_intersect_key($definition_schema[$table]['fields'][$name], $keys); $stored_spec = array_intersect_key($spec, $keys); if ($definition_spec != $stored_spec) { + // Prior to Drupal 8.3.0, the last installed schema for the 'id' and + // 'revision' fields of all content entity types was storing an + // 'int' type instead of 'serial', so we need to explicitly allow + // this conversion. + // @see system_update_8203() + // @see https://www.drupal.org/node/2841291 + if ($definition_spec['type'] == 'serial' && $stored_spec['type'] == 'int') { + return FALSE; + } return TRUE; } } diff --git a/core/modules/system/src/Tests/Update/EntityUpdateTest.php b/core/modules/system/src/Tests/Update/EntityUpdateTest.php new file mode 100644 index 0000000..8a10b3f --- /dev/null +++ b/core/modules/system/src/Tests/Update/EntityUpdateTest.php @@ -0,0 +1,62 @@ +databaseDumpFiles = [ + __DIR__ . '/../../../../system/tests/fixtures/update/drupal-8.bare.standard.php.gz', + ]; + } + + /** + * Tests system_update_8203(). + */ + public function testSystemUpdate8203() { + // The installed field storage schema is wrong before running the update. + $key_value_store = \Drupal::keyValue('entity.storage_schema.sql'); + $id_schema = $key_value_store->get('node.field_schema_data.nid', []); + $revision_id_schema = $key_value_store->get('node.field_schema_data.vid', []); + + $this->assertEqual('int', $id_schema['node']['fields']['nid']['type']); + $this->assertEqual('int', $id_schema['node_revision']['fields']['nid']['type']); + $this->assertEqual('int', $revision_id_schema['node']['fields']['vid']['type']); + $this->assertEqual('int', $revision_id_schema['node_revision']['fields']['vid']['type']); + + $this->runUpdates(); + + // Now check that the schema has been corrected. + $id_schema = $key_value_store->get('node.field_schema_data.nid', []); + $revision_id_schema = $key_value_store->get('node.field_schema_data.vid', []); + + $this->assertEqual('serial', $id_schema['node']['fields']['nid']['type']); + $this->assertEqual('int', $id_schema['node_revision']['fields']['nid']['type']); + $this->assertEqual('int', $revision_id_schema['node']['fields']['vid']['type']); + $this->assertEqual('serial', $revision_id_schema['node_revision']['fields']['vid']['type']); + + // Check that creating and loading a node still works as expected. + $node_storage = \Drupal::entityTypeManager()->getStorage('node'); + $node = $node_storage->create([ + 'title' => 'Test update', + 'type' => 'article', + ]); + $node->save(); + + $node_storage->resetCache(); + + $node = $node_storage->load($node->id()); + $this->assertEqual('Test update', $node->label()); + } + +} diff --git a/core/modules/system/system.install b/core/modules/system/system.install index f1fcfba..f3e56ca 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -13,6 +13,7 @@ use Drupal\Core\Url; use Drupal\Core\Database\Database; use Drupal\Core\DrupalKernel; +use Drupal\Core\Entity\Sql\SqlEntityStorageInterface; use Drupal\Core\Site\Settings; use Drupal\Core\StreamWrapper\PrivateStream; use Drupal\Core\StreamWrapper\PublicStream; @@ -1786,6 +1787,41 @@ function system_update_8202() { */ /** + * @addtogroup updates-8.2.6 + * @{ + */ + +/** + * Correct the last installed storage schema for every entity type's ID and + * revision fields. + */ +function system_update_8203() { + $entity_type_manager = \Drupal::entityTypeManager(); + $definition_update_manager = \Drupal::entityDefinitionUpdateManager(); + /** @var \Drupal\Core\Entity\EntityLastInstalledSchemaRepositoryInterface $entity_last_installed_schema_respository */ + $entity_last_installed_schema_respository = \Drupal::service('entity.last_installed_schema.repository'); + + foreach ($entity_type_manager->getDefinitions() as $entity_type_id => $entity_type) { + $storage = $entity_type_manager->getStorage($entity_type_id); + if ($storage instanceof SqlEntityStorageInterface) { + $field_names_to_update = [$entity_type->getKey('id')]; + if ($entity_type->isRevisionable()) { + $field_names_to_update[] = $entity_type->getKey('revision'); + } + foreach ($entity_last_installed_schema_respository->getLastInstalledFieldStorageDefinitions($entity_type_id) as $field_name => $field_definition) { + if (in_array($field_definition->getName(), $field_names_to_update)) { + $definition_update_manager->updateFieldStorageDefinition($field_definition); + } + } + } + } +} + +/** + * @} End of "addtogroup updates-8.2.6". + */ + +/** * @addtogroup updates-8.3.0 * @{ */ diff --git a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php index 8641fd3..1583e5c 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php @@ -143,6 +143,24 @@ function testSchema() { $count = db_query('SELECT COUNT(*) FROM {test_table}')->fetchField(); $this->assertEqual($count, 2, 'There were two rows.'); + // Test adding a serial field to an existing table. + db_drop_table('test_table'); + db_create_table('test_table', $table_specification); + db_field_set_default('test_table', 'test_field', 0); + db_add_field('test_table', 'test_serial', ['type' => 'serial', 'not null' => TRUE], ['primary key' => ['test_serial']]); + + $this->assertTrue($this->tryInsert(), 'Insert with a serial succeeded.'); + $max1 = db_query('SELECT MAX(test_serial) FROM {test_table}')->fetchField(); + $this->assertTrue($this->tryInsert(), 'Insert with a serial succeeded.'); + $max2 = db_query('SELECT MAX(test_serial) FROM {test_table}')->fetchField(); + $this->assertTrue($max2 > $max1, 'The serial is monotone.'); + + $count = db_query('SELECT COUNT(*) FROM {test_table}')->fetchField(); + $this->assertEqual($count, 2, 'There were two rows.'); + + // Test adding a new column and form a composite primary key with it. + db_add_field('test_table', 'test_composite_primary_key', ['type' => 'int', 'not null' => TRUE], ['primary key' => ['test_serial', 'test_composite_primary_key']]); + // Test renaming of keys and constraints. db_drop_table('test_table'); $table_specification = array( diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php index e49ee7e..e716b70 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php @@ -810,4 +810,51 @@ public function testLongNameFieldIndexes() { $this->assertFalse($this->entityDefinitionUpdateManager->needsUpdates(), 'Entity and field schema data are correctly detected.'); } + /** + * Checks that updating the main identifier field ('id') is correctly handled. + */ + public function testUpdateIdentifierField() { + // Get the storage definition for an entity identifier, the 'id' field. + $storage_definition = $this->entityDefinitionUpdateManager->getFieldStorageDefinition('id', 'entity_test_update'); + + // Update the storage definition without any changes. + $this->entityDefinitionUpdateManager->updateFieldStorageDefinition($storage_definition); + + // Check that creating, saving and loading an entity still works. + $storage = $this->entityManager->getStorage('entity_test_update'); + + $entity = $storage->create(); + $entity->save(); + + $storage->resetCache(); + $this->assertNotNull($storage->load($entity->id())); + } + + /** + * Check that updating the revision identifier field is correctly handled. + */ + public function testUpdateRevisionIdentifierField() { + $this->updateEntityTypeToRevisionable(); + $this->entityDefinitionUpdateManager->applyUpdates(); + + // Get the storage definition for the revision identifier, the 'revision_id' + // field. + $storage_definition = $this->entityDefinitionUpdateManager->getFieldStorageDefinition('revision_id', 'entity_test_update'); + + // Update the storage definition without any changes. + $this->entityDefinitionUpdateManager->updateFieldStorageDefinition($storage_definition); + + // Check that creating and saving an entity still works. + $this->entityManager->getStorage('entity_test_update')->create()->save(); + + // Check that creating, saving and loading an entity still works. + $storage = $this->entityManager->getStorage('entity_test_update'); + + $entity = $storage->create(); + $entity->save(); + + $storage->resetCache(); + $this->assertNotNull($storage->loadRevision($entity->getRevisionId())); + } + } diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php index fdf72df..e01d912 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php @@ -5,9 +5,9 @@ use Drupal\Component\Utility\SafeMarkup; /** - * Tests adding a custom bundle field. + * Tests the default entity storage schema handler. * - * @group system + * @group Entity */ class EntitySchemaTest extends EntityKernelTestBase { @@ -188,4 +188,63 @@ public function testCleanUpStorageDefinition() { $this->assertEqual($entity_type_id_count, 0, 'After uninstalling entity_test module the schema should not contains fields from entities provided by the module.'); } + /** + * Tests the installed storage schema for identifier fields. + */ + public function testIdentifierSchema() { + $this->installEntitySchema('entity_test_rev'); + + $key_value_store = \Drupal::keyValue('entity.storage_schema.sql'); + $id_schema = $key_value_store->get('entity_test_rev.field_schema_data.id', []); + $revision_id_schema = $key_value_store->get('entity_test_rev.field_schema_data.revision_id', []); + + $expected_id_schema = [ + 'entity_test_rev' => [ + 'fields' => [ + 'id' => [ + 'type' => 'serial', + 'unsigned' => TRUE, + 'size' => 'normal', + 'not null' => TRUE, + ], + ], + ], + 'entity_test_rev_revision' => [ + 'fields' => [ + 'id' => [ + 'type' => 'int', + 'unsigned' => TRUE, + 'size' => 'normal', + 'not null' => TRUE, + ], + ], + ], + ]; + $this->assertEquals($expected_id_schema, $id_schema); + + $expected_revision_id_schema = [ + 'entity_test_rev' => [ + 'fields' => [ + 'revision_id' => [ + 'type' => 'int', + 'unsigned' => TRUE, + 'size' => 'normal', + 'not null' => FALSE, + ], + ], + ], + 'entity_test_rev_revision' => [ + 'fields' => [ + 'revision_id' => [ + 'type' => 'serial', + 'unsigned' => TRUE, + 'size' => 'normal', + 'not null' => TRUE, + ], + ], + ], + ]; + $this->assertEquals($expected_revision_id_schema, $revision_id_schema); + } + } diff --git a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php index 1ce87c5..b0f2948 100644 --- a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php @@ -397,7 +397,7 @@ public function testGetSchemaRevisionable() { ), )); - $this->storage->expects($this->exactly(2)) + $this->storage->expects($this->atLeastOnce()) ->method('getRevisionTable') ->will($this->returnValue('entity_test_revision')); @@ -604,7 +604,7 @@ public function testGetSchemaRevisionableTranslatable() { ), )); - $this->storage->expects($this->exactly(3)) + $this->storage->expects($this->atLeastOnce()) ->method('getRevisionTable') ->will($this->returnValue('entity_test_revision')); $this->storage->expects($this->once())