Skip to content

Commit 32fbcd3

Browse files
szaimenszaimen
authored
Merge pull request #43445 from nextcloud/backport/43357/stable28
[stable28] fix(migration): Make naming constraint fail softer on updates
2 parents 62a8f2d + 1412eed commit 32fbcd3

File tree

1 file changed

+49
-10
lines changed

1 file changed

+49
-10
lines changed

‎lib/private/DB/MigrationService.php‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
use OCP\DB\ISchemaWrapper;
4444
use OCP\Migration\IMigrationStep;
4545
use OCP\Migration\IOutput;
46+
use OCP\Server;
4647
use Psr\Log\LoggerInterface;
4748

4849
class MigrationService {
@@ -51,18 +52,24 @@ class MigrationService {
5152
private string $migrationsPath;
5253
private string $migrationsNamespace;
5354
private IOutput $output;
55+
private LoggerInterface $logger;
5456
private Connection $connection;
5557
private string $appName;
5658
private bool $checkOracle;
5759

5860
/**
5961
* @throws \Exception
6062
*/
61-
public function __construct(string $appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null) {
63+
public function __construct(string $appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null, ?LoggerInterface $logger = null) {
6264
$this->appName = $appName;
6365
$this->connection = $connection;
66+
if ($logger === null) {
67+
$this->logger = Server::get(LoggerInterface::class);
68+
} else {
69+
$this->logger = $logger;
70+
}
6471
if ($output === null) {
65-
$this->output = new SimpleOutput(\OC::$server->get(LoggerInterface::class), $appName);
72+
$this->output = new SimpleOutput($this->logger, $appName);
6673
} else {
6774
$this->output = $output;
6875
}
@@ -433,7 +440,7 @@ public function migrateSchemaOnly(string $to = 'latest'): void {
433440
if ($toSchema instanceof SchemaWrapper) {
434441
$this->output->debug('- Checking target database schema');
435442
$targetSchema = $toSchema->getWrappedSchema();
436-
$this->ensureUniqueNamesConstraints($targetSchema);
443+
$this->ensureUniqueNamesConstraints($targetSchema, true);
437444
if ($this->checkOracle) {
438445
$beforeSchema = $this->connection->createSchema();
439446
$this->ensureOracleConstraints($beforeSchema, $targetSchema, strlen($this->connection->getPrefix()));
@@ -514,7 +521,7 @@ public function executeStep($version, $schemaOnly = false) {
514521

515522
if ($toSchema instanceof SchemaWrapper) {
516523
$targetSchema = $toSchema->getWrappedSchema();
517-
$this->ensureUniqueNamesConstraints($targetSchema);
524+
$this->ensureUniqueNamesConstraints($targetSchema, $schemaOnly);
518525
if ($this->checkOracle) {
519526
$sourceSchema = $this->connection->createSchema();
520527
$this->ensureOracleConstraints($sourceSchema, $targetSchema, strlen($this->connection->getPrefix()));
@@ -650,14 +657,26 @@ public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSche
650657
}
651658

652659
/**
660+
* Ensure naming constraints
661+
*
653662
* Naming constraints:
654663
* - Index, sequence and primary key names must be unique within a Postgres Schema
655664
*
665+
* Only on installation we want to break hard, so that all developers notice
666+
* the bugs when installing the app on any database or CI, and can work on
667+
* fixing their migrations before releasing a version incompatible with Postgres.
668+
*
669+
* In case of updates we might be running on production instances and the
670+
* administrators being faced with the error would not know how to resolve it
671+
* anyway. This can also happen with instances, that had the issue before the
672+
* current update, so we don't want to make their life more complicated
673+
* than needed.
674+
*
656675
* @param Schema $targetSchema
676+
* @param bool $isInstalling
657677
*/
658-
public function ensureUniqueNamesConstraints(Schema $targetSchema): void {
678+
public function ensureUniqueNamesConstraints(Schema $targetSchema, bool $isInstalling): void {
659679
$constraintNames = [];
660-
661680
$sequences = $targetSchema->getSequences();
662681

663682
foreach ($targetSchema->getTables() as $table) {
@@ -668,14 +687,20 @@ public function ensureUniqueNamesConstraints(Schema $targetSchema): void {
668687
}
669688

670689
if (isset($constraintNames[$thing->getName()])) {
671-
throw new \InvalidArgumentException('Index name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
690+
if ($isInstalling) {
691+
throw new \InvalidArgumentException('Index name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
692+
}
693+
$this->logErrorOrWarning('Index name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
672694
}
673695
$constraintNames[$thing->getName()] = $table->getName();
674696
}
675697

676698
foreach ($table->getForeignKeys() as $thing) {
677699
if (isset($constraintNames[$thing->getName()])) {
678-
throw new \InvalidArgumentException('Foreign key name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
700+
if ($isInstalling) {
701+
throw new \InvalidArgumentException('Foreign key name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
702+
}
703+
$this->logErrorOrWarning('Foreign key name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
679704
}
680705
$constraintNames[$thing->getName()] = $table->getName();
681706
}
@@ -688,20 +713,34 @@ public function ensureUniqueNamesConstraints(Schema $targetSchema): void {
688713
}
689714

690715
if (isset($constraintNames[$indexName])) {
691-
throw new \InvalidArgumentException('Primary index name "' . $indexName . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
716+
if ($isInstalling) {
717+
throw new \InvalidArgumentException('Primary index name "' . $indexName . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
718+
}
719+
$this->logErrorOrWarning('Primary index name "' . $indexName . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
692720
}
693721
$constraintNames[$indexName] = $table->getName();
694722
}
695723
}
696724

697725
foreach ($sequences as $sequence) {
698726
if (isset($constraintNames[$sequence->getName()])) {
699-
throw new \InvalidArgumentException('Sequence name "' . $sequence->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
727+
if ($isInstalling) {
728+
throw new \InvalidArgumentException('Sequence name "' . $sequence->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
729+
}
730+
$this->logErrorOrWarning('Sequence name "' . $sequence->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".');
700731
}
701732
$constraintNames[$sequence->getName()] = 'sequence';
702733
}
703734
}
704735

736+
protected function logErrorOrWarning(string $log): void {
737+
if ($this->output instanceof SimpleOutput) {
738+
$this->output->warning($log);
739+
} else {
740+
$this->logger->error($log);
741+
}
742+
}
743+
705744
private function ensureMigrationsAreLoaded() {
706745
if (empty($this->migrations)) {
707746
$this->migrations = $this->findMigrations();

0 commit comments

Comments
 (0)