From 589e3eef41f4190aecbb8d8af66cba0a97fcb192 Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Mon, 14 Oct 2024 10:09:36 +0200 Subject: [PATCH 1/7] test: cover nested transactions _I managed to break this behaviour in other PR so this should be covered._ It tests basic walkthrough where we nest transactions, propagate the result through the transaction stack and also that it is committed and not rolled back. --- tests/Functional/TransactionTest.php | 40 ++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/tests/Functional/TransactionTest.php b/tests/Functional/TransactionTest.php index 1421bd10a8a..77e2fd09cc9 100644 --- a/tests/Functional/TransactionTest.php +++ b/tests/Functional/TransactionTest.php @@ -2,26 +2,24 @@ namespace Doctrine\DBAL\Tests\Functional; +use Doctrine\DBAL\Connection; use Doctrine\DBAL\Driver\Exception as DriverException; use Doctrine\DBAL\Platforms\AbstractMySQLPlatform; +use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Tests\FunctionalTestCase; +use Doctrine\DBAL\Types\Types; use PDOException; use function sleep; class TransactionTest extends FunctionalTestCase { - protected function setUp(): void + public function testCommitFalse(): void { - if ($this->connection->getDatabasePlatform() instanceof AbstractMySQLPlatform) { - return; + if (! $this->connection->getDatabasePlatform() instanceof AbstractMySQLPlatform) { + $this->markTestSkipped('Restricted to MySQL.'); } - $this->markTestSkipped('Restricted to MySQL.'); - } - - public function testCommitFalse(): void - { $this->connection->executeStatement('SET SESSION wait_timeout=1'); self::assertTrue($this->connection->beginTransaction()); @@ -40,4 +38,30 @@ public function testCommitFalse(): void $this->connection->close(); } } + + public function testNestedTransactionWalkthrough(): void + { + $table = new Table('storage'); + $table->addColumn('test_int', Types::INTEGER); + $table->setPrimaryKey(['test_int']); + + $this->dropAndCreateTable($table); + + $query = 'SELECT count(test_int) FROM storage'; + + self::assertSame('0', (string) $this->connection->fetchOne($query)); + + $result = $this->connection->transactional( + static fn (Connection $connection) => $connection->transactional( + static function (Connection $connection) use ($query) { + $connection->insert('storage', ['test_int' => 1]); + + return $connection->fetchOne($query); + }, + ), + ); + + self::assertSame('1', (string) $result); + self::assertSame('1', (string) $this->connection->fetchOne($query)); + } } From 179e73dee5d3b491e8fc129838d92e67f7981bda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Fri, 18 Oct 2024 13:23:41 +0200 Subject: [PATCH 2/7] Acknowledge the existence of 3.10 (#6553) --- .doctrine-project.json | 6 ++++++ README.md | 28 ++++++++++++++++++---------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/.doctrine-project.json b/.doctrine-project.json index 29924c3e16a..9c167863a7e 100644 --- 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 index 8d68192eb78..849d8f41106 100644 --- 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,7 +21,7 @@ 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 @@ -29,7 +29,7 @@ Powerful ***D***ata***B***ase ***A***bstraction ***L***ayer with many features f [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 @@ -37,14 +37,22 @@ Powerful ***D***ata***B***ase ***A***bstraction ***L***ayer with many features f [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 From aa6e1b1bffe4e40a57c303bfe198bef8dc43b3f2 Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Sat, 19 Oct 2024 12:58:48 +0200 Subject: [PATCH 3/7] fix --- tests/Functional/TransactionTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Functional/TransactionTest.php b/tests/Functional/TransactionTest.php index ec92545c459..d21c2e6d2c1 100644 --- a/tests/Functional/TransactionTest.php +++ b/tests/Functional/TransactionTest.php @@ -67,6 +67,10 @@ private function expectConnectionLoss(callable $scenario): void public function testNestedTransactionWalkthrough(): void { + if (! $this->connection->getDatabasePlatform()->supportsSavepoints()) { + self::markTestIncomplete('Broken when savepoints are not supported.'); + } + $table = new Table('storage'); $table->addColumn('test_int', Types::INTEGER); $table->setPrimaryKey(['test_int']); From 6154259abf3fcf2ea6ada2d52ab6930a2301ce9c Mon Sep 17 00:00:00 2001 From: Joshua Behrens Date: Sat, 19 Oct 2024 17:15:18 +0200 Subject: [PATCH 4/7] Fix typo in PostgreSql documentation reference --- docs/en/reference/schema-representation.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/reference/schema-representation.rst b/docs/en/reference/schema-representation.rst index f5aa7f4e0ac..72d4198915c 100644 --- 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 `_ + `unlogged `_ Column ~~~~~~ From eeb2e5cfed12101b1cbe49fdbbd75ce2e06e18e0 Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Sat, 19 Oct 2024 23:21:12 +0200 Subject: [PATCH 5/7] test: remove ->expects(self::any()) --- tests/ConnectionTest.php | 22 +++++++++---------- .../Expression/ExpressionBuilderTest.php | 2 +- .../Visitor/DropSchemaSqlCollectorTest.php | 6 ++--- tests/StatementTest.php | 4 ++-- tests/Types/DateImmutableTypeTest.php | 2 +- tests/Types/DateTimeImmutableTypeTest.php | 2 +- tests/Types/GuidTypeTest.php | 2 +- tests/Types/TimeImmutableTypeTest.php | 2 +- 8 files changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index 719647989f9..10e1ea4d77b 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -59,7 +59,7 @@ private function getExecuteStatementMockConnection(): Connection { $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn( $this->createMock(DriverConnection::class), @@ -184,7 +184,7 @@ public function testTransactionBeginDispatchEvent(): void { $eventManager = new EventManager(); $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn( $this->createMock(DriverConnection::class), @@ -210,7 +210,7 @@ public function testTransactionCommitDispatchEvent(): void { $eventManager = new EventManager(); $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn( $this->createMock(DriverConnection::class), @@ -237,7 +237,7 @@ public function testTransactionCommitEventNotCalledAfterRollBack(): void { $eventManager = new EventManager(); $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn( $this->createMock(DriverConnection::class), @@ -273,7 +273,7 @@ public function testTransactionRollBackDispatchEvent(): void { $eventManager = new EventManager(); $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn( $this->createMock(DriverConnection::class), @@ -307,7 +307,7 @@ public function testEventManagerPassedToPlatform(): void ->with($eventManager); $driver = $this->createMock(Driver::class); - $driver->expects(self::any()) + $driver ->method('getDatabasePlatform') ->willReturn($platform); @@ -380,7 +380,7 @@ public function testSetAutoCommit(): void public function testConnectStartsTransactionInNoAutoCommitMode(): void { $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn( $this->createMock(DriverConnection::class), @@ -399,7 +399,7 @@ public function testConnectStartsTransactionInNoAutoCommitMode(): void public function testCommitStartsTransactionInNoAutoCommitMode(): void { $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn( $this->createMock(DriverConnection::class), @@ -421,7 +421,7 @@ public function testCommitReturn(bool $expectedResult): void ->method('commit')->willReturn($expectedResult); $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn($driverConnection); @@ -442,7 +442,7 @@ public static function resultProvider(): array public function testRollBackStartsTransactionInNoAutoCommitMode(): void { $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn( $this->createMock(DriverConnection::class), @@ -459,7 +459,7 @@ public function testRollBackStartsTransactionInNoAutoCommitMode(): void public function testSwitchingAutoCommitModeCommitsAllCurrentTransactions(): void { $driverMock = $this->createMock(Driver::class); - $driverMock->expects(self::any()) + $driverMock ->method('connect') ->willReturn( $this->createMock(DriverConnection::class), diff --git a/tests/Query/Expression/ExpressionBuilderTest.php b/tests/Query/Expression/ExpressionBuilderTest.php index 4ecfeb9fa08..8ccd37994ad 100644 --- a/tests/Query/Expression/ExpressionBuilderTest.php +++ b/tests/Query/Expression/ExpressionBuilderTest.php @@ -17,7 +17,7 @@ protected function setUp(): void $this->expr = new ExpressionBuilder($conn); - $conn->expects(self::any()) + $conn ->method('getExpressionBuilder') ->willReturn($this->expr); } diff --git a/tests/Schema/Visitor/DropSchemaSqlCollectorTest.php b/tests/Schema/Visitor/DropSchemaSqlCollectorTest.php index a0fb23d0fb3..ddd28fcd00a 100644 --- a/tests/Schema/Visitor/DropSchemaSqlCollectorTest.php +++ b/tests/Schema/Visitor/DropSchemaSqlCollectorTest.php @@ -43,15 +43,15 @@ private function getStubKeyConstraint(string $name): ForeignKeyConstraint { $constraint = $this->createMock(ForeignKeyConstraint::class); - $constraint->expects(self::any()) + $constraint ->method('getName') ->willReturn($name); - $constraint->expects(self::any()) + $constraint ->method('getForeignColumns') ->willReturn([]); - $constraint->expects(self::any()) + $constraint ->method('getColumns') ->willReturn([]); diff --git a/tests/StatementTest.php b/tests/StatementTest.php index 1c91859c2cd..c248f0f0dde 100644 --- a/tests/StatementTest.php +++ b/tests/StatementTest.php @@ -36,11 +36,11 @@ protected function setUp(): void ->getMock(); $this->configuration = $this->createMock(Configuration::class); - $this->conn->expects(self::any()) + $this->conn ->method('getConfiguration') ->willReturn($this->configuration); - $this->conn->expects(self::any()) + $this->conn ->method('getDriver') ->willReturn($driver); } diff --git a/tests/Types/DateImmutableTypeTest.php b/tests/Types/DateImmutableTypeTest.php index f21c2802b12..15f0ad8a110 100644 --- a/tests/Types/DateImmutableTypeTest.php +++ b/tests/Types/DateImmutableTypeTest.php @@ -97,7 +97,7 @@ public function testConvertsDateStringToPHPValue(): void public function testResetTimeFractionsWhenConvertingToPHPValue(): void { - $this->platform->expects(self::any()) + $this->platform ->method('getDateFormatString') ->willReturn('Y-m-d'); diff --git a/tests/Types/DateTimeImmutableTypeTest.php b/tests/Types/DateTimeImmutableTypeTest.php index ffe8ec91e1c..00179f1fd87 100644 --- a/tests/Types/DateTimeImmutableTypeTest.php +++ b/tests/Types/DateTimeImmutableTypeTest.php @@ -98,7 +98,7 @@ public function testConvertsDateTimeStringToPHPValue(): void public function testConvertsDateTimeStringWithMicrosecondsToPHPValue(): void { - $this->platform->expects(self::any()) + $this->platform ->method('getDateTimeFormatString') ->willReturn('Y-m-d H:i:s'); diff --git a/tests/Types/GuidTypeTest.php b/tests/Types/GuidTypeTest.php index aa9adc05314..c6d756ff72a 100644 --- a/tests/Types/GuidTypeTest.php +++ b/tests/Types/GuidTypeTest.php @@ -35,7 +35,7 @@ public function testNativeGuidSupport(): void { self::assertTrue($this->type->requiresSQLCommentHint($this->platform)); - $this->platform->expects(self::any()) + $this->platform ->method('hasNativeGuidType') ->willReturn(true); diff --git a/tests/Types/TimeImmutableTypeTest.php b/tests/Types/TimeImmutableTypeTest.php index 3e67ed02169..75baa66459c 100644 --- a/tests/Types/TimeImmutableTypeTest.php +++ b/tests/Types/TimeImmutableTypeTest.php @@ -97,7 +97,7 @@ public function testConvertsTimeStringToPHPValue(): void public function testResetDateFractionsWhenConvertingToPHPValue(): void { - $this->platform->expects(self::any()) + $this->platform ->method('getTimeFormatString') ->willReturn('H:i:s'); From 8f60369a455895a487a11a15a1e19e7923c248ec Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Mon, 21 Oct 2024 09:58:13 +0200 Subject: [PATCH 6/7] [Bug] Query Cache mangled if saved by-reference (#6552) Bug emerged in https://github.com/doctrine/dbal/pull/6510#discussion_r1804475987 The current implementation of query cache relies on the cache **not** saved by-reference; the bug has never been seen before because by default `new ArrayAdapter()` saves the cache with serialization, hence breaking the by-reference pointer. Once the _by-reference_ tecnique is used, two issues pop up: 1. `\Doctrine\DBAL\Cache\ArrayResult::$num` is never reset, so once it gets incremented in the first `\Doctrine\DBAL\Cache\ArrayResult::fetch` call, the following calls will always fail 2. Even considering fixing the `$num` property reset, a manual call on `\Doctrine\DBAL\Result::free` will by cascade call the `\Doctrine\DBAL\Cache\ArrayResult::free` method erasing all the saved results I think that the `ArrayResult` implementation is not the culprit, but rather the #6510 giving to the cache backend the internal object by reference instead of giving it a copy. --- src/Connection.php | 4 ++-- tests/Connection/CachedQueryTest.php | 36 +++++++++++++++++++++------- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index 5a66e1ad21c..a81e57e2c7d 100644 --- 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/tests/Connection/CachedQueryTest.php b/tests/Connection/CachedQueryTest.php index 3d9c825a1bd..7f2fe62ad44 100644 --- a/tests/Connection/CachedQueryTest.php +++ b/tests/Connection/CachedQueryTest.php @@ -8,27 +8,37 @@ 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']]); @@ -36,21 +46,22 @@ public function testCachedQueryWithChangedImplementationIsExecutedTwice(): void '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> */ + public static function providePsrCacheImplementations(): array + { + return [ + 'serialized' => [static fn () => new ArrayAdapter(0, true)], + 'by-reference' => [static fn () => new ArrayAdapter(0, false)], + ]; + } } From b87a89f1da3865a25b1341104281a09818b77e12 Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Mon, 21 Oct 2024 13:05:27 +0200 Subject: [PATCH 7/7] Create stubs instead of mocks (#6564) | Q | A |------------- | ----------- | Type | improvement | Fixed issues | #### Summary _This is a followup of https://github.com/doctrine/dbal/pull/6558_, I have touched only what previous PR touched. @greg0ire [requested](https://github.com/doctrine/dbal/pull/6562#pullrequestreview-2380527773) to use stubs where there are no expectations ![image](https://github.com/user-attachments/assets/f71ad45f-87f7-4575-b10d-2f46b2c6525a) --- tests/ConnectionTest.php | 74 +++++++++---------- .../Expression/ExpressionBuilderTest.php | 2 +- .../Visitor/DropSchemaSqlCollectorTest.php | 2 +- 3 files changed, 39 insertions(+), 39 deletions(-) diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index 10e1ea4d77b..5fe70dd9a51 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -57,19 +57,19 @@ protected function setUp(): void /** @return Connection&MockObject */ private function getExecuteStatementMockConnection(): Connection { - $driverMock = $this->createMock(Driver::class); + $driver = $this->createStub(Driver::class); - $driverMock + $driver ->method('connect') ->willReturn( - $this->createMock(DriverConnection::class), + $this->createStub(DriverConnection::class), ); $platform = $this->getMockForAbstractClass(AbstractPlatform::class); return $this->getMockBuilder(Connection::class) ->onlyMethods(['executeStatement']) - ->setConstructorArgs([['platform' => $platform], $driverMock]) + ->setConstructorArgs([['platform' => $platform], $driver]) ->getMock(); } @@ -183,13 +183,13 @@ public function testConnectDispatchEvent(): void public function testTransactionBeginDispatchEvent(): void { $eventManager = new EventManager(); - $driverMock = $this->createMock(Driver::class); - $driverMock + $driver = $this->createStub(Driver::class); + $driver ->method('connect') ->willReturn( - $this->createMock(DriverConnection::class), + $this->createStub(DriverConnection::class), ); - $conn = new Connection([], $driverMock, new Configuration(), $eventManager); + $conn = new Connection([], $driver, new Configuration(), $eventManager); $listenerMock = $this->createMock(TransactionBeginDispatchEventListener::class); $listenerMock ->expects(self::exactly(1)) @@ -209,13 +209,13 @@ static function (TransactionBeginEventArgs $eventArgs) use ($conn): bool { public function testTransactionCommitDispatchEvent(): void { $eventManager = new EventManager(); - $driverMock = $this->createMock(Driver::class); - $driverMock + $driver = $this->createStub(Driver::class); + $driver ->method('connect') ->willReturn( - $this->createMock(DriverConnection::class), + $this->createStub(DriverConnection::class), ); - $conn = new Connection([], $driverMock, new Configuration(), $eventManager); + $conn = new Connection([], $driver, new Configuration(), $eventManager); $listenerMock = $this->createMock(TransactionCommitDispatchEventListener::class); $listenerMock ->expects(self::exactly(1)) @@ -236,13 +236,13 @@ static function (TransactionCommitEventArgs $eventArgs) use ($conn): bool { public function testTransactionCommitEventNotCalledAfterRollBack(): void { $eventManager = new EventManager(); - $driverMock = $this->createMock(Driver::class); - $driverMock + $driver = $this->createStub(Driver::class); + $driver ->method('connect') ->willReturn( - $this->createMock(DriverConnection::class), + $this->createStub(DriverConnection::class), ); - $conn = new Connection([], $driverMock, new Configuration(), $eventManager); + $conn = new Connection([], $driver, new Configuration(), $eventManager); $rollBackListenerMock = $this->createMock(TransactionRollBackDispatchEventListener::class); $rollBackListenerMock ->expects(self::exactly(1)) @@ -272,13 +272,13 @@ static function (TransactionRollBackEventArgs $eventArgs) use ($conn): bool { public function testTransactionRollBackDispatchEvent(): void { $eventManager = new EventManager(); - $driverMock = $this->createMock(Driver::class); - $driverMock + $driver = $this->createStub(Driver::class); + $driver ->method('connect') ->willReturn( - $this->createMock(DriverConnection::class), + $this->createStub(DriverConnection::class), ); - $conn = new Connection([], $driverMock, new Configuration(), $eventManager); + $conn = new Connection([], $driver, new Configuration(), $eventManager); $listenerMock = $this->createMock(TransactionRollBackDispatchEventListener::class); $listenerMock ->expects(self::exactly(1)) @@ -306,7 +306,7 @@ public function testEventManagerPassedToPlatform(): void ->method('setEventManager') ->with($eventManager); - $driver = $this->createMock(Driver::class); + $driver = $this->createStub(Driver::class); $driver ->method('getDatabasePlatform') ->willReturn($platform); @@ -383,7 +383,7 @@ public function testConnectStartsTransactionInNoAutoCommitMode(): void $driverMock ->method('connect') ->willReturn( - $this->createMock(DriverConnection::class), + $this->createStub(DriverConnection::class), ); $conn = new Connection([], $driverMock); @@ -398,13 +398,13 @@ public function testConnectStartsTransactionInNoAutoCommitMode(): void public function testCommitStartsTransactionInNoAutoCommitMode(): void { - $driverMock = $this->createMock(Driver::class); - $driverMock + $driver = $this->createStub(Driver::class); + $driver ->method('connect') ->willReturn( - $this->createMock(DriverConnection::class), + $this->createStub(DriverConnection::class), ); - $conn = new Connection([], $driverMock); + $conn = new Connection([], $driver); $conn->setAutoCommit(false); $conn->connect(); @@ -420,12 +420,12 @@ public function testCommitReturn(bool $expectedResult): void $driverConnection->expects(self::once()) ->method('commit')->willReturn($expectedResult); - $driverMock = $this->createMock(Driver::class); - $driverMock + $driver = $this->createStub(Driver::class); + $driver ->method('connect') ->willReturn($driverConnection); - $conn = new Connection([], $driverMock); + $conn = new Connection([], $driver); $conn->connect(); $conn->beginTransaction(); @@ -441,13 +441,13 @@ public static function resultProvider(): array public function testRollBackStartsTransactionInNoAutoCommitMode(): void { - $driverMock = $this->createMock(Driver::class); - $driverMock + $driver = $this->createStub(Driver::class); + $driver ->method('connect') ->willReturn( - $this->createMock(DriverConnection::class), + $this->createStub(DriverConnection::class), ); - $conn = new Connection([], $driverMock); + $conn = new Connection([], $driver); $conn->setAutoCommit(false); $conn->connect(); @@ -458,13 +458,13 @@ public function testRollBackStartsTransactionInNoAutoCommitMode(): void public function testSwitchingAutoCommitModeCommitsAllCurrentTransactions(): void { - $driverMock = $this->createMock(Driver::class); - $driverMock + $driver = $this->createStub(Driver::class); + $driver ->method('connect') ->willReturn( - $this->createMock(DriverConnection::class), + $this->createStub(DriverConnection::class), ); - $conn = new Connection([], $driverMock); + $conn = new Connection([], $driver); $conn->connect(); $conn->beginTransaction(); diff --git a/tests/Query/Expression/ExpressionBuilderTest.php b/tests/Query/Expression/ExpressionBuilderTest.php index 8ccd37994ad..11175c5e8b3 100644 --- a/tests/Query/Expression/ExpressionBuilderTest.php +++ b/tests/Query/Expression/ExpressionBuilderTest.php @@ -13,7 +13,7 @@ class ExpressionBuilderTest extends TestCase protected function setUp(): void { - $conn = $this->createMock(Connection::class); + $conn = $this->createStub(Connection::class); $this->expr = new ExpressionBuilder($conn); diff --git a/tests/Schema/Visitor/DropSchemaSqlCollectorTest.php b/tests/Schema/Visitor/DropSchemaSqlCollectorTest.php index ddd28fcd00a..dd28419d242 100644 --- a/tests/Schema/Visitor/DropSchemaSqlCollectorTest.php +++ b/tests/Schema/Visitor/DropSchemaSqlCollectorTest.php @@ -41,7 +41,7 @@ public function testGetQueriesUsesAcceptedForeignKeys(): void private function getStubKeyConstraint(string $name): ForeignKeyConstraint { - $constraint = $this->createMock(ForeignKeyConstraint::class); + $constraint = $this->createStub(ForeignKeyConstraint::class); $constraint ->method('getName')