Avoid executing unnecessary operations (#105)

- Filter empty change bounds and change routing points operations
- Filter empty compound commands
- Ensure we only submit a new model if actual changes were applied

Relates to  eclipse-glsp/glsp#1477

Author: martin-fleck-at

packages/server/src/common/gmodel/change-bounds-operation-handler.ts

@@ -13,8 +13,8 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { GModelRoot, GNode } from '@eclipse-glsp/graph';
-import { ChangeBoundsOperation, Dimension, MaybePromise, Point } from '@eclipse-glsp/protocol';
+import { GModelRoot, GNode, isGBoundsAware } from '@eclipse-glsp/graph';
+import { ChangeBoundsOperation, Dimension, ElementAndBounds, MaybePromise, Point } from '@eclipse-glsp/protocol';
 import { injectable } from 'inversify';
 import { Command } from '../command/command';
 import { GModelOperationHandler } from './gmodel-operation-handler';
@@ -27,7 +27,23 @@ export class GModelChangeBoundsOperationHandler extends GModelOperationHandler {
     operationType = ChangeBoundsOperation.KIND;
 
     createCommand(operation: ChangeBoundsOperation): MaybePromise<Command | undefined> {
-        return this.commandOf(() => this.executeChangeBounds(operation));
+        const newBounds = operation.newBounds.filter(element => this.hasChanged(element));
+        if (newBounds.length === 0) {
+            return undefined;
+        }
+        return this.commandOf(() => this.executeChangeBounds({ ...operation, newBounds }));
+    }
+
+    protected hasChanged(element: ElementAndBounds): boolean {
+        const knownElement = this.modelState.index.find(element.elementId);
+        if (!knownElement || !isGBoundsAware(knownElement)) {
+            return true;
+        }
+        const sizeChanged = knownElement.size ? !Dimension.equals(knownElement.size, element.newSize) : true;
+        if (sizeChanged) {
+            return true;
+        }
+        return knownElement.position && element.newPosition ? !Point.equals(knownElement.position, element.newPosition) : true;
     }
 
     protected executeChangeBounds(operation: ChangeBoundsOperation): void {

packages/server/src/common/gmodel/change-routing-points-operation-handler.ts

@@ -14,10 +14,10 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import { ChangeRoutingPointsOperation, MaybePromise } from '@eclipse-glsp/protocol';
+import { GEdge } from '@eclipse-glsp/graph';
+import { ChangeRoutingPointsOperation, ElementAndRoutingPoints, MaybePromise, Point } from '@eclipse-glsp/protocol';
 import { injectable } from 'inversify';
 import { Command } from '../command/command';
-import { GLSPServerError } from '../utils/glsp-server-error';
 import { applyRoutingPoints } from '../utils/layout-util';
 import { GModelOperationHandler } from './gmodel-operation-handler';
 
@@ -26,14 +26,27 @@ export class GModelChangeRoutingPointsOperationHandler extends GModelOperationHa
     operationType = ChangeRoutingPointsOperation.KIND;
 
     createCommand(operation: ChangeRoutingPointsOperation): MaybePromise<Command | undefined> {
-        return this.commandOf(() => this.executeChangeRoutingPoints(operation));
+        const newRoutingPoints = operation.newRoutingPoints.filter(newRoutingPoints => this.hasChanged(newRoutingPoints));
+        if (newRoutingPoints.length === 0) {
+            return undefined;
+        }
+        return this.commandOf(() => this.executeChangeRoutingPoints({ ...operation, newRoutingPoints }));
     }
 
-    executeChangeRoutingPoints(operation: ChangeRoutingPointsOperation): MaybePromise<void> {
-        if (!operation.newRoutingPoints) {
-            throw new GLSPServerError('Incomplete change routingPoints  action');
+    protected hasChanged(element: ElementAndRoutingPoints): boolean {
+        const knownElement = this.modelState.index.findByClass(element.elementId, GEdge);
+        if (!knownElement || knownElement.routingPoints.length !== element.newRoutingPoints?.length) {
+            return true;
+        }
+        for (let i = 0; i < knownElement.routingPoints.length; i++) {
+            if (!Point.equals(knownElement.routingPoints[i], element.newRoutingPoints[i])) {
+                return true;
+            }
         }
+        return false;
+    }
 
+    executeChangeRoutingPoints(operation: ChangeRoutingPointsOperation): MaybePromise<void> {
         const index = this.modelState.index;
         operation.newRoutingPoints.forEach(routingPoints => applyRoutingPoints(routingPoints, index));
     }

packages/server/src/common/operations/compound-operation-handler.ts

@@ -40,6 +40,6 @@ export class CompoundOperationHandler extends OperationHandler {
                 commands.push(command);
             }
         }
-        return new CompoundCommand(commands);
+        return commands.length > 0 ? new CompoundCommand(commands) : undefined;
     }
 }

packages/server/src/common/operations/operation-action-handler.ts

@@ -67,8 +67,9 @@ export class OperationActionHandler implements ActionHandler {
         const command = await handler.execute(operation);
         if (command) {
             await this.executeCommand(command);
+            return this.submitModel();
         }
-        return this.modelSubmissionHandler.submitModel('operation');
+        return [];
     }
 
     protected async executeCommand(command: Command): Promise<void> {