Allow multiple diagrams to be opened (#44)

* Allow multiple diagrams to be opened

- Fix TheiaJsonrpcGLSPClient to only have one handler for connection
- Fix problem with filter with same ID being added multiple times

- Add formatting of CSS files to prettier
- Centralize ID handling for containers and global IDs
-- Diagram elements need locally unique IDs
-- Other elements need globally unique IDs

Fixes #16

* Updated playwright config to have timeouts for action and navigation so tests fail on actions i.s.o. timeout.

* Added logging of the id of the node it couldn't find.

* Fix "Couldn't find node" when selecting something other than an entity

* Fix "Property widget starts flashing"

Problem: We open an entity through the property view. That triggers a
rebuild of the entity document which triggers an update in all open
diagrams that have that entity as they have a reference to it. The
diagram update also resets the selection state in each diagram which
in turn updates the property view again as it reacts to selection
changes. This can lead to an update-cycle if there is more than one
diagram open with the same entity.

Fix: Since the property view reacts to selection changes and updates in
the diagram trigger such a change, there is no need for the property
view to listen actually participate in the model lifecycle itself. So
there is no need to do the open/close since we do not need to listen to
updates ourselves.

* Fix Diagram context menu issue

- Ensure that the navigator tree always gets a URI of the selected node

* Improve update handling between editors

- Ensure GLSP selection is forwarded again if active editor changes
- Only update properties if selection of active widget changes
- Add additional change reason for better update decision in GLSP

* Added example relationship and diagram to e2e-tests example worspace.
Added context menu tests on relationship and diagram files in the explorer view.

---------

Co-authored-by: Harmen Wessels <97173058+harmen-xb@users.noreply.github.com>

e2e-tests/playwright.config.ts

@@ -10,9 +10,13 @@ export default defineConfig({
    retries: process.env.CI ? 1 : 0,
    // The number of times to repeat each test, useful for debugging flaky tests
    repeatEach: 1,
-   // Timeout for each test in milliseconds: 60 seconds.
+   // Fixture timeout (for each test) in milliseconds: 60 seconds.
    timeout: 60 * 1000,
    use: {
+      // Timeout for each single action in milliseconds: 5 seconds (make sure it's less hen the fixture timeout, so it will timeout before the whole tests times out)
+      actionTimeout: 5 * 1000,
+      // Timeout for each navigation action in milliseconds: 10 seconds (make sure it's less hen the fixture timeout, so it will timeout before the whole tests times out)
+      navigationTimeout: 10 * 1000,
       baseURL: 'http://localhost:3000',
       browserName: 'chromium',
       screenshot: 'only-on-failure',

e2e-tests/src/resources/sample-workspace/example-diagram.diagram.cm

@@ -0,0 +1,15 @@
+diagram:
+    id: "Example-diagram"
+    name: "Example-diagram"
+    nodes:
+      - id: "ExampleEntityNode"
+        entity: "unknown/ExampleEntity"
+        x: 463.96875
+        y: 251
+        width: 152.28289794921875
+        height: 94
+    edges:
+      - id: "ExampleEntityToExampleEntity"
+        relationship: "unknown/ExampleEntityToExampleEntity"
+        sourceNode: "ExampleEntityNode"
+        targetNode: "ExampleEntityNode"

e2e-tests/src/resources/sample-workspace/exanple-relationship.relationship.cm

@@ -0,0 +1,5 @@
+relationship:
+    id: "ExampleEntityToExampleEntity"
+    parent: "ExampleEntity"
+    child: "ExampleEntity"
+    type: "1:1"

e2e-tests/src/tests/crossmodel-explorer-view.spec.ts

@@ -23,4 +23,26 @@ test.describe('CrossModel Explorer View', () => {
       expect(await menu.menuItemByNamePath('Open With', 'Code Editor')).toBeDefined();
       expect(await menu.menuItemByNamePath('Open With', 'Form Editor')).toBeDefined();
    });
+
+   test('code and form editor options available in the context menu on a relationship', async () => {
+      const file = await explorer.getFileStatNodeByLabel('exanple-relationship.relationship.cm');
+      expect(file).toBeDefined();
+      expect(await file.label()).toBe('exanple-relationship.relationship.cm');
+      const menu = await file.openContextMenu();
+      expect(await menu.isOpen()).toBe(true);
+      // Expect the Code and Form editor to be in the Open With menu option.
+      expect(await menu.menuItemByNamePath('Open With', 'Code Editor')).toBeDefined();
+      expect(await menu.menuItemByNamePath('Open With', 'Form Editor')).toBeDefined();
+   });
+
+   test('code and diagram editor options available in the context menu on a diagram', async () => {
+      const file = await explorer.getFileStatNodeByLabel('example-diagram.diagram.cm');
+      expect(file).toBeDefined();
+      expect(await file.label()).toBe('example-diagram.diagram.cm');
+      const menu = await file.openContextMenu();
+      expect(await menu.isOpen()).toBe(true);
+      // Expect the Code and Form editor to be in the Open With menu option.
+      expect(await menu.menuItemByNamePath('Open With', 'Code Editor')).toBeDefined();
+      expect(await menu.menuItemByNamePath('Open With', 'CrossModel Diagram')).toBeDefined();
+   });
 });

extensions/crossmodel-lang/src/glsp-server/handler/add-entity-operation-handler.ts

@@ -7,7 +7,6 @@ import { Command, JsonOperationHandler, ModelState } from '@eclipse-glsp/server'
 import { inject, injectable } from 'inversify';
 import { DiagramNode, Entity } from '../../language-server/generated/ast.js';
 import { createNodeToEntityReference } from '../../language-server/util/ast-util.js';
-import { findNextId } from '../../language-server/util/name-util.js';
 import { CrossModelState } from '../model/cross-model-state.js';
 import { CrossModelCommand } from './cross-model-command.js';
 
@@ -34,7 +33,7 @@ export class CrossModelAddEntityOperationHandler extends JsonOperationHandler {
          const node: DiagramNode = {
             $type: DiagramNode,
             $container: container,
-            id: findNextId(container, entityDescription.name + 'Node'),
+            id: this.modelState.idProvider.findNextId(DiagramNode, entityDescription.name + 'Node', container),
             entity: {
                $refText: entityDescription.name,
                ref: entityDescription.node as Entity | undefined

extensions/crossmodel-lang/src/glsp-server/handler/create-edge-operation-handler.ts

@@ -32,7 +32,7 @@ export class CrossModelCreateEdgeOperationHandler extends JsonCreateEdgeOperatio
             const edge: DiagramEdge = {
                $type: DiagramEdge,
                $container: this.modelState.diagramRoot,
-               id: relationship.id,
+               id: this.modelState.idProvider.findNextId(DiagramEdge, relationship.id, this.modelState.diagramRoot),
                relationship: {
                   ref: relationship,
                   $refText: this.modelState.idProvider.getExternalId(relationship) || relationship.id || ''
@@ -58,23 +58,23 @@ export class CrossModelCreateEdgeOperationHandler extends JsonCreateEdgeOperatio
       const source = sourceNode.entity?.ref?.id || sourceNode.entity?.$refText;
       const target = targetNode.entity?.ref?.id || targetNode.entity?.$refText;
 
-      // search for unique file name for the relationship and use file base name as relationship name
-      // if the user doesn't rename any files we should end up with unique names ;-)
-      const dirName = UriUtils.dirname(URI.parse(this.modelState.semanticUri));
-      const targetUri = UriUtils.joinPath(dirName, source + 'To' + target + '.relationship.cm');
-      const uri = Utils.findNewUri(targetUri);
-      const id = UriUtils.basename(uri).split('.')[0];
-
       // create relationship, serialize and re-read to ensure everything is up to date and linked properly
       const relationshipRoot: CrossModelRoot = { $type: 'CrossModelRoot' };
       const relationship: Relationship = {
          $type: Relationship,
          $container: relationshipRoot,
-         id,
+         id: this.modelState.idProvider.findNextId(Relationship, source + 'To' + target),
          type: '1:1',
          parent: { $refText: sourceNode.entity?.$refText || '' },
          child: { $refText: targetNode.entity?.$refText || '' }
       };
+
+      // search for unique file name for the relationship and use file base name as relationship name
+      // if the user doesn't rename any files we should end up with unique names ;-)
+      const dirName = UriUtils.dirname(URI.parse(this.modelState.semanticUri));
+      const targetUri = UriUtils.joinPath(dirName, relationship.id + '.relationship.cm');
+      const uri = Utils.findNewUri(targetUri);
+
       relationshipRoot.relationship = relationship;
       const text = this.modelState.semanticSerializer.serialize(relationshipRoot);
 

extensions/crossmodel-lang/src/glsp-server/handler/drop-entity-operation-handler.ts

@@ -7,7 +7,6 @@ import { Command, JsonOperationHandler } from '@eclipse-glsp/server';
 import { inject, injectable } from 'inversify';
 import { URI } from 'vscode-uri';
 import { CrossModelRoot, DiagramNode, isCrossModelRoot } from '../../language-server/generated/ast.js';
-import { findNextId } from '../../language-server/util/name-util.js';
 import { CrossModelState } from '../model/cross-model-state.js';
 import { CrossModelCommand } from './cross-model-command.js';
 
@@ -37,7 +36,7 @@ export class CrossModelDropEntityOperationHandler extends JsonOperationHandler {
             const node: DiagramNode = {
                $type: DiagramNode,
                $container: container,
-               id: findNextId(container, root.entity.id + 'Node'),
+               id: this.modelState.idProvider.findNextId(DiagramNode, root.entity.id + 'Node', this.modelState.diagramRoot),
                entity: {
                   $refText: this.modelState.idProvider.getExternalId(root.entity) || root.entity.id || '',
                   ref: root.entity

extensions/crossmodel-lang/src/glsp-server/model/cross-model-storage.ts

@@ -57,7 +57,7 @@ export class CrossModelStorage implements SourceModelStorage, ClientSessionListe
       this.state.setSemanticRoot(rootUri, root);
       this.toDispose.push(
          this.state.modelService.onUpdate<CrossModelRoot>(rootUri, async event => {
-            if (this.state.clientId !== event.sourceClientId) {
+            if (this.state.clientId !== event.sourceClientId || event.reason !== 'changed') {
                this.state.setSemanticRoot(rootUri, event.model);
                this.actionDispatcher.dispatchAll(await this.submissionHandler.submitModel('external'));
             }

extensions/crossmodel-lang/src/language-server/cross-model-naming.ts

@@ -2,7 +2,7 @@
  * Copyright (c) 2023 CrossBreeze.
  ********************************************************************************/
 
-import { AstNode, CstNode, findNodeForProperty, NameProvider } from 'langium';
+import { AstNode, CstNode, findNodeForProperty, isAstNode, NameProvider, streamAst } from 'langium';
 import { CrossModelServices } from './cross-model-module.js';
 import { UNKNOWN_PROJECT_REFERENCE } from './cross-model-package-manager.js';
 import { findDocument } from './util/ast-util.js';
@@ -25,6 +25,9 @@ export interface IdProvider extends NameProvider {
    getNodeId(node?: AstNode): string | undefined;
    getLocalId(node?: AstNode): string | undefined;
    getExternalId(node?: AstNode): string | undefined;
+
+   findNextId(type: string, proposal: string | undefined): string;
+   findNextId(type: string, proposal: string | undefined, container: AstNode): string;
 }
 
 /**
@@ -103,4 +106,38 @@ export class DefaultIdProvider implements NameProvider, IdProvider {
    getNameNode(node: AstNode): CstNode | undefined {
       return findNodeForProperty(node.$cstNode, ID_PROPERTY);
    }
+
+   findNextId(type: string, proposal: string | undefined): string;
+   findNextId(type: string, proposal: string | undefined, container: AstNode): string;
+   findNextId(type: string, proposal: string | undefined, container?: AstNode): string {
+      if (isAstNode(container)) {
+         return this.findNextIdInContainer(type, proposal ?? 'Element', container);
+      }
+      return this.findNextIdInIndex(type, proposal ?? 'Element');
+   }
+
+   protected findNextIdInContainer(type: string, proposal: string, container: AstNode): string {
+      const knownIds = streamAst(container)
+         .filter(node => node.$type === type)
+         .map(this.getNodeId)
+         .nonNullable()
+         .toArray();
+      return this.countToNextId(knownIds, proposal);
+   }
+
+   protected findNextIdInIndex(type: string, proposal: string): string {
+      const knownIds = this.services.shared.workspace.IndexManager.allElements(type)
+         .map(element => element.name)
+         .toArray();
+      return this.countToNextId(knownIds, proposal);
+   }
+
+   protected countToNextId(knownIds: string[], proposal: string): string {
+      let nextId = proposal;
+      let counter = 1;
+      while (knownIds.includes(nextId)) {
+         nextId = proposal + counter++;
+      }
+      return nextId;
+   }
 }

extensions/crossmodel-lang/src/language-server/cross-model-validator.ts

@@ -7,6 +7,7 @@ import { ID_PROPERTY } from './cross-model-naming.js';
 import {
    CrossModelAstType,
    DiagramEdge,
+   SystemDiagram,
    isDiagramEdge,
    isDiagramNode,
    isEntity,
@@ -24,7 +25,8 @@ export function registerValidationChecks(services: CrossModelServices): void {
 
    const checks: ValidationChecks<CrossModelAstType> = {
       AstNode: validator.checkUniqueId,
-      DiagramEdge: validator.checkDiagramEdge
+      DiagramEdge: validator.checkDiagramEdge,
+      SystemDiagram: validator.checkUniqueIdWithinDiagram
    };
    registry.register(checks, validator);
 }
@@ -61,6 +63,24 @@ export class CrossModelValidator {
       );
    }
 
+   checkUniqueIdWithinDiagram(diagram: SystemDiagram, accept: ValidationAcceptor): void {
+      const knownIds: string[] = [];
+      for (const node of diagram.nodes) {
+         if (node.id && knownIds.includes(node.id)) {
+            accept('error', 'Must provide a unique id.', { node, property: ID_PROPERTY });
+         } else if (node.id) {
+            knownIds.push(node.id);
+         }
+      }
+      for (const edge of diagram.edges) {
+         if (edge.id && knownIds.includes(edge.id)) {
+            accept('error', 'Must provide a unique id.', { node: edge, property: ID_PROPERTY });
+         } else if (edge.id) {
+            knownIds.push(edge.id);
+         }
+      }
+   }
+
    checkDiagramEdge(edge: DiagramEdge, accept: ValidationAcceptor): void {
       if (edge.sourceNode?.ref?.entity?.ref?.$type !== edge.relationship?.ref?.parent?.ref?.$type) {
          accept('error', 'Source must match type of parent', { node: edge, property: 'sourceNode' });

extensions/crossmodel-lang/src/model-server/model-server.ts

@@ -75,7 +75,7 @@ export class ModelServer implements Disposable {
       const entity: Entity | undefined = diagramNode?.entity?.ref;
 
       if (!entity?.$container.$document) {
-         throw new Error('No node found with the given id');
+         throw new Error('No node found with the given id: ' + id + ' (in ' + uri + ')');
       }
 
       const serializedEntity = toSerializable({
@@ -112,7 +112,8 @@ export class ModelServer implements Disposable {
             this.connection.sendNotification(OnUpdated, {
                uri: args.uri,
                model: toSerializable(event.model) as CrossModelRoot,
-               sourceClientId: event.sourceClientId
+               sourceClientId: event.sourceClientId,
+               reason: event.reason
             })
          )
       );

extensions/crossmodel-lang/src/model-server/open-text-document-manager.ts

@@ -21,6 +21,11 @@ import { CrossModelLanguageMetaData } from '../language-server/generated/module.
 import { AddedSharedModelServices } from './model-module.js';
 import { OpenableTextDocuments } from './openable-text-documents.js';
 
+export interface UpdateInfo {
+   changed: URI[];
+   deleted: URI[];
+}
+
 /**
  * A manager class that supports handling documents with a simple open-update-save/close lifecycle.
  *
@@ -32,6 +37,8 @@ export class OpenTextDocumentManager {
    protected langiumDocs: LangiumDocuments;
    protected documentBuilder: DocumentBuilder;
 
+   protected lastUpdate?: UpdateInfo;
+
    constructor(services: AddedSharedModelServices & LangiumDefaultSharedServices) {
       this.textDocuments = services.workspace.TextDocuments;
       this.fileSystemProvider = services.workspace.FileSystemProvider;
@@ -42,6 +49,9 @@ export class OpenTextDocumentManager {
          this.open({ clientId: event.clientId, uri: event.document.uri, languageId: event.document.languageId })
       );
       this.textDocuments.onDidClose(event => this.close({ clientId: event.clientId, uri: event.document.uri }));
+      this.documentBuilder.onUpdate((changed, deleted) => {
+         this.lastUpdate = { changed, deleted };
+      });
    }
 
    /**
@@ -72,11 +82,17 @@ export class OpenTextDocumentManager {
          const changedDocument = allChangedDocuments.find(document => document.uri.toString() === uri);
          if (changedDocument) {
             const sourceClientId = this.getSourceClientId(changedDocument, allChangedDocuments);
-            listener({
+            const event: ModelUpdatedEvent<T> = {
                model: changedDocument.parseResult.value as T,
                sourceClientId,
-               uri: changedDocument.textDocument.uri
-            });
+               uri: changedDocument.textDocument.uri,
+               reason: this.lastUpdate?.changed.includes(changedDocument.uri)
+                  ? 'changed'
+                  : this.lastUpdate?.deleted.includes(changedDocument.uri)
+                    ? 'deleted'
+                    : 'updated'
+            };
+            listener(event);
          }
       });
    }
@@ -133,7 +149,7 @@ export class OpenTextDocumentManager {
    }
 
    protected createDummyDocument(uri: string): TextDocumentItem {
-      return TextDocumentItem.create(this.normalizedUri(uri), CrossModelLanguageMetaData.languageId, 1, '');
+      return TextDocumentItem.create(this.normalizedUri(uri), CrossModelLanguageMetaData.languageId, 0, '');
    }
 
    protected async createDocumentFromFileSystem(
@@ -142,7 +158,7 @@ export class OpenTextDocumentManager {
    ): Promise<TextDocumentItem> {
       const vscUri = URI.parse(uri);
       const content = this.fileSystemProvider.readFileSync(vscUri);
-      return TextDocumentItem.create(vscUri.toString(), languageId, 1, content.toString());
+      return TextDocumentItem.create(vscUri.toString(), languageId, 0, content.toString());
    }
 
    protected normalizedUri(uri: string): string {