Skip to content

Commit

Permalink
211: Revise TypeHints and server side feedback for creation actions
Browse files Browse the repository at this point in the history
- Add a new parameter to EdgeTypeHint, 'dynamic', indicating that new
edges need to check with the server before allowing creation
- Add a new Request/Response to implement this check
  • Loading branch information
CamilleLetavernier committed Sep 11, 2023
1 parent a48db75 commit 7b1a160
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/********************************************************************************
* Copyright (c) 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
* 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.actions;

/**
* {@link ResponseAction} for a {@link RequestCheckEdgeTargetAction}.
*/
public class CheckEdgeTargetResultAction extends ResponseAction {

public static final String KIND = "setTargetTypeHints";

private boolean isValid;

public CheckEdgeTargetResultAction() {
this(false);
}

public CheckEdgeTargetResultAction(final boolean isValid) {
super(KIND);
this.setValid(isValid);
}

public boolean isValid() { return isValid; }

public void setValid(final boolean isValid) { this.isValid = isValid; }

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/********************************************************************************
* Copyright (c) 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
* 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.actions;

import org.eclipse.glsp.server.types.EdgeTypeHint;

/**
* A {@link RequestAction} used to check the validity of an edge being created. This is used
* to update creation feedback on the client side, for edges configured with a dynamic {@link EdgeTypeHint}.
*
* @see EdgeTypeHint#isDynamic()
* @see CheckEdgeTargetResultAction
*/
public class RequestCheckEdgeTargetAction extends RequestAction<CheckEdgeTargetResultAction> {

public static final String KIND = "requestCheckEdgeTarget";

private String edgeTypeId;

private String sourceElementId;

private String targetElementId;

public RequestCheckEdgeTargetAction() {
super(KIND);
}

/**
* The element type id of the Edge.
*
* @return
* the element type id of the Edge.
*/
public String getEdgeTypeId() { return edgeTypeId; }

public void setEdgeTypeId(final String edgeTypeId) { this.edgeTypeId = edgeTypeId; }

/**
* The ID of the source element of the edge being created.
*
* @return the ID of the source element of the edge being created.
*/
public String getSourceElementId() { return sourceElementId; }

public void setSourceElementId(final String sourceElementId) { this.sourceElementId = sourceElementId; }

/**
* The ID of the target element of the edge being created.
*
* @return the ID of the target element of the edge being created.
*/
public String getTargetElementId() { return targetElementId; }

public void setTargetElementId(final String targetElementId) { this.targetElementId = targetElementId; }

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.eclipse.glsp.server.actions.SetDirtyStateAction;
import org.eclipse.glsp.server.actions.SetEditModeAction;
import org.eclipse.glsp.server.actions.SetEditModeActionHandler;
import org.eclipse.glsp.server.actions.CheckEdgeTargetResultAction;
import org.eclipse.glsp.server.actions.SetViewportAction;
import org.eclipse.glsp.server.actions.StartProgressAction;
import org.eclipse.glsp.server.actions.TriggerEdgeCreationAction;
Expand All @@ -43,6 +44,7 @@
import org.eclipse.glsp.server.di.scope.DiagramGlobalScope;
import org.eclipse.glsp.server.di.scope.DiagramGlobalSingleton;
import org.eclipse.glsp.server.diagram.DiagramConfiguration;
import org.eclipse.glsp.server.diagram.DefaultRequestTargetTypeHintsActionHandler;
import org.eclipse.glsp.server.diagram.RequestTypeHintsActionHandler;
import org.eclipse.glsp.server.diagram.ServerConfigurationContribution;
import org.eclipse.glsp.server.diagram.SetTypeHintsAction;
Expand Down Expand Up @@ -320,6 +322,7 @@ protected void configureClientActions(final MultiBinding<Action> binding) {
binding.add(TriggerNodeCreationAction.class);
binding.add(UpdateModelAction.class);
binding.add(UpdateProgressAction.class);
binding.add(CheckEdgeTargetResultAction.class);
}

protected void configureActionHandlers(final MultiBinding<ActionHandler> binding) {
Expand All @@ -338,6 +341,7 @@ protected void configureActionHandlers(final MultiBinding<ActionHandler> binding
binding.add(ComputedBoundsActionHandler.class);
binding.add(SaveModelActionHandler.class);
binding.add(UndoRedoActionHandler.class);
binding.add(DefaultRequestTargetTypeHintsActionHandler.class);
}

protected Class<? extends ActionHandlerRegistry> bindActionHandlerRegistry() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/********************************************************************************
* Copyright (c) 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
* 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.diagram;

import java.util.List;

import org.eclipse.glsp.server.actions.AbstractActionHandler;
import org.eclipse.glsp.server.actions.Action;
import org.eclipse.glsp.server.actions.CheckEdgeTargetResultAction;
import org.eclipse.glsp.server.actions.RequestCheckEdgeTargetAction;

import com.google.inject.Inject;

/**
* Default handler implementation for {@link RequestCheckEdgeTargetAction}.
* This handler always rejects creation of dynamic Edges. Client applications
* should replace this default implementation with their own logic to
* conditionally accept creation of new edges configured with a Dynamic type.
*/
public class DefaultRequestTargetTypeHintsActionHandler extends AbstractActionHandler<RequestCheckEdgeTargetAction> {

@Inject
protected DiagramConfiguration diagramConfiguration;

@Override
protected List<Action> executeAction(final RequestCheckEdgeTargetAction actualAction) {
return List.of(new CheckEdgeTargetResultAction());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public interface DiagramConfiguration {
Optional<GraphExtension> getGraphExtension();

default EdgeTypeHint createDefaultEdgeTypeHint(final String elementId) {
return new EdgeTypeHint(elementId, true, true, true, null, null);
return new EdgeTypeHint(elementId, true, true, true, false, null, null);
}

default ShapeTypeHint createDefaultShapeTypeHint(final String elementId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

import java.util.List;

import org.eclipse.glsp.server.actions.RequestCheckEdgeTargetAction;

/**
* {@link ElementTypeHint Element type hints} for edges.
*/
Expand All @@ -25,12 +27,19 @@ public class EdgeTypeHint extends ElementTypeHint {
private boolean routable;
private List<String> sourceElementTypeIds;
private List<String> targetElementTypeIds;
private boolean dynamic = false;

Check warning on line 30 in plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/types/EdgeTypeHint.java

View check run for this annotation

Jenkins - GLSP / CheckStyle

ExplicitInitializationCheck

NORMAL: Variable 'dynamic' explicitly initialized to 'false' (default value for its type).
Raw output
<p>Since Checkstyle 3.2</p><p> Checks if any class or object member is explicitly initialized to default for its type value (<code>null</code> for object references, zero for numeric types and <code>char</code> and <code>false</code> for <code>boolean</code>. </p><p> Rationale: Each instance variable gets initialized twice, to the same value. Java initializes each instance variable to its default value (0 or null) before performing any initialization specified in the code. So in this case, x gets initialized to 0 twice, and bar gets initialized to null twice. So there is a minor inefficiency. This style of coding is a holdover from C/C++ style coding, and it shows that the developer isn't really confident that Java initializes instance variables to default values. </p>

public EdgeTypeHint() {}

public EdgeTypeHint(final String elementTypeId, final boolean repositionable, final boolean deletable,
final boolean routable,
final List<String> sourceElementTypeIds, final List<String> targetElementTypeIds) {
this(elementTypeId, repositionable, deletable, routable, false, sourceElementTypeIds, targetElementTypeIds);
}

public EdgeTypeHint(final String elementTypeId, final boolean repositionable, final boolean deletable,
final boolean routable, final boolean dynamic,
final List<String> sourceElementTypeIds, final List<String> targetElementTypeIds) {
super(elementTypeId, repositionable, deletable);
this.routable = routable;
this.sourceElementTypeIds = sourceElementTypeIds;
Expand All @@ -53,4 +62,15 @@ public void setTargetElementTypeIds(final List<String> targetElementTypeIds) {
this.targetElementTypeIds = targetElementTypeIds;
}

/**
* Indicates whether this type hint is dynamic or not. Dynamic edge type hints
* require an additional runtime check before creating an edge, when checking
* source and target element types is not sufficient.
*
* @see RequestCheckEdgeTargetAction
*/
public boolean isDynamic() { return dynamic; }

public void setDynamic(final boolean isDynamic) { this.dynamic = isDynamic; }

}

0 comments on commit 7b1a160

Please sign in to comment.