GLSP-1727: Keep applying computed bounds when an entry cannot be applied

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<GEdge>` 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

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<Action> 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.
+    *
+    * <p>
+    * 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.
+    * </p>
+    *
+    * @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<ElementAndBounds> 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<ElementAndAlignment> 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<ElementAndRoutingPoints> routes, final GModelIndex index) {
+      LayoutUtil.applyRoutes(routes, index);
+   }
+
 }

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,35 +47,91 @@
 
 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.
     *
+    * <p>
+    * An entry that cannot be applied is skipped and logged rather than treated as an error.
+    * </p>
+    *
     * @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<ElementAndBounds> 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<ElementAndAlignment> 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.
+    *
+    * <p>
+    * A skipped route is logged at debug level, an edge the client has not finished routing yet is expected.
+    * </p>
+    *
+    * @param routes The new routes.
+    * @param index  The model index.
+    */
+   public static void applyRoutes(final List<ElementAndRoutingPoints> routes, final GModelIndex index) {
+      routes.forEach(route -> {
+         if (applyRoute(route, index).isEmpty()) {
+            LOGGER.debug("Skipped computed route of element '" + route.getElementId() + "'");
+         }
+      });
    }
 
    /**
     * Applies the new bounds to the model.
     *
     * @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<GBoundsAware> 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<GModelElement> 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,36 +148,38 @@ public static Optional<GBoundsAware> 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<GAlignable> 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<GModelElement> 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);
       }
       return Optional.empty();
    }
 
    /**
-    * 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<GEdge> applyRoute(final ElementAndRoutingPoints route, final GModelIndex index) {
       List<GPoint> routingPoints = route.getNewRoutingPoints();
-      if (routingPoints.size() < 2) {
-         throw new GLSPServerException("Invalid Route!");
+      Optional<GEdge> edge = index.findElementByClass(route.getElementId(), GEdge.class);
+      if (edge.isEmpty() || routingPoints == null || routingPoints.size() < 2) {
+         return Optional.empty();
       }
+      EList<GPoint> 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<GPoint> 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;
    }
 

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<String> 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<ElementAndBounds> allBounds, final GModelIndex index) {
+         applied.add("bounds");
+         super.applyElementBounds(allBounds, index);
+      }
+
+      @Override
+      protected void applyAlignments(final List<ElementAndAlignment> alignments, final GModelIndex index) {
+         applied.add("alignments");
+         super.applyAlignments(alignments, index);
+      }
+
+      @Override
+      protected void applyRoutes(final List<ElementAndRoutingPoints> 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<GPoint> 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());
+   }
+}