GLSP-891: Json operation handler API (#59)

- Provides  generic,reusable operation handler implementations for JSON-based source models
- Refactor `RecordingCommand` API
- Refactor `CreationOperationHandler` interfaces to be more strict (Previously it was possible to 
  declare a handler with mixed (nodes/edges) types)
Also: Filter out non-critical warning in webpack config
Fixes eclipse-glsp/glsp#891

Author: tortmayr

examples/workflow-server/webpack.config.js

@@ -57,6 +57,6 @@ module.exports = env => {
                 }
             ]
         },
-        ignoreWarnings: [/Failed to parse source map/]
+        ignoreWarnings: [/Failed to parse source map/, /Can't resolve .* in '.*ws\/lib'/]
     };
 };

packages/server/src/common/command/recording-command.spec.ts

@@ -14,9 +14,9 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import { AnyObject } from '@eclipse-glsp/protocol';
+import { AnyObject, MaybePromise } from '@eclipse-glsp/protocol';
 import { expect } from 'chai';
-import { RecordingCommand } from './recording-command';
+import { AbstractRecordingCommand } from './recording-command';
 
 interface TestModel {
     string: string;
@@ -27,6 +27,16 @@ interface TestModel {
 
 let jsonObject: TestModel;
 
+class TestRecordingCommand<JsonObject extends AnyObject = AnyObject> extends AbstractRecordingCommand<JsonObject> {
+    constructor(protected jsonObject: JsonObject, protected doExecute: () => MaybePromise<void>) {
+        super();
+    }
+
+    protected getJsonObject(): MaybePromise<JsonObject> {
+        return this.jsonObject;
+    }
+}
+
 describe('RecordingCommand', () => {
     let beforeState: TestModel;
 
@@ -41,14 +51,14 @@ describe('RecordingCommand', () => {
 
     it('should be undoable after execution', async () => {
         // eslint-disable-next-line @typescript-eslint/no-empty-function
-        const command = new RecordingCommand(jsonObject, () => {});
+        const command = new TestRecordingCommand(jsonObject, () => {});
         expect(command.canUndo()).to.be.false;
         await command.execute();
         expect(command.canUndo()).to.be.true;
     });
 
     it('should restore the pre execution state when undo is called', async () => {
-        const command = new RecordingCommand(jsonObject, () => {
+        const command = new TestRecordingCommand(jsonObject, () => {
             jsonObject.string = 'bar';
             jsonObject.flag = false;
             jsonObject.maybe = { hello: 'world' };
@@ -60,7 +70,7 @@ describe('RecordingCommand', () => {
     });
 
     it('should restore the post execution state when redo is called', async () => {
-        const command = new RecordingCommand(jsonObject, () => {
+        const command = new TestRecordingCommand(jsonObject, () => {
             jsonObject.string = 'bar';
             jsonObject.flag = false;
             jsonObject.maybe = { hello: 'world' };

packages/server/src/common/command/recording-command.ts

@@ -15,7 +15,7 @@
  ********************************************************************************/
 
 import { GModelRootSchema } from '@eclipse-glsp/graph';
-import { AnyObject, MaybePromise } from '@eclipse-glsp/protocol';
+import { AnyObject, MaybePromise, SModelRootSchema } from '@eclipse-glsp/protocol';
 import * as jsonPatch from 'fast-json-patch';
 import { GModelSerializer } from '../features/model/gmodel-serializer';
 import { ModelState } from '../features/model/model-state';
@@ -34,6 +34,7 @@ export abstract class AbstractRecordingCommand<JsonObject extends AnyObject> imp
         const afterState = await this.getJsonObject();
         this.undoPatch = jsonPatch.compare(afterState, beforeState);
         this.redoPatch = jsonPatch.compare(beforeState, afterState);
+        this.postChange?.(afterState);
     }
 
     /**
@@ -48,8 +49,8 @@ export abstract class AbstractRecordingCommand<JsonObject extends AnyObject> imp
      */
     protected abstract doExecute(): MaybePromise<void>;
 
-    protected applyPatch(object: JsonObject, patch: jsonPatch.Operation[]): MaybePromise<void> {
-        jsonPatch.applyPatch(object, patch, false, true);
+    protected applyPatch(object: JsonObject, patch: jsonPatch.Operation[]): jsonPatch.PatchResult<JsonObject> {
+        return jsonPatch.applyPatch(object, patch, false, true);
     }
 
     /**
@@ -64,33 +65,28 @@ export abstract class AbstractRecordingCommand<JsonObject extends AnyObject> imp
 
     async undo(): Promise<void> {
         if (this.undoPatch) {
-            return this.applyPatch(await this.getJsonObject(), this.undoPatch);
+            const result = this.applyPatch(await this.getJsonObject(), this.undoPatch);
+            this.postChange?.(result.newDocument);
         }
     }
 
     async redo(): Promise<void> {
         if (this.redoPatch) {
-            return this.applyPatch(await this.getJsonObject(), this.redoPatch);
+            const result = this.applyPatch(await this.getJsonObject(), this.redoPatch);
+            this.postChange?.(result.newDocument);
         }
     }
 
     canUndo(): boolean {
         return !!this.undoPatch && !!this.redoPatch;
     }
-}
-
-/**
- * Simple base implementation of {@link AbstractRecordingCommand} that records the changes made to the given JSON object during
- * the the given `doExecute` function.
- */
-export class RecordingCommand<JsonObject extends AnyObject = AnyObject> extends AbstractRecordingCommand<JsonObject> {
-    constructor(protected jsonObject: JsonObject, protected doExecute: () => MaybePromise<void>) {
-        super();
-    }
 
-    protected getJsonObject(): MaybePromise<JsonObject> {
-        return this.jsonObject;
-    }
+    /**
+     * Optional hook that (if implemented) will be
+     * executed after every command-driven action that changed
+     * the underlying model i.e. command execute, undo and redo.
+     */
+    protected postChange?(newModel: JsonObject): MaybePromise<void>;
 }
 
 /**
@@ -102,18 +98,12 @@ export class GModelRecordingCommand extends AbstractRecordingCommand<GModelRootS
         super();
     }
 
-    override async execute(): Promise<void> {
-        await super.execute();
-        this.modelState.index.indexRoot(this.modelState.root);
-    }
-
     protected getJsonObject(): MaybePromise<GModelRootSchema> {
         return this.serializer.createSchema(this.modelState.root);
     }
 
-    protected override async applyPatch(rootSchema: GModelRootSchema, patch: jsonPatch.Operation[]): Promise<void> {
-        await super.applyPatch(rootSchema, patch);
-        const newRoot = this.serializer.createRoot(rootSchema);
+    protected override postChange(newModel: SModelRootSchema): MaybePromise<void> {
+        const newRoot = this.serializer.createRoot(newModel);
         this.modelState.updateRoot(newRoot);
     }
 }

packages/server/src/common/features/model/model-state.ts

@@ -92,6 +92,9 @@ export class DefaultModelState implements ModelState {
     }
 
     updateRoot(newRoot: GModelRoot): void {
+        if (!newRoot.revision && this.root) {
+            newRoot.revision = this.root.revision;
+        }
         this.root = newRoot;
         this.index.indexRoot(newRoot);
     }

packages/server/src/common/features/model/model-submission-handler.ts

@@ -110,9 +110,9 @@ export class ModelSubmissionHandler {
 
         const revision = this.requestModelAction ? 0 : this.modelState.root.revision! + 1;
         this.modelState.root.revision = revision;
-        const root = this.serializeGModel();
 
         if (this.diagramConfiguration.needsClientLayout) {
+            const root = this.serializeGModel();
             return [RequestBoundsAction.create(root), SetDirtyStateAction.create(this.commandStack.isDirty, { reason })];
         }
         return this.submitModelDirectly(reason);

packages/server/src/common/gmodel/gmodel-create-edge-operation-handler.ts

@@ -19,7 +19,7 @@ import { GNode } from '@eclipse-glsp/graph/lib/gnode';
 import { CreateEdgeOperation, MaybePromise, TriggerEdgeCreationAction } from '@eclipse-glsp/protocol';
 import { injectable } from 'inversify';
 import { Command } from '../command/command';
-import { CreateOperationHandler, CreateOperationKind } from '../operations/create-operation-handler';
+import { CreateEdgeOperationHandler } from '../operations/create-operation-handler';
 import { GModelOperationHandler } from './gmodel-operation-handler';
 
 /**
@@ -28,8 +28,8 @@ import { GModelOperationHandler } from './gmodel-operation-handler';
  * (i.e. all operation handlers directly modify the graphical model).
  */
 @injectable()
-export abstract class GModelCreateEdgeOperationHandler extends GModelOperationHandler implements CreateOperationHandler {
-    override readonly operationType: CreateOperationKind = CreateEdgeOperation.KIND;
+export abstract class GModelCreateEdgeOperationHandler extends GModelOperationHandler implements CreateEdgeOperationHandler {
+    override readonly operationType = CreateEdgeOperation.KIND;
     abstract override label: string;
     abstract elementTypeIds: string[];
 

packages/server/src/common/gmodel/gmodel-create-node-operation-handler.ts

@@ -19,7 +19,7 @@ import { CreateNodeOperation, MaybePromise, Point, SelectAction, TriggerNodeCrea
 import { inject, injectable } from 'inversify';
 import { ActionDispatcher } from '../actions/action-dispatcher';
 import { Command } from '../command/command';
-import { CreateOperationHandler, CreateOperationKind } from '../operations/create-operation-handler';
+import { CreateNodeOperationHandler } from '../operations/create-operation-handler';
 import { getRelativeLocation } from '../utils/layout-util';
 import { GModelOperationHandler } from './gmodel-operation-handler';
 
@@ -29,15 +29,15 @@ import { GModelOperationHandler } from './gmodel-operation-handler';
  * (i.e. all operation handlers directly modify the graphical model).
  */
 @injectable()
-export abstract class GModelCreateNodeOperationHandler extends GModelOperationHandler implements CreateOperationHandler {
+export abstract class GModelCreateNodeOperationHandler extends GModelOperationHandler implements CreateNodeOperationHandler {
     @inject(ActionDispatcher)
     protected actionDispatcher: ActionDispatcher;
 
     abstract elementTypeIds: string[];
 
     abstract override label: string;
 
-    override operationType: CreateOperationKind = CreateNodeOperation.KIND;
+    override readonly operationType = CreateNodeOperation.KIND;
 
     override createCommand(operation: CreateNodeOperation): MaybePromise<Command | undefined> {
         return this.commandOf(() => this.executeCreation(operation));

packages/server/src/common/index.ts

@@ -71,6 +71,7 @@ export * from './gmodel/index';
 export * from './launch/glsp-server-launcher';
 export * from './operations/compound-operation-handler';
 export * from './operations/create-operation-handler';
+export * from './operations/json-operation-handler';
 export * from './operations/operation-action-handler';
 export * from './operations/operation-handler';
 export * from './operations/operation-handler-registry';

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

@@ -19,6 +19,11 @@ import { Command, CompoundCommand } from '../command/command';
 import { OperationHandler } from './operation-handler';
 import { OperationHandlerRegistry } from './operation-handler-registry';
 
+/**
+ * Generic {@link OperationHandler} from {@link CompoundOperations}.
+ * Retrieves the corresponding execution commands for the list of (sub) operations
+ * and constructs a {@link CompoundCommand} for them.
+ */
 @injectable()
 export class CompoundOperationHandler extends OperationHandler {
     @inject(OperationHandlerRegistry)
@@ -27,7 +32,7 @@ export class CompoundOperationHandler extends OperationHandler {
     operationType = CompoundOperation.KIND;
 
     async createCommand(operation: CompoundOperation): Promise<Command | undefined> {
-        const maybeCommands = operation.operationList.map(op => this.operationHandlerRegistry.getExecutableCommand(op));
+        const maybeCommands = operation.operationList.map(op => this.operationHandlerRegistry.getOperationHandler(op)?.execute(op));
         const commands: Command[] = [];
 
         for await (const command of maybeCommands) {

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

@@ -19,31 +19,48 @@ import {
     hasArrayProp,
     hasFunctionProp,
     hasStringProp,
+    MaybePromise,
     TriggerEdgeCreationAction,
     TriggerNodeCreationAction
 } from '@eclipse-glsp/protocol';
+import { Command } from '../command/command';
 import { OperationHandler } from './operation-handler';
 
 /**
- * A special {@link OperationHandler} that is responsible for the handling of {@link CreateOperation}s. Depending on its
+ * A special {@link OperationHandler} that is responsible for the handling of (a subset of) {@link CreateEdgeOperation}s. Depending on its
  * operation type the triggered actions are {@link TriggerNodeCreationAction} or {@link TriggerEdgeCreationAction}s.
  */
-export interface CreateOperationHandler extends OperationHandler {
+export interface CreateEdgeOperationHandler extends OperationHandler {
+    label: string;
+    elementTypeIds: string[];
+    operationType: typeof CreateEdgeOperation.KIND;
+    getTriggerActions(): TriggerEdgeCreationAction[];
+    createCommand(operation: CreateEdgeOperation): MaybePromise<Command | undefined>;
+}
+
+export interface CreateNodeOperationHandler extends OperationHandler {
     readonly label: string;
     elementTypeIds: string[];
-    operationType: CreateOperationKind;
-    getTriggerActions(): (TriggerEdgeCreationAction | TriggerNodeCreationAction)[];
+    operationType: typeof CreateNodeOperation.KIND;
+    getTriggerActions(): TriggerNodeCreationAction[];
+    createCommand(operation: CreateNodeOperation): MaybePromise<Command | undefined>;
 }
+/**
+ * A special {@link OperationHandler} that is responsible for the handling of a node or edge creation operation. Depending on its
+ * operation type the triggered actions are {@link TriggerNodeCreationAction} or {@link TriggerEdgeCreationAction}s.
+ */
+export type CreateOperationHandler = CreateNodeOperationHandler | CreateEdgeOperationHandler;
 
 export type CreateOperationKind = typeof CreateNodeOperation.KIND | typeof CreateEdgeOperation.KIND;
 
 export namespace CreateOperationHandler {
     export function is(object: unknown): object is CreateOperationHandler {
         return (
             object instanceof OperationHandler &&
+            hasStringProp(object, 'operationType') &&
             hasStringProp(object, 'label') &&
             hasArrayProp(object, 'elementTypeIds') &&
-            hasFunctionProp(object, 'getTriggerActions')
+            hasFunctionProp(object, 'getTriggerActions', true)
         );
     }
 }