From d492397a70633fea108420a638b04627297283f5 Mon Sep 17 00:00:00 2001 From: Tobias Ortmayr Date: Thu, 27 Aug 2020 13:26:42 +0200 Subject: [PATCH] #96 Introduce InitializeClientSessionAction Also: - Use new clientProxy provider instead of deriving the proxy from the client id. - Fix/Surpress a couple of warnings - Sort members of MultiBindingDefaults Follow up for eclipse-glsp/glsp/issues/96 --- .../org/eclipse/glsp/api/action/Action.java | 6 +-- ...n.java => DisposeClientSessionAction.java} | 16 ++++-- .../kind/InitializeClientSessionAction.java | 36 +++++++++++++ .../action/DefaultActionDispatcher.java | 21 ++++++-- .../actionhandler/ClientActionHandler.java | 13 ++--- ...=> DisposeClientSessionActionHandler.java} | 6 +-- .../InitializeClientSessionActionHandler.java | 51 +++++++++++++++++++ .../glsp/server/di/DefaultGLSPModule.java | 1 + .../glsp/server/di/MultiBindingDefaults.java | 43 ++++++++-------- .../server/jsonrpc/DefaultGLSPServer.java | 17 ------- 10 files changed, 152 insertions(+), 58 deletions(-) rename plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/{DisposeClientAction.java => DisposeClientSessionAction.java} (68%) create mode 100644 plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/InitializeClientSessionAction.java rename plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/{DisposeClientActionHandler.java => DisposeClientSessionActionHandler.java} (81%) create mode 100644 plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/InitializeClientSessionActionHandler.java diff --git a/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/Action.java b/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/Action.java index 9829b0b9..e6d398ec 100644 --- a/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/Action.java +++ b/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/Action.java @@ -39,11 +39,12 @@ public static class Kind { public static final String CLEAR_MARKERS = "clearMarkers"; public static final String COMPUTED_BOUNDS = "computedBounds"; public static final String CUT_ACTION = "cut"; - public static final String DISPOSE_CLIENT = "disposeClient"; + public static final String DISPOSE_CLIENT_SESSION = "disposeClientSession"; public static final String EXECUTE_OPERATION = "executeOperation"; public static final String EXECUTE_SERVER_COMMAND = "executeServerCommand"; public static final String EXPORT_SVG = "exportSvg"; public static final String FIT_TO_SCREEN = "fit"; + public static final String INITIALIZE_CLIENT_SESSION = "initializeClientSession"; public static final String NAVIGATE_TO_TARGET = "navigateToTarget"; public static final String REDO = "glspRedo"; public static final String REQUEST_BOUNDS = "requestBounds"; @@ -68,7 +69,7 @@ public static class Kind { public static final String SET_DIRTY_STATE = "setDirtyState"; public static final String SET_EDIT_MODE = "setEditMode"; public static final String SET_EDIT_VALIDATION_RESULT = "setEditValidationResult"; - public static final String SET_LABEL_EDIT_VALIDATION_RESULT_ACTION = "setLabelEditValidationResult"; + public static final String SET_LABEL_EDIT_VALIDATION_RESULT = "setLabelEditValidationResult"; public static final String SET_LAYERS = "setLayers"; public static final String SET_MARKERS = "setMarkers"; public static final String SET_MODEL = "setModel"; @@ -82,5 +83,4 @@ public static class Kind { public static final String UPDATE_MODEL = "updateModel"; public static final String VALIDATE_LABEL_EDIT_ACTION = "validateLabelEdit"; } - } diff --git a/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/DisposeClientAction.java b/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/DisposeClientSessionAction.java similarity index 68% rename from plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/DisposeClientAction.java rename to plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/DisposeClientSessionAction.java index a079b4a2..3c3ecbdf 100644 --- a/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/DisposeClientAction.java +++ b/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/DisposeClientSessionAction.java @@ -17,10 +17,20 @@ import org.eclipse.glsp.api.action.Action; -public class DisposeClientAction extends Action { +public class DisposeClientSessionAction extends Action { - public DisposeClientAction() { - super(Action.Kind.DISPOSE_CLIENT); + private String clientId; + + public DisposeClientSessionAction() { + super(Action.Kind.DISPOSE_CLIENT_SESSION); + } + + public DisposeClientSessionAction(final String clientId) { + this(); + this.clientId = clientId; } + public String getClientId() { return clientId; } + + public void setClientId(final String clientId) { this.clientId = clientId; } } diff --git a/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/InitializeClientSessionAction.java b/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/InitializeClientSessionAction.java new file mode 100644 index 00000000..54c58667 --- /dev/null +++ b/plugins/org.eclipse.glsp.api/src/org/eclipse/glsp/api/action/kind/InitializeClientSessionAction.java @@ -0,0 +1,36 @@ +/******************************************************************************** + * Copyright (c) 2020 EclipseSource and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ +package org.eclipse.glsp.api.action.kind; + +import org.eclipse.glsp.api.action.Action; + +public class InitializeClientSessionAction extends Action { + + private String clientId; + + public InitializeClientSessionAction() { + super(Action.Kind.INITIALIZE_CLIENT_SESSION); + } + + public InitializeClientSessionAction(final String clientId) { + this(); + this.clientId = clientId; + } + + public String getClientId() { return clientId; } + + public void setClientId(final String clientId) { this.clientId = clientId; } +} diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/action/DefaultActionDispatcher.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/action/DefaultActionDispatcher.java index d1891816..c784404d 100644 --- a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/action/DefaultActionDispatcher.java +++ b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/action/DefaultActionDispatcher.java @@ -19,6 +19,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CompletableFuture; @@ -30,6 +31,7 @@ import org.eclipse.glsp.api.action.Action; import org.eclipse.glsp.api.action.ActionDispatcher; import org.eclipse.glsp.api.action.ActionMessage; +import org.eclipse.glsp.api.action.kind.InitializeClientSessionAction; import org.eclipse.glsp.api.action.kind.ResponseAction; import org.eclipse.glsp.api.handler.ActionHandler; import org.eclipse.glsp.api.model.GraphicalModelState; @@ -37,6 +39,7 @@ import org.eclipse.glsp.api.protocol.ClientSessionListener; import org.eclipse.glsp.api.protocol.ClientSessionManager; import org.eclipse.glsp.api.protocol.GLSPClient; +import org.eclipse.glsp.api.protocol.GLSPServerException; import org.eclipse.glsp.api.registry.ActionHandlerRegistry; import com.google.inject.Inject; @@ -162,6 +165,7 @@ private void handleNextMessage() } } + @SuppressWarnings("checkstyle:IllegalCatch") protected void handleMessage(final ActionMessage message) { checkThread(); final Action action = message.getAction(); @@ -184,11 +188,22 @@ protected void runAction(final Action action, final String clientId) { if (actionHandlers.isEmpty()) { throw new IllegalArgumentException("No handler registered for action: " + action); } - final GraphicalModelState modelState = modelStateProvider.getModelState(clientId) - .orElseGet(() -> modelStateProvider.create(clientId)); + + Optional modelState = modelStateProvider.getModelState(clientId); + + if (!modelState.isPresent()) { + if (action instanceof InitializeClientSessionAction) { + modelState = Optional.of(modelStateProvider.create(clientId)); + } else { + String errorMsg = String.format( + "The session for client '%s' has not been initialized yet. Could not process action: %s", clientId, + action); + throw new GLSPServerException(errorMsg); + } + } for (final ActionHandler actionHandler : actionHandlers) { - final List responses = actionHandler.execute(action, modelState).stream() + final List responses = actionHandler.execute(action, modelState.get()).stream() .map(response -> ResponseAction.respond(action, response)) .collect(Collectors.toList()); dispatchAll(clientId, responses); diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/ClientActionHandler.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/ClientActionHandler.java index c25b68d5..f471ffdd 100644 --- a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/ClientActionHandler.java +++ b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/ClientActionHandler.java @@ -24,18 +24,17 @@ import org.eclipse.glsp.api.action.ActionMessage; import org.eclipse.glsp.api.handler.ActionHandler; import org.eclipse.glsp.api.model.GraphicalModelState; -import org.eclipse.glsp.api.protocol.ClientSessionManager; import org.eclipse.glsp.api.protocol.GLSPClient; -import org.eclipse.glsp.api.protocol.GLSPServerException; import com.google.inject.Inject; +import com.google.inject.Provider; import com.google.inject.name.Named; public class ClientActionHandler implements ActionHandler { public static final String CLIENT_ACTIONS = "ClientActionHandler"; - @Inject() - protected ClientSessionManager sessionManager; + @Inject + protected Provider client; private final List> handledActionTypes; @@ -54,11 +53,7 @@ public List execute(final Action action, final GraphicalModelState model } protected void send(final String clientId, final Action action) { - GLSPClient client = GLSPServerException.getOrThrow(sessionManager.resolve(clientId), - "Unable to send a message to Client ID:" + clientId - + ". This ID does not match any known client (Client disconnected or not initialized yet?)"); - ActionMessage message = new ActionMessage(clientId, action); - client.process(message); + client.get().process(message); } } diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/DisposeClientActionHandler.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/DisposeClientSessionActionHandler.java similarity index 81% rename from plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/DisposeClientActionHandler.java rename to plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/DisposeClientSessionActionHandler.java index 2daac645..cfcc1756 100644 --- a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/DisposeClientActionHandler.java +++ b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/DisposeClientSessionActionHandler.java @@ -18,19 +18,19 @@ import java.util.List; import org.eclipse.glsp.api.action.Action; -import org.eclipse.glsp.api.action.kind.DisposeClientAction; +import org.eclipse.glsp.api.action.kind.DisposeClientSessionAction; import org.eclipse.glsp.api.model.GraphicalModelState; import org.eclipse.glsp.api.protocol.ClientSessionManager; import com.google.inject.Inject; -public class DisposeClientActionHandler extends BasicActionHandler { +public class DisposeClientSessionActionHandler extends BasicActionHandler { @Inject() protected ClientSessionManager sessionManager; @Override - protected List executeAction(final DisposeClientAction action, final GraphicalModelState modelState) { + protected List executeAction(final DisposeClientSessionAction action, final GraphicalModelState modelState) { sessionManager.disposeClientSession(modelState.getClientId()); return none(); } diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/InitializeClientSessionActionHandler.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/InitializeClientSessionActionHandler.java new file mode 100644 index 00000000..adbad2f8 --- /dev/null +++ b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/actionhandler/InitializeClientSessionActionHandler.java @@ -0,0 +1,51 @@ +/******************************************************************************** + * Copyright (c) 2020 EclipseSource and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ +package org.eclipse.glsp.server.actionhandler; + +import java.util.List; + +import org.eclipse.glsp.api.action.Action; +import org.eclipse.glsp.api.action.kind.InitializeClientSessionAction; +import org.eclipse.glsp.api.model.GraphicalModelState; +import org.eclipse.glsp.api.protocol.ClientSessionManager; +import org.eclipse.glsp.api.protocol.GLSPClient; +import org.eclipse.glsp.api.protocol.GLSPServerException; + +import com.google.inject.Inject; +import com.google.inject.Provider; + +public class InitializeClientSessionActionHandler extends BasicActionHandler { + + @Inject + protected ClientSessionManager clientSessionManager; + + @Inject + protected Provider client; + + @Override + protected List executeAction(final InitializeClientSessionAction action, + final GraphicalModelState modelState) { + if (clientSessionManager.createClientSession(client.get(), action.getClientId())) { + modelState.setClientId(action.getClientId()); + } else { + throw new GLSPServerException(String.format( + "Could not create session for client id '%s'. Another session with the same id already exists", + action.getClientId())); + } + return none(); + } + +} diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/DefaultGLSPModule.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/DefaultGLSPModule.java index 65962a0d..ee207722 100644 --- a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/DefaultGLSPModule.java +++ b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/DefaultGLSPModule.java @@ -104,6 +104,7 @@ protected void configureContextEditValidators(final MultiBindConfig config) {} + @SuppressWarnings("rawtypes") @Override protected Class bindGLSPServer() { return DefaultGLSPServer.class; diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/MultiBindingDefaults.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/MultiBindingDefaults.java index 04af891b..8419f1a6 100644 --- a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/MultiBindingDefaults.java +++ b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/MultiBindingDefaults.java @@ -48,8 +48,9 @@ import org.eclipse.glsp.api.handler.OperationHandler; import org.eclipse.glsp.server.actionhandler.ClientActionHandler; import org.eclipse.glsp.server.actionhandler.ComputedBoundsActionHandler; -import org.eclipse.glsp.server.actionhandler.DisposeClientActionHandler; +import org.eclipse.glsp.server.actionhandler.DisposeClientSessionActionHandler; import org.eclipse.glsp.server.actionhandler.ExecuteServerCommandActionHandler; +import org.eclipse.glsp.server.actionhandler.InitializeClientSessionActionHandler; import org.eclipse.glsp.server.actionhandler.OperationActionHandler; import org.eclipse.glsp.server.actionhandler.RequestClipboardDataActionHandler; import org.eclipse.glsp.server.actionhandler.RequestContextActionsHandler; @@ -76,8 +77,29 @@ import com.google.common.collect.Lists; public final class MultiBindingDefaults { + private MultiBindingDefaults() {} + public static final List> DEFAULT_ACTION_HANDLERS = Lists.newArrayList( + ClientActionHandler.class, + ComputedBoundsActionHandler.class, + DisposeClientSessionActionHandler.class, + InitializeClientSessionActionHandler.class, + OperationActionHandler.class, + RequestModelActionHandler.class, + RequestPopupModelActionHandler.class, + SaveModelActionHandler.class, + UndoRedoActionHandler.class, + ExecuteServerCommandActionHandler.class, + ResolveNavigationTargetActionHandler.class, + RequestClipboardDataActionHandler.class, + RequestNavigationTargetsActionHandler.class, + RequestTypeHintsActionHandler.class, + RequestContextActionsHandler.class, + RequestEditValidationHandler.class, + RequestMarkersHandler.class, + SetEditModeActionHandler.class); + public static final List> DEFAULT_CLIENT_ACTIONS = Lists.newArrayList( CenterAction.class, ClearMarkersAction.class, @@ -106,25 +128,6 @@ private MultiBindingDefaults() {} TriggerEdgeCreationAction.class, UpdateModelAction.class); - public static final List> DEFAULT_ACTION_HANDLERS = Lists.newArrayList( - ClientActionHandler.class, - ComputedBoundsActionHandler.class, - DisposeClientActionHandler.class, - OperationActionHandler.class, - RequestModelActionHandler.class, - RequestPopupModelActionHandler.class, - SaveModelActionHandler.class, - UndoRedoActionHandler.class, - ExecuteServerCommandActionHandler.class, - ResolveNavigationTargetActionHandler.class, - RequestClipboardDataActionHandler.class, - RequestNavigationTargetsActionHandler.class, - RequestTypeHintsActionHandler.class, - RequestContextActionsHandler.class, - RequestEditValidationHandler.class, - RequestMarkersHandler.class, - SetEditModeActionHandler.class); - public static final List> DEFAULT_OPERATION_HANDLERS = Lists.newArrayList( ApplyLabelEditOperationHandler.class, ChangeBoundsOperationHandler.class, diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/jsonrpc/DefaultGLSPServer.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/jsonrpc/DefaultGLSPServer.java index ce27f2c2..79c75e8e 100644 --- a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/jsonrpc/DefaultGLSPServer.java +++ b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/jsonrpc/DefaultGLSPServer.java @@ -23,7 +23,6 @@ import org.apache.log4j.Logger; import org.eclipse.glsp.api.action.ActionDispatcher; import org.eclipse.glsp.api.action.ActionMessage; -import org.eclipse.glsp.api.action.kind.RequestModelAction; import org.eclipse.glsp.api.jsonrpc.GLSPJsonrpcClient; import org.eclipse.glsp.api.jsonrpc.GLSPJsonrpcServer; import org.eclipse.glsp.api.model.ModelStateProvider; @@ -107,22 +106,6 @@ public void process(final ActionMessage message) { return null; }; try { - if (message.getAction() instanceof RequestModelAction) { - if (!this.sessionManager.createClientSession(clientProxy, clientId)) { - // FIXME: We currently initialize the session upon RequestModelAction. However, - // clients shouldn't be forbidden to send multiple RequestModelActions (even for the same - // diagram widget). We should use a specific "initializeClientSession" action, instead - // of relying on the RequestModelAction. - // For now, just log a warning, to avoid breaking existing clients. - log.warn( - "Received a RequestModelAction with a clientId that already exists. This may lead to unexpected results. ClientID: " - + clientId); - // throw new GLSPServerException(String.format( - // "Could not create session for client id '%s'. Another session with the same id already exists", - // clientId)); - } - } - if (!initialized) { throw new GLSPServerException( String.format("Could not process action message '%s'. The server has not been initalized yet", message));