From e20d5fa7fc567b0762226b79572f9aca6609e59f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6?= Date: Thu, 7 Apr 2022 11:18:14 +0200 Subject: [PATCH 1/4] Ensure schema change before checking OracleConstraints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ --- lib/private/DB/MigrationService.php | 35 +++++++++++++++++++---------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index 046e3a4924bb0..b228308cbfe99 100644 --- a/lib/private/DB/MigrationService.php +++ b/lib/private/DB/MigrationService.php @@ -583,20 +583,31 @@ 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 ((!$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 the table doesn't exists OR if the column doesn't exists 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 ($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) { + var_dump($sourceColumn === null, $sourceColumn->getLength(), $thing->getLength(), $sourceColumn->getName()); throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is type String, but exceeding the 4.000 length limit.'); } } From 1e5a879d2e64f7c9afe51ed40a9c8eac6d581262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6?= Date: Thu, 7 Apr 2022 11:34:13 +0200 Subject: [PATCH 2/4] Fix createNamedParameter in LDAP migrations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ --- apps/user_ldap/lib/Migration/Version1120Date20210917155206.php | 2 +- apps/user_ldap/lib/Migration/Version1141Date20220323143801.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php index bca390441d70d..b7a9b81d6a001 100644 --- 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 index be496d9215e35..10043371aae66 100644 --- 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(); From 0257315c3474a7e4e13af952c6191cedb6abcb99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6?= Date: Thu, 7 Apr 2022 11:45:54 +0200 Subject: [PATCH 3/4] Improve error logging on migration failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ --- lib/private/DB/MigrationService.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index b228308cbfe99..2b129edca26e1 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 @@ 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); } } } @@ -607,7 +606,6 @@ public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSche // 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) { - var_dump($sourceColumn === null, $sourceColumn->getLength(), $thing->getLength(), $sourceColumn->getName()); throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is type String, but exceeding the 4.000 length limit.'); } } From 82c33be744b1e9d85c780bce277c783c6d38e7bf Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 7 Apr 2022 12:42:52 +0200 Subject: [PATCH 4/4] Fix typos and empty tabs Signed-off-by: Joas Schilling --- lib/private/DB/MigrationService.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/private/DB/MigrationService.php b/lib/private/DB/MigrationService.php index 2b129edca26e1..fb7b6b7472fba 100644 --- a/lib/private/DB/MigrationService.php +++ b/lib/private/DB/MigrationService.php @@ -582,17 +582,17 @@ public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSche } foreach ($table->getColumns() as $thing) { - // If the table doesn't exists OR if the column doesn't exists in the table + // 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 ($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".'); } @@ -601,7 +601,7 @@ public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSche } else { $sourceColumn = $sourceTable->getColumn($thing->getName()); } - + // 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)