GLSP-1427: Improve diagram loading and commmand handling (#398)

## Diagram Loader
- Avoid unnecessary dispatch of empty `SetModelAction`. Await the initialize method of the action dispatcher instead
  (which already dispatches an empty model internally
- Fix behavior of `postRequestModel` hook to actually work as described in the model. Instead of requesting and awaiting the dispatch of the response `SetModelAction` we now simply dispatch the `RequestModelAction` and continue with the`postRequestModelHook`
- Use promise instead of listener for the model initalization constraint. This way we can await the `postModelInitialization` hook . I.e. the DiagramLoader.load methods actually completes once its completely finished (all hooks have been invoked and resolved)

## UiExtensions
- Fix action of startup UIExtensions. Instead of directly calling their show-Method, a corresponding `SetUIExtensionVisibilityAction` is dispatched. This ensures that the extension only get activated after the initial diagram (empty-model) is available
- Make `preInitialize´ hook of StatusOverlay async. The status overlay is used to print messages during the `loading` phase.
 By making it async, the loader waits until the overlay is actually visible => now messages get lost.
- Move tool palette initialization into from `preRequest`to`postRequest` hook. This ensures that the server request for retrieving the items is dispatched after the model request => does not block diagram loading
    - Fix unnecessary reload of the palette body if in read-only mode (palette not visible) 

## CommandStack
- Override methods that populate/manage the internal undo/redo stack to no-ops
  Commandstack undo/redo is not supported in GLSP. This means managing the stack is just unnecessary overhead

Contributed on behalf of AxonIvy AG
Fixes eclipse-glsp/glsp#1427

packages/client/src/base/command-stack.ts

@@ -14,6 +14,7 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 import {
+    CommandExecutionContext,
     CommandStack,
     Disposable,
     DisposableCollection,
@@ -51,6 +52,10 @@ export class GLSPCommandStack extends CommandStack implements Disposable {
         return this.editorContext.onModelRootChanged;
     }
 
+    /**
+     * Client-side undo/redo is not supported in GLSP. The server is responsible for handling undo/redo requests.
+     * If this method get called it's probably a mistake and a warning is logged
+     */
     override undo(): Promise<GModelRoot> {
         this.logger.warn(
             this,
@@ -59,6 +64,10 @@ export class GLSPCommandStack extends CommandStack implements Disposable {
         return this.currentModel;
     }
 
+    /**
+     * Client-side undo/redo is not supported in GLSP. The server is responsible for handling undo/redo requests.
+     * If this method get called it's probably a mistake and a warning is logged
+     */
     override redo(): Promise<GModelRoot> {
         this.logger.warn(
             this,
@@ -67,14 +76,29 @@ export class GLSPCommandStack extends CommandStack implements Disposable {
         return this.currentModel;
     }
 
+    /**
+     * Client-side undo/redo is not supported in GLSP.
+     * To avoid unnecessary infraction with the command stack (pushing/merging/popping commands)
+     * related methods are overridden to no-ops.
+     */
+    protected override pushToUndoStack(command: ICommand): void {
+        // no-op
+    }
+
+    /**
+     * Client-side undo/redo is not supported in GLSP.
+     * To avoid unnecessary infraction with the command stack (pushing/merging/popping commands)
+     * related methods are overridden to no-ops.
+     */
+    protected override mergeOrPush(command: ICommand, context: CommandExecutionContext): void {
+        // no-op
+    }
     override async execute(command: ICommand): Promise<GModelRoot> {
+        const result = await super.execute(command);
         if (command instanceof SetModelCommand || command instanceof UpdateModelCommand) {
-            const result = await super.execute(command);
             this.notifyListeners(result);
-            return result;
         }
-
-        return super.execute(command);
+        return result;
     }
 
     protected notifyListeners(root: Readonly<GModelRoot>): void {

packages/client/src/base/feedback/feedback-action-dispatcher-default.ts

@@ -27,20 +27,23 @@ import {
     TYPES,
     toTypeGuard
 } from '@eclipse-glsp/sprotty';
-import { inject, injectable } from 'inversify';
+import { inject, injectable, preDestroy } from 'inversify';
+import { IFeedbackActionDispatcher, IFeedbackEmitter, MaybeActions } from './feedback-action-dispatcher';
 import { getFeedbackRank } from './feedback-command';
 import { FeedbackEmitter } from './feedback-emitter';
-import { IFeedbackActionDispatcher, IFeedbackEmitter, MaybeActions } from './feedback-action-dispatcher';
 
 @injectable()
-export class FeedbackActionDispatcher implements IFeedbackActionDispatcher {
+export class FeedbackActionDispatcher implements IFeedbackActionDispatcher, Disposable {
     protected registeredFeedback: Map<IFeedbackEmitter, Action[]> = new Map();
 
-    @inject(TYPES.IActionDispatcherProvider) protected actionDispatcher: () => Promise<IActionDispatcher>;
+    @inject(TYPES.IActionDispatcher) protected actionDispatcher: IActionDispatcher;
+
     @inject(TYPES.ILogger) protected logger: ILogger;
 
     @inject(ActionHandlerRegistry) protected actionHandlerRegistry: ActionHandlerRegistry;
 
+    protected isDisposed = false;
+
     registerFeedback(feedbackEmitter: IFeedbackEmitter, feedbackActions: Action[], cleanupActions?: MaybeActions): Disposable {
         if (feedbackEmitter instanceof GModelElement) {
             this.logger.log(
@@ -109,11 +112,19 @@ export class FeedbackActionDispatcher implements IFeedbackActionDispatcher {
 
     protected async dispatchFeedback(actions: Action[], feedbackEmitter: IFeedbackEmitter): Promise<void> {
         try {
-            const actionDispatcher = await this.actionDispatcher();
-            await actionDispatcher.dispatchAll(actions);
+            if (this.isDisposed) {
+                return;
+            }
+            await this.actionDispatcher.dispatchAll(actions);
             this.logger.info(this, `Dispatched feedback actions for ${feedbackEmitter}`);
         } catch (reason) {
             this.logger.error(this, 'Failed to dispatch feedback actions', reason);
         }
     }
+
+    @preDestroy()
+    dispose(): void {
+        this.registeredFeedback.clear();
+        this.isDisposed = true;
+    }
 }

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

@@ -18,14 +18,13 @@ import {
     AnyObject,
     ApplicationIdProvider,
     Args,
-    EMPTY_ROOT,
     GLSPClient,
     IActionDispatcher,
     InitializeParameters,
     LazyInjector,
     MaybePromise,
+    RequestAction,
     RequestModelAction,
-    SetModelAction,
     StatusAction,
     TYPES,
     hasNumberProp
@@ -44,19 +43,23 @@ export interface IDiagramOptions {
      * corresponding client session.
      */
     clientId: string;
+
     /**
      * The diagram type i.e. diagram language this diagram is associated with.
      */
     diagramType: string;
+
     /**
      * The provider function to retrieve the GLSP client used by this diagram to communicate with the server.
-     * Multiple invocations of the provder function should always return the same `GLSPClient` instance.
+     * Multiple invocations of the provider function should always return the same {@link GLSPClient} instance.
      */
     glspClientProvider: () => Promise<GLSPClient>;
+
     /**
      * The file source URI associated with this diagram.
      */
     sourceUri?: string;
+
     /**
      * The initial edit mode of diagram. Defaults to `editable`.
      */
@@ -74,24 +77,28 @@ export interface IDiagramStartup extends Partial<Ranked> {
      * first hook that is invoked directly after {@link DiagramLoader.load} is called.
      */
     preLoadDiagram?(): MaybePromise<void>;
+
     /**
-     * Hook for services that want to execute code before the underlying GLSP client is configured and the server is initialized.
+     * Hook for services that want to execute code before the underlying {@link GLSPClient} is configured and the server is initialized.
      */
     preInitialize?(): MaybePromise<void>;
+
     /**
-     * Hook for services that want to execute code before the initial model loading request (i.e. `RequestModelAction`) but
+     * Hook for services that want to execute code before the initial model loading request (i.e. {@link RequestModelAction}) but
      * after the underlying GLSP client has been configured and the server is initialized.
      */
     preRequestModel?(): MaybePromise<void>;
+
     /**
-     * Hook for services that want to execute code 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
+     * Hook for services that want to execute code after the initial model loading request (i.e. {@link RequestModelAction}).
+     * Note that this hook is invoked directly after the {@link 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 postModelInitialization} hook.
      */
+
     postRequestModel?(): MaybePromise<void>;
-    /* Hook for services that want to execute code after the diagram model is fully initialized
-     * (i.e. `ModelInitializationConstraint` is completed).
+    /** Hook for services that want to execute code after the diagram model is fully initialized
+     * (i.e. {@link ModelInitializationConstraint} is completed).
      */
     postModelInitialization?(): MaybePromise<void>;
 }
@@ -119,7 +126,7 @@ export interface DiagramLoadingOptions {
     requestModelOptions?: Args;
 
     /**
-     * Optional partial {@link InitializeParameters} that should be used for `initializeServer` request if the underlying
+     * Optional partial {@link InitializeParameters} that should be used for {@link GLSPClient.initializeServer} request if the underlying
      * {@link GLSPClient} has not been initialized yet.
      */
     initializeParameters?: Partial<InitializeParameters>;
@@ -180,14 +187,15 @@ export class DiagramLoader {
             },
             enableNotifications: options.enableNotifications ?? true
         };
-        // Set empty place holder model until actual model from server is available
-        await this.actionDispatcher.dispatch(SetModelAction.create(EMPTY_ROOT));
+        // Ensure that the action dispatcher is initialized before starting the diagram loading process
+        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'));
+        await this.modelInitializationConstraint.onInitialized();
+        await this.invokeStartupHook('postModelInitialization');
     }
 
     protected async invokeStartupHook(hook: keyof Omit<IDiagramStartup, 'rank'>): Promise<void> {
@@ -201,10 +209,9 @@ export class DiagramLoader {
     }
 
     protected async requestModel(options: ResolvedDiagramLoadingOptions): Promise<void> {
-        const response = await this.actionDispatcher.request<SetModelAction>(
-            RequestModelAction.create({ options: options.requestModelOptions })
+        await this.actionDispatcher.dispatch(
+            RequestModelAction.create({ options: options.requestModelOptions, requestId: RequestAction.generateRequestId() })
         );
-        return this.actionDispatcher.dispatch(response);
     }
 
     protected async initialize(options: ResolvedDiagramLoadingOptions): Promise<void> {

packages/client/src/features/accessibility/toast/toast-tool.ts

@@ -14,7 +14,7 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import { Action, IActionDispatcher, IActionHandler, ICommand, TYPES } from '@eclipse-glsp/sprotty';
+import { Action, IActionDispatcher, IActionHandler, ICommand, SetUIExtensionVisibilityAction, TYPES } from '@eclipse-glsp/sprotty';
 import { inject, injectable } from 'inversify';
 import { EditorContextService } from '../../../base/editor-context-service';
 import { IDiagramStartup } from '../../../base/model/diagram-loader';
@@ -99,7 +99,7 @@ export class Toast extends GLSPAbstractUIExtension implements IActionHandler, ID
     }
 
     preInitialize(): void {
-        this.show(this.editorContext.modelRoot);
+        this.actionDispatcher.dispatch(SetUIExtensionVisibilityAction.create({ extensionId: Toast.ID, visible: true }));
     }
 
     values(obj: { [key: symbol]: ToastOptions }): ToastOptions[] {

packages/client/src/features/status/status-overlay.ts

@@ -13,9 +13,15 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { IActionDispatcher, IActionHandler, StatusAction, TYPES, codiconCSSClasses } from '@eclipse-glsp/sprotty';
+import {
+    IActionDispatcher,
+    IActionHandler,
+    SetUIExtensionVisibilityAction,
+    StatusAction,
+    TYPES,
+    codiconCSSClasses
+} from '@eclipse-glsp/sprotty';
 import { inject, injectable } from 'inversify';
-import { EditorContextService } from '../../base/editor-context-service';
 import { IDiagramStartup } from '../../base/model/diagram-loader';
 import { GLSPAbstractUIExtension } from '../../base/ui-extension/ui-extension';
 
@@ -29,9 +35,6 @@ export class StatusOverlay extends GLSPAbstractUIExtension implements IActionHan
     @inject(TYPES.IActionDispatcher)
     protected actionDispatcher: IActionDispatcher;
 
-    @inject(EditorContextService)
-    protected editorContext: EditorContextService;
-
     protected statusIconDiv?: HTMLDivElement;
     protected statusMessageDiv?: HTMLDivElement;
     protected pendingTimeout?: number;
@@ -115,7 +118,7 @@ export class StatusOverlay extends GLSPAbstractUIExtension implements IActionHan
         }
     }
 
-    preInitialize(): void {
-        this.show(this.editorContext.modelRoot);
+    preInitialize(): Promise<void> {
+        return this.actionDispatcher.dispatch(SetUIExtensionVisibilityAction.create({ extensionId: this.id(), visible: true }));
     }
 }

packages/client/src/features/tool-palette/tool-palette.ts

@@ -474,17 +474,21 @@ export class ToolPalette extends GLSPAbstractUIExtension implements IActionHandl
         this.createBody();
     }
 
-    async preRequestModel(): Promise<void> {
+    /**
+     *  @deprecated This hook is no longer used by the ToolPalette.
+     *              It is kept for compatibility reasons and will be removed in the future.
+     *              Move initialization logic to the `postRequestModel` method.
+     *              This ensures that tool palette initialization does not block the diagram loading process.
+     */
+    async preRequestModel(): Promise<void> {}
+
+    async postRequestModel(): Promise<void> {
         await this.setPaletteItems();
         if (!this.editorContext.isReadonly) {
             this.show(this.editorContext.modelRoot);
         }
     }
 
-    async postRequestModel(): Promise<void> {
-        this.reloadPaletteBody();
-    }
-
     protected async setPaletteItems(): Promise<void> {
         const requestAction = RequestContextActions.create({
             contextId: ToolPalette.ID,
@@ -506,7 +510,7 @@ export class ToolPalette extends GLSPAbstractUIExtension implements IActionHandl
     }
 
     protected async reloadPaletteBody(): Promise<void> {
-        if (this.dynamic) {
+        if (!this.editorContext.isReadonly && this.dynamic) {
             await this.setPaletteItems();
             this.paletteItemsCopy = [];
             this.requestFilterUpdate(this.searchField.value);

packages/glsp-sprotty/src/api-override.ts

@@ -135,6 +135,13 @@ export interface IVNodePostprocessor extends SIVNodePostprocessor {
 }
 
 export interface IActionDispatcher extends SIActionDispatcher {
+    /**
+     * Optional method to initialize the action dispatcher.
+     * Implementation can use this as a hook to perform any initialization tasks,
+     * like registering action handlers or setting up the initial diagram.
+     * Called by the `DiagramLoader` when starting the loading process.
+     */
+    initialize?(): Promise<void>;
     /**
      * Dispatch an action by querying all handlers that are registered for its kind.
      * The returned promise is resolved when all handler results (commands or actions)