[BUGFIX] Correct order of key / field deletion

Prevent an SQL error when deleting a key and the field belonging
to the key in the install tool at the same time.

Therefore the order of deleting keys and fields is changed from
field first and key second to key first and field second.

Resolves: #50450
Releases: master, 6.2
Change-Id: I30aff6ea6c2bf01dec6197eec8fd3aa7306cfcbb
Reviewed-on: http://review.typo3.org/22581


Reviewed-by: Helmut Hummel <helmut.hummel@typo3.org>
Tested-by: Helmut Hummel <helmut.hummel@typo3.org>
Reviewed-by: Markus Klein <klein.t3@reelworx.at>
Tested-by: Markus Klein <klein.t3@reelworx.at>

typo3/sysext/install/Classes/Service/SqlSchemaMigrationService.php

@@ -327,6 +327,14 @@ class SqlSchemaMigrationService {
 			if (is_array($diffArr[$theKey])) {
 				foreach ($diffArr[$theKey] as $table => $info) {
 					$whole_table = array();
+					if (isset($info['keys']) && is_array($info['keys'])) {
+						foreach ($info['keys'] as $fN => $fV) {
+							if (!$info['whole_table'] && $theKey === 'extra' && $remove) {
+								$statement = 'ALTER TABLE ' . $table . ($fN === 'PRIMARY' ? ' DROP PRIMARY KEY' : ' DROP KEY ' . $fN) . ';';
+								$statements['drop'][md5($statement)] = $statement;
+							}
+						}
+					}
 					if (is_array($info['fields'])) {
 						foreach ($info['fields'] as $fN => $fV) {
 							if ($info['whole_table']) {
@@ -373,10 +381,7 @@ class SqlSchemaMigrationService {
 								$whole_table[] = $fV;
 							} else {
 								if ($theKey == 'extra') {
-									if ($remove) {
-										$statement = 'ALTER TABLE ' . $table . ($fN == 'PRIMARY' ? ' DROP PRIMARY KEY' : ' DROP KEY ' . $fN) . ';';
-										$statements['drop'][md5($statement)] = $statement;
-									} else {
+									if (!$remove) {
 										$statement = 'ALTER TABLE ' . $table . ' ADD ' . $fV . ';';
 										$statements['add'][md5($statement)] = $statement;
 									}

typo3/sysext/install/Tests/Functional/SqlSchemaMigrationServiceTest.php

+<?php
+namespace TYPO3\CMS\Install\Tests\Functional;
+
+/**
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+/**
+ * Functional tests for the SQL schema migration service.
+ */
+class SqlSchemaMigrationServiceTest extends \TYPO3\CMS\Core\Tests\FunctionalTestCase {
+
+	/**
+	 * @var \TYPO3\CMS\Install\Service\SqlSchemaMigrationService
+	 */
+	protected $sqlSchemaMigrationService;
+
+	/**
+	 * Initializes a SqlSchemaMigrationService instance.
+	 */
+	public function setUp() {
+		parent::setUp();
+		$this->sqlSchemaMigrationService = new \TYPO3\CMS\Install\Service\SqlSchemaMigrationService();
+	}
+
+	/**
+	 * @test
+	 */
+	public function columnAndKeyDeletionDoesNotReturnAnError() {
+
+		// Get the current database fields.
+		$currentDatabaseSchema = $this->sqlSchemaMigrationService->getFieldDefinitions_database();
+
+		// Limit our scope to the be_users table:
+		$currentDatabaseSchemaForBeUsers = array();
+		$currentDatabaseSchemaForBeUsers['be_users'] = $currentDatabaseSchema['be_users'];
+		unset($currentDatabaseSchema);
+
+		// Create a key and a field that belongs to that key:
+		$expectedDatabaseSchemaForBeUsers = $currentDatabaseSchemaForBeUsers;
+		$expectedDatabaseSchemaForBeUsers['be_users']['fields']['functional_test_field_1'] = "tinyint(1) unsigned NOT NULL default '0'";
+		$expectedDatabaseSchemaForBeUsers['be_users']['keys']['functional_test_key_1'] = "KEY functional_test_key_1 (functional_test_field_1)";
+		$createFieldDiff = $this->sqlSchemaMigrationService->getDatabaseExtra($expectedDatabaseSchemaForBeUsers, $currentDatabaseSchemaForBeUsers);
+		$createFieldDiff = $this->sqlSchemaMigrationService->getUpdateSuggestions($createFieldDiff);
+		$this->sqlSchemaMigrationService->performUpdateQueries($createFieldDiff['add'], $createFieldDiff['add']);
+
+		// Now remove the fields again (without the renaming step).
+		unset($currentDatabaseSchemaForBeUsers['be_users']['fields']['functional_test_field_1']);
+		unset($currentDatabaseSchemaForBeUsers['be_users']['keys']['functional_test_key_1']);
+		$this->sqlSchemaMigrationService->setDeletedPrefixKey('');
+		$removeFieldDiff = $this->sqlSchemaMigrationService->getDatabaseExtra($expectedDatabaseSchemaForBeUsers, $currentDatabaseSchemaForBeUsers);
+		$removeFieldDiff = $this->sqlSchemaMigrationService->getUpdateSuggestions($removeFieldDiff, 'remove');
+		$result = $this->sqlSchemaMigrationService->performUpdateQueries($removeFieldDiff['drop'], $removeFieldDiff['drop']);
+		$this->assertTrue($result, 'performUpdateQueries() did not return TRUE, this means an error occurred: ' . (is_array($result) ? array_pop($result) : ''));
+	}
+}
\ No newline at end of file