From 6747535d6b9b31f06c391ad58bbea23ec0cdbce3 Mon Sep 17 00:00:00 2001 From: Siddharth-Dangi Date: Mon, 2 Nov 2026 18:19:26 +0530 Subject: [PATCH] fix(concerto-core): validate metamodel AST bounds and required structures Metamodel AST instances created directly (e.g. deserialized from JSON or inbound from external tools) could bypass structural validation and propagate invalid state down into printers, generators, and serializers: - NumberValidator accepted ASTs with undefined or empty bounds ({ lower: undefined, upper: undefined }) because hasOwnProperty check bypassed null guards, resulting in an unconfigured validator and returning undefined bounds. Additionally, null or undefined ASTs threw uncaught TypeErrors. - StringValidator accepted regex validators missing a pattern, with pattern: null, or with an empty pattern, which created RegExp(/null/) or /(?:)/ instead of reporting a model error. - StringValidator length bounds did not use NullUtil.isNull consistently, causing empty length bounds to be treated differently depending on whether properties were null or undefined; furthermore non-integer bounds were not rejected. - CollectionSizeValidator accepted null/undefined validators without a clean model error and did not reject non-integer bounds. - ObjectProperty with null or missing type was silently accepted during validate(), leading to downstream crashes (e.g. Printer.toCTO throwing TypeError: Cannot read properties of null (reading 'name')). - Null or undefined elements in ModelFile declarations or imports threw raw TypeErrors rather than IllegalModelException; non-array imports and declarations now also throw IllegalModelException. - Null or undefined properties in ClassDeclaration properties threw raw TypeErrors rather than IllegalModelException. - ObjectMapKeyType, ObjectMapValueType, and RelationshipMapValueType threw raw TypeErrors when type was null or missing instead of IllegalModelException. - BaseModelManager.fromAst threw raw TypeErrors when ast or ast.models was null or contained null elements. - BaseModelManager.validateAst leaked the metamodel namespace into modelFiles if deserialization validation threw an exception before cleanup. Normalize bounds using NullUtil.isNull, ensure length and size bounds are positive integers, fail fast with IllegalModelException when required structures are null, empty, or non-arrays, ensure regex pattern is a non-empty string, and clean up temporary metamodel files in a finally block. Signed-off-by: Siddharth-Dangi --- .../concerto-core/src/basemodelmanager.ts | 14 +- .../src/introspect/classdeclaration.ts | 8 + .../src/introspect/collectionsizevalidator.ts | 9 +- .../src/introspect/mapkeytype.ts | 7 + .../src/introspect/mapvaluetype.ts | 6 +- .../concerto-core/src/introspect/modelfile.ts | 15 + .../src/introspect/numbervalidator.ts | 22 +- .../concerto-core/src/introspect/property.ts | 10 +- .../src/introspect/stringvalidator.ts | 39 ++- .../concerto-core/test/astmodelmanager.js | 257 ++++++++++++++++++ .../introspect/collectionsizevalidator.js | 28 ++ .../test/introspect/mapdeclaration.js | 78 ++++++ .../test/introspect/numbervalidator.js | 42 +++ .../concerto-core/test/introspect/property.js | 47 ++++ .../test/introspect/stringvalidator.js | 58 ++++ packages/concerto-core/test/modelmanager.js | 50 +++- 16 files changed, 650 insertions(+), 40 deletions(-) diff --git a/packages/concerto-core/src/basemodelmanager.ts b/packages/concerto-core/src/basemodelmanager.ts index 994d98bd57..b29919b4d7 100644 --- a/packages/concerto-core/src/basemodelmanager.ts +++ b/packages/concerto-core/src/basemodelmanager.ts @@ -294,10 +294,10 @@ class BaseModelManager { } catch (err: unknown) { const error = err as Error; throw new MetamodelException(error.message); - } - - if (!alreadyHasMetamodel) { - this.deleteModelFile(MetaModelNamespace); + } finally { + if (!alreadyHasMetamodel) { + this.deleteModelFile(MetaModelNamespace); + } } } @@ -854,7 +854,13 @@ class BaseModelManager { */ fromAst(ast: IModels, options?: { disableValidation?: boolean }) { this.clearModelFiles(); + if (!ast || !Array.isArray(ast.models)) { + throw new IllegalModelException('Models AST must contain a models array'); + } ast.models.forEach( (model: IModel) => { + if (!model) { + throw new IllegalModelException('Model cannot be null or undefined'); + } if(!EXCLUDE_NS.includes(model.namespace)) { // excludes the internal namespaces, already added const modelFile = new ModelFile( this, model ); this.addModelFile( modelFile, null, null, true ); diff --git a/packages/concerto-core/src/introspect/classdeclaration.ts b/packages/concerto-core/src/introspect/classdeclaration.ts index e86fba57a9..c9e3d49245 100644 --- a/packages/concerto-core/src/introspect/classdeclaration.ts +++ b/packages/concerto-core/src/introspect/classdeclaration.ts @@ -108,6 +108,14 @@ class ClassDeclaration extends Declaration { for (let n = 0; n < this.ast.properties.length; n++) { let thing = this.ast.properties[n]; + if (!thing) { + throw new IllegalModelException( + `Property cannot be null or undefined, for class ${this.name}`, + this.modelFile, + this.ast.location + ); + } + if(ModelUtil.isSystemProperty(thing.name)) { throw new IllegalModelException(`Invalid field name '${thing.name}'`, this.modelFile, this.ast.location); } diff --git a/packages/concerto-core/src/introspect/collectionsizevalidator.ts b/packages/concerto-core/src/introspect/collectionsizevalidator.ts index 16180bc061..75d38c5dd0 100644 --- a/packages/concerto-core/src/introspect/collectionsizevalidator.ts +++ b/packages/concerto-core/src/introspect/collectionsizevalidator.ts @@ -43,12 +43,15 @@ class CollectionSizeValidator extends Validator { */ constructor(field: ValidatedElement, validator: ICollectionSizeValidator) { super(field, validator); - this.minSize = validator.minSize ?? null; - this.maxSize = validator.maxSize ?? null; + this.minSize = !isNull(validator?.minSize) ? validator.minSize : null; + this.maxSize = !isNull(validator?.maxSize) ? validator.maxSize : null; if (isNull(this.minSize) && isNull(this.maxSize)) { this.reportModelError('Invalid collection size, minSize and/or maxSize must be specified.'); - } else if ((this.minSize ?? 0) < 0 || (this.maxSize ?? 0) < 0) { + } else if ( + (this.minSize !== null && (!Number.isInteger(this.minSize) || this.minSize < 0)) || + (this.maxSize !== null && (!Number.isInteger(this.maxSize) || this.maxSize < 0)) + ) { this.reportModelError('minSize and/or maxSize must be positive integers.'); } else if (isNull(this.minSize) || isNull(this.maxSize)) { // this is fine and means that we don't need to check whether minSize > maxSize diff --git a/packages/concerto-core/src/introspect/mapkeytype.ts b/packages/concerto-core/src/introspect/mapkeytype.ts index 20d009d45b..2b138a6fee 100644 --- a/packages/concerto-core/src/introspect/mapkeytype.ts +++ b/packages/concerto-core/src/introspect/mapkeytype.ts @@ -96,6 +96,13 @@ class MapKeyType extends Decorated { this.type = 'String'; break; case `${MetaModelNamespace}.ObjectMapKeyType`: + if (!this.ast.type || !this.ast.type.name) { + throw new IllegalModelException( + `ObjectMapKeyType must contain property 'type', for MapDeclaration named ${this.parent.name}`, + this.modelFile, + this.ast.location + ); + } this.type = String(this.ast.type.name); break; } diff --git a/packages/concerto-core/src/introspect/mapvaluetype.ts b/packages/concerto-core/src/introspect/mapvaluetype.ts index c99896cf19..e8b49f4428 100644 --- a/packages/concerto-core/src/introspect/mapvaluetype.ts +++ b/packages/concerto-core/src/introspect/mapvaluetype.ts @@ -108,17 +108,17 @@ class MapValueType extends Decorated { case `${MetaModelNamespace}.RelationshipMapValueType`: // ObjectMapValueType must have TypeIdentifier. - if (!('type' in ast)) { + if (!('type' in ast) || !ast.type) { throw new IllegalModelException(`ObjectMapValueType must contain property 'type', for MapDeclaration named ${this.parent.name}`); } // ObjectMapValueType TypeIdentifier must be properly formed. - if (!('$class' in ast.type) || !('name' in ast.type)) { + if (typeof ast.type !== 'object' || !('$class' in ast.type) || !('name' in ast.type)) { throw new IllegalModelException(`ObjectMapValueType type must contain property '$class' and property 'name', for MapDeclaration named ${this.parent.name}`); } // And the $class must be valid. - if (ast.type.$class !== 'concerto.metamodel@1.0.0.TypeIdentifier') { + if (ast.type.$class !== `${MetaModelNamespace}.TypeIdentifier`) { throw new IllegalModelException(`ObjectMapValueType type $class must be of TypeIdentifier for MapDeclaration named ${this.parent.name}`); } diff --git a/packages/concerto-core/src/introspect/modelfile.ts b/packages/concerto-core/src/introspect/modelfile.ts index 2bdd2cc4d0..d54f67c104 100644 --- a/packages/concerto-core/src/introspect/modelfile.ts +++ b/packages/concerto-core/src/introspect/modelfile.ts @@ -766,6 +766,10 @@ class ModelFile extends Decorated { throw new Error(`Cannot create a ModelFile with an unversioned namespace: ${ast.namespace}. All models must specify a version (e.g., @1.0.0).`); } + if (ast.imports && !Array.isArray(ast.imports)) { + throw new IllegalModelException('Model imports must be an array', this, this.ast.location); + } + // Make sure to clone imports since we will add built-in imports const imports = ast.imports ? ast.imports.concat([]) : []; @@ -781,6 +785,9 @@ class ModelFile extends Decorated { this.imports = imports; this.imports.forEach((imp) => { + if (!imp) { + throw new IllegalModelException('Import cannot be null or undefined', this, this.ast.location); + } this.enforceImportVersioning(imp); switch(imp.$class) { case `${MetaModelNamespace}.ImportAll`: @@ -820,9 +827,17 @@ class ModelFile extends Decorated { return; } + if (!Array.isArray(ast.declarations)) { + throw new IllegalModelException('Model declarations must be an array', this, this.ast.location); + } + for(let n=0; n < ast.declarations.length; n++) { let thing = ast.declarations[n]; + if (!thing) { + throw new IllegalModelException('Declaration cannot be null or undefined', this, this.ast.location); + } + switch(thing.$class) { case `${MetaModelNamespace}.AssetDeclaration`: // Default super type for asset diff --git a/packages/concerto-core/src/introspect/numbervalidator.ts b/packages/concerto-core/src/introspect/numbervalidator.ts index b0e184d750..d6621085eb 100644 --- a/packages/concerto-core/src/introspect/numbervalidator.ts +++ b/packages/concerto-core/src/introspect/numbervalidator.ts @@ -48,25 +48,21 @@ class NumberValidator extends Validator{ constructor(field: ValidatedElement, ast: NumberDomainValidatorAst) { super(field, ast); - this.lowerBound = null; - this.upperBound = null; + this.lowerBound = !isNull(ast?.lower) ? (ast.lower as number) : null; + this.upperBound = !isNull(ast?.upper) ? (ast.upper as number) : null; - // the hasOwnProperty guards establish that the bound is present - if(Object.prototype.hasOwnProperty.call(ast, 'lower')) { - this.lowerBound = ast.lower as number; - } - - if(Object.prototype.hasOwnProperty.call(ast, 'upper')) { - this.upperBound = ast.upper as number; - } - - if(this.lowerBound === null && this.upperBound === null) { + if (this.lowerBound === null && this.upperBound === null) { // can't specify no upper and lower value this.reportModelError('Invalid range, lower and-or upper bound must be specified.'); + } else if ( + (this.lowerBound !== null && (typeof this.lowerBound !== 'number' || Number.isNaN(this.lowerBound))) || + (this.upperBound !== null && (typeof this.upperBound !== 'number' || Number.isNaN(this.upperBound))) + ) { + this.reportModelError('lower and-or upper bound must be numbers.'); } else if (this.lowerBound === null || this.upperBound === null) { // this is fine and means that we don't need to check whether upper > lower } else { - if(this.lowerBound > this.upperBound) { + if (this.lowerBound > this.upperBound) { this.reportModelError('Lower bound must be less than or equal to upper bound.'); } } diff --git a/packages/concerto-core/src/introspect/property.ts b/packages/concerto-core/src/introspect/property.ts index f73bfc8d58..17aaddd92c 100644 --- a/packages/concerto-core/src/introspect/property.ts +++ b/packages/concerto-core/src/introspect/property.ts @@ -151,6 +151,14 @@ class Property extends Decorated { validate(classDecl: ClassDeclaration) { super.validate(); + if (this.ast.$class === `${MetaModelNamespace}.ObjectProperty` && (!this.ast.type || !this.ast.type.name)) { + throw new IllegalModelException( + `ObjectProperty must contain property 'type', for property ${this.ast.name}`, + this.getModelFile(), + this.ast.location + ); + } + if(this.type) { classDecl.getModelFile().resolveType( 'property ' + this.getFullyQualifiedName(), this.type); } @@ -160,7 +168,7 @@ class Property extends Decorated { if(this.type && !this.isPrimitive()) { try { const resolvedType = classDecl.getModelFile().getType(this.type); - isMapType = resolvedType.isMapDeclaration?.() === true; + isMapType = (resolvedType as any)?.isMapDeclaration?.() === true; } catch(e) { // type resolution failed — will be caught by other validation } diff --git a/packages/concerto-core/src/introspect/stringvalidator.ts b/packages/concerto-core/src/introspect/stringvalidator.ts index 1290e4402f..5ec95e46c4 100644 --- a/packages/concerto-core/src/introspect/stringvalidator.ts +++ b/packages/concerto-core/src/introspect/stringvalidator.ts @@ -57,35 +57,44 @@ class StringValidator extends Validator{ this.regex = null; if (lengthValidator) { - this.minLength = lengthValidator?.minLength; - this.maxLength = lengthValidator?.maxLength; + this.minLength = !isNull(lengthValidator.minLength) ? lengthValidator.minLength : null; + this.maxLength = !isNull(lengthValidator.maxLength) ? lengthValidator.maxLength : null; - if ((this.minLength === null || this.minLength === undefined) && - (this.maxLength === null || this.maxLength === undefined)) { + if (this.minLength === null && this.maxLength === null) { // can't specify no upper and lower value this.reportModelError('Invalid string length, minLength and-or maxLength must be specified.'); - } else if ((this.minLength ?? 0) < 0 || (this.maxLength ?? 0) < 0) { + } else if ( + (this.minLength !== null && (!Number.isInteger(this.minLength) || this.minLength < 0)) || + (this.maxLength !== null && (!Number.isInteger(this.maxLength) || this.maxLength < 0)) + ) { this.reportModelError('minLength and-or maxLength must be positive integers.'); } else if (this.minLength === null || this.maxLength === null) { // this is fine and means that we don't need to check whether minLength > maxLength - } else if (this.minLength !== undefined && this.maxLength !== undefined && this.minLength > this.maxLength) { + } else if (this.minLength > this.maxLength) { this.reportModelError('minLength must be less than or equal to maxLength.'); } } if (validator) { - try { - // ScalarDeclarations have no parent, so the custom RegExp option - // is only picked up for properties - const parent = 'getParent' in field ? field.getParent() : undefined; - const CustomRegExp = (parent?.getModelFile()?.getModelManager()?.options?.regExp || RegExp) as typeof RegExp; - this.regex = new CustomRegExp(validator.pattern, validator.flags); - } - catch (exception) { + if (isNull(validator.pattern) || typeof validator.pattern !== 'string' || validator.pattern.length === 0) { this.reportModelError( - (exception as Error).message, + 'Regex pattern must be specified.', ErrorCodes.REGEX_VALIDATOR_EXCEPTION ); + } else { + try { + // ScalarDeclarations have no parent, so the custom RegExp option + // is only picked up for properties + const parent = 'getParent' in field ? field.getParent() : undefined; + const CustomRegExp = (parent?.getModelFile()?.getModelManager()?.options?.regExp || RegExp) as typeof RegExp; + this.regex = new CustomRegExp(validator.pattern, validator.flags || undefined); + } + catch (exception) { + this.reportModelError( + (exception as Error).message, + ErrorCodes.REGEX_VALIDATOR_EXCEPTION + ); + } } } diff --git a/packages/concerto-core/test/astmodelmanager.js b/packages/concerto-core/test/astmodelmanager.js index 9f72e11ea7..90f13fcb51 100644 --- a/packages/concerto-core/test/astmodelmanager.js +++ b/packages/concerto-core/test/astmodelmanager.js @@ -108,6 +108,263 @@ describe('AstModelManager', () => { /Decorator cannot be null or undefined/ ); }); + + it('should throw IllegalModelException for a null declaration', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': [], + 'declarations': [null] + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /Declaration cannot be null or undefined/ + ); + }); + + it('should throw IllegalModelException for a null import', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': [null], + 'declarations': [] + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /Import cannot be null or undefined/ + ); + }); + + it('should throw IllegalModelException for non-array imports', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': {}, + 'declarations': [] + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /Model imports must be an array/ + ); + }); + + it('should throw IllegalModelException for non-array declarations', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': [], + 'declarations': {} + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /Model declarations must be an array/ + ); + }); + + it('should throw IllegalModelException for a null property in a declaration', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': [], + 'declarations': [ + { + '$class': 'concerto.metamodel@1.0.0.ConceptDeclaration', + 'name': 'A', + 'isAbstract': false, + 'properties': [null] + } + ] + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /Property cannot be null or undefined, for class A/ + ); + }); + + it('should throw IllegalModelException for an ObjectProperty with null type', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': [], + 'declarations': [ + { + '$class': 'concerto.metamodel@1.0.0.ConceptDeclaration', + 'name': 'A', + 'isAbstract': false, + 'properties': [ + { + '$class': 'concerto.metamodel@1.0.0.ObjectProperty', + 'name': 'target', + 'isArray': false, + 'isOptional': false, + 'type': null + } + ] + } + ] + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /ObjectProperty must contain property 'type', for property target/ + ); + }); + + it('should throw IllegalModelException for a StringRegexValidator with null pattern', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': [], + 'declarations': [ + { + '$class': 'concerto.metamodel@1.0.0.ConceptDeclaration', + 'name': 'A', + 'isAbstract': false, + 'properties': [ + { + '$class': 'concerto.metamodel@1.0.0.StringProperty', + 'name': 'str', + 'isArray': false, + 'isOptional': false, + 'validator': { + '$class': 'concerto.metamodel@1.0.0.StringRegexValidator', + 'pattern': null + } + } + ] + } + ] + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /Regex pattern must be specified/ + ); + }); + + it('should throw IllegalModelException for an IntegerDomainValidator with undefined bounds', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': [], + 'declarations': [ + { + '$class': 'concerto.metamodel@1.0.0.ConceptDeclaration', + 'name': 'A', + 'isAbstract': false, + 'properties': [ + { + '$class': 'concerto.metamodel@1.0.0.IntegerProperty', + 'name': 'num', + 'isArray': false, + 'isOptional': false, + 'validator': { + '$class': 'concerto.metamodel@1.0.0.IntegerDomainValidator', + 'lower': undefined, + 'upper': undefined + } + } + ] + } + ] + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /Invalid range, lower and-or upper bound must be specified/ + ); + }); + + it('should throw IllegalModelException for a CollectionSizeValidator with undefined bounds', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': [], + 'declarations': [ + { + '$class': 'concerto.metamodel@1.0.0.ConceptDeclaration', + 'name': 'A', + 'isAbstract': false, + 'properties': [ + { + '$class': 'concerto.metamodel@1.0.0.StringProperty', + 'name': 'tags', + 'isArray': true, + 'isOptional': false, + 'sizeValidator': { + '$class': 'concerto.metamodel@1.0.0.CollectionSizeValidator', + 'minSize': undefined, + 'maxSize': undefined + } + } + ] + } + ] + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /Invalid collection size, minSize and\/or maxSize must be specified/ + ); + }); + + it('should throw IllegalModelException when sizeValidator is applied to a non-array property', () => { + const modelContent = { + '$class': 'concerto.metamodel@1.0.0.Model', + 'namespace': 'org.acme@1.0.0', + 'imports': [], + 'declarations': [ + { + '$class': 'concerto.metamodel@1.0.0.ConceptDeclaration', + 'name': 'A', + 'isAbstract': false, + 'properties': [ + { + '$class': 'concerto.metamodel@1.0.0.StringProperty', + 'name': 'scalarField', + 'isArray': false, + 'isOptional': false, + 'sizeValidator': { + '$class': 'concerto.metamodel@1.0.0.CollectionSizeValidator', + 'minSize': 1, + 'maxSize': 5 + } + } + ] + } + ] + }; + + (() => { + modelManager.addModel(modelContent, null, 'a.cto'); + }).should.throw( + IllegalModelException, + /size validator can only be applied to array or map properties/ + ); + }); }); }); diff --git a/packages/concerto-core/test/introspect/collectionsizevalidator.js b/packages/concerto-core/test/introspect/collectionsizevalidator.js index 7fd736cc6c..75583760c2 100644 --- a/packages/concerto-core/test/introspect/collectionsizevalidator.js +++ b/packages/concerto-core/test/introspect/collectionsizevalidator.js @@ -52,6 +52,34 @@ describe('CollectionSizeValidator', () => { .should.throw(/minSize and\/or maxSize must be specified/); }); + it('should throw when minSize and maxSize are undefined or null', () => { + (() => new CollectionSizeValidator(mockField, { minSize: undefined, maxSize: undefined })) + .should.throw(/minSize and\/or maxSize must be specified/); + (() => new CollectionSizeValidator(mockField, { minSize: null, maxSize: null })) + .should.throw(/minSize and\/or maxSize must be specified/); + }); + + it('should throw when validator object is null or undefined', () => { + (() => new CollectionSizeValidator(mockField, null)) + .should.throw(/minSize and\/or maxSize must be specified/); + (() => new CollectionSizeValidator(mockField, undefined)) + .should.throw(/minSize and\/or maxSize must be specified/); + }); + + it('should throw when minSize is not an integer', () => { + (() => new CollectionSizeValidator(mockField, { minSize: 1.5 })) + .should.throw(/positive integers/); + (() => new CollectionSizeValidator(mockField, { minSize: NaN })) + .should.throw(/positive integers/); + }); + + it('should throw when maxSize is not an integer', () => { + (() => new CollectionSizeValidator(mockField, { maxSize: 2.5 })) + .should.throw(/positive integers/); + (() => new CollectionSizeValidator(mockField, { maxSize: NaN })) + .should.throw(/positive integers/); + }); + it('should throw when minSize is negative', () => { (() => new CollectionSizeValidator(mockField, { minSize: -1 })) .should.throw(/positive integers/); diff --git a/packages/concerto-core/test/introspect/mapdeclaration.js b/packages/concerto-core/test/introspect/mapdeclaration.js index f565a030f1..2d9886672b 100644 --- a/packages/concerto-core/test/introspect/mapdeclaration.js +++ b/packages/concerto-core/test/introspect/mapdeclaration.js @@ -413,6 +413,37 @@ describe('MapDeclaration', () => { describe('#validate failure scenarios - Map Value', () => { + it('should throw if ObjectMapKeyType does not contain type property', () => { + (() => { + new MapDeclaration(modelFile, { + $class: 'concerto.metamodel@1.0.0.MapDeclaration', + name: 'MapPermutation0', + key: { + $class: 'concerto.metamodel@1.0.0.ObjectMapKeyType' + }, + value: { + $class: 'concerto.metamodel@1.0.0.StringMapValueType', + } + }); + }).should.throw(IllegalModelException, /ObjectMapKeyType must contain property 'type'/); + }); + + it('should throw if ObjectMapKeyType contains null type property', () => { + (() => { + new MapDeclaration(modelFile, { + $class: 'concerto.metamodel@1.0.0.MapDeclaration', + name: 'MapPermutation0', + key: { + $class: 'concerto.metamodel@1.0.0.ObjectMapKeyType', + type: null + }, + value: { + $class: 'concerto.metamodel@1.0.0.StringMapValueType', + } + }); + }).should.throw(IllegalModelException, /ObjectMapKeyType must contain property 'type'/); + }); + it('should throw if ObjectMapValueType does not contain type property ', () => { (() => { new MapDeclaration(modelFile, { @@ -428,6 +459,22 @@ describe('MapDeclaration', () => { }).should.throw(IllegalModelException); }); + it('should throw if ObjectMapValueType contains null type property', () => { + (() => { + new MapDeclaration(modelFile, { + $class: 'concerto.metamodel@1.0.0.MapDeclaration', + name: 'MapPermutation1', + key: { + $class: 'concerto.metamodel@1.0.0.StringMapKeyType' + }, + value: { + $class: 'concerto.metamodel@1.0.0.ObjectMapValueType', + type: null + } + }); + }).should.throw(IllegalModelException, /ObjectMapValueType must contain property 'type'/); + }); + it('should throw if ObjectMapValueType TypeIdentifier does not contain name property', () => { (() => { new MapDeclaration(modelFile, { @@ -465,6 +512,37 @@ describe('MapDeclaration', () => { }).should.throw(IllegalModelException); }); + it('should throw if RelationshipMapValueType does not contain type property', () => { + (() => { + new MapDeclaration(modelFile, { + $class: 'concerto.metamodel@1.0.0.MapDeclaration', + name: 'MapPermutation1', + key: { + $class: 'concerto.metamodel@1.0.0.StringMapKeyType' + }, + value: { + $class: 'concerto.metamodel@1.0.0.RelationshipMapValueType', + } + }); + }).should.throw(IllegalModelException); + }); + + it('should throw if RelationshipMapValueType contains null type property', () => { + (() => { + new MapDeclaration(modelFile, { + $class: 'concerto.metamodel@1.0.0.MapDeclaration', + name: 'MapPermutation1', + key: { + $class: 'concerto.metamodel@1.0.0.StringMapKeyType' + }, + value: { + $class: 'concerto.metamodel@1.0.0.RelationshipMapValueType', + type: null + } + }); + }).should.throw(IllegalModelException, /ObjectMapValueType must contain property 'type'/); + }); + it('should throw if ast contains illegal Map Value Property', () => { (() => { new MapDeclaration(modelFile, { diff --git a/packages/concerto-core/test/introspect/numbervalidator.js b/packages/concerto-core/test/introspect/numbervalidator.js index c3eac8725b..c2f1186aff 100644 --- a/packages/concerto-core/test/introspect/numbervalidator.js +++ b/packages/concerto-core/test/introspect/numbervalidator.js @@ -66,6 +66,48 @@ describe('NumberValidator', () => { }).should.throw(/Invalid range, lower and-or upper bound must be specified./); }); + it('should throw an error for constructor parms with undefined bounds', () => { + (() => { + new NumberValidator(mockField, { lower: undefined, upper: undefined }); + }).should.throw(/Invalid range, lower and-or upper bound must be specified./); + }); + + it('should throw an error for constructor parms with empty bounds object', () => { + (() => { + new NumberValidator(mockField, {}); + }).should.throw(/Invalid range, lower and-or upper bound must be specified./); + }); + + it('should throw an error for null or undefined ast', () => { + (() => { + new NumberValidator(mockField, null); + }).should.throw(/Invalid range, lower and-or upper bound must be specified./); + (() => { + new NumberValidator(mockField, undefined); + }).should.throw(/Invalid range, lower and-or upper bound must be specified./); + }); + + it('should throw an error for non-number lower bound', () => { + (() => { + new NumberValidator(mockField, { lower: 'abc', upper: 100 }); + }).should.throw(/lower and-or upper bound must be numbers./); + }); + + it('should throw an error for non-number upper bound', () => { + (() => { + new NumberValidator(mockField, { lower: 0, upper: 'xyz' }); + }).should.throw(/lower and-or upper bound must be numbers./); + }); + + it('should throw an error for NaN bounds', () => { + (() => { + new NumberValidator(mockField, { lower: NaN, upper: 100 }); + }).should.throw(/lower and-or upper bound must be numbers./); + (() => { + new NumberValidator(mockField, { lower: 0, upper: NaN }); + }).should.throw(/lower and-or upper bound must be numbers./); + }); + it('should throw an error for constructor parms LOWER_IS_HIGHER_THAN_UPPER', () => { (() => { new NumberValidator(mockField, LOWER_IS_HIGHER_THAN_UPPER); diff --git a/packages/concerto-core/test/introspect/property.js b/packages/concerto-core/test/introspect/property.js index fb622dd180..e5e8a1613f 100644 --- a/packages/concerto-core/test/introspect/property.js +++ b/packages/concerto-core/test/introspect/property.js @@ -118,6 +118,53 @@ describe('Property', () => { ); }); + it('should throw for an object property without a type on validate', () => { + const p = new Property(mockClassDeclaration, { + $class: `${MetaModelNamespace}.ObjectProperty`, + name: 'o', + isArray: false, + isOptional: false + }); + (() => { + p.validate(mockClassDeclaration); + }).should.throw( + IllegalModelException, + /ObjectProperty must contain property 'type', for property o/ + ); + }); + + it('should throw for an object property with null type on validate', () => { + const p = new Property(mockClassDeclaration, { + $class: `${MetaModelNamespace}.ObjectProperty`, + name: 'o', + isArray: false, + isOptional: false, + type: null + }); + (() => { + p.validate(mockClassDeclaration); + }).should.throw( + IllegalModelException, + /ObjectProperty must contain property 'type', for property o/ + ); + }); + + it('should throw for an object property with a type missing name on validate', () => { + const p = new Property(mockClassDeclaration, { + $class: `${MetaModelNamespace}.ObjectProperty`, + name: 'o', + isArray: false, + isOptional: false, + type: {} + }); + (() => { + p.validate(mockClassDeclaration); + }).should.throw( + IllegalModelException, + /ObjectProperty must contain property 'type', for property o/ + ); + }); + }); describe('#hasInstance', () => { diff --git a/packages/concerto-core/test/introspect/stringvalidator.js b/packages/concerto-core/test/introspect/stringvalidator.js index 83d8a4a388..1694a0c7f2 100644 --- a/packages/concerto-core/test/introspect/stringvalidator.js +++ b/packages/concerto-core/test/introspect/stringvalidator.js @@ -58,6 +58,64 @@ describe('StringValidator', () => { expect(err.message).to.match(/Validator error for field `myField`/); }); + it('should throw an IllegalModelException for regex validator without pattern', () => { + let err; + try { new StringValidator(mockField, {}); } catch (e) { err = e; } + expect(err).to.be.instanceOf(IllegalModelException); + expect(err.errorType).to.equal(ErrorCodes.REGEX_VALIDATOR_EXCEPTION); + expect(err.message).to.match(/Regex pattern must be specified/); + }); + + it('should throw an IllegalModelException for regex validator with null pattern', () => { + let err; + try { new StringValidator(mockField, { pattern: null }); } catch (e) { err = e; } + expect(err).to.be.instanceOf(IllegalModelException); + expect(err.errorType).to.equal(ErrorCodes.REGEX_VALIDATOR_EXCEPTION); + expect(err.message).to.match(/Regex pattern must be specified/); + }); + + it('should throw an IllegalModelException for regex validator with non-string pattern', () => { + let err; + try { new StringValidator(mockField, { pattern: 123 }); } catch (e) { err = e; } + expect(err).to.be.instanceOf(IllegalModelException); + expect(err.errorType).to.equal(ErrorCodes.REGEX_VALIDATOR_EXCEPTION); + expect(err.message).to.match(/Regex pattern must be specified/); + }); + + it('should throw an IllegalModelException for regex validator with empty pattern', () => { + let err; + try { new StringValidator(mockField, { pattern: '' }); } catch (e) { err = e; } + expect(err).to.be.instanceOf(IllegalModelException); + expect(err.errorType).to.equal(ErrorCodes.REGEX_VALIDATOR_EXCEPTION); + expect(err.message).to.match(/Regex pattern must be specified/); + }); + + it('should accept regex validator with null flags', () => { + const v = new StringValidator(mockField, { pattern: '^[A-z]', flags: null }); + expect(v.getRegex()).to.not.be.null; + }); + + it('should throw for string length with undefined bounds or empty object', () => { + (() => { + new StringValidator(mockField, null, { minLength: undefined, maxLength: undefined }); + }).should.throw(/Invalid string length, minLength and-or maxLength must be specified/); + (() => { + new StringValidator(mockField, null, {}); + }).should.throw(/Invalid string length, minLength and-or maxLength must be specified/); + }); + + it('should throw for string length with non-integer bounds', () => { + (() => { + new StringValidator(mockField, null, { minLength: 1.5 }); + }).should.throw(/minLength and-or maxLength must be positive integers/); + (() => { + new StringValidator(mockField, null, { maxLength: 2.5 }); + }).should.throw(/minLength and-or maxLength must be positive integers/); + (() => { + new StringValidator(mockField, null, { minLength: NaN }); + }).should.throw(/minLength and-or maxLength must be positive integers/); + }); + it('should throw for invalid string length', () => { (() => { new StringValidator(mockField, { pattern: '^[A-z]' }, NO_PARMS_IN_AST); diff --git a/packages/concerto-core/test/modelmanager.js b/packages/concerto-core/test/modelmanager.js index a5237eb411..5d9091e86d 100644 --- a/packages/concerto-core/test/modelmanager.js +++ b/packages/concerto-core/test/modelmanager.js @@ -42,6 +42,7 @@ chai.use(require('chai-as-promised')); const sinon = require('sinon'); const tmp = require('tmp-promise'); const { BaseModelManager } = require('../src/basemodelmanager'); +const { IllegalModelException } = require('../src/introspect/illegalmodelexception'); describe('ModelManager', () => { @@ -274,7 +275,7 @@ describe('ModelManager', () => { }); describe('#addModel', () => { - it('should throw for a bad metamodel AST', () => { + it('should throw for a bad metamodel AST and clean up metamodel file', () => { const basemodelmanager = new BaseModelManager({ metamodelValidation: true }); const ast = { $class: `${MetaModelNamespace}.Model`, @@ -284,6 +285,7 @@ describe('ModelManager', () => { (() => { basemodelmanager.addModel(ast, undefined, 'origFile'); }).should.throw('Unexpected properties for type concerto.metamodel@1.0.0.Model: undeclared'); + (basemodelmanager.getModelFile(MetaModelNamespace) === undefined).should.be.true; }); it('should throw when using an unknown metamodel version', () => { @@ -299,6 +301,52 @@ describe('ModelManager', () => { }); }); + describe('#fromAst', () => { + it('should throw IllegalModelException when ast is null or models array is missing', () => { + const basemodelmanager = new BaseModelManager(); + (() => { + basemodelmanager.fromAst(null); + }).should.throw(IllegalModelException, /Models AST must contain a models array/); + (() => { + basemodelmanager.fromAst({}); + }).should.throw(IllegalModelException, /Models AST must contain a models array/); + }); + + it('should throw IllegalModelException when a model in models is null', () => { + const basemodelmanager = new BaseModelManager(); + (() => { + basemodelmanager.fromAst({ + $class: `${MetaModelNamespace}.Models`, + models: [null] + }); + }).should.throw(IllegalModelException, /Model cannot be null or undefined/); + }); + + it('should populate model files from a valid Models AST', () => { + const basemodelmanager = new BaseModelManager(); + const ast = { + $class: `${MetaModelNamespace}.Models`, + models: [ + { + $class: `${MetaModelNamespace}.Model`, + namespace: 'org.acme.valid@1.0.0', + imports: [], + declarations: [ + { + $class: `${MetaModelNamespace}.ConceptDeclaration`, + name: 'TestConcept', + isAbstract: false, + properties: [] + } + ] + } + ] + }; + basemodelmanager.fromAst(ast); + basemodelmanager.getModelFile('org.acme.valid@1.0.0').should.not.be.undefined; + }); + }); + describe('#addModelFiles', () => { it('should add model files from strings', () => {