diff --git a/core/includes/database.inc b/core/includes/database.inc index 75744f4..240f6ee 100644 --- a/core/includes/database.inc +++ b/core/includes/database.inc @@ -905,16 +905,22 @@ function db_drop_unique_key($table, $name) { * The name of the index. * @param array $fields * An array of field names. + * @param array $spec + * The table specification of the table to be altered, as taken from a schema + * definition. See \Drupal\Core\Database\Schema::addIndex() for how to obtain + * this specification. * * @deprecated as of Drupal 8.0.x, will be removed in Drupal 9.0.0. Instead, get * a database connection injected into your service from the container, get * its schema driver, and call addIndex() on it. E.g. - * $injected_database->schema()->addIndex($table, $name, $fields); + * $injected_database->schema()->addIndex($table, $name, $fields, $spec); * + * @see hook_schema() + * @see schemaapi * @see \Drupal\Core\Database\Schema::addIndex() */ -function db_add_index($table, $name, $fields) { - return Database::getConnection()->schema()->addIndex($table, $name, $fields); +function db_add_index($table, $name, $fields, array $spec) { + return Database::getConnection()->schema()->addIndex($table, $name, $fields, $spec); } /** diff --git a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php b/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php index 8f5b710..8c1f51d 100644 --- a/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/mysql/Schema.php @@ -9,6 +9,7 @@ use Drupal\Core\Database\Database; use Drupal\Core\Database\Query\Condition; +use Drupal\Core\Database\SchemaException; use Drupal\Core\Database\SchemaObjectExistsException; use Drupal\Core\Database\SchemaObjectDoesNotExistException; use Drupal\Core\Database\Schema as DatabaseSchema; @@ -299,14 +300,17 @@ protected function createKeysSql($spec) { * Shortens indexes to 191 characters if they apply to utf8mb4-encoded * fields, in order to comply with the InnoDB index limitation of 756 bytes. * - * @param $spec + * @param array $spec * The table specification. * * @return array * List of shortened indexes. + * + * @throws \Drupal\Core\Database\SchemaException + * Thrown if field specification is missing. */ - protected function getNormalizedIndexes($spec) { - $indexes = $spec['indexes']; + protected function getNormalizedIndexes(array $spec) { + $indexes = isset($spec['indexes']) ? $spec['indexes'] : []; foreach ($indexes as $index_name => $index_fields) { foreach ($index_fields as $index_key => $index_field) { // Get the name of the field from the index specification. @@ -323,6 +327,9 @@ protected function getNormalizedIndexes($spec) { } } } + else { + throw new SchemaException("MySQL needs the '$field_name' field specification in order to normalize the '$index_name' index"); + } } } return $indexes; @@ -486,7 +493,10 @@ public function dropUniqueKey($table, $name) { return TRUE; } - public function addIndex($table, $name, $fields) { + /** + * {@inheritdoc} + */ + public function addIndex($table, $name, $fields, array $spec) { if (!$this->tableExists($table)) { throw new SchemaObjectDoesNotExistException(t("Cannot add index @name to table @table: table doesn't exist.", array('@table' => $table, '@name' => $name))); } @@ -494,7 +504,10 @@ public function addIndex($table, $name, $fields) { throw new SchemaObjectExistsException(t("Cannot add index @name to table @table: index already exists.", array('@table' => $table, '@name' => $name))); } - $this->connection->query('ALTER TABLE {' . $table . '} ADD INDEX `' . $name . '` (' . $this->createKeySql($fields) . ')'); + $spec['indexes'][$name] = $fields; + $indexes = $this->getNormalizedIndexes($spec); + + $this->connection->query('ALTER TABLE {' . $table . '} ADD INDEX `' . $name . '` (' . $this->createKeySql($indexes[$name]) . ')'); } public function dropIndex($table, $name) { diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php index e3062dd..6025a70 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php @@ -637,7 +637,10 @@ public function dropUniqueKey($table, $name) { return TRUE; } - public function addIndex($table, $name, $fields) { + /** + * {@inheritdoc} + */ + public function addIndex($table, $name, $fields, array $spec) { if (!$this->tableExists($table)) { throw new SchemaObjectDoesNotExistException(t("Cannot add index @name to table @table: table doesn't exist.", array('@table' => $table, '@name' => $name))); } @@ -779,7 +782,10 @@ protected function _createKeys($table, $new_keys) { } if (isset($new_keys['indexes'])) { foreach ($new_keys['indexes'] as $name => $fields) { - $this->addIndex($table, $name, $fields); + // Even though $new_keys is not a full schema it still has 'indexes' and + // so is a partial schema. Technically addIndex() doesn't do anything + // with it so passing an empty array would work as well. + $this->addIndex($table, $name, $fields, $new_keys); } } } diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php index 3ac3811..f417b18 100644 --- a/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php @@ -582,7 +582,10 @@ protected function mapKeyDefinition(array $key_definition, array $mapping) { return $key_definition; } - public function addIndex($table, $name, $fields) { + /** + * {@inheritdoc} + */ + public function addIndex($table, $name, $fields, array $spec) { if (!$this->tableExists($table)) { throw new SchemaObjectDoesNotExistException(t("Cannot add index @name to table @table: table doesn't exist.", array('@table' => $table, '@name' => $name))); } diff --git a/core/lib/Drupal/Core/Database/Schema.php b/core/lib/Drupal/Core/Database/Schema.php index 4ebfd40..b9ff670 100644 --- a/core/lib/Drupal/Core/Database/Schema.php +++ b/core/lib/Drupal/Core/Database/Schema.php @@ -413,13 +413,51 @@ public function fieldExists($table, $column) { * @code * $fields = ['foo', ['bar', 4]]; * @endcode + * @param array $spec + * The table specification for the table to be altered. This is used in + * order to be able to ensure that the index length is not too long. + * This schema definition can usually be obtained through hook_schema(), or + * in case the table was created by the Entity API, through the schema + * handler listed in the entity class definition. For reference, see + * SqlContentEntityStorageSchema::getDedicatedTableSchema() and + * SqlContentEntityStorageSchema::getSharedTableFieldSchema(). + * + * In order to prevent human error, it is recommended to pass in the + * complete table specification. However, in the edge case of the complete + * table specification not being available, we can pass in a partial table + * definition containing only the fields that apply to the index: + * @code + * $spec = [ + * // Example partial specification for a table: + * 'fields' => [ + * 'example_field' => [ + * 'description' => 'An example field', + * 'type' => 'varchar', + * 'length' => 32, + * 'not null' => TRUE, + * 'default' => '', + * ], + * ], + * 'indexes' => [ + * 'table_example_field' => ['example_field'], + * ], + * ]; + * @endcode + * Note that the above is a partial table definition and that we would + * usually pass a complete table definition as obtained through + * hook_schema() instead. + * + * @see schemaapi + * @see hook_schema() * * @throws \Drupal\Core\Database\SchemaObjectDoesNotExistException * If the specified table doesn't exist. * @throws \Drupal\Core\Database\SchemaObjectExistsException * If the specified table already has an index by that name. + * + * @todo remove the $spec argument whenever schema introspection is added. */ - abstract public function addIndex($table, $name, $fields); + abstract public function addIndex($table, $name, $fields, array $spec); /** * Drop an index. diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php index 8a96e51..28f5884 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php @@ -330,9 +330,13 @@ public function onEntityTypeUpdate(EntityTypeInterface $entity_type, EntityTypeI // Create new indexes and unique keys. $entity_schema = $this->getEntitySchema($entity_type, TRUE); foreach ($this->getEntitySchemaData($entity_type, $entity_schema) as $table_name => $schema) { + // Add fields schema because database driver may depend on this data to + // perform index normalization. + $schema['fields'] = $entity_schema[$table_name]['fields']; + if (!empty($schema['indexes'])) { foreach ($schema['indexes'] as $name => $specifier) { - $schema_handler->addIndex($table_name, $name, $specifier); + $schema_handler->addIndex($table_name, $name, $specifier, $schema); } } if (!empty($schema['unique keys'])) { @@ -1139,7 +1143,7 @@ protected function createSharedTableSchema(FieldStorageDefinitionInterface $stor // Check if the index exists because it might already have been // created as part of the earlier entity type update event. if (!$schema_handler->indexExists($table_name, $name)) { - $schema_handler->addIndex($table_name, $name, $specifier); + $schema_handler->addIndex($table_name, $name, $specifier, $schema[$table_name]); } } } @@ -1273,9 +1277,13 @@ protected function updateDedicatedTableSchema(FieldStorageDefinitionInterface $s $table = $table_mapping->getDedicatedDataTableName($original); $revision_table = $table_mapping->getDedicatedRevisionTableName($original); + // Get the field schemas. $schema = $storage_definition->getSchema(); $original_schema = $original->getSchema(); + // Gets the SQL schema for a dedicated tables. + $actual_schema = $this->getDedicatedTableSchema($storage_definition); + foreach ($original_schema['indexes'] as $name => $columns) { if (!isset($schema['indexes'][$name]) || $columns != $schema['indexes'][$name]) { $real_name = $this->getFieldIndexName($storage_definition, $name); @@ -1302,8 +1310,8 @@ protected function updateDedicatedTableSchema(FieldStorageDefinitionInterface $s $real_columns[] = $table_mapping->getFieldColumnName($storage_definition, $column_name); } } - $this->database->schema()->addIndex($table, $real_name, $real_columns); - $this->database->schema()->addIndex($revision_table, $real_name, $real_columns); + $this->database->schema()->addIndex($table, $real_name, $real_columns, $actual_schema[$table]); + $this->database->schema()->addIndex($revision_table, $real_name, $real_columns, $actual_schema[$revision_table]); } } $this->saveFieldSchemaData($storage_definition, $this->getDedicatedTableSchema($storage_definition)); @@ -1380,7 +1388,7 @@ protected function updateSharedTableSchema(FieldStorageDefinitionInterface $stor // Create new indexes and unique keys. if (!empty($schema[$table_name]['indexes'])) { foreach ($schema[$table_name]['indexes'] as $name => $specifier) { - $schema_handler->addIndex($table_name, $name, $specifier); + $schema_handler->addIndex($table_name, $name, $specifier, $schema[$table_name]); } } if (!empty($schema[$table_name]['unique keys'])) { diff --git a/core/modules/system/src/Tests/Database/SchemaTest.php b/core/modules/system/src/Tests/Database/SchemaTest.php index 1e77084..8ec268d 100644 --- a/core/modules/system/src/Tests/Database/SchemaTest.php +++ b/core/modules/system/src/Tests/Database/SchemaTest.php @@ -8,6 +8,7 @@ namespace Drupal\system\Tests\Database; use Drupal\Core\Database\Database; +use Drupal\Core\Database\SchemaException; use Drupal\Core\Database\SchemaObjectDoesNotExistException; use Drupal\Core\Database\SchemaObjectExistsException; use Drupal\simpletest\KernelTestBase; @@ -99,7 +100,7 @@ function testSchema() { $index_exists = Database::getConnection()->schema()->indexExists('test_table', 'test_field'); $this->assertIdentical($index_exists, FALSE, 'Fake index does not exists'); // Add index. - db_add_index('test_table', 'test_field', array('test_field')); + db_add_index('test_table', 'test_field', array('test_field'), $table_specification); // Test for created index and test for the boolean result of indexExists(). $index_exists = Database::getConnection()->schema()->indexExists('test_table', 'test_field'); $this->assertIdentical($index_exists, TRUE, 'Index created.'); @@ -295,6 +296,43 @@ function testIndexLength() { ); db_create_table('test_table_index_length', $table_specification); + $schema_object = Database::getConnection()->schema(); + + // Ensure expected exception thrown when adding index with missing info. + $expected_exception_message = "MySQL needs the 'test_field_text' field specification in order to normalize the 'test_regular' index"; + $missing_field_spec = $table_specification; + unset($missing_field_spec['fields']['test_field_text']); + try { + $schema_object->addIndex('test_table_index_length', 'test_separate', [['test_field_text', 200]], $missing_field_spec); + $this->fail('SchemaException not thrown when adding index with missing information.'); + } + catch (SchemaException $e) { + $this->assertEqual($expected_exception_message, $e->getMessage()); + } + + // Add a separate index. + $schema_object->addIndex('test_table_index_length', 'test_separate', [['test_field_text', 200]], $table_specification); + $table_specification_with_new_index = $table_specification; + $table_specification_with_new_index['indexes']['test_separate'] = [['test_field_text', 200]]; + + // Ensure that the exceptions of addIndex are thrown as expected. + + try { + $schema_object->addIndex('test_table_index_length', 'test_separate', [['test_field_text', 200]], $table_specification); + $this->fail('\Drupal\Core\Database\SchemaObjectExistsException exception missed.'); + } + catch (SchemaObjectExistsException $e) { + $this->pass('\Drupal\Core\Database\SchemaObjectExistsException thrown when index already exists.'); + } + + try { + $schema_object->addIndex('test_table_non_existing', 'test_separate', [['test_field_text', 200]], $table_specification); + $this->fail('\Drupal\Core\Database\SchemaObjectDoesNotExistException exception missed.'); + } + catch (SchemaObjectDoesNotExistException $e) { + $this->pass('\Drupal\Core\Database\SchemaObjectDoesNotExistException thrown when index already exists.'); + } + // Get index information. $results = db_query('SHOW INDEX FROM {test_table_index_length}'); $expected_lengths = array( @@ -316,11 +354,14 @@ function testIndexLength() { 'test_field_string_ascii_long' => 200, 'test_field_string_short' => NULL, ), + 'test_separate' => array( + 'test_field_text' => 191, + ), ); // Count the number of columns defined in the indexes. $column_count = 0; - foreach ($table_specification['indexes'] as $index) { + foreach ($table_specification_with_new_index['indexes'] as $index) { foreach ($index as $field) { $column_count++; } diff --git a/core/modules/system/src/Tests/Entity/EntityDefinitionUpdateTest.php b/core/modules/system/src/Tests/Entity/EntityDefinitionUpdateTest.php index 03cb236..8ae7fa5 100644 --- a/core/modules/system/src/Tests/Entity/EntityDefinitionUpdateTest.php +++ b/core/modules/system/src/Tests/Entity/EntityDefinitionUpdateTest.php @@ -7,6 +7,7 @@ namespace Drupal\system\Tests\Entity; +use Drupal\Core\Database\Database; use Drupal\Core\Database\DatabaseExceptionWrapper; use Drupal\Core\Database\IntegrityConstraintViolationException; use Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface; @@ -626,4 +627,48 @@ public function testSingleActionCalls() { $this->assertTrue($this->database->schema()->tableExists('entity_test_update_revision'), "The 'entity_test_update_revision' table has been created."); } + /** + * Ensures that a new field and index on a shared table are created. + * + * @see Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema::createSharedTableSchema + */ + public function testCreateFieldAndIndexOnSharedTable() { + $this->addBaseField(); + $this->addBaseFieldIndex(); + $this->entityDefinitionUpdateManager->applyUpdates(); + $this->assertTrue($this->database->schema()->fieldExists('entity_test_update', 'new_base_field'), "New field 'new_base_field' has been created on the 'entity_test_update' table."); + $this->assertTrue($this->database->schema()->indexExists('entity_test_update', 'entity_test_update_field__new_base_field'), "New index 'entity_test_update_field__new_base_field' has been created on the 'entity_test_update' table."); + // Check index size in for MySQL. + if (Database::getConnection()->driver() == 'mysql') { + $result = Database::getConnection()->query('SHOW INDEX FROM {entity_test_update} WHERE key_name = \'entity_test_update_field__new_base_field\' and column_name = \'new_base_field\'')->fetchObject(); + $this->assertEqual(191, $result->Sub_part, 'The index length has been restricted to 191 characters for UTF8MB4 compatibility.'); + } + } + + /** + * Ensures that a new entity level index is created when data exists. + * + * @see Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema::onEntityTypeUpdate + */ + public function testCreateIndexUsingEntityStorageSchemaWithData() { + // Save an entity. + $name = $this->randomString(); + $storage = $this->entityManager->getStorage('entity_test_update'); + $entity = $storage->create(array('name' => $name)); + $entity->save(); + + // Create an index. + $indexes = array( + 'entity_test_update__type_index' => array('type'), + ); + $this->state->set('entity_test_update.additional_entity_indexes', $indexes); + $this->entityDefinitionUpdateManager->applyUpdates(); + $this->assertTrue($this->database->schema()->indexExists('entity_test_update', 'entity_test_update__type_index'), "New index 'entity_test_update__type_index' has been created on the 'entity_test_update' table."); + // Check index size in for MySQL. + if (Database::getConnection()->driver() == 'mysql') { + $result = Database::getConnection()->query('SHOW INDEX FROM {entity_test_update} WHERE key_name = \'entity_test_update__type_index\' and column_name = \'type\'')->fetchObject(); + $this->assertEqual(191, $result->Sub_part, 'The index length has been restricted to 191 characters for UTF8MB4 compatibility.'); + } + } + } diff --git a/core/modules/system/tests/modules/update_test_schema/update_test_schema.install b/core/modules/system/tests/modules/update_test_schema/update_test_schema.install index b6aa9c3..972f901 100644 --- a/core/modules/system/tests/modules/update_test_schema/update_test_schema.install +++ b/core/modules/system/tests/modules/update_test_schema/update_test_schema.install @@ -35,7 +35,14 @@ function update_test_schema_schema() { * Schema version 8001. */ function update_test_schema_update_8001() { + $table = [ + 'fields' => [ + 'a' => ['type' => 'int', 'not null' => TRUE], + 'b' => ['type' => 'blob', 'not null' => FALSE], + ], + ]; + // Add a column. - db_add_index('update_test_schema_table', 'test', ['a']); + db_add_index('update_test_schema_table', 'test', ['a'], $table); } } diff --git a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php index 9b6f0e2..e5d6659 100644 --- a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php @@ -19,6 +19,13 @@ class SqlContentEntityStorageSchemaTest extends UnitTestCase { /** + * The mocked DB schema handler. + * + * @var \Drupal\Core\Database\Schema|\PHPUnit_Framework_MockObject_MockObject + */ + protected $dbSchemaHandler; + + /** * The mocked entity manager used in this test. * * @var \Drupal\Core\Entity\EntityManagerInterface|\PHPUnit_Framework_MockObject_MockObject @@ -1298,7 +1305,7 @@ protected function setUpStorageSchema(array $expected = array()) { ->with($this->entityType->id()) ->will($this->returnValue($this->storageDefinitions)); - $db_schema_handler = $this->getMockBuilder('Drupal\Core\Database\Schema') + $this->dbSchemaHandler = $this->getMockBuilder('Drupal\Core\Database\Schema') ->disableOriginalConstructor() ->getMock(); @@ -1307,7 +1314,7 @@ protected function setUpStorageSchema(array $expected = array()) { $expected_table_names = array_keys($expected); $expected_table_schemas = array_values($expected); - $db_schema_handler->expects($this->any()) + $this->dbSchemaHandler->expects($this->any()) ->method('createTable') ->with( $this->callback(function($table_name) use (&$invocation_count, $expected_table_names) { @@ -1327,17 +1334,21 @@ protected function setUpStorageSchema(array $expected = array()) { ->getMock(); $connection->expects($this->any()) ->method('schema') - ->will($this->returnValue($db_schema_handler)); + ->will($this->returnValue($this->dbSchemaHandler)); $key_value = $this->getMock('Drupal\Core\KeyValueStore\KeyValueStoreInterface'); $this->storageSchema = $this->getMockBuilder('Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema') ->setConstructorArgs(array($this->entityManager, $this->entityType, $this->storage, $connection)) - ->setMethods(array('installedStorageSchema', 'loadEntitySchemaData', 'hasSharedTableNameChanges')) + ->setMethods(array('installedStorageSchema', 'loadEntitySchemaData', 'hasSharedTableNameChanges', 'isTableEmpty')) ->getMock(); $this->storageSchema ->expects($this->any()) ->method('installedStorageSchema') ->will($this->returnValue($key_value)); + $this->storageSchema + ->expects($this->any()) + ->method('isTableEmpty') + ->willReturn(FALSE); } /** @@ -1380,4 +1391,76 @@ public function setUpStorageDefinition($field_name, array $schema) { } } + /** + * ::onEntityTypeUpdate + */ + public function testonEntityTypeUpdateWithNewIndex() { + $entity_type_id = 'entity_test'; + $this->entityType = $original_entity_type = new ContentEntityType(array( + 'id' => 'entity_test', + 'entity_keys' => array('id' => 'id'), + )); + + // Add a field with a really long index. + $this->setUpStorageDefinition('long_index_name', array( + 'columns' => array( + 'long_index_name' => array( + 'type' => 'int', + ), + ), + 'indexes' => array( + 'long_index_name_really_long_long_name' => array(array('long_index_name', 10)), + ), + )); + + $expected = array( + 'entity_test' => array( + 'description' => 'The base table for entity_test entities.', + 'fields' => array( + 'id' => array( + 'type' => 'serial', + 'not null' => TRUE, + ), + 'long_index_name' => array( + 'type' => 'int', + 'not null' => FALSE, + ), + ), + 'indexes' => array( + 'entity_test__b588603cb9' => array( + array('long_index_name', 10), + ), + ), + ), + ); + + $this->setUpStorageSchema($expected); + + $table_mapping = new DefaultTableMapping($this->entityType, $this->storageDefinitions); + $table_mapping->setFieldNames('entity_test', array_keys($this->storageDefinitions)); + $table_mapping->setExtraColumns('entity_test', array('default_langcode')); + + $this->storage->expects($this->any()) + ->method('getTableMapping') + ->will($this->returnValue($table_mapping)); + + $this->storageSchema->expects($this->any()) + ->method('loadEntitySchemaData') + ->willReturn([]); + + $this->dbSchemaHandler->expects($this->atLeastOnce()) + ->method('addIndex') + ->with('entity_test', 'entity_test__b588603cb9', [['long_index_name', 10]], $this->callback(function($actual_value) use ($expected) { + $this->assertEquals($expected['entity_test']['indexes'], $actual_value['indexes']); + $this->assertEquals($expected['entity_test']['fields'], $actual_value['fields']); + // If the parameters don't match, the assertions above will throw an + // exception. + return TRUE; + })); + + $this->assertNull( + $this->storageSchema->onEntityTypeUpdate($this->entityType, $original_entity_type) + ); + } + }