From 70af0bf071daa47803c73562c0570bb2c4eed684 Mon Sep 17 00:00:00 2001 From: Brendan Abbott Date: Fri, 2 Jun 2017 16:55:51 +1000 Subject: [PATCH 1/6] Parser cleanups of unnecessary logic --- src/parser/open-api/schemaParser.js | 10 +- src/parser/open-api/v3/open-api-v3-parser.js | 175 +++++++++---------- 2 files changed, 83 insertions(+), 102 deletions(-) diff --git a/src/parser/open-api/schemaParser.js b/src/parser/open-api/schemaParser.js index 07a7c6a..1c15216 100644 --- a/src/parser/open-api/schemaParser.js +++ b/src/parser/open-api/schemaParser.js @@ -92,13 +92,11 @@ function getPropertiesNode (propertiesNode, requiredProperties = []) { const outputNode = [] for (const key in propertiesNode) { - if (propertiesNode.hasOwnProperty(key)) { - const property = propertiesNode[key] - const value = getPropertyNode(key, property, requiredProperties.includes(key)) + const property = propertiesNode[key] + const value = getPropertyNode(key, property, requiredProperties.includes(key)) - if (value) { - outputNode.push(value) - } + if (value) { + outputNode.push(value) } } diff --git a/src/parser/open-api/v3/open-api-v3-parser.js b/src/parser/open-api/v3/open-api-v3-parser.js index 0e64dcb..e9ba50c 100644 --- a/src/parser/open-api/v3/open-api-v3-parser.js +++ b/src/parser/open-api/v3/open-api-v3-parser.js @@ -16,52 +16,55 @@ function getUINavigationAndServices (tags, paths, pathSortFunction = sortByAlpha const navigation = [] const services = [] - tags.forEach((tag) => { + for (var i = 0; i < tags.length; i++) { + const tag = tags[i] const navigationMethods = [] const servicesMethods = [] - const pathIds = Object.keys(paths).sort(pathSortFunction) - pathIds.forEach(pathId => { + for (var j = 0; j < pathIds.length; j++) { + const pathId = pathIds[j] const path = paths[pathId] const methodTypes = Object.keys(path).sort(methodSortFunction) - methodTypes.forEach(methodType => { + for (var k = 0; k < methodTypes.length; k++) { + const methodType = methodTypes[k] const method = path[methodType] - const methodTags = method.tags - - if (methodTags.includes(tag)) { - const link = pathId + '/' + methodType - const navigationMethod = { - type: methodType, - title: method.summary, - link - } - navigationMethods.push(navigationMethod) - - const uiRequest = getUIRequest(method.description, method.requestBody) - const uiResponses = getUIResponses(method.responses) - const servicesMethod = { - type: methodType, - link, - summary: method.summary, - request: uiRequest, - responses: uiResponses - } - - if (method.description) { - servicesMethod.description = method.description - } - - const uiParameters = getUIParameters(method.parameters) - if (uiParameters) { - servicesMethod.parameters = uiParameters - } - - servicesMethods.push(servicesMethod) + + if (!method.tags.includes(tag)) { + continue + } + + const link = pathId + '/' + methodType + const navigationMethod = { + type: methodType, + title: method.summary, + link + } + navigationMethods.push(navigationMethod) + + const uiRequest = getUIRequest(method.description, method.requestBody) + const uiResponses = getUIResponses(method.responses) + const servicesMethod = { + type: methodType, + link, + summary: method.summary, + request: uiRequest, + responses: uiResponses } - }) - }) + + if (method.description) { + servicesMethod.description = method.description + } + + const uiParameters = getUIParameters(method.parameters) + if (uiParameters) { + servicesMethod.parameters = uiParameters + } + + servicesMethods.push(servicesMethod) + } + } navigation.push({ title: tag, @@ -72,7 +75,7 @@ function getUINavigationAndServices (tags, paths, pathSortFunction = sortByAlpha title: tag, methods: servicesMethods }) - }) + } return { navigation, services } } @@ -148,43 +151,30 @@ function getUIParametersForLocation (parameters, location) { return null } - const resultArray = parameters.map(parameter => { - if (parameter.in === location) { - try { - const uiParameter = { - name: parameter.name, - description: parameter.description, - required: parameter.required - } - - // TODO: We set the type to be an array because the Property component - // handles this. Property should eventually be split and this won't be - // necessary... - if (parameter.type) { - uiParameter.type = [ parameter.type ] - } else if (parameter.schema && parameter.schema.type) { - uiParameter.type = [ parameter.schema.type ] - } + const resultArray = parameters.filter(parameter => (parameter.in === location)).map(parameter => { + const uiParameter = { + name: parameter.name, + description: parameter.description, + required: parameter.required + } - if (parameter.schema && parameter.schema.default !== undefined) { - uiParameter.defaultValue = parameter.schema.default - } + // TODO: We set the type to be an array because the Property component + // handles this. Property should eventually be split and this won't be + // necessary... + if (parameter.type) { + uiParameter.type = [ parameter.type ] + } else if (parameter.schema && parameter.schema.type) { + uiParameter.type = [ parameter.schema.type ] + } - return uiParameter - } catch (error) { - console.log(error) - console.log('Context', { parameters, parameter, location }) - } + if (parameter.schema && parameter.schema.default !== undefined) { + uiParameter.defaultValue = parameter.schema.default } - return null - }).filter(parameter => parameter) + return uiParameter + }) - if (resultArray && resultArray.length > 0) { - return resultArray - } else { - return null - } + return resultArray.length ? resultArray : null } /** @@ -224,22 +214,20 @@ function getUIResponses (responses) { const uiResponses = [] for (const statusCode in responses) { - if (responses.hasOwnProperty(statusCode)) { - const response = responses[statusCode] - - const uiResponse = { - code: statusCode, - description: response.description - } + const response = responses[statusCode] - const mediaType = getMediaType(response.content) + const uiResponse = { + code: statusCode, + description: response.description + } - if (mediaType) { - addMediaTypeInfoToUIObject(uiResponse, mediaType) - } + const mediaType = getMediaType(response.content) - uiResponses.push(uiResponse) + if (mediaType) { + addMediaTypeInfoToUIObject(uiResponse, mediaType) } + + uiResponses.push(uiResponse) } return uiResponses @@ -281,21 +269,16 @@ function getTags (paths) { const tagCollection = [] for (const pathKey in paths) { - if (paths.hasOwnProperty(pathKey)) { - const path = paths[pathKey] - - for (const methodKey in path) { - if (path.hasOwnProperty(methodKey)) { - const method = path[methodKey] - const tags = method.tags - - tags.forEach(tag => { - if (!tagCollection.includes(tag)) { - tagCollection.push(tag) - } - }) + const path = paths[pathKey] + + for (const methodKey in path) { + const method = path[methodKey] + + method.tags.forEach(tag => { + if (!tagCollection.includes(tag)) { + tagCollection.push(tag) } - } + }) } } From 1d41e3bcd2c3dea939d76d516d2d165265d2427b Mon Sep 17 00:00:00 2001 From: Brendan Abbott Date: Fri, 2 Jun 2017 18:09:04 +1000 Subject: [PATCH 2/6] Add sample fixtures for oneOf, add getTestsFromFixture helper --- src/parser/open-api/oneOfResolver.js | 4 ++ test/fixtureLoader.js | 34 ++++++++++ .../data/oneOfResolver/inputs/double.json | 27 ++++++++ .../data/oneOfResolver/inputs/properties.json | 26 +++++++ .../data/oneOfResolver/inputs/required.json | 42 ++++++++++++ .../data/oneOfResolver/outputs/double.json | 56 +++++++++++++++ .../oneOfResolver/outputs/properties.json | 56 +++++++++++++++ .../data/oneOfResolver/outputs/required.json | 68 +++++++++++++++++++ test/parser/open-api/oneOfResolver.test.js | 14 ++++ .../open-api/v3/open-api-v3-parser.test.js | 39 +++-------- 10 files changed, 337 insertions(+), 29 deletions(-) create mode 100644 src/parser/open-api/oneOfResolver.js create mode 100644 test/fixtureLoader.js create mode 100644 test/parser/open-api/data/oneOfResolver/inputs/double.json create mode 100644 test/parser/open-api/data/oneOfResolver/inputs/properties.json create mode 100644 test/parser/open-api/data/oneOfResolver/inputs/required.json create mode 100644 test/parser/open-api/data/oneOfResolver/outputs/double.json create mode 100644 test/parser/open-api/data/oneOfResolver/outputs/properties.json create mode 100644 test/parser/open-api/data/oneOfResolver/outputs/required.json create mode 100644 test/parser/open-api/oneOfResolver.test.js diff --git a/src/parser/open-api/oneOfResolver.js b/src/parser/open-api/oneOfResolver.js new file mode 100644 index 0000000..1721e32 --- /dev/null +++ b/src/parser/open-api/oneOfResolver.js @@ -0,0 +1,4 @@ + +export function resolveOneOf (obj) { + return obj +} diff --git a/test/fixtureLoader.js b/test/fixtureLoader.js new file mode 100644 index 0000000..148097a --- /dev/null +++ b/test/fixtureLoader.js @@ -0,0 +1,34 @@ +import { readdirSync, readJSONSync } from 'fs-extra' + +/** + * Given the path to the fixture data, and then the expected results, + * this function returns an array of information that can be loaded + * into tests. + * + * @param {string} inputsDir + * @param {string} expectationsDir + * @return {array} + */ +export function getTestsFromFixtures (inputsDir, expectationsDir) { + const dataFiles = readdirSync(inputsDir) + const tests = [] + + dataFiles.map(fileName => { + try { + const inputData = readJSONSync(`${inputsDir}/${fileName}`) + const outputData = readJSONSync(`${expectationsDir}/${fileName}`) + + if (inputData && outputData) { + tests.push({ + fileName, + input: inputData, + expected: outputData + }) + } + } catch (error) { + console.log(error) + } + }) + + return tests +} diff --git a/test/parser/open-api/data/oneOfResolver/inputs/double.json b/test/parser/open-api/data/oneOfResolver/inputs/double.json new file mode 100644 index 0000000..98b685d --- /dev/null +++ b/test/parser/open-api/data/oneOfResolver/inputs/double.json @@ -0,0 +1,27 @@ +{ + "type": "object", + "properties": { + "foo": { + "title": "Fooo", + "oneOf": [ + { + "type": "string" + }, + { + "type": "integer" + } + ] + }, + "bar": { + "title": "Bar" + } + }, + "oneOf": [ + { + "required": ["foo"] + }, + { + "required": ["bar"] + } + ] +} diff --git a/test/parser/open-api/data/oneOfResolver/inputs/properties.json b/test/parser/open-api/data/oneOfResolver/inputs/properties.json new file mode 100644 index 0000000..fa9c776 --- /dev/null +++ b/test/parser/open-api/data/oneOfResolver/inputs/properties.json @@ -0,0 +1,26 @@ +{ + "type": "object", + "properties": { + "customAttributes": { + "description": "A generic key-value object for allowing custom attributes in a schema. Eg. { \"customKey\": \"value\" }.", + "type": "object", + "maxProperties": 5, + "additionalProperties": { + "oneOf": [ + { + "type": "number" + }, + { + "type": "integer" + }, + { + "type": "string" + }, + { + "type": "boolean" + } + ] + } + } + } +} diff --git a/test/parser/open-api/data/oneOfResolver/inputs/required.json b/test/parser/open-api/data/oneOfResolver/inputs/required.json new file mode 100644 index 0000000..0c33060 --- /dev/null +++ b/test/parser/open-api/data/oneOfResolver/inputs/required.json @@ -0,0 +1,42 @@ +{ + "type": "object", + "additionalProperties": false, + "properties": { + "bearerToken": { + "description": "A `bearerToken` from `account` creation.", + "type": "string" + }, + "accountId": { + "description": "An `accountId` from `account` creation.", + "pattern": "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$" + }, + "email": { + "description": "A user's email address.", + "type": "string", + "format": "email" + }, + "password": { + "description": "A user's password, in plaintext.", + "type": "string", + "format": "password" + }, + "scope": { + "description": "A comma delimited list of scope names. If unknown, use `client`.", + "type": "string" + } + }, + "oneOf": [ + { + "required": [ + "bearerToken", + "accountId" + ] + }, + { + "required": [ + "email", + "password" + ] + } + ] +} diff --git a/test/parser/open-api/data/oneOfResolver/outputs/double.json b/test/parser/open-api/data/oneOfResolver/outputs/double.json new file mode 100644 index 0000000..2f33a0e --- /dev/null +++ b/test/parser/open-api/data/oneOfResolver/outputs/double.json @@ -0,0 +1,56 @@ +{ + "oneOf": [ + { + "type": "object", + "properties": { + "foo": { + "title": "Fooo", + "type": "string" + }, + "bar": { + "title": "Bar" + } + }, + "required": ["foo"] + }, + { + "type": "object", + "properties": { + "foo": { + "title": "Fooo", + "type": "integer" + }, + "bar": { + "title": "Bar" + } + }, + "required": ["foo"] + }, + { + "type": "object", + "properties": { + "foo": { + "title": "Fooo", + "type": "string" + }, + "bar": { + "title": "Bar" + } + }, + "required": ["bar"] + }, + { + "type": "object", + "properties": { + "foo": { + "title": "Fooo", + "type": "integer" + }, + "bar": { + "title": "Bar" + } + }, + "required": ["bar"] + } + ] +} diff --git a/test/parser/open-api/data/oneOfResolver/outputs/properties.json b/test/parser/open-api/data/oneOfResolver/outputs/properties.json new file mode 100644 index 0000000..ee9b50f --- /dev/null +++ b/test/parser/open-api/data/oneOfResolver/outputs/properties.json @@ -0,0 +1,56 @@ +{ + "oneOf": [ + { + "type": "object", + "properties": { + "customAttributes": { + "description": "A generic key-value object for allowing custom attributes in a schema. Eg. { \"customKey\": \"value\" }.", + "type": "object", + "maxProperties": 5, + "additionalProperties": { + "type": "number" + } + } + } + }, + { + "type": "object", + "properties": { + "customAttributes": { + "description": "A generic key-value object for allowing custom attributes in a schema. Eg. { \"customKey\": \"value\" }.", + "type": "object", + "maxProperties": 5, + "additionalProperties": { + "type": "integer" + } + } + } + }, + { + "type": "object", + "properties": { + "customAttributes": { + "description": "A generic key-value object for allowing custom attributes in a schema. Eg. { \"customKey\": \"value\" }.", + "type": "object", + "maxProperties": 5, + "additionalProperties": { + "type": "string" + } + } + } + }, + { + "type": "object", + "properties": { + "customAttributes": { + "description": "A generic key-value object for allowing custom attributes in a schema. Eg. { \"customKey\": \"value\" }.", + "type": "object", + "maxProperties": 5, + "additionalProperties": { + "type": "boolean" + } + } + } + } + ] +} diff --git a/test/parser/open-api/data/oneOfResolver/outputs/required.json b/test/parser/open-api/data/oneOfResolver/outputs/required.json new file mode 100644 index 0000000..ce0b078 --- /dev/null +++ b/test/parser/open-api/data/oneOfResolver/outputs/required.json @@ -0,0 +1,68 @@ +{ + "oneOf": [ + { + "type": "object", + "additionalProperties": false, + "properties": { + "bearerToken": { + "description": "A `bearerToken` from `account` creation.", + "type": "string" + }, + "accountId": { + "description": "An `accountId` from `account` creation.", + "pattern": "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$" + }, + "email": { + "description": "A user's email address.", + "type": "string", + "format": "email" + }, + "password": { + "description": "A user's password, in plaintext.", + "type": "string", + "format": "password" + }, + "scope": { + "description": "A comma delimited list of scope names. If unknown, use `client`.", + "type": "string" + } + }, + "required": [ + "bearerToken", + "accountId" + ] + }, + { + "type": "object", + "additionalProperties": false, + "properties": { + "bearerToken": { + "description": "A `bearerToken` from `account` creation.", + "type": "string" + }, + "accountId": { + "description": "An `accountId` from `account` creation.", + "pattern": "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$" + }, + "email": { + "description": "A user's email address.", + "type": "string", + "format": "email" + }, + "password": { + "description": "A user's password, in plaintext.", + "type": "string", + "format": "password" + }, + "scope": { + "description": "A comma delimited list of scope names. If unknown, use `client`.", + "type": "string" + } + }, + "required": [ + "email", + "password" + ] + } + ] +} diff --git a/test/parser/open-api/oneOfResolver.test.js b/test/parser/open-api/oneOfResolver.test.js new file mode 100644 index 0000000..d5127b1 --- /dev/null +++ b/test/parser/open-api/oneOfResolver.test.js @@ -0,0 +1,14 @@ +import { getTestsFromFixtures } from '../../fixtureLoader' +import { resolveOneOf } from '../../../src/parser/open-api/oneOfResolver' + +describe('resolveOneOf', () => { + const dataDirectory = __dirname + '/data/oneOfResolver' + const tests = getTestsFromFixtures(`${dataDirectory}/inputs`, `${dataDirectory}/outputs`) + + tests.forEach(test => { + it(`returns the correct result for ${test.fileName}`, async () => { + const outputDefinition = resolveOneOf(test.input) + expect(outputDefinition).toEqual(test.expected) + }) + }) +}) diff --git a/test/parser/open-api/v3/open-api-v3-parser.test.js b/test/parser/open-api/v3/open-api-v3-parser.test.js index f6ebad0..3ceab84 100644 --- a/test/parser/open-api/v3/open-api-v3-parser.test.js +++ b/test/parser/open-api/v3/open-api-v3-parser.test.js @@ -1,33 +1,14 @@ -import { readdirSync, readJSONSync } from 'fs-extra'; -import getUIReadyDefinition from '../../../../src/parser/open-api/v3/open-api-v3-parser'; +import { getTestsFromFixtures } from '../../../fixtureLoader' +import getUIReadyDefinition from '../../../../src/parser/open-api/v3/open-api-v3-parser' describe('getUIReadyDefinition', () => { - const dataDirectory = __dirname + '/data'; - const dataFiles = readdirSync(dataDirectory + '/inputs'); - - const tests = []; - - dataFiles.map(dataFile => { - try { - const inputData = readJSONSync(dataDirectory + '/inputs/' + dataFile); - const outputData = readJSONSync(dataDirectory + '/outputs/' + dataFile); - - if (inputData && outputData) { - tests.push({ - dataFile: dataFile, - inputDefinition: inputData, - expectedOutputDefinition: outputData - }); - } - } catch (error) { - console.log(error); - } - }); + const dataDirectory = __dirname + '/data' + const tests = getTestsFromFixtures(`${dataDirectory}/inputs`, `${dataDirectory}/outputs`) tests.forEach(test => { - it(`returns the correct result for ${test.dataFile}`, async () => { - const outputDefinition = await getUIReadyDefinition(test.inputDefinition); - expect(outputDefinition).toEqual(test.expectedOutputDefinition); - }); - }); -}); + it(`returns the correct result for ${test.fileName}`, async () => { + const outputDefinition = await getUIReadyDefinition(test.input) + expect(outputDefinition).toEqual(test.expected) + }) + }) +}) From e975c07f6af9c2ba86876f41cdc07b507292c89b Mon Sep 17 00:00:00 2001 From: Brendan Abbott Date: Mon, 5 Jun 2017 11:40:35 +1000 Subject: [PATCH 3/6] Swap all tests to use the new fixture loader --- test/parser/open-api/allOfResolver.test.js | 20 ++++++++------ .../{input.json => inputs/simple.json} | 0 .../{output.json => outputs/simple.json} | 0 .../complex.json} | 0 .../{inputSchema.json => inputs/simple.json} | 0 .../complex.json} | 0 .../simple.json} | 0 test/parser/open-api/schemaParser.test.js | 26 ++++++++----------- 8 files changed, 23 insertions(+), 23 deletions(-) rename test/parser/open-api/data/allOfResolver/{input.json => inputs/simple.json} (100%) rename test/parser/open-api/data/allOfResolver/{output.json => outputs/simple.json} (100%) rename test/parser/open-api/data/schemaParser/{complexInputSchema.json => inputs/complex.json} (100%) rename test/parser/open-api/data/schemaParser/{inputSchema.json => inputs/simple.json} (100%) rename test/parser/open-api/data/schemaParser/{complexOutputSchema.json => outputs/complex.json} (100%) rename test/parser/open-api/data/schemaParser/{outputSchema.json => outputs/simple.json} (100%) diff --git a/test/parser/open-api/allOfResolver.test.js b/test/parser/open-api/allOfResolver.test.js index 03db912..4544d9b 100644 --- a/test/parser/open-api/allOfResolver.test.js +++ b/test/parser/open-api/allOfResolver.test.js @@ -1,10 +1,14 @@ -import input from './data/allOfResolver/input.json'; -import expectedOutput from './data/allOfResolver/output.json'; -import { resolveAllOf } from '../../../src/parser/open-api/allOfResolver'; +import { getTestsFromFixtures } from '../../fixtureLoader' +import { resolveAllOf } from '../../../src/parser/open-api/allOfResolver' describe('resolveAllOf', () => { - it('returns the correct result', () => { - const output = resolveAllOf(input); - expect(output).toEqual(expectedOutput); - }); -}); + const dataDirectory = __dirname + '/data/allOfResolver' + const tests = getTestsFromFixtures(`${dataDirectory}/inputs`, `${dataDirectory}/outputs`) + + tests.forEach(test => { + it(`returns the correct result for ${test.fileName}`, async () => { + const outputDefinition = resolveAllOf(test.input) + expect(outputDefinition).toEqual(test.expected) + }) + }) +}) diff --git a/test/parser/open-api/data/allOfResolver/input.json b/test/parser/open-api/data/allOfResolver/inputs/simple.json similarity index 100% rename from test/parser/open-api/data/allOfResolver/input.json rename to test/parser/open-api/data/allOfResolver/inputs/simple.json diff --git a/test/parser/open-api/data/allOfResolver/output.json b/test/parser/open-api/data/allOfResolver/outputs/simple.json similarity index 100% rename from test/parser/open-api/data/allOfResolver/output.json rename to test/parser/open-api/data/allOfResolver/outputs/simple.json diff --git a/test/parser/open-api/data/schemaParser/complexInputSchema.json b/test/parser/open-api/data/schemaParser/inputs/complex.json similarity index 100% rename from test/parser/open-api/data/schemaParser/complexInputSchema.json rename to test/parser/open-api/data/schemaParser/inputs/complex.json diff --git a/test/parser/open-api/data/schemaParser/inputSchema.json b/test/parser/open-api/data/schemaParser/inputs/simple.json similarity index 100% rename from test/parser/open-api/data/schemaParser/inputSchema.json rename to test/parser/open-api/data/schemaParser/inputs/simple.json diff --git a/test/parser/open-api/data/schemaParser/complexOutputSchema.json b/test/parser/open-api/data/schemaParser/outputs/complex.json similarity index 100% rename from test/parser/open-api/data/schemaParser/complexOutputSchema.json rename to test/parser/open-api/data/schemaParser/outputs/complex.json diff --git a/test/parser/open-api/data/schemaParser/outputSchema.json b/test/parser/open-api/data/schemaParser/outputs/simple.json similarity index 100% rename from test/parser/open-api/data/schemaParser/outputSchema.json rename to test/parser/open-api/data/schemaParser/outputs/simple.json diff --git a/test/parser/open-api/schemaParser.test.js b/test/parser/open-api/schemaParser.test.js index 96e91dc..213856d 100644 --- a/test/parser/open-api/schemaParser.test.js +++ b/test/parser/open-api/schemaParser.test.js @@ -1,18 +1,14 @@ -import getUIReadySchema from '../../../src/parser/open-api/schemaParser'; - -import inputSchema from './data/schemaParser/inputSchema.json'; -import expectedOutputSchema from './data/schemaParser/outputSchema.json'; -import complexInputSchema from './data/schemaParser/complexInputSchema.json'; -import complexExpectedOutputSchema from './data/schemaParser/complexOutputSchema.json'; +import { getTestsFromFixtures } from '../../fixtureLoader' +import getUIReadySchema from '../../../src/parser/open-api/schemaParser' describe('getUIReadySchema', () => { - it('returns the correct result', () => { - const outputSchema = getUIReadySchema(inputSchema); - expect(outputSchema).toEqual(expectedOutputSchema); - }); + const dataDirectory = __dirname + '/data/schemaParser' + const tests = getTestsFromFixtures(`${dataDirectory}/inputs`, `${dataDirectory}/outputs`) - it('returns the correct result', () => { - const outputSchema = getUIReadySchema(complexInputSchema); - expect(outputSchema).toEqual(complexExpectedOutputSchema); - }); -}); + tests.forEach(test => { + it(`returns the correct result for ${test.fileName}`, async () => { + const outputDefinition = getUIReadySchema(test.input) + expect(outputDefinition).toEqual(test.expected) + }) + }) +}) From 275e310bf930d6335174b17c3b57ea5e0492726e Mon Sep 17 00:00:00 2001 From: Brendan Abbott Date: Mon, 5 Jun 2017 18:24:59 +1000 Subject: [PATCH 4/6] Implement oneOfResolver --- src/parser/open-api/oneOfResolver.js | 134 +++++++++++++++++- src/parser/open-api/schemaParser.js | 13 +- .../data/oneOfResolver/outputs/double.json | 8 +- .../data/schemaParser/inputs/complex.json | 6 + 4 files changed, 153 insertions(+), 8 deletions(-) diff --git a/src/parser/open-api/oneOfResolver.js b/src/parser/open-api/oneOfResolver.js index 1721e32..53e6799 100644 --- a/src/parser/open-api/oneOfResolver.js +++ b/src/parser/open-api/oneOfResolver.js @@ -1,4 +1,136 @@ +import cloneDeep from 'lodash/cloneDeep' +import get from 'lodash/get' +import update from 'lodash/update' +import toPath from 'lodash/toPath' + +/** + * Recursively determine if the given object has any `oneOf` properties. + * Returns an array with the paths to those properties (eg. `properties.oneOf`). + * If no `oneOf` keys were found, an empty array is returned. + * + * @param {object} obj + * @return {array} + */ +function getOneOfPaths (obj) { + let paths = [] + let walk = function (obj, path = '') { + for (var k in obj) { + // Handle if `oneOf` is found at the first level. + const currentPath = (path === '') ? k : `${path}.${k}` + + if (k === 'oneOf') { + paths.push(currentPath) + } else if (typeof obj[k] === 'object' || Array.isArray(obj[k])) { + walk(obj[k], currentPath) + } + } + } + + walk(obj) + + return paths +} + +/** + * Returns all of the possible states of `obj` when the permutations + * specified in `paths` are applied. + * + * @param {string[]} paths + * @param {object} obj + * @return {array} + */ +function getStates (paths, obj) { + let states = [ ...getStateAt(paths[0], obj) ] + + for (var i = 1; i < paths.length; i++) { + let state = states.shift() + + while (state) { + const newStates = getStateAt(paths[i], state) + + // if there are no new states, put this one back and stop + if (!newStates.length) { + states.push(state) + break + } + + states = states.concat(...newStates) + state = states.shift() + } + } + + return states +} + +/** + * Given a path and an object, return all the new states that exist + * by merging the options found at `obj.path` into the given object. + * Returns an array of possible objects + * + * @param {string} path + * @param {object} obj + * @return {array} + */ +function getStateAt (path, obj) { + const clonedObj = cloneDeep(obj) + const states = get(clonedObj, path) + + // Couldn't retrieve the state at this path, bail. + if (states === undefined) { + return [] + } + + const parentPath = getParentPath(path) + return states.map((state) => { + // No path, so add state to object directly + if (parentPath === '') { + return permutate(clonedObj, state) + } + + // Replace the path with the state + update(clonedObj, parentPath, (value) => permutate(value, state)) + + return cloneDeep(clonedObj) + }) +} + +/** + * Given a path, drop the last segment and return the shortened + * path, which will be the parent of the given path. + * + * @param {string} path + * @return {string} + */ +function getParentPath (path) { + let rootPath = toPath(path) + rootPath.pop() + + return rootPath.join('.') +} + +/** + * Take a object and applies the state to it, returning + * the resulting object + * + * @param {object} obj + * @param {object} state + * @return {object} + */ +function permutate (obj, state) { + delete obj.oneOf + + return { ...obj, ...state } +} export function resolveOneOf (obj) { - return obj + const paths = getOneOfPaths(obj) + if (paths.length === 0) { + return obj + } + + const states = getStates(paths, obj) + + return { + oneOf: states + } } diff --git a/src/parser/open-api/schemaParser.js b/src/parser/open-api/schemaParser.js index 1c15216..5527d84 100644 --- a/src/parser/open-api/schemaParser.js +++ b/src/parser/open-api/schemaParser.js @@ -1,4 +1,5 @@ import { resolveAllOf } from './allOfResolver' +import { resolveOneOf } from './oneOfResolver' import { hasConstraints, getConstraints } from './constraints/constraintsParser' const literalTypes = ['string', 'integer', 'number', 'boolean'] @@ -111,8 +112,14 @@ function getPropertiesNode (propertiesNode, requiredProperties = []) { * @return {Object} */ export default function getUIReadySchema (jsonSchema) { - const resolvedJsonSchema = resolveAllOf(jsonSchema) - const outputSchema = getPropertiesNode(resolvedJsonSchema.properties, resolvedJsonSchema.required) + let resolved = resolveAllOf(jsonSchema) + resolved = resolveOneOf(jsonSchema) - return outputSchema + if (Array.isArray(resolved.oneOf)) { + return resolved.oneOf.map( + (state) => getPropertiesNode(state.properties, state.required) + ) + } + + return getPropertiesNode(resolved.properties, resolved.required) } diff --git a/test/parser/open-api/data/oneOfResolver/outputs/double.json b/test/parser/open-api/data/oneOfResolver/outputs/double.json index 2f33a0e..bd6ac79 100644 --- a/test/parser/open-api/data/oneOfResolver/outputs/double.json +++ b/test/parser/open-api/data/oneOfResolver/outputs/double.json @@ -11,7 +11,7 @@ "title": "Bar" } }, - "required": ["foo"] + "required": ["bar"] }, { "type": "object", @@ -31,7 +31,7 @@ "properties": { "foo": { "title": "Fooo", - "type": "string" + "type": "integer" }, "bar": { "title": "Bar" @@ -44,13 +44,13 @@ "properties": { "foo": { "title": "Fooo", - "type": "integer" + "type": "string" }, "bar": { "title": "Bar" } }, - "required": ["bar"] + "required": ["foo"] } ] } diff --git a/test/parser/open-api/data/schemaParser/inputs/complex.json b/test/parser/open-api/data/schemaParser/inputs/complex.json index 88e6362..a371fdc 100644 --- a/test/parser/open-api/data/schemaParser/inputs/complex.json +++ b/test/parser/open-api/data/schemaParser/inputs/complex.json @@ -4,6 +4,12 @@ "name": { "type": "string" }, + "isRequired": { + "oneOf": [ + { "type": "boolean" }, + { "type": "integer" } + ] + }, "customAttributes": { "additionalProperties": false, "type": "array", From 5b84bc36a5a255c2645bcee584d2eae108baadf3 Mon Sep 17 00:00:00 2001 From: Brendan Abbott Date: Mon, 5 Jun 2017 18:58:15 +1000 Subject: [PATCH 5/6] Fix schemaParser test, update docs --- src/parser/open-api/oneOfResolver.js | 25 ++-- src/parser/open-api/v3/open-api-v3-parser.js | 6 +- .../data/schemaParser/outputs/complex.json | 124 +++++++++++++----- 3 files changed, 109 insertions(+), 46 deletions(-) diff --git a/src/parser/open-api/oneOfResolver.js b/src/parser/open-api/oneOfResolver.js index 53e6799..a1055d1 100644 --- a/src/parser/open-api/oneOfResolver.js +++ b/src/parser/open-api/oneOfResolver.js @@ -14,7 +14,7 @@ import toPath from 'lodash/toPath' function getOneOfPaths (obj) { let paths = [] let walk = function (obj, path = '') { - for (var k in obj) { + for (let k in obj) { // Handle if `oneOf` is found at the first level. const currentPath = (path === '') ? k : `${path}.${k}` @@ -42,7 +42,7 @@ function getOneOfPaths (obj) { function getStates (paths, obj) { let states = [ ...getStateAt(paths[0], obj) ] - for (var i = 1; i < paths.length; i++) { + for (let i = 1; i < paths.length; i++) { let state = states.shift() while (state) { @@ -75,7 +75,7 @@ function getStateAt (path, obj) { const clonedObj = cloneDeep(obj) const states = get(clonedObj, path) - // Couldn't retrieve the state at this path, bail. + // Couldn't retrieve the states at this path, bail. if (states === undefined) { return [] } @@ -84,11 +84,11 @@ function getStateAt (path, obj) { return states.map((state) => { // No path, so add state to object directly if (parentPath === '') { - return permutate(clonedObj, state) + return merge(clonedObj, state) } // Replace the path with the state - update(clonedObj, parentPath, (value) => permutate(value, state)) + update(clonedObj, parentPath, (value) => merge(value, state)) return cloneDeep(clonedObj) }) @@ -109,19 +109,28 @@ function getParentPath (path) { } /** - * Take a object and applies the state to it, returning - * the resulting object + * Take a object and merge the state to it, returning the resulting object. + * Will remove the `oneOf` key from the given `obj`. * * @param {object} obj * @param {object} state * @return {object} */ -function permutate (obj, state) { +function merge (obj, state) { delete obj.oneOf return { ...obj, ...state } } +/** + * Resolves all `oneOf` definitions present in the given object. + * If none exist, the object is returned untouched. Otherwise + * an object with a `oneOf` property whose value is an array of + * states is returned + * + * @param {object} obj + * @return {object} + */ export function resolveOneOf (obj) { const paths = getOneOfPaths(obj) if (paths.length === 0) { diff --git a/src/parser/open-api/v3/open-api-v3-parser.js b/src/parser/open-api/v3/open-api-v3-parser.js index e9ba50c..4903580 100644 --- a/src/parser/open-api/v3/open-api-v3-parser.js +++ b/src/parser/open-api/v3/open-api-v3-parser.js @@ -16,18 +16,18 @@ function getUINavigationAndServices (tags, paths, pathSortFunction = sortByAlpha const navigation = [] const services = [] - for (var i = 0; i < tags.length; i++) { + for (let i = 0; i < tags.length; i++) { const tag = tags[i] const navigationMethods = [] const servicesMethods = [] const pathIds = Object.keys(paths).sort(pathSortFunction) - for (var j = 0; j < pathIds.length; j++) { + for (let j = 0; j < pathIds.length; j++) { const pathId = pathIds[j] const path = paths[pathId] const methodTypes = Object.keys(path).sort(methodSortFunction) - for (var k = 0; k < methodTypes.length; k++) { + for (let k = 0; k < methodTypes.length; k++) { const methodType = methodTypes[k] const method = path[methodType] diff --git a/test/parser/open-api/data/schemaParser/outputs/complex.json b/test/parser/open-api/data/schemaParser/outputs/complex.json index 9eaaeb0..3b829ba 100644 --- a/test/parser/open-api/data/schemaParser/outputs/complex.json +++ b/test/parser/open-api/data/schemaParser/outputs/complex.json @@ -1,38 +1,92 @@ [ - { - "name": "name", - "required": false, - "type": [ - "string" - ] - }, - { - "name": "customAttributes", - "required": false, - "type": [ - "array" - ], - "properties": [ - { - "name": "key", - "required": true, - "type": [ - "string" - ], - "constraints": { - "pattern": "^[a-zA-Z0-9_-]+$" + [ + { + "name": "name", + "required": false, + "type": [ + "string" + ] + }, + { + "name": "isRequired", + "required": false, + "type": [ + "boolean" + ] + }, + { + "name": "customAttributes", + "properties": [ + { + "constraints": { + "pattern": "^[a-zA-Z0-9_-]+$" + }, + "name": "key", + "required": true, + "type": [ + "string" + ] + }, + { + "name": "value", + "required": true, + "type": [ + "number", + "integer", + "string", + "boolean" + ] } - }, - { - "name": "value", - "required": true, - "type": [ - "number", - "integer", - "string", - "boolean" - ] - } - ] - } + ], + "required": false, + "type": [ + "array" + ] + } + ], + [ + { + "name": "name", + "required": false, + "type": [ + "string" + ] + }, + { + "name": "isRequired", + "required": false, + "type": [ + "integer" + ] + }, + { + "name": "customAttributes", + "properties": [ + { + "constraints": { + "pattern": "^[a-zA-Z0-9_-]+$" + }, + "name": "key", + "required": true, + "type": [ + "string" + ] + }, + { + "name": "value", + "required": true, + "type": [ + "number", + "integer", + "string", + "boolean" + ] + } + ], + "required": false, + "type": [ + "array" + ] + } + ] ] From 3c14c707645503339ccbad486851ea213e5d4e11 Mon Sep 17 00:00:00 2001 From: Brendan Abbott Date: Tue, 6 Jun 2017 13:25:23 +1000 Subject: [PATCH 6/6] PR feedback --- src/parser/open-api/oneOfResolver.js | 18 +++++++++--------- src/parser/open-api/schemaParser.js | 2 +- .../data/oneOfResolver/outputs/double.json | 12 ++++++------ 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/parser/open-api/oneOfResolver.js b/src/parser/open-api/oneOfResolver.js index a1055d1..b4b61f1 100644 --- a/src/parser/open-api/oneOfResolver.js +++ b/src/parser/open-api/oneOfResolver.js @@ -9,19 +9,19 @@ import toPath from 'lodash/toPath' * If no `oneOf` keys were found, an empty array is returned. * * @param {object} obj - * @return {array} + * @return {string[]} */ function getOneOfPaths (obj) { let paths = [] let walk = function (obj, path = '') { - for (let k in obj) { + for (let key in obj) { // Handle if `oneOf` is found at the first level. - const currentPath = (path === '') ? k : `${path}.${k}` + const currentPath = (path === '') ? key : `${path}.${key}` - if (k === 'oneOf') { + if (key === 'oneOf') { paths.push(currentPath) - } else if (typeof obj[k] === 'object' || Array.isArray(obj[k])) { - walk(obj[k], currentPath) + } else if (typeof obj[key] === 'object' || Array.isArray(obj[key])) { + walk(obj[key], currentPath) } } } @@ -37,7 +37,7 @@ function getOneOfPaths (obj) { * * @param {string[]} paths * @param {object} obj - * @return {array} + * @return {object[]} */ function getStates (paths, obj) { let states = [ ...getStateAt(paths[0], obj) ] @@ -50,7 +50,7 @@ function getStates (paths, obj) { // if there are no new states, put this one back and stop if (!newStates.length) { - states.push(state) + states.unshift(state) break } @@ -69,7 +69,7 @@ function getStates (paths, obj) { * * @param {string} path * @param {object} obj - * @return {array} + * @return {object[]} */ function getStateAt (path, obj) { const clonedObj = cloneDeep(obj) diff --git a/src/parser/open-api/schemaParser.js b/src/parser/open-api/schemaParser.js index 5527d84..79f9234 100644 --- a/src/parser/open-api/schemaParser.js +++ b/src/parser/open-api/schemaParser.js @@ -113,7 +113,7 @@ function getPropertiesNode (propertiesNode, requiredProperties = []) { */ export default function getUIReadySchema (jsonSchema) { let resolved = resolveAllOf(jsonSchema) - resolved = resolveOneOf(jsonSchema) + resolved = resolveOneOf(resolved) if (Array.isArray(resolved.oneOf)) { return resolved.oneOf.map( diff --git a/test/parser/open-api/data/oneOfResolver/outputs/double.json b/test/parser/open-api/data/oneOfResolver/outputs/double.json index bd6ac79..e7a7fd2 100644 --- a/test/parser/open-api/data/oneOfResolver/outputs/double.json +++ b/test/parser/open-api/data/oneOfResolver/outputs/double.json @@ -11,20 +11,20 @@ "title": "Bar" } }, - "required": ["bar"] + "required": ["foo"] }, { "type": "object", "properties": { "foo": { "title": "Fooo", - "type": "integer" + "type": "string" }, "bar": { "title": "Bar" } }, - "required": ["foo"] + "required": ["bar"] }, { "type": "object", @@ -37,20 +37,20 @@ "title": "Bar" } }, - "required": ["bar"] + "required": ["foo"] }, { "type": "object", "properties": { "foo": { "title": "Fooo", - "type": "string" + "type": "integer" }, "bar": { "title": "Bar" } }, - "required": ["foo"] + "required": ["bar"] } ] }