From 4ac71cf08382c3dcd2753d578419d08bb748be2d Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sat, 18 Apr 2020 18:32:15 +0200 Subject: [PATCH 1/9] Allow the subscription broadcast job queue to be configured --- composer.json | 1 + .../Events/BroadcastSubscriptionListener.php | 23 ++++++------ .../Job/BroadcastSubscriptionJob.php | 37 +++++++++++++++++++ src/lighthouse.php | 5 +++ 4 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 src/Subscriptions/Job/BroadcastSubscriptionJob.php diff --git a/composer.json b/composer.json index 30379b3eb4..25223e9351 100644 --- a/composer.json +++ b/composer.json @@ -25,6 +25,7 @@ "require": { "php": ">= 7.1", "ext-json": "*", + "illuminate/bus": "5.5.* || 5.6.* || 5.7.* || 5.8.* || ^6.0 || ^7.0", "illuminate/contracts": "5.5.* || 5.6.* || 5.7.* || 5.8.* || ^6.0 || ^7.0", "illuminate/http": "5.5.* || 5.6.* || 5.7.* || 5.8.* || ^6.0 || ^7.0", "illuminate/pagination": "5.5.* || 5.6.* || 5.7.* || 5.8.* || ^6.0 || ^7.0", diff --git a/src/Subscriptions/Events/BroadcastSubscriptionListener.php b/src/Subscriptions/Events/BroadcastSubscriptionListener.php index b0ebac2453..68916244b0 100644 --- a/src/Subscriptions/Events/BroadcastSubscriptionListener.php +++ b/src/Subscriptions/Events/BroadcastSubscriptionListener.php @@ -2,32 +2,31 @@ namespace Nuwave\Lighthouse\Subscriptions\Events; -use Illuminate\Contracts\Queue\ShouldQueue; -use Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions; +use Illuminate\Contracts\Bus\Dispatcher as BusDispatcher; +use Nuwave\Lighthouse\Subscriptions\Job\BroadcastSubscriptionJob; -class BroadcastSubscriptionListener implements ShouldQueue +class BroadcastSubscriptionListener { /** - * @var \Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions + * @var \Illuminate\Contracts\Events\Dispatcher */ - protected $broadcaster; + protected $dispatcher; - public function __construct(BroadcastsSubscriptions $broadcaster) + public function __construct(BusDispatcher $dispatcher) { - $this->broadcaster = $broadcaster; + $this->dispatcher = $dispatcher; } /** * Handle the event. * - * @param \Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent $event + * @param \Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent $event */ public function handle(BroadcastSubscriptionEvent $event): void { - $this->broadcaster->broadcast( - $event->subscription, - $event->fieldName, - $event->root + $this->dispatcher->dispatch( + (new BroadcastSubscriptionJob($event)) + ->onQueue(config('lighthouse.subscriptions.broadcasts_queue_name', null)) ); } } diff --git a/src/Subscriptions/Job/BroadcastSubscriptionJob.php b/src/Subscriptions/Job/BroadcastSubscriptionJob.php new file mode 100644 index 0000000000..e32ae24c38 --- /dev/null +++ b/src/Subscriptions/Job/BroadcastSubscriptionJob.php @@ -0,0 +1,37 @@ +event = $event; + } + + /** + * Handle the event. + * + * @param \Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions $broadcaster + */ + public function handle(BroadcastsSubscriptions $broadcaster): void + { + $broadcaster->broadcast( + $this->event->subscription, + $this->event->fieldName, + $this->event->root + ); + } +} diff --git a/src/lighthouse.php b/src/lighthouse.php index 5f74410184..129c0d27ba 100644 --- a/src/lighthouse.php +++ b/src/lighthouse.php @@ -246,6 +246,11 @@ */ 'queue_broadcasts' => env('LIGHTHOUSE_QUEUE_BROADCASTS', true), + /* + * Determines the queue to use for broadcasting queue jobs. + */ + 'broadcasts_queue_name' => env('LIGHTHOUSE_BROADCASTS_QUEUE_NAME', null), + /* * Default subscription storage. * From 35fabcc6f004ad9f2c9c8837818fa54b3ae4334e Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sat, 18 Apr 2020 21:24:30 +0200 Subject: [PATCH 2/9] CS --- src/Subscriptions/Job/BroadcastSubscriptionJob.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Subscriptions/Job/BroadcastSubscriptionJob.php b/src/Subscriptions/Job/BroadcastSubscriptionJob.php index e32ae24c38..efcb0c7d7a 100644 --- a/src/Subscriptions/Job/BroadcastSubscriptionJob.php +++ b/src/Subscriptions/Job/BroadcastSubscriptionJob.php @@ -4,8 +4,8 @@ use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; -use Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent; use Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions; +use Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent; class BroadcastSubscriptionJob implements ShouldQueue { @@ -21,11 +21,6 @@ public function __construct(BroadcastSubscriptionEvent $event) $this->event = $event; } - /** - * Handle the event. - * - * @param \Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions $broadcaster - */ public function handle(BroadcastsSubscriptions $broadcaster): void { $broadcaster->broadcast( From 5f1865cc12a67a29354babf11981df412878ec86 Mon Sep 17 00:00:00 2001 From: spawnia Date: Sun, 19 Apr 2020 10:50:45 +0200 Subject: [PATCH 3/9] Add deprecations --- .../Events/BroadcastSubscriptionEvent.php | 7 ++-- .../Events/BroadcastSubscriptionListener.php | 21 ++++++++---- .../Job/BroadcastSubscriptionJob.php | 34 ++++++++++++++----- src/Subscriptions/SubscriptionBroadcaster.php | 1 + src/lighthouse.php | 2 +- 5 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/Subscriptions/Events/BroadcastSubscriptionEvent.php b/src/Subscriptions/Events/BroadcastSubscriptionEvent.php index 771391bfdf..22b6dd5788 100644 --- a/src/Subscriptions/Events/BroadcastSubscriptionEvent.php +++ b/src/Subscriptions/Events/BroadcastSubscriptionEvent.php @@ -3,8 +3,11 @@ namespace Nuwave\Lighthouse\Subscriptions\Events; use Illuminate\Queue\SerializesModels; -use Nuwave\Lighthouse\Schema\Types\GraphQLSubscription as Subscription; +use Nuwave\Lighthouse\Schema\Types\GraphQLSubscription; +/** + * @deprecated will be removed in v5 and replaced with a Job + */ class BroadcastSubscriptionEvent { use SerializesModels; @@ -21,7 +24,7 @@ class BroadcastSubscriptionEvent public $root; - public function __construct(Subscription $subscription, string $fieldName, $root) + public function __construct(GraphQLSubscription $subscription, string $fieldName, $root) { $this->subscription = $subscription; $this->fieldName = $fieldName; diff --git a/src/Subscriptions/Events/BroadcastSubscriptionListener.php b/src/Subscriptions/Events/BroadcastSubscriptionListener.php index 68916244b0..d4c8c53946 100644 --- a/src/Subscriptions/Events/BroadcastSubscriptionListener.php +++ b/src/Subscriptions/Events/BroadcastSubscriptionListener.php @@ -5,28 +5,35 @@ use Illuminate\Contracts\Bus\Dispatcher as BusDispatcher; use Nuwave\Lighthouse\Subscriptions\Job\BroadcastSubscriptionJob; +/** + * @deprecated This class is just here to preserve backwards compatiblity with v4. + * TODO remove the event and handle subscriptions as commands only in v5 + */ class BroadcastSubscriptionListener { /** * @var \Illuminate\Contracts\Events\Dispatcher */ - protected $dispatcher; + protected $busDispatcher; - public function __construct(BusDispatcher $dispatcher) + public function __construct(BusDispatcher $busDispatcher) { - $this->dispatcher = $dispatcher; + $this->busDispatcher = $busDispatcher; } /** * Handle the event. * - * @param \Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent $event + * @param \Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent $event */ public function handle(BroadcastSubscriptionEvent $event): void { - $this->dispatcher->dispatch( - (new BroadcastSubscriptionJob($event)) - ->onQueue(config('lighthouse.subscriptions.broadcasts_queue_name', null)) + $this->busDispatcher->dispatch( + (new BroadcastSubscriptionJob( + $event->subscription, + $event->fieldName, + $event->root + ))->onQueue(config('lighthouse.subscriptions.broadcasts_queue_name', null)) ); } } diff --git a/src/Subscriptions/Job/BroadcastSubscriptionJob.php b/src/Subscriptions/Job/BroadcastSubscriptionJob.php index efcb0c7d7a..4ff9c1e774 100644 --- a/src/Subscriptions/Job/BroadcastSubscriptionJob.php +++ b/src/Subscriptions/Job/BroadcastSubscriptionJob.php @@ -4,29 +4,47 @@ use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; +use Nuwave\Lighthouse\Schema\Types\GraphQLSubscription; use Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions; -use Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent; class BroadcastSubscriptionJob implements ShouldQueue { use Queueable; /** - * @var \Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent + * The subscription field that was requested. + * + * @var \Nuwave\Lighthouse\Schema\Types\GraphQLSubscription */ - public $event; + public $subscription; - public function __construct(BroadcastSubscriptionEvent $event) + /** + * The name of the field. + * + * @var string + */ + public $fieldName; + + /** + * The root element to be passed when resolving the subscription. + * + * @var mixed + */ + public $root; + + public function __construct(GraphQLSubscription $subscription, string $fieldName, $root) { - $this->event = $event; + $this->subscription = $subscription; + $this->fieldName = $fieldName; + $this->root = $root; } public function handle(BroadcastsSubscriptions $broadcaster): void { $broadcaster->broadcast( - $this->event->subscription, - $this->event->fieldName, - $this->event->root + $this->subscription, + $this->fieldName, + $this->root ); } } diff --git a/src/Subscriptions/SubscriptionBroadcaster.php b/src/Subscriptions/SubscriptionBroadcaster.php index 005ab9ab7c..53ec8bfa4c 100644 --- a/src/Subscriptions/SubscriptionBroadcaster.php +++ b/src/Subscriptions/SubscriptionBroadcaster.php @@ -66,6 +66,7 @@ public function __construct( */ public function queueBroadcast(GraphQLSubscription $subscription, string $fieldName, $root): void { + // TODO replace with a job dispatch $this->eventsDispatcher->dispatch( new BroadcastSubscriptionEvent($subscription, $fieldName, $root) ); diff --git a/src/lighthouse.php b/src/lighthouse.php index 129c0d27ba..b2293dddb5 100644 --- a/src/lighthouse.php +++ b/src/lighthouse.php @@ -259,7 +259,7 @@ 'storage' => env('LIGHTHOUSE_SUBSCRIPTION_STORAGE', 'redis'), /* - * Default subscription storage time to live. + * Default subscription storage time to live in seconds. * * Indicates how long a subscription can be active before it's automatically removed from storage. * Setting this to `null` means the subscriptions are stored forever. This may cause From 22527db6077ce655727359f1e742d34f94889993 Mon Sep 17 00:00:00 2001 From: spawnia Date: Sun, 19 Apr 2020 10:52:46 +0200 Subject: [PATCH 4/9] style --- src/Subscriptions/Job/BroadcastSubscriptionJob.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Subscriptions/Job/BroadcastSubscriptionJob.php b/src/Subscriptions/Job/BroadcastSubscriptionJob.php index 4ff9c1e774..2fe5b7ad8c 100644 --- a/src/Subscriptions/Job/BroadcastSubscriptionJob.php +++ b/src/Subscriptions/Job/BroadcastSubscriptionJob.php @@ -27,8 +27,6 @@ class BroadcastSubscriptionJob implements ShouldQueue /** * The root element to be passed when resolving the subscription. - * - * @var mixed */ public $root; From 908840e0f94952af9821a20492875fbb3559ad87 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sun, 19 Apr 2020 12:04:06 +0200 Subject: [PATCH 5/9] Add SerializesModels to subscription broadcast job --- src/Subscriptions/Job/BroadcastSubscriptionJob.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Subscriptions/Job/BroadcastSubscriptionJob.php b/src/Subscriptions/Job/BroadcastSubscriptionJob.php index 2fe5b7ad8c..583f25355a 100644 --- a/src/Subscriptions/Job/BroadcastSubscriptionJob.php +++ b/src/Subscriptions/Job/BroadcastSubscriptionJob.php @@ -4,12 +4,13 @@ use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Queue\SerializesModels; use Nuwave\Lighthouse\Schema\Types\GraphQLSubscription; use Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions; class BroadcastSubscriptionJob implements ShouldQueue { - use Queueable; + use Queueable, SerializesModels; /** * The subscription field that was requested. From 72f484fef3c8325e89d07257a809390bf7430f6b Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sun, 19 Apr 2020 12:04:17 +0200 Subject: [PATCH 6/9] Update changelog and upgrade docs --- CHANGELOG.md | 1 + UPGRADE.md | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78446edf6f..10e16c2306 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ You can find and compare releases at the [GitHub release page](https://github.co - Remove subscriber reference from topic when deleted https://github.com/nuwave/lighthouse/pull/1288 - Improve subscription context serializer https://github.com/nuwave/lighthouse/pull/1283 - Allow replacing the `SubscriptionRegistry` implementation using the container https://github.com/nuwave/lighthouse/pull/1286 +- Replace the subscription broadcast queued event handler with a queued job to allow the queue name to be specified https://github.com/nuwave/lighthouse/pull/1301 ## 4.11.0 diff --git a/UPGRADE.md b/UPGRADE.md index 2e31024c48..8a693b27af 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -116,3 +116,11 @@ The method will have to change like this: -public function purchasedItemsCount($root, array $args) +public function purchasedItemsCount(int $year, ?bool $includeReturns) ``` + +### Replace `Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent` with queue job + +The event is no longer fired. The queued listener for this event is replaced with a queued job. + +If you manually fired the event you should replace it by queing an instance of `Nuwave\Lighthouse\Subscriptions\Job\BroadcastSubscriptionJob` instead. + + From 27b20d1c3c73545d52ac8f233c4d8f4f1b9395b0 Mon Sep 17 00:00:00 2001 From: spawnia Date: Sun, 19 Apr 2020 12:18:13 +0200 Subject: [PATCH 7/9] Move job class into Subscription namespace, shorten description --- UPGRADE.md | 9 ++++----- src/Subscriptions/{Job => }/BroadcastSubscriptionJob.php | 2 +- .../Events/BroadcastSubscriptionListener.php | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) rename src/Subscriptions/{Job => }/BroadcastSubscriptionJob.php (96%) diff --git a/UPGRADE.md b/UPGRADE.md index 8a693b27af..426fb96e2b 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -92,7 +92,7 @@ configuration settings of your Laravel project. type Mutation { createUser( name: String! -- password: String! @bcrypt +- password: String! @bcrypt^ + password: String! @hash ): User! } @@ -119,8 +119,7 @@ The method will have to change like this: ### Replace `Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent` with queue job -The event is no longer fired. The queued listener for this event is replaced with a queued job. - -If you manually fired the event you should replace it by queing an instance of `Nuwave\Lighthouse\Subscriptions\Job\BroadcastSubscriptionJob` instead. - +The event is no longer fired, and the event class was removed. +The queued listener for this event is replaced with a queued job. +If you manually fired the event, replace it by queuing a `Nuwave\Lighthouse\Subscriptions\BroadcastSubscriptionJob`. diff --git a/src/Subscriptions/Job/BroadcastSubscriptionJob.php b/src/Subscriptions/BroadcastSubscriptionJob.php similarity index 96% rename from src/Subscriptions/Job/BroadcastSubscriptionJob.php rename to src/Subscriptions/BroadcastSubscriptionJob.php index 583f25355a..5958e49f99 100644 --- a/src/Subscriptions/Job/BroadcastSubscriptionJob.php +++ b/src/Subscriptions/BroadcastSubscriptionJob.php @@ -1,6 +1,6 @@ Date: Sun, 19 Apr 2020 12:29:44 +0200 Subject: [PATCH 8/9] Clarify things that might need to change when upgrading --- UPGRADE.md | 11 +++++---- src/Execution/Utils/Subscription.php | 24 ++++++++----------- .../BroadcastSubscriptionJob.php | 6 +---- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 426fb96e2b..ac9a639695 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -117,9 +117,12 @@ The method will have to change like this: +public function purchasedItemsCount(int $year, ?bool $includeReturns) ``` -### Replace `Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent` with queue job +### `Nuwave\Lighthouse\Subscriptions\Events\BroadcastSubscriptionEvent` is no longer fired -The event is no longer fired, and the event class was removed. -The queued listener for this event is replaced with a queued job. +The event is no longer fired, and the event class was removed. Lighthouse now uses a queued job instead. -If you manually fired the event, replace it by queuing a `Nuwave\Lighthouse\Subscriptions\BroadcastSubscriptionJob`. +If you manually fired the event, replace it by queuing a `Nuwave\Lighthouse\Subscriptions\BroadcastSubscriptionJob` +or a call to `Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions::queueBroadcast()`. + +In case you depend on an event being fired whenever a subscription is queued, you can bind your +own implementation of `Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions`. diff --git a/src/Execution/Utils/Subscription.php b/src/Execution/Utils/Subscription.php index feda5f2119..3173c7faac 100644 --- a/src/Execution/Utils/Subscription.php +++ b/src/Execution/Utils/Subscription.php @@ -14,7 +14,6 @@ class Subscription /** * Broadcast subscription to client(s). * - * * @throws \InvalidArgumentException */ public static function broadcast(string $subscriptionField, $root, ?bool $shouldQueue = null): void @@ -35,25 +34,22 @@ public static function broadcast(string $subscriptionField, $root, ?bool $should /** @var \Nuwave\Lighthouse\Subscriptions\Contracts\BroadcastsSubscriptions $broadcaster */ $broadcaster = app(BroadcastsSubscriptions::class); - $shouldQueue = $shouldQueue === null - ? config('lighthouse.subscriptions.queue_broadcasts', false) - : $shouldQueue; + // Default to the configuration setting if not specified + if ($shouldQueue === null) { + $shouldQueue = config('lighthouse.subscriptions.queue_broadcasts', false); + } - $method = $shouldQueue - ? 'queueBroadcast' - : 'broadcast'; + $subscription = $registry->subscription($subscriptionField); try { - call_user_func( - [$broadcaster, $method], - $registry->subscription($subscriptionField), - $subscriptionField, - $root - ); + if ($shouldQueue) { + $broadcaster->queueBroadcast($subscription, $subscriptionField, $root); + } else { + $broadcaster->broadcast($subscription, $subscriptionField, $root); + } } catch (Throwable $e) { /** @var \Nuwave\Lighthouse\Subscriptions\Contracts\SubscriptionExceptionHandler $exceptionHandler */ $exceptionHandler = app(SubscriptionExceptionHandler::class); - $exceptionHandler->handleBroadcastError($e); } } diff --git a/src/Subscriptions/BroadcastSubscriptionJob.php b/src/Subscriptions/BroadcastSubscriptionJob.php index 5958e49f99..eabf4776fa 100644 --- a/src/Subscriptions/BroadcastSubscriptionJob.php +++ b/src/Subscriptions/BroadcastSubscriptionJob.php @@ -40,10 +40,6 @@ public function __construct(GraphQLSubscription $subscription, string $fieldName public function handle(BroadcastsSubscriptions $broadcaster): void { - $broadcaster->broadcast( - $this->subscription, - $this->fieldName, - $this->root - ); + $broadcaster->broadcast($this->subscription, $this->fieldName, $this->root); } } From 4bd32e4f035a83713626304abd8e2c28a7356566 Mon Sep 17 00:00:00 2001 From: spawnia Date: Sun, 19 Apr 2020 12:31:15 +0200 Subject: [PATCH 9/9] ^ --- UPGRADE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPGRADE.md b/UPGRADE.md index ac9a639695..26857c5ff9 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -92,7 +92,7 @@ configuration settings of your Laravel project. type Mutation { createUser( name: String! -- password: String! @bcrypt^ +- password: String! @bcrypt + password: String! @hash ): User! }