Generated Proto3 from Concerto CTO models is written by ProtobufVisitor (lib/codegen/fromcto/protobuf/protobufvisitor.js). Today, npm test only checks snapshots, it does not run a Proto3 parser on the output.
When you compile with protobufjs parse(), standard repo fixtures produce invalid .proto files.
Related: bug: GraphQL verification bugs (generated output fails graphql-js) #248
Bug 1: optional map<...> in hr.cto
Cause
hr.cto defines optional map fields on concepts such as Company:
concept Company {
...
o CompanyProperties companyProperties optional
...
}
ProtobufVisitor emits optional before map<...> field definitions. In Proto3, map fields cannot use the optional label - map entries are already implicitly optional.
Reproduce
From the repo root (after npm ci):
node -e "
const fs = require('fs');
const path = require('path');
const protobuf = require('protobufjs');
const { dir } = require('tmp-promise');
const { ModelManager } = require('@accordproject/concerto-core');
const { FileWriter } = require('@accordproject/concerto-util');
const ProtobufVisitor = require('./lib/codegen/fromcto/protobuf/protobufvisitor.js');
process.env.ENABLE_MAP_TYPE = 'true';
process.env.IMPORT_ALIASING = 'true';
const MODEL_DIR = './test/codegen/fromcto/data/model';
const GOOGLE_PROTO_ROOT = path.dirname(require.resolve('protobufjs/package.json'));
(async () => {
const mm = new ModelManager();
mm.addCTOModel(fs.readFileSync(path.join(MODEL_DIR, 'hr_base.cto'), 'utf8'), 'hr_base.cto');
mm.addCTOModel(fs.readFileSync(path.join(MODEL_DIR, 'hr.cto'), 'utf8'), 'hr.cto');
const { path: out, cleanup } = await dir({ unsafeCleanup: true });
mm.accept(new ProtobufVisitor(), {
fileWriter: new FileWriter(out),
showCompositionRelationships: true,
});
const root = new protobuf.Root();
root.resolvePath = (origin, target) => {
if (target.startsWith('google/protobuf/')) {
return path.join(GOOGLE_PROTO_ROOT, target);
}
return path.join(out, path.basename(target));
};
const proto = fs.readFileSync(path.join(out, 'org.acme.hr.v1_0_0.proto'), 'utf8');
protobuf.parse(proto, root, 'org.acme.hr.v1_0_0.proto');
console.log('OK');
await cleanup();
})().catch((err) => {
console.error(err.message);
process.exit(1);
});
"
Expected error
illegal name '<' (line 19)
Generated fragment
message Company {
optional map<string, string> CompanyProperties = 1;
optional map<SSN, Employee> EmployeeDirectory = 2;
...
}
Bug 2: Custom scalar map keys (e.g. SSN) in hr_base.cto + hr.cto
Cause
hr_base.cto defines a custom scalar SSN. hr.cto uses it as a map key in declarations such as EmployeeDirectory and EmployeeTShirtSizes.
Proto3 only allows integral types (int32, int64, uint32, uint64, sint32, sint64, fixed32, fixed64, sfixed32, sfixed64) and bool/string as map key types. A custom scalar like SSN is not valid.
Reproduce
Written for powershell
@'
const protobuf = require("protobufjs");
const proto = [
'syntax = "proto3";',
'message SSN {}',
'message Employee {}',
'message Company {',
' map<SSN, Employee> EmployeeDirectory = 1;',
'}',
].join("\n");
try {
protobuf.parse(proto);
console.log("OK");
} catch (err) {
console.error(err.message);
process.exit(1);
}
'@ | node
Expected error
illegal type 'SSN' (line 5)
Generated references
optional map<SSN, Employee> EmployeeDirectory = 2;
optional map<SSN, TShirtSizeType> EmployeeTShirtSizes = 5;
map<SSN, KinTelephone> NextOfKin = 12;
Related code
| File |
Role |
lib/codegen/fromcto/protobuf/protobufvisitor.js |
Emits Proto3 (visitMapDeclaration, optional field rules) |
test/codegen/codegen.js |
Snapshot tests (no Proto3 parser on output) |
test/codegen/fromcto/protobuf/protobufvisitor.js |
Unit tests with mocks, not full HR fixtures |
Generated Proto3 from Concerto CTO models is written by
ProtobufVisitor(lib/codegen/fromcto/protobuf/protobufvisitor.js). Today,npm testonly checks snapshots, it does not run a Proto3 parser on the output.When you compile with protobufjs
parse(), standard repo fixtures produce invalid.protofiles.Related: bug: GraphQL verification bugs (generated output fails graphql-js) #248
Bug 1:
optional map<...>inhr.ctoCause
hr.ctodefines optional map fields on concepts such asCompany:ProtobufVisitoremitsoptionalbeforemap<...>field definitions. In Proto3,mapfields cannot use theoptionallabel -mapentries are already implicitly optional.Reproduce
From the repo root (after
npm ci):Expected error
Generated fragment
Bug 2: Custom scalar map keys (e.g.
SSN) inhr_base.cto+hr.ctoCause
hr_base.ctodefines a custom scalarSSN.hr.ctouses it as a map key in declarations such asEmployeeDirectoryandEmployeeTShirtSizes.Proto3 only allows integral types (
int32,int64,uint32,uint64,sint32,sint64,fixed32,fixed64,sfixed32,sfixed64) andbool/stringas map key types. A custom scalar likeSSNis not valid.Reproduce
Expected error
Generated references
Related code
lib/codegen/fromcto/protobuf/protobufvisitor.jsvisitMapDeclaration, optional field rules)test/codegen/codegen.jstest/codegen/fromcto/protobuf/protobufvisitor.js