GLSP-1116 Revise model loading (eclipse-glsp#287)

* GLSP-1116 Revise model loading - Refactor diagram loader - Remove dispatching of temporary empty set model action and instead call `actionDispatcher.initialize()` earlier which also dispatches an empty set model action under the hood - Add additional `postModelInitalization` hook for startup services that want execute logic after the model is fully initialized - Rework `ModelInitializationConstraint` - Provide `onInitialized` override that allows sync registration of listener callbacks - Refactor `setCompleted` method and remove the possiblity to set the initialized state to false. Model initialization is a one-time action. Once initialized there should be no way to "uninitialize" the constraint - Provide test cases - Add `dispatchOnceModelInitialized` utility function to action dispatcher - Ensure that type hints are requested after the model has been initialized Part-of: eclipse-glsp/glsp#1116 Part-of: eclipse-glsp/glsp#606 * GLSP-1117: Remove need for explicit definition of client actions -Extend `initializeClientSession` request to also specific the set of client action kinds. This way the server knows which actions should be sent to the client -Adapt `GLSPModelSource´ to retrieve the client actions before sending the `initializeClientSession` request - Add customized `GLSPActionHandlerRegistry` which provides a query method to retrieve all handled action kinds Part of eclipse-glsp/glsp/issues/1117 * GLSP-1071: Rename ServerStatus/ServerMessage action Part of eclipse-glsp/glsp#1071
holkerveen · Dec 21, 2024 · c0e6a7d · c0e6a7d
1 parent d96a09c
commit c0e6a7d
Show file tree

Hide file tree

Showing 28 changed files with 371 additions and 200 deletions.
diff --git a/examples/workflow-standalone/src/app.ts b/examples/workflow-standalone/src/app.ts
@@ -21,8 +21,8 @@ import {
     GLSPActionDispatcher,
     GLSPClient,
     GLSPWebSocketProvider,
-    ServerMessageAction,
-    ServerStatusAction
+    MessageAction,
+    StatusAction
 } from '@eclipse-glsp/client';
 import { Container } from 'inversify';
 import { join, resolve } from 'path';
@@ -55,10 +55,7 @@ async function initialize(connectionProvider: MessageConnection, isReconnecting
         const message = `Connection to the ${id} glsp server got closed. Connection was successfully re-established.`;
         const timeout = 5000;
         const severity = 'WARNING';
-        actionDispatcher.dispatchAll([
-            ServerStatusAction.create(message, { severity, timeout }),
-            ServerMessageAction.create(message, { severity })
-        ]);
+        actionDispatcher.dispatchAll([StatusAction.create(message, { severity, timeout }), MessageAction.create(message, { severity })]);
         return;
     }
 }

diff --git a/packages/client/src/base/action-dispatcher.ts b/packages/client/src/base/action-dispatcher.ts
@@ -15,14 +15,15 @@
  ********************************************************************************/
 import { inject, injectable } from 'inversify';
 import { Action, ActionDispatcher, RequestAction, ResponseAction } from '~glsp-sprotty';
-import { ModelInitializationConstraint } from './model-initialization-constraint';
+import { ModelInitializationConstraint } from './model/model-initialization-constraint';
 
 @injectable()
 export class GLSPActionDispatcher extends ActionDispatcher {
     protected readonly timeouts: Map<string, NodeJS.Timeout> = new Map();
     protected initializedConstraint = false;
 
-    @inject(ModelInitializationConstraint) protected initializationConstraint: ModelInitializationConstraint;
+    @inject(ModelInitializationConstraint)
+    protected initializationConstraint: ModelInitializationConstraint;
 
     override initialize(): Promise<void> {
         return super.initialize().then(() => this.startModelInitialization());
@@ -31,7 +32,7 @@ export class GLSPActionDispatcher extends ActionDispatcher {
     startModelInitialization(): void {
         if (!this.initializedConstraint) {
             this.logger.log(this, 'Starting model initialization mode');
-            this.initializationConstraint.onInitialized().then(() => this.logger.log(this, 'Model initialization completed'));
+            this.initializationConstraint.onInitialized(() => this.logger.log(this, 'Model initialization completed'));
             this.initializedConstraint = true;
         }
     }
@@ -44,6 +45,15 @@ export class GLSPActionDispatcher extends ActionDispatcher {
         return this.actionHandlerRegistry.get(action.kind).length > 0;
     }
 
+    /**
+     * Processes all given actions, by dispatching them to the corresponding handlers, after the model initialization is completed.
+     *
+     * @param actions The actions that should be dispatched after the model initialization
+     */
+    dispatchOnceModelInitialized(...actions: Action[]): void {
+        this.initializationConstraint.onInitialized(() => this.dispatchAll(actions));
+    }
+
     override dispatch(action: Action): Promise<void> {
         const result = super.dispatch(action);
         this.initializationConstraint.notifyDispatched(action);

diff --git a/packages/client/src/base/action-handler-registry.ts b/packages/client/src/base/action-handler-registry.ts
@@ -0,0 +1,30 @@
+/********************************************************************************
+ * 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
+ * http://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
+ ********************************************************************************/
+
+import { injectable } from 'inversify';
+import { ActionHandlerRegistry } from '~glsp-sprotty';
+
+@injectable()
+export class GLSPActionHandlerRegistry extends ActionHandlerRegistry {
+    /**
+     * Retrieve a set of all action kinds for which (at least) one
+     * handler is registered
+     * @returns the set of handled action kinds
+     */
+    getHandledActionKinds(): string[] {
+        return Array.from(this.elements.keys());
+    }
+}
diff --git a/packages/client/src/base/default.module.ts b/packages/client/src/base/default.module.ts
@@ -15,6 +15,7 @@
  ********************************************************************************/
 import '@vscode/codicons/dist/codicon.css';
 import {
+    ActionHandlerRegistry,
     FeatureModule,
     KeyTool,
     LocationPostprocessor,
@@ -31,16 +32,17 @@ import {
 } from '~glsp-sprotty';
 import '../../css/glsp-sprotty.css';
 import { GLSPActionDispatcher } from './action-dispatcher';
+import { GLSPActionHandlerRegistry } from './action-handler-registry';
 import { GLSPCommandStack } from './command-stack';
 import { EditorContextService } from './editor-context-service';
 import { ModifyCssFeedbackCommand } from './feedback/css-feedback';
 import { FeedbackActionDispatcher } from './feedback/feedback-action-dispatcher';
 import { FeedbackAwareUpdateModelCommand } from './feedback/update-model-command';
 import { FocusStateChangedAction } from './focus/focus-state-change-action';
 import { FocusTracker } from './focus/focus-tracker';
-import { DefaultModelInitializationConstraint, ModelInitializationConstraint } from './model-initialization-constraint';
 import { DiagramLoader } from './model/diagram-loader';
 import { GLSPModelSource } from './model/glsp-model-source';
+import { DefaultModelInitializationConstraint, ModelInitializationConstraint } from './model/model-initialization-constraint';
 import { GLSPModelRegistry } from './model/model-registry';
 import { SelectionClearingMouseListener } from './selection-clearing-mouse-listener';
 import { SelectionService } from './selection-service';
@@ -91,6 +93,8 @@ export const defaultModule = new FeatureModule((bind, unbind, isBound, rebind, .
     bindOrRebind(context, TYPES.ICommandStack).to(GLSPCommandStack).inSingletonScope();
     bind(GLSPActionDispatcher).toSelf().inSingletonScope();
     bindOrRebind(context, TYPES.IActionDispatcher).toService(GLSPActionDispatcher);
+    bind(GLSPActionHandlerRegistry).toSelf().inSingletonScope();
+    bindOrRebind(context, ActionHandlerRegistry).toService(GLSPActionHandlerRegistry);
 
     bindAsService(context, TYPES.ModelSource, GLSPModelSource);
     bind(DiagramLoader).toSelf().inSingletonScope();

diff --git a/packages/client/src/base/model/diagram-loader.ts b/packages/client/src/base/model/diagram-loader.ts
@@ -24,14 +24,15 @@ import {
     InitializeParameters,
     MaybePromise,
     RequestModelAction,
-    ServerStatusAction,
     SetModelAction,
+    StatusAction,
     TYPES,
     hasNumberProp
 } from '~glsp-sprotty';
 import { GLSPActionDispatcher } from '../action-dispatcher';
 import { Ranked } from '../ranked';
 import { GLSPModelSource } from './glsp-model-source';
+import { ModelInitializationConstraint } from './model-initialization-constraint';
 
 /**
  * Configuration options for a specific GLSP diagram instance.
@@ -79,17 +80,24 @@ export interface IDiagramStartup extends Partial<Ranked> {
      * Hook for services that should be executed after the initial model loading request (i.e. `RequestModelAction`).
      * Note that this hook is invoked directly after the `RequestModelAction` has been dispatched. It does not necessarily wait
      * until the client-server update roundtrip is completed. If you need to wait until the diagram is fully initialized use the
-     * {@link GLSPActionDispatcher.onceModelInitialized} constraint.
+     * {@link postModelInitialization} hook.
      */
     postRequestModel?(): MaybePromise<void>;
+    /* Hook for services that should be executed after the diagram model is fully initialized
+     * (i.e. `ModelInitializationConstraint` is completed).
+     */
+    postModelInitialization?(): MaybePromise<void>;
 }
 
 export namespace IDiagramStartup {
     export function is(object: unknown): object is IDiagramStartup {
         return (
             AnyObject.is(object) &&
             hasNumberProp(object, 'rank', true) &&
-            ('preInitialize' in object || 'preRequestModel' in object || 'postRequestModel' in object)
+            ('preInitialize' in object ||
+                'preRequestModel' in object ||
+                'postRequestModel' in object ||
+                'postModelInitialization' in object)
         );
     }
 }
@@ -142,6 +150,9 @@ export class DiagramLoader {
     @inject(GLSPModelSource)
     protected modelSource: GLSPModelSource;
 
+    @inject(ModelInitializationConstraint)
+    protected modelInitializationConstraint: ModelInitializationConstraint;
+
     @postConstruct()
     protected postConstruct(): void {
         this.diagramStartups.sort((a, b) => Ranked.getRank(a) - Ranked.getRank(b));
@@ -161,13 +172,15 @@ export class DiagramLoader {
             },
             enableNotifications: options.enableNotifications ?? true
         };
+        await this.actionDispatcher.initialize();
         // Set placeholder model until real model from server is available
         await this.actionDispatcher.dispatch(SetModelAction.create(EMPTY_ROOT));
         await this.invokeStartupHook('preInitialize');
         await this.initialize(resolvedOptions);
         await this.invokeStartupHook('preRequestModel');
         await this.requestModel(resolvedOptions);
         await this.invokeStartupHook('postRequestModel');
+        this.modelInitializationConstraint.onInitialized(() => this.invokeStartupHook('postModelInitialization'));
     }
 
     protected async invokeStartupHook(hook: keyof Omit<IDiagramStartup, 'rank'>): Promise<void> {
@@ -176,18 +189,13 @@ export class DiagramLoader {
         }
     }
 
-    protected requestModel(options: ResolvedDiagramLoadingOptions): Promise<void> {
-        const result = this.actionDispatcher.dispatch(RequestModelAction.create({ options: options.requestModelOptions }));
-        if (options.enableNotifications) {
-            this.actionDispatcher.dispatch(ServerStatusAction.create('', { severity: 'NONE' }));
-        }
-        return result;
+    protected async requestModel(options: ResolvedDiagramLoadingOptions): Promise<void> {
+        return this.actionDispatcher.dispatch(RequestModelAction.create({ options: options.requestModelOptions }));
     }
 
     protected async initialize(options: ResolvedDiagramLoadingOptions): Promise<void> {
-        await this.actionDispatcher.initialize();
         if (options.enableNotifications) {
-            this.actionDispatcher.dispatch(ServerStatusAction.create('Initializing...', { severity: 'INFO' }));
+            this.actionDispatcher.dispatch(StatusAction.create('Initializing...', { severity: 'INFO' }));
         }
 
         const glspClient = await this.options.glspClientProvider();
@@ -197,5 +205,9 @@ export class DiagramLoader {
         if (!glspClient.initializeResult) {
             await glspClient.initializeServer(options.initializeParameters);
         }
+
+        if (options.enableNotifications) {
+            this.actionDispatcher.dispatch(StatusAction.create('', { severity: 'NONE' }));
+        }
     }
 }
diff --git a/packages/client/src/base/model/glsp-model-source.ts b/packages/client/src/base/model/glsp-model-source.ts
@@ -17,7 +17,6 @@
 import { inject, injectable, postConstruct, preDestroy } from 'inversify';
 import {
     Action,
-    ActionHandlerRegistry,
     ActionMessage,
     Disposable,
     DisposableCollection,
@@ -28,6 +27,7 @@ import {
     SModelRootSchema,
     TYPES
 } from '~glsp-sprotty';
+import { GLSPActionHandlerRegistry } from '../action-handler-registry';
 import { IDiagramOptions } from './diagram-loader';
 /**
  * A helper interface that allows the client to mark actions that have been received from the server.
@@ -91,14 +91,23 @@ export class GLSPModelSource extends ModelSource implements Disposable {
         this.clientId = this.options.clientId ?? this.viewerOptions.baseDiv;
     }
 
-    configure(registry: ActionHandlerRegistry, initializeResult: InitializeResult): Promise<void> {
+    configure(registry: GLSPActionHandlerRegistry, initializeResult: InitializeResult): Promise<void> {
         const serverActions = initializeResult.serverActions[this.diagramType];
         if (!serverActions || serverActions.length === 0) {
             throw new Error(`No server-handled actions could be derived from the initialize result for diagramType: ${this.diagramType}!`);
         }
+        // Retrieve all currently handled action kinds. We do this before registering the server actions
+        // to ensure that the array will only contain client-side handled actions
+        const clientActionKinds = registry.getHandledActionKinds();
+
         serverActions.forEach(action => registry.register(action, this));
         this.toDispose.push(this.glspClient!.onActionMessage(message => this.messageReceived(message), this.clientId));
-        return this.glspClient!.initializeClientSession({ clientSessionId: this.clientId, diagramType: this.diagramType });
+
+        return this.glspClient!.initializeClientSession({
+            clientSessionId: this.clientId,
+            clientActionKinds,
+            diagramType: this.diagramType
+        });
     }
 
     protected messageReceived(message: ActionMessage): void {
@@ -111,7 +120,7 @@ export class GLSPModelSource extends ModelSource implements Disposable {
         this.actionDispatcher.dispatch(action);
     }
 
-    override initialize(registry: ActionHandlerRegistry): void {
+    override initialize(registry: GLSPActionHandlerRegistry): void {
         // Registering actions here is discouraged and it's recommended
         // to implemented dedicated action handlers.
         if (!this.clientId) {

diff --git a/packages/client/src/base/model/model-initialization-constraint.spec.ts b/packages/client/src/base/model/model-initialization-constraint.spec.ts
@@ -0,0 +1,95 @@
+/********************************************************************************
+ * 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
+ * http://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
+ ********************************************************************************/
+import { expect } from 'chai';
+import { Container } from 'inversify';
+import 'reflect-metadata';
+import * as sinon from 'sinon';
+import { Deferred, EMPTY_ROOT, InitializeCanvasBoundsAction, SetModelAction, UpdateModelAction } from '~glsp-sprotty';
+import { DefaultModelInitializationConstraint, ModelInitializationConstraint } from './model-initialization-constraint';
+const sandbox = sinon.createSandbox();
+const container = new Container();
+let constraint: ModelInitializationConstraint;
+// eslint-disable-next-line @typescript-eslint/no-empty-function
+const listener = sandbox.spy((): void => {});
+
+describe('DefaultModelInitializationConstraint', () => {
+    beforeEach(() => {
+        constraint = container.resolve(DefaultModelInitializationConstraint);
+        sandbox.reset();
+    });
+    it('should complete after dispatching  non empty SetModelAction and `InitializeCanvasBoundsAction`', () => {
+        expect(constraint.isCompleted).to.be.false;
+        constraint.notifyDispatched(SetModelAction.create({ id: 'model', type: 'graph' }));
+        expect(constraint.isCompleted).to.be.false;
+        constraint.notifyDispatched({ kind: InitializeCanvasBoundsAction.KIND });
+        expect(constraint.isCompleted).to.be.true;
+    });
+    it('should complete after dispatching non empty UpdateModelAction and `InitializeCanvasBoundsAction`', () => {
+        expect(constraint.isCompleted).to.be.false;
+        constraint.notifyDispatched(UpdateModelAction.create({ id: 'model', type: 'graph' }));
+        expect(constraint.isCompleted).to.be.false;
+        constraint.notifyDispatched({ kind: InitializeCanvasBoundsAction.KIND });
+        expect(constraint.isCompleted).to.be.true;
+    });
+    it('should note complete after dispatching empty SetModelAction and `InitializeCanvasBoundsAction` ', () => {
+        expect(constraint.isCompleted).to.be.false;
+        constraint.notifyDispatched(SetModelAction.create(EMPTY_ROOT));
+        expect(constraint.isCompleted).to.be.false;
+        constraint.notifyDispatched({ kind: InitializeCanvasBoundsAction.KIND });
+        expect(constraint.isCompleted).to.be.false;
+    });
+    it('should note complete after dispatching  empty UpdateModelAction and `InitializeCanvasBoundsAction ', () => {
+        expect(constraint.isCompleted).to.be.false;
+        constraint.notifyDispatched(UpdateModelAction.create(EMPTY_ROOT));
+        expect(constraint.isCompleted).to.be.false;
+        constraint.notifyDispatched({ kind: InitializeCanvasBoundsAction.KIND });
+        expect(constraint.isCompleted).to.be.false;
+    });
+    describe('onInitialized', () => {
+        it('returned promise should resolve once the constraint is initialized', async () => {
+            const initializeDeferred = new Deferred<void>();
+            const initializePromise = constraint.onInitialized();
+            initializePromise.then(() => initializeDeferred.resolve());
+            expect(initializeDeferred.state).to.be.equal('unresolved');
+            // Directly trigger the completion method simplify test logic
+            constraint['setCompleted']();
+            // Short delay of test execution to ensure that the deferred state is updated.
+            await new Promise(resolve => setTimeout(resolve, 5));
+            expect(initializeDeferred.state).to.be.equal('resolved');
+        });
+        it('registered listener should be invoked once the constraint is initialized', () => {
+            constraint.onInitialized(listener);
+            expect(listener.called).to.be.false;
+            // Directly trigger the completion method simplify test logic
+            constraint['setCompleted']();
+            expect(listener.called).to.be.true;
+        });
+        it('registered listener should be invoked directly on registration if the constraint is already initialized', () => {
+            // Directly trigger the completion method simplify test logic
+            constraint['setCompleted']();
+            constraint.onInitialized(listener);
+            expect(listener.called).to.be.true;
+        });
+        it('Disposed listener should not be invoked once the constraint is initialized', () => {
+            const toDispose = constraint.onInitialized(listener);
+            expect(listener.called).to.be.false;
+            toDispose.dispose();
+            // Directly trigger the completion method simplify test logic
+            constraint['setCompleted']();
+            expect(listener.called).to.be.false;
+        });
+    });
+});