fix(migration): support the new OCP DB schema API by ChristophWurst · Pull Request #13459 · nextcloud/mail

ChristophWurst · 2026-08-11T07:22:47Z

OCP's ISchemaWrapper::getTable() now returns OCP\DB\Schema\ITable instead of Doctrine\DBAL\Schema\Table on newer server versions. Widen the addMailboxKey() parameter types to Table|ITable so the migration works on both, and suppress the not-yet-vendored ITable stub in psalm.

Assisted-by: Claude:claude-opus-4-8

Ref nextcloud/server#63013

🤖 AI (if applicable)

The content of this PR was partly or fully generated using AI

ChristophWurst · 2026-08-11T07:22:56Z

/backport to stable5.11

ChristophWurst · 2026-08-11T07:23:00Z

/backport to stable5.10

ChristophWurst · 2026-08-11T08:59:49Z

There are more changes necessary to make Psalm happy, but there are also legit gaps.

There is no replacement to enumerate all foreign keys:

mail/lib/Migration/Version1130Date20220412111833.php

Line 116 in 1968041

$fks = $attachmentsTable->getForeignKeys();

ModifyColumn changed in a breaking way. It's not possible to be compatible with 34 and 35 at the same time:

mail/lib/Migration/Version1130Date20220412111833.php

Lines 129 to 131 in 1968041

    
           $recipientsTable->modifyColumn('id', [ 
        
           	'type' => Type::getType(Types::BIGINT), 
        
           ]);

CarlSchwan · 2026-08-11T09:19:12Z

There are more changes necessary to make Psalm happy, but there are also legit gaps.
1. There is no replacement to enumerate all foreign keys: https://github.com/nextcloud/mail/blob/1968041ebcbd120bfce97be65d40fe636428252b/lib/Migration/Version1130Date20220412111833.php#L116

This should still work, since getForeignKeys is not implemented in Table, it's forwarding the method call to the DBAL Table via the __call in OC\DB\Schema\Table

2. ModifyColumn changed in a breaking way. It's not possible to be compatible with 34 and 35 at the same time: https://github.com/nextcloud/mail/blob/1968041ebcbd120bfce97be65d40fe636428252b/lib/Migration/Version1130Date20220412111833.php#L129-L131

Same, modifyColumn in OCP\DB\Schema, has some more strict type hints but at runtime this should still work as it is just forwarding the arguments.

CarlSchwan · 2026-08-12T08:58:13Z

Looking at the latest psalm issues, I probably went to far for requiring non empty lowercase string for index. I will change that

CarlSchwan · 2026-08-12T11:14:46Z

Last psalm issue seems to be a false positive from psalm

kesselb · 2026-08-12T11:26:23Z

 		 */
 		foreach ($mailboxesTable->getIndexes() as $index) {
-			if ($index->isUnique() && $index->spansColumns(['account_id', 'name'])) {
+			if ($index->isUnique() && $index->getColumns() === ['account_id', 'name']) {


Would be nice to also expose spansColumns because it ignores the order.

As hardening we should array diff expected and actual columns and test for an empty array.

But the other matters. An index on [a,b] is very different from [b,a].

For this specific case we only need to catch the condition where the ordering is [account_id, name], right?

spansColumns does not ignore the order.
The change can be reverted, spansColumns is part of IIndex now.

CarlSchwan · 2026-08-12T11:16:07Z

-			'precision' => 10,
-			'scale' => 5,
-		]);
+		])->setPrecision(10)->setScale(5);
 		$table->addColumn('precision_important', Types::DECIMAL, [
 			'notnull' => true,
-			'precision' => 10,
-			'scale' => 5,
-		]);
+		])->setPrecision(10)->setScale(5);
 		$table->addColumn('f1_score_important', Types::DECIMAL, [
 			'notnull' => true,
-			'precision' => 10,
-			'scale' => 5,
-		]);


this can be reverted, last version support precision and scale

CarlSchwan · 2026-08-12T11:16:31Z

-			$accountsSignatureColumn = $accountsTable->getColumn('signature');

 			$this->connection->executeStatement(
-				sprintf($alterQuery, $accountsTable->getName(), $accountsSignatureColumn->getName())
+				sprintf($alterQuery, $accountsTable->getName(), 'signature')
 			);

 			$aliasesTable = $schema->getTable('mail_aliases');
-			$aliasesSignatureColumn = $accountsTable->getColumn('signature');

 			$this->connection->executeStatement(
-				sprintf($alterQuery, $aliasesTable->getName(), $aliasesSignatureColumn->getName())
+				sprintf($alterQuery, $aliasesTable->getName(), 'signature')
 			);

 			unset(
 				$accountsTable,
-				$accountsSignatureColumn,
-				$aliasesTable,
-				$aliasesSignatureColumn
+				$aliasesTable


same here getName return a non-empty-string

The issue is that IColumn::getName does not exist. It works because of __call but Psalm won't like it.

nextcloud/server#63318

ISchemaWrapper::getTable() now returns OCP\DB\Schema\ITable instead of the Doctrine Table on server 35+. Widen addMailboxKey() to accept both so the migrations keep running on server 32-35, and reference the foreign table and column by name so the call type-checks against the stricter interface. Replace the Doctrine-only calls the reduced interface no longer exposes: a column-name literal instead of IColumn::getName(), and getPrimaryKey() instead of hasPrimaryKey(). Fix the mis-cased notNull option on classification_enabled so the boolean column stays nullable, which the new schema validation now enforces. ITable exists only in the newer OCP stubs, so it is suppressed in psalm for the older matrix versions. Version1130's foreign-key enumeration and column type change forward to Doctrine at runtime and were handled server-side, so that migration is left unchanged. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com>

ChristophWurst requested a review from CarlSchwan August 11, 2026 07:22

ChristophWurst self-assigned this Aug 11, 2026

ChristophWurst added the bug label Aug 11, 2026

ChristophWurst requested a review from kesselb as a code owner August 11, 2026 07:22

ChristophWurst added the 3. to review label Aug 11, 2026

ChristophWurst requested a review from GretaD as a code owner August 11, 2026 07:22

ChristophWurst added the regression label Aug 11, 2026

backportbot Bot added the backport-request label Aug 11, 2026

ChristophWurst removed the request for review from GretaD August 11, 2026 07:23

CarlSchwan approved these changes Aug 11, 2026

View reviewed changes

susnux approved these changes Aug 11, 2026

View reviewed changes

nextcloud-command added the AI assisted This PR contains AI-assisted commits label Aug 12, 2026

kesselb reviewed Aug 12, 2026

View reviewed changes

CarlSchwan reviewed Aug 13, 2026

View reviewed changes

ChristophWurst force-pushed the fix/migration-gettable-itable-type branch from d4e6d22 to 0deecae Compare August 17, 2026 13:28

ChristophWurst added 4. to release and removed 3. to review labels Aug 17, 2026

ChristophWurst enabled auto-merge August 17, 2026 13:29

ChristophWurst changed the title ~~fix(migration): accept ITable from getTable() in addMailboxKey~~ fix(migration): support the new OCP DB schema API Aug 17, 2026

Uh oh!

Conversation

ChristophWurst commented Aug 11, 2026

🤖 AI (if applicable)

Uh oh!

ChristophWurst commented Aug 11, 2026

Uh oh!

ChristophWurst commented Aug 11, 2026

Uh oh!

ChristophWurst commented Aug 11, 2026

Uh oh!

CarlSchwan commented Aug 11, 2026

Uh oh!

CarlSchwan commented Aug 12, 2026

Uh oh!

CarlSchwan commented Aug 12, 2026

Uh oh!

kesselb Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

ChristophWurst Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

kesselb Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CarlSchwan Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

CarlSchwan Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

ChristophWurst Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

CarlSchwan Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kesselb Aug 16, 2026 •

edited

Loading