Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(map): add serialisation for map<string, string> #654

Merged
merged 17 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/concerto-core/lib/modelutil.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,18 @@ class ModelUtil {
return (typeDeclaration !== null && typeDeclaration.isEnum());
}

/**
* Returns the true if the given field is an map type
* @param {Field} field - the string
* @return {boolean} true if the field is declared as an map
* @private
*/
static isMap(field) {
const modelFile = field.getParent().getModelFile();
const typeDeclaration = modelFile.getType(field.getType());
return (typeDeclaration !== null && typeDeclaration?.isMapDeclaration?.());
}

/**
* Returns the true if the given field is a Scalar type
* @param {Field} field - the Field to test
Expand Down
8 changes: 5 additions & 3 deletions packages/concerto-core/lib/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,18 +161,20 @@ class Serializer {

// create a new instance, using the identifier field name as the ID.
let resource;
if (classDeclaration.isTransaction()) {
if (classDeclaration.isTransaction?.()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to loosen this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MapDeclaration doesn't implement isTransaction()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it should? we could move the function to the Declaration class?

resource = this.factory.newTransaction(classDeclaration.getNamespace(),
classDeclaration.getName(),
jsonObject[classDeclaration.getIdentifierFieldName()] );
} else if (classDeclaration.isEvent()) {
} else if (classDeclaration.isEvent?.()) {
resource = this.factory.newEvent(classDeclaration.getNamespace(),
classDeclaration.getName(),
jsonObject[classDeclaration.getIdentifierFieldName()] );
} else if (classDeclaration.isConcept()) {
} else if (classDeclaration.isConcept?.()) {
resource = this.factory.newConcept(classDeclaration.getNamespace(),
classDeclaration.getName(),
jsonObject[classDeclaration.getIdentifierFieldName()] );
} else if (classDeclaration.isMapDeclaration?.()) {
throw new Error('Attempting to create an Map declaration is not supported.');
} else if (classDeclaration.isEnum()) {
throw new Error('Attempting to create an ENUM declaration is not supported.');
} else {
Expand Down
53 changes: 34 additions & 19 deletions packages/concerto-core/lib/serializer/instancegenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ class InstanceGenerator {
visit(thing, parameters) {
if (thing.isClassDeclaration?.()) {
return this.visitClassDeclaration(thing, parameters);
} else if (thing.isMapDeclaration?.()) {
return this.visitMapDeclaration(thing, parameters);
} else if (thing.isRelationship?.()) {
return this.visitRelationshipDeclaration(thing, parameters);
} else if (thing.isTypeScalar?.()) {
Expand Down Expand Up @@ -152,31 +154,33 @@ class InstanceGenerator {
}
}

let classDeclaration = parameters.modelManager.getType(type);
let declaration = parameters.modelManager.getType(type);

if (classDeclaration.isEnum()) {
let enumValues = classDeclaration.getOwnProperties();
if (declaration.isEnum()) {
let enumValues = declaration.getOwnProperties();
return parameters.valueGenerator.getEnum(enumValues).getName();
}

classDeclaration = this.findConcreteSubclass(classDeclaration);
declaration = this.findConcreteSubclass(declaration);

let id = null;
if (classDeclaration.isIdentified()) {
let idFieldName = classDeclaration.getIdentifierFieldName();
let idField = classDeclaration.getProperty(idFieldName);
if (idField?.isTypeScalar?.()){
idField = idField.getScalarField();
}
if(idField?.validator?.regex){
id = parameters.valueGenerator.getRegex(fieldOrScalarDeclaration.validator.regex);
} else {
id = this.generateRandomId(classDeclaration);
if (!declaration.isMapDeclaration?.()) {
let id = null;
if (declaration.isIdentified()) {
let idFieldName = declaration.getIdentifierFieldName();
let idField = declaration.getProperty(idFieldName);
if (idField?.isTypeScalar?.()){
idField = idField.getScalarField();
}
if(idField?.validator?.regex){
id = parameters.valueGenerator.getRegex(fieldOrScalarDeclaration.validator.regex);
} else {
id = this.generateRandomId(declaration);
}
}
let resource = parameters.factory.newResource(declaration.getNamespace(), declaration.getName(), id);
parameters.stack.push(resource);
}
let resource = parameters.factory.newResource(classDeclaration.getNamespace(), classDeclaration.getName(), id);
parameters.stack.push(resource);
return classDeclaration.accept(this, parameters);
return declaration.accept(this, parameters);
}

/**
Expand All @@ -190,7 +194,7 @@ class InstanceGenerator {
* @throws {Error} if no concrete subclasses exist.
*/
findConcreteSubclass(declaration) {
if (!declaration.isAbstract()) {
if (!declaration.isAbstract?.()) {
return declaration;
}

Expand Down Expand Up @@ -229,6 +233,17 @@ class InstanceGenerator {
}
}

/**
* Visitor design pattern
* @param {MapDeclaration} mapDeclaration - the object being visited
* @param {Object} parameters - the parameter
* @return {Object} the result of visiting or null
* @private
*/
visitMapDeclaration(mapDeclaration, parameters) {
return parameters.valueGenerator.getMap();
}

/**
* Generate a random ID for a given type.
* @private
Expand Down
21 changes: 20 additions & 1 deletion packages/concerto-core/lib/serializer/jsongenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class JSONGenerator {
return this.visitClassDeclaration(thing, parameters);
} else if (thing.isRelationship?.()) {
return this.visitRelationshipDeclaration(thing, parameters);
}else if (thing.isMapDeclaration?.()) {
return this.visitMapDeclaration(thing, parameters);
} else if (thing.isTypeScalar?.()) {
return this.visitField(thing.getScalarField(), parameters);
} else if (thing.isField?.()) {
Expand All @@ -74,6 +76,18 @@ class JSONGenerator {
}
}

/**
* Visitor design pattern
* @param {MapDeclaration} mapDeclaration - the object being visited
* @param {Object} parameters - the parameter
* @return {Object} the result of visiting or null
* @private
*/
visitMapDeclaration(mapDeclaration, parameters) {
const obj = parameters.stack.pop();
return { $class: obj.$class, value: Object.fromEntries(obj.value)};
}

/**
* Visitor design pattern
* @param {ClassDeclaration} classDeclaration - the object being visited
Expand Down Expand Up @@ -148,7 +162,12 @@ class JSONGenerator {
result = this.convertToJSON(field, obj);
} else if (ModelUtil.isEnum(field)) {
result = this.convertToJSON(field, obj);
} else {
} else if (ModelUtil.isMap(field)) {
parameters.stack.push(obj);
const mapDeclaration = parameters.modelManager.getType(field.getFullyQualifiedTypeName());
result = mapDeclaration.accept(this, parameters);
}
else {
parameters.stack.push(obj);
const classDeclaration = parameters.modelManager.getType(obj.getFullyQualifiedType());
result = classDeclaration.accept(this, parameters);
Expand Down
48 changes: 31 additions & 17 deletions packages/concerto-core/lib/serializer/jsonpopulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ class JSONPopulator {

if (thing.isClassDeclaration?.()) {
return this.visitClassDeclaration(thing, parameters);
} else if (thing.isMapDeclaration?.()) {
return this.visitMapDeclaration(thing, parameters);
} else if (thing.isRelationship?.()) {
return this.visitRelationshipDeclaration(thing, parameters);
} else if (thing.isTypeScalar?.()) {
Expand Down Expand Up @@ -160,6 +162,18 @@ class JSONPopulator {
return resourceObj;
}

/**
* Visitor design pattern
* @param {MapDeclaration} mapDeclaration - the object being visited
* @param {Object} parameters - the parameter
* @return {Object} the result of visiting or null
* @private
*/
visitMapDeclaration(mapDeclaration, parameters) {
const jsonObj = parameters.jsonStack.pop();
return { $class: jsonObj.$class, value: new Map(Object.entries(jsonObj)) };
}

/**
* Visitor design pattern
* @param {Field} field - the object being visited
Expand Down Expand Up @@ -197,7 +211,7 @@ class JSONPopulator {
convertItem(field, jsonItem, parameters) {
let result = null;

if(!field.isPrimitive() && !field.isTypeEnum()) {
if(!field.isPrimitive?.() && !field.isTypeEnum?.()) {
let typeName = jsonItem.$class;
if(!typeName) {
// If the type name is not specified in the data, then use the
Expand All @@ -207,26 +221,26 @@ class JSONPopulator {
}

// This throws if the type does not exist.
const classDeclaration = parameters.modelManager.getType(typeName);
const declaration = parameters.modelManager.getType(typeName);

// create a new instance, using the identifier field name as the ID.
let subResource = null;
if (!declaration.isMapDeclaration?.()) {

// if this is identifiable, then we create a resource
if(classDeclaration.isIdentified()) {
subResource = parameters.factory.newResource(classDeclaration.getNamespace(),
classDeclaration.getName(), jsonItem[classDeclaration.getIdentifierFieldName()] );
}
else {
// otherwise we create a concept
subResource = parameters.factory.newConcept(classDeclaration.getNamespace(),
classDeclaration.getName() );
}
// create a new instance, using the identifier field name as the ID.
let subResource = null;

result = subResource;
parameters.resourceStack.push(subResource);
// if this is identifiable, then we create a resource
if (declaration.isIdentified()) {
subResource = parameters.factory.newResource(declaration.getNamespace(),
declaration.getName(), jsonItem[declaration.getIdentifierFieldName()] );
} else {
// otherwise we create a concept
subResource = parameters.factory.newConcept(declaration.getNamespace(),
declaration.getName());
}
parameters.resourceStack.push(subResource);
}
parameters.jsonStack.push(jsonItem);
classDeclaration.accept(this, parameters);
result = declaration.accept(this, parameters);
}
else {
result = this.convertToObject(field, jsonItem, parameters);
Expand Down
54 changes: 51 additions & 3 deletions packages/concerto-core/lib/serializer/resourcevalidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ class ResourceValidator {
return this.visitEnumDeclaration(thing, parameters);
} else if (thing.isClassDeclaration?.()) {
return this.visitClassDeclaration(thing, parameters);
} else if (thing.isRelationship?.()) {
} else if (thing.isMapDeclaration?.()) {
return this.visitMapDeclaration(thing, parameters);
}else if (thing.isRelationship?.()) {
return this.visitRelationshipDeclaration(thing, parameters);
} else if (thing.isTypeScalar?.()) {
return this.visitField(thing.getScalarField(), parameters);
Expand Down Expand Up @@ -104,6 +106,36 @@ class ResourceValidator {
return null;
}

/**
* Visitor design pattern
*
* @param {MapDeclaration} mapDeclaration - the object being visited
* @param {Object} parameters - the parameter
* @private
*/
visitMapDeclaration(mapDeclaration, parameters) {
const obj = parameters.stack.pop();

if (!((obj.value instanceof Map))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not want to accept simple JS objects, indexed as Maps? I think this will force users to create Maps in their code rather than just JS objects.

throw new Error('Expected a Map, but found ' + JSON.stringify(obj));
}

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

obj.value.forEach((key,value) => {
if(!ModelUtil.isSystemProperty(key)) {
if (typeof key !== 'string') {
ResourceValidator.reportInvalidMap(parameters.rootResourceIdentifier, mapDeclaration, obj);
}
if (typeof value !== 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only support values that are Strings?

ResourceValidator.reportInvalidMap(parameters.rootResourceIdentifier, mapDeclaration, obj);
}
}
});
}

/**
* Visitor design pattern
* @param {ClassDeclaration} classDeclaration - the object being visited
Expand Down Expand Up @@ -450,7 +482,7 @@ class ResourceValidator {
/**
* Throw a new error for a model violation.
* @param {string} id - the identifier of this instance.
* @param {classDeclaration} classDeclaration - the declaration of the classs
* @param {ClassDeclaration} classDeclaration - the declaration of the class
* @param {Object} value - the value of the field.
* @private
*/
Expand All @@ -466,7 +498,23 @@ class ResourceValidator {
/**
* Throw a new error for a model violation.
* @param {string} id - the identifier of this instance.
* @param {RelationshipDeclaration} relationshipDeclaration - the declaration of the classs
* @param {MapDeclaration} mapDeclaration - the declaration of the map
* @param {Object} value - the value of the field.
* @private
*/
static reportInvalidMap(id, mapDeclaration, value) {
let formatter = Globalize.messageFormatter('resourcevalidator-invalidmap');
throw new ValidationException(formatter({
resourceId: id,
classFQN: mapDeclaration.getFullyQualifiedName(),
invalidValue: value.toString()
}));
}

/**
* Throw a new error for a model violation.
* @param {string} id - the identifier of this instance.
* @param {RelationshipDeclaration} relationshipDeclaration - the declaration of the class
* @param {Object} value - the value of the field.
* @private
*/
Expand Down
16 changes: 16 additions & 0 deletions packages/concerto-core/lib/serializer/valuegenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,14 @@ class EmptyValueGenerator {
return enumValues[0];
}

/**
* Get an instanace of an empty map.
* @return {*} an map value.
*/
getMap() {
return new Map();
}

/**
* Get an array using the supplied callback to obtain array values.
* @param {Function} valueSupplier - callback to obtain values.
Expand Down Expand Up @@ -328,6 +336,14 @@ class SampleValueGenerator extends EmptyValueGenerator {
return enumValues[Math.floor(Math.random() * enumValues.length)];
}

/**
* Get a map instance with randomly generated values for key & value.
* @return {*} an enum value.
*/
getMap() {
return new Map([[this.getString(1,10), this.getString(1,10)]]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Values don't have to be strings though... won't this generate invalid maps? I think you want to return a map of N entries, with random string keys (use the same method we use to generate random strings) and then random values, based on the type of the value — again reusing the existing logic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but for now we are supporting <String, String> only. In future the method will generate random values of all types supported.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the grammar for CTO aligned with this?

}

/**
* Get an array using the supplied callback to obtain array values.
* @param {Function} valueSupplier - callback to obtain values.
Expand Down
1 change: 1 addition & 0 deletions packages/concerto-core/messages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"resourcevalidator-undeclaredfield": "Instance \"{resourceId}\" has a property named \"{propertyName}\", which is not declared in \"{fullyQualifiedTypeName}\".",
"resourcevalidator-invalidfieldassignment": "Instance \"{resourceId}\" has a property \"{propertyName}\" with type \"{objectType}\" that is not derived from \"{fieldType}\".",
"resourcevalidator-emptyidentifier": "Instance \"{resourceId}\" has an empty identifier.",
"resourcevalidator-invalidmap": "Model violation in the \"{resourceId}\" instance. Invalid Type for Map Key or Value - expected String type.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Map values have to be Strings?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per comment below: The alignment on the first release was <String, String> support only.


"typenotfounderror-defaultmessage": "Type \"{typeName}\" not found.",

Expand Down
6 changes: 6 additions & 0 deletions packages/concerto-core/test/data/model/concept.cto
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ concept InventorySets {
o String Model
o Integer invCount
o assetStatus invType // used or new?
o Dictionary dictionary optional
}

asset MakerInventory identified by makerId {
Expand All @@ -35,3 +36,8 @@ enum assetStatus {
o REPAIRED
o RETIRED
}

map Dictionary {
o String
o String
}
Loading