Let OperationHandlers return an optional command to control execution

Make OperationHandler take a generic type - Deprecate Abstract base classes and provide Basic*, EMF*, GModel* - Adapt handlers to use new base class - Elevate concept of command to OperationHandler for more control -- Move command handling from EMF to general operation handler - Let abstract handlers use recording command for compatibility -- Add new 'preExecute' method to abort execution - Minor fixes -- Convert ApplyTaskEditActionHandler from operation to action handler Fixes eclipse-glsp/glsp#864
eclipse-glsp · Jan 25, 2023 · 6bdf2f3 · 6bdf2f3
1 parent 6c99c9a
commit 6bdf2f3
Show file tree

Hide file tree

Showing 49 changed files with 715 additions and 425 deletions.
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -43,7 +43,7 @@ jobs:
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@v1
+      uses: github/codeql-action/init@v2
       with:
         languages: ${{ matrix.language }}
         # If you wish to specify custom queries, you can do so here or in a config file.
@@ -68,4 +68,4 @@ jobs:
     #   make release
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@v1
+      uses: github/codeql-action/analyze@v2
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Changes
 
 - [graph] Updated the Graph model and add Layoutable interface [#175](https://github.com/eclipse-glsp/glsp-server/pull/175) - Contributed on behalf of STMicroelectronics
+- [operation] Rework `OperationHandler` to provide an optional command instead of direct execution to allow more execution control [#187](https://github.com/eclipse-glsp/glsp-server/pull/187)
 
 ### Breaking Changes
 - [websocket] Update to Jetty Websocket 10 [#185](https://github.com/eclipse-glsp/glsp-server/pull/185) [#186](https://github.com/eclipse-glsp/glsp-server/pull/186) - Contributed on behalf of STMicroelectronics

diff --git a/...se.glsp.example.workflow/src/org/eclipse/glsp/example/workflow/WorkflowDiagramModule.java b/...se.glsp.example.workflow/src/org/eclipse/glsp/example/workflow/WorkflowDiagramModule.java
@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2019-2022 EclipseSource and others.
+ * Copyright (c) 2019-2023 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
@@ -35,7 +35,7 @@
 import org.eclipse.glsp.example.workflow.provider.PreviousNodeNavigationTargetProvider;
 import org.eclipse.glsp.example.workflow.provider.WorkflowCommandPaletteActionProvider;
 import org.eclipse.glsp.example.workflow.provider.WorkflowContextMenuItemProvider;
-import org.eclipse.glsp.example.workflow.taskedit.ApplyTaskEditOperationHandler;
+import org.eclipse.glsp.example.workflow.taskedit.ApplyTaskEditActionHandler;
 import org.eclipse.glsp.example.workflow.taskedit.EditTaskOperationHandler;
 import org.eclipse.glsp.example.workflow.taskedit.TaskEditContextActionProvider;
 import org.eclipse.glsp.example.workflow.taskedit.TaskEditValidator;
@@ -104,7 +104,7 @@ protected void configureNavigationTargetProviders(final MultiBinding<NavigationT
    }
 
    @Override
-   protected void configureOperationHandlers(final MultiBinding<OperationHandler> binding) {
+   protected void configureOperationHandlers(final MultiBinding<OperationHandler<?>> binding) {
       super.configureOperationHandlers(binding);
       binding.add(CreateAutomatedTaskHandler.class);
       binding.add(CreateManualTaskHandler.class);
@@ -116,14 +116,14 @@ protected void configureOperationHandlers(final MultiBinding<OperationHandler> b
       binding.add(CreateWeightedEdgeHandler.class);
       binding.add(CreateCategoryHandler.class);
       binding.add(EditTaskOperationHandler.class);
-      binding.add(ApplyTaskEditOperationHandler.class);
    }
 
    @Override
    protected void configureActionHandlers(final MultiBinding<ActionHandler> binding) {
       super.configureActionHandlers(binding);
       binding.rebind(RequestContextActionsHandler.class, WorkflowRequestContextActionsHandler.class);
       binding.add(LogActionHandler.class);
+      binding.add(ApplyTaskEditActionHandler.class);
    }
 
    @Override

diff --git a/...flow/taskedit/ApplyTaskEditOperation.java → ...orkflow/taskedit/ApplyTaskEditAction.java b/...flow/taskedit/ApplyTaskEditOperation.java → ...orkflow/taskedit/ApplyTaskEditAction.java
@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2020 EclipseSource and others.
+ * Copyright (c) 2020-2023 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
@@ -15,18 +15,18 @@
  ********************************************************************************/
 package org.eclipse.glsp.example.workflow.taskedit;
 
-import org.eclipse.glsp.server.operations.Operation;
+import org.eclipse.glsp.server.actions.Action;
 
-public class ApplyTaskEditOperation extends Operation {
+public class ApplyTaskEditAction extends Action {
 
    private String taskId;
    private String expression;
 
-   public ApplyTaskEditOperation() {
+   public ApplyTaskEditAction() {
       super("applyTaskEdit");
    }
 
-   public ApplyTaskEditOperation(final String taskId, final String expression) {
+   public ApplyTaskEditAction(final String taskId, final String expression) {
       this();
       this.taskId = taskId;
       this.expression = expression;

diff --git a/...skedit/ApplyTaskEditOperationHandler.java → .../taskedit/ApplyTaskEditActionHandler.java b/...skedit/ApplyTaskEditOperationHandler.java → .../taskedit/ApplyTaskEditActionHandler.java
@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2020-2021 EclipseSource and others.
+ * Copyright (c) 2020-2023 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
@@ -15,33 +15,26 @@
  ********************************************************************************/
 package org.eclipse.glsp.example.workflow.taskedit;
 
-import org.eclipse.glsp.server.actions.ActionDispatcher;
-import org.eclipse.glsp.server.model.GModelState;
-import org.eclipse.glsp.server.operations.AbstractOperationHandler;
-import org.eclipse.glsp.server.types.GLSPServerException;
-
-import com.google.inject.Inject;
+import java.util.List;
 
-public class ApplyTaskEditOperationHandler extends AbstractOperationHandler<ApplyTaskEditOperation> {
-
-   @Inject
-   protected ActionDispatcher actionDispatcher;
+import org.eclipse.glsp.server.actions.AbstractActionHandler;
+import org.eclipse.glsp.server.actions.Action;
+import org.eclipse.glsp.server.types.GLSPServerException;
 
-   @Inject
-   protected GModelState modelState;
+public class ApplyTaskEditActionHandler extends AbstractActionHandler<ApplyTaskEditAction> {
 
    @Override
-   protected void executeOperation(final ApplyTaskEditOperation operation) {
-      String text = operation.getExpression();
+   protected List<Action> executeAction(final ApplyTaskEditAction actualAction) {
+      String text = actualAction.getExpression();
       if (text.startsWith(TaskEditContextActionProvider.DURATION_PREFIX)) {
          String durationString = text.substring(TaskEditContextActionProvider.DURATION_PREFIX.length());
-         actionDispatcher.dispatch(new EditTaskOperation(operation.getTaskId(), "duration", durationString));
+         return listOf(new EditTaskOperation(actualAction.getTaskId(), "duration", durationString));
       } else if (text.startsWith(TaskEditContextActionProvider.TYPE_PREFIX)) {
          String typeString = text.substring(TaskEditContextActionProvider.TYPE_PREFIX.length());
-         actionDispatcher.dispatch(new EditTaskOperation(operation.getTaskId(), "taskType", typeString));
+         return listOf(new EditTaskOperation(actualAction.getTaskId(), "taskType", typeString));
       } else {
          throw new GLSPServerException(
-            "Cannot process 'ApplyTaskEditOperation' expression: " + operation.getExpression());
+            "Cannot process 'ApplyTaskEditOperation' expression: " + actualAction.getExpression());
       }
    }
 

diff --git a/...ple.workflow/src/org/eclipse/glsp/example/workflow/taskedit/EditTaskOperationHandler.java b/...ple.workflow/src/org/eclipse/glsp/example/workflow/taskedit/EditTaskOperationHandler.java
@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2020-2021 EclipseSource and others.
+ * Copyright (c) 2020-2023 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
@@ -15,36 +15,31 @@
  ********************************************************************************/
 package org.eclipse.glsp.example.workflow.taskedit;
 
+import java.util.Objects;
 import java.util.Optional;
 
+import org.eclipse.emf.common.command.Command;
 import org.eclipse.glsp.example.workflow.wfgraph.TaskNode;
-import org.eclipse.glsp.server.model.GModelState;
-import org.eclipse.glsp.server.operations.AbstractOperationHandler;
-import org.eclipse.glsp.server.types.GLSPServerException;
+import org.eclipse.glsp.server.operations.GModelOperationHandler;
 
-import com.google.inject.Inject;
-
-public class EditTaskOperationHandler extends AbstractOperationHandler<EditTaskOperation> {
-
-   @Inject
-   protected GModelState modelState;
+public class EditTaskOperationHandler extends GModelOperationHandler<EditTaskOperation> {
 
    @Override
-   protected void executeOperation(final EditTaskOperation operation) {
-      Optional<TaskNode> task = modelState.getIndex().findElementByClass(operation.getTaskId(), TaskNode.class);
-      if (task.isEmpty()) {
-         throw new RuntimeException("Cannot find task with id '" + operation.getTaskId() + "'");
-      }
+   public Optional<Command> createCommand(final EditTaskOperation operation) {
+      TaskNode task = modelState.getIndex().findElementByClass(operation.getTaskId(), TaskNode.class)
+         .orElseThrow(() -> new RuntimeException("Cannot find task with id '" + operation.getTaskId() + "'"));
       switch (operation.getFeature()) {
          case "duration":
-            task.get().setDuration(Integer.parseInt(operation.getValue()));
-            break;
+            int duration = Integer.parseInt(operation.getValue());
+            return Objects.equals(task.getDuration(), duration)
+               ? doNothing()
+               : commandOf(() -> task.setDuration(duration));
          case "taskType":
-            task.get().setTaskType(operation.getValue());
-            break;
+            return Objects.equals(task.getTaskType(), operation.getValue())
+               ? doNothing()
+               : commandOf(() -> task.setTaskType(operation.getValue()));
          default:
-            throw new GLSPServerException("Cannot edit task at feature '" + operation.getFeature() + "'");
+            return doNothing();
       }
    }
-
 }
diff --git a/plugins/org.eclipse.glsp.graph/src/org/eclipse/glsp/graph/GModelIndex.java b/plugins/org.eclipse.glsp.graph/src/org/eclipse/glsp/graph/GModelIndex.java
@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2019-2021 EclipseSource and others.
+ * Copyright (c) 2019-2023 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
@@ -78,6 +78,27 @@ static void remove(final GModelElement element) {
     */
    Optional<GModelElement> get(String elementId);
 
+   /**
+    * Returns an optional {@link GModelElement} by its elementId iff it also an instance of the given class.
+    *
+    * @param <T>       The type of the element to be found.
+    * @param elementId The id of the requested {@link GModelElement}.
+    * @param clazz     The class of which the found element should be an instance.
+    * @return An optional with the element of type clazz or an empty optional.
+    *
+    * @see {@link #findElementByClass(String, Class)} to search the whole parent hierarchy.
+    */
+   default <T extends GModelElement> Optional<T> getByClass(final String elementId, final Class<T> clazz) {
+      if (elementId == null) {
+         return Optional.empty();
+      }
+      Optional<GModelElement> element = get(elementId);
+      if (element.isPresent() && clazz.isInstance(element.get())) {
+         return Optional.of(clazz.cast(element.get()));
+      }
+      return Optional.empty();
+   }
+
    /**
     * Returns a set of {@link GModelElement} instances by a Collection of elementIds.
     *

diff --git a/...lsp.server.emf/src/org/eclipse/glsp/server/emf/AbstractEMFCreateEdgeOperationHandler.java b/...lsp.server.emf/src/org/eclipse/glsp/server/emf/AbstractEMFCreateEdgeOperationHandler.java
@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2022 EclipseSource and others.
+ * Copyright (c) 2022-2023 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
@@ -23,9 +23,15 @@
 /**
  * A special {@link EMFOperationHandler} that is responsible for the handling of {@link CreateEdgeOperation}s.
  * It provides an EMF command to create the specific edge.
+ *
+ * @deprecated Use {@link EMFCreateOperationHandler}
  */
+@Deprecated
 public abstract class AbstractEMFCreateEdgeOperationHandler
-   extends AbstractEMFCreateOperationHandler<CreateEdgeOperation> implements CreateEdgeOperationHandler {
+   extends AbstractEMFCreateOperationHandler<CreateEdgeOperation>
+   implements CreateEdgeOperationHandler<CreateEdgeOperation> {
+
+   protected List<String> handledElementTypeIds;
 
    public AbstractEMFCreateEdgeOperationHandler(final String... elementTypeIds) {
       super(elementTypeIds);

diff --git a/...lsp.server.emf/src/org/eclipse/glsp/server/emf/AbstractEMFCreateNodeOperationHandler.java b/...lsp.server.emf/src/org/eclipse/glsp/server/emf/AbstractEMFCreateNodeOperationHandler.java
@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2022 EclipseSource and others.
+ * Copyright (c) 2022-2023 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
@@ -23,9 +23,13 @@
 /**
  * A special {@link EMFOperationHandler} that is responsible for the handling of {@link CreateNodeOperation}s.
  * It provides an EMF command to create the specific node.
+ *
+ * @deprecated Use {@link EMFCreateNodeOperationHandler}
  */
+@Deprecated
 public abstract class AbstractEMFCreateNodeOperationHandler
-   extends AbstractEMFCreateOperationHandler<CreateNodeOperation> implements CreateNodeOperationHandler {
+   extends AbstractEMFCreateOperationHandler<CreateNodeOperation>
+   implements CreateNodeOperationHandler<CreateNodeOperation> {
 
    public AbstractEMFCreateNodeOperationHandler(final String... elementTypeIds) {
       super(elementTypeIds);

diff --git a/...se.glsp.server.emf/src/org/eclipse/glsp/server/emf/AbstractEMFCreateOperationHandler.java b/...se.glsp.server.emf/src/org/eclipse/glsp/server/emf/AbstractEMFCreateOperationHandler.java
@@ -17,11 +17,14 @@
 
 import java.util.List;
 
-import org.eclipse.glsp.server.operations.AbstractCreateOperationHandler;
 import org.eclipse.glsp.server.operations.CreateOperation;
 
+/**
+ * @deprecated Use {@link EMFCreateOperationHandler}
+ */
+@Deprecated
 public abstract class AbstractEMFCreateOperationHandler<O extends CreateOperation>
-   extends AbstractCreateOperationHandler<O> implements EMFOperationHandler<O> {
+   extends EMFCreateOperationHandler<O> {
 
    public AbstractEMFCreateOperationHandler(final String... elementTypeIds) {
       super(elementTypeIds);
@@ -31,9 +34,4 @@ public AbstractEMFCreateOperationHandler(final List<String> handledElementTypeId
       super(handledElementTypeIds);
    }
 
-   @Override
-   protected void executeOperation(final O operation) {
-      EMFOperationHandler.super.execute(operation);
-   }
-
 }
diff --git a/....eclipse.glsp.server.emf/src/org/eclipse/glsp/server/emf/AbstractEMFOperationHandler.java b/....eclipse.glsp.server.emf/src/org/eclipse/glsp/server/emf/AbstractEMFOperationHandler.java
@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2022 EclipseSource and others.
+ * Copyright (c) 2022-2023 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
@@ -15,15 +15,10 @@
  ********************************************************************************/
 package org.eclipse.glsp.server.emf;
 
-import org.eclipse.glsp.server.operations.AbstractOperationHandler;
 import org.eclipse.glsp.server.operations.Operation;
 
-public abstract class AbstractEMFOperationHandler<O extends Operation> extends AbstractOperationHandler<O>
-   implements EMFOperationHandler<O> {
-
-   @Override
-   protected void executeOperation(final O operation) {
-      EMFOperationHandler.super.execute(operation);
-   }
-
-}
+/**
+ * @deprecated Use {@link EMFOperationHandler} instead
+ */
+@Deprecated
+public abstract class AbstractEMFOperationHandler<O extends Operation> extends EMFOperationHandler<O> {}
diff --git a/....eclipse.glsp.server.emf/src/org/eclipse/glsp/server/emf/EMFCompoundOperationHandler.java b/....eclipse.glsp.server.emf/src/org/eclipse/glsp/server/emf/EMFCompoundOperationHandler.java
@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (c) 2022 EclipseSource and others.
+ * Copyright (c) 2022-2023 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
@@ -15,36 +15,12 @@
  ********************************************************************************/
 package org.eclipse.glsp.server.emf;
 
-import java.util.Optional;
-
-import org.eclipse.emf.common.command.Command;
-import org.eclipse.emf.common.command.CompoundCommand;
-import org.eclipse.glsp.server.operations.CompoundOperation;
-import org.eclipse.glsp.server.operations.Operation;
-import org.eclipse.glsp.server.operations.OperationHandlerRegistry;
-
-import com.google.inject.Inject;
+import org.eclipse.glsp.server.operations.CompoundOperationHandler;
 
 /**
  * Creates a compound command to wrap multiple commands into one command that is executed on the command stack.
+ *
+ * @deprecated Use {@link CompoundOperationHandler} directly.
  */
-public class EMFCompoundOperationHandler extends AbstractEMFOperationHandler<CompoundOperation> {
-
-   @Inject
-   protected OperationHandlerRegistry operationHandlerRegistry;
-
-   @Override
-   public Optional<Command> createCommand(final CompoundOperation operation) {
-      CompoundCommand compoundCommand = new CompoundCommand();
-      operation.getOperationList()
-         .forEach(nestedOperation -> getNestedCommand(nestedOperation).ifPresent(compoundCommand::append));
-      return compoundCommand.getCommandList().isEmpty()
-         ? Optional.empty()
-         : Optional.of(compoundCommand);
-   }
-
-   protected Optional<Command> getNestedCommand(final Operation operation) {
-      return EMFOperationHandler.getCommand(operationHandlerRegistry, operation);
-   }
-
-}
+@Deprecated
+public class EMFCompoundOperationHandler extends CompoundOperationHandler {}