From a2f3077ee802b4b291ba4f846bfceb69ec0f225f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 26 Nov 2020 11:10:41 +0100 Subject: [PATCH] Skip db migration simulation for core schema changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/private/DB/MDB2SchemaManager.php | 10 +-- lib/private/DB/Migrator.php | 103 --------------------------- lib/private/DB/MySQLMigrator.php | 22 ------ lib/private/DB/OracleMigrator.php | 8 --- tests/lib/DB/MigratorTest.php | 53 -------------- 5 files changed, 5 insertions(+), 191 deletions(-) diff --git a/lib/private/DB/MDB2SchemaManager.php b/lib/private/DB/MDB2SchemaManager.php index 18549eb1fa..30c4a87e5b 100644 --- a/lib/private/DB/MDB2SchemaManager.php +++ b/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); } } diff --git a/lib/private/DB/Migrator.php b/lib/private/DB/Migrator.php index 609ed5d6f7..d7bd9115c6 100644 --- a/lib/private/DB/Migrator.php +++ b/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])); - } } diff --git a/lib/private/DB/MySQLMigrator.php b/lib/private/DB/MySQLMigrator.php index 58c54683a3..00c29d7a6f 100644 --- 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 @@ 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'); - } } diff --git a/lib/private/DB/OracleMigrator.php b/lib/private/DB/OracleMigrator.php index 7a327a4279..e1c0c79ef5 100644 --- a/lib/private/DB/OracleMigrator.php +++ b/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 diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php index 6b75853489..12ff3bd074 100644 --- a/tests/lib/DB/MigratorTest.php +++ b/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();