diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index 60f9b65cd5fdd..f885422c928f3 100644 --- a/lib/private/DB/MigrationService.php +++ b/lib/private/DB/MigrationService.php @@ -43,6 +43,7 @@ use OCP\DB\ISchemaWrapper; use OCP\Migration\IMigrationStep; use OCP\Migration\IOutput; +use OCP\Server; use Psr\Log\LoggerInterface; class MigrationService { @@ -51,6 +52,7 @@ class MigrationService { private string $migrationsPath; private string $migrationsNamespace; private IOutput $output; + private LoggerInterface $logger; private Connection $connection; private string $appName; private bool $checkOracle; @@ -58,11 +60,16 @@ class MigrationService { /** * @throws \Exception */ - public function __construct(string $appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null) { + public function __construct(string $appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null, ?LoggerInterface $logger = null) { $this->appName = $appName; $this->connection = $connection; + if ($logger === null) { + $this->logger = Server::get(LoggerInterface::class); + } else { + $this->logger = $logger; + } if ($output === null) { - $this->output = new SimpleOutput(\OC::$server->get(LoggerInterface::class), $appName); + $this->output = new SimpleOutput($this->logger, $appName); } else { $this->output = $output; } @@ -433,7 +440,7 @@ public function migrateSchemaOnly(string $to = 'latest'): void { if ($toSchema instanceof SchemaWrapper) { $this->output->debug('- Checking target database schema'); $targetSchema = $toSchema->getWrappedSchema(); - $this->ensureUniqueNamesConstraints($targetSchema); + $this->ensureUniqueNamesConstraints($targetSchema, true); if ($this->checkOracle) { $beforeSchema = $this->connection->createSchema(); $this->ensureOracleConstraints($beforeSchema, $targetSchema, strlen($this->connection->getPrefix())); @@ -514,7 +521,7 @@ public function executeStep($version, $schemaOnly = false) { if ($toSchema instanceof SchemaWrapper) { $targetSchema = $toSchema->getWrappedSchema(); - $this->ensureUniqueNamesConstraints($targetSchema); + $this->ensureUniqueNamesConstraints($targetSchema, $schemaOnly); if ($this->checkOracle) { $sourceSchema = $this->connection->createSchema(); $this->ensureOracleConstraints($sourceSchema, $targetSchema, strlen($this->connection->getPrefix())); @@ -650,14 +657,26 @@ public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSche } /** + * Ensure naming constraints + * * Naming constraints: * - Index, sequence and primary key names must be unique within a Postgres Schema * + * Only on installation we want to break hard, so that all developers notice + * the bugs when installing the app on any database or CI, and can work on + * fixing their migrations before releasing a version incompatible with Postgres. + * + * In case of updates we might be running on production instances and the + * administrators being faced with the error would not know how to resolve it + * anyway. This can also happen with instances, that had the issue before the + * current update, so we don't want to make their life more complicated + * than needed. + * * @param Schema $targetSchema + * @param bool $isInstalling */ - public function ensureUniqueNamesConstraints(Schema $targetSchema): void { + public function ensureUniqueNamesConstraints(Schema $targetSchema, bool $isInstalling): void { $constraintNames = []; - $sequences = $targetSchema->getSequences(); foreach ($targetSchema->getTables() as $table) { @@ -668,14 +687,20 @@ public function ensureUniqueNamesConstraints(Schema $targetSchema): void { } if (isset($constraintNames[$thing->getName()])) { - throw new \InvalidArgumentException('Index name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); + if ($isInstalling) { + throw new \InvalidArgumentException('Index name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); + } + $this->logErrorOrWarning('Index name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); } $constraintNames[$thing->getName()] = $table->getName(); } foreach ($table->getForeignKeys() as $thing) { if (isset($constraintNames[$thing->getName()])) { - throw new \InvalidArgumentException('Foreign key name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); + if ($isInstalling) { + throw new \InvalidArgumentException('Foreign key name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); + } + $this->logErrorOrWarning('Foreign key name "' . $thing->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); } $constraintNames[$thing->getName()] = $table->getName(); } @@ -688,7 +713,10 @@ public function ensureUniqueNamesConstraints(Schema $targetSchema): void { } if (isset($constraintNames[$indexName])) { - throw new \InvalidArgumentException('Primary index name "' . $indexName . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); + if ($isInstalling) { + throw new \InvalidArgumentException('Primary index name "' . $indexName . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); + } + $this->logErrorOrWarning('Primary index name "' . $indexName . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); } $constraintNames[$indexName] = $table->getName(); } @@ -696,12 +724,23 @@ public function ensureUniqueNamesConstraints(Schema $targetSchema): void { foreach ($sequences as $sequence) { if (isset($constraintNames[$sequence->getName()])) { - throw new \InvalidArgumentException('Sequence name "' . $sequence->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); + if ($isInstalling) { + throw new \InvalidArgumentException('Sequence name "' . $sequence->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); + } + $this->logErrorOrWarning('Sequence name "' . $sequence->getName() . '" for table "' . $table->getName() . '" collides with the constraint on table "' . $constraintNames[$thing->getName()] . '".'); } $constraintNames[$sequence->getName()] = 'sequence'; } } + protected function logErrorOrWarning(string $log): void { + if ($this->output instanceof SimpleOutput) { + $this->output->warning($log); + } else { + $this->logger->error($log); + } + } + private function ensureMigrationsAreLoaded() { if (empty($this->migrations)) { $this->migrations = $this->findMigrations();