From 030f4120a3d6ceaf474233f8e7a69bf81c8b62f8 Mon Sep 17 00:00:00 2001 From: Dan Selman Date: Wed, 4 Oct 2023 14:39:15 +0100 Subject: [PATCH 01/12] feat(decoratormanager) add validate method (#726) * feat(decoratormanager) add validate method Signed-off-by: Ertugrul Karademir --- packages/concerto-core/api.txt | 1 + packages/concerto-core/changelog.txt | 3 +- .../concerto-core/lib/decoratormanager.js | 56 ++++++++++++++----- .../concerto-core/test/decoratormanager.js | 30 ++++++++++ .../types/lib/decoratormanager.d.ts | 15 +++++ 5 files changed, 91 insertions(+), 14 deletions(-) diff --git a/packages/concerto-core/api.txt b/packages/concerto-core/api.txt index 7a070e50e..8470be80c 100644 --- a/packages/concerto-core/api.txt +++ b/packages/concerto-core/api.txt @@ -55,6 +55,7 @@ class Concerto { } + object setCurrentTime() class DecoratorManager { + + ModelManager validate(decoratorCommandSet,ModelFile[]) throws Error + ModelManager decorateModels(ModelManager,decoratorCommandSet,object?,boolean?,boolean?) + void validateCommand(ModelManager,command) + Boolean falsyOrEqual(string||,string[]) diff --git a/packages/concerto-core/changelog.txt b/packages/concerto-core/changelog.txt index d00828ad2..c219a9205 100644 --- a/packages/concerto-core/changelog.txt +++ b/packages/concerto-core/changelog.txt @@ -24,8 +24,9 @@ # Note that the latest public API is documented using JSDocs and is available in api.txt. # -Version 3.13.0 {a7060663ad5bb322ec4ee760baa7ab1a} 2023-10-01 +Version 3.13.0 {125b7f97f8740628b2629b2793384cc7} 2023-10-03 - Update DecoratorManager to support multiple value compare +- Create DecoratorManager.validate method to validate structure of decorator command set Version 3.12.4 {7738d5490ea8438677e1d21d704bb5aa} 2023-08-31 - Adds validate and validateCommands options to DecoratorManager.decorateModels diff --git a/packages/concerto-core/lib/decoratormanager.js b/packages/concerto-core/lib/decoratormanager.js index 1136609dc..718b16d8a 100644 --- a/packages/concerto-core/lib/decoratormanager.js +++ b/packages/concerto-core/lib/decoratormanager.js @@ -19,6 +19,15 @@ const Serializer = require('./serializer'); const Factory = require('./factory'); const ModelUtil = require('./modelutil'); +// Types needed for TypeScript generation. +/* eslint-disable no-unused-vars */ +/* istanbul ignore next */ +if (global === undefined) { + const ModelFile = require('./introspect/modelfile'); +} +/* eslint-enable no-unused-vars */ + + const DCS_MODEL = `concerto version "^3.0.0" namespace org.accordproject.decoratorcommands@0.3.0 @@ -105,6 +114,39 @@ function isUnversionedNamespaceEqual(modelFile, unversionedNamespace) { * @memberof module:concerto-core */ class DecoratorManager { + + /** + * Structural validation of the decoratorCommandSet against the + * Decorator Command Set model. Note that this only checks the + * structural integrity of the command set, it cannot check + * whether the commands are valid with respect to a model manager. + * Use the options.validateCommands option with decorateModels + * method to perform semantic validation. + * @param {*} decoratorCommandSet the DecoratorCommandSet object + * @param {ModelFile[]} [modelFiles] an optional array of model + * files that are added to the validation model manager returned + * @returns {ModelManager} the model manager created for validation + * @throws {Error} throws an error if the decoratorCommandSet is invalid + */ + static validate(decoratorCommandSet, modelFiles) { + const validationModelManager = new ModelManager({ + strict: true, + metamodelValidation: true, + addMetamodel: true, + }); + if(modelFiles) { + validationModelManager.addModelFiles(modelFiles); + } + validationModelManager.addCTOModel( + DCS_MODEL, + 'decoratorcommands@0.3.0.cto' + ); + const factory = new Factory(validationModelManager); + const serializer = new Serializer(factory, validationModelManager); + serializer.fromJSON(decoratorCommandSet); + return validationModelManager; + } + /** * Applies all the decorator commands from the DecoratorCommandSet * to the ModelManager. @@ -119,19 +161,7 @@ class DecoratorManager { */ static decorateModels(modelManager, decoratorCommandSet, options) { if (options?.validate) { - const validationModelManager = new ModelManager({ - strict: true, - metamodelValidation: true, - addMetamodel: true, - }); - validationModelManager.addModelFiles(modelManager.getModelFiles()); - validationModelManager.addCTOModel( - DCS_MODEL, - 'decoratorcommands@0.2.0.cto' - ); - const factory = new Factory(validationModelManager); - const serializer = new Serializer(factory, validationModelManager); - serializer.fromJSON(decoratorCommandSet); + const validationModelManager = DecoratorManager.validate(decoratorCommandSet, modelManager.getModelFiles()); if (options?.validateCommands) { decoratorCommandSet.commands.forEach((command) => { DecoratorManager.validateCommand( diff --git a/packages/concerto-core/test/decoratormanager.js b/packages/concerto-core/test/decoratormanager.js index ac31e6c41..a635d10cb 100644 --- a/packages/concerto-core/test/decoratormanager.js +++ b/packages/concerto-core/test/decoratormanager.js @@ -57,6 +57,36 @@ describe('DecoratorManager', () => { }); }); + describe('#validate', function() { + it('should support syntax validation', async function() { + const dcs = fs.readFileSync('./test/data/decoratorcommands/web.json', 'utf-8'); + const validationModelManager = DecoratorManager.validate( JSON.parse(dcs)); + validationModelManager.should.not.be.null; + }); + + it('should support syntax validation with model files', async function() { + const testModelManager = new ModelManager({strict:true}); + const modelText = fs.readFileSync('./test/data/decoratorcommands/test.cto', 'utf-8'); + testModelManager.addCTOModel(modelText, 'test.cto'); + const dcs = fs.readFileSync('./test/data/decoratorcommands/web.json', 'utf-8'); + const validationModelManager = DecoratorManager.validate(JSON.parse(dcs), testModelManager.getModelFiles()); + validationModelManager.should.not.be.null; + validationModelManager.getType('test@1.0.0.Person').should.not.be.null; + }); + + it('should fail syntax validation', async function() { + (() => { + DecoratorManager.validate( { $class: 'invalid' }); + }).should.throw(/Namespace is not defined for type/); + }); + + it('should fail syntax validation', async function() { + (() => { + DecoratorManager.validate( { invalid: true }); + }).should.throw(/Invalid JSON data/); + }); + }); + describe('#decorateModels', function() { it('should support no validation', async function() { const testModelManager = new ModelManager({strict:true}); diff --git a/packages/concerto-core/types/lib/decoratormanager.d.ts b/packages/concerto-core/types/lib/decoratormanager.d.ts index 480e9c816..183409d8a 100644 --- a/packages/concerto-core/types/lib/decoratormanager.d.ts +++ b/packages/concerto-core/types/lib/decoratormanager.d.ts @@ -5,6 +5,20 @@ export = DecoratorManager; * @memberof module:concerto-core */ declare class DecoratorManager { + /** + * Structural validation of the decoratorCommandSet against the + * Decorator Command Set model. Note that this only checks the + * structural integrity of the command set, it cannot check + * whether the commands are valid with respect to a model manager. + * Use the options.validateCommands option with decorateModels + * method to perform semantic validation. + * @param {*} decoratorCommandSet the DecoratorCommandSet object + * @param {ModelFile[]} [modelFiles] an optional array of model + * files that are added to the validation model manager returned + * @returns {ModelManager} the model manager created for validation + * @throws {Error} throws an error if the decoratorCommandSet is invalid + */ + static validate(decoratorCommandSet: any, modelFiles?: ModelFile[]): ModelManager; /** * Applies all the decorator commands from the DecoratorCommandSet * to the ModelManager. @@ -61,4 +75,5 @@ declare class DecoratorManager { */ static executePropertyCommand(property: any, command: any): void; } +import ModelFile = require("./introspect/modelfile"); import ModelManager = require("./modelmanager"); From 43b790a347b125a627d3f9b2ac6933d1277351c0 Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Thu, 5 Oct 2023 10:22:06 +0100 Subject: [PATCH 02/12] Add a builder for the config Signed-off-by: Ertugrul Karademir --- .../concerto-analysis/src/compare-config.ts | 154 +++++++++++++----- 1 file changed, 116 insertions(+), 38 deletions(-) diff --git a/packages/concerto-analysis/src/compare-config.ts b/packages/concerto-analysis/src/compare-config.ts index cb36687e6..98ee2482d 100644 --- a/packages/concerto-analysis/src/compare-config.ts +++ b/packages/concerto-analysis/src/compare-config.ts @@ -16,55 +16,133 @@ import { ComparerFactory } from './comparer'; import { comparerFactories } from './comparers'; export enum CompareResult { - NONE, - PATCH, - MINOR, - MAJOR, - ERROR, + NONE, + PATCH, + MINOR, + MAJOR, + ERROR, } export function compareResultToString(result: CompareResult) { - switch (result) { + switch (result) { case CompareResult.NONE: - return 'none'; + return 'none'; case CompareResult.PATCH: - return 'patch'; + return 'patch'; case CompareResult.MINOR: - return 'minor'; + return 'minor'; case CompareResult.MAJOR: - return 'major'; + return 'major'; case CompareResult.ERROR: - return 'error'; - } + return 'error'; + } } export type CompareConfig = { - comparerFactories: ComparerFactory[]; - rules: Record; -} + comparerFactories: ComparerFactory[]; + rules: Record; +}; export const defaultCompareConfig: CompareConfig = { - comparerFactories, - rules: { - 'class-declaration-added': CompareResult.MINOR, - 'class-declaration-removed': CompareResult.MAJOR, - 'class-declaration-type-changed': CompareResult.MAJOR, - 'required-property-added': CompareResult.MAJOR, - 'optional-property-added': CompareResult.PATCH, - 'required-property-removed': CompareResult.MAJOR, - 'optional-property-removed': CompareResult.MAJOR, - 'namespace-changed': CompareResult.ERROR, - 'enum-value-added': CompareResult.PATCH, - 'enum-value-removed': CompareResult.MAJOR, - 'property-type-changed': CompareResult.MAJOR, - '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, - 'scalar-extends-changed': CompareResult.MAJOR, - 'scalar-validator-added' : CompareResult.MAJOR, - 'scalar-validator-removed' : CompareResult.PATCH, - 'scalar-validator-changed' : CompareResult.MAJOR, - } + comparerFactories, + rules: { + 'class-declaration-added': CompareResult.MINOR, + 'class-declaration-removed': CompareResult.MAJOR, + 'class-declaration-type-changed': CompareResult.MAJOR, + 'required-property-added': CompareResult.MAJOR, + 'optional-property-added': CompareResult.PATCH, + 'required-property-removed': CompareResult.MAJOR, + 'optional-property-removed': CompareResult.MAJOR, + 'namespace-changed': CompareResult.ERROR, + 'enum-value-added': CompareResult.PATCH, + 'enum-value-removed': CompareResult.MAJOR, + 'property-type-changed': CompareResult.MAJOR, + '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, + 'scalar-extends-changed': CompareResult.MAJOR, + 'scalar-validator-added': CompareResult.MAJOR, + 'scalar-validator-removed': CompareResult.PATCH, + 'scalar-validator-changed': CompareResult.MAJOR, + }, +}; + +const EmptyConfig: CompareConfig = { + comparerFactories: [], + rules: {}, }; + +export class CompareConfigBuilder { + /** + * A utility to build CompareConfig to be used in Compare class. + * A new compare config can be edited with provided functions and finally + * resulting config can be used by calling `build`. + * + * By default, it starts with an emoty configuration. + */ + + private _config: CompareConfig = EmptyConfig; + + /** + * Final step of the builder + * + * @returns Resulting CompareConfig object. + */ + public build(): CompareConfig { + return { ...this._config }; + } + + /** + * Sets default comparer configuration. + */ + public default() { + this._config = { ...defaultCompareConfig }; + } + + /** + * Extends existing configuration tha't built up to this point + * with the provided config. + * + * @param config + */ + public extend(config: CompareConfig) { + this._config = { + comparerFactories: [...this._config.comparerFactories, ...config.comparerFactories], + rules: { ...this._config.rules, ...config.rules }, + }; + } + + /** + * Adds a comparison outcome rule to the configuration + * + * @param ruleKey - A key that is referenced from one of the comparer factories + * @param result - A version diff outcome based on this rule + */ + public addRule(ruleKey: string, result: CompareResult) { + this._config.rules[ruleKey] = result; + } + + /** + * Removes a comparison outcome rule from the configuration + * + * @param ruleKey - A key that is referenced from one of the comparer factories + * @throws {ReferenceError} + * Thrown if the `ruleKey` does not exist in the configuration + */ + public removeRule(ruleKey: string) { + if (!this._config.rules[ruleKey]) throw new ReferenceError(`ruleKey '${ruleKey}' does not exist`); + + delete this._config.rules[ruleKey]; + } + + /** + * Add a {@link ComparerFactory} to the configuration. + * + * @param f - A {@link ComparerFactory} that should reference the rules in the configuration + */ + public addCompareFactory(f: ComparerFactory) { + this._config.comparerFactories = [...this._config.comparerFactories, f]; + } +} From c5a08a7166ddc52c42ba845c182867e18cfaa319 Mon Sep 17 00:00:00 2001 From: Jonathan-Casey <109082377+Jonathan-Casey@users.noreply.github.com> Date: Mon, 9 Oct 2023 18:06:10 +0100 Subject: [PATCH 03/12] feat(map): add property based flag (#728) * feat(map): add property based flag Signed-off-by: Jonathan Casey * fix typo Signed-off-by: Jonathan Casey --------- Signed-off-by: Jonathan Casey Signed-off-by: Ertugrul Karademir --- .../concerto-core/lib/basemodelmanager.js | 4 ++++ .../lib/introspect/mapdeclaration.js | 5 +++-- .../test/introspect/mapdeclaration.js | 22 +++++++++++++++++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/concerto-core/lib/basemodelmanager.js b/packages/concerto-core/lib/basemodelmanager.js index 2ea73bd58..365bd8ea0 100644 --- a/packages/concerto-core/lib/basemodelmanager.js +++ b/packages/concerto-core/lib/basemodelmanager.js @@ -93,6 +93,10 @@ class BaseModelManager { this.options = options; this.addRootModel(); + // TODO Remove on release of MapType + // Supports both env var and property based flag + this.enableMapType = !!options?.enableMapType; + // Cache a copy of the Metamodel ModelFile for use when validating the structure of ModelFiles later. this.metamodelModelFile = new ModelFile(this, MetaModelUtil.metaModelAst, undefined, MetaModelNamespace); diff --git a/packages/concerto-core/lib/introspect/mapdeclaration.js b/packages/concerto-core/lib/introspect/mapdeclaration.js index 2ddc54373..0bcc06e6c 100644 --- a/packages/concerto-core/lib/introspect/mapdeclaration.js +++ b/packages/concerto-core/lib/introspect/mapdeclaration.js @@ -45,8 +45,9 @@ class MapDeclaration extends Declaration { */ constructor(modelFile, ast) { // TODO remove on full release. - if(process.env.ENABLE_MAP_TYPE !== 'true') { - throw new Error('MapType feature is not enabled. Please set the environment variable "ENABLE_MAP_TYPE=true" to access this functionality.'); + const mm = modelFile.getModelManager(); + if(process.env.ENABLE_MAP_TYPE !== 'true' && !mm.enableMapType) { + throw new Error('MapType feature is not enabled. Please set the environment variable "ENABLE_MAP_TYPE=true", or add {enableMapType: true} to the ModelManger options, to access this functionality.'); } super(modelFile, ast); diff --git a/packages/concerto-core/test/introspect/mapdeclaration.js b/packages/concerto-core/test/introspect/mapdeclaration.js index 7b9b4bf2e..641abd06c 100644 --- a/packages/concerto-core/test/introspect/mapdeclaration.js +++ b/packages/concerto-core/test/introspect/mapdeclaration.js @@ -84,11 +84,29 @@ describe('MapDeclaration', () => { $class: 'concerto.metamodel@1.0.0.StringMapValueType' } }); - }).should.throw(/MapType feature is not enabled. Please set the environment variable "ENABLE_MAP_TYPE=true" to access this functionality./); + }).should.throw(/MapType feature is not enabled. Please set the environment variable "ENABLE_MAP_TYPE=true", or add {enableMapType: true} to the ModelManger options, to access this functionality/); process.env.ENABLE_MAP_TYPE = 'true'; // enable after the test run. This is necessary to ensure functioning of other tests. }); - + it('should throw if Map Type not enabled in ModelManager options', () => { + process.env.ENABLE_MAP_TYPE = 'false'; + const mm = new ModelManager({enableMapType: false}); + Util.addComposerModel(mm); + const introspectUtils = new IntrospectUtils(mm); + try { + introspectUtils.loadLastDeclaration('test/data/parser/mapdeclaration/mapdeclaration.goodkey.primitive.datetime.cto', MapDeclaration); + } catch (error) { + expect(error.message).to.equal('MapType feature is not enabled. Please set the environment variable "ENABLE_MAP_TYPE=true", or add {enableMapType: true} to the ModelManger options, to access this functionality.'); + } + }); + + it('should not throw if Map Type is enabled in ModelManager options', () => { + const mm = new ModelManager({enableMapType: true}); + Util.addComposerModel(mm); + const introspectUtils = new IntrospectUtils(mm); + let decl = introspectUtils.loadLastDeclaration('test/data/parser/mapdeclaration/mapdeclaration.goodkey.primitive.datetime.cto', MapDeclaration); + decl.validate(); + }); it('should throw if invalid $class provided for Map Key', () => { (() => From 05ad62b1f601b84a4e1d87281eb29b55379270f5 Mon Sep 17 00:00:00 2001 From: Jonathan-Casey <109082377+Jonathan-Casey@users.noreply.github.com> Date: Tue, 10 Oct 2023 11:56:30 +0100 Subject: [PATCH 04/12] Jonathan/add options jsdoc (#729) * feat(map): add jsdoc for enableMapType option Signed-off-by: Jonathan Casey * chore(*): update changelog Signed-off-by: Jonathan Casey --------- Signed-off-by: Jonathan Casey Signed-off-by: Ertugrul Karademir --- packages/concerto-core/api.txt | 2 +- packages/concerto-core/changelog.txt | 3 +++ packages/concerto-core/lib/basemodelmanager.js | 1 + packages/concerto-core/types/lib/basemodelmanager.d.ts | 4 ++++ 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/concerto-core/api.txt b/packages/concerto-core/api.txt index 8470be80c..d3d29dac7 100644 --- a/packages/concerto-core/api.txt +++ b/packages/concerto-core/api.txt @@ -2,7 +2,7 @@ class AstModelManager extends BaseModelManager { + void constructor(object?) } class BaseModelManager { - + void constructor(object?,boolean?,Object?,boolean?,boolean?,processFile?) + + void constructor(object?,boolean?,Object?,boolean?,boolean?,boolean?,processFile?) + boolean isModelManager() + boolean isStrict() + Object accept(Object,Object) diff --git a/packages/concerto-core/changelog.txt b/packages/concerto-core/changelog.txt index c219a9205..81a893ad0 100644 --- 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.13.1 {6b09c1c58abcc77eecbb44e375c2efb8} 2023-10-03 +- Add enableMapType option to BaseModelManager options + Version 3.13.0 {125b7f97f8740628b2629b2793384cc7} 2023-10-03 - Update DecoratorManager to support multiple value compare - Create DecoratorManager.validate method to validate structure of decorator command set diff --git a/packages/concerto-core/lib/basemodelmanager.js b/packages/concerto-core/lib/basemodelmanager.js index 365bd8ea0..9c2a36f1c 100644 --- a/packages/concerto-core/lib/basemodelmanager.js +++ b/packages/concerto-core/lib/basemodelmanager.js @@ -81,6 +81,7 @@ class BaseModelManager { * @param {Object} [options.regExp] - An alternative regular expression engine. * @param {boolean} [options.metamodelValidation] - When true, modelfiles will be validated * @param {boolean} [options.addMetamodel] - When true, the Concerto metamodel is added to the model manager + * @param {boolean} [options.enableMapType] - When true, the Concerto Map Type feature is enabled * @param {*} [processFile] - how to obtain a concerto AST from an input to the model manager */ constructor(options, processFile) { diff --git a/packages/concerto-core/types/lib/basemodelmanager.d.ts b/packages/concerto-core/types/lib/basemodelmanager.d.ts index bd5eb922a..ccf0fdd4e 100644 --- a/packages/concerto-core/types/lib/basemodelmanager.d.ts +++ b/packages/concerto-core/types/lib/basemodelmanager.d.ts @@ -23,6 +23,7 @@ declare class BaseModelManager { * @param {Object} [options.regExp] - An alternative regular expression engine. * @param {boolean} [options.metamodelValidation] - When true, modelfiles will be validated * @param {boolean} [options.addMetamodel] - When true, the Concerto metamodel is added to the model manager + * @param {boolean} [options.enableMapType] - When true, the Concerto Map Type feature is enabled * @param {*} [processFile] - how to obtain a concerto AST from an input to the model manager */ constructor(options?: { @@ -30,6 +31,7 @@ declare class BaseModelManager { regExp?: any; metamodelValidation?: boolean; addMetamodel?: boolean; + enableMapType?: boolean; }, processFile?: any); processFile: any; modelFiles: {}; @@ -42,7 +44,9 @@ declare class BaseModelManager { regExp?: any; metamodelValidation?: boolean; addMetamodel?: boolean; + enableMapType?: boolean; }; + enableMapType: boolean; metamodelModelFile: any; /** * Returns true From db3176786272f7ed64d766a0b3224fc005d15288 Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Tue, 10 Oct 2023 16:10:12 +0100 Subject: [PATCH 05/12] Lint and test Signed-off-by: Ertugrul Karademir --- .../concerto-analysis/src/compare-config.ts | 138 +++++++++--------- .../test/unit/compare-config.test.ts | 87 +++++++++++ 2 files changed, 157 insertions(+), 68 deletions(-) create mode 100644 packages/concerto-analysis/test/unit/compare-config.test.ts diff --git a/packages/concerto-analysis/src/compare-config.ts b/packages/concerto-analysis/src/compare-config.ts index 98ee2482d..ae684b02e 100644 --- a/packages/concerto-analysis/src/compare-config.ts +++ b/packages/concerto-analysis/src/compare-config.ts @@ -24,18 +24,18 @@ export enum CompareResult { } export function compareResultToString(result: CompareResult) { - switch (result) { + switch (result) { case CompareResult.NONE: - return 'none'; + return 'none'; case CompareResult.PATCH: - return 'patch'; + return 'patch'; case CompareResult.MINOR: - return 'minor'; + return 'minor'; case CompareResult.MAJOR: - return 'major'; + return 'major'; case CompareResult.ERROR: - return 'error'; - } + return 'error'; + } } export type CompareConfig = { @@ -44,38 +44,38 @@ export type CompareConfig = { }; export const defaultCompareConfig: CompareConfig = { - comparerFactories, - rules: { - 'class-declaration-added': CompareResult.MINOR, - 'class-declaration-removed': CompareResult.MAJOR, - 'class-declaration-type-changed': CompareResult.MAJOR, - 'required-property-added': CompareResult.MAJOR, - 'optional-property-added': CompareResult.PATCH, - 'required-property-removed': CompareResult.MAJOR, - 'optional-property-removed': CompareResult.MAJOR, - 'namespace-changed': CompareResult.ERROR, - 'enum-value-added': CompareResult.PATCH, - 'enum-value-removed': CompareResult.MAJOR, - 'property-type-changed': CompareResult.MAJOR, - '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, - 'scalar-extends-changed': CompareResult.MAJOR, - 'scalar-validator-added': CompareResult.MAJOR, - 'scalar-validator-removed': CompareResult.PATCH, - 'scalar-validator-changed': CompareResult.MAJOR, - }, + comparerFactories, + rules: { + 'class-declaration-added': CompareResult.MINOR, + 'class-declaration-removed': CompareResult.MAJOR, + 'class-declaration-type-changed': CompareResult.MAJOR, + 'required-property-added': CompareResult.MAJOR, + 'optional-property-added': CompareResult.PATCH, + 'required-property-removed': CompareResult.MAJOR, + 'optional-property-removed': CompareResult.MAJOR, + 'namespace-changed': CompareResult.ERROR, + 'enum-value-added': CompareResult.PATCH, + 'enum-value-removed': CompareResult.MAJOR, + 'property-type-changed': CompareResult.MAJOR, + '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, + 'scalar-extends-changed': CompareResult.MAJOR, + 'scalar-validator-added': CompareResult.MAJOR, + 'scalar-validator-removed': CompareResult.PATCH, + 'scalar-validator-changed': CompareResult.MAJOR, + }, }; const EmptyConfig: CompareConfig = { - comparerFactories: [], - rules: {}, + comparerFactories: [], + rules: {}, }; export class CompareConfigBuilder { - /** + /** * A utility to build CompareConfig to be used in Compare class. * A new compare config can be edited with provided functions and finally * resulting config can be used by calling `build`. @@ -83,66 +83,68 @@ export class CompareConfigBuilder { * By default, it starts with an emoty configuration. */ - private _config: CompareConfig = EmptyConfig; + private _config: CompareConfig = EmptyConfig; - /** + /** * Final step of the builder * - * @returns Resulting CompareConfig object. + * @returns {CompareConfig} Resulting CompareConfig object. */ - public build(): CompareConfig { - return { ...this._config }; - } + public build(): CompareConfig { + return { ...this._config }; + } - /** + /** * Sets default comparer configuration. */ - public default() { - this._config = { ...defaultCompareConfig }; - } + public default() { + this._config = { ...defaultCompareConfig }; + } - /** + /** * Extends existing configuration tha't built up to this point * with the provided config. * - * @param config + * @param {CompareConfig} config - The configuration to extend with */ - public extend(config: CompareConfig) { - this._config = { - comparerFactories: [...this._config.comparerFactories, ...config.comparerFactories], - rules: { ...this._config.rules, ...config.rules }, - }; - } + public extend(config: CompareConfig) { + this._config = { + comparerFactories: [...this._config.comparerFactories, ...config.comparerFactories], + rules: { ...this._config.rules, ...config.rules }, + }; + } - /** + /** * Adds a comparison outcome rule to the configuration * - * @param ruleKey - A key that is referenced from one of the comparer factories - * @param result - A version diff outcome based on this rule + * @param {string} ruleKey - A key that is referenced from one of the comparer factories + * @param {CompareResult} result - A version diff outcome based on this rule */ - public addRule(ruleKey: string, result: CompareResult) { - this._config.rules[ruleKey] = result; - } + public addRule(ruleKey: string, result: CompareResult) { + this._config.rules[ruleKey] = result; + } - /** + /** * Removes a comparison outcome rule from the configuration * - * @param ruleKey - A key that is referenced from one of the comparer factories + * @param {string} ruleKey - A key that is referenced from one of the comparer factories * @throws {ReferenceError} * Thrown if the `ruleKey` does not exist in the configuration */ - public removeRule(ruleKey: string) { - if (!this._config.rules[ruleKey]) throw new ReferenceError(`ruleKey '${ruleKey}' does not exist`); + public removeRule(ruleKey: string) { + if (!this._config.rules[ruleKey]) { + throw new ReferenceError(`ruleKey '${ruleKey}' does not exist`); + } - delete this._config.rules[ruleKey]; - } + delete this._config.rules[ruleKey]; + } - /** + /** * Add a {@link ComparerFactory} to the configuration. * - * @param f - A {@link ComparerFactory} that should reference the rules in the configuration + * @param {ComparerFactory} f - A {@link ComparerFactory} that should reference the rules in the configuration */ - public addCompareFactory(f: ComparerFactory) { - this._config.comparerFactories = [...this._config.comparerFactories, f]; - } + public addCompareFactory(f: ComparerFactory) { + this._config.comparerFactories = [...this._config.comparerFactories, f]; + } } diff --git a/packages/concerto-analysis/test/unit/compare-config.test.ts b/packages/concerto-analysis/test/unit/compare-config.test.ts new file mode 100644 index 000000000..e6e8f6bbb --- /dev/null +++ b/packages/concerto-analysis/test/unit/compare-config.test.ts @@ -0,0 +1,87 @@ +import { CompareConfig, CompareConfigBuilder, CompareResult } from '../../src/compare-config'; + +describe('CompareConfigBuilder', () => { + it('Should start with empty config', () => { + const builder = new CompareConfigBuilder(); + + const actual = builder.build(); + + expect(actual.comparerFactories.length).toEqual(0); + expect(Object.keys(actual.rules).length).toEqual(0); + }); + + it('Should add default config with `default`', () => { + const builder = new CompareConfigBuilder(); + builder.default(); + + const actual = builder.build(); + + expect(actual.comparerFactories.length).toEqual(11); + expect(Object.keys(actual.rules).length).toEqual(20); + expect(actual.rules['class-declaration-added']).toEqual(CompareResult.MINOR); + expect(actual.rules['optional-property-added']).toEqual(CompareResult.PATCH); + expect(actual.rules['map-value-type-changed']).toEqual(CompareResult.MAJOR); + }); + + it('Should extend config', () => { + const newRules = { + 'a-new-rule': CompareResult.MAJOR + }; + const toExtend: CompareConfig = { + comparerFactories: [ + () => ({}), + ], + rules: newRules + }; + const builder = new CompareConfigBuilder(); + builder.default(); + builder.extend(toExtend); + + const actual = builder.build(); + + expect(actual.comparerFactories.length).toEqual(12); + expect(Object.keys(actual.rules).length).toEqual(21); + expect(actual.rules['a-new-rule']).toEqual(CompareResult.MAJOR); + }); + + it('Should add a new comparer factory', () => { + const builder = new CompareConfigBuilder(); + builder.default(); + builder.addCompareFactory(() => ({})); + + const actual = builder.build(); + + expect(actual.comparerFactories.length).toEqual(12); + expect(Object.keys(actual.rules).length).toEqual(20); + }); + + it('Should add a new rule', () => { + const builder = new CompareConfigBuilder(); + builder.default(); + builder.addRule('a-new-rule', CompareResult.MAJOR); + + const actual = builder.build(); + + expect(actual.comparerFactories.length).toEqual(11); + expect(Object.keys(actual.rules).length).toEqual(21); + expect(actual.rules['a-new-rule']).toEqual(CompareResult.MAJOR); + }); + + it('Should remove an existing rule', () => { + const builder = new CompareConfigBuilder(); + builder.default(); + builder.removeRule('optional-property-added'); + + const actual = builder.build(); + + expect(actual.comparerFactories.length).toEqual(11); + expect(Object.keys(actual.rules).length).toEqual(20); + expect(actual.rules['optional-property-added']).toBeFalsy(); + }); + + it('Should throw while removing a rule that does not exist', () => { + const builder = new CompareConfigBuilder(); + builder.default(); + expect(() => builder.removeRule('does-not-exist')).toThrow('ruleKey \'does-not-exist\' does not exist'); + }); +}); From 200acfcb8090368a6ec1035aaa93cc34384e5b3a Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Tue, 10 Oct 2023 18:03:53 +0100 Subject: [PATCH 06/12] Signing off Signed-off-by: Ertugrul Karademir From 49ce66907c1fb9a21e6c9f9ad45d1981d4669372 Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Tue, 10 Oct 2023 18:22:30 +0100 Subject: [PATCH 07/12] Renaming a method Signed-off-by: Ertugrul Karademir --- packages/concerto-analysis/src/compare-config.ts | 2 +- packages/concerto-analysis/test/unit/compare-config.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/concerto-analysis/src/compare-config.ts b/packages/concerto-analysis/src/compare-config.ts index ae684b02e..926d58ad6 100644 --- a/packages/concerto-analysis/src/compare-config.ts +++ b/packages/concerto-analysis/src/compare-config.ts @@ -144,7 +144,7 @@ export class CompareConfigBuilder { * * @param {ComparerFactory} f - A {@link ComparerFactory} that should reference the rules in the configuration */ - public addCompareFactory(f: ComparerFactory) { + public addComparerFactory(f: ComparerFactory) { this._config.comparerFactories = [...this._config.comparerFactories, f]; } } diff --git a/packages/concerto-analysis/test/unit/compare-config.test.ts b/packages/concerto-analysis/test/unit/compare-config.test.ts index e6e8f6bbb..10587e9b9 100644 --- a/packages/concerto-analysis/test/unit/compare-config.test.ts +++ b/packages/concerto-analysis/test/unit/compare-config.test.ts @@ -47,7 +47,7 @@ describe('CompareConfigBuilder', () => { it('Should add a new comparer factory', () => { const builder = new CompareConfigBuilder(); builder.default(); - builder.addCompareFactory(() => ({})); + builder.addComparerFactory(() => ({})); const actual = builder.build(); From 93d6e1bd84119a269f81a908c1113df292f8ac4e Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Tue, 10 Oct 2023 18:24:04 +0100 Subject: [PATCH 08/12] Fix comment alignments Signed-off-by: Ertugrul Karademir --- .../concerto-analysis/src/compare-config.ts | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/packages/concerto-analysis/src/compare-config.ts b/packages/concerto-analysis/src/compare-config.ts index 926d58ad6..17555e154 100644 --- a/packages/concerto-analysis/src/compare-config.ts +++ b/packages/concerto-analysis/src/compare-config.ts @@ -76,37 +76,37 @@ const EmptyConfig: CompareConfig = { export class CompareConfigBuilder { /** - * A utility to build CompareConfig to be used in Compare class. - * A new compare config can be edited with provided functions and finally - * resulting config can be used by calling `build`. - * - * By default, it starts with an emoty configuration. - */ + * A utility to build CompareConfig to be used in Compare class. + * A new compare config can be edited with provided functions and finally + * resulting config can be used by calling `build`. + * + * By default, it starts with an emoty configuration. + */ private _config: CompareConfig = EmptyConfig; /** - * Final step of the builder - * - * @returns {CompareConfig} Resulting CompareConfig object. - */ + * Final step of the builder + * + * @returns {CompareConfig} Resulting CompareConfig object. + */ public build(): CompareConfig { return { ...this._config }; } /** - * Sets default comparer configuration. - */ + * Sets default comparer configuration. + */ public default() { this._config = { ...defaultCompareConfig }; } /** - * Extends existing configuration tha't built up to this point - * with the provided config. - * - * @param {CompareConfig} config - The configuration to extend with - */ + * Extends existing configuration tha't built up to this point + * with the provided config. + * + * @param {CompareConfig} config - The configuration to extend with + */ public extend(config: CompareConfig) { this._config = { comparerFactories: [...this._config.comparerFactories, ...config.comparerFactories], @@ -115,22 +115,22 @@ export class CompareConfigBuilder { } /** - * Adds a comparison outcome rule to the configuration - * - * @param {string} ruleKey - A key that is referenced from one of the comparer factories - * @param {CompareResult} result - A version diff outcome based on this rule - */ + * Adds a comparison outcome rule to the configuration + * + * @param {string} ruleKey - A key that is referenced from one of the comparer factories + * @param {CompareResult} result - A version diff outcome based on this rule + */ public addRule(ruleKey: string, result: CompareResult) { this._config.rules[ruleKey] = result; } /** - * Removes a comparison outcome rule from the configuration - * - * @param {string} ruleKey - A key that is referenced from one of the comparer factories - * @throws {ReferenceError} - * Thrown if the `ruleKey` does not exist in the configuration - */ + * Removes a comparison outcome rule from the configuration + * + * @param {string} ruleKey - A key that is referenced from one of the comparer factories + * @throws {ReferenceError} + * Thrown if the `ruleKey` does not exist in the configuration + */ public removeRule(ruleKey: string) { if (!this._config.rules[ruleKey]) { throw new ReferenceError(`ruleKey '${ruleKey}' does not exist`); @@ -140,10 +140,10 @@ export class CompareConfigBuilder { } /** - * Add a {@link ComparerFactory} to the configuration. - * - * @param {ComparerFactory} f - A {@link ComparerFactory} that should reference the rules in the configuration - */ + * Add a {@link ComparerFactory} to the configuration. + * + * @param {ComparerFactory} f - A {@link ComparerFactory} that should reference the rules in the configuration + */ public addComparerFactory(f: ComparerFactory) { this._config.comparerFactories = [...this._config.comparerFactories, f]; } From 8a03577b300dc0e9f116b8f91f103c4a1852b2d7 Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Tue, 10 Oct 2023 18:37:21 +0100 Subject: [PATCH 09/12] Clone instead of copy Signed-off-by: Ertugrul Karademir --- packages/concerto-analysis/src/compare-config.ts | 13 ++++++++++--- .../test/unit/compare-config.test.ts | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/concerto-analysis/src/compare-config.ts b/packages/concerto-analysis/src/compare-config.ts index 17555e154..a99d636db 100644 --- a/packages/concerto-analysis/src/compare-config.ts +++ b/packages/concerto-analysis/src/compare-config.ts @@ -91,14 +91,21 @@ export class CompareConfigBuilder { * @returns {CompareConfig} Resulting CompareConfig object. */ public build(): CompareConfig { - return { ...this._config }; + return { + comparerFactories: [...this._config.comparerFactories], + rules: { ...this._config.rules }, + }; } /** - * Sets default comparer configuration. + * Adds default comparer configuration onto the configuration + * baing built. */ public default() { - this._config = { ...defaultCompareConfig }; + this._config = { + comparerFactories: [...this._config.comparerFactories, ...defaultCompareConfig.comparerFactories], + rules: { ...this._config.rules, ...defaultCompareConfig.rules }, + }; } /** diff --git a/packages/concerto-analysis/test/unit/compare-config.test.ts b/packages/concerto-analysis/test/unit/compare-config.test.ts index 10587e9b9..24aeaf569 100644 --- a/packages/concerto-analysis/test/unit/compare-config.test.ts +++ b/packages/concerto-analysis/test/unit/compare-config.test.ts @@ -75,7 +75,7 @@ describe('CompareConfigBuilder', () => { const actual = builder.build(); expect(actual.comparerFactories.length).toEqual(11); - expect(Object.keys(actual.rules).length).toEqual(20); + expect(Object.keys(actual.rules).length).toEqual(19); expect(actual.rules['optional-property-added']).toBeFalsy(); }); From 6cb8697e89cc8353c950e6ad2652fc46c5251f41 Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Tue, 10 Oct 2023 18:41:09 +0100 Subject: [PATCH 10/12] Export builder with the main package Signed-off-by: Ertugrul Karademir --- packages/concerto-analysis/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/concerto-analysis/src/index.ts b/packages/concerto-analysis/src/index.ts index 9555d6d14..2f8bf3324 100644 --- a/packages/concerto-analysis/src/index.ts +++ b/packages/concerto-analysis/src/index.ts @@ -13,6 +13,6 @@ */ export { Compare } from './compare'; -export { CompareConfig, CompareResult, compareResultToString } from './compare-config'; +export { CompareConfig, CompareResult, CompareConfigBuilder, compareResultToString } from './compare-config'; export { CompareFinding, CompareResults } from './compare-results'; From 0dbc9befd6b06ebc9162a44a6ad0aeec07d254b0 Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Fri, 13 Oct 2023 15:33:42 +0100 Subject: [PATCH 11/12] Typo fixes Signed-off-by: Ertugrul Karademir --- packages/concerto-analysis/src/compare-config.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/concerto-analysis/src/compare-config.ts b/packages/concerto-analysis/src/compare-config.ts index a99d636db..85b5b6700 100644 --- a/packages/concerto-analysis/src/compare-config.ts +++ b/packages/concerto-analysis/src/compare-config.ts @@ -76,11 +76,11 @@ const EmptyConfig: CompareConfig = { export class CompareConfigBuilder { /** - * A utility to build CompareConfig to be used in Compare class. + * A utility to build {@link CompareConfig} to be used in {@link Compare} class. * A new compare config can be edited with provided functions and finally * resulting config can be used by calling `build`. * - * By default, it starts with an emoty configuration. + * By default, it starts with an empty configuration. */ private _config: CompareConfig = EmptyConfig; @@ -98,9 +98,9 @@ export class CompareConfigBuilder { } /** - * Adds default comparer configuration onto the configuration - * baing built. - */ + * Adds default comparer configuration onto the configuration + * being built. + */ public default() { this._config = { comparerFactories: [...this._config.comparerFactories, ...defaultCompareConfig.comparerFactories], @@ -109,7 +109,7 @@ export class CompareConfigBuilder { } /** - * Extends existing configuration tha't built up to this point + * Extends existing configuration that's built upto this point * with the provided config. * * @param {CompareConfig} config - The configuration to extend with From f5628181ba9d0ffc10dd9f8916181aee3951a4e8 Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Fri, 13 Oct 2023 15:40:19 +0100 Subject: [PATCH 12/12] Complete builder pattern Signed-off-by: Ertugrul Karademir --- .../concerto-analysis/src/compare-config.ts | 26 +++++++++++++++---- .../test/unit/compare-config.test.ts | 22 +++++----------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/packages/concerto-analysis/src/compare-config.ts b/packages/concerto-analysis/src/compare-config.ts index 85b5b6700..75c725e91 100644 --- a/packages/concerto-analysis/src/compare-config.ts +++ b/packages/concerto-analysis/src/compare-config.ts @@ -100,12 +100,16 @@ export class CompareConfigBuilder { /** * Adds default comparer configuration onto the configuration * being built. + * + * @returns {CompareConfigBuilder} A reference to the builder object to chain */ - public default() { + public default(): CompareConfigBuilder { this._config = { comparerFactories: [...this._config.comparerFactories, ...defaultCompareConfig.comparerFactories], rules: { ...this._config.rules, ...defaultCompareConfig.rules }, }; + + return this; } /** @@ -113,12 +117,15 @@ export class CompareConfigBuilder { * with the provided config. * * @param {CompareConfig} config - The configuration to extend with + * @returns {CompareConfigBuilder} A reference to the builder object to chain */ - public extend(config: CompareConfig) { + public extend(config: CompareConfig): CompareConfigBuilder { this._config = { comparerFactories: [...this._config.comparerFactories, ...config.comparerFactories], rules: { ...this._config.rules, ...config.rules }, }; + + return this; } /** @@ -126,32 +133,41 @@ export class CompareConfigBuilder { * * @param {string} ruleKey - A key that is referenced from one of the comparer factories * @param {CompareResult} result - A version diff outcome based on this rule + * @returns {CompareConfigBuilder} A reference to the builder object to chain */ - public addRule(ruleKey: string, result: CompareResult) { + public addRule(ruleKey: string, result: CompareResult): CompareConfigBuilder { this._config.rules[ruleKey] = result; + + return this; } /** * Removes a comparison outcome rule from the configuration * * @param {string} ruleKey - A key that is referenced from one of the comparer factories + * @returns {CompareConfigBuilder} A reference to the builder object to chain * @throws {ReferenceError} * Thrown if the `ruleKey` does not exist in the configuration */ - public removeRule(ruleKey: string) { + public removeRule(ruleKey: string): CompareConfigBuilder { if (!this._config.rules[ruleKey]) { throw new ReferenceError(`ruleKey '${ruleKey}' does not exist`); } delete this._config.rules[ruleKey]; + + return this; } /** * Add a {@link ComparerFactory} to the configuration. * * @param {ComparerFactory} f - A {@link ComparerFactory} that should reference the rules in the configuration + * @returns {CompareConfigBuilder} A reference to the builder object to chain */ - public addComparerFactory(f: ComparerFactory) { + public addComparerFactory(f: ComparerFactory): CompareConfigBuilder { this._config.comparerFactories = [...this._config.comparerFactories, f]; + + return this; } } diff --git a/packages/concerto-analysis/test/unit/compare-config.test.ts b/packages/concerto-analysis/test/unit/compare-config.test.ts index 24aeaf569..d37fcba63 100644 --- a/packages/concerto-analysis/test/unit/compare-config.test.ts +++ b/packages/concerto-analysis/test/unit/compare-config.test.ts @@ -12,9 +12,8 @@ describe('CompareConfigBuilder', () => { it('Should add default config with `default`', () => { const builder = new CompareConfigBuilder(); - builder.default(); - const actual = builder.build(); + const actual = builder.default().build(); expect(actual.comparerFactories.length).toEqual(11); expect(Object.keys(actual.rules).length).toEqual(20); @@ -34,10 +33,8 @@ describe('CompareConfigBuilder', () => { rules: newRules }; const builder = new CompareConfigBuilder(); - builder.default(); - builder.extend(toExtend); - const actual = builder.build(); + const actual = builder.default().extend(toExtend).build(); expect(actual.comparerFactories.length).toEqual(12); expect(Object.keys(actual.rules).length).toEqual(21); @@ -46,10 +43,8 @@ describe('CompareConfigBuilder', () => { it('Should add a new comparer factory', () => { const builder = new CompareConfigBuilder(); - builder.default(); - builder.addComparerFactory(() => ({})); - const actual = builder.build(); + const actual = builder.default().addComparerFactory(() => ({})).build(); expect(actual.comparerFactories.length).toEqual(12); expect(Object.keys(actual.rules).length).toEqual(20); @@ -57,10 +52,8 @@ describe('CompareConfigBuilder', () => { it('Should add a new rule', () => { const builder = new CompareConfigBuilder(); - builder.default(); - builder.addRule('a-new-rule', CompareResult.MAJOR); - const actual = builder.build(); + const actual = builder.default().addRule('a-new-rule', CompareResult.MAJOR).build(); expect(actual.comparerFactories.length).toEqual(11); expect(Object.keys(actual.rules).length).toEqual(21); @@ -69,10 +62,8 @@ describe('CompareConfigBuilder', () => { it('Should remove an existing rule', () => { const builder = new CompareConfigBuilder(); - builder.default(); - builder.removeRule('optional-property-added'); - const actual = builder.build(); + const actual = builder.default().removeRule('optional-property-added').build(); expect(actual.comparerFactories.length).toEqual(11); expect(Object.keys(actual.rules).length).toEqual(19); @@ -81,7 +72,6 @@ describe('CompareConfigBuilder', () => { it('Should throw while removing a rule that does not exist', () => { const builder = new CompareConfigBuilder(); - builder.default(); - expect(() => builder.removeRule('does-not-exist')).toThrow('ruleKey \'does-not-exist\' does not exist'); + expect(() => builder.default().removeRule('does-not-exist')).toThrow('ruleKey \'does-not-exist\' does not exist'); }); });