Skip to content

Commit 6895230

Browse files
blizzzblizzz
authored
Merge pull request #10555 from nextcloud/bugfix/10518/only-check-added-items
Only enforce schema limits for supported apps
2 parents c739937 + 85a0e10 commit 6895230

File tree

2 files changed

+134
-28
lines changed

2 files changed

+134
-28
lines changed

‎lib/private/DB/MigrationService.php‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,12 @@
2525

2626
use Doctrine\DBAL\Platforms\OraclePlatform;
2727
use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
28-
use Doctrine\DBAL\Schema\Column;
2928
use Doctrine\DBAL\Schema\Index;
3029
use Doctrine\DBAL\Schema\Schema;
3130
use Doctrine\DBAL\Schema\SchemaException;
3231
use Doctrine\DBAL\Schema\Sequence;
32+
use Doctrine\DBAL\Schema\Table;
33+
use OC\App\InfoParser;
3334
use OC\IntegrityCheck\Helpers\AppLocator;
3435
use OC\Migration\SimpleOutput;
3536
use OCP\AppFramework\App;
@@ -51,6 +52,8 @@ class MigrationService {
5152
private $connection;
5253
/** @var string */
5354
private $appName;
55+
/** @var bool */
56+
private $checkOracle;
5457

5558
/**
5659
* MigrationService constructor.
@@ -72,6 +75,7 @@ public function __construct($appName, IDBConnection $connection, IOutput $output
7275
if ($appName === 'core') {
7376
$this->migrationsPath = \OC::$SERVERROOT . '/core/Migrations';
7477
$this->migrationsNamespace = 'OC\\Core\\Migrations';
78+
$this->checkOracle = true;
7579
} else {
7680
if (null === $appLocator) {
7781
$appLocator = new AppLocator();
@@ -80,6 +84,21 @@ public function __construct($appName, IDBConnection $connection, IOutput $output
8084
$namespace = App::buildAppNamespace($appName);
8185
$this->migrationsPath = "$appPath/lib/Migration";
8286
$this->migrationsNamespace = $namespace . '\\Migration';
87+
88+
$infoParser = new InfoParser();
89+
$info = $infoParser->parse($appPath . '/appinfo/info.xml');
90+
if (!isset($info['dependencies']['database'])) {
91+
$this->checkOracle = true;
92+
} else {
93+
$this->checkOracle = false;
94+
foreach ($info['dependencies']['database'] as $database) {
95+
if (\is_string($database) && $database === 'oci') {
96+
$this->checkOracle = true;
97+
} else if (\is_array($database) && isset($database['@value']) && $database['@value'] === 'oci') {
98+
$this->checkOracle = true;
99+
}
100+
}
101+
}
83102
}
84103
}
85104

@@ -457,8 +476,10 @@ public function executeStep($version, $schemaOnly = false) {
457476

458477
if ($toSchema instanceof SchemaWrapper) {
459478
$targetSchema = $toSchema->getWrappedSchema();
460-
// TODO re-enable once stable14 is branched of: https://github.com/nextcloud/server/issues/10518
461-
// $this->ensureOracleIdentifierLengthLimit($targetSchema, strlen($this->connection->getPrefix()));
479+
if ($this->checkOracle) {
480+
$sourceSchema = $this->connection->createSchema();
481+
$this->ensureOracleIdentifierLengthLimit($sourceSchema, $targetSchema, strlen($this->connection->getPrefix()));
482+
}
462483
$this->connection->migrateToSchema($targetSchema);
463484
$toSchema->performDropTableCalls();
464485
}
@@ -472,34 +493,39 @@ public function executeStep($version, $schemaOnly = false) {
472493
$this->markAsExecuted($version);
473494
}
474495

475-
public function ensureOracleIdentifierLengthLimit(Schema $schema, int $prefixLength) {
476-
$sequences = $schema->getSequences();
496+
public function ensureOracleIdentifierLengthLimit(Schema $sourceSchema, Schema $targetSchema, int $prefixLength) {
497+
$sequences = $targetSchema->getSequences();
477498

478-
foreach ($schema->getTables() as $table) {
479-
if (\strlen($table->getName()) - $prefixLength > 27) {
480-
throw new \InvalidArgumentException('Table name "' . $table->getName() . '" is too long.');
499+
foreach ($targetSchema->getTables() as $table) {
500+
try {
501+
$sourceTable = $sourceSchema->getTable($table->getName());
502+
} catch (SchemaException $e) {
503+
if (\strlen($table->getName()) - $prefixLength > 27) {
504+
throw new \InvalidArgumentException('Table name "' . $table->getName() . '" is too long.');
505+
}
506+
$sourceTable = null;
481507
}
482508

483509
foreach ($table->getColumns() as $thing) {
484-
if (\strlen($thing->getName()) - $prefixLength > 27) {
510+
if ((!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName())) && \strlen($thing->getName()) - $prefixLength > 27) {
485511
throw new \InvalidArgumentException('Column name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
486512
}
487513
}
488514

489515
foreach ($table->getIndexes() as $thing) {
490-
if (\strlen($thing->getName()) - $prefixLength > 27) {
516+
if ((!$sourceTable instanceof Table || !$sourceTable->hasIndex($thing->getName())) && \strlen($thing->getName()) - $prefixLength > 27) {
491517
throw new \InvalidArgumentException('Index name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
492518
}
493519
}
494520

495521
foreach ($table->getForeignKeys() as $thing) {
496-
if (\strlen($thing->getName()) - $prefixLength > 27) {
522+
if ((!$sourceTable instanceof Table || !$sourceTable->hasForeignKey($thing->getName())) && \strlen($thing->getName()) - $prefixLength > 27) {
497523
throw new \InvalidArgumentException('Foreign key name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
498524
}
499525
}
500526

501527
$primaryKey = $table->getPrimaryKey();
502-
if ($primaryKey instanceof Index) {
528+
if ($primaryKey instanceof Index && (!$sourceTable instanceof Table || !$sourceTable->hasPrimaryKey())) {
503529
$indexName = strtolower($primaryKey->getName());
504530
$isUsingDefaultName = $indexName === 'primary';
505531

@@ -528,7 +554,7 @@ public function ensureOracleIdentifierLengthLimit(Schema $schema, int $prefixLen
528554
}
529555

530556
foreach ($sequences as $sequence) {
531-
if (\strlen($sequence->getName()) - $prefixLength > 27) {
557+
if (!$sourceSchema->hasSequence($sequence->getName()) && \strlen($sequence->getName()) - $prefixLength > 27) {
532558
throw new \InvalidArgumentException('Sequence name "' . $sequence->getName() . '" is too long.');
533559
}
534560
}

‎tests/lib/DB/MigrationsTest.php‎

Lines changed: 95 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Doctrine\DBAL\Schema\ForeignKeyConstraint;
1717
use Doctrine\DBAL\Schema\Index;
1818
use Doctrine\DBAL\Schema\Schema;
19+
use Doctrine\DBAL\Schema\SchemaException;
1920
use Doctrine\DBAL\Schema\Sequence;
2021
use Doctrine\DBAL\Schema\Table;
2122
use OC\DB\Connection;
@@ -102,13 +103,12 @@ public function testExecuteStepWithSchemaChange() {
102103
->method('migrateToSchema');
103104

104105
$wrappedSchema = $this->createMock(Schema::class);
105-
// TODO re-enable once stable14 is branched of: https://github.com/nextcloud/server/issues/10518
106-
/*$wrappedSchema->expects($this->once())
106+
$wrappedSchema->expects($this->once())
107107
->method('getTables')
108108
->willReturn([]);
109109
$wrappedSchema->expects($this->once())
110110
->method('getSequences')
111-
->willReturn([]);*/
111+
->willReturn([]);
112112

113113
$schemaResult = $this->createMock(SchemaWrapper::class);
114114
$schemaResult->expects($this->once())
@@ -239,12 +239,12 @@ public function testEnsureOracleIdentifierLengthLimitValid() {
239239
->willReturn(\str_repeat('a', 30));
240240

241241
$table = $this->createMock(Table::class);
242-
$table->expects($this->once())
242+
$table->expects($this->atLeastOnce())
243243
->method('getName')
244244
->willReturn(\str_repeat('a', 30));
245245

246246
$sequence = $this->createMock(Sequence::class);
247-
$sequence->expects($this->once())
247+
$sequence->expects($this->atLeastOnce())
248248
->method('getName')
249249
->willReturn(\str_repeat('a', 30));
250250

@@ -269,7 +269,15 @@ public function testEnsureOracleIdentifierLengthLimitValid() {
269269
->method('getSequences')
270270
->willReturn([$sequence]);
271271

272-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
272+
$sourceSchema = $this->createMock(Schema::class);
273+
$sourceSchema->expects($this->any())
274+
->method('getTable')
275+
->willThrowException(new SchemaException());
276+
$sourceSchema->expects($this->any())
277+
->method('hasSequence')
278+
->willReturn(false);
279+
280+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
273281
}
274282

275283
public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKey() {
@@ -304,7 +312,15 @@ public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKey() {
304312
->method('getSequences')
305313
->willReturn([]);
306314

307-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
315+
$sourceSchema = $this->createMock(Schema::class);
316+
$sourceSchema->expects($this->any())
317+
->method('getTable')
318+
->willThrowException(new SchemaException());
319+
$sourceSchema->expects($this->any())
320+
->method('hasSequence')
321+
->willReturn(false);
322+
323+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
308324
}
309325

310326
public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKeyDefault() {
@@ -349,7 +365,15 @@ public function testEnsureOracleIdentifierLengthLimitValidWithPrimaryKeyDefault(
349365
->method('getSequences')
350366
->willReturn([]);
351367

352-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
368+
$sourceSchema = $this->createMock(Schema::class);
369+
$sourceSchema->expects($this->any())
370+
->method('getTable')
371+
->willThrowException(new SchemaException());
372+
$sourceSchema->expects($this->any())
373+
->method('hasSequence')
374+
->willReturn(false);
375+
376+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
353377
}
354378

355379
/**
@@ -366,7 +390,15 @@ public function testEnsureOracleIdentifierLengthLimitTooLongTableName() {
366390
->method('getTables')
367391
->willReturn([$table]);
368392

369-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
393+
$sourceSchema = $this->createMock(Schema::class);
394+
$sourceSchema->expects($this->any())
395+
->method('getTable')
396+
->willThrowException(new SchemaException());
397+
$sourceSchema->expects($this->any())
398+
->method('hasSequence')
399+
->willReturn(false);
400+
401+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
370402
}
371403

372404
/**
@@ -411,7 +443,15 @@ public function testEnsureOracleIdentifierLengthLimitTooLongPrimaryWithDefault()
411443
->method('getTables')
412444
->willReturn([$table]);
413445

414-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
446+
$sourceSchema = $this->createMock(Schema::class);
447+
$sourceSchema->expects($this->any())
448+
->method('getTable')
449+
->willThrowException(new SchemaException());
450+
$sourceSchema->expects($this->any())
451+
->method('hasSequence')
452+
->willReturn(false);
453+
454+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
415455
}
416456

417457
/**
@@ -446,7 +486,15 @@ public function testEnsureOracleIdentifierLengthLimitTooLongPrimaryWithName() {
446486
->method('getTables')
447487
->willReturn([$table]);
448488

449-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
489+
$sourceSchema = $this->createMock(Schema::class);
490+
$sourceSchema->expects($this->any())
491+
->method('getTable')
492+
->willThrowException(new SchemaException());
493+
$sourceSchema->expects($this->any())
494+
->method('hasSequence')
495+
->willReturn(false);
496+
497+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
450498
}
451499

452500
/**
@@ -472,7 +520,15 @@ public function testEnsureOracleIdentifierLengthLimitTooLongColumnName() {
472520
->method('getTables')
473521
->willReturn([$table]);
474522

475-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
523+
$sourceSchema = $this->createMock(Schema::class);
524+
$sourceSchema->expects($this->any())
525+
->method('getTable')
526+
->willThrowException(new SchemaException());
527+
$sourceSchema->expects($this->any())
528+
->method('hasSequence')
529+
->willReturn(false);
530+
531+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
476532
}
477533

478534
/**
@@ -501,7 +557,15 @@ public function testEnsureOracleIdentifierLengthLimitTooLongIndexName() {
501557
->method('getTables')
502558
->willReturn([$table]);
503559

504-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
560+
$sourceSchema = $this->createMock(Schema::class);
561+
$sourceSchema->expects($this->any())
562+
->method('getTable')
563+
->willThrowException(new SchemaException());
564+
$sourceSchema->expects($this->any())
565+
->method('hasSequence')
566+
->willReturn(false);
567+
568+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
505569
}
506570

507571
/**
@@ -533,7 +597,15 @@ public function testEnsureOracleIdentifierLengthLimitTooLongForeignKeyName() {
533597
->method('getTables')
534598
->willReturn([$table]);
535599

536-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
600+
$sourceSchema = $this->createMock(Schema::class);
601+
$sourceSchema->expects($this->any())
602+
->method('getTable')
603+
->willThrowException(new SchemaException());
604+
$sourceSchema->expects($this->any())
605+
->method('hasSequence')
606+
->willReturn(false);
607+
608+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
537609
}
538610

539611
/**
@@ -553,6 +625,14 @@ public function testEnsureOracleIdentifierLengthLimitTooLongSequenceName() {
553625
->method('getSequences')
554626
->willReturn([$sequence]);
555627

556-
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$schema, 3]);
628+
$sourceSchema = $this->createMock(Schema::class);
629+
$sourceSchema->expects($this->any())
630+
->method('getTable')
631+
->willThrowException(new SchemaException());
632+
$sourceSchema->expects($this->any())
633+
->method('hasSequence')
634+
->willReturn(false);
635+
636+
self::invokePrivate($this->migrationService, 'ensureOracleIdentifierLengthLimit', [$sourceSchema, $schema, 3]);
557637
}
558638
}

0 commit comments

Comments
 (0)