From 4cd413b777c9d1db3213f133f3b41a7503e78efe Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Mon, 11 Nov 2024 18:06:00 -0300 Subject: [PATCH] [11.x] Skip the number of connections transacting while testing to run callbacks (#53377) * Skip the number of connections transacting while testing to run callbacks The testing DatabaseTransactionsManager was hardcoding the number of connections to skip as 1. In a multi-db context, it must skip the same number of connections transacting defined on the tests. * Pass the connection names array instead of the number of connections * Fix tests for the testing DatabaseTransactionsManager * Fix DatabaseTransactions trait * [11.x] Test Improvements Signed-off-by: Mior Muhammad Zaki * wip Signed-off-by: Mior Muhammad Zaki * wip Signed-off-by: Mior Muhammad Zaki * wip Signed-off-by: Mior Muhammad Zaki * Adds tests for afterCommit callbacks when using multi-dbs * Update DatabaseTransactionsManager.php --------- Signed-off-by: Mior Muhammad Zaki Co-authored-by: Mior Muhammad Zaki Co-authored-by: Taylor Otwell --- .../Testing/DatabaseTransactions.php | 6 +- .../Testing/DatabaseTransactionsManager.php | 20 ++++++- .../Foundation/Testing/RefreshDatabase.php | 6 +- .../DatabaseTransactionsManagerTest.php | 25 +++++--- ...freshDatabaseOnMultipleConnectionsTest.php | 57 +++++++++++++++++++ ...ithAfterCommitUsingRefreshDatabaseTest.php | 4 +- 6 files changed, 105 insertions(+), 13 deletions(-) create mode 100644 tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index 83a686f3558c..f84a23fe51d4 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php @@ -13,9 +13,11 @@ public function beginDatabaseTransaction() { $database = $this->app->make('db'); - $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager); + $connections = $this->connectionsToTransact(); - foreach ($this->connectionsToTransact() as $name) { + $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager($connections)); + + foreach ($connections as $name) { $connection = $database->connection($name); $connection->setTransactionManager($transactionsManager); $dispatcher = $connection->getEventDispatcher(); diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php index bc5450486d48..7ee71d7b9edf 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php @@ -6,6 +6,24 @@ class DatabaseTransactionsManager extends BaseManager { + /** + * The names of the connections transacting during tests. + */ + protected array $connectionsTransacting; + + /** + * Create a new database transaction manager instance. + * + * @param array $connectionsTransacting + * @return void + */ + public function __construct(array $connectionsTransacting) + { + parent::__construct(); + + $this->connectionsTransacting = $connectionsTransacting; + } + /** * Register a transaction callback. * @@ -31,7 +49,7 @@ public function addCallback($callback) */ public function callbackApplicableTransactions() { - return $this->pendingTransactions->skip(1)->values(); + return $this->pendingTransactions->skip(count($this->connectionsTransacting))->values(); } /** diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 4c4e084ab0fa..03d8d558c737 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -82,9 +82,11 @@ public function beginDatabaseTransaction() { $database = $this->app->make('db'); - $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager); + $connections = $this->connectionsToTransact(); - foreach ($this->connectionsToTransact() as $name) { + $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager($connections)); + + foreach ($connections as $name) { $connection = $database->connection($name); $connection->setTransactionManager($transactionsManager); diff --git a/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php b/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php index 82ad8b410bee..e82d83e0ab2e 100644 --- a/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php +++ b/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php @@ -9,8 +9,8 @@ class DatabaseTransactionsManagerTest extends TestCase { public function testItExecutesCallbacksImmediatelyIfThereIsOnlyOneTransaction() { - $testObject = new TestingDatabaseTransactionsManagerTestObject(); - $manager = new DatabaseTransactionsManager; + $testObject = new TestingDatabaseTransactionsManagerTestObject; + $manager = new DatabaseTransactionsManager([null]); $manager->begin('foo', 1); @@ -22,7 +22,7 @@ public function testItExecutesCallbacksImmediatelyIfThereIsOnlyOneTransaction() public function testItIgnoresTheBaseTransactionForCallbackApplicableTransactions() { - $manager = new DatabaseTransactionsManager; + $manager = new DatabaseTransactionsManager([null]); $manager->begin('foo', 1); $manager->begin('foo', 2); @@ -33,7 +33,7 @@ public function testItIgnoresTheBaseTransactionForCallbackApplicableTransactions public function testCommittingDoesNotRemoveTheBasePendingTransaction() { - $manager = new DatabaseTransactionsManager; + $manager = new DatabaseTransactionsManager([null]); $manager->begin('foo', 1); @@ -50,8 +50,8 @@ public function testCommittingDoesNotRemoveTheBasePendingTransaction() public function testItExecutesCallbacksForTheSecondTransaction() { - $testObject = new TestingDatabaseTransactionsManagerTestObject(); - $manager = new DatabaseTransactionsManager; + $testObject = new TestingDatabaseTransactionsManagerTestObject; + $manager = new DatabaseTransactionsManager([null]); $manager->begin('foo', 1); $manager->begin('foo', 2); @@ -67,17 +67,28 @@ public function testItExecutesCallbacksForTheSecondTransaction() public function testItExecutesTransactionCallbacksAtLevelOne() { - $manager = new DatabaseTransactionsManager; + $manager = new DatabaseTransactionsManager([null]); $this->assertFalse($manager->afterCommitCallbacksShouldBeExecuted(0)); $this->assertTrue($manager->afterCommitCallbacksShouldBeExecuted(1)); $this->assertFalse($manager->afterCommitCallbacksShouldBeExecuted(2)); } + + public function testSkipsTheNumberOfConnectionsTransacting() + { + $manager = new DatabaseTransactionsManager([null]); + + $manager->begin('foo', 1); + $manager->begin('foo', 2); + + $this->assertCount(1, $manager->callbackApplicableTransactions()); + } } class TestingDatabaseTransactionsManagerTestObject { public $ran = false; + public $runs = 0; public function handle() diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php new file mode 100644 index 000000000000..b09a3f32cd00 --- /dev/null +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php @@ -0,0 +1,57 @@ + 'sqlite', 'database' => ':memory:', 'foreign_key_constraints' => false])] +class EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest extends EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest +{ + /** {@inheritDoc} */ + protected function connectionsToTransact() + { + return [null, 'second']; + } + + /** {@inheritDoc} */ + protected function afterRefreshingDatabase() + { + artisan($this, 'migrate', ['--database' => 'second']); + } + + public function testAfterCommitCallbacksAreCalledCorrectlyWhenNoAppTransaction() + { + $called = false; + + DB::afterCommit(function () use (&$called) { + $called = true; + }); + + $this->assertTrue($called); + } + + public function testAfterCommitCallbacksAreCalledWithWrappingTransactionsCorrectly() + { + $calls = []; + + DB::transaction(function () use (&$calls) { + DB::afterCommit(function () use (&$calls) { + $calls[] = 'first transaction callback'; + }); + + DB::connection('second')->transaction(function () use (&$calls) { + DB::connection('second')->afterCommit(function () use (&$calls) { + $calls[] = 'second transaction callback'; + }); + }); + }); + + $this->assertEquals([ + 'second transaction callback', + 'first transaction callback', + ], $calls); + } +} diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php index 21bb8291d480..49a66da7c1c9 100644 --- a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php @@ -17,6 +17,7 @@ class EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest extends TestCas */ protected $driver; + /** {@inheritDoc} */ protected function setUp(): void { $this->beforeApplicationDestroyed(function () { @@ -28,7 +29,8 @@ protected function setUp(): void parent::setUp(); } - protected function getEnvironmentSetUp($app) + /** {@inheritDoc} */ + protected function defineEnvironment($app) { $connection = $app['config']->get('database.default');