feat(map): Adds map declaration analysis to concerto-analysis (#691)

* feat(maps): export MapDeclaration type definitions Signed-off-by: jonathan.casey <[email protected]> * feat(maps): update JSDoc and type definition Signed-off-by: jonathan.casey <[email protected]> * feat(maps): adds map-declaration comparer Signed-off-by: jonathan.casey <[email protected]> * feat(maps): extends comparer for map declaration Signed-off-by: jonathan.casey <[email protected]> * feat(maps): refactor util function Signed-off-by: jonathan.casey <[email protected]> * feat(maps): adds test coverage Signed-off-by: jonathan.casey <[email protected]> * feat(maps): JSDoc Signed-off-by: jonathan.casey <[email protected]> * feat(maps): adds changelog hash Signed-off-by: jonathan.casey <[email protected]> * feat(maps): add ENABLE_MAP_TYPE env var for test run Signed-off-by: jonathan.casey <[email protected]> * feat(maps): remove improbable branch Signed-off-by: jonathan.casey <[email protected]> * feat(maps): tighten optional chain call Signed-off-by: jonathan.casey <[email protected]> * feat(maps): adds additional assertion to satisfy coveralls Signed-off-by: jonathan.casey <[email protected]> * feat(maps): add more cov Signed-off-by: jonathan.casey <[email protected]> * feat(maps): remove optional chaining Signed-off-by: jonathan.casey <[email protected]> --------- Signed-off-by: jonathan.casey <[email protected]>
accordproject · Aug 23, 2023 · 09b3d15 · 09b3d15
1 parent 43c96e0
commit 09b3d15
Show file tree

Hide file tree

Showing 19 changed files with 196 additions and 42 deletions.
diff --git a/packages/concerto-analysis/src/compare-config.ts b/packages/concerto-analysis/src/compare-config.ts
@@ -60,5 +60,7 @@ export const defaultCompareConfig: CompareConfig = {
         'property-validator-added': CompareResult.MAJOR,
         'property-validator-removed': CompareResult.PATCH,
         'property-validator-changed': CompareResult.MAJOR,
+        'map-key-type-changed': CompareResult.MAJOR,
+        'map-value-type-changed': CompareResult.MAJOR
     }
 };
diff --git a/packages/concerto-analysis/src/compare-utils.ts b/packages/concerto-analysis/src/compare-utils.ts
@@ -12,23 +12,28 @@
  * limitations under the License.
  */
 
-import { ClassDeclaration, EnumValueDeclaration, Field, NumberValidator, Property, RelationshipDeclaration, StringValidator, Validator } from '@accordproject/concerto-core';
+import { ClassDeclaration, EnumValueDeclaration, Field, MapDeclaration, NumberValidator, Property, RelationshipDeclaration, StringValidator, Validator } from '@accordproject/concerto-core';
+import Declaration from '@accordproject/concerto-core/types/lib/introspect/declaration';
 
-export function getClassDeclarationType(classDeclaration: ClassDeclaration) {
-    if (classDeclaration.isAsset()) {
-        return 'asset';
-    } else if (classDeclaration.isConcept()) {
-        return 'concept';
-    } else if (classDeclaration.isEnum()) {
-        return 'enum';
-    } else if (classDeclaration.isEvent()) {
-        return 'event';
-    } else if (classDeclaration.isParticipant()) {
-        return 'participant';
-    } else if (classDeclaration.isTransaction()) {
-        return 'transaction';
-    } else {
-        throw new Error(`unknown class declaration type "${classDeclaration}"`);
+export function getDeclarationType(declaration: Declaration) {
+    if (declaration instanceof ClassDeclaration) {
+        if (declaration.isAsset()) {
+            return 'asset';
+        } else if (declaration.isConcept()) {
+            return 'concept';
+        } else if (declaration.isEnum()) {
+            return 'enum';
+        } else if (declaration.isEvent()) {
+            return 'event';
+        } else if (declaration.isParticipant()) {
+            return 'participant';
+        } else if (declaration.isTransaction()) {
+            return 'transaction';
+        } else {
+            throw new Error(`unknown class declaration type "${declaration}"`);
+        }
+    } else if (declaration instanceof MapDeclaration) {
+        return 'map';
     }
 }
 

diff --git a/packages/concerto-analysis/src/compare.ts b/packages/concerto-analysis/src/compare.ts
@@ -12,7 +12,7 @@
  * limitations under the License.
  */
 
-import { ClassDeclaration, ModelFile, Property } from '@accordproject/concerto-core';
+import { ClassDeclaration, MapDeclaration, ModelFile, Property } from '@accordproject/concerto-core';
 import { CompareConfig, CompareResult, defaultCompareConfig } from './compare-config';
 import { CompareFinding } from './compare-message';
 import { CompareResults } from './compare-results';
@@ -65,6 +65,18 @@ export class Compare {
         removed.forEach(a => comparers.forEach(comparer => comparer.compareProperty?.(a, undefined)));
     }
 
+    private getAddedMapDeclarations(a: MapDeclaration[], b: MapDeclaration[]): MapDeclaration[] {
+        return b.filter(bItem => !a.some(aItem => bItem.getName() === aItem.getName()));
+    }
+
+    private getMatchingMapDeclarations(a: MapDeclaration[], b: MapDeclaration[]): [a: MapDeclaration, b: MapDeclaration][] {
+        return a.map(aItem => [aItem, b.find(bItem => aItem.getName() === bItem.getName())]).filter(([, b]) => !!b) as [MapDeclaration, MapDeclaration][];
+    }
+
+    private getRemovedMapDeclarations(a: MapDeclaration[], b: MapDeclaration[]): MapDeclaration[] {
+        return a.filter(aItem => !b.some(bItem => aItem.getName() === bItem.getName()));
+    }
+
     private getAddedClassDeclarations(a: ClassDeclaration[], b: ClassDeclaration[]): ClassDeclaration[] {
         return b.filter(bItem => !a.some(aItem => bItem.getName() === aItem.getName()));
     }
@@ -77,8 +89,21 @@ export class Compare {
         return a.filter(aItem => !b.some(bItem => aItem.getName() === bItem.getName()));
     }
 
+    private compareMapDeclarations(comparers: Comparer[], a: MapDeclaration[], b: MapDeclaration[]) {
+        const added = this.getAddedMapDeclarations(a, b);
+        const matching = this.getMatchingMapDeclarations(a, b);
+        const removed = this.getRemovedMapDeclarations(a, b);
+        added.forEach(b => comparers.forEach(comparer => comparer.compareMapDeclaration?.(undefined, b)));
+        matching.forEach(([a, b]) => comparers.forEach(comparer => comparer.compareMapDeclaration?.(a, b)));
+        removed.forEach(a => comparers.forEach(comparer => comparer.compareMapDeclaration?.(a, undefined)));
+    }
+
     private compareClassDeclaration(comparers: Comparer[], a: ClassDeclaration, b: ClassDeclaration) {
         comparers.forEach(comparer => comparer.compareClassDeclaration?.(a, b));
+        // MapDeclarations do not contain properties, nothing to compare.
+        if(a instanceof MapDeclaration || b instanceof MapDeclaration) {
+            return;
+        }
         this.compareProperties(comparers, a.getOwnProperties(), b.getOwnProperties());
     }
 
@@ -94,6 +119,7 @@ export class Compare {
     private compareModelFiles(comparers: Comparer[], a: ModelFile, b: ModelFile) {
         comparers.forEach(comparer => comparer.compareModelFiles?.(a, b));
         this.compareClassDeclarations(comparers, a.getAllDeclarations(), b.getAllDeclarations());
+        this.compareMapDeclarations(comparers, a.getMapDeclarations(), b.getMapDeclarations());
     }
 
     private buildResults(findings: CompareFinding[]) {

diff --git a/packages/concerto-analysis/src/comparer.ts b/packages/concerto-analysis/src/comparer.ts
@@ -12,7 +12,7 @@
  * limitations under the License.
  */
 
-import { ClassDeclaration, ModelFile, Property } from '@accordproject/concerto-core';
+import { ClassDeclaration, MapDeclaration, ModelFile, Property } from '@accordproject/concerto-core';
 import { CompareContext } from './compare-context';
 
 /**
@@ -34,7 +34,16 @@ export type Comparer = {
     compareClassDeclaration?: (a: ClassDeclaration | undefined, b: ClassDeclaration | undefined) => void;
 
     /**
-     * Called to compare two properties. If a is undefined, but b is defined, then this property was
+     * Called to compare two map declarations. If a is undefined, but b is defined, then this map declaration was
+     * created in the second model. If a is defined, but b is undefined, then this map declaration was removed in the
+     * second model.
+     * @param a The first map declaration for comparision, or undefined if it is undefined in the first model.
+     * @param b The second map declaration for comparision, or undefined if it is undefined in the second model.
+     */
+    compareMapDeclaration?: (a: MapDeclaration | undefined, b: MapDeclaration | undefined) => void;
+
+    /**
+     * Called to compare two properties. If a is undefined, but b is definecd, then this property was
      * created in the second model. If a is defined, but b is undefined, then this property was removed in the
      * second model.
      * @param a The first property for comparision, or undefined if it is undefined in the first model.

diff --git a/packages/concerto-analysis/src/comparers/class-declarations.ts b/packages/concerto-analysis/src/comparers/class-declarations.ts
@@ -12,15 +12,16 @@
  * limitations under the License.
  */
 
-import { getClassDeclarationType } from '../compare-utils';
+import { getDeclarationType } from '../compare-utils';
 import { ComparerFactory } from '../comparer';
 
+// todo rename to declaration.ts , rename all classDeclaration -> declaration
 const classDeclarationAdded: ComparerFactory = (context) => ({
     compareClassDeclaration: (a, b) => {
         if (a || !b) {
             return;
         }
-        const type = getClassDeclarationType(b);
+        const type = getDeclarationType(b);
         context.report({
             key: 'class-declaration-added',
             message: `The ${type} "${b.getName()}" was added`,
@@ -34,7 +35,7 @@ const classDeclarationRemoved: ComparerFactory = (context) => ({
         if (!a || b) {
             return;
         }
-        const type = getClassDeclarationType(a);
+        const type = getDeclarationType(a);
         context.report({
             key: 'class-declaration-removed',
             message: `The ${type} "${a.getName()}" was removed`,
@@ -48,8 +49,8 @@ const classDeclarationTypeChanged: ComparerFactory = (context) => ({
         if (!a || !b) {
             return;
         }
-        const aType = getClassDeclarationType(a);
-        const bType = getClassDeclarationType(b);
+        const aType = getDeclarationType(a);
+        const bType = getDeclarationType(b);
         if (aType === bType) {
             return;
         }

diff --git a/packages/concerto-analysis/src/comparers/index.ts b/packages/concerto-analysis/src/comparers/index.ts
@@ -15,9 +15,11 @@
 import { classDeclarationComparerFactories } from './class-declarations';
 import { modelFileComparerFactories } from './model-files';
 import { propertyComparerFactories } from './properties';
+import { mapDeclarationComparerFactories } from './map-declarations';
 
 export const comparerFactories = [
     ...classDeclarationComparerFactories,
     ...propertyComparerFactories,
-    ...modelFileComparerFactories
+    ...modelFileComparerFactories,
+    ...mapDeclarationComparerFactories
 ];
diff --git a/packages/concerto-analysis/src/comparers/map-declarations.ts b/packages/concerto-analysis/src/comparers/map-declarations.ts
@@ -0,0 +1,42 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { ComparerFactory } from '../comparer';
+
+const mapDeclarationTypeChanged: ComparerFactory = (context) => ({
+    compareMapDeclaration: (a, b) => {
+
+        if (!a || !b) {
+            return;
+        }
+
+        if(a.getKey().getType() !== b.getKey().getType()) {
+            context.report({
+                key: 'map-key-type-changed',
+                message: `The map key type was changed to "${b.getKey().getType()}"`,
+                element: b
+            });
+        }
+
+        if(a.getValue().getType() !== b.getValue().getType()) {
+            context.report({
+                key: 'map-value-type-changed',
+                message: `The map value type was changed to "${b.getValue().getType()}"`,
+                element: b
+            });
+        }
+    },
+});
+
+export const mapDeclarationComparerFactories = [mapDeclarationTypeChanged];
diff --git a/packages/concerto-analysis/src/comparers/properties.ts b/packages/concerto-analysis/src/comparers/properties.ts
@@ -14,15 +14,15 @@
 
 import { EnumValueDeclaration, Field, ModelUtil } from '@accordproject/concerto-core';
 import * as semver from 'semver';
-import { getClassDeclarationType, getPropertyType, getValidatorType } from '../compare-utils';
+import { getDeclarationType, getPropertyType, getValidatorType } from '../compare-utils';
 import { ComparerFactory } from '../comparer';
 
 const propertyAdded: ComparerFactory = (context) => ({
     compareProperty: (a, b) => {
         if (a || !b) {
             return;
         }
-        const classDeclarationType = getClassDeclarationType(b.getParent());
+        const classDeclarationType = getDeclarationType(b.getParent());
         if (b instanceof EnumValueDeclaration) {
             context.report({
                 key: 'enum-value-added',
@@ -58,7 +58,7 @@ const propertyRemoved: ComparerFactory = (context) => ({
         if (!a || b) {
             return;
         }
-        const classDeclarationType = getClassDeclarationType(a.getParent());
+        const classDeclarationType = getDeclarationType(a.getParent());
         if (a instanceof EnumValueDeclaration) {
             context.report({
                 key: 'enum-value-removed',
@@ -96,7 +96,7 @@ const propertyTypeChanged: ComparerFactory = (context) => ({
         }
         const aType = getPropertyType(a);
         const bType = getPropertyType(b);
-        const classDeclarationType = getClassDeclarationType(a.getParent());
+        const classDeclarationType = getDeclarationType(a.getParent());
         if (aType !== bType) {
             context.report({
                 key: 'property-type-changed',
@@ -197,7 +197,7 @@ const propertyValidatorChanged: ComparerFactory = (context) => ({
         }
         const aValidator = a.getValidator();
         const bValidator = b.getValidator();
-        const classDeclarationType = getClassDeclarationType(a.getParent());
+        const classDeclarationType = getDeclarationType(a.getParent());
         if (!aValidator && !bValidator) {
             return;
         } else if (!aValidator && bValidator) {

diff --git a/packages/concerto-analysis/test/fixtures/map-added.cto b/packages/concerto-analysis/test/fixtures/map-added.cto
@@ -0,0 +1,6 @@
+namespace [email protected]
+
+map Thing {
+    o String
+    o String
+}
diff --git a/packages/concerto-analysis/test/fixtures/map-changed-key.cto b/packages/concerto-analysis/test/fixtures/map-changed-key.cto
@@ -0,0 +1,6 @@
+namespace [email protected]
+
+map Thing {
+    o DateTime
+    o String
+}
diff --git a/packages/concerto-analysis/test/fixtures/map-changed-value.cto b/packages/concerto-analysis/test/fixtures/map-changed-value.cto
@@ -0,0 +1,6 @@
+namespace [email protected]
+
+map Thing {
+    o String
+    o DateTime
+}
diff --git a/packages/concerto-analysis/test/unit/compare-utils.test.ts b/packages/concerto-analysis/test/unit/compare-utils.test.ts
@@ -1,5 +1,5 @@
 import { ClassDeclaration, ModelFile, ModelManager, Property, Validator, Field } from '@accordproject/concerto-core';
-import { getClassDeclarationType, getPropertyType, getValidatorType } from '../../src/compare-utils';
+import { getDeclarationType, getPropertyType, getValidatorType } from '../../src/compare-utils';
 
 // This test suite should disappear once we port concerto-core to TypeScript because the error branches will be enforced by the transpiler.
 
@@ -19,7 +19,7 @@ const field = new Field(classDeclaration, propertyAst);
 const validator = new Validator(field, undefined);
 
 test('should throw for unknown class declaration type', () => {
-    expect(() => getClassDeclarationType(classDeclaration)).toThrow('unknown class declaration type "ClassDeclaration {[email protected] super=Concept enum=false abstract=false}"');
+    expect(() => getDeclarationType(classDeclaration)).toThrow('unknown class declaration type "ClassDeclaration {[email protected] super=Concept enum=false abstract=false}"');
 });
 
 test('should throw for unknown class property type', () => {

diff --git a/packages/concerto-analysis/test/unit/compare.test.ts b/packages/concerto-analysis/test/unit/compare.test.ts
@@ -3,6 +3,7 @@ import { Parser } from '@accordproject/concerto-cto';
 import * as fs from 'fs/promises';
 import * as path from 'path';
 import { Compare, CompareResult, compareResultToString } from '../../src';
+import { defaultCompareConfig } from '../../src/compare-config';
 
 async function getModelFile(modelManager: ModelManager, fileName: string) {
     const filePath = path.resolve(__dirname, '..', 'fixtures', fileName);
@@ -68,8 +69,9 @@ test('should detect a change of namespace', async () => {
     expect(results.result).toBe(CompareResult.ERROR);
 });
 
-['asset', 'concept', 'enum', 'event', 'participant', 'transaction'].forEach(type => {
+['asset', 'concept', 'enum', 'event', 'participant', 'transaction', 'map'].forEach(type => {
     test(`should detect a ${type} being added`, async () => {
+        process.env.ENABLE_MAP_TYPE = 'true'; // TODO Remove on release of MapType
         const [a, b] = await getModelFiles('empty.cto', `${type}-added.cto`);
         const results = new Compare().compare(a, b);
         expect(results.findings).toEqual(expect.arrayContaining([
@@ -229,6 +231,34 @@ test('should detect an array changing to a scalar', async () => {
     expect(results.result).toBe(CompareResult.MAJOR);
 });
 
+test('should detect a map key type changing from x to y', async () => {
+    process.env.ENABLE_MAP_TYPE = 'true'; // TODO Remove on release of MapType
+    const [a, b] = await getModelFiles('map-added.cto', 'map-changed-key.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'map-key-type-changed',
+            message: 'The map key type was changed to "DateTime"'
+        })
+    ]));
+    expect(results.findings[0].message).toBe('The map key type was changed to "DateTime"');
+    expect(results.result).toBe(CompareResult.MAJOR);
+});
+
+test('should detect a map value type changing from x to y', async () => {
+    process.env.ENABLE_MAP_TYPE = 'true'; // TODO Remove on release of MapType
+    const [a, b] = await getModelFiles('map-added.cto', 'map-changed-value.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'map-value-type-changed',
+            message: 'The map value type was changed to "DateTime"'
+        })
+    ]));
+    expect(results.findings[0].message).toBe('The map value type was changed to "DateTime"');
+    expect(results.result).toBe(CompareResult.MAJOR);
+});
+
 test('should detect a primitive typed field changing to a declaration typed field', async () => {
     const [a, b] = await getModelFiles('primitive-to-declaration-a.cto', 'primitive-to-declaration-b.cto');
     const results = new Compare().compare(a, b);
@@ -450,3 +480,8 @@ test('should detect a string validator being changed on a property (incompatible
     ]));
     expect(results.result).toBe(CompareResult.MAJOR);
 });
+
+test('should give a MAJOR CompareResult for Map Type compare config rules)', async () => {
+    expect(defaultCompareConfig.rules['map-key-type-changed']).toBe(CompareResult.MAJOR);
+    expect(defaultCompareConfig.rules['map-value-type-changed']).toBe(CompareResult.MAJOR);
+});
diff --git a/packages/concerto-core/api.txt b/packages/concerto-core/api.txt
@@ -160,8 +160,8 @@ class MapDeclaration extends Declaration {
    + string getFullyQualifiedName() 
    + ModelFile getModelFile() 
    + string getName() 
-   + string getKey() 
-   + string getValue() 
+   + MapKeyType getKey() 
+   + MapValueType getValue() 
    + String toString() 
    + string declarationKind() 
    + boolean isMapDeclaration() 

diff --git a/packages/concerto-core/changelog.txt b/packages/concerto-core/changelog.txt
@@ -24,6 +24,9 @@
 # Note that the latest public API is documented using JSDocs and is available in api.txt.
 #
 
+Version 3.10.1 {399b057614d1178c02b744184b09f845} 2023-08-22
+- Change return type for MapDeclaration :: MapKeyType MapValueType
+
 Version 3.10.0 {3bba55da8dbb522de132359415eb7eeb} 2023-08-11
 - Add MetamodelException