From 6bf02028a8a55182fe4bb4a564d6834aff88c347 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Fri, 7 Aug 2015 12:13:59 +0100
Subject: [PATCH] Issue #2544954 by stefan.r, jhedstrom:
SqlContentEntityStorageSchema does not detect column-level schema changes in
non-base-fields
---
.../Sql/SqlContentEntityStorageSchema.php | 72 ++++++++++--
...qlContentEntityStorageSchemaColumnTest.php | 104 ++++++++++++++++++
2 files changed, 164 insertions(+), 12 deletions(-)
create mode 100644 core/modules/field/src/Tests/Entity/Update/SqlContentEntityStorageSchemaColumnTest.php
diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
index 463b9eb8bb3b..b92636c6357c 100644
--- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
+++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
@@ -186,27 +186,44 @@ public function requiresFieldStorageSchemaChanges(FieldStorageDefinitionInterfac
return TRUE;
}
+ if ($storage_definition->hasCustomStorage()) {
+ // The field has custom storage, so we don't know if a schema change is
+ // needed or not, but since per the initial checks earlier in this
+ // function, nothing about the definition changed that we manage, we
+ // return FALSE.
+ return FALSE;
+ }
+
+ return $this->getSchemaFromStorageDefinition($storage_definition) != $this->loadFieldSchemaData($original);
+ }
+
+ /**
+ * Gets the schema data for the given field storage definition.
+ *
+ * @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage_definition
+ * The field storage definition. The field that must not have custom
+ * storage, i.e. the storage must take care of storing the field.
+ *
+ * @return array
+ * The schema data.
+ */
+ protected function getSchemaFromStorageDefinition(FieldStorageDefinitionInterface $storage_definition) {
+ assert('!$storage_definition->hasCustomStorage();');
+ $table_mapping = $this->storage->getTableMapping();
+ $schema = [];
if ($table_mapping->requiresDedicatedTableStorage($storage_definition)) {
- return $this->getDedicatedTableSchema($storage_definition) != $this->loadFieldSchemaData($original);
+ $schema = $this->getDedicatedTableSchema($storage_definition);
}
elseif ($table_mapping->allowsSharedTableStorage($storage_definition)) {
$field_name = $storage_definition->getName();
- $schema = array();
foreach (array_diff($table_mapping->getTableNames(), $table_mapping->getDedicatedTableNames()) as $table_name) {
if (in_array($field_name, $table_mapping->getFieldNames($table_name))) {
$column_names = $table_mapping->getColumnNames($storage_definition->getName());
$schema[$table_name] = $this->getSharedTableFieldSchema($storage_definition, $table_name, $column_names);
}
}
- return $schema != $this->loadFieldSchemaData($original);
- }
- else {
- // The field has custom storage, so we don't know if a schema change is
- // needed or not, but since per the initial checks earlier in this
- // function, nothing about the definition changed that we manage, we
- // return FALSE.
- return FALSE;
}
+ return $schema;
}
/**
@@ -1267,7 +1284,7 @@ protected function updateDedicatedTableSchema(FieldStorageDefinitionInterface $s
}
}
else {
- if ($storage_definition->getColumns() != $original->getColumns()) {
+ if ($this->hasColumnChanges($storage_definition, $original)) {
throw new FieldStorageDefinitionUpdateForbiddenException('The SQL storage cannot change the schema for an existing field (' . $storage_definition->getName() . ' in ' . $storage_definition->getTargetEntityTypeId() . ' entity) with data.');
}
// There is data, so there are no column changes. Drop all the prior
@@ -1356,7 +1373,7 @@ protected function updateSharedTableSchema(FieldStorageDefinitionInterface $stor
}
}
else {
- if ($storage_definition->getColumns() != $original->getColumns()) {
+ if ($this->hasColumnChanges($storage_definition, $original)) {
throw new FieldStorageDefinitionUpdateForbiddenException('The SQL storage cannot change the schema for an existing field (' . $storage_definition->getName() . ' in ' . $storage_definition->getTargetEntityTypeId() . ' entity) with data.');
}
@@ -1746,6 +1763,7 @@ protected function getEntityIndexName(ContentEntityTypeInterface $entity_type, $
protected function getFieldIndexName(FieldStorageDefinitionInterface $storage_definition, $index) {
return $storage_definition->getName() . '_' . $index;
}
+
/**
* Checks whether a database table is non-existent or empty.
*
@@ -1766,4 +1784,34 @@ protected function isTableEmpty($table_name) {
->fetchField();
}
+ /**
+ * Compares schemas to check for changes in the column definitions.
+ *
+ * @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage_definition
+ * Current field storage definition.
+ * @param \Drupal\Core\Field\FieldStorageDefinitionInterface $original
+ * The original field storage definition.
+ *
+ * @return bool
+ * Returns TRUE if there are schema changes in the column definitions.
+ */
+ protected function hasColumnChanges(FieldStorageDefinitionInterface $storage_definition, FieldStorageDefinitionInterface $original) {
+ if ($storage_definition->getColumns() != $original->getColumns()) {
+ // Base field definitions have schema data stored in the original
+ // definition.
+ return TRUE;
+ }
+
+ if (!$storage_definition->hasCustomStorage()) {
+ $schema = $this->getSchemaFromStorageDefinition($storage_definition);
+ foreach ($this->loadFieldSchemaData($original) as $table => $spec) {
+ if ($spec['fields'] != $schema[$table]['fields']) {
+ return TRUE;
+ }
+ }
+ }
+
+ return FALSE;
+ }
+
}
diff --git a/core/modules/field/src/Tests/Entity/Update/SqlContentEntityStorageSchemaColumnTest.php b/core/modules/field/src/Tests/Entity/Update/SqlContentEntityStorageSchemaColumnTest.php
new file mode 100644
index 000000000000..379382c040fc
--- /dev/null
+++ b/core/modules/field/src/Tests/Entity/Update/SqlContentEntityStorageSchemaColumnTest.php
@@ -0,0 +1,104 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\field\Tests\Entity\Update\SqlContentEntityStorageSchemaColumnTest.
+ */
+
+namespace Drupal\field\Tests\Entity\Update;
+
+use Drupal\Core\Entity\Exception\FieldStorageDefinitionUpdateForbiddenException;
+use Drupal\entity_test\Entity\EntityTestRev;
+use Drupal\field\Entity\FieldConfig;
+use Drupal\field\Entity\FieldStorageConfig;
+use Drupal\simpletest\KernelTestBase;
+
+/**
+ * Tests that schema changes in fields with data are detected during updates.
+ *
+ * @group Entity
+ */
+class SqlContentEntityStorageSchemaColumnTest extends KernelTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public static $modules = ['entity_test', 'field', 'text', 'user'];
+
+ /**
+ * The created entity.
+ *
+ * @var \Drupal\Core\Entity\Entity
+ */
+ protected $entity;
+
+ /**
+ * The field.
+ *
+ * @var \Drupal\field\FieldConfigInterface
+ */
+ protected $field;
+
+ /**
+ * The field storage.
+ *
+ * @var \Drupal\field\FieldStorageConfigInterface
+ */
+ protected $fieldStorage;
+
+ /**
+ * {@inheritdoc}
+ */
+ public function setUp() {
+ parent::setUp();
+
+ $this->installEntitySchema('entity_test_rev');
+ $this->installEntitySchema('user');
+
+ $field_name = 'test';
+ $this->fieldStorage = FieldStorageConfig::create([
+ 'field_name' => $field_name,
+ 'entity_type' => 'entity_test_rev',
+ 'type' => 'string',
+ 'cardinality' => 1,
+ ]);
+ $this->fieldStorage->save();
+
+ $this->field = FieldConfig::create([
+ 'field_name' => $field_name,
+ 'entity_type' => 'entity_test_rev',
+ 'bundle' => 'entity_test_rev',
+ 'required' => TRUE,
+ ]);
+ $this->field->save();
+
+ // Create an entity with field data.
+ $this->entity = EntityTestRev::create([
+ 'user_id' => mt_rand(1, 10),
+ 'name' => $this->randomMachineName(),
+ $field_name => $this->randomString(),
+ ]);
+ $this->entity->save();
+ }
+
+ /**
+ * Tests that column-level schema changes are detected for fields with data.
+ */
+ public function testColumnUpdate() {
+ // Change the field type in the stored schema.
+ $schema = \Drupal::keyValue('entity.storage_schema.sql')->get('entity_test_rev.field_schema_data.test');
+ $schema['entity_test_rev__test']['fields']['test_value']['type'] = 'varchar_ascii';
+ \Drupal::keyValue('entity.storage_schema.sql')->set('entity_test_rev.field_schema_data.test', $schema);
+
+ // Now attempt to run automatic updates. An exception should be thrown
+ // since there is data in the table.
+ try {
+ \Drupal::service('entity.definition_update_manager')->applyUpdates();
+ $this->fail('Failed to detect a schema change in a field with data.');
+ }
+ catch (FieldStorageDefinitionUpdateForbiddenException $e) {
+ $this->pass('Detected a schema change in a field with data.');
+ }
+ }
+
+}
--
GitLab