fix(migration): Make naming constraint fail softer on updates

[szaimen]

Fix#41253
Fix#43173

Follow-up to #39506

Admin check will be handled in #43432

[joshtrichards]

Just logging it for now does seem a practical solution. Seems friendlier and doesn't require a bunch of other engineering.

We could also do this in a setup check maybe? Not sure about resource usage though. The advantage there is we could provide a doc link + further encourage cleanup prior to upgrade runs.

[szaimen]

Just logging it for now does seem a practical solution. Seems friendlier and doesn't require a bunch of other engineering.

We could also do this in a setup check maybe? Not sure about resource usage though. The advantage there is we could provide a doc link + further encourage cleanup prior to upgrade runs.

Yes, this was my idea. :)

The admin check will be handled in #43432

[szaimen]

/backport to stable28

[nickvergessen]

https://github.com/nextcloud/server/blame/cc6de72b11eff0a5b9c7d099281c0ae91afa7320/lib/private/DB/MigrationService.php#L654

Naming constraints:

• • Index, sequence and primary key names must be unique within a Postgres Schema

It will break at least on postgres anyway. To help developers we should just not allow this and always break so they are aware.

Also logging this is of 0 help as no dev checks their logs on CI runs to look for such things.

Definitely a No from me.

[szaimen]

Idea from Joas:

We could weaken it for the update case, to only report in that case, but installing such an app/version/migration must be breaking

Basically similarly to the oracle check we could hand in the sourceSchema

server/lib/private/DB/MigrationService.php

Lines 517 to 521 in 6f39d82

	$this->ensureUniqueNamesConstraints($targetSchema);
	if ($this->checkOracle) {
	$sourceSchema = $this->connection->createSchema();
	$this->ensureOracleConstraints($sourceSchema, $targetSchema, strlen($this->connection->getPrefix()));
	}

And check if the collision existed there as well already and in that case log the error

nextcloud/server/lib/private/DB/MigrationService.php
Zeile 517 bis 521 (6f39d82)
517 $this->ensureUniqueNamesConstraints($targetSchema);
518 if ($this->checkOracle) {
519 $sourceSchema = $this->connection->createSchema();
520 $this->ensureOracleConstraints($sourceSchema, $targetSchema, strlen($this->connection->getPrefix()));
521 }

[nickvergessen]

Went for a more simplistic approach now, with hopefully no "production" impact (unless the release is containing a new broken migration), but it still fails hard on installation:

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.

Dismissing my review as I did the patch now

[szaimen]

Thanks a lot for your help on this @nickvergessen ! 😊

[szaimen]

/backport to stable28

[szaimen]

/backport to stable28