diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php index 3349ebd30d..cdb9ae43a6 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php @@ -1358,11 +1358,24 @@ protected function createSharedTableSchema(FieldStorageDefinitionInterface $stor // Create field columns. $schema[$table_name] = $this->getSharedTableFieldSchema($storage_definition, $table_name, $column_names); if (!$only_save) { + // The entity schema needs to be checked because the field schema is + // potentially incomplete. + // @todo Remove this in + // https://www.drupal.org/project/drupal/issues/2929120 + $entity_schema = $this->getEntitySchema($this->entityType); 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() + $new_keys = []; + if (isset($entity_schema[$table_name]['primary key']) && array_intersect($column_names, $entity_schema[$table_name]['primary key'])) { + $new_keys = ['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, $new_keys); } } if (!empty($schema[$table_name]['indexes'])) { @@ -1439,7 +1452,9 @@ protected function deleteSharedTableSchema(FieldStorageDefinitionInterface $stor if ($field_name == $deleted_field_name) { $schema = $this->getSharedTableFieldSchema($storage_definition, $table_name, $column_names); - // Drop indexes and unique keys first. + // Drop the primary key, indexes and unique keys first. + $this->dropPrimaryKey($table_name, $column_names); + if (!empty($schema['indexes'])) { foreach ($schema['indexes'] as $name => $specifier) { $schema_handler->dropIndex($table_name, $name); @@ -1625,7 +1640,9 @@ protected function updateSharedTableSchema(FieldStorageDefinitionInterface $stor } } - // Drop original indexes and unique keys. + // Drop the original primary key, indexes and unique keys first. + $this->dropPrimaryKey($table_name, $column_names); + if (!empty($original_schema[$table_name]['indexes'])) { foreach ($original_schema[$table_name]['indexes'] as $name => $specifier) { $schema_handler->dropIndex($table_name, $name); @@ -1636,7 +1653,19 @@ protected function updateSharedTableSchema(FieldStorageDefinitionInterface $stor $schema_handler->dropUniqueKey($table_name, $name); } } - // Create new indexes and unique keys. + // Create new primary key, indexes and unique keys. + // The entity schema needs to be checked because the field schema is + // potentially incomplete. + // @todo Remove this in + // https://www.drupal.org/project/drupal/issues/2929120 + $entity_schema = $this->getEntitySchema($this->entityType); + foreach ($schema[$table_name]['fields'] as $name => $specifier) { + // Check if the field is part of the primary keys. + if (isset($entity_schema[$table_name]['primary key']) && array_intersect($column_names, $entity_schema[$table_name]['primary key'])) { + $schema_handler->addPrimaryKey($table_name, $entity_schema[$table_name]['primary key']); + break; + } + } if (!empty($schema[$table_name]['indexes'])) { foreach ($schema[$table_name]['indexes'] as $name => $specifier) { // Check if the index exists because it might already have been @@ -2286,4 +2315,35 @@ protected function addUniqueKey($table, $name, array $specifier) { $schema_handler->addUniqueKey($table, $name, $specifier); } + /** + * Removes the primary key of a table if the passed columns are part of it. + * + * @param string $table_name + * The name of the table whose primary key is to be deleted. + * @param string[] $column_names + * The list of columns to check whether they are part of the primary key. + * + * @internal + */ + private function dropPrimaryKey($table_name, $column_names) { + $schema_handler = $this->database->schema(); + // The entity schema needs to be checked because the field schema is + // potentially incomplete. + // @todo Remove this in + // https://www.drupal.org/project/drupal/issues/2929120 + $entity_schema = $this->getEntitySchema($this->entityType); + if (array_intersect($column_names, $entity_schema[$table_name]['primary key'])) { + // Dropping a primary key must not leave a serial key, so we need to + // change any serial field to integer first. + foreach ($entity_schema[$table_name]['primary key'] as $column_name) { + if ($entity_schema[$table_name]['fields'][$column_name]['type'] === 'serial') { + $field_schema = $entity_schema[$table_name]['fields'][$column_name]; + $field_schema['type'] = 'int'; + $schema_handler->changeField($table_name, $column_name, $column_name, $field_schema); + } + } + $schema_handler->dropPrimaryKey($table_name); + } + } + } diff --git a/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.install b/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.install index c56d5b5669..a165267b8a 100644 --- a/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.install +++ b/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.install @@ -6,18 +6,24 @@ */ /** - * Implements hook_install(). + * Implements hook_module_preinstall(). */ -function contact_storage_test_install() { - $entity_manager = \Drupal::entityManager(); - $entity_type = $entity_manager->getDefinition('contact_message'); - - // Recreate the original entity type definition, in order to notify the - // manager of what changed. The change of storage backend will trigger - // schema installation. +function contact_storage_test_module_preinstall($module) { + // This test module alters the definition of the 'contact_message' entity type + // so we need to inform the system about our changes before additional field + // storage definitions are added by the module installer. // @see contact_storage_test_entity_type_alter() - $original = clone $entity_type; - $original->setStorageClass('Drupal\Core\Entity\ContentEntityNullStorage'); + // @see contact_storage_test_entity_base_field_info + if ($module === 'contact_storage_test') { + $definition_update_manager = \Drupal::entityDefinitionUpdateManager(); + $entity_type = $definition_update_manager->getEntityType('contact_message'); + + $entity_type->setStorageClass('\Drupal\Core\Entity\Sql\SqlContentEntityStorage'); + $keys = $entity_type->getKeys(); + $keys['id'] = 'id'; + $entity_type->set('entity_keys', $keys); + $entity_type->set('base_table', 'contact_message'); - $entity_manager->onEntityTypeUpdate($entity_type, $original); + $definition_update_manager->updateEntityType($entity_type); + } } diff --git a/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.module b/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.module index cef976c6ff..fbcd362572 100644 --- a/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.module +++ b/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.module @@ -33,8 +33,8 @@ function contact_storage_test_entity_base_field_info(EntityTypeInterface $entity */ function contact_storage_test_entity_type_alter(array &$entity_types) { /** @var $entity_types \Drupal\Core\Entity\EntityTypeInterface[] */ - // Set the controller class for nodes to an alternate implementation of the - // Drupal\Core\Entity\EntityStorageInterface interface. + // Set the controller class for contact messages to an alternate + // implementation of the \Drupal\Core\Entity\EntityStorageInterface interface. $entity_types['contact_message']->setStorageClass('\Drupal\Core\Entity\Sql\SqlContentEntityStorage'); $keys = $entity_types['contact_message']->getKeys(); $keys['id'] = 'id'; diff --git a/core/tests/Drupal/KernelTests/Core/Database/AssertSchemaTrait.php b/core/tests/Drupal/KernelTests/Core/Database/AssertSchemaTrait.php new file mode 100644 index 0000000000..8ba2564707 --- /dev/null +++ b/core/tests/Drupal/KernelTests/Core/Database/AssertSchemaTrait.php @@ -0,0 +1,55 @@ +databaseType(); + + switch ($db_type) { + case 'mysql': + $result = $connection->query("SHOW KEYS FROM {" . $table_name . "} WHERE Key_name = 'PRIMARY'")->fetchAllAssoc('Column_name'); + $this->assertSame($primary_key, array_keys($result)); + + break; + case 'pgsql': + $result = $connection->query("SELECT a.attname, format_type(a.atttypid, a.atttypmod) AS data_type + FROM pg_index i + JOIN pg_attribute a ON a.attrelid = i.indrelid AND a.attnum = ANY(i.indkey) + WHERE i.indrelid = '{" . $table_name . "}'::regclass AND i.indisprimary") + ->fetchAllAssoc('attname'); + $this->assertSame($primary_key, array_keys($result)); + + break; + case 'sqlite': + // For SQLite we need access to the protected + // \Drupal\Core\Database\Driver\sqlite\Schema::introspectSchema() method + // because we have no other way of getting the table prefixes needed for + // running a straight PRAGMA query. + $schema_object = $connection->schema(); + $reflection = new \ReflectionMethod($schema_object, 'introspectSchema'); + $reflection->setAccessible(TRUE); + + $table_info = $reflection->invoke($schema_object, $table_name); + $this->assertSame($primary_key, $table_info['primary key']); + + break; + } + } + +} diff --git a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php index e239098715..608c748606 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php @@ -16,6 +16,8 @@ */ class SchemaTest extends KernelTestBase { + use AssertSchemaTrait; + /** * A global counter for table and field creation. */ @@ -826,46 +828,4 @@ public function testFindTables() { Database::setActiveConnection('default'); } - /** - * Tests the primary keys of a table. - * - * @param string $table_name - * The name of the table to check. - * @param array $primary_key - * The expected key column specifier for a table's primary key. - */ - protected function assertPrimaryKeyColumns($table_name, array $primary_key = []) { - $db_type = Database::getConnection()->databaseType(); - - switch ($db_type) { - case 'mysql': - $result = Database::getConnection()->query("SHOW KEYS FROM {" . $table_name . "} WHERE Key_name = 'PRIMARY'")->fetchAllAssoc('Column_name'); - $this->assertSame($primary_key, array_keys($result)); - - break; - case 'pgsql': - $result = Database::getConnection()->query("SELECT a.attname, format_type(a.atttypid, a.atttypmod) AS data_type - FROM pg_index i - JOIN pg_attribute a ON a.attrelid = i.indrelid AND a.attnum = ANY(i.indkey) - WHERE i.indrelid = '{" . $table_name . "}'::regclass AND i.indisprimary") - ->fetchAllAssoc('attname'); - $this->assertSame($primary_key, array_keys($result)); - - break; - case 'sqlite': - // For SQLite we need access to the protected - // \Drupal\Core\Database\Driver\sqlite\Schema::introspectSchema() method - // because we have no other way of getting the table prefixes needed for - // running a straight PRAGMA query. - $schema_object = Database::getConnection()->schema(); - $reflection = new \ReflectionMethod($schema_object, 'introspectSchema'); - $reflection->setAccessible(TRUE); - - $table_info = $reflection->invoke($schema_object, $table_name); - $this->assertSame($primary_key, $table_info['primary key']); - - break; - } - } - } diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php index 3e285ff69b..a31228eff8 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php @@ -3,14 +3,19 @@ namespace Drupal\KernelTests\Core\Entity; use Drupal\Component\Utility\SafeMarkup; +use Drupal\Core\Entity\EntityTypeInterface; +use Drupal\Core\Field\BaseFieldDefinition; +use Drupal\KernelTests\Core\Database\AssertSchemaTrait; /** - * Tests adding a custom bundle field. + * Tests the default entity storage schema handler. * - * @group system + * @group Entity */ class EntitySchemaTest extends EntityKernelTestBase { + use AssertSchemaTrait; + /** * The database connection used. * @@ -109,6 +114,163 @@ public function testEntitySchemaUpdate() { $this->assertTrue($schema_handler->tableExists($dedicated_tables[0]), SafeMarkup::format('Field schema correct for the @table table.', ['@table' => $table])); } + /** + * Tests deleting and creating a field that is part of a primary key. + * + * @param string $entity_type_id + * The ID of the entity type whose schema is being tested. + * @param string $field_name + * The name of the field that is being re-installed. + * + * @dataProvider providerTestPrimaryKeyUpdate + */ + public function testPrimaryKeyUpdate($entity_type_id, $field_name) { + // EntityKernelTestBase::setUp() already installs the schema for the + // 'entity_test' entity type. + if ($entity_type_id !== 'entity_test') { + $this->installEntitySchema($entity_type_id); + } + + /* @var \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface $update_manager */ + $update_manager = $this->container->get('entity.definition_update_manager'); + $entity_type = $update_manager->getEntityType($entity_type_id); + + /* @see \Drupal\Core\Entity\ContentEntityBase::baseFieldDefinitions() */ + switch ($field_name) { + case 'id': + $field = BaseFieldDefinition::create('integer') + ->setLabel('ID') + ->setReadOnly(TRUE) + ->setSetting('unsigned', TRUE); + break; + + case 'revision_id': + $field = BaseFieldDefinition::create('integer') + ->setLabel('Revision ID') + ->setReadOnly(TRUE) + ->setSetting('unsigned', TRUE); + break; + + case 'langcode': + $field = BaseFieldDefinition::create('language') + ->setLabel('Language'); + if ($entity_type->isRevisionable()) { + $field->setRevisionable(TRUE); + } + if ($entity_type->isTranslatable()) { + $field->setTranslatable(TRUE); + } + break; + } + + $field + ->setName($field_name) + ->setTargetEntityTypeId($entity_type_id) + ->setProvider($entity_type->getProvider()); + + // First test explicitly deleting and re-installing a field. + // Make sure all primary keys are there to start with. + $this->assertPrimaryKeys($entity_type); + // Then uninstall the field and make sure all primary keys that the field + // is part of have been removed. + $update_manager->uninstallFieldStorageDefinition($field); + $this->assertPrimaryKeys($entity_type, $field_name); + // Finally reinstall the field and make sure the primary keys have been + // recreated. + $update_manager->installFieldStorageDefinition($field->getName(), $entity_type_id, $field->getProvider(), $field); + $this->assertPrimaryKeys($entity_type); + + // Now test updating a field without data. This will end up deleting + // and re-creating the field, similar to the code above. + $update_manager->updateFieldStorageDefinition($field); + $this->assertPrimaryKeys($entity_type); + + // Now test updating a field with data. + /* @var \Drupal\Core\Entity\FieldableEntityStorageInterface $storage */ + $storage = $this->entityManager->getStorage($entity_type_id); + // The schema of ID fields is incorrectly recreated as 'int' instead of + // 'serial', so we manually have to specify an ID. + // @todo Remove this in https://www.drupal.org/project/drupal/issues/2928906 + $storage->create(['id' => 1, 'revision_id' => 1])->save(); + $this->assertTrue($storage->countFieldData($field, TRUE)); + $update_manager->updateFieldStorageDefinition($field); + $this->assertPrimaryKeys($entity_type); + $this->assertTrue($storage->countFieldData($field, TRUE)); + } + + /** + * Asserts that the primary keys are as expected for a given entity type. + * + * @param \Drupal\Core\Entity\EntityTypeInterface $entity_type + * The entity type whose primary keys are being checked. + * @param string $removed_field + * (optional) The name of a field that has been removed. If passed, any + * primary key that would otherwise have contained this field will be + * checked to have been dropped. + */ + protected function assertPrimaryKeys(EntityTypeInterface $entity_type, $removed_field = NULL) { + $base_table = $entity_type->getBaseTable(); + $revision_table = $entity_type->getRevisionTable(); + $data_table = $entity_type->getDataTable(); + $revision_data_table = $entity_type->getRevisionDataTable(); + + $id_key = $entity_type->getKey('id'); + $revision_key = $entity_type->getKey('revision'); + $langcode_key = $entity_type->getKey('langcode'); + + // Build up a map of primary keys depending on the entity type + // configuration. + $key_map = []; + $key_map[$base_table] = [$id_key]; + if ($entity_type->isRevisionable()) { + $key_map[$revision_table] = [$revision_key]; + } + if ($entity_type->isTranslatable()) { + $key_map[$data_table] = [$id_key, $langcode_key]; + } + if ($entity_type->isRevisionable() && $entity_type->isTranslatable()) { + $key_map[$revision_data_table] = [$revision_key, $langcode_key]; + } + + foreach ($key_map as $table => $primary_key) { + // Assert that the primary key has been dropped if a field name was passed + // and the given primary key contains this field. + if (!$removed_field || !in_array($removed_field, $primary_key, TRUE)) { + $this->assertPrimaryKeyColumns($table, $primary_key); + } + else { + $this->assertPrimaryKeyColumns($table, []); + } + } + } + + /** + * Provides test cases for EntitySchemaTest::testPrimaryKeyUpdate() + * + * @return array + * An array of test cases consisting of an entity type ID and a field name. + */ + public function providerTestPrimaryKeyUpdate() { + // Build up test cases for all possible entity type configurations. + // For each entity type we test reinstalling each field that is part of + // any table's primary key. + $tests = []; + + $tests['entity_test:id'] = ['entity_test', 'id']; + + $tests['entity_test_rev:id'] = ['entity_test_rev', 'id']; + $tests['entity_test_rev:revision_id'] = ['entity_test_rev', 'revision_id']; + + $tests['entity_test_mul:id'] = ['entity_test_mul', 'id']; + $tests['entity_test_mul:langcode'] = ['entity_test_mul', 'langcode']; + + $tests['entity_test_mulrev:id'] = ['entity_test_mulrev', 'id']; + $tests['entity_test_mulrev:revision_id'] = ['entity_test_mulrev', 'revision_id']; + $tests['entity_test_mulrev:langcode'] = ['entity_test_mulrev', 'langcode']; + + return $tests; + } + /** * {@inheritdoc} */