From bcfde9e15913b6b668c3e76f06287f2df93f48ca Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Sat, 13 Dec 2014 16:03:32 +0100
Subject: [PATCH] Issue #2392209 by plach:
DefaultTableMapping::getFieldColumName is broken for base tables
---
.../Core/Entity/Sql/DefaultTableMapping.php | 40 +++++----
.../Sql/SqlContentEntityStorageException.php | 16 ++++
.../Core/Entity/Sql/TableMappingInterface.php | 8 +-
.../Entity/Sql/DefaultTableMappingTest.php | 86 ++++++++++++++++++-
.../Sql/SqlContentEntityStorageTest.php | 2 +-
5 files changed, 129 insertions(+), 23 deletions(-)
create mode 100644 core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageException.php
diff --git a/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php b/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php
index f992081ad581..c63ca4d82504 100644
--- a/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php
+++ b/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php
@@ -7,6 +7,7 @@
namespace Drupal\Core\Entity\Sql;
+use Drupal\Component\Utility\String;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
/**
@@ -130,20 +131,34 @@ public function getFieldNames($table_name) {
*/
public function getColumnNames($field_name) {
if (!isset($this->columnMapping[$field_name])) {
- $column_names = array_keys($this->fieldStorageDefinitions[$field_name]->getColumns());
- if (count($column_names) == 1) {
- $this->columnMapping[$field_name] = array(reset($column_names) => $field_name);
- }
- else {
- $this->columnMapping[$field_name] = array();
- foreach ($column_names as $column_name) {
- $this->columnMapping[$field_name][$column_name] = $field_name . '__' . $column_name;
- }
+ $this->columnMapping[$field_name] = array();
+ $storage_definition = $this->fieldStorageDefinitions[$field_name];
+ foreach (array_keys($this->fieldStorageDefinitions[$field_name]->getColumns()) as $property_name) {
+ $this->columnMapping[$field_name][$property_name] = $this->getFieldColumnName($storage_definition, $property_name);
}
}
return $this->columnMapping[$field_name];
}
+ /**
+ * {@inheritdoc}
+ */
+ public function getFieldColumnName(FieldStorageDefinitionInterface $storage_definition, $property_name) {
+ $field_name = $storage_definition->getName();
+
+ if ($this->allowsSharedTableStorage($storage_definition)) {
+ $column_name = count($storage_definition->getColumns()) == 1 ? $field_name : $field_name . '__' . $property_name;
+ }
+ elseif ($this->requiresDedicatedTableStorage($storage_definition)) {
+ $column_name = !in_array($property_name, $this->getReservedColumns()) ? $field_name . '_' . $property_name : $property_name;
+ }
+ else {
+ throw new SqlContentEntityStorageException(String::format('Column information not available for the "@field_name" field.', array('@field_name' => $field_name)));
+ }
+
+ return $column_name;
+ }
+
/**
* Adds field columns for a table to the table mapping.
*
@@ -319,11 +334,4 @@ protected function generateFieldTableName(FieldStorageDefinitionInterface $stora
return $table_name;
}
- /**
- * {@inheritdoc}
- */
- public function getFieldColumnName(FieldStorageDefinitionInterface $storage_definition, $column) {
- return in_array($column, $this->getReservedColumns()) ? $column : $storage_definition->getName() . '_' . $column;
- }
-
}
diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageException.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageException.php
new file mode 100644
index 000000000000..61f610e407ce
--- /dev/null
+++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageException.php
@@ -0,0 +1,16 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Entity\Sql\SqlContentEntityStorageSchemaException.
+ */
+
+namespace Drupal\Core\Entity\Sql;
+
+use Drupal\Core\Entity\EntityStorageException;
+
+/**
+ * Exception thrown when a SQL storage operation fails.
+ */
+class SqlContentEntityStorageException extends EntityStorageException {
+}
diff --git a/core/lib/Drupal/Core/Entity/Sql/TableMappingInterface.php b/core/lib/Drupal/Core/Entity/Sql/TableMappingInterface.php
index 9607fe4073b6..19a45b1e7af1 100644
--- a/core/lib/Drupal/Core/Entity/Sql/TableMappingInterface.php
+++ b/core/lib/Drupal/Core/Entity/Sql/TableMappingInterface.php
@@ -90,16 +90,16 @@ public function getExtraColumns($table_name);
public function getReservedColumns();
/**
- * Generates a column name for a field.
+ * Generates a column name for a field property.
*
* @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage_definition
* The field storage definition.
- * @param string $column
- * The name of the column.
+ * @param string $property_name
+ * The name of the property.
*
* @return string
* A string containing a generated column name for a field data table that is
* unique among all other fields.
*/
- public function getFieldColumnName(FieldStorageDefinitionInterface $storage_definition, $column);
+ public function getFieldColumnName(FieldStorageDefinitionInterface $storage_definition, $property_name);
}
diff --git a/core/tests/Drupal/Tests/Core/Entity/Sql/DefaultTableMappingTest.php b/core/tests/Drupal/Tests/Core/Entity/Sql/DefaultTableMappingTest.php
index 007efc35e805..883819095c2a 100644
--- a/core/tests/Drupal/Tests/Core/Entity/Sql/DefaultTableMappingTest.php
+++ b/core/tests/Drupal/Tests/Core/Entity/Sql/DefaultTableMappingTest.php
@@ -7,6 +7,7 @@
namespace Drupal\Tests\Core\Entity\Sql;
use Drupal\Core\Entity\Sql\DefaultTableMapping;
+use Drupal\Core\Entity\Sql\SqlContentEntityStorageException;
use Drupal\Tests\UnitTestCase;
/**
@@ -234,6 +235,87 @@ public function testGetExtraColumns() {
$this->assertSame($expected, $table_mapping->getExtraColumns('foo'));
}
+ /**
+ * Tests DefaultTableMapping::getFieldColumnName() with valid parameters.
+ *
+ * @param bool $base_field
+ * Flag indicating whether the field should be treated as a base or bundle
+ * field.
+ * @param string[] $columns
+ * An array of available field column names.
+ * @param string $column
+ * The name of the column to be processed.
+ * @param string $expected
+ * The expected result.
+ *
+ * @covers ::getFieldColumnName
+ *
+ * @dataProvider providerTestGetFieldColumnName
+ */
+ public function testGetFieldColumnName($base_field, $columns, $column, $expected) {
+ $definitions['test'] = $this->setUpDefinition('test', $columns, $base_field);
+ $table_mapping = new DefaultTableMapping($definitions);
+ $result = $table_mapping->getFieldColumnName($definitions['test'], $column);
+ $this->assertEquals($expected, $result);
+ }
+
+ /**
+ * Tests DefaultTableMapping::getFieldColumnName() with invalid parameters.
+ *
+ * @param bool $base_field
+ * Flag indicating whether the field should be treated as a base or bundle
+ * field.
+ * @param string[] $columns
+ * An array of available field column names.
+ * @param string $column
+ * The name of the column to be processed.
+ *
+ * @expectedException \Drupal\Core\Entity\Sql\SqlContentEntityStorageException
+ * @expectedExceptionMessage Column information not available for the "test" field.
+ *
+ * @covers ::getFieldColumnName
+ *
+ * @dataProvider providerTestGetFieldColumnName
+ */
+ public function testGetFieldColumnNameInvalid($base_field, $columns, $column) {
+ $definitions['test'] = $this->setUpDefinition('test', $columns, $base_field);
+
+ // Mark field storage definition as custom storage.
+ $definitions['test']->expects($this->any())
+ ->method('hasCustomStorage')
+ ->willReturn(TRUE);
+
+ $table_mapping = new DefaultTableMapping($definitions);
+ $table_mapping->getFieldColumnName($definitions['test'], $column);
+ }
+
+ /**
+ * Provides test data for testGetFieldColumnName().
+ *
+ * @return array[]
+ * An nested array where each inner array has the following values: test
+ * field name, base field status, list of field columns, name of the column
+ * to be retrieved, expected result, whether an exception is expected.
+ */
+ public function providerTestGetFieldColumnName() {
+ $data = [];
+ // Base field with single column.
+ $data[] = [TRUE, ['foo'], 'foo', 'test'];
+
+ // Base field with multiple columns.
+ $data[] = [TRUE, ['foo', 'bar'], 'foo', 'test__foo'];
+ $data[] = [TRUE, ['foo', 'bar'], 'bar', 'test__bar'];
+ // Bundle field with single column.
+ $data[] = [FALSE, ['foo'], 'foo', 'test_foo'];
+ // Bundle field with multiple columns.
+ $data[] = [FALSE, ['foo', 'bar'], 'foo', 'test_foo'];
+ $data[] = [FALSE, ['foo', 'bar'], 'bar', 'test_bar'];
+ // Bundle field with reserved column.
+ $data[] = [FALSE, ['foo', 'bar'], 'deleted', 'deleted'];
+
+ return $data;
+ }
+
/**
* Sets up a field storage definition for the test.
*
@@ -244,11 +326,11 @@ public function testGetExtraColumns() {
*
* @return \Drupal\Core\Field\FieldStorageDefinitionInterface|\PHPUnit_Framework_MockObject_MockObject
*/
- protected function setUpDefinition($name, array $column_names) {
+ protected function setUpDefinition($name, array $column_names, $base_field = TRUE) {
$definition = $this->getMock('Drupal\Tests\Core\Field\TestBaseFieldDefinitionInterface');
$definition->expects($this->any())
->method('isBaseField')
- ->will($this->returnValue(TRUE));
+ ->willReturn($base_field);
$definition->expects($this->any())
->method('getName')
->will($this->returnValue($name));
diff --git a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageTest.php b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageTest.php
index 3a37f909b4a1..cfa5cd9061e1 100644
--- a/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageTest.php
+++ b/core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageTest.php
@@ -281,7 +281,7 @@ public function testOnEntityTypeCreate() {
);
$this->fieldDefinitions = $this->mockFieldDefinitions(array('id'));
- $this->fieldDefinitions['id']->expects($this->once())
+ $this->fieldDefinitions['id']->expects($this->any())
->method('getColumns')
->will($this->returnValue($columns));
$this->fieldDefinitions['id']->expects($this->once())
--
GitLab