Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Queued subscriptions to others #2600

Open
MissaelAnda opened this issue Aug 16, 2024 · 1 comment
Open

Queued subscriptions to others #2600

MissaelAnda opened this issue Aug 16, 2024 · 1 comment
Labels
discussion Requires input from multiple people

Comments

@MissaelAnda
Copy link
Contributor

What problem does this feature proposal attempt to solve?
When broadcasting subscriptions you can filter out the user that triggered the broadcast with:

    public function filter(Subscriber $subscriber, mixed $root): bool
    {
        return $subscriber->socket_id !== request()->header('X-Socket-ID');
    }

The problem comes when the broadcast is queued instead of dispatching it synchronously, when queued request() is always empty and the filter will be ignorred.

Which possible solutions should be considered?

One solution could be passing the request to the BroadcastSubscriptionJob and setting it on execution:

use Illuminate\Support\Facades\App;
use Illuminate\Support\Facades\Facade;

class BroadcastSubscriptionJob implements ShouldQueue
{
    use Queueable;
    use SerializesModels;

    public function __construct(
        /**
         * The subscription field that was requested.
         */
        public GraphQLSubscription $subscription,
        /**
         * The name of the field.
         */
        public string $fieldName,
        /**
         * The root element to be passed when resolving the subscription.
         */
        public mixed $root,
        /**
         * The request context when the broadcast was triggered
         */
        public Request $request,
    ) {}

    public function handle(BroadcastsSubscriptions $broadcaster): void
    {
        App::instance('request', $this->request);
        Facade::clearResolvedInstance('request');

        $broadcaster->broadcast($this->subscription, $this->fieldName, $this->root);
    }
}

Other solution could be adding a "native" toOthers functionality to the broadcaster:

    public function queueBroadcast(GraphQLSubscription $subscription, string $fieldName, mixed $root, bool $toOthers = false): void
    {
        $broadcastSubscriptionJob = new BroadcastSubscriptionJob($subscription, $fieldName, $root, $toOthers);
        $broadcastSubscriptionJob->onQueue(config('lighthouse.subscriptions.broadcasts_queue_name'));

        $this->busDispatcher->dispatch($broadcastSubscriptionJob);
    }

    public function broadcast(GraphQLSubscription $subscription, string $fieldName, mixed $root, bool $toOthers = false): void
    {
        $topic = $subscription->decodeTopic($fieldName, $root);

        $subscribers = $this->subscriptionStorage
            ->subscribersByTopic($topic)
            ->when($toOthers && Broadcast::socket())
            ->filter(static fn (Subscriber $subscriber): bool => $subscriber->socket_id !== Broadcast::socket())
            ->filter(static fn (Subscriber $subscriber): bool => $subscription->filter($subscriber, $root));

        $this->subscriptionIterator->process(
            $subscribers,
            function (Subscriber $subscriber) use ($root): void {
                $subscriber->root = $root;

                $result = $this->graphQL->executeParsedQuery(
                    $subscriber->query,
                    $subscriber->context,
                    $subscriber->variables,
                    $subscriber,
                );
                $this->broadcastManager->broadcast($subscriber, $result);
            },
        );
    }

The problem with this solution is that it could break compatibility with the BroadcastsSubscriptions interface.

@stayallive
Copy link
Collaborator

What about $subscriber->context->request()? I have not tested it but the request is stored when the subscription is created so it should be there.

@stayallive stayallive added the discussion Requires input from multiple people label Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Requires input from multiple people
Projects
None yet
Development

No branches or pull requests

2 participants