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

[8.x] Enable only-to-others functionality when using Ably broadcast driver #40234

Merged
merged 1 commit into from Jan 3, 2022

Conversation

leeovery
Copy link
Contributor

@leeovery leeovery commented Jan 3, 2022

TL;DR

This PR enables the only to others broadcast functionality when using the Ably broadcast driver and the AblyJs frontend client.

Issue

When using the Ably broadcast driver the only to others functionality provided by Laravel does not work when using the AblyJs client on the frontend.

This is because Laravel includes the socketId in the payload array but Ably requires it to be a top-level property called connectionKey.

(source: https://faqs.ably.com/is-it-possible-to-prevent-messages-published-being-echoed-back-to-the-publishing-client)

Fix

This PR refactors the Ably broadcaster to pass a fully-formed Ably Message object to the Ably publish method, where we can also include the socketId as the connectionKey.

Currently, we pass a string event and an array payload to the publish method, and the Ably Rest client builds the Message object for us, but there is no way to include the connectionKey as a top-level property. (source: https://github.com/ably/ably-php/blob/main/src/Channel.php#L111)

To ensure backwards compatibility, the socket property will remain in the payload array too. It's possible people are working around this missing feature by using that property on the frontend.

With this PR merged, to enable the only to others functionality with Ably, you then only need to set the broadcast event to include the socket ID as per the Laravel docs, and then set echoMessages=false on the frontend when using the AblyJs client. This will prevent echoing messages back to originator.

Thanks
Lee

…able dontBroadcastToCurrentUser() functionality in Laravel. Use echoMessages=false on frontend when using ablyjs to avoid echoing messages back to originator.
@leeovery leeovery changed the title [8.*] Enable dontBroadcastToCurrentUser functionality when using Ably broadcast driver. [8.*] Enable only-to-others functionality when using Ably broadcast driver. Jan 3, 2022
@leeovery
Copy link
Contributor Author

leeovery commented Jan 3, 2022

To add, you also need to configure the axios interceptor. Echo isnt currently fully compatible with Ably. But thats a frontend issue and easily added:

eg:

const broadcaster = new Ably.Realtime({
  ...options,
  echoMessages: false,
});

let interceptorId;
broadcaster.connection.on('connected', () => {
 interceptorId = $axios.interceptors.request.use((config) => {
    const socketId = broadcaster.connection.key;
    if (socketId) {
      config.headers['X-Socket-Id'] = socketId;
    }

    return config;
  });
});
broadcaster.connection.on('suspended', () => {
  if (interceptorId) {
    $axios.interceptors.request.eject(interceptorId);
  }
});

@driesvints driesvints changed the title [8.*] Enable only-to-others functionality when using Ably broadcast driver. [8.x] Enable only-to-others functionality when using Ably broadcast driver Jan 3, 2022
@taylorotwell
Copy link
Member

Don't we currently document using Ably via the Pusher JS client and Pusher protocol support enabled on the Ably control panel?

@leeovery
Copy link
Contributor Author

leeovery commented Jan 3, 2022

Yea we do. But by using that you miss out on some Ably features in seems. They suggest not doing this too in their docs (obviously lol).

Point 3 under the General header: https://faqs.ably.com/using-the-ably-pusher-protocol-adapter

This PR wont break that functionality though as its purely additive. Just enables the feature for those that choose to go the Ably way in order to get the extra features and security.

@taylorotwell taylorotwell merged commit e15d438 into laravel:8.x Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants