Merge pull request #24384 from nextcloud/cleanup/remove-old-migration-check

Remove dead code from checking core/apps before upgrades

lib/private/DB/MDB2SchemaManager.php

@@ -71,15 +71,15 @@ class MDB2SchemaManager {
 		$config = \OC::$server->getConfig();
 		$dispatcher = \OC::$server->getEventDispatcher();
 		if ($platform instanceof SqlitePlatform) {
-			return new SQLiteMigrator($this->conn, $random, $config, $dispatcher);
+			return new SQLiteMigrator($this->conn, $config, $dispatcher);
 		} elseif ($platform instanceof OraclePlatform) {
-			return new OracleMigrator($this->conn, $random, $config, $dispatcher);
+			return new OracleMigrator($this->conn, $config, $dispatcher);
 		} elseif ($platform instanceof MySQLPlatform) {
-			return new MySQLMigrator($this->conn, $random, $config, $dispatcher);
+			return new MySQLMigrator($this->conn, $config, $dispatcher);
 		} elseif ($platform instanceof PostgreSQL94Platform) {
-			return new PostgreSqlMigrator($this->conn, $random, $config, $dispatcher);
+			return new PostgreSqlMigrator($this->conn, $config, $dispatcher);
 		} else {
-			return new Migrator($this->conn, $random, $config, $dispatcher);
+			return new Migrator($this->conn, $config, $dispatcher);
 		}
 	}
 

lib/private/DB/Migrator.php

@@ -35,14 +35,10 @@ use Doctrine\DBAL\Exception;
 use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Schema\AbstractAsset;
 use Doctrine\DBAL\Schema\Comparator;
-use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\Schema;
-use Doctrine\DBAL\Schema\SchemaConfig;
-use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Types\StringType;
 use Doctrine\DBAL\Types\Type;
 use OCP\IConfig;
-use OCP\Security\ISecureRandom;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
 use function preg_match;
@@ -52,9 +48,6 @@ class Migrator {
 	/** @var \Doctrine\DBAL\Connection */
 	protected $connection;
 
-	/** @var ISecureRandom */
-	private $random;
-
 	/** @var IConfig */
 	protected $config;
 
@@ -66,16 +59,13 @@ class Migrator {
 
 	/**
 	 * @param \Doctrine\DBAL\Connection $connection
-	 * @param ISecureRandom $random
 	 * @param IConfig $config
 	 * @param EventDispatcherInterface $dispatcher
 	 */
 	public function __construct(\Doctrine\DBAL\Connection $connection,
-								ISecureRandom $random,
 								IConfig $config,
 								EventDispatcherInterface $dispatcher = null) {
 		$this->connection = $connection;
-		$this->random = $random;
 		$this->config = $config;
 		$this->dispatcher = $dispatcher;
 	}
@@ -106,73 +96,6 @@ class Migrator {
 		return $script;
 	}
 
-	/**
-	 * Create a unique name for the temporary table
-	 *
-	 * @param string $name
-	 * @return string
-	 */
-	protected function generateTemporaryTableName($name) {
-		return $this->config->getSystemValue('dbtableprefix', 'oc_') . $name . '_' . $this->random->generate(13, ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS);
-	}
-
-	/**
-	 * Check the migration of a table on a copy so we can detect errors before messing with the real table
-	 *
-	 * @param \Doctrine\DBAL\Schema\Table $table
-	 * @throws \OC\DB\MigrationException
-	 */
-	protected function checkTableMigrate(Table $table) {
-		$name = $table->getName();
-		$tmpName = $this->generateTemporaryTableName($name);
-
-		$this->copyTable($name, $tmpName);
-
-		//create the migration schema for the temporary table
-		$tmpTable = $this->renameTableSchema($table, $tmpName);
-		$schemaConfig = new SchemaConfig();
-		$schemaConfig->setName($this->connection->getDatabase());
-		$schema = new Schema([$tmpTable], [], $schemaConfig);
-
-		try {
-			$this->applySchema($schema);
-			$this->dropTable($tmpName);
-		} catch (Exception $e) {
-			// pgsql needs to commit it's failed transaction before doing anything else
-			if ($this->connection->isTransactionActive()) {
-				$this->connection->commit();
-			}
-			$this->dropTable($tmpName);
-			throw new MigrationException($table->getName(), $e->getMessage());
-		}
-	}
-
-	/**
-	 * @param \Doctrine\DBAL\Schema\Table $table
-	 * @param string $newName
-	 * @return \Doctrine\DBAL\Schema\Table
-	 */
-	protected function renameTableSchema(Table $table, $newName) {
-		/**
-		 * @var \Doctrine\DBAL\Schema\Index[] $indexes
-		 */
-		$indexes = $table->getIndexes();
-		$newIndexes = [];
-		foreach ($indexes as $index) {
-			if ($index->isPrimary()) {
-				// do not rename primary key
-				$indexName = $index->getName();
-			} else {
-				// avoid conflicts in index names
-				$indexName = $this->config->getSystemValue('dbtableprefix', 'oc_') . $this->random->generate(13, ISecureRandom::CHAR_LOWER);
-			}
-			$newIndexes[] = new Index($indexName, $index->getColumns(), $index->isUnique(), $index->isPrimary());
-		}
-
-		// foreign keys are not supported so we just set it to an empty array
-		return new Table($newName, $table->getColumns(), $newIndexes, [], [], $table->getOptions());
-	}
-
 	/**
 	 * @throws Exception
 	 */
@@ -260,25 +183,6 @@ class Migrator {
 		}
 	}
 
-	/**
-	 * @param string $sourceName
-	 * @param string $targetName
-	 */
-	protected function copyTable($sourceName, $targetName) {
-		$quotedSource = $this->connection->quoteIdentifier($sourceName);
-		$quotedTarget = $this->connection->quoteIdentifier($targetName);
-
-		$this->connection->exec('CREATE TABLE ' . $quotedTarget . ' (LIKE ' . $quotedSource . ')');
-		$this->connection->exec('INSERT INTO ' . $quotedTarget . ' SELECT * FROM ' . $quotedSource);
-	}
-
-	/**
-	 * @param string $name
-	 */
-	protected function dropTable($name) {
-		$this->connection->exec('DROP TABLE ' . $this->connection->quoteIdentifier($name));
-	}
-
 	/**
 	 * @param $statement
 	 * @return string
@@ -303,11 +207,4 @@ class Migrator {
 		}
 		$this->dispatcher->dispatch('\OC\DB\Migrator::executeSql', new GenericEvent($sql, [$step + 1, $max]));
 	}
-
-	private function emitCheckStep($tableName, $step, $max) {
-		if (is_null($this->dispatcher)) {
-			return;
-		}
-		$this->dispatcher->dispatch('\OC\DB\Migrator::checkTable', new GenericEvent($tableName, [$step + 1, $max]));
-	}
 }

lib/private/DB/MySQLMigrator.php

@@ -27,7 +27,6 @@
 namespace OC\DB;
 
 use Doctrine\DBAL\Schema\Schema;
-use Doctrine\DBAL\Schema\Table;
 
 class MySQLMigrator extends Migrator {
 	/**
@@ -52,25 +51,4 @@ class MySQLMigrator extends Migrator {
 
 		return $schemaDiff;
 	}
-
-	/**
-	 * Speed up migration test by disabling autocommit and unique indexes check
-	 *
-	 * @param \Doctrine\DBAL\Schema\Table $table
-	 * @throws \OC\DB\MigrationException
-	 */
-	protected function checkTableMigrate(Table $table) {
-		$this->connection->exec('SET autocommit=0');
-		$this->connection->exec('SET unique_checks=0');
-
-		try {
-			parent::checkTableMigrate($table);
-		} catch (\Exception $e) {
-			$this->connection->exec('SET unique_checks=1');
-			$this->connection->exec('SET autocommit=1');
-			throw new MigrationException($table->getName(), $e->getMessage());
-		}
-		$this->connection->exec('SET unique_checks=1');
-		$this->connection->exec('SET autocommit=1');
-	}
 }

lib/private/DB/OracleMigrator.php

@@ -204,14 +204,6 @@ class OracleMigrator extends Migrator {
 		return $schemaDiff;
 	}
 
-	/**
-	 * @param string $name
-	 * @return string
-	 */
-	protected function generateTemporaryTableName($name) {
-		return 'oc_' . uniqid();
-	}
-
 	/**
 	 * @param $statement
 	 * @return string

tests/lib/DB/MigratorTest.php

@@ -15,7 +15,6 @@ use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Schema\SchemaConfig;
-use OC\DB\SchemaWrapper;
 use OCP\IConfig;
 
 /**
@@ -131,58 +130,6 @@ class MigratorTest extends \Test\TestCase {
 		return $this->connection->getDatabasePlatform() instanceof MySQLPlatform;
 	}
 
-
-	public function testDuplicateKeyUpgrade() {
-		$this->expectException(Exception\UniqueConstraintViolationException::class);
-
-		if ($this->isSQLite()) {
-			$this->markTestSkipped('sqlite does not throw errors when creating a new key on existing data');
-		}
-		[$startSchema, $endSchema] = $this->getDuplicateKeySchemas();
-		$migrator = $this->manager->getMigrator();
-		$migrator->migrate($startSchema);
-
-		$this->connection->insert($this->tableName, ['id' => 1, 'name' => 'foo']);
-		$this->connection->insert($this->tableName, ['id' => 2, 'name' => 'bar']);
-		$this->connection->insert($this->tableName, ['id' => 2, 'name' => 'qwerty']);
-
-		try {
-			$migrator->migrate($endSchema);
-		} catch (Exception\UniqueConstraintViolationException $e) {
-			if (!$this->isMySQL()) {
-				$this->connection->rollBack();
-			}
-			throw $e;
-		}
-	}
-
-	public function testChangeToString() {
-		[$startSchema, $endSchema] = $this->getChangedTypeSchema('integer', 'string');
-		$migrator = $this->manager->getMigrator();
-		$migrator->migrate($startSchema);
-		$schema = new SchemaWrapper($this->connection);
-		$table = $schema->getTable(substr($this->tableName, 3));
-		$this->assertEquals('integer', $table->getColumn('id')->getType()->getName());
-
-		$this->connection->insert($this->tableName, ['id' => 1, 'name' => 'foo']);
-		$this->connection->insert($this->tableName, ['id' => 2, 'name' => 'bar']);
-		$this->connection->insert($this->tableName, ['id' => 3, 'name' => 'qwerty']);
-
-		$migrator->migrate($endSchema);
-		$this->addToAssertionCount(1);
-
-		$qb = $this->connection->getQueryBuilder();
-		$result = $qb->select('*')->from(substr($this->tableName, 3))->execute();
-		$this->assertEquals([
-			['id' => 1, 'name' => 'foo'],
-			['id' => 2, 'name' => 'bar'],
-			['id' => 3, 'name' => 'qwerty']
-		], $result->fetchAll());
-		$schema = new SchemaWrapper($this->connection);
-		$table = $schema->getTable(substr($this->tableName, 3));
-		$this->assertEquals('string', $table->getColumn('id')->getType()->getName());
-	}
-
 	public function testUpgrade() {
 		[$startSchema, $endSchema] = $this->getDuplicateKeySchemas();
 		$migrator = $this->manager->getMigrator();