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

Ensure schema change before checking OracleConstraints
nextcloud · Apr 7, 2022 · 8b53b6d · 8b53b6d
2 parents 0e29dc8 + 82c33be
commit 8b53b6d
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 15 deletions.
diff --git a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php
@@ -129,7 +129,7 @@ protected function getSelectQuery(string $table): IQueryBuilder {
 		$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
@@ -51,7 +51,7 @@ public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $
 			$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
@@ -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 @@ public function migrate($to = 'latest', $schemaOnly = false) {
 		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 @@ public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSche
 			}
 
 			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.');
 				}
 			}