Merge pull request #25 from IBM/check-for-request-body-names

feat: flag operations with non-form request bodies that do not specify a name

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 |

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',

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,18 +30,68 @@ 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
+              });
+            }
+          }
         }
       }
     });
   });
 
   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);
+}

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,12 +20,173 @@ 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(
       'Request bodies MUST specify a `content` property'
     );
     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);
+  });
 });