Skip to content

Commit

Permalink
Merge a860da8 into 29506ee
Browse files Browse the repository at this point in the history
  • Loading branch information
jonaslagoni committed May 20, 2021
2 parents 29506ee + a860da8 commit 83cb459
Show file tree
Hide file tree
Showing 8 changed files with 200 additions and 5 deletions.
10 changes: 8 additions & 2 deletions docs/interpretation_of_JSON_Schema_draft_7.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,28 @@ The algorithm tries to get to a model whose data can be validated against the JS
We only provide the underlying structure of the schema file for the model formats such as `maxItems`, `uniqueItems`, `multipleOf`, etc, are not transformed.

## Interpreter
The main functionality is located in the `Transformer` class. This class ensures to recursively create (or retrieve from a cache) a `CommonModel` representation of a Schema. We have tried to keep the functionality split out into separate functions to reduce complexity and ensure it is easier to maintain. This main function also ensures to split any created models into separate ones if needed.
The main functionality is located in the `Interpreter` class. This class ensures to recursively create (or retrieve from a cache) a `CommonModel` representation of a Schema. We have tried to keep the functionality split out into separate functions to reduce complexity and ensure it is easier to maintain. This main function also ensures to split any created models into separate ones if needed.

The order of transformation:
- [type](#determining-the-type-for-the-model)
- `required` are determined as is.
- `properties` are determined as is, where duplicate properties for the model are merged.
- [allOf](#allOf-sub-schemas)
- [oneOf/anyOf/then/else](#Processing-sub-schemas)

## allOf sub schemas
`allOf` are a bit different then the other [combination keywords](#Processing-sub-schemas) since it can imply inheritance.

So dependant on whether the simplify option `allowInheritance` is true or false we interpret it as inheritance or simply merge the models together.

## Determining the type for the model
To determine the types for the model we use the following interpretation (and in that order):
- `true` schema infers all model types (`object`, `string`, `number`, `array`, `boolean`, `null`, `integer`).
- Usage of `type` infers the initial model type.
- Usage of `properties` infers `object` model type.

## Processing sub schemas
The following JSON Schema keywords are merged with the already transformed model:
The following JSON Schema keywords are merged with the already interpreted model:
- oneOf
- anyOf
- then
Expand Down
31 changes: 31 additions & 0 deletions src/interpreter/InterpretAllOf.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { Logger } from '../utils';
import { CommonModel } from '../models/CommonModel';
import { Schema } from '../models/Schema';
import { Interpreter } from './Interpreter';
import { isModelObject } from './Utils';

/**
* Interpreter function for JSON Schema draft 7 allOf keyword.
*
* It either merges allOf schemas into existing model or if allowed, create inheritance.
*
* @param schema
* @param model
* @param interpreter
*/
export default function interpretAllOf(schema: Schema | boolean, model: CommonModel, interpreter : Interpreter) {
if (typeof schema === 'boolean' || schema.allOf === undefined) return;

for (const allOfSchema of schema.allOf) {
const interpretedModels = interpreter.interpret(allOfSchema);
if (interpretedModels.length === 0) continue;
const interpretedModel = interpretedModels[0];
if (isModelObject(interpretedModel) && interpreter.options.allowInheritance === true) {
Logger.info(`Processing allOf, inheritance is allowed, ${model.$id} inherits from ${interpretedModel.$id}`, model, interpretedModel);
model.addExtendedModel(interpretedModel);
} else {
Logger.info('Processing allOf, inheritance is not allowed. AllOf model is merged together with already interpreted model', model, interpretedModel);
interpreter.combineSchemas(allOfSchema, model, schema);
}
}
}
2 changes: 2 additions & 0 deletions src/interpreter/Interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { CommonModel, Schema } from '../models';
import { SimplificationOptions } from '../models/SimplificationOptions';
import { interpretName, isModelObject } from './Utils';
import interpretProperties from './InterpretProperties';
import interpretAllOf from './InterpretAllOf';
import { Logger } from '../utils';

export class Interpreter {
Expand Down Expand Up @@ -78,6 +79,7 @@ export class Interpreter {
model.required = schema.required || model.required;

interpretProperties(schema, model, this);
interpretAllOf(schema, model, this);

this.combineSchemas(schema.oneOf, model, schema);
this.combineSchemas(schema.anyOf, model, schema);
Expand Down
20 changes: 20 additions & 0 deletions src/models/CommonModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,26 @@ export class CommonModel extends CommonSchema<CommonModel> {
}
}

/**
* Adds another model this model should extend.
*
* It is only allowed to extend if the other model have $id and is not already being extended.
*
* @param extendedModel
*/
addExtendedModel(extendedModel: CommonModel) {
if (extendedModel.$id === undefined) {
Logger.error('Found no $id for allOf model and cannot extend the existing model, this should never happen.', this, extendedModel);
return;
}
this.extend = this.extend || [];
if (this.extend.includes(extendedModel.$id)) {
Logger.info(`${this.$id} model already extends model ${extendedModel.$id}.`, this, extendedModel);
return;
}
this.extend.push(extendedModel.$id);
}

/**
* This function returns an array of `$id`s from all the CommonModel's it immediate depends on.
*/
Expand Down
9 changes: 9 additions & 0 deletions test/interpreter/Intepreter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {Interpreter} from '../../src/interpreter/Interpreter';
import {isModelObject, interpretName} from '../../src/interpreter/Utils';
import interpretProperties from '../../src/interpreter/InterpretProperties';
import { CommonModel, Schema } from '../../src/models';
import interpretAllOf from '../../src/interpreter/InterpretAllOf';

let mockedIsModelObjectReturn = false;
jest.mock('../../src/interpreter/Utils', () => {
Expand All @@ -13,6 +14,7 @@ jest.mock('../../src/interpreter/Utils', () => {
}
});
jest.mock('../../src/interpreter/InterpretProperties');
jest.mock('../../src/interpreter/InterpretAllOf');
CommonModel.mergeCommonModels = jest.fn();
/**
* Some of these test are purely theoretical and have little if any merit
Expand Down Expand Up @@ -160,6 +162,13 @@ describe('Interpreter', function() {
expect(interpretProperties).toHaveBeenNthCalledWith(1, schema, expect.anything(), expect.anything());
});

test('should always try to interpret allOf', function() {
const schema = {};
const interpreter = new Interpreter();
interpreter.interpret(schema);
expect(interpretAllOf).toHaveBeenNthCalledWith(1, schema, expect.anything(), expect.anything());
});

test('should support primitive roots', function() {
const schema = { 'type': 'string' };
const interpreter = new Interpreter();
Expand Down
93 changes: 93 additions & 0 deletions test/interpreter/InterpretAllOf.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/* eslint-disable no-undef */
import { CommonModel } from '../../src/models/CommonModel';
import { Interpreter } from '../../src/interpreter/Interpreter';
import { isModelObject } from '../../src/interpreter/Utils';
import interpretAllOf from '../../src/interpreter/InterpretAllOf';
import { SimplificationOptions } from '../../src/models/SimplificationOptions';

let interpreterOptions: SimplificationOptions = {};
let interpretedReturnModels = [new CommonModel()];
jest.mock('../../src/interpreter/Interpreter', () => {
return {
Interpreter: jest.fn().mockImplementation(() => {
return {
interpret: jest.fn().mockImplementation(() => {return interpretedReturnModels}),
combineSchemas: jest.fn(),
options: interpreterOptions
};
})
};
});
jest.mock('../../src/models/CommonModel');
let mockedIsModelObjectReturn = false;
jest.mock('../../src/interpreter/Utils', () => {
return {
isModelObject: jest.fn().mockImplementation(() => {
return mockedIsModelObjectReturn;
})
}
});
CommonModel.mergeCommonModels = jest.fn();
/**
* Some of these test are purely theoretical and have little if any merit
* on a JSON Schema which actually makes sense but are used to test the principles.
*/
describe('Interpretation of allOf', () => {

beforeEach(() => {
jest.clearAllMocks();
interpreterOptions = {allowInheritance: true};
interpretedReturnModels = [new CommonModel()];
mockedIsModelObjectReturn = false;
});
afterAll(() => {
jest.restoreAllMocks();
});

test('should not do anything if schema does not contain allOf', function() {
const model = new CommonModel();
const interpreter = new Interpreter();
interpretAllOf({}, model, interpreter);
expect(interpreter.combineSchemas).not.toHaveBeenCalled();
expect(JSON.stringify(model)).toEqual(JSON.stringify(new CommonModel()));
});

test('should combine schemas if inheritance is disabled', function() {
const model = new CommonModel();
const schema = { allOf: [{}] };
interpreterOptions.allowInheritance = false;
const interpreter = new Interpreter(interpreterOptions);
interpretAllOf(schema, model, interpreter);
expect(interpreter.combineSchemas).toHaveBeenNthCalledWith(1, schema.allOf[0], model, schema);
expect(JSON.stringify(model)).toEqual(JSON.stringify(new CommonModel()));
});

test('should ignore model if interpreter cannot interpret schema', function() {
const model = new CommonModel();
const schema = { allOf: [{}] };
interpretedReturnModels.pop();
const interpreter = new Interpreter(interpreterOptions);
interpretAllOf(schema, model, interpreter);
expect(interpreter.combineSchemas).not.toHaveBeenCalled();
expect(JSON.stringify(model)).toEqual(JSON.stringify(new CommonModel()));
});
test('should handle empty allOf array', function() {
const model = new CommonModel();
const schema = { allOf: [] };
const interpreter = new Interpreter(interpreterOptions);
interpretAllOf(schema, model, interpreter);
expect(interpreter.combineSchemas).not.toHaveBeenCalled();
expect(JSON.stringify(model)).toEqual(JSON.stringify(new CommonModel()));
});
test('should extend model', function() {
const model = new CommonModel();
const schema = { allOf: [{type: "object", $id: "test"}] };
interpretedReturnModels[0].$id = "test";
mockedIsModelObjectReturn = true;
const interpreter = new Interpreter(interpreterOptions);
interpretAllOf(schema, model, interpreter);
expect(interpreter.combineSchemas).not.toHaveBeenCalled();
expect(isModelObject).toHaveBeenCalled();
expect(model.addExtendedModel).toHaveBeenCalledWith(interpretedReturnModels[0]);
});
});
17 changes: 14 additions & 3 deletions test/interpreter/InterpretProperties.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
import { CommonModel } from '../../src/models/CommonModel';
import { Interpreter } from '../../src/interpreter/Interpreter';
import interpretProperties from '../../src/interpreter/InterpretProperties';
const mockedSimplifierModel = new CommonModel();
let interpretedReturnModels = [new CommonModel()];
jest.mock('../../src/interpreter/Interpreter', () => {
return {
Interpreter: jest.fn().mockImplementation(() => {
return {
interpret: jest.fn().mockImplementation(() => {
return [mockedSimplifierModel];
return interpretedReturnModels;
})
};
})
Expand All @@ -24,6 +24,7 @@ describe('Interpretation of properties', () => {

beforeEach(() => {
jest.clearAllMocks();
interpretedReturnModels = [new CommonModel()];
});
afterAll(() => {
jest.restoreAllMocks();
Expand All @@ -42,6 +43,14 @@ describe('Interpretation of properties', () => {
interpretProperties(true, model, interpreter);
expect(JSON.stringify(model)).toEqual(JSON.stringify(new CommonModel()));
});
test('should ignore model if interpreter cannot interpret property schema', () => {
const schema: any = { properties: { 'property1': { type: 'string' } } };
const model = new CommonModel();
const interpreter = new Interpreter();
interpretedReturnModels.pop();
interpretProperties(schema, model, interpreter);
expect(model.addProperty).not.toHaveBeenCalled();
});
test('should infer type of model', () => {
const schema: any = { properties: { 'property1': { type: 'string' } } };
const model = new CommonModel();
Expand All @@ -51,10 +60,12 @@ describe('Interpretation of properties', () => {
});
test('should go trough properties and add it to model', () => {
const schema: any = { properties: { 'property1': { type: 'string' } } };
const interpretedModel = new CommonModel();
interpretedReturnModels = [interpretedModel];
const model = new CommonModel();
const interpreter = new Interpreter();
interpretProperties(schema, model, interpreter);
expect(interpreter.interpret).toHaveBeenNthCalledWith(1, { type: 'string' });
expect(model.addProperty).toHaveBeenNthCalledWith(1, "property1", mockedSimplifierModel, schema);
expect(model.addProperty).toHaveBeenNthCalledWith(1, "property1", interpretedModel, schema);
});
});
23 changes: 23 additions & 0 deletions test/models/CommonModel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,29 @@ describe('CommonModel', function() {
expect(CommonModel.mergeCommonModels).toHaveBeenNthCalledWith(1, propertyModel, propertyModel, {});
});
});
describe('addExtendedModel', function() {
test('should extend model', function() {
const extendedModel = new CommonModel();
extendedModel.$id = "test";
const model = new CommonModel();
model.addExtendedModel(extendedModel);
expect(model.extend).toEqual(["test"]);
});
test('should ignore model if it has no $id', function() {
const extendedModel = new CommonModel();
const model = new CommonModel();
model.addExtendedModel(extendedModel);
expect(model.extend).toBeUndefined();
});
test('should ignore duplicate model $id', function() {
const extendedModel = new CommonModel();
extendedModel.$id = "test";
const model = new CommonModel();
model.addExtendedModel(extendedModel);
model.addExtendedModel(extendedModel);
expect(model.extend).toEqual(["test"]);
});
});
describe('setTypes', function() {
test('should set multiple types', function() {
const model = new CommonModel();
Expand Down

0 comments on commit 83cb459

Please sign in to comment.