diff --git a/lib/codegen/fromcto/avro/avrovisitor.js b/lib/codegen/fromcto/avro/avrovisitor.js index 107a3835..36cafffc 100644 --- a/lib/codegen/fromcto/avro/avrovisitor.js +++ b/lib/codegen/fromcto/avro/avrovisitor.js @@ -48,7 +48,7 @@ class AvroVisitor { } else if (thing.isMapDeclaration?.()) { return; } else if (thing.isTypeScalar?.()) { - return this.visitField(thing.getScalarField(), parameters); + return this.visitField(thing.getScalarField(), parameters, thing.isOptional()); } else if (thing.isField?.()) { return this.visitField(thing, parameters); } else if (thing.isRelationship?.()) { @@ -167,17 +167,19 @@ class AvroVisitor { * Visitor design pattern * @param {Field} field - the object being visited * @param {Object} parameters - the parameter + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional()`. + * Passed explicitly for scalar-typed properties, because `getScalarField()` does not copy `optional`. * @return {Object} the result of visiting or null * @private */ - visitField(field, parameters) { + visitField(field, parameters, isOptional = field.isOptional()) { let avroType = this.toAvroType(field.getType()); if (field.isArray()) { avroType = `array<${avroType}>`; } - if (field.isOptional()) { + if (isOptional) { avroType = `union { null, ${avroType} }`; } diff --git a/lib/codegen/fromcto/graphql/graphqlvisitor.js b/lib/codegen/fromcto/graphql/graphqlvisitor.js index 45fd6ceb..e1cef4ad 100644 --- a/lib/codegen/fromcto/graphql/graphqlvisitor.js +++ b/lib/codegen/fromcto/graphql/graphqlvisitor.js @@ -49,7 +49,7 @@ class GraphQLVisitor { } else if (thing.isModelFile?.()) { return this.visitModelFile(thing, parameters); } else if (thing.isTypeScalar?.()) { - return this.visitField(thing.getScalarField(), parameters); + return this.visitField(thing.getScalarField(), parameters, thing.isOptional()); } else if (thing.isField?.()) { return this.visitField(thing, parameters); } else if (thing.isMapDeclaration?.()) { @@ -140,17 +140,19 @@ class GraphQLVisitor { * Visitor design pattern * @param {Field} field - the object being visited * @param {Object} parameters - the parameter + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional()`. + * Passed explicitly for scalar-typed properties, because `getScalarField()` does not copy `optional`. * @return {Object} the result of visiting or null * @private */ - visitField(field, parameters) { + visitField(field, parameters, isOptional = field.isOptional()) { let type = this.toGraphQLType( this.namespaces ? field.getFullyQualifiedTypeName() : field.getType() ); if(field.isArray()) { type = `[${type}]`; } - if(!field.isOptional()) { + if(!isOptional) { type = `${type}!`; } diff --git a/lib/codegen/fromcto/protobuf/protobufvisitor.js b/lib/codegen/fromcto/protobuf/protobufvisitor.js index 9cde4e0a..31abe0b4 100644 --- a/lib/codegen/fromcto/protobuf/protobufvisitor.js +++ b/lib/codegen/fromcto/protobuf/protobufvisitor.js @@ -41,14 +41,16 @@ class ProtobufVisitor { /** * Transform a Concerto meta property into a Proto3 field rule. * @param {Object} field - the Concerto meta property + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional()`. + * Callers pass it explicitly for scalar-typed properties, because `getScalarField()` does not copy `optional`. * @return {Object} the Proto3 field rule * @public */ - concertoToProto3FieldRule(field) { + concertoToProto3FieldRule(field, isOptional = field.isOptional()) { // An array (repeated) in Proto3 is implicitly optional. if (field.isArray()) { return 'repeated'; - } else if (field.isOptional()) { + } else if (isOptional) { return 'optional'; } else { return null; @@ -178,7 +180,7 @@ class ProtobufVisitor { } else if (thing.isClassDeclaration?.()) { return this.visitClassDeclaration(thing, parameters); } else if (thing.isTypeScalar?.()) { - return this.visitField(thing.getScalarField(), parameters); + return this.visitField(thing.getScalarField(), parameters, thing.isOptional()); } else if (thing.isField?.()) { return this.visitField(thing, parameters); } else if (thing.isRelationship?.()) { @@ -388,12 +390,13 @@ class ProtobufVisitor { * Visitor design pattern * @param {Object} field - the object being visited * @param {Object} parameters - the parameter + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional()` * @private */ - visitField(field, parameters) { + visitField(field, parameters, isOptional = field.isOptional()) { debug('entering visitField', field.getName()); - const preposition = this.concertoToProto3FieldRule(field); + const preposition = this.concertoToProto3FieldRule(field, isOptional); if (ModelUtil.isMap(field)) { const mapDeclaration = field.getModelFile().getType(field.getType()); diff --git a/lib/codegen/fromcto/rust/rustvisitor.js b/lib/codegen/fromcto/rust/rustvisitor.js index a356bf05..4fd1fd93 100644 --- a/lib/codegen/fromcto/rust/rustvisitor.js +++ b/lib/codegen/fromcto/rust/rustvisitor.js @@ -190,7 +190,7 @@ class RustVisitor { } else if (thing.isMapDeclaration?.()) { return this.visitMapDeclaration(thing, parameters); } else if (thing.isTypeScalar?.()) { - return this.visitField(thing.getScalarField(), parameters); + return this.visitField(thing.getScalarField(), parameters, thing.isOptional()); } else if (thing.isField?.()) { return this.visitField(thing, parameters); } else if (thing.isRelationship?.()) { @@ -399,10 +399,12 @@ class RustVisitor { * Visitor design pattern * @param {Field} field - the object being visited * @param {Object} parameters - the parameter + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional?.()`. + * Passed explicitly for scalar-typed properties, because `getScalarField()` does not copy `optional`. * @return {Object} the result of visiting or null * @private */ - visitField(field, parameters) { + visitField(field, parameters, isOptional = field.isOptional?.()) { // Check for decorators const decorators = field.getDecorators?.() || []; @@ -511,7 +513,7 @@ class RustVisitor { // Determine HashMap type let hashMapType = `HashMap<${rustKeyType}, ${rustValueType}>`; - if (field.isOptional?.()) { + if (isOptional) { hashMapType = this.wrapAsOption(hashMapType); } // Add description for HashMap fields @@ -522,7 +524,7 @@ class RustVisitor { parameters.fileWriter.writeLine(2, `rename = "${field.name}",`); // Add skip_serializing_if for optional HashMap fields - if (field.isOptional?.()) { + if (isOptional) { parameters.fileWriter.writeLine( 2, 'skip_serializing_if = "Option::is_none",' @@ -535,7 +537,7 @@ class RustVisitor { if (isKeyDateTime && isValueDateTime) { // Both key and value are DateTime - if (field.isOptional?.()) { + if (isOptional) { parameters.fileWriter.writeLine( 2, 'serialize_with = "serialize_hashmap_datetime_both_option",' @@ -556,7 +558,7 @@ class RustVisitor { } } else if (isKeyDateTime) { // Only key is DateTime - if (field.isOptional?.()) { + if (isOptional) { parameters.fileWriter.writeLine( 2, 'serialize_with = "serialize_hashmap_datetime_key_option",' @@ -577,7 +579,7 @@ class RustVisitor { } } else if (isValueDateTime) { // Only value is DateTime - if (field.isOptional?.()) { + if (isOptional) { parameters.fileWriter.writeLine( 2, 'serialize_with = "serialize_hashmap_datetime_value_option",' @@ -616,7 +618,7 @@ class RustVisitor { parameters.fileWriter.writeLine(1, '#[serde('); parameters.fileWriter.writeLine(2, `rename = "${field.name}",`); - if (field.isOptional?.()) { + if (isOptional) { parameters.fileWriter.writeLine( 2, 'skip_serializing_if = "Option::is_none",' @@ -624,7 +626,7 @@ class RustVisitor { type = this.wrapAsOption(type); } if (this.isDateField(field.type)) { - if (field.isArray?.() && field.isOptional?.()) { + if (field.isArray?.() && isOptional) { // Option>> parameters.fileWriter.writeLine( 2, @@ -644,7 +646,7 @@ class RustVisitor { 2, 'deserialize_with = "deserialize_datetime_array",' ); - } else if (field.isOptional?.()) { + } else if (isOptional) { // Option> parameters.fileWriter.writeLine( 2, diff --git a/test/codegen/fromcto/avro/avrovisitor.js b/test/codegen/fromcto/avro/avrovisitor.js index a841e726..e9bb0c71 100644 --- a/test/codegen/fromcto/avro/avrovisitor.js +++ b/test/codegen/fromcto/avro/avrovisitor.js @@ -39,6 +39,18 @@ concept Person identified by email { } `; +const MODEL_OPTIONAL_SCALAR = ` +namespace test@1.0.0 + +scalar SSN extends String + +concept Person { + o SSN ssn + o SSN taxId optional + o String name optional +} +`; + describe('Avro', function () { describe('samples', () => { @@ -76,4 +88,19 @@ describe('Avro', function () { }).should.throw('Unrecognised type: string, value: \'Something of unrecognised type\''); }); }); + + describe('optional scalar properties', () => { + it('should generate an optional scalar property as a nullable union', () => { + const modelManager = new ModelManager(); + modelManager.addCTOModel(MODEL_OPTIONAL_SCALAR); + const visitor = new AvroVisitor(); + const fileWriter = new InMemoryWriter(); + modelManager.accept(visitor, { fileWriter }); + const file = fileWriter.getFilesInMemory().get('test@1.0.0.avdl'); + file.should.match(/^\s*string ssn;$/m); + file.should.match(/^\s*union \{ null, string \} taxId;$/m); + // The same as an optional property declared with the primitive type + file.should.match(/^\s*union \{ null, string \} name;$/m); + }); + }); }); diff --git a/test/codegen/fromcto/graphql/graphqlvisitor.js b/test/codegen/fromcto/graphql/graphqlvisitor.js index fab34b23..8f10f1c1 100644 --- a/test/codegen/fromcto/graphql/graphqlvisitor.js +++ b/test/codegen/fromcto/graphql/graphqlvisitor.js @@ -29,6 +29,7 @@ const Field = require('@accordproject/concerto-core').Field; const RelationshipDeclaration = require('@accordproject/concerto-core').RelationshipDeclaration; const FileWriter = require('@accordproject/concerto-util').FileWriter; const { ModelUtil } = require('@accordproject/concerto-core'); +const { InMemoryWriter } = require('@accordproject/concerto-util'); let sandbox = sinon.createSandbox(); @@ -56,6 +57,18 @@ asset Vehicle { } `; +const MODEL_OPTIONAL_SCALAR = ` +namespace test@1.0.0 + +scalar SSN extends String + +concept Person { + o SSN ssn + o SSN taxId optional + o String name optional +} +`; + describe('GraphQLVisitor', function () { let graphQLVisitor; let mockFileWriter; @@ -424,4 +437,18 @@ ref: String arrayRef: String) on OBJECT | FIELD_DEFINITION`]); }); }); + + describe('optional scalar properties', () => { + it('should not mark an optional scalar property as non-null', () => { + const modelManager = new ModelManager(); + modelManager.addCTOModel(MODEL_OPTIONAL_SCALAR); + const fileWriter = new InMemoryWriter(); + modelManager.accept(new GraphQLVisitor(), { fileWriter }); + const file = fileWriter.getFilesInMemory().get('model.gql'); + file.should.match(/^\s*ssn: String!$/m); + file.should.match(/^\s*taxId: String$/m); + // The same as an optional property declared with the primitive type + file.should.match(/^\s*name: String$/m); + }); + }); }); \ No newline at end of file diff --git a/test/codegen/fromcto/protobuf/data/org.accordproject.protocol.v1_0_0-expected.proto b/test/codegen/fromcto/protobuf/data/org.accordproject.protocol.v1_0_0-expected.proto index 7039a281..055da9cb 100644 --- a/test/codegen/fromcto/protobuf/data/org.accordproject.protocol.v1_0_0-expected.proto +++ b/test/codegen/fromcto/protobuf/data/org.accordproject.protocol.v1_0_0-expected.proto @@ -48,12 +48,12 @@ message Function { repeated string emittedTypes = 2; string name = 3; string requestType = 4; - string responseType = 5; + optional string responseType = 5; } message Logic { repeated Function functions = 1; - string stateType = 2; + optional string stateType = 2; } message Template { @@ -89,7 +89,7 @@ message Agreement { Metadata metadata = 7; repeated string references = 8; repeated Signature signatures = 9; - string state = 10; + optional string state = 10; string template = 11; } @@ -133,7 +133,7 @@ message _Subclasses_of_class_ConversionOptions { } message PdfConversionOptions { - string styles = 1; + optional string styles = 1; } enum FeatureType { @@ -160,7 +160,7 @@ message TriggerResponse { optional string errorDetails = 1; optional string errorMessage = 2; bool isError = 3; - string result = 4; + optional string result = 4; } enum AgreementStatusType { diff --git a/test/codegen/fromcto/protobuf/protobufvisitor.js b/test/codegen/fromcto/protobuf/protobufvisitor.js index 077956df..983340f4 100644 --- a/test/codegen/fromcto/protobuf/protobufvisitor.js +++ b/test/codegen/fromcto/protobuf/protobufvisitor.js @@ -42,6 +42,18 @@ const ModelUtil = require('@accordproject/concerto-core').ModelUtil; let sandbox = sinon.createSandbox(); +const MODEL_OPTIONAL_SCALAR = ` +namespace test@1.0.0 + +scalar SSN extends String + +concept Person { + o SSN ssn + o SSN taxId optional + o String name optional +} +`; + describe('ProtobufVisitor', function () { let protobufVisitor; let mockFileWriter; @@ -774,4 +786,18 @@ describe('ProtobufVisitor', function () { ); }); }); + + describe('optional scalar properties', () => { + it('should generate an optional scalar property with the optional keyword', () => { + const modelManager = new ModelManager(); + modelManager.addCTOModel(MODEL_OPTIONAL_SCALAR); + const fileWriter = new InMemoryWriter(); + modelManager.accept(new ProtobufVisitor(), { fileWriter }); + const file = fileWriter.getFilesInMemory().get('test.v1_0_0.proto'); + file.should.match(/^\s*string ssn = \d+;$/m); + file.should.match(/^\s*optional string taxId = \d+;$/m); + // The same as an optional property declared with the primitive type + file.should.match(/^\s*optional string name = \d+;$/m); + }); + }); }); diff --git a/test/codegen/fromcto/rust/rustvisitor.js b/test/codegen/fromcto/rust/rustvisitor.js index 313856c3..b8aadd68 100644 --- a/test/codegen/fromcto/rust/rustvisitor.js +++ b/test/codegen/fromcto/rust/rustvisitor.js @@ -33,9 +33,22 @@ const ModelManager = require('@accordproject/concerto-core').ModelManager; const RelationshipDeclaration = require('@accordproject/concerto-core').RelationshipDeclaration; const FileWriter = require('@accordproject/concerto-util').FileWriter; +const { InMemoryWriter } = require('@accordproject/concerto-util'); let sandbox = sinon.createSandbox(); +const MODEL_OPTIONAL_SCALAR = ` +namespace test@1.0.0 + +scalar SSN extends String + +concept Person { + o SSN ssn + o SSN taxId optional + o String name optional +} +`; + describe('RustVisitor', function () { let rustVisitor; let mockFileWriter; @@ -1809,4 +1822,18 @@ describe('RustVisitor', function () { ).calledOnce.should.be.ok; }); }); + + describe('optional scalar properties', () => { + it('should generate an optional scalar property as an Option', () => { + const modelManager = new ModelManager(); + modelManager.addCTOModel(MODEL_OPTIONAL_SCALAR); + const fileWriter = new InMemoryWriter(); + modelManager.accept(new RustVisitor(), { fileWriter }); + const file = fileWriter.getFilesInMemory().get('test_1_0_0.rs'); + file.should.match(/pub ssn: String,/); + file.should.match(/rename = "taxId",\s+skip_serializing_if = "Option::is_none",\s+\)\]\s+pub tax_id: Option,/); + // The same as an optional property declared with the primitive type + file.should.match(/rename = "name",\s+skip_serializing_if = "Option::is_none",\s+\)\]\s+pub name: Option,/); + }); + }); }); diff --git a/types/lib/codegen/fromcto/avro/avrovisitor.d.ts b/types/lib/codegen/fromcto/avro/avrovisitor.d.ts index 9f982dae..1903dbd8 100644 --- a/types/lib/codegen/fromcto/avro/avrovisitor.d.ts +++ b/types/lib/codegen/fromcto/avro/avrovisitor.d.ts @@ -57,6 +57,8 @@ declare class AvroVisitor { * Visitor design pattern * @param {Field} field - the object being visited * @param {Object} parameters - the parameter + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional()`. + * Passed explicitly for scalar-typed properties, because `getScalarField()` does not copy `optional`. * @return {Object} the result of visiting or null * @private */ diff --git a/types/lib/codegen/fromcto/graphql/graphqlvisitor.d.ts b/types/lib/codegen/fromcto/graphql/graphqlvisitor.d.ts index 6deecc6c..5eaca423 100644 --- a/types/lib/codegen/fromcto/graphql/graphqlvisitor.d.ts +++ b/types/lib/codegen/fromcto/graphql/graphqlvisitor.d.ts @@ -52,6 +52,8 @@ declare class GraphQLVisitor { * Visitor design pattern * @param {Field} field - the object being visited * @param {Object} parameters - the parameter + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional()`. + * Passed explicitly for scalar-typed properties, because `getScalarField()` does not copy `optional`. * @return {Object} the result of visiting or null * @private */ diff --git a/types/lib/codegen/fromcto/protobuf/protobufvisitor.d.ts b/types/lib/codegen/fromcto/protobuf/protobufvisitor.d.ts index b0afbff9..3fd8f057 100644 --- a/types/lib/codegen/fromcto/protobuf/protobufvisitor.d.ts +++ b/types/lib/codegen/fromcto/protobuf/protobufvisitor.d.ts @@ -17,10 +17,12 @@ declare class ProtobufVisitor { /** * Transform a Concerto meta property into a Proto3 field rule. * @param {Object} field - the Concerto meta property + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional()`. + * Callers pass it explicitly for scalar-typed properties, because `getScalarField()` does not copy `optional`. * @return {Object} the Proto3 field rule * @public */ - public concertoToProto3FieldRule(field: any): any; + public concertoToProto3FieldRule(field: any, isOptional?: boolean): any; /** * Transform a Concerto primitive type into a Proto3 one. * @param {Object} type - the Concerto primitive type @@ -128,6 +130,7 @@ declare class ProtobufVisitor { * Visitor design pattern * @param {Object} field - the object being visited * @param {Object} parameters - the parameter + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional()` * @private */ private visitField; diff --git a/types/lib/codegen/fromcto/rust/rustvisitor.d.ts b/types/lib/codegen/fromcto/rust/rustvisitor.d.ts index c8e1ff0f..3c16f321 100644 --- a/types/lib/codegen/fromcto/rust/rustvisitor.d.ts +++ b/types/lib/codegen/fromcto/rust/rustvisitor.d.ts @@ -59,6 +59,8 @@ declare class RustVisitor { * Visitor design pattern * @param {Field} field - the object being visited * @param {Object} parameters - the parameter + * @param {boolean} [isOptional] - whether the property is optional, defaults to `field.isOptional?.()`. + * Passed explicitly for scalar-typed properties, because `getScalarField()` does not copy `optional`. * @return {Object} the result of visiting or null * @private */