Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 15 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/concerto-analysis/src/compare-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
};
37 changes: 21 additions & 16 deletions packages/concerto-analysis/src/compare-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
}

Expand Down
28 changes: 27 additions & 1 deletion packages/concerto-analysis/src/compare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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()));
}
Expand All @@ -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());
}

Expand All @@ -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[]) {
Expand Down
13 changes: 11 additions & 2 deletions packages/concerto-analysis/src/comparer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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.
Expand Down
11 changes: 6 additions & 5 deletions packages/concerto-analysis/src/comparers/class-declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand All @@ -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`,
Expand All @@ -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;
}
Expand Down
4 changes: 3 additions & 1 deletion packages/concerto-analysis/src/comparers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
];
42 changes: 42 additions & 0 deletions packages/concerto-analysis/src/comparers/map-declarations.ts
Original file line number Diff line number Diff line change
@@ -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
jonathan-casey marked this conversation as resolved.
Show resolved Hide resolved
});
}

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
jonathan-casey marked this conversation as resolved.
Show resolved Hide resolved
});
}
},
});

export const mapDeclarationComparerFactories = [mapDeclarationTypeChanged];
10 changes: 5 additions & 5 deletions packages/concerto-analysis/src/comparers/properties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions packages/concerto-analysis/test/fixtures/map-added.cto
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace [email protected]

map Thing {
o String
o String
}
6 changes: 6 additions & 0 deletions packages/concerto-analysis/test/fixtures/map-changed-key.cto
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace [email protected]

map Thing {
o DateTime
o String
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace [email protected]

map Thing {
o String
o DateTime
}
4 changes: 2 additions & 2 deletions packages/concerto-analysis/test/unit/compare-utils.test.ts
Original file line number Diff line number Diff line change
@@ -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.

Expand All @@ -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', () => {
Expand Down
37 changes: 36 additions & 1 deletion packages/concerto-analysis/test/unit/compare.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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([
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
});
4 changes: 2 additions & 2 deletions packages/concerto-core/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions packages/concerto-core/changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading