diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php index dfae89736329739f036a98e587e14c9b2b758511..455a3fcddd2f769bbfabe0fadd718ba343ca4cf5 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php @@ -10,6 +10,7 @@ use Drupal\Core\Database\SchemaException; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\ContentEntityStorageBase; +use Drupal\Core\Entity\ContentEntityTypeInterface; use Drupal\Core\Entity\EntityBundleListenerInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityManagerInterface; @@ -305,27 +306,41 @@ public function setTemporary($temporary) { * {@inheritdoc} */ public function getTableMapping(array $storage_definitions = NULL) { - $table_mapping = $this->tableMapping; + // If a new set of field storage definitions is passed, for instance when + // comparing old and new storage schema, we compute the table mapping + // without caching. + if ($storage_definitions) { + return $this->getCustomTableMapping($this->entityType, $storage_definitions); + } // If we are using our internal storage definitions, which is our main use - // case, we can statically cache the computed table mapping. If a new set - // of field storage definitions is passed, for instance when comparing old - // and new storage schema, we compute the table mapping without caching. - if (!isset($this->tableMapping) || $storage_definitions) { - $table_mapping_class = $this->temporary ? TemporaryTableMapping::class : DefaultTableMapping::class; - $definitions = $storage_definitions ?: $this->entityManager->getFieldStorageDefinitions($this->entityTypeId); - - /** @var \Drupal\Core\Entity\Sql\DefaultTableMapping|\Drupal\Core\Entity\Sql\TemporaryTableMapping $table_mapping */ - $table_mapping = $table_mapping_class::create($this->entityType, $definitions); - - // Cache the computed table mapping only if we are using our internal - // storage definitions. - if (!$storage_definitions) { - $this->tableMapping = $table_mapping; - } + // case, we can statically cache the computed table mapping. + if (!isset($this->tableMapping)) { + $storage_definitions = $this->entityManager->getFieldStorageDefinitions($this->entityTypeId); + + $this->tableMapping = $this->getCustomTableMapping($this->entityType, $storage_definitions); } - return $table_mapping; + return $this->tableMapping; + } + + /** + * Gets a table mapping for the specified entity type and storage definitions. + * + * @param \Drupal\Core\Entity\ContentEntityTypeInterface $entity_type + * An entity type definition. + * @param \Drupal\Core\Field\FieldStorageDefinitionInterface[] $storage_definitions + * An array of field storage definitions to be used to compute the table + * mapping. + * + * @return \Drupal\Core\Entity\Sql\TableMappingInterface + * A table mapping object for the entity's tables. + * + * @internal + */ + public function getCustomTableMapping(ContentEntityTypeInterface $entity_type, array $storage_definitions) { + $table_mapping_class = $this->temporary ? TemporaryTableMapping::class : DefaultTableMapping::class; + return $table_mapping_class::create($entity_type, $storage_definitions); } /** @@ -1397,14 +1412,6 @@ public function onEntityTypeDelete(EntityTypeInterface $entity_type) { * {@inheritdoc} */ public function onFieldStorageDefinitionCreate(FieldStorageDefinitionInterface $storage_definition) { - // If we are adding a field stored in a shared table we need to recompute - // the table mapping. - // @todo This does not belong here. Remove it once we are able to generate a - // fresh table mapping in the schema handler. See - // https://www.drupal.org/node/2274017. - if ($this->getTableMapping()->allowsSharedTableStorage($storage_definition)) { - $this->tableMapping = NULL; - } $this->wrapSchemaException(function () use ($storage_definition) { $this->getStorageSchema()->onFieldStorageDefinitionCreate($storage_definition); }); @@ -1629,16 +1636,7 @@ public function countFieldData($storage_definition, $as_bool = FALSE) { elseif ($table_mapping->allowsSharedTableStorage($storage_definition)) { // Ascertain the table this field is mapped too. $field_name = $storage_definition->getName(); - try { - $table_name = $table_mapping->getFieldTableName($field_name); - } - catch (SqlContentEntityStorageException $e) { - // This may happen when changing field storage schema, since we are not - // able to use a table mapping matching the passed storage definition. - // @todo Revisit this once we are able to instantiate the table mapping - // properly. See https://www.drupal.org/node/2274017. - $table_name = $this->dataTable ?: $this->baseTable; - } + $table_name = $table_mapping->getFieldTableName($field_name); $query = $this->database->select($table_name, 't'); $or = $query->orConditionGroup(); foreach (array_keys($storage_definition->getColumns()) as $property_name) { diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php index cfe0736b1569200e4e392ad59616bb83f7e9d93a..c73a28ca943557afdabeb143c11e077690a3fbfb 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php @@ -388,36 +388,21 @@ public function onEntityTypeUpdate(EntityTypeInterface $entity_type, EntityTypeI public function onEntityTypeDelete(EntityTypeInterface $entity_type) { $this->checkEntityType($entity_type); $schema_handler = $this->database->schema(); - $actual_definition = $this->entityManager->getDefinition($entity_type->id()); - // @todo Instead of switching the wrapped entity type, we should be able to - // instantiate a new table mapping for each entity type definition. See - // https://www.drupal.org/node/2274017. - $this->storage->setEntityType($entity_type); - - // Delete entity tables. - foreach ($this->getEntitySchemaTables() as $table_name) { + + $field_storage_definitions = $this->entityManager->getLastInstalledFieldStorageDefinitions($entity_type->id()); + $table_mapping = $this->storage->getCustomTableMapping($entity_type, $field_storage_definitions); + + // Delete entity and field tables. + foreach ($table_mapping->getTableNames() as $table_name) { if ($schema_handler->tableExists($table_name)) { $schema_handler->dropTable($table_name); } } - // Delete dedicated field tables. - $field_storage_definitions = $this->entityManager->getLastInstalledFieldStorageDefinitions($entity_type->id()); - $this->originalDefinitions = $field_storage_definitions; - $table_mapping = $this->storage->getTableMapping($field_storage_definitions); + // Delete the field schema data. foreach ($field_storage_definitions as $field_storage_definition) { - // If we have a field having dedicated storage we need to drop it, - // otherwise we just remove the related schema data. - if ($table_mapping->requiresDedicatedTableStorage($field_storage_definition)) { - $this->deleteDedicatedTableSchema($field_storage_definition); - } - elseif ($table_mapping->allowsSharedTableStorage($field_storage_definition)) { - $this->deleteFieldSchemaData($field_storage_definition); - } + $this->deleteFieldSchemaData($field_storage_definition); } - $this->originalDefinitions = NULL; - - $this->storage->setEntityType($actual_definition); // Delete the entity schema. $this->deleteEntitySchemaData($entity_type); @@ -680,13 +665,6 @@ protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $res $entity_type_id = $entity_type->id(); if (!isset($this->schema[$entity_type_id]) || $reset) { - // Back up the storage definition and replace it with the passed one. - // @todo Instead of switching the wrapped entity type, we should be able - // to instantiate a new table mapping for each entity type definition. - // See https://www.drupal.org/node/2274017. - $actual_definition = $this->entityManager->getDefinition($entity_type_id); - $this->storage->setEntityType($entity_type); - // Prepare basic information about the entity type. $tables = $this->getEntitySchemaTables(); @@ -703,7 +681,7 @@ protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $res } // We need to act only on shared entity schema tables. - $table_mapping = $this->storage->getTableMapping(); + $table_mapping = $this->storage->getCustomTableMapping($entity_type, $this->fieldStorageDefinitions); $table_names = array_diff($table_mapping->getTableNames(), $table_mapping->getDedicatedTableNames()); foreach ($table_names as $table_name) { if (!isset($schema[$table_name])) { @@ -753,9 +731,6 @@ protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $res } $this->schema[$entity_type_id] = $schema; - - // Restore the actual definition. - $this->storage->setEntityType($actual_definition); } return $this->schema[$entity_type_id]; diff --git a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php index bf7c8f7f6a79e7a288e33a857df856b491ad1cca..cf18e2bb9c5ef5afc82f230d6bd4b445b44ca239 100644 --- a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php @@ -370,6 +370,9 @@ public function testGetSchemaBase() { $this->storage->expects($this->any()) ->method('getTableMapping') ->will($this->returnValue($table_mapping)); + $this->storage->expects($this->any()) + ->method('getCustomTableMapping') + ->will($this->returnValue($table_mapping)); $this->assertNull( $this->storageSchema->onEntityTypeCreate($this->entityType) @@ -478,6 +481,9 @@ public function testGetSchemaRevisionable() { $this->storage->expects($this->any()) ->method('getTableMapping') ->will($this->returnValue($table_mapping)); + $this->storage->expects($this->any()) + ->method('getCustomTableMapping') + ->will($this->returnValue($table_mapping)); $this->storageSchema->onEntityTypeCreate($this->entityType); } @@ -586,6 +592,9 @@ public function testGetSchemaTranslatable() { $this->storage->expects($this->any()) ->method('getTableMapping') ->will($this->returnValue($table_mapping)); + $this->storage->expects($this->any()) + ->method('getCustomTableMapping') + ->will($this->returnValue($table_mapping)); $this->assertNull( $this->storageSchema->onEntityTypeCreate($this->entityType) @@ -806,6 +815,9 @@ public function testGetSchemaRevisionableTranslatable() { $this->storage->expects($this->any()) ->method('getTableMapping') ->will($this->returnValue($table_mapping)); + $this->storage->expects($this->any()) + ->method('getCustomTableMapping') + ->will($this->returnValue($table_mapping)); $this->storageSchema->onEntityTypeCreate($this->entityType); } @@ -1303,6 +1315,9 @@ public function testRequiresEntityStorageSchemaChanges(ContentEntityTypeInterfac $this->storage->expects($this->any()) ->method('getTableMapping') ->will($this->returnValue($table_mapping)); + $this->storage->expects($this->any()) + ->method('getCustomTableMapping') + ->will($this->returnValue($table_mapping)); // Setup storage schema. if ($change_schema) { @@ -1487,6 +1502,9 @@ public function testonEntityTypeUpdateWithNewIndex() { $this->storage->expects($this->any()) ->method('getTableMapping') ->will($this->returnValue($table_mapping)); + $this->storage->expects($this->any()) + ->method('getCustomTableMapping') + ->will($this->returnValue($table_mapping)); $this->storageSchema->expects($this->any()) ->method('loadEntitySchemaData')