diff --git a/README.md b/README.md index 348e3edaa..55fa9e84f 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 | warning | ###### 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..bbb7bb9ad 100644 --- a/src/plugins/validation/oas3/semantic-validators/operations.js +++ b/src/plugins/validation/oas3/semantic-validators/operations.js @@ -1,16 +1,22 @@ // 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'); +const at = require('lodash/at'); -module.exports.validate = function({ resolvedSpec }, config) { +module.exports.validate = function({ resolvedSpec, jsSpec }, config) { const result = {}; result.error = []; result.warning = []; 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 +30,55 @@ 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 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; + + // 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 = + '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 +86,12 @@ module.exports.validate = function({ resolvedSpec }, config) { return { errors: result.error, warnings: result.warning }; }; + +function isFormParameter(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..585f602f7 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': { @@ -25,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( @@ -33,4 +28,165 @@ describe('validation plugin - semantic - operations - oas3', function() { ); expect(res.warnings.length).toEqual(0); }); + + 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': { + post: { + summary: 'this is a summary', + operationId: 'operationId', + requestBody: { + description: 'body for request', + content: { + 'application/json': { + schema: { + type: 'array', + items: { + type: 'string' + } + } + } + } + } + } + } + } + }; + + 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( + '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-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: { + '/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, jsSpec: spec }, config); + expect(res.warnings.length).toEqual(0); + expect(res.errors.length).toEqual(0); + }); + + 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, 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); + }); });