Skip db migration simulation for core schema changes

Signed-off-by: Julius Härtl <jus@bitgrid.net>
nextcloud · Dec 9, 2020 · 1c2a9b4 · 1c2a9b4
1 parent f318423
commit 1c2a9b4
Show file tree

Hide file tree

Showing 6 changed files with 6 additions and 217 deletions.
diff --git a/lib/private/DB/MDB2SchemaManager.php b/lib/private/DB/MDB2SchemaManager.php
@@ -83,15 +83,15 @@ public function getMigrator() {
 		$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 PostgreSqlPlatform) {
-			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);
 		}
 	}
 

diff --git a/lib/private/DB/Migrator.php b/lib/private/DB/Migrator.php
@@ -33,14 +33,10 @@
 
 use Doctrine\DBAL\DBALException;
 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;
 
@@ -49,9 +45,6 @@ class Migrator {
 	/** @var \Doctrine\DBAL\Connection */
 	protected $connection;
 
-	/** @var ISecureRandom */
-	private $random;
-
 	/** @var IConfig */
 	protected $config;
 
@@ -63,16 +56,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;
 	}
@@ -101,101 +91,6 @@ public function generateChangeScript(Schema $targetSchema) {
 		return $script;
 	}
 
-	/**
-	 * @param Schema $targetSchema
-	 * @throws \OC\DB\MigrationException
-	 */
-	public function checkMigrate(Schema $targetSchema) {
-		$this->noEmit = true;
-		/**@var \Doctrine\DBAL\Schema\Table[] $tables */
-		$tables = $targetSchema->getTables();
-		$filterExpression = $this->getFilterExpression();
-		$this->connection->getConfiguration()->
-			setFilterSchemaAssetsExpression($filterExpression);
-		$existingTables = $this->connection->getSchemaManager()->listTableNames();
-
-		$step = 0;
-		foreach ($tables as $table) {
-			if (strpos($table->getName(), '.')) {
-				list(, $tableName) = explode('.', $table->getName());
-			} else {
-				$tableName = $table->getName();
-			}
-			$this->emitCheckStep($tableName, $step++, count($tables));
-			// don't need to check for new tables
-			if (array_search($tableName, $existingTables) !== false) {
-				$this->checkTableMigrate($table);
-			}
-		}
-	}
-
-	/**
-	 * 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 (DBALException $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, [], 0, $table->getOptions());
-	}
-
 	public function createSchema() {
 		$filterExpression = $this->getFilterExpression();
 		$this->connection->getConfiguration()->setFilterSchemaAssetsExpression($filterExpression);
@@ -263,25 +158,6 @@ protected function applySchema(Schema $targetSchema, \Doctrine\DBAL\Connection $
 		$connection->commit();
 	}
 
-	/**
-	 * @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
@@ -306,11 +182,4 @@ protected function emit($sql, $step, $max) {
 		}
 		$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]));
-	}
 }
diff --git a/lib/private/DB/MySQLMigrator.php b/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 @@ protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $conn
 
 		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');
-	}
 }
diff --git a/lib/private/DB/OracleMigrator.php b/lib/private/DB/OracleMigrator.php
@@ -204,14 +204,6 @@ protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $conn
 		return $schemaDiff;
 	}
 
-	/**
-	 * @param string $name
-	 * @return string
-	 */
-	protected function generateTemporaryTableName($name) {
-		return 'oc_' . uniqid();
-	}
-
 	/**
 	 * @param $statement
 	 * @return string

diff --git a/lib/private/DB/SQLiteMigrator.php b/lib/private/DB/SQLiteMigrator.php
@@ -26,40 +26,12 @@
 
 namespace OC\DB;
 
-use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Types\BigIntType;
 use Doctrine\DBAL\Types\Type;
 
 class SQLiteMigrator extends Migrator {
 
-	/**
-	 * @param \Doctrine\DBAL\Schema\Schema $targetSchema
-	 * @throws \OC\DB\MigrationException
-	 *
-	 * For sqlite we simple make a copy of the entire database, and test the migration on that
-	 */
-	public function checkMigrate(\Doctrine\DBAL\Schema\Schema $targetSchema) {
-		$dbFile = $this->connection->getDatabase();
-		$tmpFile = $this->buildTempDatabase();
-		copy($dbFile, $tmpFile);
-
-		$connectionParams = [
-			'path' => $tmpFile,
-			'driver' => 'pdo_sqlite',
-		];
-		$conn = \Doctrine\DBAL\DriverManager::getConnection($connectionParams);
-		try {
-			$this->applySchema($targetSchema, $conn);
-			$conn->close();
-			unlink($tmpFile);
-		} catch (DBALException $e) {
-			$conn->close();
-			unlink($tmpFile);
-			throw new MigrationException('', $e->getMessage());
-		}
-	}
-
 	/**
 	 * @return string
 	 */

diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php
@@ -104,25 +104,6 @@ private function isSQLite() {
 		return $this->connection->getDriver() instanceof \Doctrine\DBAL\Driver\PDOSqlite\Driver;
 	}
 
-
-	public function testDuplicateKeyUpgrade() {
-		$this->expectException(\OC\DB\MigrationException::class);
-
-		if ($this->isSQLite()) {
-			$this->markTestSkipped('sqlite does not throw errors when creating a new key on existing data');
-		}
-		list($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']);
-
-		$migrator->checkMigrate($endSchema);
-		$this->fail('checkMigrate should have failed');
-	}
-
 	public function testUpgrade() {
 		list($startSchema, $endSchema) = $this->getDuplicateKeySchemas();
 		$migrator = $this->manager->getMigrator();
@@ -132,7 +113,6 @@ public function testUpgrade() {
 		$this->connection->insert($this->tableName, ['id' => 2, 'name' => 'bar']);
 		$this->connection->insert($this->tableName, ['id' => 3, 'name' => 'qwerty']);
 
-		$migrator->checkMigrate($endSchema);
 		$migrator->migrate($endSchema);
 		$this->addToAssertionCount(1);
 	}
@@ -151,7 +131,6 @@ public function testUpgradeDifferentPrefix() {
 		$this->connection->insert($this->tableName, ['id' => 2, 'name' => 'bar']);
 		$this->connection->insert($this->tableName, ['id' => 3, 'name' => 'qwerty']);
 
-		$migrator->checkMigrate($endSchema);
 		$migrator->migrate($endSchema);
 		$this->addToAssertionCount(1);
 
@@ -190,7 +169,6 @@ public function testAddingPrimaryKeyWithAutoIncrement() {
 		$migrator = $this->manager->getMigrator();
 		$migrator->migrate($startSchema);
 
-		$migrator->checkMigrate($endSchema);
 		$migrator->migrate($endSchema);
 
 		$this->addToAssertionCount(1);
@@ -212,7 +190,7 @@ public function testReservedKeywords() {
 		$migrator = $this->manager->getMigrator();
 		$migrator->migrate($startSchema);
 
-		$migrator->checkMigrate($endSchema);
+
 		$migrator->migrate($endSchema);
 
 		$this->addToAssertionCount(1);