Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dead code from checking core/apps before upgrades #24384

Merged
merged 1 commit into from
Mar 17, 2021
Merged

Remove dead code from checking core/apps before upgrades #24384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions lib/private/DB/MDB2SchemaManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,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 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);
}
}

Expand Down
103 changes: 0 additions & 103 deletions lib/private/DB/Migrator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,10 @@
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;
Expand All @@ -52,9 +48,6 @@ class Migrator {
/** @var \Doctrine\DBAL\Connection */
protected $connection;

/** @var ISecureRandom */
private $random;

/** @var IConfig */
protected $config;

Expand All @@ -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;
}
Expand Down Expand Up @@ -106,73 +96,6 @@ public function generateChangeScript(Schema $targetSchema) {
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
*/
Expand Down Expand Up @@ -260,25 +183,6 @@ protected function applySchema(Schema $targetSchema, \Doctrine\DBAL\Connection $
}
}

/**
* @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
Expand All @@ -303,11 +207,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]));
}
}
22 changes: 0 additions & 22 deletions lib/private/DB/MySQLMigrator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
namespace OC\DB;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\DBAL\Schema\Table;

class MySQLMigrator extends Migrator {
/**
Expand All @@ -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');
}
}
8 changes: 0 additions & 8 deletions lib/private/DB/OracleMigrator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 0 additions & 53 deletions tests/lib/DB/MigratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Doctrine\DBAL\Platforms\SqlitePlatform;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\DBAL\Schema\SchemaConfig;
use OC\DB\SchemaWrapper;
use OCP\IConfig;

/**
Expand Down Expand Up @@ -131,58 +130,6 @@ private function isMySQL() {
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();
Expand Down