Merge pull request #31872 from nextcloud/fix/oracle-contrainst

Ensure schema change before checking OracleConstraints

3 files changed, 24 insertions, 15 deletions

diff --git a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php
index bca390441d7..b7a9b81d6a0 100644
--- a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php
+++ b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php
@@ -129,7 +129,7 @@ class Version1120Date20210917155206 extends SimpleMigrationStep {
 		$qb = $this->dbc->getQueryBuilder();
 		$qb->select('owncloud_name', 'directory_uuid')
 			->from($table)
-			->where($qb->expr()->gt($qb->func()->octetLength('owncloud_name'), '64', IQueryBuilder::PARAM_INT));
+			->where($qb->expr()->gt($qb->func()->octetLength('owncloud_name'), $qb->createNamedParameter('64'), IQueryBuilder::PARAM_INT));
 		return $qb;
 	}
 
diff --git a/apps/user_ldap/lib/Migration/Version1141Date20220323143801.php b/apps/user_ldap/lib/Migration/Version1141Date20220323143801.php
index be496d9215e..10043371aae 100644
--- a/apps/user_ldap/lib/Migration/Version1141Date20220323143801.php
+++ b/apps/user_ldap/lib/Migration/Version1141Date20220323143801.php
@@ -51,7 +51,7 @@ class Version1141Date20220323143801 extends SimpleMigrationStep {
 			$qb = $this->dbc->getQueryBuilder();
 			$qb->select('ldap_dn')
 				->from($tableName)
-				->where($qb->expr()->gt($qb->func()->octetLength('ldap_dn'), '4000', IQueryBuilder::PARAM_INT));
+				->where($qb->expr()->gt($qb->func()->octetLength('ldap_dn'), $qb->createNamedParameter('4000'), IQueryBuilder::PARAM_INT));
 
 			$dnsTooLong = [];
 			$result = $qb->executeQuery();
diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php
index 046e3a4924b..fb7b6b7472f 100644
--- a/lib/private/DB/MigrationService.php
+++ b/lib/private/DB/MigrationService.php
@@ -27,7 +27,6 @@
  */
 namespace OC\DB;
 
-use Doctrine\DBAL\Exception\DriverException;
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Schema\Index;
@@ -424,10 +423,10 @@ class MigrationService {
 		foreach ($toBeExecuted as $version) {
 			try {
 				$this->executeStep($version, $schemaOnly);
-			} catch (DriverException $e) {
+			} catch (\Exception $e) {
 				// The exception itself does not contain the name of the migration,
 				// so we wrap it here, to make debugging easier.
-				throw new \Exception('Database error when running migration ' . $to . ' for app ' . $this->getApp(), 0, $e);
+				throw new \Exception('Database error when running migration ' . $version . ' for app ' . $this->getApp() . PHP_EOL. $e->getMessage(), 0, $e);
 			}
 		}
 	}
@@ -583,20 +582,30 @@ class MigrationService {
 			}
 
 			foreach ($table->getColumns() as $thing) {
-				if ((!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName())) && \strlen($thing->getName()) > 30) {
-					throw new \InvalidArgumentException('Column name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
-				}
+				// If the table doesn't exist OR if the column doesn't exist in the table
+				if (!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName())) {
+					if (\strlen($thing->getName()) > 30) {
+						throw new \InvalidArgumentException('Column name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
+					}
 
-				if ((!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName())) && $thing->getNotnull() && $thing->getDefault() === ''
-					&& $sourceTable instanceof Table && !$sourceTable->hasColumn($thing->getName())) {
-					throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is NotNull, but has empty string or null as default.');
-				}
+					if ($thing->getNotnull() && $thing->getDefault() === ''
+						&& $sourceTable instanceof Table && !$sourceTable->hasColumn($thing->getName())) {
+						throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is NotNull, but has empty string or null as default.');
+					}
+
+					if ($thing->getNotnull() && $thing->getType()->getName() === Types::BOOLEAN) {
+						throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is type Bool and also NotNull, so it can not store "false".');
+					}
 
-				if ((!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName())) && $thing->getNotnull() && $thing->getType()->getName() === Types::BOOLEAN) {
-					throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is type Bool and also NotNull, so it can not store "false".');
+					$sourceColumn = null;
+				} else {
+					$sourceColumn = $sourceTable->getColumn($thing->getName());
 				}
 
-				if ($thing->getLength() > 4000 && $thing->getType()->getName() === Types::STRING) {
+				// If the column was just created OR the length changed OR the type changed
+				// we will NOT detect invalid length if the column is not modified
+				if (($sourceColumn === null || $sourceColumn->getLength() !== $thing->getLength() || $sourceColumn->getType()->getName() !== Types::STRING)
+					&& $thing->getLength() > 4000 && $thing->getType()->getName() === Types::STRING) {
 					throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is type String, but exceeding the 4.000 length limit.');
 				}
 			}