fix(migration): support the new OCP DB schema API - #13459
Conversation
|
/backport to stable5.11 |
|
/backport to stable5.10 |
|
There are more changes necessary to make Psalm happy, but there are also legit gaps.
|
This should still work, since getForeignKeys is not implemented in Table, it's forwarding the method call to the DBAL Table via the
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. |
|
Looking at the latest psalm issues, I probably went to far for requiring non empty lowercase string for index. I will change that |
|
Last psalm issue seems to be a false positive from psalm |
| */ | ||
| foreach ($mailboxesTable->getIndexes() as $index) { | ||
| if ($index->isUnique() && $index->spansColumns(['account_id', 'name'])) { | ||
| if ($index->isUnique() && $index->getColumns() === ['account_id', 'name']) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
spansColumns does not ignore the order.
The change can be reverted, spansColumns is part of IIndex now.
| '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, | ||
| ]); |
There was a problem hiding this comment.
this can be reverted, last version support precision and scale
| $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 |
There was a problem hiding this comment.
same here getName return a non-empty-string
There was a problem hiding this comment.
The issue is that IColumn::getName does not exist. It works because of __call but Psalm won't like it.
d4e6d22 to
0deecae
Compare
ChristophWurst
changed the title
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. Constrain the sort direction in MessageMapper::findByIds() to a literal so it satisfies IQueryBuilder::orderBy()'s tightened, taint-checked parameter. Normalizing in one place keeps every caller working and avoids threading a direction type through the search stack. 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>
0deecae to
b5283cf
Compare
5 participants
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)