From 1a7ffdce5f383e2ce80d43973ab05c266ae133bb Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Fri, 8 Dec 2023 20:36:34 +0100 Subject: [PATCH] Fix test cases and clean up id validation - Only export root nodes and entity attributes for external reference - Properly check exported ids and local ids --- ...m => example-relationship.relationship.cm} | 0 .../tests/crossmodel-explorer-view.spec.ts | 7 ++- .../Order_Customer.relationship.cm | 2 +- .../src/language-server/cross-model-naming.ts | 4 ++ .../src/language-server/cross-model-scope.ts | 4 +- .../language-server/cross-model-validator.ts | 58 +++++++++---------- packages/protocol/src/index.ts | 1 + yarn.lock | 12 ++++ 8 files changed, 52 insertions(+), 36 deletions(-) rename e2e-tests/src/resources/sample-workspace/{exanple-relationship.relationship.cm => example-relationship.relationship.cm} (100%) diff --git a/e2e-tests/src/resources/sample-workspace/exanple-relationship.relationship.cm b/e2e-tests/src/resources/sample-workspace/example-relationship.relationship.cm similarity index 100% rename from e2e-tests/src/resources/sample-workspace/exanple-relationship.relationship.cm rename to e2e-tests/src/resources/sample-workspace/example-relationship.relationship.cm diff --git a/e2e-tests/src/tests/crossmodel-explorer-view.spec.ts b/e2e-tests/src/tests/crossmodel-explorer-view.spec.ts index bd96531..d73a8db 100644 --- a/e2e-tests/src/tests/crossmodel-explorer-view.spec.ts +++ b/e2e-tests/src/tests/crossmodel-explorer-view.spec.ts @@ -22,17 +22,19 @@ test.describe('CrossModel Explorer View', () => { // 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(); + await menu.close(); }); test('code and form editor options available in the context menu on a relationship', async () => { - const file = await explorer.getFileStatNodeByLabel('exanple-relationship.relationship.cm'); + const file = await explorer.getFileStatNodeByLabel('example-relationship.relationship.cm'); expect(file).toBeDefined(); - expect(await file.label()).toBe('exanple-relationship.relationship.cm'); + expect(await file.label()).toBe('example-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(); + await menu.close(); }); test('code and diagram editor options available in the context menu on a diagram', async () => { @@ -44,5 +46,6 @@ test.describe('CrossModel Explorer View', () => { // 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(); + await menu.close(); }); }); diff --git a/examples/mapping-example/ExampleCRM/relationships/Order_Customer.relationship.cm b/examples/mapping-example/ExampleCRM/relationships/Order_Customer.relationship.cm index 47edc7d..76ef9d4 100644 --- a/examples/mapping-example/ExampleCRM/relationships/Order_Customer.relationship.cm +++ b/examples/mapping-example/ExampleCRM/relationships/Order_Customer.relationship.cm @@ -2,4 +2,4 @@ relationship: id: Order_Customer parent: Customer child: Order - type: '1:1' \ No newline at end of file + type: "1:1" \ No newline at end of file diff --git a/extensions/crossmodel-lang/src/language-server/cross-model-naming.ts b/extensions/crossmodel-lang/src/language-server/cross-model-naming.ts index 634d82a..4ea0665 100644 --- a/extensions/crossmodel-lang/src/language-server/cross-model-naming.ts +++ b/extensions/crossmodel-lang/src/language-server/cross-model-naming.ts @@ -9,6 +9,10 @@ import { findDocument } from './util/ast-util.js'; export const ID_PROPERTY = 'id'; +export type IdentifiableAstNode = AstNode & { + id?: string; +}; + export type IdentifiedAstNode = AstNode & { [ID_PROPERTY]: string; }; diff --git a/extensions/crossmodel-lang/src/language-server/cross-model-scope.ts b/extensions/crossmodel-lang/src/language-server/cross-model-scope.ts index 401948c..58a7000 100644 --- a/extensions/crossmodel-lang/src/language-server/cross-model-scope.ts +++ b/extensions/crossmodel-lang/src/language-server/cross-model-scope.ts @@ -73,8 +73,8 @@ export class CrossModelScopeComputation extends DefaultScopeComputation { ): Promise { const docRoot = document.parseResult.value; if (isCrossModelRoot(docRoot) && (docRoot.diagram || docRoot.mapping)) { - // we do not export anything from diagrams or mappings, they are self-contained - return []; + // we do not export anything from diagrams or mappings except their root node + super.computeExportsForNode(parentNode, document, () => [], cancelToken); } return super.computeExportsForNode(parentNode, document, children, cancelToken); } diff --git a/extensions/crossmodel-lang/src/language-server/cross-model-validator.ts b/extensions/crossmodel-lang/src/language-server/cross-model-validator.ts index 88b69ae..bfffd80 100644 --- a/extensions/crossmodel-lang/src/language-server/cross-model-validator.ts +++ b/extensions/crossmodel-lang/src/language-server/cross-model-validator.ts @@ -3,15 +3,14 @@ ********************************************************************************/ import { AstNode, ValidationAcceptor, ValidationChecks } from 'langium'; import type { CrossModelServices } from './cross-model-module.js'; -import { ID_PROPERTY } from './cross-model-naming.js'; +import { ID_PROPERTY, IdentifiableAstNode } from './cross-model-naming.js'; import { CrossModelAstType, DiagramEdge, - Mapping, SourceObject, - SystemDiagram, isEntity, isEntityAttribute, + isMapping, isRelationship, isSystemDiagram } from './generated/ast.js'; @@ -24,10 +23,8 @@ export function registerValidationChecks(services: CrossModelServices): void { const validator = services.validation.CrossModelValidator; const checks: ValidationChecks = { - AstNode: validator.checkUniqueId, + AstNode: validator.checkNode, DiagramEdge: validator.checkDiagramEdge, - SystemDiagram: validator.checkUniqueIdWithinDiagram, - Mapping: validator.checkUniqueIdWithinMapping, SourceObject: validator.checkSourceObject }; registry.register(checks, validator); @@ -39,46 +36,45 @@ export function registerValidationChecks(services: CrossModelServices): void { export class CrossModelValidator { constructor(protected services: CrossModelServices) {} - checkUniqueId(node: AstNode, accept: ValidationAcceptor): void { - const elementName = this.services.references.IdProvider.getNodeId(node); - if (!elementName) { - if (this.shouldHaveId(node)) { - accept('error', 'Missing required id field', { node, property: ID_PROPERTY }); - } + checkNode(node: AstNode, accept: ValidationAcceptor): void { + this.checkUniqueExternalId(node, accept); + this.checkUniqueNodeId(node, accept); + } + + protected checkUniqueExternalId(node: AstNode, accept: ValidationAcceptor): void { + if (!this.isExported(node)) { + return; + } + const externalId = this.services.references.IdProvider.getExternalId(node); + if (!externalId) { + accept('error', 'Missing required id field', { node, property: ID_PROPERTY }); return; } const allElements = Array.from(this.services.shared.workspace.IndexManager.allElements()); - const duplicates = allElements.filter(description => description.name === elementName); + const duplicates = allElements.filter(description => description.name === externalId); if (duplicates.length > 1) { accept('error', 'Must provide a unique id.', { node, property: ID_PROPERTY }); } } - protected shouldHaveId(node: AstNode): boolean { - return isEntity(node) || isEntityAttribute(node) || isRelationship(node) || isSystemDiagram(node); + protected isExported(node: AstNode): boolean { + // we export anything with an id from entities and relationships and all root nodes, see CrossModelScopeComputation + return isEntity(node) || isEntityAttribute(node) || isRelationship(node) || isSystemDiagram(node) || isMapping(node); } - 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); - } + protected checkUniqueNodeId(node: AstNode, accept: ValidationAcceptor): void { + if (isSystemDiagram(node)) { + this.markDuplicateIds(node.edges, accept); + this.markDuplicateIds(node.nodes, accept); } - 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); - } + if (isMapping(node)) { + this.markDuplicateIds(node.sourceObjects, accept); } } - checkUniqueIdWithinMapping(mapping: Mapping, accept: ValidationAcceptor): void { + protected markDuplicateIds(nodes: IdentifiableAstNode[], accept: ValidationAcceptor): void { const knownIds: string[] = []; - for (const node of mapping.sourceObjects) { + for (const node of nodes) { if (node.id && knownIds.includes(node.id)) { accept('error', 'Must provide a unique id.', { node, property: ID_PROPERTY }); } else if (node.id) { diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index c46254c..4bbd081 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -1,6 +1,7 @@ /******************************************************************************** * Copyright (c) 2023 CrossBreeze. ********************************************************************************/ +import 'reflect-metadata'; export * from './glsp/actions'; export * from './integration'; export * from './model-service/protocol'; diff --git a/yarn.lock b/yarn.lock index f7da03c..69ee159 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3334,6 +3334,13 @@ "@types/scheduler" "*" csstype "^3.0.2" +"@types/reflect-metadata@^0.1.0": + version "0.1.0" + resolved "https://registry.yarnpkg.com/@types/reflect-metadata/-/reflect-metadata-0.1.0.tgz#592805bdf6d63dd7229773218afeba37ac2eab16" + integrity sha512-bXltFLY3qhzCnVYP5iUpeSICagQ8rc9K2liS+8M0lBcz54BHs3O6W5UvqespVSuebo1BXLi+/y9ioELAW9SC2A== + dependencies: + reflect-metadata "*" + "@types/responselike@^1.0.0": version "1.0.2" resolved "https://registry.yarnpkg.com/@types/responselike/-/responselike-1.0.2.tgz#8de1b0477fd7c12df77e50832fa51701a8414bd6" @@ -12446,6 +12453,11 @@ redent@^2.0.0: indent-string "^3.0.0" strip-indent "^2.0.0" +reflect-metadata@*: + version "0.1.14" + resolved "https://registry.yarnpkg.com/reflect-metadata/-/reflect-metadata-0.1.14.tgz#24cf721fe60677146bb77eeb0e1f9dece3d65859" + integrity sha512-ZhYeb6nRaXCfhnndflDK8qI6ZQ/YcWZCISRAWICW9XYqMUwjZM9Z0DveWX/ABN01oxSHwVxKQmxeYZSsm0jh5A== + reflect-metadata@^0.1.10: version "0.1.13" resolved "https://registry.yarnpkg.com/reflect-metadata/-/reflect-metadata-0.1.13.tgz#67ae3ca57c972a2aa1642b10fe363fe32d49dc08"