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(tools): improve JSON schema inference #549

Merged
merged 1 commit into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion packages/concerto-cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ require('yargs')
type: 'string'
});
yargs.option('namespace', {
describe: 'The namepspace for the output model',
describe: 'The namespace for the output model',
type: 'string',
});
yargs.option('typeName', {
Expand Down
4 changes: 0 additions & 4 deletions packages/concerto-cli/test/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -563,8 +563,6 @@ describe('concerto-cli', () => {
);
obj.should.equal(`namespace concerto.test.jsonSchema

import org.accordproject.time.* from https://models.accordproject.org/[email protected]

concept Root {
o String name optional
o Root[] children optional
Expand All @@ -582,8 +580,6 @@ concept Root {
);
obj.should.equal(`namespace petstore

import org.accordproject.time.* from https://models.accordproject.org/[email protected]

concept Pet {
o NewPet pet optional
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,26 @@ function parseIdUri(id) {

/**
* Infer a type name for a definition. Examines $id, title and parent declaration
* @param {*} definition the input object
* @param {*} context the processing context
* @param {object} definition - the input object
* @param {*} context - the processing context
* @param {boolean} [skipDictionary] - if true, this function will not use the dictionary help inference
* @returns {string} A name for the definition
* @private
*/
function inferTypeName(definition, context) {
function inferTypeName(definition, context, skipDictionary) {
if (definition.$ref) {
return normalizeType(definition.$ref);
}

const name = context.parents.peek();
const { type } = parseIdUri(definition.$id) ||
{ type: definition.title || name };
return normalizeType(type);

if (skipDictionary || context.dictionary.has(normalizeType(type))){
return normalizeType(type);
}
// We fallback to a stringified object representation. This is "untyped".
return 'String';
}

/**
Expand Down Expand Up @@ -122,7 +128,8 @@ function inferType(definition, context) {
if (definition.format === 'date-time' || definition.format === 'date') {
return 'DateTime';
} else {
throw new Error(`Format '${definition.format}' in '${name}' is not supported`);
console.warn(`Format '${definition.format}' in '${name}' is not supported. It has been ignored.`);
return 'String';
}
}
return 'String';
Expand All @@ -143,13 +150,15 @@ function inferType(definition, context) {

// Hack until we support union types.
// https://github.com/accordproject/concerto/issues/292
if (definition.anyOf){
const alternative = definition.anyOf || definition.oneOf;
if (alternative){
const keyword = definition.anyOf ? 'anyOf' : 'oneOf';
console.warn(
`Keyword 'anyOf' in definition '${name}' is not fully supported. Defaulting to first alternative.`
`Keyword '${keyword}' in definition '${name}' is not fully supported. Defaulting to first alternative.`
);

// Just choose the first item
return inferType(definition.anyOf[0], context);
return inferType(alternative[0], context);
}

throw new Error(`Unsupported definition: ${JSON.stringify(definition)}`);
Expand Down Expand Up @@ -253,9 +262,13 @@ function inferDeclaration(definition, context) {
} else if (definition.type) {
if (definition.type === 'object') {
inferConcept(definition, context);
} else if (definition.type === 'array') {
console.warn(
`Type keyword 'array' in definition '${name}' is not supported. It has been ignored.`
);
} else {
throw new Error(
`Type keyword '${definition.type}' in definition '${name}' not supported.`
`Type keyword '${definition.type}' in definition '${name}' is not supported.`
);
}
} else {
Expand All @@ -265,7 +278,7 @@ function inferDeclaration(definition, context) {
!key.startsWith('x-') // Ignore custom extensions
);
console.warn(
`Keyword(s) '${badKeys.join('\', \'')}' in definition '${name}' is not supported.`
`Keyword(s) '${badKeys.join('\', \'')}' in definition '${name}' are not supported.`
);
}
}
Expand Down Expand Up @@ -304,18 +317,32 @@ function inferModelFile(defaultNamespace, defaultType, schema) {
const context = {
parents: new TypedStack(),
writer: new Writer(),
dictionary: new Set(),
};

context.writer.writeLine(0, `namespace ${namespace}`);
context.writer.writeLine(0, '');

// Add imports
// TODO we need some heuristic or metadata to identify Concerto dependencies rather than making assumptions
context.writer.writeLine(0, 'import org.accordproject.time.* from https://models.accordproject.org/[email protected]');
context.writer.writeLine(0, '');

// Create definitions
const defs = schema.definitions || schema.$defs || schema?.components?.schemas ||[];

// Build a dictionary
context.dictionary.add(defaultType);
if (schema.$id) {
context.dictionary.add(normalizeType(parseIdUri(schema.$id).type));
}
Object.keys(defs).forEach((key) => {
context.parents.push(key);
const definition = defs[key];
const typeName = inferTypeName(definition, context, true);
if (context.dictionary.has(typeName)){
throw new Error(`Duplicate definition found for type '${typeName}'.`);
}
context.dictionary.add(typeName);
context.parents.pop();
});

// Visit each declaration
Object.keys(defs).forEach((key) => {
context.parents.push(key);
const definition = defs[key];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
namespace org.acme

import org.accordproject.time.* from https://models.accordproject.org/[email protected]

concept Children {
o String name
o Integer age
Expand All @@ -16,6 +14,8 @@ concept Children {
o String[] emptyArray
o Pet[] favoritePets
o Stuff[] stuff
o String json optional
o Double alternation optional
}

enum Color {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,16 @@
},
"required": ["$class", "sku", "price", "product"]
}
},
"json": {
"type": "object"
},
"alternation": {
"oneOf": [
{ "type": "number" },
{ "type": "string" }
]

}
},
"required": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ describe('inferModel', function () {
});
cto.should.equal(`namespace org.acme

import org.accordproject.time.* from https://models.accordproject.org/[email protected]

enum Root {
o one
o two
Expand All @@ -86,13 +84,11 @@ enum Root {
}
}
});
// TODO This is not a valid CTO model, because we don't generate definitions for inline sub-schemas.
// TODO Generate definitions for inline sub-schemas.
cto.should.equal(`namespace org.acme

import org.accordproject.time.* from https://models.accordproject.org/[email protected]

concept Root {
o Xs[] xs optional
o String[] xs optional
}

`);
Expand All @@ -112,8 +108,6 @@ concept Root {
);
cto.should.equal(`namespace org.acme

import org.accordproject.time.* from https://models.accordproject.org/[email protected]

concept Root {
o String name optional
o Root[] children optional
Expand All @@ -127,8 +121,6 @@ concept Root {
const cto = inferModel('org.acme', 'Root', schema);
cto.should.equal(`namespace com.example

import org.accordproject.time.* from https://models.accordproject.org/[email protected]

concept Veggie {
o String veggieName
o Boolean veggieLike
Expand All @@ -147,8 +139,6 @@ concept Arrays {
const cto = inferModel('org.acme', 'Root', schema);
cto.should.equal(`namespace com.example

import org.accordproject.time.* from https://models.accordproject.org/[email protected]

concept Geographical_location {
o String name default="home" regex=/[\\w\\s]+/ optional
o Double latitude
Expand Down Expand Up @@ -190,23 +180,28 @@ concept Geographical_location {
}).should.throw('\'additionalProperties\' are not supported in Concerto');
});

it('should not generate when unsupported formats are used', async () => {
(function () {
inferModel('org.acme', 'Root', {
$schema: 'http://json-schema.org/draft-07/schema#',
definitions: {
Foo: {
type: 'object',
properties: {
email: {
type: 'string',
format: 'email'
}
it('should quietly accept unsupported formats', async () => {
const cto = inferModel('org.acme', 'Root', {
$schema: 'http://json-schema.org/draft-07/schema#',
definitions: {
Foo: {
type: 'object',
properties: {
email: {
type: 'string',
format: 'email'
}
}
}
});
}).should.throw('Format \'email\' in \'email\' is not supported');
}
});
cto.should.equal(`namespace org.acme

concept Foo {
o String email optional
}

`);
});

it('should not generate when unsupported type keywords are used', async () => {
Expand All @@ -219,7 +214,7 @@ concept Geographical_location {
}
}
});
}).should.throw('Type keyword \'null\' in definition \'Foo\' not supported.');
}).should.throw('Type keyword \'null\' in definition \'Foo\' is not supported.');
});

it('should not generate when unsupported type keywords are used in an object', async () => {
Expand All @@ -240,11 +235,36 @@ concept Geographical_location {
}).should.throw('Type keyword \'null\' in \'email\' is not supported');
});

it('should not fail for unsupported keywords', async () => {
inferModel('org.acme', 'Root', {
it('should not generate when duplicate definitions are found', async () => {
(function () {
inferModel('org.acme', 'Foo', {
$schema: 'http://json-schema.org/draft-07/schema#',
definitions: {
'Foo': {
'type': 'object',
},
}
});
}).should.throw('Duplicate definition found for type \'Foo\'');
});

it('should quietly accept array definitions', async () => {
const cto = inferModel('org.acme', 'Root', {
type: 'array'
});
cto.should.equal(`namespace org.acme

`);
});

it('should quietly accept unsupported definitions', async () => {
const cto = inferModel('org.acme', 'Root', {
'allOf': [
{ 'type': 'string' }
]
});
cto.should.equal(`namespace org.acme

`);
});
});