Skip to content

Commit 37feee4

Browse files
MorrisJobkeMorrisJobke
authored
Merge pull request #24384 from nextcloud/cleanup/remove-old-migration-check
Remove dead code from checking core/apps before upgrades
2 parents 459e0b3 + a2f3077 commit 37feee4

File tree

5 files changed

+5
-191
lines changed

5 files changed

+5
-191
lines changed

‎lib/private/DB/MDB2SchemaManager.php

+5-5
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,15 @@ public function getMigrator() {
7171
$config = \OC::$server->getConfig();
7272
$dispatcher = \OC::$server->getEventDispatcher();
7373
if ($platform instanceof SqlitePlatform) {
74-
return new SQLiteMigrator($this->conn, $random, $config, $dispatcher);
74+
return new SQLiteMigrator($this->conn, $config, $dispatcher);
7575
} elseif ($platform instanceof OraclePlatform) {
76-
return new OracleMigrator($this->conn, $random, $config, $dispatcher);
76+
return new OracleMigrator($this->conn, $config, $dispatcher);
7777
} elseif ($platform instanceof MySQLPlatform) {
78-
return new MySQLMigrator($this->conn, $random, $config, $dispatcher);
78+
return new MySQLMigrator($this->conn, $config, $dispatcher);
7979
} elseif ($platform instanceof PostgreSQL94Platform) {
80-
return new PostgreSqlMigrator($this->conn, $random, $config, $dispatcher);
80+
return new PostgreSqlMigrator($this->conn, $config, $dispatcher);
8181
} else {
82-
return new Migrator($this->conn, $random, $config, $dispatcher);
82+
return new Migrator($this->conn, $config, $dispatcher);
8383
}
8484
}
8585

‎lib/private/DB/Migrator.php

-103
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,10 @@
3535
use Doctrine\DBAL\Platforms\MySQLPlatform;
3636
use Doctrine\DBAL\Schema\AbstractAsset;
3737
use Doctrine\DBAL\Schema\Comparator;
38-
use Doctrine\DBAL\Schema\Index;
3938
use Doctrine\DBAL\Schema\Schema;
40-
use Doctrine\DBAL\Schema\SchemaConfig;
41-
use Doctrine\DBAL\Schema\Table;
4239
use Doctrine\DBAL\Types\StringType;
4340
use Doctrine\DBAL\Types\Type;
4441
use OCP\IConfig;
45-
use OCP\Security\ISecureRandom;
4642
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
4743
use Symfony\Component\EventDispatcher\GenericEvent;
4844
use function preg_match;
@@ -52,9 +48,6 @@ class Migrator {
5248
/** @var \Doctrine\DBAL\Connection */
5349
protected $connection;
5450

55-
/** @var ISecureRandom */
56-
private $random;
57-
5851
/** @var IConfig */
5952
protected $config;
6053

@@ -66,16 +59,13 @@ class Migrator {
6659

6760
/**
6861
* @param \Doctrine\DBAL\Connection $connection
69-
* @param ISecureRandom $random
7062
* @param IConfig $config
7163
* @param EventDispatcherInterface $dispatcher
7264
*/
7365
public function __construct(\Doctrine\DBAL\Connection $connection,
74-
ISecureRandom $random,
7566
IConfig $config,
7667
EventDispatcherInterface $dispatcher = null) {
7768
$this->connection = $connection;
78-
$this->random = $random;
7969
$this->config = $config;
8070
$this->dispatcher = $dispatcher;
8171
}
@@ -106,73 +96,6 @@ public function generateChangeScript(Schema $targetSchema) {
10696
return $script;
10797
}
10898

109-
/**
110-
* Create a unique name for the temporary table
111-
*
112-
* @param string $name
113-
* @return string
114-
*/
115-
protected function generateTemporaryTableName($name) {
116-
return $this->config->getSystemValue('dbtableprefix', 'oc_') . $name . '_' . $this->random->generate(13, ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS);
117-
}
118-
119-
/**
120-
* Check the migration of a table on a copy so we can detect errors before messing with the real table
121-
*
122-
* @param \Doctrine\DBAL\Schema\Table $table
123-
* @throws \OC\DB\MigrationException
124-
*/
125-
protected function checkTableMigrate(Table $table) {
126-
$name = $table->getName();
127-
$tmpName = $this->generateTemporaryTableName($name);
128-
129-
$this->copyTable($name, $tmpName);
130-
131-
//create the migration schema for the temporary table
132-
$tmpTable = $this->renameTableSchema($table, $tmpName);
133-
$schemaConfig = new SchemaConfig();
134-
$schemaConfig->setName($this->connection->getDatabase());
135-
$schema = new Schema([$tmpTable], [], $schemaConfig);
136-
137-
try {
138-
$this->applySchema($schema);
139-
$this->dropTable($tmpName);
140-
} catch (Exception $e) {
141-
// pgsql needs to commit it's failed transaction before doing anything else
142-
if ($this->connection->isTransactionActive()) {
143-
$this->connection->commit();
144-
}
145-
$this->dropTable($tmpName);
146-
throw new MigrationException($table->getName(), $e->getMessage());
147-
}
148-
}
149-
150-
/**
151-
* @param \Doctrine\DBAL\Schema\Table $table
152-
* @param string $newName
153-
* @return \Doctrine\DBAL\Schema\Table
154-
*/
155-
protected function renameTableSchema(Table $table, $newName) {
156-
/**
157-
* @var \Doctrine\DBAL\Schema\Index[] $indexes
158-
*/
159-
$indexes = $table->getIndexes();
160-
$newIndexes = [];
161-
foreach ($indexes as $index) {
162-
if ($index->isPrimary()) {
163-
// do not rename primary key
164-
$indexName = $index->getName();
165-
} else {
166-
// avoid conflicts in index names
167-
$indexName = $this->config->getSystemValue('dbtableprefix', 'oc_') . $this->random->generate(13, ISecureRandom::CHAR_LOWER);
168-
}
169-
$newIndexes[] = new Index($indexName, $index->getColumns(), $index->isUnique(), $index->isPrimary());
170-
}
171-
172-
// foreign keys are not supported so we just set it to an empty array
173-
return new Table($newName, $table->getColumns(), $newIndexes, [], [], $table->getOptions());
174-
}
175-
17699
/**
177100
* @throws Exception
178101
*/
@@ -260,25 +183,6 @@ protected function applySchema(Schema $targetSchema, \Doctrine\DBAL\Connection $
260183
}
261184
}
262185

263-
/**
264-
* @param string $sourceName
265-
* @param string $targetName
266-
*/
267-
protected function copyTable($sourceName, $targetName) {
268-
$quotedSource = $this->connection->quoteIdentifier($sourceName);
269-
$quotedTarget = $this->connection->quoteIdentifier($targetName);
270-
271-
$this->connection->exec('CREATE TABLE ' . $quotedTarget . ' (LIKE ' . $quotedSource . ')');
272-
$this->connection->exec('INSERT INTO ' . $quotedTarget . ' SELECT * FROM ' . $quotedSource);
273-
}
274-
275-
/**
276-
* @param string $name
277-
*/
278-
protected function dropTable($name) {
279-
$this->connection->exec('DROP TABLE ' . $this->connection->quoteIdentifier($name));
280-
}
281-
282186
/**
283187
* @param $statement
284188
* @return string
@@ -303,11 +207,4 @@ protected function emit($sql, $step, $max) {
303207
}
304208
$this->dispatcher->dispatch('\OC\DB\Migrator::executeSql', new GenericEvent($sql, [$step + 1, $max]));
305209
}
306-
307-
private function emitCheckStep($tableName, $step, $max) {
308-
if (is_null($this->dispatcher)) {
309-
return;
310-
}
311-
$this->dispatcher->dispatch('\OC\DB\Migrator::checkTable', new GenericEvent($tableName, [$step + 1, $max]));
312-
}
313210
}

‎lib/private/DB/MySQLMigrator.php

-22
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
namespace OC\DB;
2828

2929
use Doctrine\DBAL\Schema\Schema;
30-
use Doctrine\DBAL\Schema\Table;
3130

3231
class MySQLMigrator extends Migrator {
3332
/**
@@ -52,25 +51,4 @@ protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $conn
5251

5352
return $schemaDiff;
5453
}
55-
56-
/**
57-
* Speed up migration test by disabling autocommit and unique indexes check
58-
*
59-
* @param \Doctrine\DBAL\Schema\Table $table
60-
* @throws \OC\DB\MigrationException
61-
*/
62-
protected function checkTableMigrate(Table $table) {
63-
$this->connection->exec('SET autocommit=0');
64-
$this->connection->exec('SET unique_checks=0');
65-
66-
try {
67-
parent::checkTableMigrate($table);
68-
} catch (\Exception $e) {
69-
$this->connection->exec('SET unique_checks=1');
70-
$this->connection->exec('SET autocommit=1');
71-
throw new MigrationException($table->getName(), $e->getMessage());
72-
}
73-
$this->connection->exec('SET unique_checks=1');
74-
$this->connection->exec('SET autocommit=1');
75-
}
7654
}

‎lib/private/DB/OracleMigrator.php

-8
Original file line numberDiff line numberDiff line change
@@ -204,14 +204,6 @@ protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $conn
204204
return $schemaDiff;
205205
}
206206

207-
/**
208-
* @param string $name
209-
* @return string
210-
*/
211-
protected function generateTemporaryTableName($name) {
212-
return 'oc_' . uniqid();
213-
}
214-
215207
/**
216208
* @param $statement
217209
* @return string

‎tests/lib/DB/MigratorTest.php

-53
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use Doctrine\DBAL\Platforms\SqlitePlatform;
1616
use Doctrine\DBAL\Schema\Schema;
1717
use Doctrine\DBAL\Schema\SchemaConfig;
18-
use OC\DB\SchemaWrapper;
1918
use OCP\IConfig;
2019

2120
/**
@@ -131,58 +130,6 @@ private function isMySQL() {
131130
return $this->connection->getDatabasePlatform() instanceof MySQLPlatform;
132131
}
133132

134-
135-
public function testDuplicateKeyUpgrade() {
136-
$this->expectException(Exception\UniqueConstraintViolationException::class);
137-
138-
if ($this->isSQLite()) {
139-
$this->markTestSkipped('sqlite does not throw errors when creating a new key on existing data');
140-
}
141-
[$startSchema, $endSchema] = $this->getDuplicateKeySchemas();
142-
$migrator = $this->manager->getMigrator();
143-
$migrator->migrate($startSchema);
144-
145-
$this->connection->insert($this->tableName, ['id' => 1, 'name' => 'foo']);
146-
$this->connection->insert($this->tableName, ['id' => 2, 'name' => 'bar']);
147-
$this->connection->insert($this->tableName, ['id' => 2, 'name' => 'qwerty']);
148-
149-
try {
150-
$migrator->migrate($endSchema);
151-
} catch (Exception\UniqueConstraintViolationException $e) {
152-
if (!$this->isMySQL()) {
153-
$this->connection->rollBack();
154-
}
155-
throw $e;
156-
}
157-
}
158-
159-
public function testChangeToString() {
160-
[$startSchema, $endSchema] = $this->getChangedTypeSchema('integer', 'string');
161-
$migrator = $this->manager->getMigrator();
162-
$migrator->migrate($startSchema);
163-
$schema = new SchemaWrapper($this->connection);
164-
$table = $schema->getTable(substr($this->tableName, 3));
165-
$this->assertEquals('integer', $table->getColumn('id')->getType()->getName());
166-
167-
$this->connection->insert($this->tableName, ['id' => 1, 'name' => 'foo']);
168-
$this->connection->insert($this->tableName, ['id' => 2, 'name' => 'bar']);
169-
$this->connection->insert($this->tableName, ['id' => 3, 'name' => 'qwerty']);
170-
171-
$migrator->migrate($endSchema);
172-
$this->addToAssertionCount(1);
173-
174-
$qb = $this->connection->getQueryBuilder();
175-
$result = $qb->select('*')->from(substr($this->tableName, 3))->execute();
176-
$this->assertEquals([
177-
['id' => 1, 'name' => 'foo'],
178-
['id' => 2, 'name' => 'bar'],
179-
['id' => 3, 'name' => 'qwerty']
180-
], $result->fetchAll());
181-
$schema = new SchemaWrapper($this->connection);
182-
$table = $schema->getTable(substr($this->tableName, 3));
183-
$this->assertEquals('string', $table->getColumn('id')->getType()->getName());
184-
}
185-
186133
public function testUpgrade() {
187134
[$startSchema, $endSchema] = $this->getDuplicateKeySchemas();
188135
$migrator = $this->manager->getMigrator();

0 commit comments

Comments
 (0)