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
eclipse-glsp · Sep 13, 2023 · f4ba969 · f4ba969
1 parent 256d64d
commit f4ba969
Show file tree

Hide file tree

Showing 13 changed files with 239 additions and 104 deletions.
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/default.module.ts b/packages/client/src/base/default.module.ts
@@ -38,9 +38,9 @@ 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';

diff --git a/packages/client/src/base/model/diagram-loader.ts b/packages/client/src/base/model/diagram-loader.ts
@@ -19,19 +19,18 @@ import {
     AnyObject,
     ApplicationIdProvider,
     Args,
-    EMPTY_ROOT,
     GLSPClient,
     InitializeParameters,
     MaybePromise,
     RequestModelAction,
     ServerStatusAction,
-    SetModelAction,
     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 +78,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 +148,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 +170,13 @@ export class DiagramLoader {
             },
             enableNotifications: options.enableNotifications ?? true
         };
-        // Set placeholder model until real model from server is available
-        await this.actionDispatcher.dispatch(SetModelAction.create(EMPTY_ROOT));
+        await this.actionDispatcher.initialize();
         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,16 +185,11 @@ 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' }));
         }
@@ -197,5 +201,9 @@ export class DiagramLoader {
         if (!glspClient.initializeResult) {
             await glspClient.initializeServer(options.initializeParameters);
         }
+
+        if (options.enableNotifications) {
+            this.actionDispatcher.dispatch(ServerStatusAction.create('', { severity: 'NONE' }));
+        }
     }
 }
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;
+        });
+    });
+});
diff --git a/...c/base/model-initialization-constraint.ts → .../model/model-initialization-constraint.ts b/...c/base/model-initialization-constraint.ts → .../model/model-initialization-constraint.ts
@@ -14,7 +14,7 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 import { injectable } from 'inversify';
-import { Action, Deferred, InitializeCanvasBoundsAction, SetModelAction, UpdateModelAction } from '~glsp-sprotty';
+import { Action, Deferred, Disposable, Emitter, InitializeCanvasBoundsAction, SetModelAction, UpdateModelAction } from '~glsp-sprotty';
 
 /**
  * The constraint defining when the initialization of the GLSP model is completed.
@@ -37,32 +37,65 @@ import { Action, Deferred, InitializeCanvasBoundsAction, SetModelAction, UpdateM
 @injectable()
 export abstract class ModelInitializationConstraint {
     protected completion: Deferred<void> = new Deferred();
-    protected completed = false;
 
+    protected _isCompleted = false;
     get isCompleted(): boolean {
-        return this.completed;
+        return this._isCompleted;
     }
 
-    protected setCompleted(isCompleted: boolean): void {
-        this.completed = isCompleted;
-        if (isCompleted) {
-            this.completion.resolve();
+    protected onInitializedEmitter = new Emitter<void>();
+
+    /**
+     * Register a listener that will be invoked once the initialization process
+     * has been completed. If the initialization is already completed on registration
+     * the given listener will be invoked right away
+     * @param listener
+     */
+    onInitialized(listener: () => void): Disposable;
+
+    /**
+     * Retrieve a promise that resolves once the initialization process
+     * has been completed.
+     * @returns the initialization promise
+     */
+    onInitialized(): Promise<void>;
+    onInitialized(listener?: () => void): Promise<void> | Disposable {
+        if (!listener) {
+            return this.completion.promise;
         }
+        if (this.isCompleted) {
+            listener();
+            return Disposable.empty();
+        }
+        return this.onInitializedEmitter.event(listener);
     }
 
-    onInitialized(): Promise<void> {
-        return this.completion.promise;
+    protected setCompleted(): void {
+        if (!this.isCompleted) {
+            this._isCompleted = true;
+            this.completion.resolve();
+            this.onInitializedEmitter.fire();
+            this.onInitializedEmitter.dispose();
+        }
     }
 
     notifyDispatched(action: Action): void {
         if (this.isCompleted) {
             return;
         }
         if (this.isInitializedAfter(action)) {
-            this.setCompleted(true);
+            this.setCompleted();
         }
     }
 
+    /**
+     * Central method to check the initialization state. Is invoked
+     * for every action dispatched by the `ActionDispatcher` (until the initialization has completed).
+     *  Should
+     * return `true` once the action has been passed which marks the end
+     * of the initialization process.
+     * @param action The last dispatched action
+     */
     abstract isInitializedAfter(action: Action): boolean;
 }
 

diff --git a/packages/client/src/features/accessibility/element-navigation/diagram-navigation-tool.ts b/packages/client/src/features/accessibility/element-navigation/diagram-navigation-tool.ts
@@ -86,28 +86,26 @@ export class ElementNavigatorKeyListener extends KeyListener {
     }
 
     registerShortcutKey(): void {
-        this.tool.actionDispatcher.onceModelInitialized().then(() => {
-            this.tool.actionDispatcher.dispatchAll([
-                SetAccessibleKeyShortcutAction.create({
-                    token: this.token,
-                    keys: [
-                        { shortcuts: ['N'], description: 'Activate default navigation', group: 'Navigation', position: 0 },
-                        {
-                            shortcuts: ['ALT', 'N'],
-                            description: 'Activate position based navigation',
-                            group: 'Navigation',
-                            position: 1
-                        },
-                        {
-                            shortcuts: ['⬅  ⬆  ➡  ⬇'],
-                            description: 'Navigate by relation or neighbors according to navigation mode',
-                            group: 'Navigation',
-                            position: 2
-                        }
-                    ]
-                })
-            ]);
-        });
+        this.tool.actionDispatcher.dispatchOnceModelInitialized(
+            SetAccessibleKeyShortcutAction.create({
+                token: this.token,
+                keys: [
+                    { shortcuts: ['N'], description: 'Activate default navigation', group: 'Navigation', position: 0 },
+                    {
+                        shortcuts: ['ALT', 'N'],
+                        description: 'Activate position based navigation',
+                        group: 'Navigation',
+                        position: 1
+                    },
+                    {
+                        shortcuts: ['⬅  ⬆  ➡  ⬇'],
+                        description: 'Navigate by relation or neighbors according to navigation mode',
+                        group: 'Navigation',
+                        position: 2
+                    }
+                ]
+            })
+        );
     }
 
     override keyDown(element: SModelElement, event: KeyboardEvent): Action[] {