From e957a92346984583b30edd6328b22294b0bc5b92 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 15 Mar 2016 16:09:16 +0200 Subject: [PATCH] allOf merging fixes (fixes #31) --- lib/components/JsonSchema/json-schema.js | 1 + lib/components/base.js | 41 ++++++++++++------ lib/components/base.spec.js | 46 ++++++++++++++++++--- tests/schemas/base-component-joinallof.json | 18 +++++++- 4 files changed, 86 insertions(+), 20 deletions(-) diff --git a/lib/components/JsonSchema/json-schema.js b/lib/components/JsonSchema/json-schema.js index 29b547d9..c4010023 100644 --- a/lib/components/JsonSchema/json-schema.js +++ b/lib/components/JsonSchema/json-schema.js @@ -73,6 +73,7 @@ export default class JsonSchema extends BaseComponent { let props = Object.keys(schema.properties).map((prop, idx) => { let propData = schema.properties[prop]; let propPointer = JsonPointer.join(this.pointer, ['properties', prop]); + this.joinAllOf(propData); propData = JsonSchema.injectPropData(propData, prop, propPointer, this.requiredMap, schema); if (propData.isDiscriminator) discriminatorFieldIdx = idx; return propData; diff --git a/lib/components/base.js b/lib/components/base.js index c8e7146a..64c88c6b 100644 --- a/lib/components/base.js +++ b/lib/components/base.js @@ -17,6 +17,21 @@ function safeConcat(a, b) { return res.concat(b); } +function defaults(target, src) { + var props = Object.keys(src); + + var index = -1, + length = props.length; + + while (++index < length) { + var key = props[index]; + if (target[key] === undefined) { + target[key] = src[key]; + } + } + return target; +} + function snapshot(obj) { if(obj == null || typeof(obj) != 'object') { return obj; @@ -158,34 +173,34 @@ export class BaseComponent { joinAllOf(schema = this.componentSchema, opts) { var self = this; function merge(into, schemas) { - if (into.required || into.properties) { - let errMessage = `Can\'t merge allOf: properties or required fields are specified on the same level as allOf - ${into}`; - throw new Error(errMessage); - } - into.required = []; - into.properties = {}; for (let subSchema of schemas) { if (opts && opts.omitParent && subSchema.discriminator) continue; - // TODO: add support for merge array schemas - if (typeof subSchema !== 'object' || subSchema.type !== 'object') { - let errMessage = `Can\'t merge allOf: only subschemas with type: object can be merged + if (typeof subSchema !== 'object') { + let errMessage = `Items of allOf should be Object: ${typeof subSchema} found ${subSchema}`; throw new Error(errMessage); } self.joinAllOf(subSchema); + if (into.type && into.type !== subSchema.type) { + let errMessage = `allOf merging error: schemas with different types can't be merged`; + throw new Error(errMessage); + } + // TODO: add check if can be merged correctly (no different properties with the same name) - if (subSchema.properties) { + if (subSchema.type === 'object' && subSchema.properties) { + into.properties || (into.properties = {}); Object.assign(into.properties, subSchema.properties); } - if (subSchema.required) { + if (subSchema.type === 'object' && subSchema.required) { + into.required || (into.required = []); into.required.push(...subSchema.required); } + + defaults(into, subSchema); } - into.type = 'object'; into.allOf = null; } if (schema.allOf) { diff --git a/lib/components/base.spec.js b/lib/components/base.spec.js index 800ad5e7..32a59a8e 100644 --- a/lib/components/base.spec.js +++ b/lib/components/base.spec.js @@ -217,15 +217,51 @@ describe('Redoc components', () => { }); }); - describe('Incorrect or not supported allOf', () => { - it('should throw when properties or required is set on the same level as allOf', () => { - component.pointer = '/definitions/BadAllOf2'; + describe('AllOf with other properties on the allOf level', () => { + let joined; + beforeAll(() => { + component.pointer = '/definitions/AllOfWithOther'; component.ngOnInit(); component.dereference(); - (() => component.joinAllOf()).should.throw(); + component.joinAllOf(); + joined = component.componentSchema; }); - it('should throw when merging non-object schemas', () => { + it('should remove $allOf field', () => { + expect(joined.allOf).toBeNull(); + }); + + it('should set type object', () => { + joined.type.should.be.equal('object'); + }); + + it('should merge properties', () => { + Object.keys(joined.properties).length.should.be.equal(1); + Object.keys(joined.properties).should.be.deepEqual(['id']); + }); + + it('should merge required', () => { + joined.required.length.should.be.equal(1); + joined.required.should.be.deepEqual(['id']); + }); + + it('should preserve parent properties', () => { + joined.description.should.be.equal('Test'); + joined.readOnly.should.be.equal(true); + }); + }); + + describe('allOf edgecases', () => { + it('should merge properties and required when defined on allOf level', () => { + component.pointer = '/definitions/PropertiesOnAllOfLevel'; + component.ngOnInit(); + component.dereference(); + (() => component.joinAllOf()).should.not.throw(); + let joined = component.componentSchema; + Object.keys(joined.properties).length.should.be.equal(3); + }); + + it('should throw when merging schemas with different types', () => { component.pointer = '/definitions/BadAllOf1'; component.ngOnInit(); component.dereference(); diff --git a/tests/schemas/base-component-joinallof.json b/tests/schemas/base-component-joinallof.json index 904bc0b2..7d3cd2bb 100644 --- a/tests/schemas/base-component-joinallof.json +++ b/tests/schemas/base-component-joinallof.json @@ -8,6 +8,7 @@ "basePath": "/v2/", "definitions": { "Simple": { + "description": "simple", "type": "object", "required": ["id"], "properties": { @@ -57,6 +58,15 @@ } ] }, + "AllOfWithOther": { + "description": "Test", + "readOnly": true, + "allOf": [ + { + "$ref": "#/definitions/Simple" + } + ] + }, "BadAllOf1": { "allOf": [ { @@ -67,8 +77,12 @@ } ] }, - "BadAllOf2": { - "properties": {}, + "PropertiesOnAllOfLevel": { + "properties": { + "prop": { + "type": "string" + } + }, "allOf": [ { "$ref": "#/definitions/Simple"