From 1925bc696e65652928a0f3a1ec238f874926e2fb Mon Sep 17 00:00:00 2001 From: Matt Roberts Date: Wed, 10 Aug 2022 11:55:22 +0100 Subject: [PATCH 1/3] chore(jsonpopulator): remove data values from validation error messages Signed-off-by: Matt Roberts --- .../lib/serializer/jsonpopulator.js | 34 ++++++--- .../test/serializer/jsonpopulator.js | 73 +++++++++++++------ .../types/lib/serializer/jsonpopulator.d.ts | 3 +- 3 files changed, 75 insertions(+), 35 deletions(-) diff --git a/packages/concerto-core/lib/serializer/jsonpopulator.js b/packages/concerto-core/lib/serializer/jsonpopulator.js index a8f25976d7..d9178e91a7 100644 --- a/packages/concerto-core/lib/serializer/jsonpopulator.js +++ b/packages/concerto-core/lib/serializer/jsonpopulator.js @@ -14,6 +14,7 @@ 'use strict'; +const { TypedStack } = require('@accordproject/concerto-util'); const Relationship = require('../model/relationship'); const Util = require('../util'); const ModelUtil = require('../modelutil'); @@ -100,7 +101,9 @@ class JSONPopulator { * @return {Object} the result of visiting or null * @private */ - visit(thing, parameters) { + visit(thing, parameters = {}) { + parameters.path ?? (parameters.path = new TypedStack('$')); + if (thing.isClassDeclaration?.()) { return this.visitClassDeclaration(thing, parameters); } else if (thing.isRelationship?.()) { @@ -122,6 +125,7 @@ class JSONPopulator { visitClassDeclaration(classDeclaration, parameters) { const jsonObj = parameters.jsonStack.pop(); const resourceObj = parameters.resourceStack.pop(); + parameters.path ?? (parameters.path = new TypedStack('$')); const properties = getAssignableProperties(jsonObj); validateProperties(properties, classDeclaration); @@ -136,12 +140,13 @@ class JSONPopulator { } } if (value !== null) { + parameters.path.push(`.${property}`); parameters.jsonStack.push(value); const classProperty = classDeclaration.getProperty(property); resourceObj[property] = classProperty.accept(this,parameters); + parameters.path.pop(); } }); - return resourceObj; } @@ -153,6 +158,7 @@ class JSONPopulator { * @private */ visitField(field, parameters) { + parameters.path ?? (parameters.path = new TypedStack('$')); let jsonObj = parameters.jsonStack.pop(); let result = null; @@ -164,8 +170,10 @@ class JSONPopulator { } result = []; for(let n=0; n < jsonObj.length; n++) { + parameters.path.push(`[${n}]`); const jsonItem = jsonObj[n]; result.push(this.convertItem(field,jsonItem, parameters)); + parameters.path.pop(); } } else { @@ -222,7 +230,7 @@ class JSONPopulator { classDeclaration.accept(this, parameters); } else { - result = this.convertToObject(field,jsonItem); + result = this.convertToObject(field, jsonItem, parameters); } return result; @@ -233,22 +241,25 @@ class JSONPopulator { * * @param {Field} field - the field declaration of the object * @param {Object} json - the JSON object to convert to a Concerto Object + * @param {Object} parameters - the parameters * @return {string} the text representation */ - convertToObject(field, json) { + convertToObject(field, json, parameters = {}) { let result = null; + parameters.path ?? (parameters.path = new TypedStack('$')); + const path = parameters.path.stack.join(''); switch(field.getType()) { case 'DateTime': { if (json && typeof json === 'object' && typeof json.isBefore === 'function') { result = json; } else if (typeof json !== 'string') { - throw new ValidationException(`Expected value ${JSON.stringify(json)} to be of type ${field.getType()}`); + throw new ValidationException(`Expected value at path \`${path}\` to be of type \`${field.getType()}\``); } else { result = dayjs.utc(json).utcOffset(this.utcOffset); } if (!result.isValid()) { - throw new ValidationException(`Expected value ${JSON.stringify(json)} to be of type ${field.getType()}`); + throw new ValidationException(`Expected value at path \`${path}\` to be of type \`${field.getType()}\``); } } break; @@ -257,12 +268,12 @@ class JSONPopulator { const num = this.ergo ? json.$nat : json; if (typeof num === 'number') { if (Math.trunc(num) !== num) { - throw new ValidationException(`Expected value ${JSON.stringify(json)} to be of type ${field.getType()}`); + throw new ValidationException(`Expected value at path \`${path}\` to be of type \`${field.getType()}\``); } else { result = num; } } else { - throw new ValidationException(`Expected value ${JSON.stringify(json)} to be of type ${field.getType()}`); + throw new ValidationException(`Expected value at path \`${path}\` to be of type \`${field.getType()}\``); } } break; @@ -270,7 +281,7 @@ class JSONPopulator { if (typeof json === 'number') { result = parseFloat(json); } else { - throw new ValidationException(`Expected value ${JSON.stringify(json)} to be of type ${field.getType()}`); + throw new ValidationException(`Expected value at path \`${path}\` to be of type \`${field.getType()}\``); } } break; @@ -278,7 +289,7 @@ class JSONPopulator { if (typeof json === 'boolean') { result = json; } else { - throw new ValidationException(`Expected value ${JSON.stringify(json)} to be of type ${field.getType()}`); + throw new ValidationException(`Expected value at path \`${path}\` to be of type \`${field.getType()}\``); } } break; @@ -286,7 +297,7 @@ class JSONPopulator { if (typeof json === 'string') { result = json; } else { - throw new ValidationException(`Expected value ${JSON.stringify(json)} to be of type ${field.getType()}`); + throw new ValidationException(`Expected value at path \`${path}\` to be of type \`${field.getType()}\``); } break; default: { @@ -314,6 +325,7 @@ class JSONPopulator { * @private */ visitRelationshipDeclaration(relationshipDeclaration, parameters) { + parameters.path ?? (parameters.path = new TypedStack('$')); let jsonObj = parameters.jsonStack.pop(); let result = null; diff --git a/packages/concerto-core/test/serializer/jsonpopulator.js b/packages/concerto-core/test/serializer/jsonpopulator.js index 09716271c2..e587d5c7b5 100644 --- a/packages/concerto-core/test/serializer/jsonpopulator.js +++ b/packages/concerto-core/test/serializer/jsonpopulator.js @@ -123,7 +123,7 @@ describe('JSONPopulator', () => { field.getType.returns('DateTime'); (() => { jsonPopulator.convertToObject(field, 'foo'); - }).should.throw(ValidationException, /Expected value "foo" to be of type DateTime/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `DateTime`/); }); it('should not convert to dates from null', () => { @@ -131,7 +131,7 @@ describe('JSONPopulator', () => { field.getType.returns('DateTime'); (() => { jsonPopulator.convertToObject(field, null); - }).should.throw(ValidationException, /Expected value null to be of type DateTime/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `DateTime`/); }); it('should not convert to dates from undefined', () => { @@ -139,7 +139,7 @@ describe('JSONPopulator', () => { field.getType.returns('DateTime'); (() => { jsonPopulator.convertToObject(field, undefined); - }).should.throw(ValidationException, /Expected value undefined to be of type DateTime/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `DateTime`/); }); it('should not convert to dates when not in ISO 8601 format', () => { @@ -147,7 +147,7 @@ describe('JSONPopulator', () => { field.getType.returns('DateTime'); (() => { jsonPopulator.convertToObject(field, 'abc'); - }).should.throw(ValidationException, /Expected value "abc" to be of type DateTime/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `DateTime`/); }); it('should not convert to integers from strings', () => { @@ -155,7 +155,7 @@ describe('JSONPopulator', () => { field.getType.returns('Integer'); (() => { jsonPopulator.convertToObject(field, '32768'); - }).should.throw(ValidationException, /Expected value "32768" to be of type Integer/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Integer`/); }); it('should not convert to integer from null', () => { @@ -163,7 +163,7 @@ describe('JSONPopulator', () => { field.getType.returns('Integer'); (() => { jsonPopulator.convertToObject(field, null); - }).should.throw(ValidationException, /Expected value null to be of type Integer/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Integer`/); }); it('should not convert to integer from undefined', () => { @@ -171,7 +171,7 @@ describe('JSONPopulator', () => { field.getType.returns('Integer'); (() => { jsonPopulator.convertToObject(field, undefined); - }).should.throw(ValidationException, /Expected value undefined to be of type Integer/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Integer`/); }); it('should convert to integers from numbers', () => { @@ -188,7 +188,7 @@ describe('JSONPopulator', () => { field.getType.returns('Long'); (() => { jsonPopulator.convertToObject(field, '32768'); - }).should.throw(ValidationException, /Expected value "32768" to be of type Long/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Long`/); }); it('should not convert to long from null', () => { @@ -196,7 +196,7 @@ describe('JSONPopulator', () => { field.getType.returns('Long'); (() => { jsonPopulator.convertToObject(field, null); - }).should.throw(ValidationException, /Expected value null to be of type Long/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Long`/); }); it('should not convert to long from undefined', () => { @@ -204,7 +204,7 @@ describe('JSONPopulator', () => { field.getType.returns('Long'); (() => { jsonPopulator.convertToObject(field, undefined); - }).should.throw(ValidationException, /Expected value undefined to be of type Long/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Long`/); }); it('should convert to longs from numbers', () => { @@ -221,7 +221,7 @@ describe('JSONPopulator', () => { field.getType.returns('Long'); (() => { jsonPopulator.convertToObject(field, 32.768); - }).should.throw(ValidationException, /Expected value 32.768 to be of type Long/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Long`/); }); it('should not convert to doubles from strings', () => { @@ -229,7 +229,7 @@ describe('JSONPopulator', () => { field.getType.returns('Double'); (() => { jsonPopulator.convertToObject(field, '32.768'); - }).should.throw(ValidationException, /Expected value "32.768" to be of type Double/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Double`/); }); it('should not convert to double from null', () => { @@ -237,7 +237,7 @@ describe('JSONPopulator', () => { field.getType.returns('Double'); (() => { jsonPopulator.convertToObject(field, null); - }).should.throw(ValidationException, /Expected value null to be of type Double/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Double`/); }); it('should not convert to double from undefined', () => { @@ -245,7 +245,7 @@ describe('JSONPopulator', () => { field.getType.returns('Double'); (() => { jsonPopulator.convertToObject(field, undefined); - }).should.throw(ValidationException, /Expected value undefined to be of type Double/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Double`/); }); it('should convert to doubles from numbers', () => { @@ -267,7 +267,7 @@ describe('JSONPopulator', () => { field.getType.returns('Boolean'); (() => { jsonPopulator.convertToObject(field, 'true'); - }).should.throw(ValidationException, /Expected value "true" to be of type Boolean/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Boolean`/); }); it('should not convert to booleans from numbers', () => { @@ -275,7 +275,7 @@ describe('JSONPopulator', () => { field.getType.returns('Boolean'); (() => { jsonPopulator.convertToObject(field, 32.768); - }).should.throw(ValidationException, /Expected value 32.768 to be of type Boolean/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Boolean`/); }); it('should not convert to boolean from null', () => { @@ -283,7 +283,7 @@ describe('JSONPopulator', () => { field.getType.returns('Boolean'); (() => { jsonPopulator.convertToObject(field, null); - }).should.throw(ValidationException, /Expected value null to be of type Boolean/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Boolean`/); }); it('should not convert to boolean from undefined', () => { @@ -291,7 +291,7 @@ describe('JSONPopulator', () => { field.getType.returns('Boolean'); (() => { jsonPopulator.convertToObject(field, undefined); - }).should.throw(ValidationException, /Expected value undefined to be of type Boolean/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `Boolean`/); }); it('should convert to strings from strings', () => { @@ -306,7 +306,7 @@ describe('JSONPopulator', () => { field.getType.returns('String'); (() => { jsonPopulator.convertToObject(field, 32.768); - }).should.throw(ValidationException, /Expected value 32.768 to be of type String/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `String`/); }); it('should not convert to string from null', () => { @@ -314,7 +314,7 @@ describe('JSONPopulator', () => { field.getType.returns('String'); (() => { jsonPopulator.convertToObject(field, null); - }).should.throw(ValidationException, /Expected value null to be of type String/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `String`/); }); it('should not convert to string from undefined', () => { @@ -322,7 +322,7 @@ describe('JSONPopulator', () => { field.getType.returns('String'); (() => { jsonPopulator.convertToObject(field, undefined); - }).should.throw(ValidationException, /Expected value undefined to be of type String/); + }).should.throw(ValidationException, /Expected value at path `\$` to be of type `String`/); }); }); @@ -472,6 +472,33 @@ describe('JSONPopulator', () => { }); + describe('#visit', () => { + it('should throw if the type of a nested field is invalid', () => { + let options = { + jsonStack: new TypedStack({ + $class: 'org.acme.MyContainerAsset2', + assetId: 'assetContainer1', + myAssets: [{ + $class: 'org.acme.MyAsset1', + assetId: 'asset1', + assetValue: 'string' // this is invalid + }] + }), + resourceStack: new TypedStack({}), + factory: mockFactory, + modelManager: modelManager + }; + + let mockResource1 = sinon.createStubInstance(Resource); + mockFactory.newResource.withArgs('org.acme', 'MyAsset1', 'asset1').returns(mockResource1); + let mockResource2 = sinon.createStubInstance(Resource); + mockFactory.newResource.withArgs('org.acme', 'MyAsset1', 'asset2').returns(mockResource2); + (() => { + jsonPopulator.visit(modelManager.getType('org.acme.MyContainerAsset2'), options); + }).should.throw(/Expected value at path `\$.myAssets\[0\].assetValue` to be of type `Integer`/); + }); + }); + describe('#visitRelationshipDeclaration', () => { it('should create a new relationship from a string', () => { @@ -604,7 +631,7 @@ describe('JSONPopulator', () => { let mockRelationship2 = sinon.createStubInstance(Relationship); mockFactory.newRelationship.withArgs('org.acme', 'MyAsset1', 'asset2').returns(mockRelationship2); let relationships = jsonPopulator.visitRelationshipDeclaration(relationshipDeclaration2, options); - relationships.should.have.length.of(2); + relationships.should.have.lengthOf(2); relationships[0].should.be.an.instanceOf(Relationship); relationships[1].should.be.an.instanceOf(Relationship); }); @@ -650,7 +677,7 @@ describe('JSONPopulator', () => { let mockResource2 = sinon.createStubInstance(Resource); mockFactory.newResource.withArgs('org.acme', 'MyAsset1', 'asset2').returns(mockResource2); let subResources = jsonPopulator.visitRelationshipDeclaration(relationshipDeclaration2, options); - subResources.should.have.length.of(2); + subResources.should.have.lengthOf(2); subResources[0].should.be.an.instanceOf(Resource); subResources[1].should.be.an.instanceOf(Resource); }); diff --git a/packages/concerto-core/types/lib/serializer/jsonpopulator.d.ts b/packages/concerto-core/types/lib/serializer/jsonpopulator.d.ts index 980a01d7bb..c34186bfb8 100644 --- a/packages/concerto-core/types/lib/serializer/jsonpopulator.d.ts +++ b/packages/concerto-core/types/lib/serializer/jsonpopulator.d.ts @@ -60,9 +60,10 @@ declare class JSONPopulator { * * @param {Field} field - the field declaration of the object * @param {Object} json - the JSON object to convert to a Concerto Object + * @param {Object} parameters - the parameters * @return {string} the text representation */ - convertToObject(field: Field, json: any): string; + convertToObject(field: Field, json: any, parameters?: any): string; /** * Visitor design pattern * @param {RelationshipDeclaration} relationshipDeclaration - the object being visited From 209a91e4226c8d89a347fdd4c18d4c0b5d2ee959 Mon Sep 17 00:00:00 2001 From: Matt Roberts Date: Wed, 10 Aug 2022 12:23:21 +0100 Subject: [PATCH 2/3] test(jsonpopulator): increase coverage Signed-off-by: Matt Roberts --- .../test/serializer/jsonpopulator.js | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/packages/concerto-core/test/serializer/jsonpopulator.js b/packages/concerto-core/test/serializer/jsonpopulator.js index e587d5c7b5..b29b30656f 100644 --- a/packages/concerto-core/test/serializer/jsonpopulator.js +++ b/packages/concerto-core/test/serializer/jsonpopulator.js @@ -497,6 +497,67 @@ describe('JSONPopulator', () => { jsonPopulator.visit(modelManager.getType('org.acme.MyContainerAsset2'), options); }).should.throw(/Expected value at path `\$.myAssets\[0\].assetValue` to be of type `Integer`/); }); + + it('should allow injection of a root object path', () => { + let options = { + jsonStack: new TypedStack({ + $class: 'org.acme.MyContainerAsset2', + assetId: 'assetContainer1', + myAssets: [{ + $class: 'org.acme.MyAsset1', + assetId: 'asset1', + assetValue: 'string' // this is invalid + }] + }), + path: new TypedStack('$.rootObj'), + resourceStack: new TypedStack({}), + factory: mockFactory, + modelManager: modelManager + }; + + let mockResource1 = sinon.createStubInstance(Resource); + mockFactory.newResource.withArgs('org.acme', 'MyAsset1', 'asset1').returns(mockResource1); + let mockResource2 = sinon.createStubInstance(Resource); + mockFactory.newResource.withArgs('org.acme', 'MyAsset1', 'asset2').returns(mockResource2); + (() => { + jsonPopulator.visit(modelManager.getType('org.acme.MyContainerAsset2'), options); + }).should.throw(/Expected value at path `\$.rootObj.myAssets\[0\].assetValue` to be of type `Integer`/); + }); + }); + + describe('#visitField', () => { + it('should visit a Field resource', () => { + let options = { + jsonStack: new TypedStack('field'), + resourceStack: new TypedStack({}), + factory: mockFactory, + modelManager: modelManager + }; + + let field = sinon.createStubInstance(Field); + field.isPrimitive.returns('String'); + field.isField.returns(true); + field.getType.returns('String'); + let value = jsonPopulator.visitField(field, options); + value.should.equal('field'); + }); + + it('should allow injection of a root object path', () => { + let options = { + jsonStack: new TypedStack('field'), + path: new TypedStack('$.rootObj'), + resourceStack: new TypedStack({}), + factory: mockFactory, + modelManager: modelManager + }; + + let field = sinon.createStubInstance(Field); + field.isPrimitive.returns('String'); + field.isField.returns(true); + field.getType.returns('String'); + let value = jsonPopulator.visitField(field, options); + value.should.equal('field'); + }); }); describe('#visitRelationshipDeclaration', () => { From 6f2f834b9b7c5ae7ef2e2020efe1223214b39a21 Mon Sep 17 00:00:00 2001 From: Matt Roberts Date: Wed, 10 Aug 2022 14:11:45 +0100 Subject: [PATCH 3/3] test(jsonpopulator): increase coverage again! --- .../test/serializer/jsonpopulator.js | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/concerto-core/test/serializer/jsonpopulator.js b/packages/concerto-core/test/serializer/jsonpopulator.js index b29b30656f..be863ad356 100644 --- a/packages/concerto-core/test/serializer/jsonpopulator.js +++ b/packages/concerto-core/test/serializer/jsonpopulator.js @@ -560,6 +560,36 @@ describe('JSONPopulator', () => { }); }); + describe('#visitClassDeclaration', () => { + it('should visit a ClassDeclaration resource', () => { + let options = { + jsonStack: new TypedStack({ + $class: 'org.acme.MyAsset1', + assetId: 'asset1' + }), + resourceStack: new TypedStack({}), + factory: mockFactory, + modelManager: modelManager + }; + jsonPopulator.visitClassDeclaration(modelManager.getType('org.acme.MyAsset1'), options); + }); + + it('should allow injection of a root object path', () => { + let options = { + jsonStack: new TypedStack({ + $class: 'org.acme.MyAsset1', + assetId: 'asset1' + }), + path: new TypedStack('$.rootObj'), + resourceStack: new TypedStack({}), + factory: mockFactory, + modelManager: modelManager + }; + jsonPopulator.visitClassDeclaration(modelManager.getType('org.acme.MyAsset1'), options); + }); + }); + + describe('#visitRelationshipDeclaration', () => { it('should create a new relationship from a string', () => {