Merge pull request #31513 from nextcloud/techdebt/noid/enforce-primary-keys

Enforce primary keys as they are required to be performant in MySQL c…

lib/private/DB/MigrationService.php

@@ -634,6 +634,8 @@ class MigrationService {
 				if ($isUsingDefaultName && \strlen($table->getName()) - $prefixLength >= 23) {
 					throw new \InvalidArgumentException('Primary index name on "' . $table->getName() . '" is too long.');
 				}
+			} elseif (!$primaryKey instanceof Index) {
+				throw new \InvalidArgumentException('Table "' . $table->getName() . '" has no primary key and therefor will not behave sane in clustered setups.');
 			}
 		}
 

tests/lib/DB/MigrationsTest.php

@@ -246,6 +246,8 @@ class MigrationsTest extends \Test\TestCase {
 			->method('getName')
 			->willReturn(\str_repeat('a', 30));
 
+		$primaryKey = $this->createMock(Index::class);
+
 		$table->expects($this->once())
 			->method('getColumns')
 			->willReturn([$column]);
@@ -257,7 +259,7 @@ class MigrationsTest extends \Test\TestCase {
 			->willReturn([$foreignKey]);
 		$table->expects($this->once())
 			->method('getPrimaryKey')
-			->willReturn(null);
+			->willReturn($primaryKey);
 
 		$schema = $this->createMock(Schema::class);
 		$schema->expects($this->once())
@@ -607,6 +609,46 @@ class MigrationsTest extends \Test\TestCase {
 	}
 
 
+	public function testEnsureOracleConstraintsNoPrimaryKey() {
+		$this->expectException(\InvalidArgumentException::class);
+
+		$table = $this->createMock(Table::class);
+		$table->expects($this->atLeastOnce())
+			->method('getName')
+			->willReturn(\str_repeat('a', 30));
+		$table->expects($this->once())
+			->method('getColumns')
+			->willReturn([]);
+		$table->expects($this->once())
+			->method('getIndexes')
+			->willReturn([]);
+		$table->expects($this->once())
+			->method('getForeignKeys')
+			->willReturn([]);
+		$table->expects($this->once())
+			->method('getPrimaryKey')
+			->willReturn(null);
+
+		$schema = $this->createMock(Schema::class);
+		$schema->expects($this->once())
+			->method('getTables')
+			->willReturn([$table]);
+		$schema->expects($this->once())
+			->method('getSequences')
+			->willReturn([]);
+
+		$sourceSchema = $this->createMock(Schema::class);
+		$sourceSchema->expects($this->any())
+			->method('getTable')
+			->willThrowException(new SchemaException());
+		$sourceSchema->expects($this->any())
+			->method('hasSequence')
+			->willReturn(false);
+
+		self::invokePrivate($this->migrationService, 'ensureOracleConstraints', [$sourceSchema, $schema, 3]);
+	}
+
+
 	public function testEnsureOracleConstraintsTooLongSequenceName() {
 		$this->expectException(\InvalidArgumentException::class);