From e957a92346984583b30edd6328b22294b0bc5b92 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 15 Mar 2016 16:09:16 +0200 Subject: [PATCH 1/5] 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" From 3a6e2c14c4866f67b962243f8df049507d355371 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 15 Mar 2016 16:09:42 +0200 Subject: [PATCH 2/5] Nested allOf handling --- lib/components/JsonSchema/json-schema.js | 1 - lib/components/base.js | 26 +++++++++++++++++---- lib/components/base.spec.js | 11 +++++++++ tests/schemas/base-component-joinallof.json | 15 ++++++++++++ 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/lib/components/JsonSchema/json-schema.js b/lib/components/JsonSchema/json-schema.js index c4010023..29b547d9 100644 --- a/lib/components/JsonSchema/json-schema.js +++ b/lib/components/JsonSchema/json-schema.js @@ -73,7 +73,6 @@ 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 64c88c6b..16367906 100644 --- a/lib/components/base.js +++ b/lib/components/base.js @@ -171,7 +171,6 @@ export class BaseComponent { } joinAllOf(schema = this.componentSchema, opts) { - var self = this; function merge(into, schemas) { for (let subSchema of schemas) { if (opts && opts.omitParent && subSchema.discriminator) continue; @@ -182,13 +181,15 @@ export class BaseComponent { 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); } + if (into.type === 'array') { + console.warn('allOf: subschemas with type array are not supported yet'); + } + // TODO: add check if can be merged correctly (no different properties with the same name) if (subSchema.type === 'object' && subSchema.properties) { into.properties || (into.properties = {}); @@ -203,9 +204,24 @@ export class BaseComponent { } into.allOf = null; } - if (schema.allOf) { - merge(schema, schema.allOf); + + function traverse(obj) { + if (obj === null || typeof(obj) !== 'object') { + return; + } + + for(var key in obj) { + if (obj.hasOwnProperty(key)) { + traverse(obj[key]); + } + } + + if (obj.allOf) { + merge(obj, obj.allOf); + } } + + traverse(schema); } /** diff --git a/lib/components/base.spec.js b/lib/components/base.spec.js index 32a59a8e..6091ebd6 100644 --- a/lib/components/base.spec.js +++ b/lib/components/base.spec.js @@ -267,6 +267,17 @@ describe('Redoc components', () => { component.dereference(); (() => component.joinAllOf()).should.throw(); }); + + it('should handle nested allOF', () => { + component.pointer = '/definitions/NestedAllOf'; + component.ngOnInit(); + component.dereference(); + (() => component.joinAllOf()).should.not.throw(); + let joined = component.componentSchema; + Object.keys(joined.properties).length.should.be.equal(4); + Object.keys(joined.properties).should.be.deepEqual(['prop1', 'prop2', 'prop3', 'prop4']); + joined.required.should.be.deepEqual(['prop1', 'prop3']); + }); }); xdescribe('Merge array allOf', () => { diff --git a/tests/schemas/base-component-joinallof.json b/tests/schemas/base-component-joinallof.json index 7d3cd2bb..4014b2f2 100644 --- a/tests/schemas/base-component-joinallof.json +++ b/tests/schemas/base-component-joinallof.json @@ -97,6 +97,21 @@ } } ] + }, + "NestedAllOf": { + "allOf": [ + { + "$ref": "#/definitions/SimpleAllOf" + }, + { + "type": "object", + "properties": { + "prop4": { + "type": "string" + } + } + } + ] } }, "paths": { From 95cef711b2ce8dda59308f8d6da8123c4980497e Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 15 Mar 2016 16:10:33 +0200 Subject: [PATCH 3/5] Fix footer and element styling --- lib/components/Redoc/redoc.scss | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/components/Redoc/redoc.scss b/lib/components/Redoc/redoc.scss index 69b72833..a446e4f8 100644 --- a/lib/components/Redoc/redoc.scss +++ b/lib/components/Redoc/redoc.scss @@ -139,21 +139,21 @@ api-logo { color: $red; border: 1px solid rgba(38,50,56,0.1); } +} + +footer { + text-align: right; + padding: 10px; + font-size: 15px; + background-color: white; strong { font-size: 18px; color: $headers-color; } - - footer { - text-align: right; - padding: 10px; - font-size: 15px; - } } - /* markdown elements */ :host .redoc-markdown-block { From 8945f1d6431c7c9fd75a77821a8003fcfb06abfb Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 15 Mar 2016 16:11:43 +0200 Subject: [PATCH 4/5] v0.7.3 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 59b4f73e..243c79ca 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "redoc", "description": "Swagger-generated API Reference Documentation", - "version": "0.7.2", + "version": "0.7.3", "repository": { "type": "git", "url": "git://github.com/Rebilly/ReDoc" From 0d3d9213de9b98aaf1d7b39e3496ba0db29d5619 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 15 Mar 2016 16:14:39 +0200 Subject: [PATCH 5/5] exclude pushpay.com spec from e2e test --- tests/e2e/redoc.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/e2e/redoc.spec.js b/tests/e2e/redoc.spec.js index c767db9b..7db651ce 100644 --- a/tests/e2e/redoc.spec.js +++ b/tests/e2e/redoc.spec.js @@ -61,6 +61,7 @@ describe('APIs.guru specs test', ()=> { delete apisGuruList['googleapis.com:mirror']; // bad urls in images delete apisGuruList['googleapis.com:discovery']; // non-string references delete apisGuruList['clarify.io']; // non-string references + delete apisGuruList['pushpay.com']; // https://github.com/Rebilly/ReDoc/issues/30 // run quick version of e2e test on all builds except releases if (process.env.TRAVIS && !process.env.TRAVIS_TAG) {