eclipse-glsp#122 eclipse-glsp#123 Centralize configuration in the ser…

…ver (eclipse-glsp#95) - Merge ServerConfiguration and DiagramConfiguration - Centralize model update protocol in Model Submission Handler fixes eclipse-glsp/glsp/issues/122 fixes eclipse-glsp/glsp/issues/123 Signed-off-by: Camille Letavernier <[email protected]> Co-authored-by: Camille Letavernier <[email protected]>
MatthiasHofstaetter · Dec 16, 2020 · 17b959b · 17b959b
1 parent 2ee2d54
commit 17b959b
Show file tree

Hide file tree

Showing 14 changed files with 125 additions and 96 deletions.
diff --git a/....example.workflow/src/org/eclipse/glsp/example/workflow/WorkflowDiagramConfiguration.java b/....example.workflow/src/org/eclipse/glsp/example/workflow/WorkflowDiagramConfiguration.java
@@ -43,6 +43,7 @@
 import org.eclipse.glsp.server.diagram.DiagramConfiguration;
 import org.eclipse.glsp.server.diagram.EdgeTypeHint;
 import org.eclipse.glsp.server.diagram.ShapeTypeHint;
+import org.eclipse.glsp.server.layout.ServerLayoutKind;
 
 public class WorkflowDiagramConfiguration implements DiagramConfiguration {
 
@@ -96,4 +97,12 @@ public EdgeTypeHint createDefaultEdgeTypeHint(final String elementId) {
       return hint;
    }
 
+   @Override
+   public ServerLayoutKind getLayoutKind() { return ServerLayoutKind.MANUAL; }
+
+   @Override
+   public boolean needsClientLayout() {
+      return true;
+   }
+
 }
diff --git a/...lipse.glsp.example.workflow/src/org/eclipse/glsp/example/workflow/WorkflowGLSPModule.java b/...lipse.glsp.example.workflow/src/org/eclipse/glsp/example/workflow/WorkflowGLSPModule.java
@@ -57,7 +57,6 @@
 import org.eclipse.glsp.server.features.popup.PopupModelFactory;
 import org.eclipse.glsp.server.features.validation.ModelValidator;
 import org.eclipse.glsp.server.layout.ILayoutEngine;
-import org.eclipse.glsp.server.layout.ServerLayoutConfiguration;
 import org.eclipse.glsp.server.operations.OperationHandler;
 import org.eclipse.glsp.server.protocol.GLSPServer;
 import org.eclipse.glsp.server.utils.MultiBinding;
@@ -70,11 +69,6 @@ protected Class<? extends GLSPServer> bindGLSPServer() {
       return WorkflowGLSPServer.class;
    }
 
-   @Override
-   protected Class<? extends ServerLayoutConfiguration> bindServerLayoutConfiguration() {
-      return WorkflowServerLayoutConfiguration.class;
-   }
-
    @Override
    protected void configureContextActionsProviders(final MultiBinding<ContextActionsProvider> binding) {
       super.configureContextActionsProviders(binding);

diff --git a/...ple.workflow/src/org/eclipse/glsp/example/workflow/WorkflowServerLayoutConfiguration.java b/...ple.workflow/src/org/eclipse/glsp/example/workflow/WorkflowServerLayoutConfiguration.java
diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/GLSPModule.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/di/GLSPModule.java
@@ -36,7 +36,6 @@
 import org.eclipse.glsp.server.features.validation.ModelValidator;
 import org.eclipse.glsp.server.jsonrpc.GraphGsonConfiguratorFactory;
 import org.eclipse.glsp.server.layout.ILayoutEngine;
-import org.eclipse.glsp.server.layout.ServerLayoutConfiguration;
 import org.eclipse.glsp.server.model.ModelStateProvider;
 import org.eclipse.glsp.server.operations.OperationHandlerRegistry;
 import org.eclipse.glsp.server.protocol.ClientSessionManager;
@@ -61,7 +60,6 @@ protected void configure() {
       bind(LabelEditValidator.class).to(bindLabelEditValidator());
       bind(ModelStateProvider.class).to(bindModelStateProvider());
       bind(GraphGsonConfiguratorFactory.class).to(bindGraphGsonConfiguratorFactory());
-      bind(ServerLayoutConfiguration.class).to(bindServerLayoutConfiguration()).in(Singleton.class);
       bind(ToolPaletteItemProvider.class).to(bindToolPaletteItemProvider());
       bind(CommandPaletteActionProvider.class).to(bindCommandPaletteActionProvider());
       bind(ContextMenuItemProvider.class).to(bindContextMenuItemProvider());
@@ -112,10 +110,6 @@ protected Class<? extends LabelEditValidator> bindLabelEditValidator() {
       return LabelEditValidator.NullImpl.class;
    }
 
-   protected Class<? extends ServerLayoutConfiguration> bindServerLayoutConfiguration() {
-      return ServerLayoutConfiguration.NullImpl.class;
-   }
-
    protected Class<? extends CommandPaletteActionProvider> bindCommandPaletteActionProvider() {
       return CommandPaletteActionProvider.NullImpl.class;
    }

diff --git a/...ins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/diagram/DiagramConfiguration.java b/...ins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/diagram/DiagramConfiguration.java
@@ -21,6 +21,7 @@
 
 import org.eclipse.emf.ecore.EClass;
 import org.eclipse.emf.ecore.EPackage;
+import org.eclipse.glsp.server.layout.ServerLayoutKind;
 
 public interface DiagramConfiguration {
 
@@ -41,4 +42,10 @@ default EdgeTypeHint createDefaultEdgeTypeHint(final String elementId) {
    default ShapeTypeHint createDefaultNodeTypeHint(final String elementId) {
       return new ShapeTypeHint(elementId, true, true, true, false);
    }
+
+   default ServerLayoutKind getLayoutKind() { return ServerLayoutKind.NONE; }
+
+   default boolean needsClientLayout() {
+      return true;
+   }
 }
diff --git a/...eclipse.glsp.server/src/org/eclipse/glsp/server/diagram/DiagramConfigurationRegistry.java b/...eclipse.glsp.server/src/org/eclipse/glsp/server/diagram/DiagramConfigurationRegistry.java
@@ -15,10 +15,15 @@
  ********************************************************************************/
 package org.eclipse.glsp.server.diagram;
 
+import static org.eclipse.glsp.server.protocol.GLSPServerException.getOrThrow;
+
 import java.util.HashMap;
 import java.util.Map;
 
 import org.eclipse.emf.ecore.EClass;
+import org.eclipse.glsp.server.model.GModelState;
+import org.eclipse.glsp.server.protocol.GLSPServerException;
+import org.eclipse.glsp.server.utils.ClientOptions;
 import org.eclipse.glsp.server.utils.Registry;
 
 public interface DiagramConfigurationRegistry extends Registry<String, DiagramConfiguration> {
@@ -28,4 +33,22 @@ default Map<String, EClass> getCollectiveTypeMappings() {
       getAll().stream().map(DiagramConfiguration::getTypeMappings).forEach(collectiveTypeMappings::putAll);
       return collectiveTypeMappings;
    }
+
+   /**
+    * Return the {@link DiagramConfiguration} from this registry, corresponding to the specified
+    * modelState. The modelState is expected to have the {@link ClientOptions#DIAGRAM_TYPE} option,
+    * and this DiagramType should exist in this registry. Otherwise, a {@link GLSPServerException}
+    * will be thrown.
+    *
+    * @param modelState
+    *                      the {@link GModelState}
+    * @return
+    *         the {@link DiagramConfiguration} corresponding to the specified modelState
+    * @throws GLSPServerException if the {@link ClientOptions#DIAGRAM_TYPE} associated to the modelState
+    *                                doesn't exist in this registry
+    */
+   default DiagramConfiguration get(final GModelState modelState) {
+      return getOrThrow(ClientOptions.getValue(modelState.getClientOptions(), ClientOptions.DIAGRAM_TYPE)
+         .flatMap(this::get), "Unsupported diagram kind");
+   }
 }
diff --git a/...p.server/src/org/eclipse/glsp/server/features/core/model/ComputedBoundsActionHandler.java b/...p.server/src/org/eclipse/glsp/server/features/core/model/ComputedBoundsActionHandler.java
@@ -26,20 +26,19 @@
 import com.google.inject.Inject;
 
 public class ComputedBoundsActionHandler extends BasicActionHandler<ComputedBoundsAction> {
+
    @Inject
    protected ModelSubmissionHandler submissionHandler;
 
    @Override
    public List<Action> executeAction(final ComputedBoundsAction action, final GModelState modelState) {
-
       synchronized (submissionHandler.getModelLock()) {
          GModelRoot model = modelState.getRoot();
          if (model != null && model.getRevision() == action.getRevision()) {
             LayoutUtil.applyBounds(model, action, modelState);
-            return submissionHandler.doSubmitModel(true, modelState);
+            return submissionHandler.submitModel(true, modelState, false);
          }
       }
-
       return none();
    }
 

diff --git a/...e.glsp.server/src/org/eclipse/glsp/server/features/core/model/ModelSubmissionHandler.java b/...e.glsp.server/src/org/eclipse/glsp/server/features/core/model/ModelSubmissionHandler.java
@@ -21,8 +21,12 @@
 
 import org.eclipse.glsp.graph.GModelRoot;
 import org.eclipse.glsp.server.actions.Action;
+import org.eclipse.glsp.server.actions.ActionDispatcher;
+import org.eclipse.glsp.server.actions.ActionHandler;
+import org.eclipse.glsp.server.actions.SetDirtyStateAction;
+import org.eclipse.glsp.server.diagram.DiagramConfiguration;
+import org.eclipse.glsp.server.diagram.DiagramConfigurationRegistry;
 import org.eclipse.glsp.server.layout.ILayoutEngine;
-import org.eclipse.glsp.server.layout.ServerLayoutConfiguration;
 import org.eclipse.glsp.server.layout.ServerLayoutKind;
 import org.eclipse.glsp.server.model.GModelState;
 
@@ -31,24 +35,78 @@
 
 @Singleton
 public class ModelSubmissionHandler {
+
    @Inject(optional = true)
    protected ILayoutEngine layoutEngine = new ILayoutEngine.NullImpl();
+
    @Inject
-   protected ServerLayoutConfiguration serverConfiguration;
+   protected DiagramConfigurationRegistry diagramConfigurationRegistry;
+
    private final Object modelLock = new Object();
    private final int revision = 0;
 
-   public List<Action> doSubmitModel(final boolean update, final GModelState modelState) {
+   /**
+    * <p>
+    * Compute and return a list of actions to update the client-side model, based on the specified modelState.
+    * These actions are not processed by this {@link ModelSubmissionHandler}, but should be either manually
+    * dispatched to the {@link ActionDispatcher}, or simply returned as the result of an
+    * {@link ActionHandler#execute(Action, GModelState)} method.
+    * </p>
+    * <p>
+    * Equivalent to <code>doSubmitModel(update, modelState, true)</code>.
+    * </p>
+    *
+    * @param update
+    *                      <code>true</code> if this is an update to an existing model; <code>false</code>
+    *                      if this is a new model (e.g. after loading a model)
+    * @param modelState
+    *                      The {@link GModelState}
+    * @return
+    *         A list of Actions to be processed in order to submit the model
+    */
+   public List<Action> submitModel(final boolean update, final GModelState modelState) {
+      return submitModel(update, modelState, true);
+   }
+
+   /**
+    * <p>
+    * Compute and return a list of actions to update the client-side model, based on the specified modelState.
+    * These actions are not processed by this {@link ModelSubmissionHandler}, but should be either manually
+    * dispatched to the {@link ActionDispatcher}, or simply returned as the result of an
+    * {@link ActionHandler#execute(Action, GModelState)} method.
+    * </p>
+    *
+    * @param update
+    *                      <code>true</code> if this is an update to an existing model; <code>false</code>
+    *                      if this is a new model (e.g. after loading a model)
+    * @param modelState
+    *                      The {@link GModelState}
+    * @param layout
+    *                      Whether layout should be processed. This should be <code>true</code> for most actions;
+    *                      <code>false</code> for actions that already react to client-layout changes
+    *                      (i.e. {@link ComputedBoundsAction} ).
+    * @return
+    *         A list of Actions to be processed in order to submit the model.
+    */
+   public List<Action> submitModel(final boolean update, final GModelState modelState, final boolean layout) {
       GModelRoot newRoot = modelState.getRoot();
-      if (serverConfiguration.getLayoutKind() == ServerLayoutKind.AUTOMATIC) {
+      DiagramConfiguration diagramConfiguration = diagramConfigurationRegistry.get(modelState);
+      if (diagramConfiguration.getLayoutKind() == ServerLayoutKind.AUTOMATIC) {
          layoutEngine.layout(modelState);
       }
+
+      boolean needsClientLayout = layout && diagramConfiguration.needsClientLayout();
+
       synchronized (modelLock) {
          if (newRoot.getRevision() == revision) {
             if (update) {
-               return Arrays.asList(new UpdateModelAction(newRoot, true));
+               Action updateModel = needsClientLayout ? new RequestBoundsAction(modelState.getRoot())
+                  : new UpdateModelAction(newRoot, true);
+               return Arrays.asList(updateModel, new SetDirtyStateAction(modelState.isDirty()));
             }
-            return Arrays.asList(new SetModelAction(newRoot));
+            Action setModel = needsClientLayout ? new RequestBoundsAction(modelState.getRoot())
+               : new SetModelAction(newRoot);
+            return Arrays.asList(setModel);
          }
       }
       return Collections.emptyList();

diff --git a/...lsp.server/src/org/eclipse/glsp/server/features/core/model/RequestModelActionHandler.java b/...lsp.server/src/org/eclipse/glsp/server/features/core/model/RequestModelActionHandler.java
@@ -22,7 +22,6 @@
 import org.eclipse.glsp.server.actions.BasicActionHandler;
 import org.eclipse.glsp.server.features.modelsourcewatcher.ModelSourceWatcher;
 import org.eclipse.glsp.server.model.GModelState;
-import org.eclipse.glsp.server.utils.ClientOptions;
 
 import com.google.inject.Inject;
 
@@ -34,20 +33,16 @@ public class RequestModelActionHandler extends BasicActionHandler<RequestModelAc
    @Inject
    private ModelSourceWatcher modelSourceWatcher;
 
+   @Inject
+   protected ModelSubmissionHandler modelSubmissionHandler;
+
    @Override
    public List<Action> executeAction(final RequestModelAction action, final GModelState modelState) {
+      modelState.setClientOptions(action.getOptions());
       GModelRoot model = modelFactory.loadModel(action, modelState);
       modelState.setRoot(model);
-      modelState.setClientOptions(action.getOptions());
-
       modelSourceWatcher.startWatching(modelState);
-
-      boolean needsClientLayout = ClientOptions.getBoolValue(action.getOptions(),
-         ClientOptions.NEEDS_CLIENT_LAYOUT);
-
-      Action responseAction = needsClientLayout ? new RequestBoundsAction(modelState.getRoot())
-         : new SetModelAction(modelState.getRoot());
-      return listOf(responseAction);
+      return modelSubmissionHandler.submitModel(false, modelState);
    }
 
 }
diff --git a/...ipse.glsp.server/src/org/eclipse/glsp/server/features/undoredo/UndoRedoActionHandler.java b/...ipse.glsp.server/src/org/eclipse/glsp/server/features/undoredo/UndoRedoActionHandler.java
@@ -20,23 +20,26 @@
 import org.apache.log4j.Logger;
 import org.eclipse.glsp.server.actions.Action;
 import org.eclipse.glsp.server.actions.ActionHandler;
-import org.eclipse.glsp.server.actions.SetDirtyStateAction;
-import org.eclipse.glsp.server.features.core.model.RequestBoundsAction;
+import org.eclipse.glsp.server.features.core.model.ModelSubmissionHandler;
 import org.eclipse.glsp.server.model.GModelState;
 
 import com.google.common.collect.Lists;
+import com.google.inject.Inject;
 
 public class UndoRedoActionHandler implements ActionHandler {
    private static final Logger LOG = Logger.getLogger(UndoRedoActionHandler.class);
 
+   @Inject
+   protected ModelSubmissionHandler modelSubmissionHandler;
+
    @Override
    public List<Action> execute(final Action action, final GModelState modelState) {
       if (action instanceof UndoAction && modelState.canUndo()) {
          modelState.undo();
-         return listOf(new RequestBoundsAction(modelState.getRoot()), new SetDirtyStateAction(modelState.isDirty()));
+         return modelSubmissionHandler.submitModel(true, modelState);
       } else if (action instanceof RedoAction && modelState.canRedo()) {
          modelState.redo();
-         return listOf(new RequestBoundsAction(modelState.getRoot()), new SetDirtyStateAction(modelState.isDirty()));
+         return modelSubmissionHandler.submitModel(true, modelState);
       }
       LOG.warn("Cannot undo or redo");
       return none();

diff --git a/...org.eclipse.glsp.server/src/org/eclipse/glsp/server/layout/ServerLayoutConfiguration.java b/...org.eclipse.glsp.server/src/org/eclipse/glsp/server/layout/ServerLayoutConfiguration.java
diff --git a/...rg.eclipse.glsp.server/src/org/eclipse/glsp/server/operations/OperationActionHandler.java b/...rg.eclipse.glsp.server/src/org/eclipse/glsp/server/operations/OperationActionHandler.java
@@ -20,8 +20,7 @@
 
 import org.eclipse.glsp.server.actions.Action;
 import org.eclipse.glsp.server.actions.BasicActionHandler;
-import org.eclipse.glsp.server.actions.SetDirtyStateAction;
-import org.eclipse.glsp.server.features.core.model.RequestBoundsAction;
+import org.eclipse.glsp.server.features.core.model.ModelSubmissionHandler;
 import org.eclipse.glsp.server.internal.gmodel.commandstack.GModelRecordingCommand;
 import org.eclipse.glsp.server.model.GModelState;
 import org.eclipse.glsp.server.utils.ServerMessageUtil;
@@ -32,6 +31,9 @@ public class OperationActionHandler extends BasicActionHandler<Operation> {
    @Inject
    protected OperationHandlerRegistry operationHandlerRegistry;
 
+   @Inject
+   protected ModelSubmissionHandler modelSubmissionHandler;
+
    @Override
    public boolean handles(final Action action) {
       return action instanceof Operation;
@@ -55,7 +57,7 @@ protected List<Action> executeHandler(final Operation operation, final Operation
       GModelRecordingCommand command = new GModelRecordingCommand(modelState.getRoot(), handler.getLabel(),
          () -> handler.execute(operation, modelState));
       modelState.execute(command);
-      return listOf(new RequestBoundsAction(modelState.getRoot()), new SetDirtyStateAction(modelState.isDirty()));
+      return modelSubmissionHandler.submitModel(true, modelState);
    }
 
    public static Optional<? extends OperationHandler> getOperationHandler(final Operation operation,