vaadin / collaboration-engine

The simplest way to build real-time collaboration into web apps
https://vaadin.com/collaboration
Other
3 stars 1 forks source link

Add API to handle failed topic connection #14

Closed pekam closed 3 years ago

pekam commented 3 years ago

To fix issue #13, there needs to be a way for openTopicConnection to inform the caller that the connection failed. This can happen when the end user quota of the license has been exceeded. The same solution could be used also for actual connection problems when running CE as a standalone server.

Currently openTopicConnection returns a Registration for closing the connection. In case of a refused connection, it returns a NO-OP callback. One option would be to return null in this case, but it would not be very obvious, and could easily lead to NPEs.

Another option would be to add a new overload of openTopicConnection with a callback to handle the potential failed connection:

public Registration openTopicConnection(ConnectionContext context,
        String topicId, UserInfo localUser,
        SerializableFunction<TopicConnection, Registration> connectionActivationCallback,
        Command connectionFailedHandler)

The downside is that the parameter list is already very long, which is a code smell.

A third option, and the one we've decided to implement, is to make openTopicConnection return an extended version of Registration:

public interface TopicConnectionRegistration extends Registration {
    void onConnectionFailed(ConnectionFailedAction connectionFailedAction);
}

@FunctionalInterface
public interface ConnectionFailedAction {
    void onConnectionFailed(ConnectionFailedEvent event);
}

public class ConnectionFailedEvent extends EventObject {
    public ConnectionFailedEvent(TopicConnectionRegistration source) {
        super(source);
    }
}

Right now the event doesn't have any useful information, but later when the failure can happen also because of network issues, we can add information about the cause of the failure to the event.

This would be similar to Flow's DomListenerRegistration, and could be used in CollaborationBinder's case like:

topicRegistration = CollaborationEngine.getInstance()
        .openTopicConnection(connectionContext, topicId, localUser,
                topic -> bindToTopic(topic, initialBeanSupplier));
topicRegistration.onConnectionFailed(e -> {
    // Populate the form with initialBeanSupplier
});

If the connection has already failed when adding the action, it should be called immediately.

The action should be invoked through ConnectionContext::dispatchAction.

UPDATE 27 Nov 2020: We don't actually need this API to fix CollaborationBinder, as we have CollaborationEngine::requestAccess to check if a topic connection can be established or not. This feature might still become useful once CE is running as a standalone server. In that case, it might be beneficial to have a single roundtrip that opens the connection if possible and returns the status.

Legioth commented 3 years ago

Might make sense to pass an event object to the connection failed callback so that we can e.g. include information about the cause (e.g. lack of quota vs network connectivity issue).

pekam commented 3 years ago

Might make sense to pass an event object to the connection failed callback so that we can e.g. include information about the cause (e.g. lack of quota vs network connectivity issue).

Issue description has been updated to include the event object.