[BUGFIX] False-Positives in SQL comparison 30/19630/9
authorMario Rimann <mario.rimann@typo3.org>
Fri, 5 Apr 2013 21:10:29 +0000 (23:10 +0200)
committerWouter Wolters <typo3@wouterwolters.nl>
Tue, 11 Jun 2013 20:21:02 +0000 (22:21 +0200)
When one has e.g. a field "foo INT(11) DEFAULT '0' NOT NULL" in
it's ext_tables.sql definition, the comparison will always complain
about that field, since the underlying DB lists this field as "int(11)"
which is lowercased.

This integrates a regex that lowercases the field types before
comparing the field from the definition against the existing field.

Change-Id: If76abbbca56d0ef0ab796a7f4e6bee1197ac39e6
Resolves: #41344
Releases: 6.2, 6.1, 6.0, 4.7, 4.5
Reviewed-on: https://review.typo3.org/19630
Tested-by: Dmitry Dulepov
Tested-by: Wouter Wolters
Reviewed-by: Dmitry Dulepov
Reviewed-by: Markus Klein
Reviewed-by: Wouter Wolters
typo3/sysext/install/Classes/Sql/SchemaMigrator.php
typo3/sysext/install/Tests/Unit/SchemaMigratorTest.php [new file with mode: 0644]

index 16c04a6..a4b2ff8 100644 (file)
@@ -361,6 +361,21 @@ class SchemaMigrator {
                                                                                $extraArr[$table][$theKey][$fieldN] = $fieldC;
                                                                        } else {
                                                                                $fieldC = trim($fieldC);
+
+                                                                               // Lowercase the field type to surround false-positive schema changes to be
+                                                                               // reported just because of different caseing of characters
+                                                                               // The regex does just trigger for the first word followed by round brackets
+                                                                               // that contain a length. It does not trigger for e.g. "PRIMARY KEY" because
+                                                                               // "PRIMARY KEY" is being returned from the DB in upper case.
+                                                                               $fieldC = preg_replace_callback(
+                                                                                       '/^([a-zA-Z0-9]+\(.*\))(\s)(.*)/',
+                                                                                       create_function(
+                                                                                               '$matches',
+                                                                                               'return strtolower($matches[1]) . $matches[2] . $matches[3];'
+                                                                                       ),
+                                                                                       $fieldC
+                                                                               );
+
                                                                                if ($ignoreNotNullWhenComparing) {
                                                                                        $fieldC = str_replace(' NOT NULL', '', $fieldC);
                                                                                        $FDcomp[$table][$theKey][$fieldN] = str_replace(' NOT NULL', '', $FDcomp[$table][$theKey][$fieldN]);
diff --git a/typo3/sysext/install/Tests/Unit/SchemaMigratorTest.php b/typo3/sysext/install/Tests/Unit/SchemaMigratorTest.php
new file mode 100644 (file)
index 0000000..6aa25de
--- /dev/null
@@ -0,0 +1,167 @@
+<?php
+namespace TYPO3\CMS\Install\Tests\Unit;
+
+/***************************************************************
+ *  Copyright notice
+ *
+ *  (c) 2013 Mario Rimann <mario.rimann@typo3.org>
+ *  All rights reserved
+ *
+ *  This script is part of the TYPO3 project. The TYPO3 project is
+ *  free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  The GNU General Public License can be found at
+ *  http://www.gnu.org/copyleft/gpl.html.
+ *  A copy is found in the textfile GPL.txt and important notices to the license
+ *  from the author is found in LICENSE.txt distributed with these scripts.
+ *
+ *
+ *  This script is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  This copyright notice MUST APPEAR in all copies of the script!
+ ***************************************************************/
+
+use TYPO3\CMS\Install\Sql\SchemaMigrator;
+
+/**
+ * Testcase for class "\TYPO3\CMS\Install\Sql\SchemaMigrator"
+ *
+ * @author Mario Rimann <mario.rimann@typo3.org>
+ */
+class SchemaMigratorTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
+
+       /**
+        * @var \TYPO3\CMS\Install\Sql\SchemaMigrator
+        */
+       protected $fixture;
+
+       /**
+        * Sets up the fixture for testing
+        */
+       public function setUp() {
+               $this->fixture = new SchemaMigrator();
+       }
+
+       /**
+        * Tears down the fixture
+        */
+       public function tearDown() {
+               unset($this->fixture);
+       }
+
+       /**
+        * @test
+        */
+       public function getDatabaseExtraFindsChangedFields() {
+               $differenceArray = $this->fixture->getDatabaseExtra(
+                       array(
+                               'tx_foo' => array(
+                                       'fields' => array(
+                                               'foo' => 'varchar(999) DEFAULT \'0\' NOT NULL'
+                                       )
+                               )
+                       ),
+                       array(
+                               'tx_foo' => array(
+                                       'fields' => array(
+                                               'foo' => 'varchar(255) DEFAULT \'0\' NOT NULL'
+                                       )
+                               )
+                       )
+               );
+
+
+
+               $this->assertEquals(
+                       $differenceArray,
+                       array(
+                               'extra' => array(),
+                               'diff' => array(
+                                       'tx_foo' => array(
+                                               'fields' => array(
+                                                       'foo' => 'varchar(999) DEFAULT \'0\''
+                                               )
+                                       )
+                               ),
+                               diff_currentValues => array(
+                                       'tx_foo' => array(
+                                               'fields' => array(
+                                                       'foo' => 'varchar(255) DEFAULT \'0\''
+                                               )
+                                       )
+                               )
+                       )
+               );
+       }
+
+       /**
+        * @test
+        */
+       public function getDatabaseExtraIgnoresCaseDifference() {
+               $differenceArray = $this->fixture->getDatabaseExtra(
+                       array(
+                               'tx_foo' => array(
+                                       'fields' => array(
+                                               'foo' => 'INT(11) DEFAULT \'0\' NOT NULL'
+                                       )
+                               )
+                       ),
+                       array(
+                               'tx_foo' => array(
+                                       'fields' => array(
+                                               'foo' => 'int(11) DEFAULT \'0\' NOT NULL'
+                                       )
+                               )
+                       )
+               );
+
+
+               $this->assertEquals(
+                       $differenceArray,
+                       array(
+                               'extra' => array(),
+                               'diff' => array(),
+                               diff_currentValues => NULL
+                       )
+               );
+       }
+
+       /**
+        * @test
+        */
+       public function getDatabaseExtraDoesNotLowercaseReservedWordsForTheComparison() {
+               $differenceArray = $this->fixture->getDatabaseExtra(
+                       array(
+                               'tx_foo' => array(
+                                       'fields' => array(
+                                               'PRIMARY KEY (md5hash)'
+                                       )
+                               )
+                       ),
+                       array(
+                               'tx_foo' => array(
+                                       'fields' => array(
+                                               'PRIMARY KEY (md5hash)')
+                               )
+                       )
+               );
+
+
+               $this->assertEquals(
+                       $differenceArray,
+                       array(
+                               'extra' => array(),
+                               'diff' => array(),
+                               diff_currentValues => NULL
+                       )
+               );
+       }
+}
+
+?>
\ No newline at end of file