Merge branch '4.3.x' into 5.0.x

* 4.3.x: Create stubs instead of mocks (#6564) [Bug] Query Cache mangled if saved by-reference (#6552) test: remove ->expects(self::any()) Fix typo in PostgreSql documentation reference fix Acknowledge the existence of 3.10 (#6553) Simplify tracking implicitly created indexes Remove handling unuique constraint column names as associative array (#6549) test: cover nested transactions
doctrine · Oct 21, 2024 · 0c0c25c · 0c0c25c
2 parents c6d139a + ce220bc
commit 0c0c25c
Show file tree

Hide file tree

Showing 14 changed files with 161 additions and 78 deletions.
diff --git a/.doctrine-project.json b/.doctrine-project.json
@@ -35,6 +35,12 @@
             "slug": "4.0",
             "maintained": false
         },
+        {
+            "name": "3.10",
+            "branchName": "3.10.x",
+            "slug": "3.10",
+            "upcoming": true
+        },
         {
             "name": "3.9",
             "branchName": "3.9.x",

diff --git a/README.md b/README.md
@@ -1,11 +1,11 @@
 # Doctrine DBAL
 
-|                   [5.0-dev][5.0]                    |                   [4.3-dev][4.3]                    |                     [4.2][4.2]                      |                     [3.9][3.9]                      |
-|:---------------------------------------------------:|:---------------------------------------------------:|:---------------------------------------------------:|:---------------------------------------------------:|
-|      [![GitHub Actions][GA 5.0 image]][GA 5.0]      |      [![GitHub Actions][GA 4.3 image]][GA 4.3]      |      [![GitHub Actions][GA 4.2 image]][GA 4.2]      |      [![GitHub Actions][GA 3.9 image]][GA 3.9]      |
-|   [![AppVeyor][AppVeyor 5.0 image]][AppVeyor 5.0]   |   [![AppVeyor][AppVeyor 4.3 image]][AppVeyor 4.3]   |   [![AppVeyor][AppVeyor 4.2 image]][AppVeyor 4.2]   |   [![AppVeyor][AppVeyor 3.9 image]][AppVeyor 3.9]   |
-| [![Code Coverage][Coverage 5.0 image]][CodeCov 5.0] | [![Code Coverage][Coverage 4.3 image]][CodeCov 4.3] | [![Code Coverage][Coverage 4.2 image]][CodeCov 4.2] | [![Code Coverage][Coverage 3.9 image]][CodeCov 3.9] |
-|                         N/A                         |                         N/A                         |     [![Type Coverage][TypeCov image]][TypeCov]      |                         N/A                         |
+|                   [5.0-dev][5.0]                    |                   [4.3-dev][4.3]                    |                     [4.2][4.2]                      |                     [3.10][3.10]                      |                     [3.9][3.9]                      |
+|:---------------------------------------------------:|:---------------------------------------------------:|:---------------------------------------------------:|:-----------------------------------------------------:|:---------------------------------------------------:|
+|      [![GitHub Actions][GA 5.0 image]][GA 5.0]      |      [![GitHub Actions][GA 4.3 image]][GA 4.3]      |      [![GitHub Actions][GA 4.2 image]][GA 4.2]      |      [![GitHub Actions][GA 3.10 image]][GA 3.10]      |      [![GitHub Actions][GA 3.9 image]][GA 3.9]      |
+|   [![AppVeyor][AppVeyor 5.0 image]][AppVeyor 5.0]   |   [![AppVeyor][AppVeyor 4.3 image]][AppVeyor 4.3]   |   [![AppVeyor][AppVeyor 4.2 image]][AppVeyor 4.2]   |   [![AppVeyor][AppVeyor 3.10 image]][AppVeyor 3.10]   |   [![AppVeyor][AppVeyor 3.9 image]][AppVeyor 3.9]   |
+| [![Code Coverage][Coverage 5.0 image]][CodeCov 5.0] | [![Code Coverage][Coverage 4.3 image]][CodeCov 4.3] | [![Code Coverage][Coverage 4.2 image]][CodeCov 4.2] | [![Code Coverage][Coverage 3.10 image]][CodeCov 3.10] | [![Code Coverage][Coverage 3.9 image]][CodeCov 3.9] |
+|                         N/A                         |                         N/A                         |     [![Type Coverage][TypeCov image]][TypeCov]      |                         N/A                           |                         N/A                         |
 
 Powerful ***D***ata***B***ase ***A***bstraction ***L***ayer with many features for database schema introspection and schema management.
 
@@ -21,30 +21,38 @@ Powerful ***D***ata***B***ase ***A***bstraction ***L***ayer with many features f
   [AppVeyor 5.0]: https://ci.appveyor.com/project/doctrine/dbal/branch/5.0.x
   [AppVeyor 5.0 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/5.0.x?svg=true
   [GA 5.0]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A5.0.x
-  [GA 5.0 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=5.0.x
+  [GA 5.0 image]: https://github.com/doctrine/dbal/actions/workflows/continuous-integration.yml/badge.svg?branch=5.0.x
 
   [Coverage 4.3 image]: https://codecov.io/gh/doctrine/dbal/branch/4.3.x/graph/badge.svg
   [4.3]: https://github.com/doctrine/dbal/tree/4.3.x
   [CodeCov 4.3]: https://codecov.io/gh/doctrine/dbal/branch/4.3.x
   [AppVeyor 4.3]: https://ci.appveyor.com/project/doctrine/dbal/branch/4.3.x
   [AppVeyor 4.3 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/4.3.x?svg=true
   [GA 4.3]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A4.3.x
-  [GA 4.3 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=4.3.x
+  [GA 4.3 image]: https://github.com/doctrine/dbal/actions/workflows/continuous-integration.yml/badge.svg?branch=4.3.x
 
   [Coverage 4.2 image]: https://codecov.io/gh/doctrine/dbal/branch/4.2.x/graph/badge.svg
   [4.2]: https://github.com/doctrine/dbal/tree/4.2.x
   [CodeCov 4.2]: https://codecov.io/gh/doctrine/dbal/branch/4.2.x
   [AppVeyor 4.2]: https://ci.appveyor.com/project/doctrine/dbal/branch/4.2.x
   [AppVeyor 4.2 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/4.2.x?svg=true
   [GA 4.2]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A4.2.x
-  [GA 4.2 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=4.2.x
+  [GA 4.2 image]: https://github.com/doctrine/dbal/actions/workflows/continuous-integration.yml/badge.svg?branch=4.2.x
   [TypeCov]: https://shepherd.dev/github/doctrine/dbal
   [TypeCov image]: https://shepherd.dev/github/doctrine/dbal/coverage.svg
 
+  [Coverage 3.10 image]: https://codecov.io/gh/doctrine/dbal/branch/3.10.x/graph/badge.svg
+  [3.10]: https://github.com/doctrine/dbal/tree/3.10.x
+  [CodeCov 3.10]: https://codecov.io/gh/doctrine/dbal/branch/3.10.x
+  [AppVeyor 3.10]: https://ci.appveyor.com/project/doctrine/dbal/branch/3.10.x
+  [AppVeyor 3.10 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/3.10.x?svg=true
+  [GA 3.10]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A3.10.x
+  [GA 3.10 image]: https://github.com/doctrine/dbal/actions/workflows/continuous-integration.yml/badge.svg?branch=3.10.x
+
   [Coverage 3.9 image]: https://codecov.io/gh/doctrine/dbal/branch/3.9.x/graph/badge.svg
   [3.9]: https://github.com/doctrine/dbal/tree/3.9.x
   [CodeCov 3.9]: https://codecov.io/gh/doctrine/dbal/branch/3.9.x
   [AppVeyor 3.9]: https://ci.appveyor.com/project/doctrine/dbal/branch/3.9.x
   [AppVeyor 3.9 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/3.9.x?svg=true
   [GA 3.9]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A3.9.x
-  [GA 3.9 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=3.9.x
+  [GA 3.9 image]: https://github.com/doctrine/dbal/actions/workflows/continuous-integration.yml/badge.svg?branch=3.9.x
diff --git a/docs/en/reference/schema-representation.rst b/docs/en/reference/schema-representation.rst
@@ -93,7 +93,7 @@ and absolutely not portable.
 -  **engine** (string): The DB engine used for the table. Currently only supported on MySQL.
 
 -  **unlogged** (boolean): Set a PostgreSQL table type as
-  `unlogged <https://www.postgresql.org/docs/current/sql-createtable.htmll>`_
+  `unlogged <https://www.postgresql.org/docs/current/sql-createtable.html>`_
 
 Column
 ~~~~~~

diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -102,9 +102,7 @@
 
     <!-- See https://github.com/squizlabs/PHP_CodeSniffer/issues/2837 -->
     <rule ref="Squiz.NamingConventions.ValidVariableName.NotCamelCaps">
-        <exclude-pattern>src/Connection.php</exclude-pattern>
-        <exclude-pattern>src/Schema/Comparator.php</exclude-pattern>
-        <exclude-pattern>src/SQLParserUtils.php</exclude-pattern>
+        <exclude-pattern>src/Schema/Table.php</exclude-pattern>
     </rule>
 
     <!-- some statement classes close cursor using an empty while-loop -->

diff --git a/src/Connection.php b/src/Connection.php
@@ -811,7 +811,7 @@ public function executeCacheQuery(string $sql, array $params, array $types, Quer
             }
 
             if (isset($value[$realKey]) && $value[$realKey] instanceof ArrayResult) {
-                return new Result($value[$realKey], $this);
+                return new Result(clone $value[$realKey], $this);
             }
         } else {
             $value = [];
@@ -837,7 +837,7 @@ public function executeCacheQuery(string $sql, array $params, array $types, Quer
 
         $resultCache->save($item);
 
-        return new Result($value[$realKey], $this);
+        return new Result(clone $value[$realKey], $this);
     }
 
     /**

diff --git a/src/Schema/Exception/PrimaryKeyAlreadyExists.php b/src/Schema/Exception/PrimaryKeyAlreadyExists.php
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Schema\Exception;
+
+use Doctrine\DBAL\Schema\SchemaException;
+use LogicException;
+
+use function sprintf;
+
+/** @psalm-immutable */
+final class PrimaryKeyAlreadyExists extends LogicException implements SchemaException
+{
+    public static function new(string $tableName): self
+    {
+        return new self(
+            sprintf('Primary key was already defined on table "%s".', $tableName),
+        );
+    }
+}
diff --git a/src/Schema/Table.php b/src/Schema/Table.php
@@ -11,14 +11,13 @@
 use Doctrine\DBAL\Schema\Exception\IndexDoesNotExist;
 use Doctrine\DBAL\Schema\Exception\IndexNameInvalid;
 use Doctrine\DBAL\Schema\Exception\InvalidTableName;
+use Doctrine\DBAL\Schema\Exception\PrimaryKeyAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\UniqueConstraintDoesNotExist;
 use Doctrine\DBAL\Types\Type;
 use LogicException;
 
 use function array_merge;
 use function array_values;
-use function in_array;
-use function is_string;
 use function preg_match;
 use function sprintf;
 use function strtolower;
@@ -31,15 +30,20 @@ class Table extends AbstractAsset
     /** @var Column[] */
     protected array $_columns = [];
 
-    /** @var Index[] */
-    private array $implicitIndexes = [];
-
     /** @var array<string, string> keys are new names, values are old names */
     protected array $renamedColumns = [];
 
     /** @var Index[] */
     protected array $_indexes = [];
 
+    /**
+     * The keys of this array are the names of the indexes that were implicitly created as backing for foreign key
+     * constraints. The values are not used but must be non-null for {@link isset()} to work correctly.
+     *
+     * @var array<string,true>
+     */
+    private array $implicitIndexNames = [];
+
     protected ?string $_primaryKeyName = null;
 
     /** @var UniqueConstraint[] */
@@ -607,36 +611,41 @@ protected function _addColumn(Column $column): void
     /**
      * Adds an index to the table.
      */
-    protected function _addIndex(Index $indexCandidate): self
+    protected function _addIndex(Index $index): self
     {
-        $indexName               = $indexCandidate->getName();
-        $indexName               = $this->normalizeIdentifier($indexName);
-        $replacedImplicitIndexes = [];
+        $indexName = $this->normalizeIdentifier($index->getName());
 
-        foreach ($this->implicitIndexes as $name => $implicitIndex) {
-            if (! $implicitIndex->isFulfilledBy($indexCandidate) || ! isset($this->_indexes[$name])) {
+        $replacedImplicitIndexNames = [];
+
+        foreach ($this->implicitIndexNames as $implicitIndexName => $_) {
+            if (! isset($this->_indexes[$implicitIndexName])) {
                 continue;
             }
 
-            $replacedImplicitIndexes[] = $name;
+            if (! $this->_indexes[$implicitIndexName]->isFulfilledBy($index)) {
+                continue;
+            }
+
+            $replacedImplicitIndexNames[$implicitIndexName] = true;
         }
 
-        if (
-            (isset($this->_indexes[$indexName]) && ! in_array($indexName, $replacedImplicitIndexes, true)) ||
-            ($this->_primaryKeyName !== null && $indexCandidate->isPrimary())
-        ) {
+        if (isset($this->_indexes[$indexName]) && ! isset($replacedImplicitIndexNames[$indexName])) {
             throw IndexAlreadyExists::new($indexName, $this->_name);
         }
 
-        foreach ($replacedImplicitIndexes as $name) {
-            unset($this->_indexes[$name], $this->implicitIndexes[$name]);
+        if ($this->_primaryKeyName !== null && $index->isPrimary()) {
+            throw PrimaryKeyAlreadyExists::new($this->_name);
+        }
+
+        foreach ($replacedImplicitIndexNames as $name => $_) {
+            unset($this->_indexes[$name], $this->implicitIndexNames[$name]);
         }
 
-        if ($indexCandidate->isPrimary()) {
+        if ($index->isPrimary()) {
             $this->_primaryKeyName = $indexName;
         }
 
-        $this->_indexes[$indexName] = $indexCandidate;
+        $this->_indexes[$indexName] = $index;
 
         return $this;
     }
@@ -672,7 +681,7 @@ protected function _addUniqueConstraint(UniqueConstraint $constraint): self
             }
         }
 
-        $this->implicitIndexes[$this->normalizeIdentifier($indexName)] = $indexCandidate;
+        $this->implicitIndexNames[$this->normalizeIdentifier($indexName)] = true;
 
         return $this;
     }
@@ -710,7 +719,7 @@ protected function _addForeignKeyConstraint(ForeignKeyConstraint $constraint): s
         }
 
         $this->_addIndex($indexCandidate);
-        $this->implicitIndexes[$this->normalizeIdentifier($indexName)] = $indexCandidate;
+        $this->implicitIndexNames[$this->normalizeIdentifier($indexName)] = true;
 
         return $this;
     }
@@ -757,13 +766,7 @@ private function _createUniqueConstraint(
             throw IndexNameInvalid::new($indexName);
         }
 
-        foreach ($columns as $index => $value) {
-            if (is_string($index)) {
-                $columnName = $index;
-            } else {
-                $columnName = $value;
-            }
-
+        foreach ($columns as $columnName) {
             if (! $this->hasColumn($columnName)) {
                 throw ColumnDoesNotExist::new($columnName, $this->_name);
             }

diff --git a/tests/Connection/CachedQueryTest.php b/tests/Connection/CachedQueryTest.php
@@ -8,49 +8,60 @@
 use Doctrine\DBAL\Cache\QueryCacheProfile;
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Driver;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
+use Psr\Cache\CacheItemPoolInterface;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
 
 class CachedQueryTest extends TestCase
 {
-    public function testCachedQuery(): void
+    #[DataProvider('providePsrCacheImplementations')]
+    public function testCachedQuery(callable $psrCacheProvider): void
     {
-        $cache = new ArrayAdapter();
+        $cache = $psrCacheProvider();
 
         $connection = $this->createConnection(1, ['foo'], [['bar']]);
         $qcp        = new QueryCacheProfile(0, __FUNCTION__, $cache);
 
-        self::assertSame([['foo' => 'bar']], $connection->executeCacheQuery('SELECT 1', [], [], $qcp)
+        $firstResult = $connection->executeCacheQuery('SELECT 1', [], [], $qcp);
+        self::assertSame([['foo' => 'bar']], $firstResult
+            ->fetchAllAssociative());
+        $firstResult->free();
+        $secondResult = $connection->executeCacheQuery('SELECT 1', [], [], $qcp);
+        self::assertSame([['foo' => 'bar']], $secondResult
             ->fetchAllAssociative());
+        $secondResult->free();
         self::assertSame([['foo' => 'bar']], $connection->executeCacheQuery('SELECT 1', [], [], $qcp)
             ->fetchAllAssociative());
 
         self::assertCount(1, $cache->getItem(__FUNCTION__)->get());
     }
 
-    public function testCachedQueryWithChangedImplementationIsExecutedTwice(): void
+    #[DataProvider('providePsrCacheImplementations')]
+    public function testCachedQueryWithChangedImplementationIsExecutedTwice(callable $psrCacheProvider): void
     {
         $connection = $this->createConnection(2, ['baz'], [['qux']]);
 
         self::assertSame([['baz' => 'qux']], $connection->executeCacheQuery(
             'SELECT 1',
             [],
             [],
-            new QueryCacheProfile(0, __FUNCTION__, new ArrayAdapter()),
+            new QueryCacheProfile(0, __FUNCTION__, $psrCacheProvider()),
         )->fetchAllAssociative());
 
         self::assertSame([['baz' => 'qux']], $connection->executeCacheQuery(
             'SELECT 1',
             [],
             [],
-            new QueryCacheProfile(0, __FUNCTION__, new ArrayAdapter()),
+            new QueryCacheProfile(0, __FUNCTION__, $psrCacheProvider()),
         )->fetchAllAssociative());
     }
 
-    public function testOldCacheFormat(): void
+    #[DataProvider('providePsrCacheImplementations')]
+    public function testOldCacheFormat(callable $psrCacheProvider): void
     {
         $connection = $this->createConnection(1, ['foo'], [['bar']]);
-        $cache      = new ArrayAdapter();
+        $cache      = $psrCacheProvider();
         $qcp        = new QueryCacheProfile(0, __FUNCTION__, $cache);
 
         [$cacheKey, $realKey] = $qcp->generateCacheKeys('SELECT 1', [], [], []);
@@ -83,4 +94,13 @@ private function createConnection(int $expectedQueryCount, array $columnNames, a
 
         return new Connection([], $driver);
     }
+
+    /** @return array<non-empty-string, list<callable():CacheItemPoolInterface>> */
+    public static function providePsrCacheImplementations(): array
+    {
+        return [
+            'serialized' => [static fn () => new ArrayAdapter(0, true)],
+            'by-reference' => [static fn () => new ArrayAdapter(0, false)],
+        ];
+    }
 }