Merge pull request #39506 from nextcloud/techdebt/39488/ensure-index-…

…name-uniqueness feat(db): Ensure that index names are unique across the database
nextcloud · Jul 24, 2023 · 7548e62 · 7548e62
2 parents 0411cc6 + b80692d
commit 7548e62
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 2 deletions.
diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php
@@ -448,6 +448,7 @@ public function migrateSchemaOnly($to = 'latest') {
 
 		if ($toSchema instanceof SchemaWrapper) {
 			$targetSchema = $toSchema->getWrappedSchema();
+			$this->ensureUniqueNamesConstraints($targetSchema);
 			if ($this->checkOracle) {
 				$beforeSchema = $this->connection->createSchema();
 				$this->ensureOracleConstraints($beforeSchema, $targetSchema, strlen($this->connection->getPrefix()));
@@ -525,6 +526,7 @@ public function executeStep($version, $schemaOnly = false) {
 
 		if ($toSchema instanceof SchemaWrapper) {
 			$targetSchema = $toSchema->getWrappedSchema();
+			$this->ensureUniqueNamesConstraints($targetSchema);
 			if ($this->checkOracle) {
 				$sourceSchema = $this->connection->createSchema();
 				$this->ensureOracleConstraints($sourceSchema, $targetSchema, strlen($this->connection->getPrefix()));
@@ -659,6 +661,59 @@ public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSche
 		}
 	}
 
+	/**
+	 * Naming constraints:
+	 * - Index, sequence and primary key names must be unique within a Postgres Schema
+	 *
+	 * @param Schema $targetSchema
+	 */
+	public function ensureUniqueNamesConstraints(Schema $targetSchema): void {
+		$constraintNames = [];
+
+		$sequences = $targetSchema->getSequences();
+
+		foreach ($targetSchema->getTables() as $table) {
+			foreach ($table->getIndexes() as $thing) {
+				$indexName = strtolower($thing->getName());
+				if ($indexName === 'primary' || $thing->isPrimary()) {
+					continue;
+				}
+
+				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()] . '".');
+				}
+				$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()] . '".');
+				}
+				$constraintNames[$thing->getName()] = $table->getName();
+			}
+
+			$primaryKey = $table->getPrimaryKey();
+			if ($primaryKey instanceof Index) {
+				$indexName = strtolower($primaryKey->getName());
+				if ($indexName === 'primary') {
+					continue;
+				}
+
+				if (isset($constraintNames[$indexName])) {
+					throw new \InvalidArgumentException('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()] . '".');
+			}
+			$constraintNames[$sequence->getName()] = 'sequence';
+		}
+	}
+
 	private function ensureMigrationsAreLoaded() {
 		if (empty($this->migrations)) {
 			$this->migrations = $this->findMigrations();

diff --git a/tests/lib/DB/MigrationsTest.php b/tests/lib/DB/MigrationsTest.php
@@ -101,10 +101,10 @@ public function testExecuteStepWithSchemaChange() {
 			->method('migrateToSchema');
 
 		$wrappedSchema = $this->createMock(Schema::class);
-		$wrappedSchema->expects($this->once())
+		$wrappedSchema->expects($this->exactly(2))
 			->method('getTables')
 			->willReturn([]);
-		$wrappedSchema->expects($this->once())
+		$wrappedSchema->expects($this->exactly(2))
 			->method('getSequences')
 			->willReturn([]);