Upgrade DBAL to v3.4.x (#1069)

atk4 · Oct 25, 2022 · 1d64b69 · 1d64b69
1 parent 7c09563
commit 1d64b69
Show file tree

Hide file tree

Showing 14 changed files with 75 additions and 103 deletions.
diff --git a/.github/release-drafter.yml b/.github/release-drafter.yml
@@ -1,14 +1,9 @@
 categories:
-  - title: "Breaking Changes"
-    labels:
-      - "BC-break"
-  - title: "Major Features"
+  - title: "Major features"
     labels:
       - "MAJOR"
-  - title: "Documentation enhancements"
+  - title: "Breaking changes"
     labels:
-      - "Documentation :books:"
-template: |
-  ## What’s Changed
-
-  $CHANGES
+      - "BC-break"
+  - title: "Other changes"
+template: $CHANGES
diff --git a/.github/workflows/build-docs.yml → .github/workflows/build-changelog.yml b/.github/workflows/build-docs.yml → .github/workflows/build-changelog.yml
@@ -1,4 +1,4 @@
-name: Build Docs
+name: Build changelog
 
 on:
   push:

diff --git a/.github/workflows/build-release.yml b/.github/workflows/build-release.yml
@@ -1,4 +1,4 @@
-name: Build Release
+name: Build release
 
 on:
   push:

diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml
@@ -34,10 +34,10 @@ jobs:
       - name: Setup cache 1/2
         id: composer-cache
         run: |
-          echo "::set-output name=dir::$(composer config cache-files-dir)"
+          echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
 
       - name: Setup cache 2/2
-        uses: actions/cache@v2
+        uses: actions/cache@v3
         with:
           path: ${{ steps.composer-cache.outputs.dir }}
           key: ${{ runner.os }}-composer-smoke-${{ matrix.php }}-${{ matrix.type }}-${{ hashFiles('composer.json') }}
@@ -128,10 +128,10 @@ jobs:
       - name: Setup cache 1/2
         id: composer-cache
         run: |
-          echo "::set-output name=dir::$(composer config cache-files-dir)"
+          echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
 
       - name: Setup cache 2/2
-        uses: actions/cache@v2
+        uses: actions/cache@v3
         with:
           path: ${{ steps.composer-cache.outputs.dir }}
           key: ${{ runner.os }}-composer-${{ matrix.php }}-${{ matrix.type }}-${{ hashFiles('composer.json') }}
@@ -244,15 +244,16 @@ jobs:
           php -d opcache.enable_cli=1 vendor/bin/phpunit --exclude-group none $(if [ -n "$LOG_COVERAGE" ]; then echo --coverage-text; else echo --no-coverage; fi) -v
           if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-oracle-oci8.cov; fi
 
-      - name: Upload coverage logs 1/2 (only for latest Phpunit)
+      - name: Upload coverage logs 1/2 (only for coverage)
         if: env.LOG_COVERAGE
         run: |
           ls -l coverage | wc -l
           php -d memory_limit=2G vendor/bin/phpcov merge coverage/ --clover coverage/merged.xml
 
-      - name: Upload coverage logs 2/2 (only for latest Phpunit)
+      - name: Upload coverage logs 2/2 (only for coverage)
         if: env.LOG_COVERAGE
-        uses: codecov/codecov-action@v1
+        uses: codecov/codecov-action@v3
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
-          file: coverage/merged.xml
+          fail_ci_if_error: true
+          files: coverage/merged.xml
diff --git a/composer.json b/composer.json
@@ -36,13 +36,13 @@
     "require": {
         "php": ">=7.4 <8.3",
         "atk4/core": "dev-develop",
-        "doctrine/dbal": "~3.3.7",
+        "doctrine/dbal": "~3.4.5",
         "mvorisek/atk4-hintable": "~1.9.0"
     },
     "require-release": {
         "php": ">=7.4 <8.3",
         "atk4/core": "~4.0.0",
-        "doctrine/dbal": "~3.3.7",
+        "doctrine/dbal": "~3.4.5",
         "mvorisek/atk4-hintable": "~1.9.0"
     },
     "require-dev": {

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -19,7 +19,22 @@ parameters:
         -
             message: '~^Call to method (getVarcharTypeDeclarationSQL|getClobTypeDeclarationSQL|getCreateTableSQL|getCurrentDatabaseExpression|initializeDoctrineTypeMappings)\(\) of deprecated class Doctrine\\DBAL\\Platforms\\(PostgreSQLPlatform|SQLServerPlatform|AbstractPlatform):\nUse.+instead\.$~'
             path: '*'
-            count: 6
+            count: 5
+        # https://github.com/phpstan/phpstan-deprecation-rules/issues/75
+        -
+            message: '~^Call to deprecated method getVarcharTypeDeclarationSQL\(\) of class AnonymousClass\w+:\nUse \{@link getStringTypeDeclarationSQL\(\)\} instead\.$~'
+            path: '*'
+            count: 1
+
+        # AbstractPlatform::getIdentitySequenceName() method is deprecated in DBAL 3.4+,
+        # in DBAL 4.0 OraclePlatform::getIdentitySequenceName() is protected and
+        # PostgreSQLPlatform::getIdentitySequenceName() is removed:
+        # https://github.com/doctrine/dbal/blob/3.5.1/src/Platforms/PostgreSQLPlatform.php#L611
+        # https://github.com/doctrine/dbal/blob/4.0.0-beta1/src/Platforms/PostgreSQLPlatform.php#L297
+        -
+            message: '~^Call to deprecated method getIdentitySequenceName\(\) of class Doctrine\\DBAL\\Platforms\\(PostgreSQLPlatform|OraclePlatform)\.$~'
+            path: '*'
+            count: 3
 
         # TODO these rules are generated, this ignores should be fixed in the code
         # for src/Schema/TestCase.php

diff --git a/src/Persistence/Sql/DbalDriverMiddleware.php b/src/Persistence/Sql/DbalDriverMiddleware.php
@@ -21,6 +21,7 @@
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Query as DbalQuery;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
+use Doctrine\DBAL\Schema\OracleSchemaManager;
 use Doctrine\DBAL\Schema\SqliteSchemaManager;
 
 class DbalDriverMiddleware extends AbstractDriverMiddleware
@@ -67,6 +68,10 @@ public function getSchemaManager(DbalConnection $connection, AbstractPlatform $p
             return new class($connection, $platform) extends SqliteSchemaManager { // @phpstan-ignore-line
                 use Sqlite\SchemaManagerTrait;
             };
+        } elseif ($platform instanceof OraclePlatform) {
+            return new class($connection, $platform) extends OracleSchemaManager { // @phpstan-ignore-line
+                use Oracle\SchemaManagerTrait;
+            };
         }
 
         return parent::getSchemaManager($connection, $platform);

diff --git a/src/Persistence/Sql/Expression.php b/src/Persistence/Sql/Expression.php
@@ -607,14 +607,8 @@ protected function _execute(?object $connection, bool $fromExecuteStatement)
                         ->addMoreInfo('type', gettype($val));
                 }
 
-                if (is_string($val) && $platform instanceof OraclePlatform && strlen($val) > 2000) {
-                    $valRef = $val;
-                    $bind = $statement->bindParam($key, $valRef, ParameterType::STRING, strlen($val));
-                    unset($valRef);
-                } else {
-                    $bind = $statement->bindValue($key, $val, $type);
-                }
-                if ($bind === false) {
+                $bindResult = $statement->bindValue($key, $val, $type);
+                if (!$bindResult) {
                     throw (new Exception('Unable to bind parameter'))
                         ->addMoreInfo('param', $key)
                         ->addMoreInfo('value', $val)

diff --git a/src/Persistence/Sql/Mssql/PlatformTrait.php b/src/Persistence/Sql/Mssql/PlatformTrait.php
@@ -48,7 +48,7 @@ protected function getCreateColumnCommentSQL($tableName, $columnName, $comment)
         if (str_contains($tableName, '.')) {
             [$schemaName, $tableName] = explode('.', $tableName, 2);
         } else {
-            $schemaName = $this->getDefaultSchemaName();
+            $schemaName = 'dbo';
         }
 
         return $this->getAddExtendedPropertySQL(
@@ -68,7 +68,7 @@ protected function getAlterColumnCommentSQL($tableName, $columnName, $comment)
         if (str_contains($tableName, '.')) {
             [$schemaName, $tableName] = explode('.', $tableName, 2);
         } else {
-            $schemaName = $this->getDefaultSchemaName();
+            $schemaName = 'dbo';
         }
 
         return $this->getUpdateExtendedPropertySQL(
@@ -88,7 +88,7 @@ protected function getDropColumnCommentSQL($tableName, $columnName)
         if (str_contains($tableName, '.')) {
             [$schemaName, $tableName] = explode('.', $tableName, 2);
         } else {
-            $schemaName = $this->getDefaultSchemaName();
+            $schemaName = 'dbo';
         }
 
         return $this->getDropExtendedPropertySQL(
@@ -183,7 +183,7 @@ protected function getCommentOnTableSQL(string $tableName, ?string $comment): st
         if (str_contains($tableName, '.')) {
             [$schemaName, $tableName] = explode('.', $tableName, 2);
         } else {
-            $schemaName = $this->getDefaultSchemaName();
+            $schemaName = 'dbo';
         }
 
         return $this->getAddExtendedPropertySQL(

diff --git a/src/Persistence/Sql/Oracle/PlatformTrait.php b/src/Persistence/Sql/Oracle/PlatformTrait.php
@@ -104,17 +104,9 @@ public function getCreateAutoincrementSql($name, $table, $start = 1)
     public function getListDatabasesSQL(): string
     {
         // ignore Oracle maintained schemas, improve tests performance
-        return 'SELECT username FROM sys.all_users'
-            . ' WHERE oracle_maintained = \'N\'';
-    }
-
-    public function getListTablesSQL(): string
-    {
-        // ignore Oracle maintained tables, improve tests performance
+        // self::getListTablesSQL() is never used, thanks to https://github.com/doctrine/dbal/pull/5268 replaced by OracleSchemaManager::selectTableNames()
         // self::getListViewsSQL() does not need filtering, as there is no Oracle VIEW by default
-        return 'SELECT * FROM sys.user_tables'
-            . ' LEFT JOIN sys.user_objects ON user_objects.object_type = \'TABLE\''
-            . ' AND user_objects.object_name = user_tables.table_name'
+        return 'SELECT username FROM sys.all_users'
             . ' WHERE oracle_maintained = \'N\'';
     }
 }
diff --git a/src/Persistence/Sql/Oracle/SchemaManagerTrait.php b/src/Persistence/Sql/Oracle/SchemaManagerTrait.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Atk4\Data\Persistence\Sql\Oracle;
+
+use Doctrine\DBAL\Result as DbalResult;
+
+trait SchemaManagerTrait
+{
+    protected function selectTableNames(string $databaseName): DbalResult
+    {
+        // ignore Oracle maintained tables, improve tests performance
+        // self::selectTableColumns() impl. once needed or wait for https://github.com/doctrine/dbal/issues/5764
+        $sql = <<<'EOF'
+            SELECT all_tables.table_name
+            FROM sys.all_tables
+            INNER JOIN sys.user_objects ON user_objects.object_type = 'TABLE'
+                AND user_objects.object_name = all_tables.table_name
+            WHERE owner = :OWNER AND oracle_maintained = 'N'
+            ORDER BY all_tables.table_name
+            EOF;
+
+        return $this->_conn->executeQuery($sql, ['OWNER' => $databaseName]);
+    }
+}
diff --git a/src/Persistence/Sql/Sqlite/PlatformTrait.php b/src/Persistence/Sql/Sqlite/PlatformTrait.php
@@ -5,7 +5,6 @@
 namespace Atk4\Data\Persistence\Sql\Sqlite;
 
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
-use Doctrine\DBAL\Schema\Identifier;
 use Doctrine\DBAL\Schema\TableDiff;
 
 trait PlatformTrait
@@ -15,46 +14,6 @@ public function getIdentifierQuoteCharacter(): string
         return '`';
     }
 
-    public function supportsForeignKeyConstraints(): bool
-    {
-        // backport https://github.com/doctrine/dbal/pull/5427, remove once DBAL 3.3.x support is dropped
-        return true;
-    }
-
-    protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array
-    {
-        // https://github.com/doctrine/dbal/pull/5486
-        return [];
-    }
-
-    // fix quoted table name support
-    // TODO submit a PR with fixed SqlitePlatform to DBAL
-
-    private function unquoteTableIdentifier(string $tableName): string
-    {
-        return (new Identifier($tableName))->getName();
-    }
-
-    public function getListTableConstraintsSQL($table)
-    {
-        return parent::getListTableConstraintsSQL($this->unquoteTableIdentifier($table));
-    }
-
-    public function getListTableColumnsSQL($table, $database = null)
-    {
-        return parent::getListTableColumnsSQL($this->unquoteTableIdentifier($table), $database);
-    }
-
-    public function getListTableIndexesSQL($table, $database = null)
-    {
-        return parent::getListTableIndexesSQL($this->unquoteTableIdentifier($table), $database);
-    }
-
-    public function getListTableForeignKeysSQL($table, $database = null)
-    {
-        return parent::getListTableForeignKeysSQL($this->unquoteTableIdentifier($table), $database);
-    }
-
     public function getAlterTableSQL(TableDiff $diff): array
     {
         // fix https://github.com/doctrine/dbal/pull/5501

diff --git a/src/Persistence/Sql/Sqlite/SchemaManagerTrait.php b/src/Persistence/Sql/Sqlite/SchemaManagerTrait.php
@@ -30,22 +30,6 @@ public function alterTable(TableDiff $tableDiff): void
         }
     }
 
-    protected function _getPortableTableForeignKeysList($tableForeignKeys): array
-    {
-        $foreignKeys = parent::_getPortableTableForeignKeysList($tableForeignKeys);
-
-        // fix https://github.com/doctrine/dbal/pull/5486/files#r920239919
-        foreach ($foreignKeys as $foreignKey) {
-            \Closure::bind(function () use ($foreignKey) {
-                if (ctype_digit($foreignKey->_name)) {
-                    $foreignKey->_name = '';
-                }
-            }, null, Identifier::class)();
-        }
-
-        return $foreignKeys;
-    }
-
     // fix quoted table name support for private SqliteSchemaManager::getCreateTableSQL() method
     // https://github.com/doctrine/dbal/blob/3.3.7/src/Schema/SqliteSchemaManager.php#L539
     // TODO submit a PR with fixed SqliteSchemaManager to DBAL

diff --git a/src/Schema/TestSqlPersistence.php b/src/Schema/TestSqlPersistence.php
@@ -31,9 +31,10 @@ public function getConnection(): Persistence\Sql\Connection
                     )->executeStatement();
                 }
 
+                // @phpstan-ignore-next-line SQLLogger is deprecated
                 $this->getConnection()->getConnection()->getConfiguration()->setSQLLogger(
                     // @phpstan-ignore-next-line SQLLogger is deprecated
-                    null ?? new class() implements SQLLogger {
+                    new class() implements SQLLogger {
                         public function startQuery($sql, array $params = null, array $types = null): void
                         {
                             // fix https://github.com/doctrine/dbal/issues/5525