diff --git a/core/lib/Drupal/Core/Entity/ContentEntityDatabaseStorage.php b/core/lib/Drupal/Core/Entity/ContentEntityDatabaseStorage.php index 076615c..746be1f 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityDatabaseStorage.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityDatabaseStorage.php @@ -14,6 +14,7 @@ use Drupal\Core\Entity\Schema\ContentEntitySchemaHandlerInterface; use Drupal\Core\Entity\Sql\DefaultTableMapping; use Drupal\Core\Entity\Sql\SqlStorageInterface; +use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Field\FieldStorageDefinitionInterface; use Drupal\Core\Language\Language; use Drupal\field\FieldInfo; @@ -185,19 +186,11 @@ public function __construct(EntityTypeInterface $entity_type, Connection $databa $this->database = $database; $this->fieldInfo = $field_info; $this->entityManager = $entity_manager; - $this->storageDefinitions = $entity_manager->getFieldStorageDefinitions($entity_type->id(), FALSE); // @todo Remove the check for FieldDefinitionInterface::isMultiple() when // multiple-value base fields are supported in // https://drupal.org/node/2248977. - // @todo Configurable fields currently have a storage that supports multiple - // values even when their cardinality is set to 1. Therefore the - // FieldDefinitionInterface::isMultiple() check is not sufficient to - // filter them out. From the perspective of the storage - // FieldConfig::isMultiple() should always return TRUE. This cannot be - // implemented that way, however, as the same method is used to decide - // whether or not to display a multiple widget on forms. - $this->storageDefinitions = array_filter($this->storageDefinitions, function (FieldStorageDefinitionInterface $definition) { - return !$definition->isMultiple() && !($definition instanceof FieldConfigInterface); + $this->storageDefinitions = array_filter($entity_manager->getBaseFieldDefinitions($entity_type->id()), function (FieldDefinitionInterface $definition) { + return !$definition->isComputed() && !$definition->hasCustomStorage() && !$definition->isMultiple(); }); $this->initTableLayout(); @@ -521,10 +514,10 @@ protected function attachPropertyData(array &$entities) { $table_mapping = $this->getTableMapping(); $translations = array(); if ($this->revisionDataTable) { - $data_column_mapping = array_diff_key($table_mapping->getFieldColumns($this->revisionDataTable), $table_mapping->getFieldColumns($this->baseTable)); + $data_fields = array_diff_key($table_mapping->getFieldNames($this->revisionDataTable), $table_mapping->getFieldNames($this->baseTable)); } else { - $data_column_mapping = $table_mapping->getFieldColumns($this->dataTable); + $data_fields = $table_mapping->getFieldNames($this->dataTable); } foreach ($data as $values) { @@ -536,7 +529,8 @@ protected function attachPropertyData(array &$entities) { $translations[$id][$langcode] = TRUE; - foreach ($data_column_mapping as $field_name => $columns) { + foreach ($data_fields as $field_name) { + $columns = $table_mapping->getColumnMapping($field_name); // Do not key single-column fields by property name. if (count($columns) == 1) { $entities[$id][$field_name][$langcode] = $values[reset($columns)]; @@ -915,7 +909,9 @@ protected function mapToStorageRecord(ContentEntityInterface $entity, $table_nam } $record = new \stdClass(); - foreach ($this->getTableMapping()->getFieldColumns($table_name) as $field_name => $columns) { + $table_mapping = $this->getTableMapping(); + foreach ($table_mapping->getFieldNames($table_name) as $field_name) { + $columns = $table_mapping->getColumnMapping($field_name); if (!empty($this->storageDefinitions[$field_name])) { $definition = $this->storageDefinitions[$field_name]; foreach ($columns as $column_name => $schema_name) { diff --git a/core/lib/Drupal/Core/Entity/EntityManager.php b/core/lib/Drupal/Core/Entity/EntityManager.php index 125f42c..7385ae3 100644 --- a/core/lib/Drupal/Core/Entity/EntityManager.php +++ b/core/lib/Drupal/Core/Entity/EntityManager.php @@ -495,12 +495,12 @@ protected function buildBundleFieldDefinitions($entity_type_id, $bundle, array $ /** * {@inheritdoc} */ - public function getFieldStorageDefinitions($entity_type_id, $include_custom = TRUE) { + public function getFieldStorageDefinitions($entity_type_id) { if (!isset($this->fieldStorageDefinitions[$entity_type_id])) { $this->fieldStorageDefinitions[$entity_type_id] = array(); // Add all non-computed base fields. foreach ($this->getBaseFieldDefinitions($entity_type_id) as $field_name => $definition) { - if (!$definition->isComputed() && ($include_custom || !$definition->hasCustomStorage())) { + if (!$definition->isComputed()) { $this->fieldStorageDefinitions[$entity_type_id][$field_name] = $definition; } } @@ -511,7 +511,7 @@ public function getFieldStorageDefinitions($entity_type_id, $include_custom = TR } else { // Rebuild the definitions and put it into the cache. - $field_storage_definitions = $this->buildFieldStorageDefinitions($entity_type_id, $include_custom); + $field_storage_definitions = $this->buildFieldStorageDefinitions($entity_type_id); $this->cache->set($cid, $field_storage_definitions, Cache::PERMANENT, array('entity_types' => TRUE, 'entity_field_info' => TRUE)); } $this->fieldStorageDefinitions[$entity_type_id] += $field_storage_definitions; @@ -525,32 +525,27 @@ public function getFieldStorageDefinitions($entity_type_id, $include_custom = TR * @param string $entity_type_id * The entity type ID. Only entity types that implement * \Drupal\Core\Entity\ContentEntityInterface are supported - * @param bool $include_custom - * Whether or not to include storage definitions that have a custom storage. * * @return \Drupal\Core\Field\FieldStorageDefinitionInterface[] * An array of field storage definitions, keyed by field name. */ - protected function buildFieldStorageDefinitions($entity_type_id, $include_custom = TRUE) { + protected function buildFieldStorageDefinitions($entity_type_id) { $entity_type = $this->getDefinition($entity_type_id); $field_definitions = array(); // Retrieve base field definitions from modules. foreach ($this->moduleHandler->getImplementations('entity_field_storage_info') as $module) { - /** @var \Drupal\Core\Field\FieldStorageDefinitionInterface[] $module_definitions */ $module_definitions = $this->moduleHandler->invoke($module, 'entity_field_storage_info', array($entity_type)); if (!empty($module_definitions)) { // Ensure the provider key actually matches the name of the provider // defining the field. foreach ($module_definitions as $field_name => $definition) { - if ($include_custom || !$definition->hasCustomStorage()) { - // @todo Remove this check once FieldDefinitionInterface exposes a - // proper provider setter. See https://drupal.org/node/2225961. - if ($definition instanceof FieldDefinition) { - $definition->setProvider($module); - } - $field_definitions[$field_name] = $definition; + // @todo Remove this check once FieldDefinitionInterface exposes a + // proper provider setter. See https://drupal.org/node/2225961. + if ($definition instanceof FieldDefinition) { + $definition->setProvider($module); } + $field_definitions[$field_name] = $definition; } } } diff --git a/core/lib/Drupal/Core/Entity/EntityManagerInterface.php b/core/lib/Drupal/Core/Entity/EntityManagerInterface.php index 8e1b267..7ee0255 100644 --- a/core/lib/Drupal/Core/Entity/EntityManagerInterface.php +++ b/core/lib/Drupal/Core/Entity/EntityManagerInterface.php @@ -67,9 +67,6 @@ public function getFieldDefinitions($entity_type_id, $bundle); * * @param string $entity_type_id * The entity type ID. Only content entities are supported. - * @param bool $include_custom - * (optional) Whether or not to include storage definitions that have a - * custom storage. Defaults to TRUE. * * @return \Drupal\Core\Field\FieldStorageDefinitionInterface[] * The array of field storage definitions for the entity type, keyed by @@ -77,7 +74,7 @@ public function getFieldDefinitions($entity_type_id, $bundle); * * @see \Drupal\Core\Field\FieldStorageDefinitionInterface */ - public function getFieldStorageDefinitions($entity_type_id, $include_custom = TRUE); + public function getFieldStorageDefinitions($entity_type_id); /** * Creates a new access controller instance. diff --git a/core/lib/Drupal/Core/Entity/Schema/ContentEntitySchemaHandler.php b/core/lib/Drupal/Core/Entity/Schema/ContentEntitySchemaHandler.php index 42c5c92..81c50bf 100644 --- a/core/lib/Drupal/Core/Entity/Schema/ContentEntitySchemaHandler.php +++ b/core/lib/Drupal/Core/Entity/Schema/ContentEntitySchemaHandler.php @@ -82,7 +82,8 @@ public function getSchema() { $table_mapping = $this->storage->getTableMapping(); foreach ($table_mapping->getTableNames() as $table_name) { // Add the schema from field definitions. - foreach ($table_mapping->getFieldColumns($table_name) as $field_name => $column_names) { + foreach ($table_mapping->getFieldNames($table_name) as $field_name) { + $column_names = $table_mapping->getColumnMapping($field_name); $this->addFieldSchema($schema[$table_name], $field_name, $column_names); } diff --git a/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php b/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php index ac5465f..0fa6e92 100644 --- a/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php +++ b/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php @@ -16,21 +16,21 @@ class DefaultTableMapping implements TableMappingInterface { * * @var \Drupal\Core\Field\FieldStorageDefinitionInterface[] */ - protected $storageDefinitions; + protected $storageDefinitions = array(); /** - * A mapping of entity field names to the respective database columns. + * A list of field names per table. * * This corresponds to the return value of - * TableMappingInterface::getFieldColumns() except that this variable is + * TableMappingInterface::getFieldNames() except that this variable is * additionally keyed by table name. * * @var array[] */ - protected $fieldColumns = array(); + protected $fieldNames = array(); /** - * A list of database columns which store denormalized data. + * A list of database columns which store denormalized data per table. * * This corresponds to the return value of * TableMappingInterface::getExtraColumns() except that this variable is @@ -41,6 +41,17 @@ class DefaultTableMapping implements TableMappingInterface { protected $extraColumns = array(); /** + * A list of all database columns per table. + * + * This data is derived from static::$storageDefinitions, static::$fieldNames, + * and static::$extraColumns, but is stored separately to avoid repeated + * processing. + * + * @var array[] + */ + protected $allColumns = array(); + + /** * Constructs a DefaultTableMapping. * * @param \Drupal\Core\Field\FieldStorageDefinitionInterface[] $storage_definitions @@ -55,28 +66,53 @@ public function __construct(array $storage_definitions) { * {@inheritdoc} */ public function getTableNames() { - return array_unique(array_merge(array_keys($this->fieldColumns), array_keys($this->extraColumns))); + return array_unique(array_merge(array_keys($this->fieldNames), array_keys($this->extraColumns))); } /** * {@inheritdoc} */ public function getAllColumns($table_name) { - $columns = call_user_func_array('array_merge', array_map('array_values', $this->getFieldColumns($table_name))); - return array_merge($columns, $this->getExtraColumns($table_name)); + if (!isset($this->allColumns[$table_name])) { + $this->allColumns[$table_name] = array(); + + foreach ($this->getFieldNames($table_name) as $field_name) { + $this->allColumns[$table_name] = array_merge($this->allColumns[$table_name], array_values($this->getColumnMapping($field_name))); + } + + $this->allColumns[$table_name] = array_merge($this->allColumns[$table_name], $this->getExtraColumns($table_name)); + } + return $this->allColumns[$table_name]; } /** * {@inheritdoc} */ - public function getFieldColumns($table_name) { - if (isset($this->fieldColumns[$table_name])) { - return $this->fieldColumns[$table_name]; + public function getFieldNames($table_name) { + if (isset($this->fieldNames[$table_name])) { + return $this->fieldNames[$table_name]; } return array(); } /** + * {@inheritdoc} + */ + public function getColumnMapping($field_name) { + $column_names = array_keys($this->storageDefinitions[$field_name]->getColumns()); + if (count($column_names) == 1) { + $mapping = array(reset($column_names) => $field_name); + } + else { + $mapping = array(); + foreach ($column_names as $column_name) { + $mapping[$column_name] = $field_name . '__' . $column_name; + } + } + return $mapping; + } + + /** * Adds field columns for a table to the table mapping. * * @param string $table_name @@ -87,18 +123,7 @@ public function getFieldColumns($table_name) { * @return $this */ public function addFieldColumns($table_name, array $field_names) { - foreach ($field_names as $field_name) { - $column_names = array_keys($this->storageDefinitions[$field_name]->getColumns()); - if (count($column_names) == 1) { - $this->fieldColumns[$table_name][$field_name] = array(reset($column_names) => $field_name); - } - else { - foreach ($column_names as $column_name) { - $this->fieldColumns[$table_name][$field_name][$column_name] = $field_name . '__' . $column_name; - } - } - } - + $this->fieldNames[$table_name] = $field_names; return $this; } diff --git a/core/lib/Drupal/Core/Entity/Sql/TableMappingInterface.php b/core/lib/Drupal/Core/Entity/Sql/TableMappingInterface.php index 1107122..69e42d9 100644 --- a/core/lib/Drupal/Core/Entity/Sql/TableMappingInterface.php +++ b/core/lib/Drupal/Core/Entity/Sql/TableMappingInterface.php @@ -32,43 +32,28 @@ public function getTableNames(); public function getAllColumns($table_name); /** - * Returns a mapping of entity field names to the respective database columns. + * Returns a list of entity field names for the given table. * - * For a StringItem field, for example, (a single-column field) named 'title' - * DefaultTableMapping::getFieldColumns() would return: - * @code - * array( - * 'title' => array( - * 'value' => 'title', - * ), - * ), - * @endcode - * - * For a TextItem field (a multi-column field) named 'description' - * DefaultTableMapping::getFieldColumns() would return: - * @code - * array( - * 'description' => array( - * 'value' => 'description__value', - * 'format' => 'description__format', - * ), - * ), - * @endcode + * @param string $table_name + * The name of the table to return the field names for. * - * The values of the inner array ('title' for the single-column field and - * 'description__value', and 'description__format' for the multi-column field - * in the example above) depend on the table mapping implementation. + * @return string[] + * An array of field names for the given table. + */ + public function getFieldNames($table_name); + + /** + * Returns a mapping of field columns to database columns for a given field. * - * @param string $table_name - * The name of the table to return the columns for. + * @param string $field_name + * The name of the entity field to return the column mapping for. * - * @return array[] - * An array where the keys are the names of the entity fields and the values - * are in turn arrays whose keys are the names of the columns as specified - * in the field item's schema() method and the values are the respective - * database column names for the respective entity fields. + * @return string[] + * The keys of this array are the keys of the array returned by + * FieldStorageDefinitionInterface::getColumns() while the respective values + * are the names of the database columns for this table mapping. */ - public function getFieldColumns($table_name); + public function getColumnMapping($field_name); /** * Returns a list of database columns which store denormalized data. diff --git a/core/tests/Drupal/Tests/Core/Entity/Schema/ContentEntitySchemaHandlerTest.php b/core/tests/Drupal/Tests/Core/Entity/Schema/ContentEntitySchemaHandlerTest.php index 390fc57..7c5cd52 100644 --- a/core/tests/Drupal/Tests/Core/Entity/Schema/ContentEntitySchemaHandlerTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/Schema/ContentEntitySchemaHandlerTest.php @@ -43,6 +43,13 @@ class ContentEntitySchemaHandlerTest extends UnitTestCase { protected $storage; /** + * A list of table names to use for the test. + * + * @var string[] + */ + protected $tableNames; + + /** * The mocked field definitions used in this test. * * @var \Drupal\Core\Field\FieldStorageDefinitionInterface[]|\PHPUnit_Framework_MockObject_MockObject[] @@ -93,44 +100,41 @@ public function setUp() { * This tests that the schema is generated correctly for non-revisionable, * non-translatable entities. * + * @param bool $uuid_key + * Whether or not the tested entity type should have a UUID key. + * * @covers ::getSchema + * + * @dataProvider providerTestGetSchemaLayoutBase */ - public function testGetSchemaLayoutBase() { + public function testGetSchemaLayoutBase($uuid_key) { $this->entityType->expects($this->any()) ->method('getKey') ->will($this->returnValueMap(array( array('id', 'id'), - array('uuid', NULL), + array('uuid', $uuid_key ? 'uuid' : NULL), ))); $this->storage->expects($this->once()) ->method('getTableLayout') ->will($this->returnValue(ContentEntityDatabaseStorage::LAYOUT_BASE)); - $this->setupBaseLayoutFieldDefinitions(); + $this->setUpBaseLayoutFieldDefinitions(); + if ($uuid_key) { + $this->setUpStorageDefinition('uuid', array( + 'columns' => array( + 'value' => array( + 'type' => 'varchar', + 'length' => 128, + ), + ), + )); + } - $this->setupSchemaHandler(); + $this->setUpSchemaHandler(); - $table_mapping = $this->getMock('Drupal\Core\Entity\Sql\TableMappingInterface'); - $table_mapping->expects($this->once()) - ->method('getTableNames') - ->will($this->returnValue(array('entity_test'))); - $table_mapping->expects($this->once()) - ->method('getFieldColumns') - ->with('entity_test') - ->will($this->returnValue(array( - 'id' => array('value' => 'id'), - 'name' => array('value' => 'name'), - 'type' => array('value' => 'type'), - ))); - $table_mapping->expects($this->once()) - ->method('getExtraColumns') - ->with('entity_test') - ->will($this->returnValue(array())); - - $this->storage->expects($this->once()) - ->method('getTableMapping') - ->will($this->returnValue($table_mapping)); + $this->tableNames = array('entity_test'); + $this->setUpTableMapping(); $expected = array( 'entity_test' => array( @@ -159,112 +163,41 @@ public function testGetSchemaLayoutBase() { 'foreign keys' => array(), ), ); + if ($uuid_key) { + $expected['entity_test']['fields']['uuid'] = array( + 'type' => 'varchar', + 'length' => 128, + 'description' => 'The uuid field.', + 'not null' => TRUE, + ); + $expected['entity_test']['unique keys']['entity_test__uuid'] = array('uuid'); + } $actual = $this->schemaHandler->getSchema(); $this->assertEquals($expected, $actual); } /** - * Tests ContentEntitySchemaHandler::getSchema() with a basic table layout. + * Provides data for testGetSchemaLayoutBase(). * - * This tests that the schema is generated correctly for non-revisionable, - * non-translatable entities with a UUID key. - * - * @covers ::getSchema + * @return array + * Returns a nested array where each inner array returns a boolean, + * indicating whether or not the tested entity type should include a UUID + * key. */ - public function testGetSchemaLayoutBaseWithUuid() { - $this->entityType->expects($this->any()) - ->method('getKey') - ->will($this->returnValueMap(array( - array('id', 'id'), - array('uuid', 'uuid'), - ))); - - $this->storage->expects($this->once()) - ->method('getTableLayout') - ->will($this->returnValue(ContentEntityDatabaseStorage::LAYOUT_BASE)); - - $this->setupBaseLayoutFieldDefinitions(); - $this->setupStorageDefinition('uuid', array( - 'columns' => array( - 'value' => array( - 'type' => 'varchar', - 'length' => 128, - ), - ), - )); - - $this->setupSchemaHandler(); - - $table_mapping = $this->getMock('Drupal\Core\Entity\Sql\TableMappingInterface'); - $table_mapping->expects($this->once()) - ->method('getTableNames') - ->will($this->returnValue(array('entity_test'))); - $table_mapping->expects($this->once()) - ->method('getFieldColumns') - ->with('entity_test') - ->will($this->returnValue(array( - 'id' => array('value' => 'id'), - 'uuid' => array('value' => 'uuid'), - 'name' => array('value' => 'name'), - 'type' => array('value' => 'type'), - ))); - $table_mapping->expects($this->once()) - ->method('getExtraColumns') - ->with('entity_test') - ->will($this->returnValue(array())); - - $this->storage->expects($this->once()) - ->method('getTableMapping') - ->will($this->returnValue($table_mapping)); - - $expected = array( - 'entity_test' => array( - 'description' => 'The base table for entity_test entities.', - 'fields' => array( - 'id' => array( - 'type' => 'serial', - 'description' => 'The id field.', - 'not null' => TRUE, - ), - 'name' => array( - 'type' => 'varchar', - 'length' => 255, - 'description' => 'The name field.', - 'not null' => FALSE, - ), - 'type' => array( - 'type' => 'varchar', - 'length' => 32, - 'description' => 'The type field.', - 'not null' => FALSE, - ), - 'uuid' => array( - 'type' => 'varchar', - 'length' => 128, - 'description' => 'The uuid field.', - 'not null' => TRUE, - ), - ), - 'primary key' => array('id'), - 'unique keys' => array( - 'entity_test__uuid' => array('uuid'), - ), - 'indexes' => array(), - 'foreign keys' => array(), - ), + public function providerTestGetSchemaLayoutBase() { + return array( + array(FALSE), + array(TRUE), ); - $actual = $this->schemaHandler->getSchema(); - - $this->assertEquals($expected, $actual); } /** * Sets up the field definitions that are used for the base storage layout. */ - protected function setupBaseLayoutFieldDefinitions() { + protected function setUpBaseLayoutFieldDefinitions() { // @see \Drupal\Core\Field\Plugin\Field\FieldType\IntegerItem::schema() - $this->setupStorageDefinition('id', array( + $this->setUpStorageDefinition('id', array( 'columns' => array( 'value' => array( 'type' => 'int', @@ -272,7 +205,7 @@ protected function setupBaseLayoutFieldDefinitions() { ), )); // @see \Drupal\Core\Field\Plugin\Field\FieldType\StringItem::schema() - $this->setupStorageDefinition('name', array( + $this->setUpStorageDefinition('name', array( 'columns' => array( 'value' => array( 'type' => 'varchar', @@ -281,7 +214,7 @@ protected function setupBaseLayoutFieldDefinitions() { ), )); // @see \Drupal\Core\Field\Plugin\Field\FieldType\StringItem::schema() - $this->setupStorageDefinition('type', array( + $this->setUpStorageDefinition('type', array( 'columns' => array( 'value' => array( 'type' => 'varchar', @@ -296,7 +229,7 @@ protected function setupBaseLayoutFieldDefinitions() { * * This uses the field definitions set in $this->fieldDefinitions. */ - public function setupSchemaHandler() { + public function setUpSchemaHandler() { $this->entityManager->expects($this->once()) ->method('getFieldStorageDefinitions') ->with('entity_test') @@ -309,6 +242,40 @@ public function setupSchemaHandler() { } /** + * Sets up the table mapping. + * + * This uses the field definitions set in static::$tablesNames, + * static::$fieldDefinitions. + */ + public function setUpTableMapping() { + $field_names = array_keys($this->storageDefinitions); + + $table_mapping = $this->getMock('Drupal\Core\Entity\Sql\TableMappingInterface'); + $table_mapping->expects($this->once()) + ->method('getTableNames') + ->will($this->returnValue($this->tableNames)); + + $table_mapping->expects($this->once()) + ->method('getFieldNames') + ->with(reset($this->tableNames)) + ->will($this->returnValue($field_names)); + $table_mapping->expects($this->any()) + ->method('getColumnMapping') + ->will($this->returnCallback(function ($field_name) { + return array('value' => $field_name); + })); + + $table_mapping->expects($this->once()) + ->method('getExtraColumns') + ->with('entity_test') + ->will($this->returnValue(array())); + + $this->storage->expects($this->once()) + ->method('getTableMapping') + ->will($this->returnValue($table_mapping)); + } + + /** * Sets up a field definition. * * @param string $field_name @@ -317,7 +284,7 @@ public function setupSchemaHandler() { * The schema array of the field definition, as returned from * FieldDefinitionInterface::schema(). */ - public function setupStorageDefinition($field_name, array $schema) { + public function setUpStorageDefinition($field_name, array $schema) { $this->storageDefinitions[$field_name] = $this->getMock('Drupal\Core\Field\FieldStorageDefinitionInterface'); $this->storageDefinitions[$field_name]->expects($this->once()) ->method('getDescription')