Skip to content

Commit

Permalink
feat(map): refactor map shape (accordproject#674)
Browse files Browse the repository at this point in the history 
* feat(map): refactor map shape

Signed-off-by: jonathan.casey <[email protected]>

* feat(*): map can be empty

Signed-off-by: jonathan.casey <[email protected]>

* feat(*): map cannot use private reserved properties as keys

Signed-off-by: jonathan.casey <[email protected]>

* feat(*): refactor test

Signed-off-by: jonathan.casey <[email protected]>

---------

Signed-off-by: jonathan.casey <[email protected]>
  • Loading branch information
@jonathan-casey
jonathan-casey authored Jul 25, 2023
1 parent ac70543 commit 1ba9c07
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 52 deletions.
2 changes: 1 addition & 1 deletion packages/concerto-core/lib/serializer/jsongenerator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class JSONGenerator {
*/
visitMapDeclaration(mapDeclaration, parameters) {
const obj = parameters.stack.pop();
return { $class: obj.$class, value: Object.fromEntries(obj.value)};
return Object.fromEntries(obj);
}

/**
Expand Down
14 changes: 4 additions & 10 deletions packages/concerto-core/lib/serializer/jsonpopulator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function getAssignableProperties(resourceData, classDeclaration) {
}

if (properties.includes('$timestamp') &&
!(classDeclaration.isTransaction() || classDeclaration.isEvent())
!(classDeclaration.isTransaction?.() || classDeclaration.isEvent?.())
) {
const errorText = `Unexpected property for type ${classDeclaration.getFullyQualifiedName()}: $timestamp`;
throw new ValidationException(errorText);
Expand Down Expand Up @@ -172,17 +172,11 @@ class JSONPopulator {
visitMapDeclaration(mapDeclaration, parameters) {
const jsonObj = parameters.jsonStack.pop();
parameters.path ?? (parameters.path = new TypedStack('$'));
const path = parameters.path.stack.join('');

if(!jsonObj.$class) {
throw new Error(`Invalid JSON data at "${path}". Map value does not contain a $class type identifier.`);
}

if(!jsonObj.value) {
throw new Error(`Invalid JSON data at "${path}". Map value does not contain a value property.`);
}
// throws if Map contains private reserved properties as keys.
getAssignableProperties(jsonObj, mapDeclaration);

return { $class: jsonObj.$class, value: new Map(Object.entries(jsonObj.value)) };
return new Map(Object.entries(jsonObj));
}

/**
Expand Down
11 changes: 8 additions & 3 deletions packages/concerto-core/lib/serializer/resourcevalidator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,20 @@ class ResourceValidator {
visitMapDeclaration(mapDeclaration, parameters) {
const obj = parameters.stack.pop();

if (!((obj.value instanceof Map))) {
if (!((obj instanceof Map))) {
throw new Error('Expected a Map, but found ' + JSON.stringify(obj));
}

if (obj.$class !== mapDeclaration.getFullyQualifiedName()) {
if(!obj.has('$class')) {
throw new Error('Invalid Map. Map must contain a properly formatted $class property');
}

if (obj.get('$class') !== mapDeclaration.getFullyQualifiedName()) {
throw new Error(`$class value must match ${mapDeclaration.getFullyQualifiedName()}`);
}

obj.value.forEach((value, key) => {

obj.forEach((value, key) => {
if(!ModelUtil.isSystemProperty(key)) {
if (typeof key !== 'string') {
ResourceValidator.reportInvalidMap(parameters.rootResourceIdentifier, mapDeclaration, obj);
Expand Down
6 changes: 2 additions & 4 deletions packages/concerto-core/test/model/concept.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,8 @@ describe('Concept', function () {
invType:'NEWBATCH',
dictionary: {
$class: 'org.acme.biznet.Dictionary',
value: {
'key1': 'value1',
'key2': 'value2',
}
key1: 'value1',
key2: 'value2',
}
};
const obj = serializer.fromJSON(jsObject);
Expand Down
54 changes: 24 additions & 30 deletions packages/concerto-core/test/serializer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,16 +243,15 @@ describe('Serializer', () => {
});
});

it('should generate concept with a map value', () => {
it('should generate concept with a Map value', () => {
let address = factory.newConcept('org.acme.sample', 'Address');
address.city = 'Winchester';
address.country = 'UK';
address.elevation = 3.14;
address.postcode = 'SO21 2JN';
address.dict = {
$class: 'org.acme.sample.Dictionary',
value: new Map(Object.entries({'Lorem':'Ipsum'}))
};
address.dict = new Map();
address.dict.set('$class', 'org.acme.sample.Dictionary');
address.dict.set('Lorem', 'Ipsum');

// todo test for reserved identifiers in keys ($class)
const json = serializer.toJSON(address);
Expand All @@ -264,13 +263,12 @@ describe('Serializer', () => {
postcode: 'SO21 2JN',
dict: {
$class: 'org.acme.sample.Dictionary',
value: { 'Lorem': 'Ipsum' }
Lorem: 'Ipsum'
}
});
});


it('should throw if the value for a map is not a Map instance', () => {
it('should throw if the value for a Map is not a Map instance', () => {
let address = factory.newConcept('org.acme.sample', 'Address');
address.city = 'Winchester';
address.country = 'UK';
Expand All @@ -288,10 +286,10 @@ describe('Serializer', () => {
address.country = 'UK';
address.elevation = 3.14;
address.postcode = 'SO21 2JN';
address.dict = {
$class: 'org.acme.sample.PhoneBook',
value: new Map(Object.entries({'Lorem':'Ipsum'}))
};
address.dict = new Map();
address.dict.set('$class', 'org.acme.sample.PhoneBook'); // dict is not a PhoneBook.
address.dict.set('Lorem', 'Ipsum');

(() => {
serializer.toJSON(address);
}).should.throw('$class value must match org.acme.sample.Dictionary');
Expand Down Expand Up @@ -397,8 +395,7 @@ describe('Serializer', () => {
resource.postcode.should.equal('SO21 2JN');
});

it('should deserialize a valid concept with a map', () => {

it('should deserialize a valid concept with a Map', () => {
let json = {
$class: 'org.acme.sample.Address',
city: 'Winchester',
Expand All @@ -407,9 +404,7 @@ describe('Serializer', () => {
postcode: 'SO21 2JN',
dict: {
'$class': 'org.acme.sample.Dictionary',
value: {
'Lorem': 'Ipsum'
}
'Lorem': 'Ipsum'
}
};
let resource = serializer.fromJSON(json);
Expand All @@ -419,12 +414,12 @@ describe('Serializer', () => {
resource.country.should.equal('UK');
resource.elevation.should.equal(3.14);
resource.postcode.should.equal('SO21 2JN');
resource.dict.$class.should.equal('org.acme.sample.Dictionary');
resource.dict.value.should.be.an.instanceOf(Map);
resource.dict.value.get('Lorem').should.equal('Ipsum');
resource.dict.should.be.an.instanceOf(Map);
resource.dict.get('$class').should.equal('org.acme.sample.Dictionary');
resource.dict.get('Lorem').should.equal('Ipsum');
});

it('should throw an error when deserializing a map without a $class property', () => {
it('should throw an error when deserializing a Map without a $class property', () => {

let json = {
$class: 'org.acme.sample.Address',
Expand All @@ -434,17 +429,17 @@ describe('Serializer', () => {
postcode: 'SO21 2JN',
dict: {
// '$class': 'org.acme.sample.Dictionary',
value: {
'Lorem': 'Ipsum'
}
'Lorem': 'Ipsum'
}
};
(() => {
serializer.fromJSON(json);
}).should.throw('Invalid JSON data at "$.dict". Map value does not contain a $class type identifier.');
}).should.throw('Invalid Map. Map must contain a properly formatted $class property');
});

it('should throw an error when deserializing a map without a value property', () => {

it('should throw an error when deserializing a Map with a private reserved property', () => {

let json = {
$class: 'org.acme.sample.Address',
city: 'Winchester',
Expand All @@ -453,14 +448,13 @@ describe('Serializer', () => {
postcode: 'SO21 2JN',
dict: {
'$class': 'org.acme.sample.Dictionary',
// value: {
// 'Lorem': 'Ipsum'
// }
'$namespace': 'com.reserved.property',
'Lorem': 'Ipsum'
}
};
(() => {
serializer.fromJSON(json);
}).should.throw('Invalid JSON data at "$.dict". Map value does not contain a value property.');
}).should.throw('Unexpected reserved properties for type org.acme.sample.Dictionary: $namespace');
});

it('should throw validation errors if the validate flag is not specified', () => {
Expand Down
7 changes: 3 additions & 4 deletions packages/concerto-core/test/serializer/resourcevalidator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,15 +348,15 @@ describe('ResourceValidator', function () {

describe('#visitMapDeclaration', function() {
it('should validate map', function () {
const map = { $class: 'org.acme.map.PhoneBook', value: new Map([['Lorem', 'Ipsum']]) };
const map = new Map([['$class', 'org.acme.map.PhoneBook'], ['Lorem', 'Ipsum']]);
const typedStack = new TypedStack(map);
const mapDeclaration = modelManager.getType('org.acme.map.PhoneBook');
const parameters = { stack : typedStack, 'modelManager' : modelManager, rootResourceIdentifier : 'TEST' };
mapDeclaration.accept(resourceValidator,parameters );
});

it('should not validate map with bad value', function () {
const map = { $class: 'org.acme.map.PhoneBook', value: new Map([['Lorem', 3]]) };
const map = new Map([['$class', 'org.acme.map.PhoneBook'], ['Lorem', 3]]);
const typedStack = new TypedStack(map);
const mapDeclaration = modelManager.getType('org.acme.map.PhoneBook');
const parameters = { stack : typedStack, 'modelManager' : modelManager, rootResourceIdentifier : 'TEST' };
Expand All @@ -367,8 +367,7 @@ describe('ResourceValidator', function () {
});

it('should not validate map with bad key', function () {
const map = { $class: 'org.acme.map.PhoneBook', value: new Map([[1, 'Ipsum']]) };

const map = new Map([['$class', 'org.acme.map.PhoneBook'], [1, 'Ipsum']]);
const typedStack = new TypedStack(map);
const mapDeclaration = modelManager.getType('org.acme.map.PhoneBook');
const parameters = { stack : typedStack, 'modelManager' : modelManager, rootResourceIdentifier : 'TEST' };
Expand Down

0 comments on commit 1ba9c07

Please sign in to comment.