ChannelAuthorizer supports only synchronous flow

[barsumek]

What is the issue?

Currently ChannelAuthorizer expects a synchronous authorize implementation that returns a string.

It would be nice to rewrite it in a more asynchronous way, so that ChannelAuthorizer interface wouldn't force the implementation to block the thread if authorize requires async job.

It limits Kotlin usage e.g. in your React Native SDK, that needs async job by design (of React Native bridge):
Native Java/Kotlin module sends an event to Javascript and later Javascript calls Native module with auth object.

Related issue in your React Native SDK: pusher/pusher-websocket-react-native#37

Is it a crash report? Submit stack traces or anything that you think would help

Nope, but please check the linked issues for more context and possible freezes/errors.

Any improvements you suggest

The React Native SDK (Android/Kotlin code) uses mutex to block the thread and wait until JavaScript returns proper auth object through RN bridge.
It's not really a common flow in RN world (to block the thread in RN module), because it can lead to various issues/freezes/crashes/leaks.
Instead, it is preferred to implement logic in an asynchronous way (listeners/storing callbacks) since RN bridge is also asynchronous.

There was a similar but more serious issue in iOS part of your React Native SDK:
pusher/pusher-websocket-react-native#34
But it can be fixed by simply storing callbacks instead of blocking the thread:
pusher/pusher-websocket-react-native#36
However, it is not possible to implement it like this in Android (Kotlin) part, because of the ChannelAuthorizer interface.

So my suggestion would be to rewrite ChannelAuthorizer in a way that supports async/delayed response.
For example (there might be a better way, but this should do the job):

// old
String authorize(String channelName, String socketId)
// new
void authorize(String channelName, String socketId, Callable<String> authResultCallback)

I know it would be a breaking change, but it should be possible to add backwards compatible util (that simply calls authResultCallback instead of return) or support both ways.

---

CC @pusher/mobile

[benjamin-tang-pusher]

Hi, thanks for describing a possible solution. We will take a look and see if it is a feature we can prioritise.

[JimmyTai]

I met this issue too recently. I am trying to implement batch authentication in java.
After dig into it, there are 2 possible solution.

1. Use callback function to provide authentication data, like @barsumek's suggestion. pusher-websocket-swift use completionHandler too.
2. Let ChannelManager.toSubscribeMessage() run in a new thread.

For example

in Factory.java, use Executors.newCachedThreadPool to create multiple thread.

    public synchronized  void queueOnAuthenticationThread(final Runnable r) {
        if (authenticationQueue == null) {
            authenticationQueue = Executors.newCachedThreadPool(new DaemonThreadFactory("authenticationQueue"));
        }
        authenticationQueue.execute(r);
    }

in ChannelManager.java

    private void sendOrQueueSubscribeMessage(final InternalChannel channel) {
        factory.queueOnAuthenticationThread(() -> {
            if (connection.getState() == ConnectionState.CONNECTED) {
                try {
                    final String message = channel.toSubscribeMessage();
                    factory.queueOnEventThread(() -> {
                        connection.sendMessage(message);
                        channel.updateState(ChannelState.SUBSCRIBE_SENT);
                    });
                } catch (final AuthorizationFailureException e) {
                    handleAuthenticationFailure(channel, e);
                }
            }
        });
    }

But this solution seems break the order of subscribe/unsubscribe.
Please feel free to give a feedback.

Hope we could have a conclusion and then I will prepare a PR for this issue.

[JimmyTai]

Maybe per channel object lock could avoid subscribe/unsubscribe order issue.

cc @benjamin-tang-pusher