Commit f27fd1f5 authored by alexpott's avatar alexpott

Issue #2337927 by effulgentsia, plach, fago: Fixed...

Issue #2337927 by effulgentsia, plach, fago: Fixed SqlContentEntityStorage::onFieldStorageDefinition(Create|Update|Delete)() is broken for base fields.
parent fb4e8cdf
......@@ -189,8 +189,13 @@ protected function getChangeList() {
// Detect updated field storage definitions.
foreach (array_intersect_key($storage_definitions, $original_storage_definitions) as $field_name => $storage_definition) {
// @todo Support non-storage-schema-changing definition updates too:
// https://www.drupal.org/node/2336895.
if ($this->requiresFieldStorageSchemaChanges($storage_definition, $original_storage_definitions[$field_name])) {
// https://www.drupal.org/node/2336895. So long as we're checking
// based on schema change requirements rather than definition
// equality, skip the check if the entity type itself needs to be
// updated, since that can affect the schema of all fields, so we
// want to process that update first without reporting false
// positives here.
if (!isset($change_list[$entity_type_id]['entity_type']) && $this->requiresFieldStorageSchemaChanges($storage_definition, $original_storage_definitions[$field_name])) {
$field_changes[$field_name] = static::DEFINITION_UPDATED;
}
}
......
......@@ -1165,7 +1165,7 @@ protected function deleteLastInstalledDefinition($entity_type_id) {
* {@inheritdoc}
*/
public function getLastInstalledFieldStorageDefinitions($entity_type_id) {
return $this->installedDefinitions->get($entity_type_id . '.field_storage_definitions');
return $this->installedDefinitions->get($entity_type_id . '.field_storage_definitions', array());
}
/**
......
......@@ -67,4 +67,12 @@ public function requiresFieldStorageSchemaChanges(FieldStorageDefinitionInterfac
*/
public function requiresFieldDataMigration(FieldStorageDefinitionInterface $storage_definition, FieldStorageDefinitionInterface $original);
/**
* Performs final cleanup after all data of a field has been purged.
*
* @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage_definition
* The field being purged.
*/
public function finalizePurge(FieldStorageDefinitionInterface $storage_definition);
}
......@@ -15,7 +15,7 @@
class DefaultTableMapping implements TableMappingInterface {
/**
* A list of field storage definitions that are available for this mapping.
* The field storage definitions of this mapping.
*
* @var \Drupal\Core\Field\FieldStorageDefinitionInterface[]
*/
......@@ -101,7 +101,16 @@ public function getAllColumns($table_name) {
$this->allColumns[$table_name] = array_merge($this->allColumns[$table_name], array_values($this->getColumnNames($field_name)));
}
$this->allColumns[$table_name] = array_merge($this->allColumns[$table_name], $this->getExtraColumns($table_name));
// There is just one field for each dedicated storage table, thus
// $field_name can only refer to it.
if (isset($field_name) && $this->requiresDedicatedTableStorage($this->fieldStorageDefinitions[$field_name])) {
// Unlike in shared storage tables, in dedicated ones field columns are
// positioned last.
$this->allColumns[$table_name] = array_merge($this->getExtraColumns($table_name), $this->allColumns[$table_name]);
}
else {
$this->allColumns[$table_name] = array_merge($this->allColumns[$table_name], $this->getExtraColumns($table_name));
}
}
return $this->allColumns[$table_name];
}
......@@ -179,6 +188,47 @@ public function setExtraColumns($table_name, array $column_names) {
return $this;
}
/**
* Checks whether the given field can be stored in a shared table.
*
* @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage_definition
* The field storage definition.
*
* @return bool
* TRUE if the field can be stored in a dedicated table, FALSE otherwise.
*/
public function allowsSharedTableStorage(FieldStorageDefinitionInterface $storage_definition) {
return !$storage_definition->hasCustomStorage() && $storage_definition->isBaseField() && !$storage_definition->isMultiple();
}
/**
* Checks whether the given field has to be stored in a dedicated table.
*
* @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage_definition
* The field storage definition.
*
* @return bool
* TRUE if the field can be stored in a dedicated table, FALSE otherwise.
*/
public function requiresDedicatedTableStorage(FieldStorageDefinitionInterface $storage_definition) {
return !$storage_definition->hasCustomStorage() && !$this->allowsSharedTableStorage($storage_definition);
}
/**
* Returns a list of dedicated table names for this mapping.
*
* @return string[]
* An array of table names.
*/
public function getDedicatedTableNames() {
$table_mapping = $this;
$definitions = array_filter($this->fieldStorageDefinitions, function($definition) use ($table_mapping) { return $table_mapping->requiresDedicatedTableStorage($definition); });
$data_tables = array_map(function($definition) use ($table_mapping) { return $table_mapping->getDedicatedDataTableName($definition); }, $definitions);
$revision_tables = array_map(function($definition) use ($table_mapping) { return $table_mapping->getDedicatedRevisionTableName($definition); }, $definitions);
$dedicated_tables = array_merge(array_values($data_tables), array_values($revision_tables));
return $dedicated_tables;
}
/**
* {@inheritdoc}
*/
......
......@@ -17,9 +17,13 @@ interface SqlEntityStorageInterface extends EntityStorageInterface {
/**
* Gets a table mapping for the entity's SQL tables.
*
* @param \Drupal\Core\Field\FieldStorageDefinitionInterface[] $storage_definitions
* (optional) An array of field storage definitions to be used to compute
* the table mapping. Defaults to the ones provided by the entity manager.
*
* @return \Drupal\Core\Entity\Sql\TableMappingInterface
* A table mapping object for the entity's tables.
*/
public function getTableMapping();
public function getTableMapping(array $storage_definitions = NULL);
}
......@@ -821,6 +821,17 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
$version = max(max($versions), $version);
}
// Notify the entity manager that this module's entity types are new,
// so that it can notify all interested handlers. For example, a
// SQL-based storage handler can use this as an opportunity to create
// the necessary database tables.
$entity_manager = \Drupal::entityManager();
foreach ($entity_manager->getDefinitions() as $entity_type) {
if ($entity_type->getProvider() == $module) {
$entity_manager->onEntityTypeCreate($entity_type);
}
}
// Install default configuration of the module.
$config_installer = \Drupal::service('config.installer');
if ($sync_status) {
......@@ -844,17 +855,6 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
}
drupal_set_installed_schema_version($module, $version);
// Notify the entity manager that this module's entity types are new,
// so that it can notify all interested handlers. For example, a
// SQL-based storage handler can use this as an opportunity to create
// the necessary database tables.
$entity_manager = \Drupal::entityManager();
foreach ($entity_manager->getDefinitions() as $entity_type) {
if ($entity_type->getProvider() == $module) {
$entity_manager->onEntityTypeCreate($entity_type);
}
}
// Record the fact that it was installed.
$modules_installed[] = $module;
......
......@@ -553,11 +553,6 @@ public function getSchema() {
'foreign keys' => array(),
);
// Check that the schema does not include forbidden column names.
if (array_intersect(array_keys($schema['columns']), static::getReservedColumns())) {
throw new FieldException('Illegal field type columns.');
}
// Merge custom indexes with those specified by the field type. Custom
// indexes prevail.
$schema['indexes'] = $this->indexes + $schema['indexes'];
......@@ -583,19 +578,17 @@ public function getColumns() {
}
/**
* A list of columns that can not be used as field type columns.
*
* @return array
* {@inheritdoc}
*/
public static function getReservedColumns() {
return array('deleted');
public function hasCustomStorage() {
return !empty($this->definition['custom_storage']) || $this->isComputed();
}
/**
* {@inheritdoc}
*/
public function hasCustomStorage() {
return !empty($this->definition['custom_storage']) || $this->isComputed();
public function isBaseField() {
return TRUE;
}
/**
......
......@@ -297,6 +297,18 @@ public function getProvider();
*/
public function hasCustomStorage();
/**
* Determines whether the field is a base field.
*
* Base fields are not specific to a given bundle or a set of bundles. This
* excludes configurable fields, as they are always attached to a specific
* bundle.
*
* @return bool
* Whether the field is a base field.
*/
public function isBaseField();
/**
* Returns a unique identifier for the field.
*
......
......@@ -7,8 +7,8 @@
namespace Drupal\aggregator;
use Drupal\Core\Entity\ContentEntityTypeInterface;
use Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
/**
* Defines the feed schema handler.
......@@ -18,22 +18,25 @@ class FeedStorageSchema extends SqlContentEntityStorageSchema {
/**
* {@inheritdoc}
*/
protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $reset = FALSE) {
$schema = parent::getEntitySchema($entity_type, $reset);
// Marking the respective fields as NOT NULL makes the indexes more
// performant.
$schema['aggregator_feed']['fields']['url']['not null'] = TRUE;
$schema['aggregator_feed']['fields']['queued']['not null'] = TRUE;
$schema['aggregator_feed']['fields']['title']['not null'] = TRUE;
$schema['aggregator_feed']['indexes'] += array(
'aggregator_feed__url' => array(array('url', 255)),
'aggregator_feed__queued' => array('queued'),
);
$schema['aggregator_feed']['unique keys'] += array(
'aggregator_feed__title' => array('title'),
);
protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $storage_definition, $table_name, array $column_mapping) {
$schema = parent::getSharedTableFieldSchema($storage_definition, $table_name, $column_mapping);
$field_name = $storage_definition->getName();
if ($table_name == 'aggregator_feed') {
switch ($field_name) {
case 'url':
$this->addSharedTableFieldIndex($storage_definition, $schema, TRUE, 255);
break;
case 'queued':
$this->addSharedTableFieldIndex($storage_definition, $schema, TRUE);
break;
case 'title':
$this->addSharedTableFieldUniqueKey($storage_definition, $schema);
break;
}
}
return $schema;
}
......
......@@ -9,6 +9,7 @@
use Drupal\Core\Entity\ContentEntityTypeInterface;
use Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
/**
* Defines the item schema handler.
......@@ -18,22 +19,21 @@ class ItemStorageSchema extends SqlContentEntityStorageSchema {
/**
* {@inheritdoc}
*/
protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $reset = FALSE) {
$schema = parent::getEntitySchema($entity_type, $reset);
// Marking the respective fields as NOT NULL makes the indexes more
// performant.
$schema['aggregator_item']['fields']['timestamp']['not null'] = TRUE;
$schema['aggregator_item']['indexes'] += array(
'aggregator_item__timestamp' => array('timestamp'),
);
$schema['aggregator_item']['foreign keys'] += array(
'aggregator_item__aggregator_feed' => array(
'table' => 'aggregator_feed',
'columns' => array('fid' => 'fid'),
),
);
protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $storage_definition, $table_name, array $column_mapping) {
$schema = parent::getSharedTableFieldSchema($storage_definition, $table_name, $column_mapping);
$field_name = $storage_definition->getName();
if ($table_name == 'aggregator_item') {
switch ($field_name) {
case 'timestamp':
$this->addSharedTableFieldIndex($storage_definition, $schema, TRUE);
break;
case 'fid':
$this->addSharedTableFieldForeignKey($storage_definition, $schema, 'aggregator_feed', 'fid');
break;
}
}
return $schema;
}
......
......@@ -9,6 +9,7 @@
use Drupal\Core\Entity\ContentEntityTypeInterface;
use Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
/**
* Defines the block content schema handler.
......@@ -21,10 +22,6 @@ class BlockContentStorageSchema extends SqlContentEntityStorageSchema {
protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $reset = FALSE) {
$schema = parent::getEntitySchema($entity_type, $reset);
// Marking the respective fields as NOT NULL makes the indexes more
// performant.
$schema['block_content_field_data']['fields']['info']['not null'] = TRUE;
$schema['block_content_field_data']['unique keys'] += array(
'block_content__info' => array('info', 'langcode'),
);
......@@ -32,4 +29,24 @@ protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $res
return $schema;
}
/**
* {@inheritdoc}
*/
protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $storage_definition, $table_name, array $column_mapping) {
$schema = parent::getSharedTableFieldSchema($storage_definition, $table_name, $column_mapping);
$field_name = $storage_definition->getName();
if ($table_name == 'block_content_field_data') {
switch ($field_name) {
case 'info':
// Improves the performance of the block_content__info index defined
// in getEntitySchema().
$schema['fields'][$field_name]['not null'] = TRUE;
break;
}
}
return $schema;
}
}
......@@ -9,6 +9,7 @@
use Drupal\Core\Entity\ContentEntityTypeInterface;
use Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
/**
* Defines the comment schema handler.
......@@ -21,13 +22,6 @@ class CommentStorageSchema extends SqlContentEntityStorageSchema {
protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $reset = FALSE) {
$schema = parent::getEntitySchema($entity_type, $reset);
// Marking the respective fields as NOT NULL makes the indexes more
// performant.
$schema['comment_field_data']['fields']['created']['not null'] = TRUE;
$schema['comment_field_data']['fields']['thread']['not null'] = TRUE;
unset($schema['comment_field_data']['indexes']['comment_field__pid__target_id']);
unset($schema['comment_field_data']['indexes']['comment_field__entity_id__target_id']);
$schema['comment_field_data']['indexes'] += array(
'comment__status_pid' => array('pid', 'status'),
'comment__num_new' => array(
......@@ -45,16 +39,40 @@ protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $res
'comment_type',
'default_langcode',
),
'comment__created' => array('created'),
);
$schema['comment_field_data']['foreign keys'] += array(
'comment__author' => array(
'table' => 'users',
'columns' => array('uid' => 'uid'),
),
);
return $schema;
}
/**
* {@inheritdoc}
*/
protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $storage_definition, $table_name, array $column_mapping) {
$schema = parent::getSharedTableFieldSchema($storage_definition, $table_name, $column_mapping);
$field_name = $storage_definition->getName();
if ($table_name == 'comment_field_data') {
// Remove unneeded indexes.
unset($schema['indexes']['comment_field__pid__target_id']);
unset($schema['indexes']['comment_field__entity_id__target_id']);
switch ($field_name) {
case 'thread':
// Improves the performance of the comment__num_new index defined
// in getEntitySchema().
$schema['fields'][$field_name]['not null'] = TRUE;
break;
case 'created':
$this->addSharedTableFieldIndex($storage_definition, $schema, TRUE);
break;
case 'uid':
$this->addSharedTableFieldForeignKey($storage_definition, $schema, 'users', 'uid');
}
}
return $schema;
}
}
......@@ -13,15 +13,11 @@
function contact_storage_test_entity_base_field_info(\Drupal\Core\Entity\EntityTypeInterface $entity_type) {
if ($entity_type->id() == 'contact_message') {
$fields = array();
$fields['id'] = BaseFieldDefinition::create('integer')
->setLabel(t('Message ID'))
->setDescription(t('The message ID.'))
->setReadOnly(TRUE)
// Explicitly set this to 'contact' so that
// SqlContentEntityStorage::usesDedicatedTable() doesn't attempt to
// put the ID in a dedicated table.
// @todo Remove when https://www.drupal.org/node/1498720 is in.
->setProvider('contact')
->setSetting('unsigned', TRUE);
return $fields;
......
......@@ -424,11 +424,6 @@ public function getSchema() {
'foreign keys' => array(),
);
// Check that the schema does not include forbidden column names.
if (array_intersect(array_keys($schema['columns']), static::getReservedColumns())) {
throw new FieldException(String::format('Illegal field type @field_type on @field_name.', array('@field_type' => $this->type, '@field_name' => $this->name)));
}
// Merge custom indexes with those specified by the field type. Custom
// indexes prevail.
$schema['indexes'] = $this->indexes + $schema['indexes'];
......@@ -446,6 +441,13 @@ public function hasCustomStorage() {
return FALSE;
}
/**
* {@inheritdoc}
*/
public function isBaseField() {
return FALSE;
}
/**
* {@inheritdoc}
*/
......@@ -607,15 +609,6 @@ public function isQueryable() {
return TRUE;
}
/**
* A list of columns that can not be used as field type columns.
*
* @return array
*/
public static function getReservedColumns() {
return array('deleted');
}
/**
* Determines whether a field has any data.
*
......
......@@ -7,8 +7,8 @@
namespace Drupal\file;
use Drupal\Core\Entity\ContentEntityTypeInterface;
use Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
/**
* Defines the file schema handler.
......@@ -18,24 +18,25 @@ class FileStorageSchema extends SqlContentEntityStorageSchema {
/**
* {@inheritdoc}
*/
protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $reset = FALSE) {
$schema = parent::getEntitySchema($entity_type, $reset);
// Marking the respective fields as NOT NULL makes the indexes more
// performant.
$schema['file_managed']['fields']['status']['not null'] = TRUE;
$schema['file_managed']['fields']['changed']['not null'] = TRUE;
$schema['file_managed']['fields']['uri']['not null'] = TRUE;
// @todo There should be a 'binary' field type or setting.
$schema['file_managed']['fields']['uri']['binary'] = TRUE;
$schema['file_managed']['indexes'] += array(
'file__status' => array('status'),
'file__changed' => array('changed'),
);
$schema['file_managed']['unique keys'] += array(
'file__uri' => array('uri'),
);
protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $storage_definition, $table_name, array $column_mapping) {
$schema = parent::getSharedTableFieldSchema($storage_definition, $table_name, $column_mapping);
$field_name = $storage_definition->getName();
if ($table_name == 'file_managed') {
switch ($field_name) {
case 'status':
case 'changed':
$this->addSharedTableFieldIndex($storage_definition, $schema, TRUE);
break;
case 'uri':
$this->addSharedTableFieldUniqueKey($storage_definition, $schema, TRUE);
// @todo There should be a 'binary' field type or setting:
// https://www.drupal.org/node/2068655.
$schema['fields'][$field_name]['binary'] = TRUE;
break;
}
}
return $schema;
}
......
......@@ -9,6 +9,7 @@
use Drupal\Core\Entity\ContentEntityTypeInterface;
use Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
/**
* Defines the node schema handler.
......@@ -23,31 +24,11 @@ protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $res
// Marking the respective fields as NOT NULL makes the indexes more
// performant.
$schema['node_field_data']['fields']['changed']['not null'] = TRUE;
$schema['node_field_data']['fields']['created']['not null'] = TRUE;
$schema['node_field_data']['fields']['default_langcode']['not null'] = TRUE;
$schema['node_field_data']['fields']['promote']['not null'] = TRUE;
$schema['node_field_data']['fields']['status']['not null'] = TRUE;
$schema['node_field_data']['fields']['sticky']['not null'] = TRUE;
$schema['node_field_data']['fields']['title']['not null'] = TRUE;
$schema['node_field_revision']['fields']['default_langcode']['not null'] = TRUE;
// @todo Revisit index definitions in https://drupal.org/node/2015277.
$schema['node_revision']['indexes'] += array(
'node__langcode' => array('langcode'),
);
$schema['node_revision']['foreign keys'] += array(
'node__revision_author' => array(
'table' => 'users',
'columns' => array('revision_uid' => 'uid'),
),
);
$schema['node_field_data']['indexes'] += array(
'node__changed' => array('changed'),
'node__created' => array('created'),
'node__default_langcode' => array('default_langcode'),
'node__langcode' => array('langcode'),
'node__frontpage' => array('promote', 'status', 'sticky', 'created'),
'node__status_type' => array('status', 'type', 'nid'),
'node__title_type' => array('title', array('type', 4)),
......@@ -55,10 +36,59 @@ protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $res
$schema['node_field_revision']['indexes'] += array(
'node__default_langcode' => array('default_langcode'),
'node__langcode' => array('langcode'),
);
return $schema;
}
/**
* {@inheritdoc}
*/
protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $storage_definition, $table_name, array $column_mapping) {
$schema = parent::getSharedTableFieldSchema($storage_definition, $table_name, $column_mapping);
$field_name = $storage_definition->getName();
if ($table_name == 'node_revision') {
switch ($field_name) {
case 'langcode':
$this->addSharedTableFieldIndex($storage_definition, $schema, TRUE);
break;
case 'revision_uid':
$this->addSharedTableFieldForeignKey($storage_definition, $schema, 'users', 'uid');
break;
}
}
if ($table_name == 'node_field_data') {
switch ($field_name) {
case 'promote':
case 'status':
case 'sticky':
case 'title':
// Improves the performance of the indexes defined
// in getEntitySchema().
$schema['fields'][$field_name]['not null'] = TRUE;
break;
case 'changed':
case 'created':
case 'langcode':
// @todo Revisit index definitions: https://drupal.org/node/2015277.
$this->addSharedTableFieldIndex($storage_definition, $schema, TRUE);
break;
}
}
if ($table_name == 'node_field_revision') {
switch ($field_name) {
case 'langcode':
$this->addSharedTableFieldIndex($storage_definition, $schema, TRUE);
break;
}
}
return $schema;
}
}
......@@ -14,6 +14,13 @@
*/
class EntityBundleFieldTest extends EntityUnitTestBase {
/**
* Modules to enable.
*
* @var array
*/
public static $modules = array('entity_schema_test');
/**
* The module handler.
*
......@@ -39,59 +46,42 @@ protected function setUp() {
$this->database = $this->container->get('database');
}
/**
* Tests the custom bundle field creation and deletion.
*/
public function testCustomBundleFieldCreateDelete() {
// Install the module which adds the field.
$this->moduleHandler->install(array('entity_bundle_field_test'), FALSE);
$definition = $this->entityManager->getFieldDefinitions('entity_test', 'custom')['custom_field'];
$this->assertNotNull($definition, 'Field definition found.');
// Make sure the table has been created.
/** @var \Drupal\Core\Entity\Sql\DefaultTableMapping $table_mapping */
$table_mapping = $this->entityManager->getStorage('entity_test')->getTableMapping();
$table = $table_mapping->getDedicatedDataTableName($definition);
$this->assertTrue($this->database->schema()->tableExists($table), 'Table created');
$this->moduleHandler->uninstall(array('entity_bundle_field_test'), FALSE);
$this->assertFalse($this->database->schema()->tableExists($table), 'Table dropped');
}
/**
* Tests making use of a custom bundle field.
*/
public function testCustomBundleFieldUsage() {
entity_test_create_bundle('custom');
// Check that an entity with bundle entity_test does not have the custom
// field.
$this->moduleHandler->install(array('entity_bundle_field_test'), FALSE);
$storage = $this->entityManager->getStorage('entity_test');
$entity = $storage->create([
'type' => 'entity_test',