[FOLLOWUP][BUGFIX] Respect SQL table names max length in install tool 72/50872/3
authorManuel Glauser <mail@manuelglauser.ch>
Sat, 3 Dec 2016 14:01:56 +0000 (15:01 +0100)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Sat, 31 Dec 2016 14:39:31 +0000 (15:39 +0100)
Consider the table and column name length limitations of the
various database platforms when calling / executing the
database analyser.

Resolves: #78636
Releases: master
Change-Id: I78ad5ea849c722ebc3448beb043dac4846e4d731
Reviewed-on: https://review.typo3.org/50872
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Jan Helke <typo3@helke.de>
Tested-by: Jan Helke <typo3@helke.de>
Reviewed-by: Joerg Boesche <typo3@joergboesche.de>
Tested-by: Joerg Boesche <typo3@joergboesche.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/core/Classes/Database/Schema/ConnectionMigrator.php
typo3/sysext/core/Tests/Unit/Database/Schema/ConnectionMigratorTest.php

index c74d39a..78e3897 100644 (file)
@@ -44,7 +44,44 @@ class ConnectionMigrator
     /**
      * @var int
      */
-    protected $maxTableNameLength = 64;
+    protected $tableAndFieldMaxNameLengthsPerDbPlatform = [
+        'default' => [
+            'tables' => 30,
+            'columns' => 30
+        ],
+        'mysql' => [
+            'tables' => 64,
+            'columns' => 64
+        ],
+        'drizzle_pdo_mysql' => 'mysql',
+        'mysqli' => 'mysql',
+        'pdo_mysql' => 'mysql',
+        'pdo_sqlite' => 'mysql',
+        'postgresql' => [
+            'tables' => 63,
+            'columns' => 63
+        ],
+        'sqlserver' => [
+            'tables' => 128,
+            'columns' => 128
+        ],
+        'pdo_sqlsrv' => 'sqlserver',
+        'sqlsrv' => 'sqlserver',
+        'ibm' => [
+            'tables' => 30,
+            'columns' => 30
+        ],
+        'ibm_db2' => 'ibm',
+        'pdo_ibm' => 'ibm',
+        'oci8' => [
+            'tables' => 30,
+            'columns' => 30
+        ],
+        'sqlanywhere' => [
+            'tables' => 128,
+            'columns' => 128
+        ]
+    ];
 
     /**
      * @var Connection
@@ -880,14 +917,7 @@ class ConnectionMigrator
                 $fromTable = $removedTable
             );
 
-            $tableDiff->newName = $this->deletedPrefix . $removedTable->getName();
-            if (strlen($tableDiff->newName) > $this->maxTableNameLength) {
-                $shortTableName = substr(
-                    $removedTable->getName(),
-                    strlen($removedTable->getName()) + strlen($this->deletedPrefix) - $this->maxTableNameLength
-                );
-                $tableDiff->newName = $this->deletedPrefix . $shortTableName;
-            }
+            $tableDiff->newName = substr($this->deletedPrefix . $removedTable->getName(), 0, $this->getMaxTableNameLength());
             $schemaDiff->changedTables[$index] = $tableDiff;
             unset($schemaDiff->removedTables[$index]);
         }
@@ -917,8 +947,9 @@ class ConnectionMigrator
                 }
 
                 // Build a new column object with the same properties as the removed column
+                $renamedColumnName = substr($this->deletedPrefix . $removedColumn->getName(), 0, $this->getMaxColumnNameLength());
                 $renamedColumn = new Column(
-                    $this->connection->quoteIdentifier($this->deletedPrefix . $removedColumn->getName()),
+                    $this->connection->quoteIdentifier($renamedColumnName),
                     $removedColumn->getType(),
                     array_diff_key($removedColumn->toArray(), ['name', 'type'])
                 );
@@ -944,6 +975,57 @@ class ConnectionMigrator
     }
 
     /**
+     * Retrieve the database platform-specific limitations on column and schema name sizes as
+     * defined in the tableAndFieldMaxNameLengthsPerDbPlatform property.
+     *
+     * @param string $databasePlatform
+     * @return array
+     */
+    protected function getTableAndFieldNameMaxLengths(string $databasePlatform = '')
+    {
+        if ($databasePlatform === '') {
+            $databasePlatform = $this->connection->getDatabasePlatform()->getName();
+        }
+        $databasePlatform = strtolower($databasePlatform);
+
+        if (isset($this->tableAndFieldMaxNameLengthsPerDbPlatform[$databasePlatform])) {
+            $nameLengthRestrictions = $this->tableAndFieldMaxNameLengthsPerDbPlatform[$databasePlatform];
+        } else {
+            $nameLengthRestrictions = $this->tableAndFieldMaxNameLengthsPerDbPlatform['default'];
+        }
+
+        if (is_string($nameLengthRestrictions)) {
+            return $this->getTableAndFieldNameMaxLengths($nameLengthRestrictions);
+        } else {
+            return $nameLengthRestrictions;
+        }
+    }
+
+    /**
+     * Get the maximum table name length possible for the given DB platform.
+     *
+     * @param string $databasePlatform
+     * @return string
+     */
+    protected function getMaxTableNameLength(string $databasePlatform = '')
+    {
+        $nameLengthRestrictions = $this->getTableAndFieldNameMaxLengths($databasePlatform);
+        return $nameLengthRestrictions['tables'];
+    }
+
+    /**
+     * Get the maximum column name length possible for the given DB platform.
+     *
+     * @param string $databasePlatform
+     * @return string
+     */
+    protected function getMaxColumnNameLength(string $databasePlatform = '')
+    {
+        $nameLengthRestrictions = $this->getTableAndFieldNameMaxLengths($databasePlatform);
+        return $nameLengthRestrictions['columns'];
+    }
+
+    /**
      * Return the amount of records in the given table.
      *
      * @param string $tableName
index 6fcce2b..cedd532 100644 (file)
@@ -16,6 +16,7 @@ namespace TYPO3\CMS\Core\Tests\Unit\Database;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\SchemaDiff;
 use Doctrine\DBAL\Schema\Table;
 use TYPO3\CMS\Core\Database\Connection;
@@ -27,33 +28,136 @@ use TYPO3\CMS\Core\Utility\GeneralUtility;
  */
 class ConnectionMigratorTest extends \TYPO3\CMS\Components\TestingFramework\Core\UnitTestCase
 {
+    /**
+     * @var array
+     */
+    protected $tableAndFieldMaxNameLengthsPerDbPlatform = [
+        'default' => [
+            'tables' => 10,
+            'columns' => 10,
+        ],
+        'dbplatform_type1' => [
+            'tables' => 15,
+            'columns' => 15,
+        ],
+        'dbplatform_type2' => 'dbplatform_type1'
+    ];
 
     /**
-     * @test
+     * Utility method to quickly create a 'ConnectionMigratorMock' instance for
+     * a specific database platform.
+     *
+     * @param string $databasePlatformName
+     * @return \PHPUnit_Framework_MockObject_MockObject|\TYPO3\CMS\Core\Tests\AccessibleObjectInterface
      */
-    public function tableNamesStickToTheMaximumCharactersWhenPrefixedForRemoval()
+    private function getConnectionMigratorMock($databasePlatformName='default')
+    {
+        $platformMock = $this->getMockBuilder(\Doctrine\DBAL\Platforms\AbstractPlatform::class)->disableOriginalConstructor()->getMock();
+        $platformMock->method('getName')->willReturn($databasePlatformName);
+
+        $connectionMock = $this->getMockBuilder(Connection::class)->setMethods(['getDatabasePlatform', 'quoteIdentifier'])->disableOriginalConstructor()->getMock();
+        $connectionMock->method('getDatabasePlatform')->willReturn($platformMock);
+        $connectionMock->method('quoteIdentifier')->willReturnArgument(0);
+
+        $connectionMigrator = $this->getAccessibleMock(ConnectionMigrator::class, null, [], '', false);
+        $connectionMigrator->_set('connection', $connectionMock);
+        $connectionMigrator->_set('tableAndFieldMaxNameLengthsPerDbPlatform', $this->tableAndFieldMaxNameLengthsPerDbPlatform);
+
+        return $connectionMigrator;
+    }
+
+    /**
+     * Utility method to create a table mock instance with a much too long
+     * table name in any case.
+     *
+     * @return \PHPUnit_Framework_MockObject_MockObject|\TYPO3\CMS\Core\Tests\AccessibleObjectInterface
+     */
+    private function getTableMock()
     {
-        $maxTableNameLength = 64;
         $ridiculouslyLongTableName = 'table_name_that_is_ridiculously_long_' . random_bytes(200);
         $tableMock = $this->getAccessibleMock(Table::class, ['getQuotedName'], [$ridiculouslyLongTableName]);
         $tableMock->expects($this->any())->method('getQuotedName')->withAnyParameters()->will($this->returnValue($ridiculouslyLongTableName));
 
-        $platform = $this->getMockBuilder(\Doctrine\DBAL\Platforms\AbstractPlatform::class)->disableOriginalConstructor()->getMock();
+        return $tableMock;
+    }
+
+    /**
+     * @test
+     */
+    public function tableNamesStickToTheMaximumCharactersWhenPrefixedForRemoval()
+    {
+        $connectionMigrator = $this->getConnectionMigratorMock('dbplatform_type1');
+        $tableMock = $this->getTableMock();
 
-        $connectionMock = $this->getMockBuilder(Connection::class)->setMethods(['getDatabasePlatform'])->disableOriginalConstructor()->getMock();
-        $connectionMock->method('getDatabasePlatform')->willReturn($platform);
+        $originalSchemaDiff = GeneralUtility::makeInstance(SchemaDiff::class, null, null, [$tableMock]);
+        $renamedSchemaDiff = $connectionMigrator->_call('migrateUnprefixedRemovedTablesToRenames', $originalSchemaDiff);
 
-        $connectionMigrator = $this->getAccessibleMock(ConnectionMigrator::class, null, [], '', false);
-        $connectionMigrator->_set('connection', $connectionMock);
+        $this->assertStringStartsWith('zzz_deleted_', $renamedSchemaDiff->changedTables[0]->newName);
+        $this->assertLessThanOrEqual(
+            $this->tableAndFieldMaxNameLengthsPerDbPlatform['dbplatform_type1']['tables'],
+            strlen($renamedSchemaDiff->changedTables[0]->newName)
+        );
+    }
+
+    /**
+     * @test
+     */
+    public function databasePlatformNamingRestrictionGetsResolved()
+    {
+        $connectionMigrator = $this->getConnectionMigratorMock('dbplatform_type2');
+        $tableMock = $this->getTableMock();
 
         $originalSchemaDiff = GeneralUtility::makeInstance(SchemaDiff::class, null, null, [$tableMock]);
+        $renamedSchemaDiff = $connectionMigrator->_call('migrateUnprefixedRemovedTablesToRenames', $originalSchemaDiff);
+
+        $this->assertLessThanOrEqual(
+            $this->tableAndFieldMaxNameLengthsPerDbPlatform['dbplatform_type1']['tables'],
+            strlen($renamedSchemaDiff->changedTables[0]->newName)
+        );
+    }
+
+    /**
+     * @test
+     */
+    public function whenPassingAnUnknownDatabasePlatformTheDefaultTableAndFieldNameRestrictionsApply()
+    {
+        $connectionMigrator = $this->getConnectionMigratorMock('dummydbplatformthatdoesntexist');
+        $tableMock = $this->getTableMock();
 
+        $originalSchemaDiff = GeneralUtility::makeInstance(SchemaDiff::class, null, null, [$tableMock]);
         $renamedSchemaDiff = $connectionMigrator->_call('migrateUnprefixedRemovedTablesToRenames', $originalSchemaDiff);
 
-        $this->assertStringStartsWith('zzz_deleted_', $renamedSchemaDiff->changedTables[0]->newName);
         $this->assertLessThanOrEqual(
-            $maxTableNameLength,
+            $this->tableAndFieldMaxNameLengthsPerDbPlatform['default']['tables'],
             strlen($renamedSchemaDiff->changedTables[0]->newName)
         );
     }
+
+    /**
+     * @test
+     */
+    public function columnNamesStickToTheMaximumCharactersWhenPrefixedForRemoval()
+    {
+        $connectionMigrator = $this->getConnectionMigratorMock('dbplatform_type1');
+        $tableMock = $this->getAccessibleMock(Table::class, ['getQuotedName'], ['test_table']);
+        $columnMock = $this->getAccessibleMock(
+            Column::class,
+            ['getQuotedName'],
+            [
+                'a_column_name_waaaaay_over_20_characters',
+                $this->getAccessibleMock(\Doctrine\DBAL\Types\StringType::class, [], [], '', false)
+            ]
+        );
+        $columnMock->expects($this->any())->method('getQuotedName')->withAnyParameters()->will($this->returnValue('a_column_name_waaaaay_over_20_characters'));
+
+        $originalSchemaDiff = GeneralUtility::makeInstance(SchemaDiff::class, null, null, [$tableMock]);
+        $originalSchemaDiff->changedTables[0]->removedColumns[] = $columnMock;
+        $renamedSchemaDiff = $connectionMigrator->_call('migrateUnprefixedRemovedFieldsToRenames', $originalSchemaDiff);
+
+        $this->assertStringStartsWith('zzz_deleted_', $renamedSchemaDiff->changedTables[0]->changedColumns[0]->column->getName());
+        $this->assertLessThanOrEqual(
+            $this->tableAndFieldMaxNameLengthsPerDbPlatform['dbplatform_type1']['columns'],
+            strlen($renamedSchemaDiff->changedTables[0]->changedColumns[0]->column->getName())
+        );
+    }
 }