fix(core): fix wildcard import parsing, import hygiene (#504)

Signed-off-by: Matt Roberts <[email protected]>

packages/concerto-core/lib/basemodelmanager.js

@@ -359,11 +359,12 @@ class BaseModelManager {
             fileDownloader = new FileDownloader(new DefaultFileLoader(this.processFile), (file) => MetaModelUtil.getExternalImports(file.ast));
         }
 
-        const externalModels = await fileDownloader.downloadExternalDependencies(this.getModelFiles(), options);
         const originalModelFiles = {};
         Object.assign(originalModelFiles, this.modelFiles);
 
         try {
+            const externalModels = await fileDownloader.downloadExternalDependencies(this.getModelFiles(), options);
+
             const externalModelFiles = [];
             externalModels.forEach((file) => {
                 const mf = new ModelFile(this, file.ast, file.definitions, file.fileName);
@@ -380,6 +381,7 @@ class BaseModelManager {
             this.validateModelFiles();
             return externalModelFiles;
         } catch (err) {
+            // Restore original files
             this.modelFiles = {};
             Object.assign(this.modelFiles, originalModelFiles);
             throw err;

packages/concerto-core/lib/introspect/modelfile.js

@@ -215,18 +215,45 @@ class ModelFile extends Decorated {
      */
     validate() {
         super.validate();
+
+        // A dictionary of imports to versions to track unique namespaces
+        const importsMap = new Map();
+
         // Validate all of the imports to check that they reference
         // namespaces or types that actually exist.
         this.getImports().forEach((importFqn) => {
             const importNamespace = ModelUtil.getNamespace(importFqn);
             const importShortName = ModelUtil.getShortName(importFqn);
             const modelFile = this.getModelManager().getModelFile(importNamespace);
+            const { name, version: importVersion } = ModelUtil.parseNamespace(importNamespace);
+
             if (!modelFile) {
                 let formatter = Globalize.messageFormatter('modelmanager-gettype-noregisteredns');
                 throw new IllegalModelException(formatter({
                     type: importFqn
                 }), this);
             }
+
+            const existingNamespaceVersion = importsMap.get(name);
+            // undefined means we haven't seen this namespace before,
+            // null means we have seen it before but it didn't have a version
+            const unseenNamespace = existingNamespaceVersion === undefined;
+
+            // This check is needed because we automatically add both versioned and unversioned versions of
+            // the root namespace for backwards compatibillity unless we're running in strict mode
+            const isGlobalModel = name === 'concerto';
+
+            const differentVersionsOfSameNamespace = !unseenNamespace && existingNamespaceVersion !== importVersion;
+            if (!isGlobalModel && differentVersionsOfSameNamespace){
+                let formatter = Globalize.messageFormatter('modelmanager-gettype-duplicatensimport');
+                throw new IllegalModelException(formatter({
+                    namespace: importNamespace,
+                    version1: existingNamespaceVersion,
+                    version2: importVersion
+                }), this);
+            }
+            importsMap.set(name, importVersion);
+
             if (importFqn.endsWith('*')) {
                 // This is a wildcard import, org.acme.*
                 // Doesn't matter if 0 or 100 types in the namespace.

packages/concerto-core/lib/modelutil.js

@@ -44,7 +44,7 @@ class ModelUtil {
     }
 
     /**
-     * Returns the namespace for a the fully qualified name of a type
+     * Returns the namespace for the fully qualified name of a type
      * @param {string} fqn - the fully qualified identifier of a type
      * @return {string} - namespace of the type (everything before the last dot)
      * or the empty string if there is no dot

packages/concerto-core/messages/en.json

@@ -49,6 +49,7 @@
         "modelmanager-resolvetype-notypeinnsforcontext": "No type \"{type}\" in namespace \"{namespace}\" for \"{context}\".",
 
         "modelmanager-gettype-noregisteredns": "Namespace is not defined for type \"{type}\".",
+        "modelmanager-gettype-duplicatensimport": "Importing types from different versions (\"{version1}\", \"{version2}\") of the same namespace \"{namespace}\" is not permitted.",
         "modelmanager-gettype-notypeinns": "Type \"{type}\" is not defined in namespace \"{namespace}\".",
 
         "serializer-constructor-factorynull": "\"Factory\" cannot be \"null\".",

packages/concerto-core/test/introspect/modelfile.js

@@ -263,6 +263,59 @@ describe('ModelFile', () => {
             (() => modelFile2.validate()).should.not.throw();
         });
 
+        it('should throw when attempting to import types from different versions of the same namespace', () => {
+            const myModelManager = new ModelManager();
+
+            const freddo1 = `namespace [email protected]
+            concept Chocolate {}`;
+
+            const freddo2 = `namespace [email protected]
+            concept Chocolate {}`;
+
+            const acme = `namespace [email protected]
+            import { Chocolate } from [email protected]
+            import { Chocolate } from [email protected]
+            `;
+
+            let modelFile1 = ParserUtil.newModelFile(myModelManager, freddo1);
+            myModelManager.addModelFile(modelFile1);
+
+            let modelFile2 = ParserUtil.newModelFile(myModelManager, freddo2);
+            myModelManager.addModelFile(modelFile2);
+
+            let modelFile3 = ParserUtil.newModelFile(myModelManager, acme);
+
+            (() => {
+                modelFile3.validate();
+            }).should.throw('Importing types from different versions ("1.0.0", "2.0.0") of the same namespace "[email protected]" is not permitted.');
+        });
+
+        it('should throw when attempting to import types from unversioned and versioned versions of the same namespace', () => {
+            const myModelManager = new ModelManager();
+
+            const freddo1 = `namespace org.freddos
+            concept Chocolate {}`;
+
+            const freddo2 = `namespace [email protected]
+            concept Chocolate {}`;
+
+            const acme = `namespace [email protected]
+            import { Chocolate } from org.freddos
+            import { Chocolate } from [email protected]
+            `;
+
+            let modelFile1 = ParserUtil.newModelFile(myModelManager, freddo1);
+            myModelManager.addModelFile(modelFile1);
+
+            let modelFile2 = ParserUtil.newModelFile(myModelManager, freddo2);
+            myModelManager.addModelFile(modelFile2);
+
+            let modelFile3 = ParserUtil.newModelFile(myModelManager, acme);
+
+            (() => {
+                modelFile3.validate();
+            }).should.throw('Importing types from different versions ("null", "2.0.0") of the same namespace "[email protected]" is not permitted.');
+        });
     });
 
     describe('#getDefinitions', () => {

packages/concerto-core/types/lib/modelutil.d.ts

@@ -14,7 +14,7 @@ declare class ModelUtil {
      */
     static getShortName(fqn: string): string;
     /**
-     * Returns the namespace for a the fully qualified name of a type
+     * Returns the namespace for the fully qualified name of a type
      * @param {string} fqn - the fully qualified identifier of a type
      * @return {string} - namespace of the type (everything before the last dot)
      * or the empty string if there is no dot