GLSP-960: Follow-up

- Fix potential race condition of glsp client retrieval in glsp-model-source/diagram-loader 
  => DIagram loader no retrieves the client and then forwards it to the model source
- Provide `DiagamLoaderOptions` to support dynamic configuration of the loading behavior

Follow-up for eclipse-glsp/glsp/issues/960

examples/workflow-standalone/src/app.ts

@@ -49,7 +49,7 @@ async function initialize(connectionProvider: MessageConnection, isReconnecting
     container = createContainer({ clientId, diagramType, glspClientProvider: async () => glspClient, sourceUri: examplePath });
     const actionDispatcher = container.get(GLSPActionDispatcher);
     const diagramLoader = container.get(DiagramLoader);
-    await diagramLoader.load({ isReconnecting });
+    await diagramLoader.load({ requestModelOptions: { isReconnecting } });
 
     if (isReconnecting) {
         const message = `Connection to the ${id} glsp server got closed. Connection was successfully re-established.`;

packages/client/src/base/model/diagram-loader.ts

@@ -30,6 +30,7 @@ import {
 } from '~glsp-sprotty';
 import { GLSPActionDispatcher } from '../action-dispatcher';
 import { Ranked } from '../ranked';
+import { GLSPModelSource } from './glsp-model-source';
 
 /**
  * Configuration options for a specific GLSP diagram instance.
@@ -92,6 +93,28 @@ export namespace IDiagramStartup {
     }
 }
 
+export interface DiagramLoadingOptions {
+    /**
+     * Optional custom options that should be used the initial {@link RequestModelAction}.
+     * These options will be merged with the default options (`diagramType` and `sourceUri`).
+     * Defaults to an empty object if not defined.
+     */
+    requestModelOptions?: Args;
+
+    /**
+     * Optional arguments that should be attached `initializeServer` request if the underlying
+     * {@link GLSPClient} has not been initialized yet.
+     * Defaults to an empty object if not defined.
+     */
+    initializeParametersArgs?: Args;
+
+    /**
+     * Flag to enable/disable client side notifications during the loading process.
+     * Defaults to `true` if not defined
+     */
+    enableNotifications?: boolean;
+}
+
 /**
  * The central component responsible for initializing the diagram and loading the graphical model
  * from the GLSP server.
@@ -110,20 +133,26 @@ export class DiagramLoader {
     @optional()
     protected diagramStartups: IDiagramStartup[] = [];
 
-    protected enableLoadingNotifications = true;
+    @inject(GLSPModelSource)
+    protected modelSource: GLSPModelSource;
 
     @postConstruct()
     protected postConstruct(): void {
         this.diagramStartups.sort((a, b) => Ranked.getRank(a) - Ranked.getRank(b));
     }
 
-    async load(requestModelOptions: Args = {}): Promise<void> {
+    async load(options: DiagramLoadingOptions = {}): Promise<void> {
+        const resolvedOptions: Required<DiagramLoadingOptions> = {
+            requestModelOptions: options.requestModelOptions ?? {},
+            initializeParametersArgs: options.initializeParametersArgs ?? {},
+            enableNotifications: options.enableNotifications ?? true
+        };
         // Set placeholder model until real model from server is available
         await this.actionDispatcher.dispatch(SetModelAction.create(EMPTY_ROOT));
         await this.invokeStartupHook('preInitialize');
-        await this.configureGLSPClient();
+        await this.initialize(resolvedOptions);
         await this.invokeStartupHook('preRequestModel');
-        await this.requestModel(requestModelOptions);
+        await this.requestModel(resolvedOptions);
         await this.invokeStartupHook('postRequestModel');
     }
 
@@ -133,22 +162,27 @@ export class DiagramLoader {
         }
     }
 
-    protected requestModel(requestModelOptions: Args = {}): Promise<void> {
-        const options = { sourceUri: this.options.sourceUri, diagramType: this.options.diagramType, ...requestModelOptions } as Args;
-        const result = this.actionDispatcher.dispatch(RequestModelAction.create({ options }));
-        if (this.enableLoadingNotifications) {
+    protected requestModel(options: Required<DiagramLoadingOptions>): Promise<void> {
+        const requestOptions = {
+            sourceUri: this.options.sourceUri,
+            diagramType: this.options.diagramType,
+            ...(options.requestModelOptions ?? {})
+        } as Args;
+        const result = this.actionDispatcher.dispatch(RequestModelAction.create({ options: requestOptions }));
+        if (options.enableNotifications) {
             this.actionDispatcher.dispatch(ServerStatusAction.create('', { severity: 'NONE' }));
         }
         return result;
     }
 
-    protected async configureGLSPClient(): Promise<void> {
-        const glspClient = await this.options.glspClientProvider();
-
-        if (this.enableLoadingNotifications) {
+    protected async initialize(options: Required<DiagramLoadingOptions>): Promise<void> {
+        await this.actionDispatcher.initialize();
+        if (options.enableNotifications) {
             this.actionDispatcher.dispatch(ServerStatusAction.create('Initializing...', { severity: 'INFO' }));
         }
 
+        const glspClient = await this.options.glspClientProvider();
+
         await glspClient.start();
 
         if (!glspClient.initializeResult) {
@@ -157,5 +191,7 @@ export class DiagramLoader {
                 protocolVersion: GLSPClient.protocolVersion
             });
         }
+
+        return this.modelSource.connect(glspClient);
     }
 }

packages/client/src/base/model/glsp-model-source.ts

@@ -19,35 +19,34 @@ import {
     Action,
     ActionHandlerRegistry,
     ActionMessage,
+    Deferred,
     Disposable,
     DisposableCollection,
     GLSPClient,
     ILogger,
-    InitializeResult,
     ModelSource,
     SModelRootSchema,
-    TYPES,
-    Writable
+    TYPES
 } from '~glsp-sprotty';
 import { IDiagramOptions } from './diagram-loader';
 /**
  * A helper interface that allows the client to mark actions that have been received from the server.
  */
 export interface ServerAction extends Action {
-    _receivedFromServer: true;
+    __receivedFromServer: true;
 }
 
 export namespace ServerAction {
     export function is(object: unknown): object is ServerAction {
-        return Action.is(object) && '_receivedFromServer' in object && object._receivedFromServer === true;
+        return Action.is(object) && '__receivedFromServer' in object && object.__receivedFromServer === true;
     }
 
     /**
-     * Mark the given action as {@link ServerAction} by attaching the "_receivedFromServer" property
+     * Mark the given action as {@link ServerAction} by attaching the "__receivedFromServer" property
      * @param action The action that should be marked as server action
      */
     export function mark(action: Action): void {
-        (action as ServerAction)._receivedFromServer = true;
+        (action as ServerAction).__receivedFromServer = true;
     }
 }
 
@@ -71,8 +70,10 @@ export class GLSPModelSource extends ModelSource implements Disposable {
 
     protected toDispose = new DisposableCollection();
     clientId: string;
-    readonly glspClient: GLSPClient | undefined;
     protected _currentRoot: SModelRootSchema;
+    protected actionHandlerRegistryDeferred = new Deferred<ActionHandlerRegistry>();
+
+    protected glspClient: GLSPClient | undefined;
 
     get diagramType(): string {
         return this.options.diagramType;
@@ -87,14 +88,18 @@ export class GLSPModelSource extends ModelSource implements Disposable {
         this.clientId = this.options.clientId ?? this.viewerOptions.baseDiv;
     }
 
-    configure(registry: ActionHandlerRegistry, initializeResult: InitializeResult): Promise<void> {
-        const serverActions = initializeResult.serverActions[this.diagramType];
+    async connect(glspClient: GLSPClient): Promise<void> {
+        const registry = await this.actionHandlerRegistryDeferred.promise;
+        const initializeResult = glspClient.initializeResult;
+        const serverActions = initializeResult?.serverActions[this.diagramType];
         if (!serverActions || serverActions.length === 0) {
+            // eslint-disable-next-line max-len
             throw new Error(`No server-handled actions could be derived from the initialize result for diagramType: ${this.diagramType}!`);
         }
         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 });
+        this.toDispose.push(glspClient.onActionMessage(message => this.messageReceived(message), this.clientId));
+        this.glspClient = glspClient;
+        return glspClient.initializeClientSession({ clientSessionId: this.clientId, diagramType: this.diagramType });
     }
 
     protected messageReceived(message: ActionMessage): void {
@@ -114,14 +119,7 @@ export class GLSPModelSource extends ModelSource implements Disposable {
             this.clientId = this.viewerOptions.baseDiv;
         }
 
-        this.options.glspClientProvider().then(glspClient => {
-            (this as Writable<GLSPModelSource>).glspClient = glspClient;
-            if (glspClient.initializeResult) {
-                this.configure(registry, glspClient.initializeResult);
-            } else {
-                glspClient.onServerInitialized(result => this.configure(registry, result));
-            }
-        });
+        this.actionHandlerRegistryDeferred.resolve(registry);
     }
 
     handle(action: Action): void {