From a229fc000d5e3e135a650c692a90b021e20031ed Mon Sep 17 00:00:00 2001 From: Dustin Popp Date: Fri, 15 Feb 2019 16:24:27 -0600 Subject: [PATCH 1/2] feat: flag operations with non-form request bodies that do not specify a name --- README.md | 2 + src/.defaultsForValidator.js | 3 +- .../oas3/semantic-validators/operations.js | 36 ++++++- test/plugins/validation/oas3/operations.js | 99 +++++++++++++++++-- 4 files changed, 131 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 348e3edaa..c5e70775e 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,7 @@ The supported rules are described below: | no_array_responses | Flag any operations with a top-level array response. | shared | | parameter_order | Flag any operations with optional parameters before a required param. | shared | | no_request_body_content | [Flag any operations with a `requestBody` that does not have a `content` field.][3] | oas3 | +| no_request_body_name | Flag any operations with a non-form `requestBody` that does not have a name set with `x-codegen-request-body-name`. | oas3 | ##### parameters | Rule | Description | Spec | @@ -307,6 +308,7 @@ The default values for each rule are described below. | Rule | Default | | --------------------------- | --------| | no_request_body_content | error | +| no_request_body_name | error | ###### parameters | Rule | Default | diff --git a/src/.defaultsForValidator.js b/src/.defaultsForValidator.js index 82e2e008c..e232d715b 100644 --- a/src/.defaultsForValidator.js +++ b/src/.defaultsForValidator.js @@ -73,7 +73,8 @@ const defaults = { }, 'oas3': { 'operations': { - 'no_request_body_content': 'error' + 'no_request_body_content': 'error', + 'no_request_body_name': 'warning' }, 'parameters': { 'no_in_property': 'error', diff --git a/src/plugins/validation/oas3/semantic-validators/operations.js b/src/plugins/validation/oas3/semantic-validators/operations.js index 0d9e59c5d..4a56bdbf9 100644 --- a/src/plugins/validation/oas3/semantic-validators/operations.js +++ b/src/plugins/validation/oas3/semantic-validators/operations.js @@ -1,6 +1,9 @@ // Assertation 1. Request body objects must have a `content` property // https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#requestBodyObject +// Assertation 2. Operations with non-form request bodies should set the `x-codegen-request-body-name` +// annotation (for code generation purposes) + const pick = require('lodash/pick'); const each = require('lodash/each'); @@ -11,6 +14,8 @@ module.exports.validate = function({ resolvedSpec }, config) { config = config.operations; + const REQUEST_BODY_NAME = 'x-codegen-request-body-name'; + // get, head, and delete are not in this list because they are not allowed // to have request bodies const allowedOps = ['post', 'put', 'patch', 'options', 'trace']; @@ -24,14 +29,32 @@ module.exports.validate = function({ resolvedSpec }, config) { // Assertation 1 if (op.requestBody) { const requestBodyContent = op.requestBody.content; - if (!requestBodyContent || !Object.keys(requestBodyContent).length) { + const requestBodyMimeTypes = + op.requestBody.content && Object.keys(requestBodyContent); + if (!requestBodyContent || !requestBodyMimeTypes.length) { const checkStatus = config.no_request_body_content; if (checkStatus !== 'off') { - result.error.push({ + result[checkStatus].push({ path: `paths.${pathName}.${opName}.requestBody`, message: 'Request bodies MUST specify a `content` property' }); } + } else { + // request body has content + const firstMimeType = requestBodyMimeTypes[0]; // code generation uses the first mime type + const hasRequestBodyName = + op[REQUEST_BODY_NAME] && op[REQUEST_BODY_NAME].trim().length; + if (isBodyParameter(firstMimeType) && !hasRequestBodyName) { + const checkStatus = config.no_request_body_name; + if (checkStatus != 'off') { + const message = + 'Operations with non-form request bodies should set a name with the x-codegen-request-body-name annotation.'; + result[checkStatus].push({ + path: `paths.${pathName}.${opName}`, + message + }); + } + } } } }); @@ -39,3 +62,12 @@ module.exports.validate = function({ resolvedSpec }, config) { return { errors: result.error, warnings: result.warning }; }; + +function isBodyParameter(mimeType) { + const formDataMimeTypes = [ + 'multipart/form-data', + 'application/x-www-form-urlencoded', + 'application/octet-stream' + ]; + return !formDataMimeTypes.includes(mimeType); +} diff --git a/test/plugins/validation/oas3/operations.js b/test/plugins/validation/oas3/operations.js index 0a5710e3b..47aaaf6d0 100644 --- a/test/plugins/validation/oas3/operations.js +++ b/test/plugins/validation/oas3/operations.js @@ -2,15 +2,10 @@ const expect = require('expect'); const { validate } = require('../../../../src/plugins/validation/oas3/semantic-validators/operations'); +const config = require('../../../../src/.defaultsForValidator').defaults.oas3; describe('validation plugin - semantic - operations - oas3', function() { it('should complain about a request body not having a content field', function() { - const config = { - operations: { - no_request_body_content: 'error' - } - }; - const spec = { paths: { '/pets': { @@ -33,4 +28,96 @@ describe('validation plugin - semantic - operations - oas3', function() { ); expect(res.warnings.length).toEqual(0); }); + + it('should warn about an operation with a non-form request body that does not set a name', function() { + const spec = { + paths: { + '/pets': { + post: { + summary: 'this is a summary', + operationId: 'operationId', + requestBody: { + description: 'body for request', + content: { + 'application/json': { + schema: { + type: 'string' + } + } + } + } + } + } + } + }; + + const res = validate({ resolvedSpec: spec }, config); + expect(res.warnings.length).toEqual(1); + expect(res.warnings[0].path).toEqual('paths./pets.post'); + expect(res.warnings[0].message).toEqual( + 'Operations with non-form request bodies should set a name with the x-codegen-request-body-name annotation.' + ); + expect(res.errors.length).toEqual(0); + }); + + it('should not warn about an operation with a non-form request body that sets a name', function() { + const spec = { + paths: { + '/pets': { + post: { + 'x-codegen-request-body-name': 'goodRequestBody', + summary: 'this is a summary', + operationId: 'operationId', + requestBody: { + description: 'body for request', + content: { + 'application/json': { + schema: { + type: 'string' + } + } + } + } + } + } + } + }; + + const res = validate({ resolvedSpec: spec }, config); + expect(res.warnings.length).toEqual(0); + expect(res.errors.length).toEqual(0); + }); + + // should not warn about a form request body + it('should not warn about an operation with a form request body that does not set a name', function() { + const spec = { + paths: { + '/pets': { + post: { + summary: 'this is a summary', + operationId: 'operationId', + requestBody: { + description: 'body for request', + content: { + 'multipart/form-data': { + schema: { + type: 'object', + properties: { + name: { + type: 'string' + } + } + } + } + } + } + } + } + } + }; + + const res = validate({ resolvedSpec: spec }, config); + expect(res.warnings.length).toEqual(0); + expect(res.errors.length).toEqual(0); + }); }); From d87497e5df0cef141fed6dc160dfbfaa2ef7efad Mon Sep 17 00:00:00 2001 From: Dustin Popp Date: Tue, 30 Apr 2019 11:34:11 -0500 Subject: [PATCH 2/2] refactor: change rules a bit for request body names check --- README.md | 2 +- .../oas3/semantic-validators/operations.js | 32 ++++++- test/plugins/validation/oas3/operations.js | 83 +++++++++++++++++-- 3 files changed, 105 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index c5e70775e..55fa9e84f 100644 --- a/README.md +++ b/README.md @@ -308,7 +308,7 @@ The default values for each rule are described below. | Rule | Default | | --------------------------- | --------| | no_request_body_content | error | -| no_request_body_name | error | +| no_request_body_name | warning | ###### parameters | Rule | Default | diff --git a/src/plugins/validation/oas3/semantic-validators/operations.js b/src/plugins/validation/oas3/semantic-validators/operations.js index 4a56bdbf9..bbb7bb9ad 100644 --- a/src/plugins/validation/oas3/semantic-validators/operations.js +++ b/src/plugins/validation/oas3/semantic-validators/operations.js @@ -6,8 +6,9 @@ const pick = require('lodash/pick'); const each = require('lodash/each'); +const at = require('lodash/at'); -module.exports.validate = function({ resolvedSpec }, config) { +module.exports.validate = function({ resolvedSpec, jsSpec }, config) { const result = {}; result.error = []; result.warning = []; @@ -42,9 +43,32 @@ module.exports.validate = function({ resolvedSpec }, config) { } else { // request body has content const firstMimeType = requestBodyMimeTypes[0]; // code generation uses the first mime type + const oneContentType = requestBodyMimeTypes.length === 1; + const isJson = firstMimeType === 'application/json'; + + const hasArraySchema = + requestBodyContent[firstMimeType].schema && + requestBodyContent[firstMimeType].schema.type === 'array'; + const hasRequestBodyName = op[REQUEST_BODY_NAME] && op[REQUEST_BODY_NAME].trim().length; - if (isBodyParameter(firstMimeType) && !hasRequestBodyName) { + + // non-array json responses with only one content type will have + // the body exploded in sdk generation, no need for name + const explodingBody = oneContentType && isJson && !hasArraySchema; + + // referenced request bodies have names + const referencedRequestBody = Boolean( + at(jsSpec, `paths.${pathName}.${opName}.requestBody`)[0].$ref + ); + + // form params do not need names + if ( + !isFormParameter(firstMimeType) && + !explodingBody && + !referencedRequestBody && + !hasRequestBodyName + ) { const checkStatus = config.no_request_body_name; if (checkStatus != 'off') { const message = @@ -63,11 +87,11 @@ module.exports.validate = function({ resolvedSpec }, config) { return { errors: result.error, warnings: result.warning }; }; -function isBodyParameter(mimeType) { +function isFormParameter(mimeType) { const formDataMimeTypes = [ 'multipart/form-data', 'application/x-www-form-urlencoded', 'application/octet-stream' ]; - return !formDataMimeTypes.includes(mimeType); + return formDataMimeTypes.includes(mimeType); } diff --git a/test/plugins/validation/oas3/operations.js b/test/plugins/validation/oas3/operations.js index 47aaaf6d0..585f602f7 100644 --- a/test/plugins/validation/oas3/operations.js +++ b/test/plugins/validation/oas3/operations.js @@ -20,7 +20,7 @@ describe('validation plugin - semantic - operations - oas3', function() { } }; - const res = validate({ resolvedSpec: spec }, config); + const res = validate({ resolvedSpec: spec, jsSpec: spec }, config); expect(res.errors.length).toEqual(1); expect(res.errors[0].path).toEqual('paths./pets.post.requestBody'); expect(res.errors[0].message).toEqual( @@ -29,7 +29,7 @@ describe('validation plugin - semantic - operations - oas3', function() { expect(res.warnings.length).toEqual(0); }); - it('should warn about an operation with a non-form request body that does not set a name', function() { + it('should warn about an operation with a non-form, array schema request body that does not set a name', function() { const spec = { paths: { '/pets': { @@ -41,7 +41,10 @@ describe('validation plugin - semantic - operations - oas3', function() { content: { 'application/json': { schema: { - type: 'string' + type: 'array', + items: { + type: 'string' + } } } } @@ -51,7 +54,7 @@ describe('validation plugin - semantic - operations - oas3', function() { } }; - const res = validate({ resolvedSpec: spec }, config); + const res = validate({ resolvedSpec: spec, jsSpec: spec }, config); expect(res.warnings.length).toEqual(1); expect(res.warnings[0].path).toEqual('paths./pets.post'); expect(res.warnings[0].message).toEqual( @@ -60,6 +63,33 @@ describe('validation plugin - semantic - operations - oas3', function() { expect(res.errors.length).toEqual(0); }); + it('should not warn about an operation with a non-array json request body that does not set a name', function() { + const spec = { + paths: { + '/pets': { + post: { + summary: 'this is a summary', + operationId: 'operationId', + requestBody: { + description: 'body for request', + content: { + 'application/json': { + schema: { + type: 'string' + } + } + } + } + } + } + } + }; + + const res = validate({ resolvedSpec: spec, jsSpec: spec }, config); + expect(res.warnings.length).toEqual(0); + expect(res.errors.length).toEqual(0); + }); + it('should not warn about an operation with a non-form request body that sets a name', function() { const spec = { paths: { @@ -83,12 +113,11 @@ describe('validation plugin - semantic - operations - oas3', function() { } }; - const res = validate({ resolvedSpec: spec }, config); + const res = validate({ resolvedSpec: spec, jsSpec: spec }, config); expect(res.warnings.length).toEqual(0); expect(res.errors.length).toEqual(0); }); - // should not warn about a form request body it('should not warn about an operation with a form request body that does not set a name', function() { const spec = { paths: { @@ -116,7 +145,47 @@ describe('validation plugin - semantic - operations - oas3', function() { } }; - const res = validate({ resolvedSpec: spec }, config); + const res = validate({ resolvedSpec: spec, jsSpec: spec }, config); + expect(res.warnings.length).toEqual(0); + expect(res.errors.length).toEqual(0); + }); + + it('should not warn about an operation with a referenced request body that does not set a name', function() { + const resolvedSpec = { + paths: { + '/pets': { + post: { + summary: 'this is a summary', + operationId: 'operationId', + requestBody: { + content: { + 'application/json': { + schema: { + type: 'string' + } + } + } + } + } + } + } + }; + + const jsSpec = { + paths: { + '/pets': { + post: { + summary: 'this is a summary', + operationId: 'operationId', + requestBody: { + $ref: '#/components/requestBodies/SomeBody' + } + } + } + } + }; + + const res = validate({ resolvedSpec, jsSpec }, config); expect(res.warnings.length).toEqual(0); expect(res.errors.length).toEqual(0); });