From c4d4fc15ede725f3e733cfdf70f6a312374df43d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Wed, 11 Oct 2023 02:58:48 -0300 Subject: [PATCH 01/18] wip --- .../Contracts/Events/TransactionAware.php | 6 ++ src/Illuminate/Events/Dispatcher.php | 71 ++++++++++++----- .../Events/TransactionAwareEventTest.php | 78 +++++++++++++++++++ 3 files changed, 136 insertions(+), 19 deletions(-) create mode 100644 src/Illuminate/Contracts/Events/TransactionAware.php create mode 100644 tests/Integration/Events/TransactionAwareEventTest.php diff --git a/src/Illuminate/Contracts/Events/TransactionAware.php b/src/Illuminate/Contracts/Events/TransactionAware.php new file mode 100644 index 000000000000..8e7b25e8cb44 --- /dev/null +++ b/src/Illuminate/Contracts/Events/TransactionAware.php @@ -0,0 +1,6 @@ +shouldBroadcast($payload)) { - $this->broadcastEvent($payload[0]); - } + $executeEvent = function () use ($halt, $event, $payload) { + if ($this->shouldBroadcast($payload)) { + $this->broadcastEvent($payload[0]); + } - $responses = []; + $responses = []; - foreach ($this->getListeners($event) as $listener) { - $response = $listener($event, $payload); + foreach ($this->getListeners($event) as $listener) { + $response = $listener($event, $payload); - // If a response is returned from the listener and event halting is enabled - // we will just return this response, and not call the rest of the event - // listeners. Otherwise we will add the response on the response list. - if ($halt && ! is_null($response)) { - return $response; - } + // If a response is returned from the listener and event halting is enabled + // we will just return this response, and not call the rest of the event + // listeners. Otherwise we will add the response on the response list. + if ($halt && !is_null($response)) { + return $response; + } - // If a boolean false is returned from a listener, we will stop propagating - // the event to any further listeners down in the chain, else we keep on - // looping through the listeners and firing every one in our sequence. - if ($response === false) { - break; + // If a boolean false is returned from a listener, we will stop propagating + // the event to any further listeners down in the chain, else we keep on + // looping through the listeners and firing every one in our sequence. + if ($response === false) { + break; + } + + $responses[] = $response; } - $responses[] = $response; + return $halt ? null : $responses; + }; + + // If the event is meant to be reserved upon a database transaction failure, + // we will add the actual event dispatching to a callback that executes + // once the database transaction has been committed within the DB. + if ( + $this->isEventObject($event, $payload) && + $payload[0] instanceof TransactionAware && + $this->container->bound('db.transactions') + ) { + $this->container->make('db.transactions')->addCallback($executeEvent); + + return null; } - return $halt ? null : $responses; + return $executeEvent(); } /** @@ -706,4 +724,19 @@ public function getRawListeners() { return $this->listeners; } + + /** + * + * @param mixed $event + * @param mixed $payload + * @return bool + */ + protected function isEventObject($event, $payload) + { + if (! isset($payload[0]) || ! is_object($payload[0])) { + return false; + } + + return get_class($payload[0]) === $event; + } } diff --git a/tests/Integration/Events/TransactionAwareEventTest.php b/tests/Integration/Events/TransactionAwareEventTest.php new file mode 100644 index 000000000000..9367af02482d --- /dev/null +++ b/tests/Integration/Events/TransactionAwareEventTest.php @@ -0,0 +1,78 @@ +assertTrue(TransactionAwareTestEvent::$ran); + } + + public function testEventIsNotDispatchedIfTransactionFails() + { + Event::listen(TransactionAwareTestEvent::class, TransactionAwareListener::class); + + try { + DB::transaction(function () { + Event::dispatch(new TransactionAwareTestEvent); + + throw new \Exception; + }); + } catch (\Exception) { + + } + + $this->assertFalse(TransactionAwareTestEvent::$ran); + } + + public function testEventIsDispatchedIfTransactionSucceeds() + { + Event::listen(TransactionAwareTestEvent::class, TransactionAwareListener::class); + + DB::transaction(function () { + Event::dispatch(new TransactionAwareTestEvent); + }); + + $this->assertTrue(TransactionAwareTestEvent::$ran); + } +} + +class TransactionUnawareTestEvent +{ + public static $ran = false; +} + +class TransactionAwareTestEvent implements TransactionAware +{ + public static $ran = false; +} + +class TransactionAwareListener +{ + public function handle(object $event) + { + $event::$ran = true; + } +} From 8bea21adf93ea6b3b526f28abaaea4c535e67b64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Wed, 11 Oct 2023 11:42:43 -0300 Subject: [PATCH 02/18] Refactor --- src/Illuminate/Events/Dispatcher.php | 92 +++++++++++++--------------- 1 file changed, 44 insertions(+), 48 deletions(-) diff --git a/src/Illuminate/Events/Dispatcher.php b/src/Illuminate/Events/Dispatcher.php index d622389b72eb..652ec2096fab 100755 --- a/src/Illuminate/Events/Dispatcher.php +++ b/src/Illuminate/Events/Dispatcher.php @@ -233,6 +233,8 @@ public function until($event, $payload = []) */ public function dispatch($event, $payload = [], $halt = false) { + $isEventObject = is_object($event); + // When the given "event" is actually an object we will assume it is an event // object and use the class as the event name and this event itself as the // payload to the handler, which makes object based events quite simple. @@ -240,50 +242,59 @@ public function dispatch($event, $payload = [], $halt = false) $event, $payload ); - $executeEvent = function () use ($halt, $event, $payload) { - if ($this->shouldBroadcast($payload)) { - $this->broadcastEvent($payload[0]); - } - - $responses = []; - - foreach ($this->getListeners($event) as $listener) { - $response = $listener($event, $payload); - - // If a response is returned from the listener and event halting is enabled - // we will just return this response, and not call the rest of the event - // listeners. Otherwise we will add the response on the response list. - if ($halt && !is_null($response)) { - return $response; - } - - // If a boolean false is returned from a listener, we will stop propagating - // the event to any further listeners down in the chain, else we keep on - // looping through the listeners and firing every one in our sequence. - if ($response === false) { - break; - } - - $responses[] = $response; - } - - return $halt ? null : $responses; - }; - // If the event is meant to be reserved upon a database transaction failure, // we will add the actual event dispatching to a callback that executes // once the database transaction has been committed within the DB. if ( - $this->isEventObject($event, $payload) && + $isEventObject && $payload[0] instanceof TransactionAware && $this->container->bound('db.transactions') ) { - $this->container->make('db.transactions')->addCallback($executeEvent); + $this->container->make('db.transactions')->addCallback(fn () => $this->executeListeners($event, $payload, $halt)); return null; } - return $executeEvent(); + return $this->executeListeners($event, $payload, $halt); + } + + /** + * Broadcast and event and call its listeners. + * + * @param string|object $event + * @param mixed $payload + * @param bool $halt + * @return array|null + */ + protected function executeListeners($event, $payload, $halt = false) + { + if ($this->shouldBroadcast($payload)) { + $this->broadcastEvent($payload[0]); + } + + $responses = []; + + foreach ($this->getListeners($event) as $listener) { + $response = $listener($event, $payload); + + // If a response is returned from the listener and event halting is enabled + // we will just return this response, and not call the rest of the event + // listeners. Otherwise we will add the response on the response list. + if ($halt && !is_null($response)) { + return $response; + } + + // If a boolean false is returned from a listener, we will stop propagating + // the event to any further listeners down in the chain, else we keep on + // looping through the listeners and firing every one in our sequence. + if ($response === false) { + break; + } + + $responses[] = $response; + } + + return $halt ? null : $responses; } /** @@ -724,19 +735,4 @@ public function getRawListeners() { return $this->listeners; } - - /** - * - * @param mixed $event - * @param mixed $payload - * @return bool - */ - protected function isEventObject($event, $payload) - { - if (! isset($payload[0]) || ! is_object($payload[0])) { - return false; - } - - return get_class($payload[0]) === $event; - } } From 68c960596923df00ad083a5565ce82ec82e43adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Wed, 11 Oct 2023 11:42:49 -0300 Subject: [PATCH 03/18] Add EventFake support --- .../Support/Testing/Fakes/EventFake.php | 24 +++++++++- tests/Integration/Events/EventFakeTest.php | 44 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Support/Testing/Fakes/EventFake.php b/src/Illuminate/Support/Testing/Fakes/EventFake.php index 7a32315ce5d5..24d865a55f7c 100644 --- a/src/Illuminate/Support/Testing/Fakes/EventFake.php +++ b/src/Illuminate/Support/Testing/Fakes/EventFake.php @@ -3,7 +3,9 @@ namespace Illuminate\Support\Testing\Fakes; use Closure; +use Illuminate\Container\Container; use Illuminate\Contracts\Events\Dispatcher; +use Illuminate\Contracts\Events\TransactionAware; use Illuminate\Support\Arr; use Illuminate\Support\Str; use Illuminate\Support\Traits\ForwardsCalls; @@ -297,7 +299,7 @@ public function dispatch($event, $payload = [], $halt = false) $name = is_object($event) ? get_class($event) : (string) $event; if ($this->shouldFakeEvent($name, $payload)) { - $this->events[$name][] = func_get_args(); + $this->fakeEvent($event, $name); } else { return $this->dispatcher->dispatch($event, $payload, $halt); } @@ -329,6 +331,26 @@ protected function shouldFakeEvent($eventName, $payload) ->isNotEmpty(); } + /** + * Push the event onto the fake events array immediately or after the next database transaction. + * + * @param string|object $event + * @param string$name + * @return void + */ + protected function fakeEvent($event, $name) + { + if ( + $event instanceof TransactionAware && + Container::getInstance()->bound('db.transactions') + ) { + return Container::getInstance()->make('db.transactions') + ->addCallback(fn() => $this->events[$name][] = func_get_args()); + } + + $this->events[$name][] = func_get_args(); + } + /** * Determine whether an event should be dispatched or not. * diff --git a/tests/Integration/Events/EventFakeTest.php b/tests/Integration/Events/EventFakeTest.php index 2d85bbd01274..f18477c1f817 100644 --- a/tests/Integration/Events/EventFakeTest.php +++ b/tests/Integration/Events/EventFakeTest.php @@ -3,9 +3,12 @@ namespace Illuminate\Tests\Integration\Events; use Closure; +use Exception; +use Illuminate\Contracts\Events\TransactionAware; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Arr; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Schema; use Orchestra\Testbench\TestCase; @@ -182,6 +185,41 @@ public function testMissingMethodsAreForwarded() $this->assertEquals('bar', Event::fake()->foo()); } + + public function testTransactionAwareEventsAreNotDispatchedIfTransactionFails() + { + Event::fake(); + + try { + DB::transaction(function () { + Event::dispatch(new TransactionAwareEvent()); + + throw new Exception('foo'); + }); + } catch (Exception $e) { + } + + Event::assertNotDispatched(TransactionAwareEvent::class); + } + + public function testTransactionAwareEventsAreDispatchedIfTransactionSucceeds() + { + Event::fake(); + + DB::transaction(function () { + Event::dispatch(new TransactionAwareEvent()); + }); + + Event::assertDispatched(TransactionAwareEvent::class); + } + + public function testTransactionAwareEventsAreDispatchedIfThereIsNoTransaction() + { + Event::fake(); + + Event::dispatch(new TransactionAwareEvent()); + Event::assertDispatched(TransactionAwareEvent::class); + } } class Post extends Model @@ -248,3 +286,9 @@ public function __invoke($event) // } } + +class TransactionAwareEvent implements TransactionAware +{ + // +} + From 1533420f33af7af79e0dee074e75bf54e1cfe9cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Wed, 11 Oct 2023 11:51:49 -0300 Subject: [PATCH 04/18] remove strict types --- tests/Integration/Events/TransactionAwareEventTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Integration/Events/TransactionAwareEventTest.php b/tests/Integration/Events/TransactionAwareEventTest.php index 9367af02482d..38482b4c8bf6 100644 --- a/tests/Integration/Events/TransactionAwareEventTest.php +++ b/tests/Integration/Events/TransactionAwareEventTest.php @@ -1,11 +1,8 @@ Date: Wed, 11 Oct 2023 12:22:41 -0300 Subject: [PATCH 05/18] Make styleCI happy --- src/Illuminate/Contracts/Events/TransactionAware.php | 3 ++- src/Illuminate/Events/Dispatcher.php | 2 +- src/Illuminate/Support/Testing/Fakes/EventFake.php | 6 +++--- tests/Integration/Events/EventFakeTest.php | 1 - tests/Integration/Events/TransactionAwareEventTest.php | 1 - 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Illuminate/Contracts/Events/TransactionAware.php b/src/Illuminate/Contracts/Events/TransactionAware.php index 8e7b25e8cb44..350c4dc020d7 100644 --- a/src/Illuminate/Contracts/Events/TransactionAware.php +++ b/src/Illuminate/Contracts/Events/TransactionAware.php @@ -3,4 +3,5 @@ namespace Illuminate\Contracts\Events; interface TransactionAware -{} +{ +} diff --git a/src/Illuminate/Events/Dispatcher.php b/src/Illuminate/Events/Dispatcher.php index 652ec2096fab..7c84e61b98d5 100755 --- a/src/Illuminate/Events/Dispatcher.php +++ b/src/Illuminate/Events/Dispatcher.php @@ -280,7 +280,7 @@ protected function executeListeners($event, $payload, $halt = false) // If a response is returned from the listener and event halting is enabled // we will just return this response, and not call the rest of the event // listeners. Otherwise we will add the response on the response list. - if ($halt && !is_null($response)) { + if ($halt && ! is_null($response)) { return $response; } diff --git a/src/Illuminate/Support/Testing/Fakes/EventFake.php b/src/Illuminate/Support/Testing/Fakes/EventFake.php index 24d865a55f7c..80e7f639f0eb 100644 --- a/src/Illuminate/Support/Testing/Fakes/EventFake.php +++ b/src/Illuminate/Support/Testing/Fakes/EventFake.php @@ -334,8 +334,8 @@ protected function shouldFakeEvent($eventName, $payload) /** * Push the event onto the fake events array immediately or after the next database transaction. * - * @param string|object $event - * @param string$name + * @param string|object $event + * @param string $name * @return void */ protected function fakeEvent($event, $name) @@ -345,7 +345,7 @@ protected function fakeEvent($event, $name) Container::getInstance()->bound('db.transactions') ) { return Container::getInstance()->make('db.transactions') - ->addCallback(fn() => $this->events[$name][] = func_get_args()); + ->addCallback(fn () => $this->events[$name][] = func_get_args()); } $this->events[$name][] = func_get_args(); diff --git a/tests/Integration/Events/EventFakeTest.php b/tests/Integration/Events/EventFakeTest.php index f18477c1f817..5daf1b846516 100644 --- a/tests/Integration/Events/EventFakeTest.php +++ b/tests/Integration/Events/EventFakeTest.php @@ -291,4 +291,3 @@ class TransactionAwareEvent implements TransactionAware { // } - diff --git a/tests/Integration/Events/TransactionAwareEventTest.php b/tests/Integration/Events/TransactionAwareEventTest.php index 38482b4c8bf6..179f1effcbc8 100644 --- a/tests/Integration/Events/TransactionAwareEventTest.php +++ b/tests/Integration/Events/TransactionAwareEventTest.php @@ -38,7 +38,6 @@ public function testEventIsNotDispatchedIfTransactionFails() throw new \Exception; }); } catch (\Exception) { - } $this->assertFalse(TransactionAwareTestEvent::$ran); From a3fd202d46429a115bf5dcb9ae24464a95d80ebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Wed, 11 Oct 2023 12:49:31 -0300 Subject: [PATCH 06/18] Fix test --- src/Illuminate/Support/Testing/Fakes/EventFake.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Support/Testing/Fakes/EventFake.php b/src/Illuminate/Support/Testing/Fakes/EventFake.php index 80e7f639f0eb..41ac97f66ac4 100644 --- a/src/Illuminate/Support/Testing/Fakes/EventFake.php +++ b/src/Illuminate/Support/Testing/Fakes/EventFake.php @@ -299,7 +299,7 @@ public function dispatch($event, $payload = [], $halt = false) $name = is_object($event) ? get_class($event) : (string) $event; if ($this->shouldFakeEvent($name, $payload)) { - $this->fakeEvent($event, $name); + $this->fakeEvent($event, $name, func_get_args()); } else { return $this->dispatcher->dispatch($event, $payload, $halt); } @@ -336,19 +336,20 @@ protected function shouldFakeEvent($eventName, $payload) * * @param string|object $event * @param string $name + * @param array $args * @return void */ - protected function fakeEvent($event, $name) + protected function fakeEvent($event, $name, $args) { if ( $event instanceof TransactionAware && Container::getInstance()->bound('db.transactions') ) { return Container::getInstance()->make('db.transactions') - ->addCallback(fn () => $this->events[$name][] = func_get_args()); + ->addCallback(fn () => $this->events[$name][] = $args); } - $this->events[$name][] = func_get_args(); + $this->events[$name][] = $args; } /** From 6b29903f2ccd731e5927c844b7fc214a9e59afc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Wed, 11 Oct 2023 12:54:43 -0300 Subject: [PATCH 07/18] Add missing test for EventFake --- tests/Integration/Events/EventFakeTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/Integration/Events/EventFakeTest.php b/tests/Integration/Events/EventFakeTest.php index 5daf1b846516..649b8a4eecb3 100644 --- a/tests/Integration/Events/EventFakeTest.php +++ b/tests/Integration/Events/EventFakeTest.php @@ -12,6 +12,7 @@ use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Schema; use Orchestra\Testbench\TestCase; +use PHPUnit\Framework\ExpectationFailedException; class EventFakeTest extends TestCase { @@ -220,6 +221,22 @@ public function testTransactionAwareEventsAreDispatchedIfThereIsNoTransaction() Event::dispatch(new TransactionAwareEvent()); Event::assertDispatched(TransactionAwareEvent::class); } + + public function testAssertNothingDispatchedTransactionAware() + { + Event::fake(); + Event::assertNothingDispatched(); + + Event::dispatch(new TransactionAwareEvent); + Event::dispatch(new TransactionAwareEvent); + + try { + Event::assertNothingDispatched(); + $this->fail(); + } catch (ExpectationFailedException $e) { + $this->assertStringContainsString('2 unexpected events were dispatched.', $e->getMessage()); + } + } } class Post extends Model From e12daa50dca763f74ff9b40a8f444ce45dd78c92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Wed, 25 Oct 2023 11:31:16 -0300 Subject: [PATCH 08/18] Add test to handle nested transactions --- .../Events/TransactionAwareEventTest.php | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/Integration/Events/TransactionAwareEventTest.php b/tests/Integration/Events/TransactionAwareEventTest.php index 179f1effcbc8..2c4a0a6e4655 100644 --- a/tests/Integration/Events/TransactionAwareEventTest.php +++ b/tests/Integration/Events/TransactionAwareEventTest.php @@ -53,6 +53,34 @@ public function testEventIsDispatchedIfTransactionSucceeds() $this->assertTrue(TransactionAwareTestEvent::$ran); } + + public function testItHandlesNestedTransactions() + { + // We are going to dispatch 2 different events in 2 different transactions. + // The parent transaction will succeed, but the nested transaction is going to fail and be rolled back. + // We want to ensure the event dispatched on the child transaction does not get published, since it failed, + // however, the event dispatched on the parent transaction should still be dispatched as usual. + Event::listen(TransactionAwareTestEvent::class, TransactionAwareListener::class); + Event::listen(AnotherTransactionAwareTestEvent::class, AnotherTransactionAwareListener::class); + + DB::transaction(function () { + try { + DB::transaction(function () { + // This event should not be dispatched since the transaction is going to fail. + Event::dispatch(new TransactionAwareTestEvent); + throw new \Exception; + }); + } catch (\Exception) { + + } + + // This event should be dispatched, as the parent transaction does not fail. + Event::dispatch(new AnotherTransactionAwareTestEvent); + }); + + $this->assertFalse(TransactionAwareTestEvent::$ran); + $this->assertTrue(AnotherTransactionAwareTestEvent::$ran); + } } class TransactionUnawareTestEvent @@ -65,6 +93,11 @@ class TransactionAwareTestEvent implements TransactionAware public static $ran = false; } +class AnotherTransactionAwareTestEvent implements TransactionAware +{ + public static $ran = false; +} + class TransactionAwareListener { public function handle(object $event) @@ -72,3 +105,11 @@ public function handle(object $event) $event::$ran = true; } } + +class AnotherTransactionAwareListener +{ + public function handle(object $event) + { + $event::$ran = true; + } +} From c307e84ff2d949eeff7b69290173a3b79aa06ec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Wed, 25 Oct 2023 11:39:26 -0300 Subject: [PATCH 09/18] fix typo --- src/Illuminate/Events/Dispatcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Events/Dispatcher.php b/src/Illuminate/Events/Dispatcher.php index 7c84e61b98d5..22d76240f3c8 100755 --- a/src/Illuminate/Events/Dispatcher.php +++ b/src/Illuminate/Events/Dispatcher.php @@ -259,7 +259,7 @@ public function dispatch($event, $payload = [], $halt = false) } /** - * Broadcast and event and call its listeners. + * Broadcast an event and call its listeners. * * @param string|object $event * @param mixed $payload From 909d9c582ea437c3db1f7228f21e40e3d81f0e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Wed, 25 Oct 2023 11:43:28 -0300 Subject: [PATCH 10/18] Make styleci happy --- tests/Integration/Events/TransactionAwareEventTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Integration/Events/TransactionAwareEventTest.php b/tests/Integration/Events/TransactionAwareEventTest.php index 2c4a0a6e4655..0de3a8d000f2 100644 --- a/tests/Integration/Events/TransactionAwareEventTest.php +++ b/tests/Integration/Events/TransactionAwareEventTest.php @@ -71,7 +71,6 @@ public function testItHandlesNestedTransactions() throw new \Exception; }); } catch (\Exception) { - } // This event should be dispatched, as the parent transaction does not fail. From b031d8ff37a011b69c633c04288228c21d13eeb0 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 26 Oct 2023 14:10:00 -0500 Subject: [PATCH 11/18] formatting, inject manager resolver --- src/Illuminate/Events/Dispatcher.php | 61 ++++++++++++++----- .../Events/EventServiceProvider.php | 4 ++ 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/Illuminate/Events/Dispatcher.php b/src/Illuminate/Events/Dispatcher.php index 22d76240f3c8..5f36f99dddf3 100755 --- a/src/Illuminate/Events/Dispatcher.php +++ b/src/Illuminate/Events/Dispatcher.php @@ -57,6 +57,13 @@ class Dispatcher implements DispatcherContract */ protected $queueResolver; + /** + * The database transaction manager resolver instance. + * + * @var callable + */ + protected $transactionManagerResolver; + /** * Create a new event dispatcher instance. * @@ -233,29 +240,28 @@ public function until($event, $payload = []) */ public function dispatch($event, $payload = [], $halt = false) { - $isEventObject = is_object($event); - // When the given "event" is actually an object we will assume it is an event // object and use the class as the event name and this event itself as the // payload to the handler, which makes object based events quite simple. - [$event, $payload] = $this->parseEventAndPayload( - $event, $payload - ); - - // If the event is meant to be reserved upon a database transaction failure, - // we will add the actual event dispatching to a callback that executes - // once the database transaction has been committed within the DB. - if ( - $isEventObject && + [$isEventObject, $event, $payload] = [ + is_object($event), + ...$this->parseEventAndPayload($event, $payload) + ]; + + // If the event is meant to be reserved upon a database transaction failure + // we will add the actual event dispatching to a callback which executes + // after the database transaction has been "committed" within this DB. + if ($isEventObject && $payload[0] instanceof TransactionAware && - $this->container->bound('db.transactions') - ) { - $this->container->make('db.transactions')->addCallback(fn () => $this->executeListeners($event, $payload, $halt)); + $this->resolveTransactionManager()) { + $this->resolveTransactionManager()->addCallback( + fn () => $this->invokeListeners($event, $payload, $halt) + ); return null; } - return $this->executeListeners($event, $payload, $halt); + return $this->invokeListeners($event, $payload, $halt); } /** @@ -266,7 +272,7 @@ public function dispatch($event, $payload = [], $halt = false) * @param bool $halt * @return array|null */ - protected function executeListeners($event, $payload, $halt = false) + protected function invokeListeners($event, $payload, $halt = false) { if ($this->shouldBroadcast($payload)) { $this->broadcastEvent($payload[0]); @@ -726,6 +732,29 @@ public function setQueueResolver(callable $resolver) return $this; } + /** + * Get the database transaction manager implementation from the resolver. + * + * @return \Illuminate\Database\DatabaseTransactionsManager|null + */ + protected function resolveTransactionManager() + { + return call_user_func($this->transactionManagerResolver); + } + + /** + * Set the database transaction manager resolver implementation. + * + * @param callable $resolver + * @return $this + */ + public function setTransactionManagerResolver(callable $resolver) + { + $this->transactionManagerResolver = $resolver; + + return $this; + } + /** * Gets the raw, unprepared listeners. * diff --git a/src/Illuminate/Events/EventServiceProvider.php b/src/Illuminate/Events/EventServiceProvider.php index 15fb60b10bba..cf9fbe25e3c3 100755 --- a/src/Illuminate/Events/EventServiceProvider.php +++ b/src/Illuminate/Events/EventServiceProvider.php @@ -17,6 +17,10 @@ public function register() $this->app->singleton('events', function ($app) { return (new Dispatcher($app))->setQueueResolver(function () use ($app) { return $app->make(QueueFactoryContract::class); + })->setTransactionManagerResolver(function () use ($app) { + return $app->bound('db.transactions') + ? $app->make('db.transactions') + : null; }); }); } From 414bba166106253fe9f882750d795d053d3b4bac Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 26 Oct 2023 14:21:30 -0500 Subject: [PATCH 12/18] formatting --- src/Illuminate/Events/Dispatcher.php | 10 +++++----- src/Illuminate/Support/Testing/Fakes/EventFake.php | 5 +---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Illuminate/Events/Dispatcher.php b/src/Illuminate/Events/Dispatcher.php index 5f36f99dddf3..fd5118e677d7 100755 --- a/src/Illuminate/Events/Dispatcher.php +++ b/src/Illuminate/Events/Dispatcher.php @@ -248,13 +248,13 @@ public function dispatch($event, $payload = [], $halt = false) ...$this->parseEventAndPayload($event, $payload) ]; - // If the event is meant to be reserved upon a database transaction failure - // we will add the actual event dispatching to a callback which executes - // after the database transaction has been "committed" within this DB. + // If the event is not intended to be dispatched unless the current database + // transaction is successful, we'll register a callback which will handle + // dispatching this event on the next successful DB transaction commit. if ($isEventObject && $payload[0] instanceof TransactionAware && - $this->resolveTransactionManager()) { - $this->resolveTransactionManager()->addCallback( + ! is_null($transactions = $this->resolveTransactionManager())) { + $transactions->addCallback( fn () => $this->invokeListeners($event, $payload, $halt) ); diff --git a/src/Illuminate/Support/Testing/Fakes/EventFake.php b/src/Illuminate/Support/Testing/Fakes/EventFake.php index 41ac97f66ac4..a19a44c1e9d0 100644 --- a/src/Illuminate/Support/Testing/Fakes/EventFake.php +++ b/src/Illuminate/Support/Testing/Fakes/EventFake.php @@ -341,10 +341,7 @@ protected function shouldFakeEvent($eventName, $payload) */ protected function fakeEvent($event, $name, $args) { - if ( - $event instanceof TransactionAware && - Container::getInstance()->bound('db.transactions') - ) { + if ($event instanceof TransactionAware && Container::getInstance()->bound('db.transactions')) { return Container::getInstance()->make('db.transactions') ->addCallback(fn () => $this->events[$name][] = $args); } From b2f9f2e7eab4d3cb6c5725a97eb6c70407ecf47b Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 26 Oct 2023 16:41:53 -0500 Subject: [PATCH 13/18] formatting --- ...ware.php => ShouldDispatchAfterCommit.php} | 3 +- src/Illuminate/Events/Dispatcher.php | 4 +- .../Support/Testing/Fakes/EventFake.php | 4 +- tests/Integration/Events/EventFakeTest.php | 28 ++++++------ ...=> ShouldDispatchAfterCommitEventTest.php} | 44 +++++++++---------- 5 files changed, 42 insertions(+), 41 deletions(-) rename src/Illuminate/Contracts/Events/{TransactionAware.php => ShouldDispatchAfterCommit.php} (54%) rename tests/Integration/Events/{TransactionAwareEventTest.php => ShouldDispatchAfterCommitEventTest.php} (53%) diff --git a/src/Illuminate/Contracts/Events/TransactionAware.php b/src/Illuminate/Contracts/Events/ShouldDispatchAfterCommit.php similarity index 54% rename from src/Illuminate/Contracts/Events/TransactionAware.php rename to src/Illuminate/Contracts/Events/ShouldDispatchAfterCommit.php index 350c4dc020d7..8f1fbdd4d697 100644 --- a/src/Illuminate/Contracts/Events/TransactionAware.php +++ b/src/Illuminate/Contracts/Events/ShouldDispatchAfterCommit.php @@ -2,6 +2,7 @@ namespace Illuminate\Contracts\Events; -interface TransactionAware +interface ShouldDispatchAfterCommit { + // } diff --git a/src/Illuminate/Events/Dispatcher.php b/src/Illuminate/Events/Dispatcher.php index fd5118e677d7..bc23b3325c4e 100755 --- a/src/Illuminate/Events/Dispatcher.php +++ b/src/Illuminate/Events/Dispatcher.php @@ -9,7 +9,7 @@ use Illuminate\Contracts\Broadcasting\ShouldBroadcast; use Illuminate\Contracts\Container\Container as ContainerContract; use Illuminate\Contracts\Events\Dispatcher as DispatcherContract; -use Illuminate\Contracts\Events\TransactionAware; +use Illuminate\Contracts\Events\ShouldDispatchAfterCommit; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Support\Arr; @@ -252,7 +252,7 @@ public function dispatch($event, $payload = [], $halt = false) // transaction is successful, we'll register a callback which will handle // dispatching this event on the next successful DB transaction commit. if ($isEventObject && - $payload[0] instanceof TransactionAware && + $payload[0] instanceof ShouldDispatchAfterCommit && ! is_null($transactions = $this->resolveTransactionManager())) { $transactions->addCallback( fn () => $this->invokeListeners($event, $payload, $halt) diff --git a/src/Illuminate/Support/Testing/Fakes/EventFake.php b/src/Illuminate/Support/Testing/Fakes/EventFake.php index a19a44c1e9d0..5f18cc71bf8d 100644 --- a/src/Illuminate/Support/Testing/Fakes/EventFake.php +++ b/src/Illuminate/Support/Testing/Fakes/EventFake.php @@ -5,7 +5,7 @@ use Closure; use Illuminate\Container\Container; use Illuminate\Contracts\Events\Dispatcher; -use Illuminate\Contracts\Events\TransactionAware; +use Illuminate\Contracts\Events\ShouldDispatchAfterCommit; use Illuminate\Support\Arr; use Illuminate\Support\Str; use Illuminate\Support\Traits\ForwardsCalls; @@ -341,7 +341,7 @@ protected function shouldFakeEvent($eventName, $payload) */ protected function fakeEvent($event, $name, $args) { - if ($event instanceof TransactionAware && Container::getInstance()->bound('db.transactions')) { + if ($event instanceof ShouldDispatchAfterCommit && Container::getInstance()->bound('db.transactions')) { return Container::getInstance()->make('db.transactions') ->addCallback(fn () => $this->events[$name][] = $args); } diff --git a/tests/Integration/Events/EventFakeTest.php b/tests/Integration/Events/EventFakeTest.php index 649b8a4eecb3..a7e9b97cf096 100644 --- a/tests/Integration/Events/EventFakeTest.php +++ b/tests/Integration/Events/EventFakeTest.php @@ -4,7 +4,7 @@ use Closure; use Exception; -use Illuminate\Contracts\Events\TransactionAware; +use Illuminate\Contracts\Events\ShouldDispatchAfterCommit; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Arr; @@ -187,48 +187,48 @@ public function testMissingMethodsAreForwarded() $this->assertEquals('bar', Event::fake()->foo()); } - public function testTransactionAwareEventsAreNotDispatchedIfTransactionFails() + public function testShouldDispatchAfterCommitEventsAreNotDispatchedIfTransactionFails() { Event::fake(); try { DB::transaction(function () { - Event::dispatch(new TransactionAwareEvent()); + Event::dispatch(new ShouldDispatchAfterCommitEvent()); throw new Exception('foo'); }); } catch (Exception $e) { } - Event::assertNotDispatched(TransactionAwareEvent::class); + Event::assertNotDispatched(ShouldDispatchAfterCommitEvent::class); } - public function testTransactionAwareEventsAreDispatchedIfTransactionSucceeds() + public function testShouldDispatchAfterCommitEventsAreDispatchedIfTransactionSucceeds() { Event::fake(); DB::transaction(function () { - Event::dispatch(new TransactionAwareEvent()); + Event::dispatch(new ShouldDispatchAfterCommitEvent()); }); - Event::assertDispatched(TransactionAwareEvent::class); + Event::assertDispatched(ShouldDispatchAfterCommitEvent::class); } - public function testTransactionAwareEventsAreDispatchedIfThereIsNoTransaction() + public function testShouldDispatchAfterCommitEventsAreDispatchedIfThereIsNoTransaction() { Event::fake(); - Event::dispatch(new TransactionAwareEvent()); - Event::assertDispatched(TransactionAwareEvent::class); + Event::dispatch(new ShouldDispatchAfterCommitEvent()); + Event::assertDispatched(ShouldDispatchAfterCommitEvent::class); } - public function testAssertNothingDispatchedTransactionAware() + public function testAssertNothingDispatchedShouldDispatchAfterCommit() { Event::fake(); Event::assertNothingDispatched(); - Event::dispatch(new TransactionAwareEvent); - Event::dispatch(new TransactionAwareEvent); + Event::dispatch(new ShouldDispatchAfterCommitEvent); + Event::dispatch(new ShouldDispatchAfterCommitEvent); try { Event::assertNothingDispatched(); @@ -304,7 +304,7 @@ public function __invoke($event) } } -class TransactionAwareEvent implements TransactionAware +class ShouldDispatchAfterCommitEvent implements ShouldDispatchAfterCommit { // } diff --git a/tests/Integration/Events/TransactionAwareEventTest.php b/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php similarity index 53% rename from tests/Integration/Events/TransactionAwareEventTest.php rename to tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php index 0de3a8d000f2..4d1ba999ac13 100644 --- a/tests/Integration/Events/TransactionAwareEventTest.php +++ b/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php @@ -2,56 +2,56 @@ namespace Illuminate\Tests\Integration\Events; -use Illuminate\Contracts\Events\TransactionAware; +use Illuminate\Contracts\Events\ShouldDispatchAfterCommit; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; use Mockery as m; use Orchestra\Testbench\TestCase; -class TransactionAwareEventTest extends TestCase +class ShouldDispatchAfterCommitEventTest extends TestCase { protected function tearDown(): void { TransactionUnawareTestEvent::$ran = false; - TransactionAwareTestEvent::$ran = false; + ShouldDispatchAfterCommitTestEvent::$ran = false; m::close(); } public function testEventIsDispatchedIfThereIsNoTransaction() { - Event::listen(TransactionAwareTestEvent::class, TransactionAwareListener::class); + Event::listen(ShouldDispatchAfterCommitTestEvent::class, ShouldDispatchAfterCommitListener::class); - Event::dispatch(new TransactionAwareTestEvent); + Event::dispatch(new ShouldDispatchAfterCommitTestEvent); - $this->assertTrue(TransactionAwareTestEvent::$ran); + $this->assertTrue(ShouldDispatchAfterCommitTestEvent::$ran); } public function testEventIsNotDispatchedIfTransactionFails() { - Event::listen(TransactionAwareTestEvent::class, TransactionAwareListener::class); + Event::listen(ShouldDispatchAfterCommitTestEvent::class, ShouldDispatchAfterCommitListener::class); try { DB::transaction(function () { - Event::dispatch(new TransactionAwareTestEvent); + Event::dispatch(new ShouldDispatchAfterCommitTestEvent); throw new \Exception; }); } catch (\Exception) { } - $this->assertFalse(TransactionAwareTestEvent::$ran); + $this->assertFalse(ShouldDispatchAfterCommitTestEvent::$ran); } public function testEventIsDispatchedIfTransactionSucceeds() { - Event::listen(TransactionAwareTestEvent::class, TransactionAwareListener::class); + Event::listen(ShouldDispatchAfterCommitTestEvent::class, ShouldDispatchAfterCommitListener::class); DB::transaction(function () { - Event::dispatch(new TransactionAwareTestEvent); + Event::dispatch(new ShouldDispatchAfterCommitTestEvent); }); - $this->assertTrue(TransactionAwareTestEvent::$ran); + $this->assertTrue(ShouldDispatchAfterCommitTestEvent::$ran); } public function testItHandlesNestedTransactions() @@ -60,25 +60,25 @@ public function testItHandlesNestedTransactions() // The parent transaction will succeed, but the nested transaction is going to fail and be rolled back. // We want to ensure the event dispatched on the child transaction does not get published, since it failed, // however, the event dispatched on the parent transaction should still be dispatched as usual. - Event::listen(TransactionAwareTestEvent::class, TransactionAwareListener::class); - Event::listen(AnotherTransactionAwareTestEvent::class, AnotherTransactionAwareListener::class); + Event::listen(ShouldDispatchAfterCommitTestEvent::class, ShouldDispatchAfterCommitListener::class); + Event::listen(AnotherShouldDispatchAfterCommitTestEvent::class, AnotherShouldDispatchAfterCommitListener::class); DB::transaction(function () { try { DB::transaction(function () { // This event should not be dispatched since the transaction is going to fail. - Event::dispatch(new TransactionAwareTestEvent); + Event::dispatch(new ShouldDispatchAfterCommitTestEvent); throw new \Exception; }); } catch (\Exception) { } // This event should be dispatched, as the parent transaction does not fail. - Event::dispatch(new AnotherTransactionAwareTestEvent); + Event::dispatch(new AnotherShouldDispatchAfterCommitTestEvent); }); - $this->assertFalse(TransactionAwareTestEvent::$ran); - $this->assertTrue(AnotherTransactionAwareTestEvent::$ran); + $this->assertFalse(ShouldDispatchAfterCommitTestEvent::$ran); + $this->assertTrue(AnotherShouldDispatchAfterCommitTestEvent::$ran); } } @@ -87,17 +87,17 @@ class TransactionUnawareTestEvent public static $ran = false; } -class TransactionAwareTestEvent implements TransactionAware +class ShouldDispatchAfterCommitTestEvent implements ShouldDispatchAfterCommit { public static $ran = false; } -class AnotherTransactionAwareTestEvent implements TransactionAware +class AnotherShouldDispatchAfterCommitTestEvent implements ShouldDispatchAfterCommit { public static $ran = false; } -class TransactionAwareListener +class ShouldDispatchAfterCommitListener { public function handle(object $event) { @@ -105,7 +105,7 @@ public function handle(object $event) } } -class AnotherTransactionAwareListener +class AnotherShouldDispatchAfterCommitListener { public function handle(object $event) { From e6e05a404b23a7363193c680b6863e9b81fddf32 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 26 Oct 2023 16:42:59 -0500 Subject: [PATCH 14/18] formatting --- src/Illuminate/Support/Testing/Fakes/EventFake.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Support/Testing/Fakes/EventFake.php b/src/Illuminate/Support/Testing/Fakes/EventFake.php index 5f18cc71bf8d..4a4fc7c5b22d 100644 --- a/src/Illuminate/Support/Testing/Fakes/EventFake.php +++ b/src/Illuminate/Support/Testing/Fakes/EventFake.php @@ -336,17 +336,17 @@ protected function shouldFakeEvent($eventName, $payload) * * @param string|object $event * @param string $name - * @param array $args + * @param array $arguments * @return void */ - protected function fakeEvent($event, $name, $args) + protected function fakeEvent($event, $name, $arguments) { if ($event instanceof ShouldDispatchAfterCommit && Container::getInstance()->bound('db.transactions')) { return Container::getInstance()->make('db.transactions') - ->addCallback(fn () => $this->events[$name][] = $args); + ->addCallback(fn () => $this->events[$name][] = $arguments); } - $this->events[$name][] = $args; + $this->events[$name][] = $arguments; } /** From ede1587b20bc31a6c88dbbaebaa37c7615e61048 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 26 Oct 2023 17:03:32 -0500 Subject: [PATCH 15/18] formatting --- src/Illuminate/Events/Dispatcher.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Events/Dispatcher.php b/src/Illuminate/Events/Dispatcher.php index bc23b3325c4e..21a3f5636c32 100755 --- a/src/Illuminate/Events/Dispatcher.php +++ b/src/Illuminate/Events/Dispatcher.php @@ -560,7 +560,7 @@ protected function createQueuedHandlerCallable($class, $method) */ protected function handlerShouldBeDispatchedAfterDatabaseTransactions($listener) { - return ($listener->afterCommit ?? null) && $this->container->bound('db.transactions'); + return ($listener->afterCommit ?? null) && $this->resolveTransactionManager(); } /** @@ -575,7 +575,7 @@ protected function createCallbackForListenerRunningAfterCommits($listener, $meth return function () use ($method, $listener) { $payload = func_get_args(); - $this->container->make('db.transactions')->addCallback( + $this->resolveTransactionManager()->addCallback( function () use ($listener, $method, $payload) { $listener->$method(...$payload); } From 7ed5cef33d54e6887c5c9f0cb494e5e88534739f Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Fri, 27 Oct 2023 10:11:15 -0500 Subject: [PATCH 16/18] more thorough solution --- .../Events/ShouldHandleEventsAfterCommit.php | 8 ++++++++ .../Contracts/Queue/ShouldQueueAfterCommit.php | 8 ++++++++ .../Eloquent/BroadcastsEventsAfterCommit.php | 18 ++++++++++++++++++ src/Illuminate/Events/Dispatcher.php | 13 +++++++++++-- src/Illuminate/Mail/SendQueuedMailable.php | 8 +++++++- .../Notifications/SendQueuedNotifications.php | 9 ++++++++- src/Illuminate/Queue/Queue.php | 5 +++++ 7 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 src/Illuminate/Contracts/Events/ShouldHandleEventsAfterCommit.php create mode 100644 src/Illuminate/Contracts/Queue/ShouldQueueAfterCommit.php create mode 100644 src/Illuminate/Database/Eloquent/BroadcastsEventsAfterCommit.php diff --git a/src/Illuminate/Contracts/Events/ShouldHandleEventsAfterCommit.php b/src/Illuminate/Contracts/Events/ShouldHandleEventsAfterCommit.php new file mode 100644 index 000000000000..8c70510370aa --- /dev/null +++ b/src/Illuminate/Contracts/Events/ShouldHandleEventsAfterCommit.php @@ -0,0 +1,8 @@ +afterCommit ?? null) && $this->resolveTransactionManager(); + return (($listener->afterCommit ?? null) || + $listener instanceof ShouldHandleEventsAfterCommit) && + $this->resolveTransactionManager(); } /** @@ -659,7 +663,12 @@ protected function propagateListenerOptions($listener, $job) return tap($job, function ($job) use ($listener) { $data = array_values($job->data); - $job->afterCommit = property_exists($listener, 'afterCommit') ? $listener->afterCommit : null; + if ($listener instanceof ShouldQueueAfterCommit) { + $job->afterCommit = true; + } else { + $job->afterCommit = property_exists($listener, 'afterCommit') ? $listener->afterCommit : null; + } + $job->backoff = method_exists($listener, 'backoff') ? $listener->backoff(...$data) : ($listener->backoff ?? null); $job->maxExceptions = $listener->maxExceptions ?? null; $job->retryUntil = method_exists($listener, 'retryUntil') ? $listener->retryUntil(...$data) : null; diff --git a/src/Illuminate/Mail/SendQueuedMailable.php b/src/Illuminate/Mail/SendQueuedMailable.php index 28a72c47c7de..b9fec9d03849 100644 --- a/src/Illuminate/Mail/SendQueuedMailable.php +++ b/src/Illuminate/Mail/SendQueuedMailable.php @@ -6,6 +6,7 @@ use Illuminate\Contracts\Mail\Factory as MailFactory; use Illuminate\Contracts\Mail\Mailable as MailableContract; use Illuminate\Contracts\Queue\ShouldBeEncrypted; +use Illuminate\Contracts\Queue\ShouldQueueAfterCommit; use Illuminate\Queue\InteractsWithQueue; class SendQueuedMailable @@ -57,7 +58,12 @@ public function __construct(MailableContract $mailable) { $this->mailable = $mailable; - $this->afterCommit = property_exists($mailable, 'afterCommit') ? $mailable->afterCommit : null; + if ($mailable instanceof ShouldQueueAfterCommit) { + $this->afterCommit = true; + } else { + $this->afterCommit = property_exists($mailable, 'afterCommit') ? $mailable->afterCommit : null; + } + $this->connection = property_exists($mailable, 'connection') ? $mailable->connection : null; $this->maxExceptions = property_exists($mailable, 'maxExceptions') ? $mailable->maxExceptions : null; $this->queue = property_exists($mailable, 'queue') ? $mailable->queue : null; diff --git a/src/Illuminate/Notifications/SendQueuedNotifications.php b/src/Illuminate/Notifications/SendQueuedNotifications.php index 19af18853667..f63d7bf9afe4 100644 --- a/src/Illuminate/Notifications/SendQueuedNotifications.php +++ b/src/Illuminate/Notifications/SendQueuedNotifications.php @@ -5,6 +5,7 @@ use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Contracts\Queue\ShouldQueueAfterCommit; use Illuminate\Database\Eloquent\Collection as EloquentCollection; use Illuminate\Database\Eloquent\Model; use Illuminate\Queue\InteractsWithQueue; @@ -80,7 +81,13 @@ public function __construct($notifiables, $notification, array $channels = null) $this->tries = property_exists($notification, 'tries') ? $notification->tries : null; $this->timeout = property_exists($notification, 'timeout') ? $notification->timeout : null; $this->maxExceptions = property_exists($notification, 'maxExceptions') ? $notification->maxExceptions : null; - $this->afterCommit = property_exists($notification, 'afterCommit') ? $notification->afterCommit : null; + + if ($notification instanceof ShouldQueueAfterCommit) { + $this->afterCommit = true; + } else { + $this->afterCommit = property_exists($notification, 'afterCommit') ? $notification->afterCommit : null; + } + $this->shouldBeEncrypted = $notification instanceof ShouldBeEncrypted; } diff --git a/src/Illuminate/Queue/Queue.php b/src/Illuminate/Queue/Queue.php index 03085f60a05d..0ce7ad1ac1ce 100755 --- a/src/Illuminate/Queue/Queue.php +++ b/src/Illuminate/Queue/Queue.php @@ -7,6 +7,7 @@ use Illuminate\Container\Container; use Illuminate\Contracts\Encryption\Encrypter; use Illuminate\Contracts\Queue\ShouldBeEncrypted; +use Illuminate\Contracts\Queue\ShouldQueueAfterCommit; use Illuminate\Queue\Events\JobQueued; use Illuminate\Support\Arr; use Illuminate\Support\InteractsWithTime; @@ -325,6 +326,10 @@ function () use ($payload, $queue, $delay, $callback, $job) { */ protected function shouldDispatchAfterCommit($job) { + if (is_object($job) && $job instanceof ShouldQueueAfterCommit) { + return true; + } + if (! $job instanceof Closure && is_object($job) && isset($job->afterCommit)) { return $job->afterCommit; } From 984f9aac923488b55cfb045d939e3b07d1ebc618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Sat, 28 Oct 2023 20:04:31 -0300 Subject: [PATCH 17/18] Add additional test for nested transactions --- .../ShouldDispatchAfterCommitEventTest.php | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php b/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php index 4d1ba999ac13..82832cdced60 100644 --- a/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php +++ b/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php @@ -80,6 +80,29 @@ public function testItHandlesNestedTransactions() $this->assertFalse(ShouldDispatchAfterCommitTestEvent::$ran); $this->assertTrue(AnotherShouldDispatchAfterCommitTestEvent::$ran); } + + public function testItOnlyDispatchesNestedTransactionsEventsAfterTheRootTransactionIsCommitted() + { + Event::listen(ShouldDispatchAfterCommitTestEvent::class, ShouldDispatchAfterCommitListener::class); + Event::listen(AnotherShouldDispatchAfterCommitTestEvent::class, AnotherShouldDispatchAfterCommitListener::class); + + DB::transaction(function () { + DB::transaction(function () { + Event::dispatch(new ShouldDispatchAfterCommitTestEvent); + }); + + // Although the child transaction has been concluded, the parent transaction has not. + // The event dispatched on the child transaction should not have been dispatched. + $this->assertFalse(ShouldDispatchAfterCommitTestEvent::$ran); + + Event::dispatch(new AnotherShouldDispatchAfterCommitTestEvent); + }); + + // Now that the parent transaction has been committed, the event + // on the child transaction should also have been dispatched. + $this->assertTrue(ShouldDispatchAfterCommitTestEvent::$ran); + $this->assertTrue(AnotherShouldDispatchAfterCommitTestEvent::$ran); + } } class TransactionUnawareTestEvent From 300ee7630c580e89d3d5299c7456863a983b515a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateus=20Guimar=C3=A3es?= Date: Sat, 28 Oct 2023 20:32:43 -0300 Subject: [PATCH 18/18] Add additional nested transaction test --- .../ShouldDispatchAfterCommitEventTest.php | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php b/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php index 82832cdced60..6984f26a6a35 100644 --- a/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php +++ b/tests/Integration/Events/ShouldDispatchAfterCommitEventTest.php @@ -86,6 +86,30 @@ public function testItOnlyDispatchesNestedTransactionsEventsAfterTheRootTransact Event::listen(ShouldDispatchAfterCommitTestEvent::class, ShouldDispatchAfterCommitListener::class); Event::listen(AnotherShouldDispatchAfterCommitTestEvent::class, AnotherShouldDispatchAfterCommitListener::class); + DB::transaction(function () { + Event::dispatch(new AnotherShouldDispatchAfterCommitTestEvent); + + DB::transaction(function () { + Event::dispatch(new ShouldDispatchAfterCommitTestEvent); + }); + + // Although the child transaction has been concluded, the parent transaction has not. + // The event dispatched on the child transaction should not have been dispatched. + $this->assertFalse(ShouldDispatchAfterCommitTestEvent::$ran); + $this->assertFalse(AnotherShouldDispatchAfterCommitTestEvent::$ran); + }); + + // Now that the parent transaction has been committed, the event + // on the child transaction should also have been dispatched. + $this->assertTrue(ShouldDispatchAfterCommitTestEvent::$ran); + $this->assertTrue(AnotherShouldDispatchAfterCommitTestEvent::$ran); + } + + public function testItOnlyDispatchesNestedTransactionsEventsAfterTheRootTransactionIsCommitedDifferentOrder() + { + Event::listen(ShouldDispatchAfterCommitTestEvent::class, ShouldDispatchAfterCommitListener::class); + Event::listen(AnotherShouldDispatchAfterCommitTestEvent::class, AnotherShouldDispatchAfterCommitListener::class); + DB::transaction(function () { DB::transaction(function () { Event::dispatch(new ShouldDispatchAfterCommitTestEvent); @@ -95,6 +119,8 @@ public function testItOnlyDispatchesNestedTransactionsEventsAfterTheRootTransact // The event dispatched on the child transaction should not have been dispatched. $this->assertFalse(ShouldDispatchAfterCommitTestEvent::$ran); + // The main difference with this test is that we dispatch an event on the parent transaction + // at the end. This is important due to how the DatabaseTransactionsManager works. Event::dispatch(new AnotherShouldDispatchAfterCommitTestEvent); });