Unverified Commit 23bb5935 authored by Alex Pott's avatar Alex Pott
Browse files

Issue #2928906 by tstoeckler, hchonov, gnunes, gease, amateescu, daffie,... 

Issue #2928906 by tstoeckler, hchonov, gnunes, gease, amateescu, daffie, alexpott, catch, claudiu.cristea: The field schema incorrectly stores serial fields as int
parent 62395e72

core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php

+22 −8
Original line number Diff line number Diff line
@@ -561,6 +561,12 @@ protected function postUpdateEntityTypeSchema(EntityTypeInterface $entity_type, 
      $this->fieldStorageDefinitions = $field_storage_definitions;

      $this->saveEntitySchemaData($entity_type, $new_entity_schema);



      // The storage needs to use the updated definitions and table mapping

      // before generating and saving the final field schema data.

      $this->storage->setEntityType($entity_type);

      $this->storage->setFieldStorageDefinitions($field_storage_definitions);

      $this->storage->setTableMapping($new_table_mapping);



      // Store the updated field schema for each field storage.

      foreach ($field_storage_definitions as $field_storage_definition) {

        if ($new_table_mapping->requiresDedicatedTableStorage($field_storage_definition)) {
@@ -1404,31 +1410,31 @@ protected function addTableDefaults(&$schema) { 
  /**

   * Processes the gathered schema for a base table.

   *

   * This function will be removed in Drupal 9.0.x.

   *

   * @param \Drupal\Core\Entity\ContentEntityTypeInterface $entity_type

   *   The entity type.

   * @param array $schema

   *   The table schema, passed by reference.

   *

   * @see https://www.drupal.org/node/3111613

   */

  protected function processBaseTable(ContentEntityTypeInterface $entity_type, array &$schema) {

    // Process the schema for the 'id' entity key only if it exists.

    if ($entity_type->hasKey('id')) {

      $this->processIdentifierSchema($schema, $entity_type->getKey('id'));

    }

  }



  /**

   * Processes the gathered schema for a base table.

   *

   * This function will be removed in Drupal 9.0.x.

   *

   * @param \Drupal\Core\Entity\ContentEntityTypeInterface $entity_type

   *   The entity type.

   * @param array $schema

   *   The table schema, passed by reference.

   *

   * @see https://www.drupal.org/node/3111613

   */

  protected function processRevisionTable(ContentEntityTypeInterface $entity_type, array &$schema) {

    // Process the schema for the 'revision' entity key only if it exists.

    if ($entity_type->hasKey('revision')) {

      $this->processIdentifierSchema($schema, $entity_type->getKey('revision'));

    }

  }



  /**
@@ -2047,6 +2053,7 @@ protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $st 


    $field_name = $storage_definition->getName();

    $base_table = $this->storage->getBaseTable();

    $revision_table = $this->storage->getRevisionTable();



    // Define the initial values, if any.

    $initial_value = $initial_value_from_field = [];
@@ -2125,6 +2132,13 @@ protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $st 
      $schema['foreign keys'] = $this->getFieldForeignKeys($field_name, $field_schema, $column_mapping);

    }



    // Process the 'id' and 'revision' entity keys for the base and revision

    // tables.

    if (($table_name === $base_table && $field_name === $this->entityType->getKey('id')) ||

      ($table_name === $revision_table && $field_name === $this->entityType->getKey('revision'))) {

      $this->processIdentifierSchema($schema, $field_name);

    }



    return $schema;

  }

core/modules/system/system.install

+58 −0
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 
use Drupal\Core\Entity\ContentEntityTypeInterface;

use Drupal\Core\Entity\EntityTypeInterface;

use Drupal\Core\Entity\FieldableEntityInterface;

use Drupal\Core\Entity\Sql\SqlContentEntityStorage;

use Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema;

use Drupal\Core\Extension\Extension;

use Drupal\Core\Field\BaseFieldDefinition;

use Drupal\Core\File\FileSystemInterface;
@@ -2521,3 +2523,59 @@ function system_update_8805() { 
    }

  }

}



/**

 * Update the stored schema data for entity identifier fields.

 */

function system_update_8901() {

  $definition_update_manager = \Drupal::entityDefinitionUpdateManager();

  $entity_type_manager = \Drupal::entityTypeManager();

  $installed_storage_schema = \Drupal::keyValue('entity.storage_schema.sql');



  foreach ($definition_update_manager->getEntityTypes() as $entity_type_id => $entity_type) {

    // Ensure that we are dealing with a non-deleted entity type that uses the

    // default SQL storage.

    if (!$entity_type_manager->hasDefinition($entity_type_id)) {

      continue;

    }



    $storage = $entity_type_manager->getStorage($entity_type_id);

    if (!$storage instanceof SqlContentEntityStorage) {

      continue;

    }



    foreach (['id', 'revision'] as $key) {

      if (!$entity_type->hasKey($key)) {

        continue;

      }



      $field_name = $entity_type->getKey($key);

      $field_storage_definition = $definition_update_manager->getFieldStorageDefinition($field_name, $entity_type_id);

      if (!$field_storage_definition) {

        continue;

      }

      if ($field_storage_definition->getType() !== 'integer') {

        continue;

      }



      // Retrieve the storage schema in order to use its own method for updating

      // the identifier schema - ::processIdentifierSchema(). This is needed

      // because some storage schemas might not use serial identifiers.

      // @see \Drupal\user\UserStorageSchema::processIdentifierSchema()

      $ref_get_storage_schema = new \ReflectionMethod($storage, 'getStorageSchema');

      $ref_get_storage_schema->setAccessible(TRUE);

      $storage_schema = $ref_get_storage_schema->invoke($storage);



      if ($storage_schema instanceof SqlContentEntityStorageSchema) {

        $field_schema_data = $installed_storage_schema->get($entity_type_id . '.field_schema_data.' . $field_storage_definition->getName(), []);

        $table = $key === 'id' ? $entity_type->getBaseTable() : $entity_type->getRevisionTable();



        $ref_process_identifier_schema = new \ReflectionMethod($storage_schema, 'processIdentifierSchema');

        $ref_process_identifier_schema->setAccessible(TRUE);

        $ref_process_identifier_schema->invokeArgs($storage_schema, [&$field_schema_data[$table], $field_name]);



        $installed_storage_schema->set($entity_type_id . '.field_schema_data.' . $field_storage_definition->getName(), $field_schema_data);

      }

    }

  }

}

core/modules/system/tests/src/Functional/Update/IdentifierFieldSchemaTypeUpdateTest.php

0 → 100644
+63 −0
Original line number Diff line number Diff line 
<?php



namespace Drupal\Tests\system\Functional\Update;



use Drupal\FunctionalTests\Update\UpdatePathTestBase;



/**

 * Tests the upgrade path for updating the stored type of identifier fields.

 *

 * @see https://www.drupal.org/node/2928906

 *

 * @group Update

 * @group legacy

 */

class IdentifierFieldSchemaTypeUpdateTest extends UpdatePathTestBase {



  /**

   * {@inheritdoc}

   */

  protected function setDatabaseDumpFiles() {

    $this->databaseDumpFiles = [

      __DIR__ . '/../../../fixtures/update/drupal-8.8.0.bare.standard.php.gz',

    ];

  }



  /**

   * Tests system_update_8901().

   */

  public function testSystemUpdate8901() {

    // The installed field storage schema is wrong before running the update.

    $key_value_store = \Drupal::keyValue('entity.storage_schema.sql');

    $id_schema = $key_value_store->get('node.field_schema_data.nid', []);

    $revision_id_schema = $key_value_store->get('node.field_schema_data.vid', []);



    $this->assertEquals('int', $id_schema['node']['fields']['nid']['type']);

    $this->assertEquals('int', $id_schema['node_revision']['fields']['nid']['type']);

    $this->assertEquals('int', $revision_id_schema['node']['fields']['vid']['type']);

    $this->assertEquals('int', $revision_id_schema['node_revision']['fields']['vid']['type']);



    $this->runUpdates();



    // Now check that the schema has been corrected.

    $id_schema = $key_value_store->get('node.field_schema_data.nid', []);

    $revision_id_schema = $key_value_store->get('node.field_schema_data.vid', []);



    $this->assertEquals('serial', $id_schema['node']['fields']['nid']['type']);

    $this->assertEquals('int', $id_schema['node_revision']['fields']['nid']['type']);

    $this->assertEquals('int', $revision_id_schema['node']['fields']['vid']['type']);

    $this->assertEquals('serial', $revision_id_schema['node_revision']['fields']['vid']['type']);



    // Check that creating and loading a node still works as expected.

    $node_storage = \Drupal::entityTypeManager()->getStorage('node');

    $node = $node_storage->create([

      'title' => 'Test update',

      'type' => 'article',

    ]);

    $node->save();



    $node = $node_storage->load($node->id());

    $this->assertEquals('Test update', $node->label());

  }



}

core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php

+59 −0
Original line number Diff line number Diff line
@@ -371,4 +371,63 @@ public function testCleanUpStorageDefinition() { 
    $this->assertEqual($entity_type_id_count, 0, 'After uninstalling entity_test module the schema should not contains fields from entities provided by the module.');

  }



  /**

   * Tests the installed storage schema for identifier fields.

   */

  public function testIdentifierSchema() {

    $this->installEntitySchema('entity_test_rev');



    $key_value_store = \Drupal::keyValue('entity.storage_schema.sql');

    $id_schema = $key_value_store->get('entity_test_rev.field_schema_data.id', []);

    $revision_id_schema = $key_value_store->get('entity_test_rev.field_schema_data.revision_id', []);



    $expected_id_schema = [

      'entity_test_rev' => [

        'fields' => [

          'id' => [

            'type' => 'serial',

            'unsigned' => TRUE,

            'size' => 'normal',

            'not null' => TRUE,

          ],

        ],

      ],

      'entity_test_rev_revision' => [

        'fields' => [

          'id' => [

            'type' => 'int',

            'unsigned' => TRUE,

            'size' => 'normal',

            'not null' => TRUE,

          ],

        ],

      ],

    ];

    $this->assertEquals($expected_id_schema, $id_schema);



    $expected_revision_id_schema = [

      'entity_test_rev' => [

        'fields' => [

          'revision_id' => [

            'type' => 'int',

            'unsigned' => TRUE,

            'size' => 'normal',

            'not null' => FALSE,

          ],

        ],

      ],

      'entity_test_rev_revision' => [

        'fields' => [

          'revision_id' => [

            'type' => 'serial',

            'unsigned' => TRUE,

            'size' => 'normal',

            'not null' => TRUE,

          ],

        ],

      ],

    ];

    $this->assertEquals($expected_revision_id_schema, $revision_id_schema);

  }



}

core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php

+2 −2
Original line number Diff line number Diff line
@@ -426,7 +426,7 @@ public function testGetSchemaRevisionable() { 
      ->method('getRevisionMetadataKeys')

      ->will($this->returnValue([]));



    $this->storage->expects($this->exactly(1))

    $this->storage->expects($this->exactly(9))

      ->method('getRevisionTable')

      ->will($this->returnValue('entity_test_revision'));
@@ -647,7 +647,7 @@ public function testGetSchemaRevisionableTranslatable() { 
      ->method('isTranslatable')

      ->will($this->returnValue(TRUE));



    $this->storage->expects($this->exactly(2))

    $this->storage->expects($this->exactly(30))

      ->method('getRevisionTable')

      ->will($this->returnValue('entity_test_revision'));