Skip to content

Commit

Permalink
Merge pull request #43445 from nextcloud/backport/43357/stable28
Browse files Browse the repository at this point in the history 
[stable28] fix(migration): Make naming constraint fail softer on updates
  • Loading branch information
@szaimen
szaimen authored Feb 8, 2024
2 parents 62a8f2d + 1412eed commit 32fbcd3
Showing 1 changed file with 49 additions and 10 deletions.
59 changes: 49 additions & 10 deletions lib/private/DB/MigrationService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -51,18 +52,24 @@ class MigrationService {
private string $migrationsPath;
private string $migrationsNamespace;
private IOutput $output;
private LoggerInterface $logger;
private Connection $connection;
private string $appName;
private bool $checkOracle;

/**
* @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;
}
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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) {
Expand All @@ -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();
}
Expand All @@ -688,20 +713,34 @@ 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();
}
}

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();
Expand Down

0 comments on commit 32fbcd3

Please sign in to comment.