diff --git a/CHANGELOG.md b/CHANGELOG.md index cda394d5..3e3ea706 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ ### Potentially Breaking Changes +- [layout] Keep applying computed bounds when an individual entry cannot be applied [#295](https://github.com/eclipse-glsp/glsp-server/pull/295) + - `LayoutUtil.applyRoute` now returns `Optional` instead of `GEdge` + - `LayoutUtil.applyBounds`, `applyAlignment` and `applyRoute` no longer throw for an element the index cannot resolve, they report it as not applied. `applyRoutingPoints` stays strict. + - `ComputedBoundsActionHandler` applies the computed bounds through the new overridable `applyBounds`, `applyElementBounds`, `applyAlignments` and `applyRoutes` methods, so adjusting one kind no longer means taking over `executeAction` and its model lock + - `LayoutUtil.applyBounds(GModelRoot, ComputedBoundsAction, GModelState)` is deprecated. It dispatches to the static per-kind methods directly and therefore bypasses the overridable ones of `ComputedBoundsActionHandler`. + ## [v2.7.0 - 01/06/2026](https://github.com/eclipse-glsp/glsp-server/releases/tag/v2.7.0) ### Changes diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/features/core/model/ComputedBoundsActionHandler.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/features/core/model/ComputedBoundsActionHandler.java index d4be507d..f1827a72 100644 --- a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/features/core/model/ComputedBoundsActionHandler.java +++ b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/features/core/model/ComputedBoundsActionHandler.java @@ -1,5 +1,5 @@ /******************************************************************************** - * Copyright (c) 2019-2023 EclipseSource and others. + * Copyright (c) 2019-2026 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 @@ -17,10 +17,14 @@ import java.util.List; +import org.eclipse.glsp.graph.GModelIndex; import org.eclipse.glsp.graph.GModelRoot; import org.eclipse.glsp.server.actions.AbstractActionHandler; import org.eclipse.glsp.server.actions.Action; import org.eclipse.glsp.server.model.GModelState; +import org.eclipse.glsp.server.types.ElementAndAlignment; +import org.eclipse.glsp.server.types.ElementAndBounds; +import org.eclipse.glsp.server.types.ElementAndRoutingPoints; import org.eclipse.glsp.server.utils.LayoutUtil; import com.google.inject.Inject; @@ -44,11 +48,59 @@ public List executeAction(final ComputedBoundsAction action) { GModelRoot model = modelState.getRoot(); if (model != null && action.getRevision().isPresent() && action.getRevision().get().doubleValue() == model.getRevision()) { - LayoutUtil.applyBounds(model, action, modelState); + applyBounds(model, action); return submissionHandler.submitModelDirectly(); } } return none(); } + /** + * Applies everything the client computed for the given model. + * + *

+ * Override this, or one of the per-kind methods it delegates to, to adjust what is applied. Overriding + * {@link #executeAction(ComputedBoundsAction)} instead would also take over the revision check and the model lock. + *

+ * + * @param root The model root whose revision the action matched. + * @param action The computed bounds action. + */ + protected void applyBounds(final GModelRoot root, final ComputedBoundsAction action) { + GModelIndex index = modelState.getIndex(); + applyElementBounds(action.getBounds(), index); + applyAlignments(action.getAlignments(), index); + applyRoutes(action.getRoutes(), index); + } + + /** + * Applies the computed bounds of several elements. + * + * @param allBounds The new bounds. + * @param index The model index. + */ + protected void applyElementBounds(final List allBounds, final GModelIndex index) { + LayoutUtil.applyElementBounds(allBounds, index); + } + + /** + * Applies the computed alignments of several elements. + * + * @param alignments The new alignments. + * @param index The model index. + */ + protected void applyAlignments(final List alignments, final GModelIndex index) { + LayoutUtil.applyAlignments(alignments, index); + } + + /** + * Applies the computed routes. + * + * @param routes The new routes. + * @param index The model index. + */ + protected void applyRoutes(final List routes, final GModelIndex index) { + LayoutUtil.applyRoutes(routes, index); + } + } diff --git a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/utils/LayoutUtil.java b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/utils/LayoutUtil.java index 49271eae..0bd486b8 100644 --- a/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/utils/LayoutUtil.java +++ b/plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/utils/LayoutUtil.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2019-2022 EclipseSource and others. + * Copyright (c) 2019-2026 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 @@ -21,6 +21,8 @@ import java.util.List; import java.util.Optional; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.eclipse.emf.common.util.EList; import org.eclipse.emf.ecore.util.EcoreUtil; import org.eclipse.glsp.graph.GAlignable; @@ -45,21 +47,78 @@ public final class LayoutUtil { + protected static Logger LOGGER = LogManager.getLogger(LayoutUtil.class); + private LayoutUtil() {} /** * Apply the computed bounds from the given {@link ComputedBoundsAction} to the model. * + *

+ * An entry that cannot be applied is skipped and logged rather than treated as an error. + *

+ * * @param root The model root. * @param action The computed bounds action. * @param modelState The model state + * @deprecated Use + * {@link org.eclipse.glsp.server.features.core.model.ComputedBoundsActionHandler#applyBounds(GModelRoot, ComputedBoundsAction)} + * instead. This method dispatches to the static per-kind methods directly and therefore bypasses the + * overridable ones of the handler. */ + @Deprecated public static void applyBounds(final GModelRoot root, final ComputedBoundsAction action, final GModelState modelState) { GModelIndex index = modelState.getIndex(); - action.getBounds().forEach(bounds -> applyBounds(bounds, index)); - action.getAlignments().forEach(alignment -> applyAlignment(alignment, index)); - action.getRoutes().forEach(route -> applyRoute(route, index)); + applyElementBounds(action.getBounds(), index); + applyAlignments(action.getAlignments(), index); + applyRoutes(action.getRoutes(), index); + } + + /** + * Applies the computed bounds of several elements. + * + * @param allBounds The new bounds. + * @param index The model index. + */ + public static void applyElementBounds(final List allBounds, final GModelIndex index) { + allBounds.forEach(bounds -> { + if (applyBounds(bounds, index).isEmpty()) { + LOGGER.warn("Skipped computed bounds of element '" + bounds.getElementId() + "'"); + } + }); + } + + /** + * Applies the computed alignments of several elements. + * + * @param alignments The new alignments. + * @param index The model index. + */ + public static void applyAlignments(final List alignments, final GModelIndex index) { + alignments.forEach(alignment -> { + if (applyAlignment(alignment, index).isEmpty()) { + LOGGER.warn("Skipped computed alignment of element '" + alignment.getElementId() + "'"); + } + }); + } + + /** + * Applies the computed routes. + * + *

+ * A skipped route is logged at debug level, an edge the client has not finished routing yet is expected. + *

+ * + * @param routes The new routes. + * @param index The model index. + */ + public static void applyRoutes(final List routes, final GModelIndex index) { + routes.forEach(route -> { + if (applyRoute(route, index).isEmpty()) { + LOGGER.debug("Skipped computed route of element '" + route.getElementId() + "'"); + } + }); } /** @@ -67,13 +126,12 @@ public static void applyBounds(final GModelRoot root, final ComputedBoundsAction * * @param bounds The new bounds. * @param index The model index. - * @return The changed element. + * @return The changed element, or empty if the bounds could not be applied to any element. */ public static Optional applyBounds(final ElementAndBounds bounds, final GModelIndex index) { - GModelElement element = getOrThrow(index.get(bounds.getElementId()), - "Model element not found! ID: " + bounds.getElementId()); - if (element instanceof GBoundsAware) { - GBoundsAware bae = (GBoundsAware) element; + Optional element = index.get(bounds.getElementId()); + if (element.isPresent() && element.get() instanceof GBoundsAware) { + GBoundsAware bae = (GBoundsAware) element.get(); if (bounds.getNewPosition() != null) { bae.setPosition(GraphUtil.copy(bounds.getNewPosition())); } @@ -90,13 +148,12 @@ public static Optional applyBounds(final ElementAndBounds bounds, * * @param alignment The new alignment. * @param index The model index. - * @return The changed element. + * @return The changed element, or empty if the alignment could not be applied to any element. */ public static Optional applyAlignment(final ElementAndAlignment alignment, final GModelIndex index) { - GModelElement element = getOrThrow(index.get(alignment.getElementId()), - "Model element not found! ID: " + alignment.getElementId()); - if (element instanceof GAlignable) { - GAlignable alignable = (GAlignable) element; + Optional element = index.get(alignment.getElementId()); + if (element.isPresent() && element.get() instanceof GAlignable) { + GAlignable alignable = (GAlignable) element.get(); alignable.setAlignment(alignment.getNewAlignment()); return Optional.of(alignable); } @@ -104,22 +161,25 @@ public static Optional applyAlignment(final ElementAndAlignment alig } /** - * Applies the new route to the model. + * Applies the new route to the model. A route needs at least a source and a target point to describe an edge. * * @param route The new route. * @param index The model index. - * @return The changed element. + * @return The changed edge, or empty if the route could not be applied to any edge. */ - public static GEdge applyRoute(final ElementAndRoutingPoints route, final GModelIndex index) { + public static Optional applyRoute(final ElementAndRoutingPoints route, final GModelIndex index) { List routingPoints = route.getNewRoutingPoints(); - if (routingPoints.size() < 2) { - throw new GLSPServerException("Invalid Route!"); + Optional edge = index.findElementByClass(route.getElementId(), GEdge.class); + if (edge.isEmpty() || routingPoints == null || routingPoints.size() < 2) { + return Optional.empty(); } + EList edgeRoutingPoints = edge.get().getRoutingPoints(); + edgeRoutingPoints.clear(); + edgeRoutingPoints.addAll(routingPoints); // first and last point mark the source and target point - GEdge edge = applyRoutingPoints(route, index); - EList edgeRoutingPoints = edge.getRoutingPoints(); - edge.getArgs().put(GArguments.KEY_EDGE_SOURCE_POINT, edgeRoutingPoints.remove(0)); - edge.getArgs().put(GArguments.KEY_EDGE_TARGET_POINT, edgeRoutingPoints.remove(edgeRoutingPoints.size() - 1)); + edge.get().getArgs().put(GArguments.KEY_EDGE_SOURCE_POINT, edgeRoutingPoints.remove(0)); + edge.get().getArgs().put(GArguments.KEY_EDGE_TARGET_POINT, + edgeRoutingPoints.remove(edgeRoutingPoints.size() - 1)); return edge; } diff --git a/tests/org.eclipse.glsp.server.test/src/org/eclipse/glsp/server/features/core/model/ComputedBoundsActionHandlerTest.java b/tests/org.eclipse.glsp.server.test/src/org/eclipse/glsp/server/features/core/model/ComputedBoundsActionHandlerTest.java new file mode 100644 index 00000000..12e251e2 --- /dev/null +++ b/tests/org.eclipse.glsp.server.test/src/org/eclipse/glsp/server/features/core/model/ComputedBoundsActionHandlerTest.java @@ -0,0 +1,132 @@ +/******************************************************************************** + * Copyright (c) 2026 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.features.core.model; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; + +import org.eclipse.glsp.graph.GEdge; +import org.eclipse.glsp.graph.GGraph; +import org.eclipse.glsp.graph.GModelIndex; +import org.eclipse.glsp.graph.GNode; +import org.eclipse.glsp.graph.GPoint; +import org.eclipse.glsp.graph.GraphFactory; +import org.eclipse.glsp.graph.util.GraphUtil; +import org.eclipse.glsp.server.model.DefaultGModelState; +import org.eclipse.glsp.server.model.GModelState; +import org.eclipse.glsp.server.types.ElementAndAlignment; +import org.eclipse.glsp.server.types.ElementAndBounds; +import org.eclipse.glsp.server.types.ElementAndRoutingPoints; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class ComputedBoundsActionHandlerTest { + + private static final String NODE_ID = "node0"; + private static final String EDGE_ID = "edge0"; + + /** Records which per-kind methods the apply step reached, and drops the routes. */ + private static class TestHandler extends ComputedBoundsActionHandler { + private final List applied = new ArrayList<>(); + + TestHandler(final GModelState state) { + this.modelState = state; + } + + void apply(final GGraph root, final ComputedBoundsAction action) { + applyBounds(root, action); + } + + @Override + protected void applyElementBounds(final List allBounds, final GModelIndex index) { + applied.add("bounds"); + super.applyElementBounds(allBounds, index); + } + + @Override + protected void applyAlignments(final List alignments, final GModelIndex index) { + applied.add("alignments"); + super.applyAlignments(alignments, index); + } + + @Override + protected void applyRoutes(final List routes, final GModelIndex index) { + applied.add("routes"); + // deliberately not delegating, an adopter may drop what the client reported + } + } + + private GGraph graph; + private GNode node; + private GEdge edge; + private TestHandler handler; + + @BeforeEach + void setUpGraph() { + graph = GraphFactory.eINSTANCE.createGGraph(); + graph.setId("graphId"); + graph.setRevision(1); + + node = GraphFactory.eINSTANCE.createGNode(); + node.setId(NODE_ID); + node.setSize(GraphUtil.dimension(1, 1)); + + GNode target = GraphFactory.eINSTANCE.createGNode(); + target.setId("node1"); + + edge = GraphFactory.eINSTANCE.createGEdge(); + edge.setId(EDGE_ID); + edge.setSourceId(NODE_ID); + edge.setTargetId(target.getId()); + + graph.getChildren().add(node); + graph.getChildren().add(target); + graph.getChildren().add(edge); + + DefaultGModelState modelState = new DefaultGModelState(); + modelState.updateRoot(graph); + handler = new TestHandler(modelState); + } + + private ComputedBoundsAction computedBounds() { + ElementAndBounds bounds = new ElementAndBounds(entry -> { + entry.setElementId(NODE_ID); + entry.setNewSize(GraphUtil.dimension(10, 20)); + }); + // three points, so a routing point survives the source and target being split off + List route = List.of(GraphUtil.point(0, 0), GraphUtil.point(5, 5), GraphUtil.point(10, 10)); + return new ComputedBoundsAction(List.of(bounds), List.of(), + List.of(new ElementAndRoutingPoints(EDGE_ID, route)), graph.getRevision()); + } + + @Test + void appliesEveryKindOfComputedBounds() { + handler.apply(graph, computedBounds()); + + assertEquals(List.of("bounds", "alignments", "routes"), handler.applied); + } + + @Test + void letsAnOverrideReplaceASingleKind() { + handler.apply(graph, computedBounds()); + + assertEquals(10, node.getSize().getWidth()); + assertTrue(edge.getRoutingPoints().isEmpty()); + } +} diff --git a/tests/org.eclipse.glsp.server.test/src/org/eclipse/glsp/server/utils/LayoutUtilTest.java b/tests/org.eclipse.glsp.server.test/src/org/eclipse/glsp/server/utils/LayoutUtilTest.java new file mode 100644 index 00000000..d6eaa15b --- /dev/null +++ b/tests/org.eclipse.glsp.server.test/src/org/eclipse/glsp/server/utils/LayoutUtilTest.java @@ -0,0 +1,156 @@ +/******************************************************************************** + * Copyright (c) 2026 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.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Optional; + +import org.eclipse.glsp.graph.GEdge; +import org.eclipse.glsp.graph.GGraph; +import org.eclipse.glsp.graph.GModelIndex; +import org.eclipse.glsp.graph.GNode; +import org.eclipse.glsp.graph.GPoint; +import org.eclipse.glsp.graph.GraphFactory; +import org.eclipse.glsp.graph.builder.impl.GArguments; +import org.eclipse.glsp.graph.util.GraphUtil; +import org.eclipse.glsp.server.features.core.model.ComputedBoundsAction; +import org.eclipse.glsp.server.types.ElementAndAlignment; +import org.eclipse.glsp.server.types.ElementAndBounds; +import org.eclipse.glsp.server.types.ElementAndRoutingPoints; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class LayoutUtilTest { + + private static final String NODE_ID = "node0"; + private static final String EDGE_ID = "edge0"; + private static final String UNKNOWN_ID = "does-not-exist"; + + private GGraph graph; + private GNode node; + private GModelIndex index; + + @BeforeEach + void setUpGraph() { + graph = GraphFactory.eINSTANCE.createGGraph(); + graph.setId("graphId"); + graph.setRevision(1); + + node = GraphFactory.eINSTANCE.createGNode(); + node.setId(NODE_ID); + node.setSize(GraphUtil.dimension(1, 1)); + + GNode target = GraphFactory.eINSTANCE.createGNode(); + target.setId("node1"); + + GEdge edge = GraphFactory.eINSTANCE.createGEdge(); + edge.setId(EDGE_ID); + edge.setSourceId(NODE_ID); + edge.setTargetId(target.getId()); + + graph.getChildren().add(node); + graph.getChildren().add(target); + graph.getChildren().add(edge); + + index = GModelIndex.create(graph); + } + + private static ElementAndBounds bounds(final String elementId) { + return new ElementAndBounds(bounds -> { + bounds.setElementId(elementId); + bounds.setNewSize(GraphUtil.dimension(10, 20)); + }); + } + + private static ElementAndAlignment alignment(final String elementId) { + return new ElementAndAlignment(alignment -> { + alignment.setElementId(elementId); + alignment.setNewAlignment(GraphUtil.point(1, 1)); + }); + } + + private static ElementAndRoutingPoints route(final String elementId, final GPoint... points) { + return new ElementAndRoutingPoints(elementId, List.of(points)); + } + + private void assertNodeResized() { + assertEquals(10, node.getSize().getWidth()); + assertEquals(20, node.getSize().getHeight()); + } + + @Test + void appliesTheReportedBounds() { + assertTrue(LayoutUtil.applyBounds(bounds(NODE_ID), index).isPresent()); + assertNodeResized(); + } + + @Test + void reportsBoundsOfAnUnresolvableElementAsNotApplied() { + assertTrue(LayoutUtil.applyBounds(bounds(UNKNOWN_ID), index).isEmpty()); + } + + @Test + void reportsAlignmentOfAnUnresolvableElementAsNotApplied() { + assertTrue(LayoutUtil.applyAlignment(alignment(UNKNOWN_ID), index).isEmpty()); + } + + @Test + void appliesTheReportedRoute() { + GPoint source = GraphUtil.point(0, 0); + GPoint middle = GraphUtil.point(5, 5); + GPoint target = GraphUtil.point(10, 10); + + Optional applied = LayoutUtil.applyRoute(route(EDGE_ID, source, middle, target), index); + + assertTrue(applied.isPresent()); + // the source and target point are moved into the args, only the intermediate points remain as routing points + assertEquals(List.of(middle), applied.get().getRoutingPoints()); + assertEquals(source, applied.get().getArgs().get(GArguments.KEY_EDGE_SOURCE_POINT)); + assertEquals(target, applied.get().getArgs().get(GArguments.KEY_EDGE_TARGET_POINT)); + } + + @Test + void reportsRouteOfAnUnresolvableEdgeAsNotApplied() { + assertTrue( + LayoutUtil.applyRoute(route(UNKNOWN_ID, GraphUtil.point(0, 0), GraphUtil.point(10, 10)), index).isEmpty()); + } + + @Test + void reportsRouteOfAnElementThatIsNoEdgeAsNotApplied() { + assertTrue( + LayoutUtil.applyRoute(route(NODE_ID, GraphUtil.point(0, 0), GraphUtil.point(10, 10)), index).isEmpty()); + } + + @Test + void reportsRouteWithoutSourceAndTargetPointAsNotApplied() { + assertTrue(LayoutUtil.applyRoute(route(EDGE_ID, GraphUtil.point(0, 0)), index).isEmpty()); + } + + @Test + void keepsApplyingComputedBoundsAfterAnEntryThatCannotBeApplied() { + ComputedBoundsAction action = new ComputedBoundsAction(List.of(bounds(UNKNOWN_ID), bounds(NODE_ID)), + List.of(alignment(UNKNOWN_ID)), List.of(route(EDGE_ID, GraphUtil.point(0, 0))), graph.getRevision()); + + LayoutUtil.applyElementBounds(action.getBounds(), index); + LayoutUtil.applyAlignments(action.getAlignments(), index); + LayoutUtil.applyRoutes(action.getRoutes(), index); + + assertNodeResized(); + } +}