From 18d672c38fd6df8dfb41a670b119c11f298461b7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 5 Jul 2023 09:08:10 +0200 Subject: [PATCH 01/10] chore(deps): Bump doctrine/dbal to supported 3.7.0 Signed-off-by: Joas Schilling --- 3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty b/3rdparty index a983a316487c6..e6c45c6c0d4f9 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit a983a316487c666d6602923f0c067e32299501ba +Subproject commit e6c45c6c0d4f92ffec621446fcc3b34584772b13 From c4ed7f9d21afefb670f95560c02d991f16b2366d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 1 Jun 2023 07:55:37 +0200 Subject: [PATCH 02/10] fix(dbal): Doctrine\DBAL\Connection::PARAM_* types are deprecated Signed-off-by: Joas Schilling --- lib/public/DB/QueryBuilder/IQueryBuilder.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index 63fdfb6597137..491ac6d8e2f37 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -27,6 +27,7 @@ */ namespace OCP\DB\QueryBuilder; +use Doctrine\DBAL\ArrayParameterType; use Doctrine\DBAL\Connection; use Doctrine\DBAL\ParameterType; use OCP\DB\Exception; @@ -72,11 +73,11 @@ interface IQueryBuilder { /** * @since 9.0.0 */ - public const PARAM_INT_ARRAY = Connection::PARAM_INT_ARRAY; + public const PARAM_INT_ARRAY = ArrayParameterType::INTEGER; /** * @since 9.0.0 */ - public const PARAM_STR_ARRAY = Connection::PARAM_STR_ARRAY; + public const PARAM_STR_ARRAY = ArrayParameterType::STRING; /** * @since 24.0.0 Indicates how many rows can be deleted at once with MySQL From 00acf1bae1b35dcbd13e858ccc1a706f2c6e1d8c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 5 Jul 2023 10:17:49 +0200 Subject: [PATCH 03/10] feat(CI): Add a stub for SensitiveParameter, so Psalm is not failing on Doctrine/DBAL Signed-off-by: Joas Schilling --- build/stubs/SensitiveParameter.phpstub | 5 +++++ psalm.xml | 1 + 2 files changed, 6 insertions(+) create mode 100644 build/stubs/SensitiveParameter.phpstub diff --git a/build/stubs/SensitiveParameter.phpstub b/build/stubs/SensitiveParameter.phpstub new file mode 100644 index 0000000000000..9571513892785 --- /dev/null +++ b/build/stubs/SensitiveParameter.phpstub @@ -0,0 +1,5 @@ + + From ad839dbb0a095911da519ef19c6a8b494d0f1697 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 5 Jul 2023 10:19:45 +0200 Subject: [PATCH 04/10] fix(sqlite): Remove no longer required autoincrement fix - I installed current master and exported the schema as SQL - Then I went to this branch, removed the content of the run() method (so made it no-op) - I installed again and exported the schema as SQL - The files are exactly the same, so whatever we tried to fix was fixed since 2015 in doctrine dbal Signed-off-by: Joas Schilling --- lib/composer/composer/autoload_classmap.php | 1 - lib/composer/composer/autoload_static.php | 1 - lib/private/DB/Connection.php | 4 +- lib/private/Repair.php | 4 - lib/private/Repair/SqliteAutoincrement.php | 100 ------------------ .../Repair/RepairSqliteAutoincrementTest.php | 90 ---------------- 6 files changed, 2 insertions(+), 198 deletions(-) delete mode 100644 lib/private/Repair/SqliteAutoincrement.php delete mode 100644 tests/lib/Repair/RepairSqliteAutoincrementTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 6e5bf85efcf93..15b0c76dc39c2 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1594,7 +1594,6 @@ 'OC\\Repair\\RepairDavShares' => $baseDir . '/lib/private/Repair/RepairDavShares.php', 'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php', 'OC\\Repair\\RepairMimeTypes' => $baseDir . '/lib/private/Repair/RepairMimeTypes.php', - 'OC\\Repair\\SqliteAutoincrement' => $baseDir . '/lib/private/Repair/SqliteAutoincrement.php', 'OC\\RichObjectStrings\\Validator' => $baseDir . '/lib/private/RichObjectStrings/Validator.php', 'OC\\Route\\CachingRouter' => $baseDir . '/lib/private/Route/CachingRouter.php', 'OC\\Route\\Route' => $baseDir . '/lib/private/Route/Route.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index ef88bcd2a9b19..888551c95e21d 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1627,7 +1627,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair\\RepairDavShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairDavShares.php', 'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php', 'OC\\Repair\\RepairMimeTypes' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairMimeTypes.php', - 'OC\\Repair\\SqliteAutoincrement' => __DIR__ . '/../../..' . '/lib/private/Repair/SqliteAutoincrement.php', 'OC\\RichObjectStrings\\Validator' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/Validator.php', 'OC\\Route\\CachingRouter' => __DIR__ . '/../../..' . '/lib/private/Route/CachingRouter.php', 'OC\\Route\\Route' => __DIR__ . '/../../..' . '/lib/private/Route/Route.php', diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index 2bd1d4c824ac8..d214eaec216c7 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -42,7 +42,7 @@ use Doctrine\DBAL\Exception; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Platforms\OraclePlatform; -use Doctrine\DBAL\Platforms\PostgreSQL94Platform; +use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\DBAL\Platforms\SqlitePlatform; use Doctrine\DBAL\Result; use Doctrine\DBAL\Schema\Schema; @@ -602,7 +602,7 @@ private function getMigrator() { return new OracleMigrator($this, $config, $dispatcher); } elseif ($platform instanceof MySQLPlatform) { return new MySQLMigrator($this, $config, $dispatcher); - } elseif ($platform instanceof PostgreSQL94Platform) { + } elseif ($platform instanceof PostgreSQLPlatform) { return new PostgreSqlMigrator($this, $config, $dispatcher); } else { return new Migrator($this, $config, $dispatcher); diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 05624a2423a22..5c68c1069938e 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -84,7 +84,6 @@ use OC\Repair\RepairDavShares; use OC\Repair\RepairInvalidShares; use OC\Repair\RepairMimeTypes; -use OC\Repair\SqliteAutoincrement; use OC\Template\JSCombiner; use Psr\Log\LoggerInterface; use Throwable; @@ -235,14 +234,11 @@ public static function getExpensiveRepairSteps() { * @return IRepairStep[] */ public static function getBeforeUpgradeRepairSteps() { - /** @var Connection $connection */ - $connection = \OC::$server->get(Connection::class); /** @var ConnectionAdapter $connectionAdapter */ $connectionAdapter = \OC::$server->get(ConnectionAdapter::class); $config = \OC::$server->getConfig(); $steps = [ new Collation(\OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class), $connectionAdapter, true), - new SqliteAutoincrement($connection), new SaveAccountsTableData($connectionAdapter, $config), new DropAccountTermsTable($connectionAdapter), ]; diff --git a/lib/private/Repair/SqliteAutoincrement.php b/lib/private/Repair/SqliteAutoincrement.php deleted file mode 100644 index 1199370f221cf..0000000000000 --- a/lib/private/Repair/SqliteAutoincrement.php +++ /dev/null @@ -1,100 +0,0 @@ - - * @author Thomas Müller - * @author Vincent Petry - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see - * - */ -namespace OC\Repair; - -use Doctrine\DBAL\Platforms\SqlitePlatform; -use Doctrine\DBAL\Schema\ColumnDiff; -use Doctrine\DBAL\Schema\SchemaDiff; -use Doctrine\DBAL\Schema\SchemaException; -use Doctrine\DBAL\Schema\TableDiff; -use OCP\Migration\IOutput; -use OCP\Migration\IRepairStep; - -/** - * Fixes Sqlite autoincrement by forcing the SQLite table schemas to be - * altered in order to retrigger SQL schema generation through OCSqlitePlatform. - */ -class SqliteAutoincrement implements IRepairStep { - /** - * @var \OC\DB\Connection - */ - protected $connection; - - /** - * @param \OC\DB\Connection $connection - */ - public function __construct($connection) { - $this->connection = $connection; - } - - public function getName() { - return 'Repair SQLite autoincrement'; - } - - /** - * Fix mime types - */ - public function run(IOutput $out) { - if (!$this->connection->getDatabasePlatform() instanceof SqlitePlatform) { - return; - } - - $sourceSchema = $this->connection->getSchemaManager()->createSchema(); - - $schemaDiff = new SchemaDiff(); - - foreach ($sourceSchema->getTables() as $tableSchema) { - $primaryKey = $tableSchema->getPrimaryKey(); - if (!$primaryKey) { - continue; - } - - $columnNames = $primaryKey->getColumns(); - - // add a column diff for every primary key column, - // but do not actually change anything, this will - // force the generation of SQL statements to alter - // those tables, which will then trigger the - // specific SQL code from OCSqlitePlatform - try { - $tableDiff = new TableDiff($tableSchema->getName()); - $tableDiff->fromTable = $tableSchema; - foreach ($columnNames as $columnName) { - $columnSchema = $tableSchema->getColumn($columnName); - $columnDiff = new ColumnDiff($columnSchema->getName(), $columnSchema); - $tableDiff->changedColumns[$columnSchema->getName()] = $columnDiff; - $schemaDiff->changedTables[] = $tableDiff; - } - } catch (SchemaException $e) { - // ignore - } - } - - $this->connection->beginTransaction(); - foreach ($schemaDiff->toSql($this->connection->getDatabasePlatform()) as $sql) { - $this->connection->executeQuery($sql); - } - $this->connection->commit(); - } -} diff --git a/tests/lib/Repair/RepairSqliteAutoincrementTest.php b/tests/lib/Repair/RepairSqliteAutoincrementTest.php deleted file mode 100644 index b4be47d015735..0000000000000 --- a/tests/lib/Repair/RepairSqliteAutoincrementTest.php +++ /dev/null @@ -1,90 +0,0 @@ - - * This file is licensed under the Affero General Public License version 3 or - * later. - * See the COPYING-README file. - */ - -namespace Test\Repair; - -use OC\DB\Connection; -use OCP\Migration\IOutput; - -/** - * Tests for fixing the SQLite id recycling - * - * @group DB - */ -class RepairSqliteAutoincrementTest extends \Test\TestCase { - /** - * @var \OC\Repair\SqliteAutoincrement - */ - private $repair; - - /** - * @var Connection - */ - private $connection; - - /** - * @var string - */ - private $tableName; - - /** - * @var \OCP\IConfig - */ - private $config; - - protected function setUp(): void { - parent::setUp(); - - $this->connection = \OC::$server->get(\OC\DB\Connection::class); - $this->config = \OC::$server->getConfig(); - if (!$this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\SqlitePlatform) { - $this->markTestSkipped("Test only relevant on Sqlite"); - } - - $dbPrefix = $this->config->getSystemValueString('dbtableprefix', 'oc_'); - $this->tableName = $this->getUniqueID($dbPrefix . 'autoinc_test'); - $this->connection->prepare('CREATE TABLE ' . $this->tableName . '("someid" INTEGER NOT NULL, "text" VARCHAR(16), PRIMARY KEY("someid"))')->execute(); - - $this->repair = new \OC\Repair\SqliteAutoincrement($this->connection); - } - - protected function tearDown(): void { - $this->connection->getSchemaManager()->dropTable($this->tableName); - parent::tearDown(); - } - - /** - * Tests whether autoincrement works - * - * @return boolean true if autoincrement works, false otherwise - */ - protected function checkAutoincrement() { - $this->connection->executeUpdate('INSERT INTO ' . $this->tableName . ' ("text") VALUES ("test")'); - $insertId = $this->connection->lastInsertId(); - $this->connection->executeUpdate('DELETE FROM ' . $this->tableName . ' WHERE "someid" = ?', [$insertId]); - - // insert again - $this->connection->executeUpdate('INSERT INTO ' . $this->tableName . ' ("text") VALUES ("test2")'); - $newInsertId = $this->connection->lastInsertId(); - - return ($insertId !== $newInsertId); - } - - public function testConvertIdColumn() { - $this->assertFalse($this->checkAutoincrement()); - - /** @var IOutput | \PHPUnit\Framework\MockObject\MockObject $outputMock */ - $outputMock = $this->getMockBuilder('\OCP\Migration\IOutput') - ->disableOriginalConstructor() - ->getMock(); - - $this->repair->run($outputMock); - - $this->assertTrue($this->checkAutoincrement()); - } -} From b202b139dd145a92626893a58bbc83bdfdda5880 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 5 Jul 2023 10:24:33 +0200 Subject: [PATCH 05/10] fix(postgres): Remove old Postgres 9.4 workaround Postgres 10 is the minimum in the meantime and doctrine/dbal fixed this in 2.6.0 already ref https://github.com/doctrine/dbal/pull/2614 Signed-off-by: Joas Schilling --- lib/composer/composer/autoload_classmap.php | 1 - lib/composer/composer/autoload_static.php | 1 - lib/private/DB/Connection.php | 3 -- lib/private/DB/PostgreSqlMigrator.php | 55 --------------------- tests/lib/DB/MigratorTest.php | 4 -- 5 files changed, 64 deletions(-) delete mode 100644 lib/private/DB/PostgreSqlMigrator.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 15b0c76dc39c2..367385b0359f4 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1201,7 +1201,6 @@ 'OC\\DB\\OracleConnection' => $baseDir . '/lib/private/DB/OracleConnection.php', 'OC\\DB\\OracleMigrator' => $baseDir . '/lib/private/DB/OracleMigrator.php', 'OC\\DB\\PgSqlTools' => $baseDir . '/lib/private/DB/PgSqlTools.php', - 'OC\\DB\\PostgreSqlMigrator' => $baseDir . '/lib/private/DB/PostgreSqlMigrator.php', 'OC\\DB\\PreparedStatement' => $baseDir . '/lib/private/DB/PreparedStatement.php', 'OC\\DB\\QueryBuilder\\CompositeExpression' => $baseDir . '/lib/private/DB/QueryBuilder/CompositeExpression.php', 'OC\\DB\\QueryBuilder\\ExpressionBuilder\\ExpressionBuilder' => $baseDir . '/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 888551c95e21d..bdf1cb9f5bd3b 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1234,7 +1234,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\DB\\OracleConnection' => __DIR__ . '/../../..' . '/lib/private/DB/OracleConnection.php', 'OC\\DB\\OracleMigrator' => __DIR__ . '/../../..' . '/lib/private/DB/OracleMigrator.php', 'OC\\DB\\PgSqlTools' => __DIR__ . '/../../..' . '/lib/private/DB/PgSqlTools.php', - 'OC\\DB\\PostgreSqlMigrator' => __DIR__ . '/../../..' . '/lib/private/DB/PostgreSqlMigrator.php', 'OC\\DB\\PreparedStatement' => __DIR__ . '/../../..' . '/lib/private/DB/PreparedStatement.php', 'OC\\DB\\QueryBuilder\\CompositeExpression' => __DIR__ . '/../../..' . '/lib/private/DB/QueryBuilder/CompositeExpression.php', 'OC\\DB\\QueryBuilder\\ExpressionBuilder\\ExpressionBuilder' => __DIR__ . '/../../..' . '/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php', diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index d214eaec216c7..7d246b49c62e2 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -42,7 +42,6 @@ use Doctrine\DBAL\Exception; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Platforms\OraclePlatform; -use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\DBAL\Platforms\SqlitePlatform; use Doctrine\DBAL\Result; use Doctrine\DBAL\Schema\Schema; @@ -602,8 +601,6 @@ private function getMigrator() { return new OracleMigrator($this, $config, $dispatcher); } elseif ($platform instanceof MySQLPlatform) { return new MySQLMigrator($this, $config, $dispatcher); - } elseif ($platform instanceof PostgreSQLPlatform) { - return new PostgreSqlMigrator($this, $config, $dispatcher); } else { return new Migrator($this, $config, $dispatcher); } diff --git a/lib/private/DB/PostgreSqlMigrator.php b/lib/private/DB/PostgreSqlMigrator.php deleted file mode 100644 index 92a0842e1a739..0000000000000 --- a/lib/private/DB/PostgreSqlMigrator.php +++ /dev/null @@ -1,55 +0,0 @@ - - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see - * - */ -namespace OC\DB; - -use Doctrine\DBAL\Schema\Schema; - -class PostgreSqlMigrator extends Migrator { - /** - * @param Schema $targetSchema - * @param \Doctrine\DBAL\Connection $connection - * @return \Doctrine\DBAL\Schema\SchemaDiff - */ - protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $connection) { - $schemaDiff = parent::getDiff($targetSchema, $connection); - - foreach ($schemaDiff->changedTables as $tableDiff) { - // fix default value in brackets - pg 9.4 is returning a negative default value in () - // see https://github.com/doctrine/dbal/issues/2427 - foreach ($tableDiff->changedColumns as $column) { - $column->changedProperties = array_filter($column->changedProperties, function ($changedProperties) use ($column) { - if ($changedProperties !== 'default') { - return true; - } - $fromDefault = $column->fromColumn->getDefault(); - $toDefault = $column->column->getDefault(); - $fromDefault = trim((string) $fromDefault, '()'); - - // by intention usage of != - return $fromDefault != $toDefault; - }); - } - } - - return $schemaDiff; - } -} diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php index 4d7d9cab19fa9..bd6f91da1aaa6 100644 --- a/tests/lib/DB/MigratorTest.php +++ b/tests/lib/DB/MigratorTest.php @@ -13,14 +13,12 @@ use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Platforms\OraclePlatform; -use Doctrine\DBAL\Platforms\PostgreSQL94Platform; use Doctrine\DBAL\Platforms\SqlitePlatform; use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\SchemaConfig; use OC\DB\Migrator; use OC\DB\MySQLMigrator; use OC\DB\OracleMigrator; -use OC\DB\PostgreSqlMigrator; use OC\DB\SQLiteMigrator; use OCP\DB\Types; use OCP\IConfig; @@ -69,8 +67,6 @@ private function getMigrator(): Migrator { return new OracleMigrator($this->connection, $this->config, $dispatcher); } elseif ($platform instanceof MySQLPlatform) { return new MySQLMigrator($this->connection, $this->config, $dispatcher); - } elseif ($platform instanceof PostgreSQL94Platform) { - return new PostgreSqlMigrator($this->connection, $this->config, $dispatcher); } return new Migrator($this->connection, $this->config, $dispatcher); } From 160298c5569901cc5d33ed74646f28fa41734fed Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 5 Jul 2023 11:25:01 +0200 Subject: [PATCH 06/10] fix(mysql): Remove custom MySQL workaround from 2015 Signed-off-by: Joas Schilling --- lib/composer/composer/autoload_classmap.php | 1 - lib/composer/composer/autoload_static.php | 1 - lib/private/DB/Connection.php | 2 - lib/private/DB/MySQLMigrator.php | 50 --------------------- tests/lib/DB/MigratorTest.php | 8 ---- 5 files changed, 62 deletions(-) delete mode 100644 lib/private/DB/MySQLMigrator.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 367385b0359f4..108a2f74b5031 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1194,7 +1194,6 @@ 'OC\\DB\\MissingColumnInformation' => $baseDir . '/lib/private/DB/MissingColumnInformation.php', 'OC\\DB\\MissingIndexInformation' => $baseDir . '/lib/private/DB/MissingIndexInformation.php', 'OC\\DB\\MissingPrimaryKeyInformation' => $baseDir . '/lib/private/DB/MissingPrimaryKeyInformation.php', - 'OC\\DB\\MySQLMigrator' => $baseDir . '/lib/private/DB/MySQLMigrator.php', 'OC\\DB\\MySqlTools' => $baseDir . '/lib/private/DB/MySqlTools.php', 'OC\\DB\\OCSqlitePlatform' => $baseDir . '/lib/private/DB/OCSqlitePlatform.php', 'OC\\DB\\ObjectParameter' => $baseDir . '/lib/private/DB/ObjectParameter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index bdf1cb9f5bd3b..ae1f13fc89bde 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1227,7 +1227,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\DB\\MissingColumnInformation' => __DIR__ . '/../../..' . '/lib/private/DB/MissingColumnInformation.php', 'OC\\DB\\MissingIndexInformation' => __DIR__ . '/../../..' . '/lib/private/DB/MissingIndexInformation.php', 'OC\\DB\\MissingPrimaryKeyInformation' => __DIR__ . '/../../..' . '/lib/private/DB/MissingPrimaryKeyInformation.php', - 'OC\\DB\\MySQLMigrator' => __DIR__ . '/../../..' . '/lib/private/DB/MySQLMigrator.php', 'OC\\DB\\MySqlTools' => __DIR__ . '/../../..' . '/lib/private/DB/MySqlTools.php', 'OC\\DB\\OCSqlitePlatform' => __DIR__ . '/../../..' . '/lib/private/DB/OCSqlitePlatform.php', 'OC\\DB\\ObjectParameter' => __DIR__ . '/../../..' . '/lib/private/DB/ObjectParameter.php', diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index 7d246b49c62e2..df35e0b5e0dc3 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -599,8 +599,6 @@ private function getMigrator() { return new SQLiteMigrator($this, $config, $dispatcher); } elseif ($platform instanceof OraclePlatform) { return new OracleMigrator($this, $config, $dispatcher); - } elseif ($platform instanceof MySQLPlatform) { - return new MySQLMigrator($this, $config, $dispatcher); } else { return new Migrator($this, $config, $dispatcher); } diff --git a/lib/private/DB/MySQLMigrator.php b/lib/private/DB/MySQLMigrator.php deleted file mode 100644 index 0f8cbb309f38e..0000000000000 --- a/lib/private/DB/MySQLMigrator.php +++ /dev/null @@ -1,50 +0,0 @@ - - * @author Thomas Müller - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see - * - */ -namespace OC\DB; - -use Doctrine\DBAL\Schema\Schema; - -class MySQLMigrator extends Migrator { - /** - * @param Schema $targetSchema - * @param \Doctrine\DBAL\Connection $connection - * @return \Doctrine\DBAL\Schema\SchemaDiff - */ - protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $connection) { - $platform = $connection->getDatabasePlatform(); - $platform->registerDoctrineTypeMapping('enum', 'string'); - $platform->registerDoctrineTypeMapping('bit', 'string'); - - $schemaDiff = parent::getDiff($targetSchema, $connection); - - // identifiers need to be quoted for mysql - foreach ($schemaDiff->changedTables as $tableDiff) { - $tableDiff->name = $this->connection->quoteIdentifier($tableDiff->name); - foreach ($tableDiff->changedColumns as $column) { - $column->oldColumnName = $this->connection->quoteIdentifier($column->oldColumnName); - } - } - - return $schemaDiff; - } -} diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php index bd6f91da1aaa6..3e6fb66c4c2bf 100644 --- a/tests/lib/DB/MigratorTest.php +++ b/tests/lib/DB/MigratorTest.php @@ -11,13 +11,11 @@ use Doctrine\DBAL\Exception; use Doctrine\DBAL\ParameterType; -use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Platforms\OraclePlatform; use Doctrine\DBAL\Platforms\SqlitePlatform; use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\SchemaConfig; use OC\DB\Migrator; -use OC\DB\MySQLMigrator; use OC\DB\OracleMigrator; use OC\DB\SQLiteMigrator; use OCP\DB\Types; @@ -65,8 +63,6 @@ private function getMigrator(): Migrator { return new SQLiteMigrator($this->connection, $this->config, $dispatcher); } elseif ($platform instanceof OraclePlatform) { return new OracleMigrator($this->connection, $this->config, $dispatcher); - } elseif ($platform instanceof MySQLPlatform) { - return new MySQLMigrator($this->connection, $this->config, $dispatcher); } return new Migrator($this->connection, $this->config, $dispatcher); } @@ -138,10 +134,6 @@ private function isSQLite() { return $this->connection->getDatabasePlatform() instanceof SqlitePlatform; } - private function isMySQL() { - return $this->connection->getDatabasePlatform() instanceof MySQLPlatform; - } - public function testUpgrade() { [$startSchema, $endSchema] = $this->getDuplicateKeySchemas(); $migrator = $this->getMigrator(); From 919207873ed0f89421d95560cc8134e936028258 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 5 Jul 2023 11:42:58 +0200 Subject: [PATCH 07/10] fix(dbal): Move migrator away from deprecated calls Signed-off-by: Joas Schilling --- lib/private/DB/Migrator.php | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/private/DB/Migrator.php b/lib/private/DB/Migrator.php index 46be512440a0a..ad964967f40b3 100644 --- a/lib/private/DB/Migrator.php +++ b/lib/private/DB/Migrator.php @@ -31,7 +31,6 @@ use Doctrine\DBAL\Exception; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Schema\AbstractAsset; -use Doctrine\DBAL\Schema\Comparator; use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\SchemaDiff; use Doctrine\DBAL\Types\StringType; @@ -75,7 +74,7 @@ public function generateChangeScript(Schema $targetSchema) { $schemaDiff = $this->getDiff($targetSchema, $this->connection); $script = ''; - $sqls = $schemaDiff->toSql($this->connection->getDatabasePlatform()); + $sqls = $this->connection->getDatabasePlatform()->getAlterSchemaSQL($schemaDiff); foreach ($sqls as $sql) { $script .= $this->convertStatementToScript($sql); } @@ -95,18 +94,18 @@ public function createSchema() { } return preg_match($filterExpression, $asset) === 1; }); - return $this->connection->getSchemaManager()->createSchema(); + return $this->connection->createSchemaManager()->introspectSchema(); } /** * @return SchemaDiff */ protected function getDiff(Schema $targetSchema, Connection $connection) { - // adjust varchar columns with a length higher then getVarcharMaxLength to clob + // adjust varchar columns with a length higher than getVarcharMaxLength to clob foreach ($targetSchema->getTables() as $table) { foreach ($table->getColumns() as $column) { if ($column->getType() instanceof StringType) { - if ($column->getLength() > $connection->getDatabasePlatform()->getVarcharMaxLength()) { + if ($column->getLength() > 4000) { $column->setType(Type::getType('text')); $column->setLength(null); } @@ -122,7 +121,7 @@ protected function getDiff(Schema $targetSchema, Connection $connection) { } return preg_match($filterExpression, $asset) === 1; }); - $sourceSchema = $connection->getSchemaManager()->createSchema(); + $sourceSchema = $connection->createSchemaManager()->introspectSchema(); // remove tables we don't know about foreach ($sourceSchema->getTables() as $table) { @@ -137,9 +136,8 @@ protected function getDiff(Schema $targetSchema, Connection $connection) { } } - /** @psalm-suppress InternalMethod */ - $comparator = new Comparator(); - return $comparator->compare($sourceSchema, $targetSchema); + $comparator = $connection->createSchemaManager()->createComparator(); + return $comparator->compareSchemas($sourceSchema, $targetSchema); } /** @@ -155,7 +153,7 @@ protected function applySchema(Schema $targetSchema, Connection $connection = nu if (!$connection->getDatabasePlatform() instanceof MySQLPlatform) { $connection->beginTransaction(); } - $sqls = $schemaDiff->toSql($connection->getDatabasePlatform()); + $sqls = $connection->getDatabasePlatform()->getAlterSchemaSQL($schemaDiff); $step = 0; foreach ($sqls as $sql) { $this->emit($sql, $step++, count($sqls)); @@ -178,7 +176,7 @@ protected function convertStatementToScript($statement) { } protected function getFilterExpression() { - return '/^' . preg_quote($this->config->getSystemValueString('dbtableprefix', 'oc_')) . '/'; + return '/^' . preg_quote($this->config->getSystemValueString('dbtableprefix', 'oc_'), '/') . '/'; } protected function emit(string $sql, int $step, int $max): void { From f8ee6c4769900736dcc0642b9f85199f7d1bd1a9 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 29 Sep 2023 14:47:07 +0200 Subject: [PATCH 08/10] fix(oracle): Move away from internal and deprecated SchemaDiff API Signed-off-by: Joas Schilling --- lib/private/DB/OracleMigrator.php | 239 +++++++++++------------------- 1 file changed, 90 insertions(+), 149 deletions(-) diff --git a/lib/private/DB/OracleMigrator.php b/lib/private/DB/OracleMigrator.php index 4828110074cf2..5abab1a34e24c 100644 --- a/lib/private/DB/OracleMigrator.php +++ b/lib/private/DB/OracleMigrator.php @@ -1,5 +1,8 @@ * @copyright Copyright (c) 2016, ownCloud, Inc. * * @author Christoph Wurst @@ -29,176 +32,114 @@ namespace OC\DB; use Doctrine\DBAL\Exception; -use Doctrine\DBAL\Schema\Column; -use Doctrine\DBAL\Schema\ColumnDiff; -use Doctrine\DBAL\Schema\ForeignKeyConstraint; -use Doctrine\DBAL\Schema\Index; use Doctrine\DBAL\Schema\Schema; -use Doctrine\DBAL\Schema\Table; class OracleMigrator extends Migrator { - /** - * Quote a column's name but changing the name requires recreating - * the column instance and copying over all properties. - * - * @param Column $column old column - * @return Column new column instance with new name - */ - protected function quoteColumn(Column $column) { - $newColumn = new Column( - $this->connection->quoteIdentifier($column->getName()), - $column->getType() - ); - $newColumn->setAutoincrement($column->getAutoincrement()); - $newColumn->setColumnDefinition($column->getColumnDefinition()); - $newColumn->setComment($column->getComment()); - $newColumn->setDefault($column->getDefault()); - $newColumn->setFixed($column->getFixed()); - $newColumn->setLength($column->getLength()); - $newColumn->setNotnull($column->getNotnull()); - $newColumn->setPrecision($column->getPrecision()); - $newColumn->setScale($column->getScale()); - $newColumn->setUnsigned($column->getUnsigned()); - $newColumn->setPlatformOptions($column->getPlatformOptions()); - $newColumn->setCustomSchemaOptions($column->getPlatformOptions()); - return $newColumn; - } - - /** - * Quote an index's name but changing the name requires recreating - * the index instance and copying over all properties. - * - * @param Index $index old index - * @return Index new index instance with new name - */ - protected function quoteIndex($index) { - return new Index( - //TODO migrate existing uppercase indexes, then $this->connection->quoteIdentifier($index->getName()), - $index->getName(), - array_map(function ($columnName) { - return $this->connection->quoteIdentifier($columnName); - }, $index->getColumns()), - $index->isUnique(), - $index->isPrimary(), - $index->getFlags(), - $index->getOptions() - ); - } - - /** - * Quote an ForeignKeyConstraint's name but changing the name requires recreating - * the ForeignKeyConstraint instance and copying over all properties. - * - * @param ForeignKeyConstraint $fkc old fkc - * @return ForeignKeyConstraint new fkc instance with new name - */ - protected function quoteForeignKeyConstraint($fkc) { - return new ForeignKeyConstraint( - array_map(function ($columnName) { - return $this->connection->quoteIdentifier($columnName); - }, $fkc->getLocalColumns()), - $this->connection->quoteIdentifier($fkc->getForeignTableName()), - array_map(function ($columnName) { - return $this->connection->quoteIdentifier($columnName); - }, $fkc->getForeignColumns()), - $fkc->getName(), - $fkc->getOptions() - ); - } - /** * @param Schema $targetSchema * @param \Doctrine\DBAL\Connection $connection * @return \Doctrine\DBAL\Schema\SchemaDiff * @throws Exception */ - protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $connection) { - $schemaDiff = parent::getDiff($targetSchema, $connection); - + protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $connection): \Doctrine\DBAL\Schema\SchemaDiff { // oracle forces us to quote the identifiers - $schemaDiff->newTables = array_map(function (Table $table) { - return new Table( + $quotedSchema = new Schema(); + foreach ($targetSchema->getTables() as $table) { + $quotedTable = $quotedSchema->createTable( $this->connection->quoteIdentifier($table->getName()), - array_map(function (Column $column) { - return $this->quoteColumn($column); - }, $table->getColumns()), - array_map(function (Index $index) { - return $this->quoteIndex($index); - }, $table->getIndexes()), - [], - array_map(function (ForeignKeyConstraint $fck) { - return $this->quoteForeignKeyConstraint($fck); - }, $table->getForeignKeys()), - $table->getOptions() ); - }, $schemaDiff->newTables); - $schemaDiff->removedTables = array_map(function (Table $table) { - return new Table( - $this->connection->quoteIdentifier($table->getName()), - $table->getColumns(), - $table->getIndexes(), - [], - $table->getForeignKeys(), - $table->getOptions() - ); - }, $schemaDiff->removedTables); - - foreach ($schemaDiff->changedTables as $tableDiff) { - $tableDiff->name = $this->connection->quoteIdentifier($tableDiff->name); - - $tableDiff->addedColumns = array_map(function (Column $column) { - return $this->quoteColumn($column); - }, $tableDiff->addedColumns); - - foreach ($tableDiff->changedColumns as $column) { - $column->oldColumnName = $this->connection->quoteIdentifier($column->oldColumnName); - // auto increment is not relevant for oracle and can anyhow not be applied on change - $column->changedProperties = array_diff($column->changedProperties, ['autoincrement', 'unsigned']); + foreach ($table->getColumns() as $column) { + $newColumn = $quotedTable->addColumn( + $this->connection->quoteIdentifier($column->getName()), + $column->getType()->getTypeRegistry()->lookupName($column->getType()), + ); + $newColumn->setAutoincrement($column->getAutoincrement()); + $newColumn->setColumnDefinition($column->getColumnDefinition()); + $newColumn->setComment($column->getComment()); + $newColumn->setDefault($column->getDefault()); + $newColumn->setFixed($column->getFixed()); + $newColumn->setLength($column->getLength()); + $newColumn->setNotnull($column->getNotnull()); + $newColumn->setPrecision($column->getPrecision()); + $newColumn->setScale($column->getScale()); + $newColumn->setUnsigned($column->getUnsigned()); + $newColumn->setPlatformOptions($column->getPlatformOptions()); } - // remove columns that no longer have changed (because autoincrement and unsigned are not supported) - $tableDiff->changedColumns = array_filter($tableDiff->changedColumns, function (ColumnDiff $column) { - return count($column->changedProperties) > 0; - }); - - $tableDiff->removedColumns = array_map(function (Column $column) { - return $this->quoteColumn($column); - }, $tableDiff->removedColumns); - - $tableDiff->renamedColumns = array_map(function (Column $column) { - return $this->quoteColumn($column); - }, $tableDiff->renamedColumns); - - $tableDiff->addedIndexes = array_map(function (Index $index) { - return $this->quoteIndex($index); - }, $tableDiff->addedIndexes); - $tableDiff->changedIndexes = array_map(function (Index $index) { - return $this->quoteIndex($index); - }, $tableDiff->changedIndexes); - - $tableDiff->removedIndexes = array_map(function (Index $index) { - return $this->quoteIndex($index); - }, $tableDiff->removedIndexes); + foreach ($table->getIndexes() as $index) { + if ($index->isPrimary()) { + $quotedTable->setPrimaryKey( + array_map(function ($columnName) { + return $this->connection->quoteIdentifier($columnName); + }, $index->getColumns()), + //TODO migrate existing uppercase indexes, then $this->connection->quoteIdentifier($index->getName()), + $index->getName(), + ); + } elseif ($index->isUnique()) { + $quotedTable->addUniqueIndex( + array_map(function ($columnName) { + return $this->connection->quoteIdentifier($columnName); + }, $index->getColumns()), + //TODO migrate existing uppercase indexes, then $this->connection->quoteIdentifier($index->getName()), + $index->getName(), + $index->getOptions(), + ); + } else { + $quotedTable->addIndex( + array_map(function ($columnName) { + return $this->connection->quoteIdentifier($columnName); + }, $index->getColumns()), + //TODO migrate existing uppercase indexes, then $this->connection->quoteIdentifier($index->getName()), + $index->getName(), + $index->getFlags(), + $index->getOptions(), + ); + } + } - $tableDiff->renamedIndexes = array_map(function (Index $index) { - return $this->quoteIndex($index); - }, $tableDiff->renamedIndexes); + foreach ($table->getUniqueConstraints() as $constraint) { + $quotedTable->addUniqueConstraint( + array_map(function ($columnName) { + return $this->connection->quoteIdentifier($columnName); + }, $constraint->getColumns()), + $this->connection->quoteIdentifier($constraint->getName()), + $constraint->getFlags(), + $constraint->getOptions(), + ); + } - $tableDiff->addedForeignKeys = array_map(function (ForeignKeyConstraint $fkc) { - return $this->quoteForeignKeyConstraint($fkc); - }, $tableDiff->addedForeignKeys); + foreach ($table->getForeignKeys() as $foreignKey) { + $quotedTable->addForeignKeyConstraint( + $this->connection->quoteIdentifier($foreignKey->getForeignTableName()), + array_map(function ($columnName) { + return $this->connection->quoteIdentifier($columnName); + }, $foreignKey->getLocalColumns()), + array_map(function ($columnName) { + return $this->connection->quoteIdentifier($columnName); + }, $foreignKey->getForeignColumns()), + $foreignKey->getOptions(), + $this->connection->quoteIdentifier($foreignKey->getName()), + ); + } - $tableDiff->changedForeignKeys = array_map(function (ForeignKeyConstraint $fkc) { - return $this->quoteForeignKeyConstraint($fkc); - }, $tableDiff->changedForeignKeys); + foreach ($table->getOptions() as $option => $value) { + $quotedTable->addOption( + $option, + $value, + ); + } + } - $tableDiff->removedForeignKeys = array_map(function (ForeignKeyConstraint $fkc) { - return $this->quoteForeignKeyConstraint($fkc); - }, $tableDiff->removedForeignKeys); + foreach ($targetSchema->getSequences() as $sequence) { + $quotedSchema->createSequence( + $sequence->getName(), + $sequence->getAllocationSize(), + $sequence->getInitialValue(), + ); } - return $schemaDiff; + return parent::getDiff($quotedSchema, $connection); } /** From ccb01b19a0196b1cfd02fd004deebe63915287ac Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 29 Sep 2023 17:39:42 +0200 Subject: [PATCH 09/10] fix(sqlite): Remove some old SQLite cheats - Doctrine correctly forces integer for autoincrement by now - Doctrine correctly maintains integer types by now Signed-off-by: Joas Schilling --- lib/private/DB/SQLiteMigrator.php | 11 ----------- tests/lib/DB/MigratorTest.php | 4 ---- 2 files changed, 15 deletions(-) diff --git a/lib/private/DB/SQLiteMigrator.php b/lib/private/DB/SQLiteMigrator.php index cbb39070a480a..e0102e105b2bf 100644 --- a/lib/private/DB/SQLiteMigrator.php +++ b/lib/private/DB/SQLiteMigrator.php @@ -24,8 +24,6 @@ namespace OC\DB; use Doctrine\DBAL\Schema\Schema; -use Doctrine\DBAL\Types\BigIntType; -use Doctrine\DBAL\Types\Type; class SQLiteMigrator extends Migrator { /** @@ -34,21 +32,12 @@ class SQLiteMigrator extends Migrator { * @return \Doctrine\DBAL\Schema\SchemaDiff */ protected function getDiff(Schema $targetSchema, \Doctrine\DBAL\Connection $connection) { - $platform = $connection->getDatabasePlatform(); - $platform->registerDoctrineTypeMapping('tinyint unsigned', 'integer'); - $platform->registerDoctrineTypeMapping('smallint unsigned', 'integer'); - $platform->registerDoctrineTypeMapping('varchar ', 'string'); - foreach ($targetSchema->getTables() as $table) { foreach ($table->getColumns() as $column) { // column comments are not supported on SQLite if ($column->getComment() !== null) { $column->setComment(null); } - // with sqlite autoincrement columns is of type integer - if ($column->getType() instanceof BigIntType && $column->getAutoincrement()) { - $column->setType(Type::getType('integer')); - } } } diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php index 3e6fb66c4c2bf..8447e0931e7ec 100644 --- a/tests/lib/DB/MigratorTest.php +++ b/tests/lib/DB/MigratorTest.php @@ -130,10 +130,6 @@ private function getSchemaConfig() { return $config; } - private function isSQLite() { - return $this->connection->getDatabasePlatform() instanceof SqlitePlatform; - } - public function testUpgrade() { [$startSchema, $endSchema] = $this->getDuplicateKeySchemas(); $migrator = $this->getMigrator(); From 570159e6101cfa09d4035da2e2811bc7ecd71dd5 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 4 Oct 2023 10:29:41 +0200 Subject: [PATCH 10/10] fix(DB): Update comment to state why we still use the max 4k limit Signed-off-by: Joas Schilling --- lib/private/DB/Migrator.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/private/DB/Migrator.php b/lib/private/DB/Migrator.php index ad964967f40b3..1d960e72dc52f 100644 --- a/lib/private/DB/Migrator.php +++ b/lib/private/DB/Migrator.php @@ -101,7 +101,9 @@ public function createSchema() { * @return SchemaDiff */ protected function getDiff(Schema $targetSchema, Connection $connection) { - // adjust varchar columns with a length higher than getVarcharMaxLength to clob + // Adjust STRING columns with a length higher than 4000 to TEXT (clob) + // for consistency between the supported databases and + // old vs. new installations. foreach ($targetSchema->getTables() as $table) { foreach ($table->getColumns() as $column) { if ($column->getType() instanceof StringType) {