Skip to content

Commit 2c4c810

Browse files
dpopp07dpopp07
authored
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
2 parents cf64905 + d87497e commit 2c4c810

File tree

4 files changed

+226
-11
lines changed

4 files changed

+226
-11
lines changed

‎README.md

+2
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ The supported rules are described below:
183183
| no_array_responses | Flag any operations with a top-level array response. | shared |
184184
| parameter_order | Flag any operations with optional parameters before a required param. | shared |
185185
| no_request_body_content | [Flag any operations with a `requestBody` that does not have a `content` field.][3] | oas3 |
186+
| 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 |
186187

187188
##### parameters
188189
| Rule | Description | Spec |
@@ -307,6 +308,7 @@ The default values for each rule are described below.
307308
| Rule | Default |
308309
| --------------------------- | --------|
309310
| no_request_body_content | error |
311+
| no_request_body_name | warning |
310312

311313
###### parameters
312314
| Rule | Default |

‎src/.defaultsForValidator.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ const defaults = {
7373
},
7474
'oas3': {
7575
'operations': {
76-
'no_request_body_content': 'error'
76+
'no_request_body_content': 'error',
77+
'no_request_body_name': 'warning'
7778
},
7879
'parameters': {
7980
'no_in_property': 'error',

‎src/plugins/validation/oas3/semantic-validators/operations.js

+59-3
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
11
// Assertation 1. Request body objects must have a `content` property
22
// https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#requestBodyObject
33

4+
// Assertation 2. Operations with non-form request bodies should set the `x-codegen-request-body-name`
5+
// annotation (for code generation purposes)
6+
47
const pick = require('lodash/pick');
58
const each = require('lodash/each');
9+
const at = require('lodash/at');
610

7-
module.exports.validate = function({ resolvedSpec }, config) {
11+
module.exports.validate = function({ resolvedSpec, jsSpec }, config) {
812
const result = {};
913
result.error = [];
1014
result.warning = [];
1115

1216
config = config.operations;
1317

18+
const REQUEST_BODY_NAME = 'x-codegen-request-body-name';
19+
1420
// get, head, and delete are not in this list because they are not allowed
1521
// to have request bodies
1622
const allowedOps = ['post', 'put', 'patch', 'options', 'trace'];
@@ -24,18 +30,68 @@ module.exports.validate = function({ resolvedSpec }, config) {
2430
// Assertation 1
2531
if (op.requestBody) {
2632
const requestBodyContent = op.requestBody.content;
27-
if (!requestBodyContent || !Object.keys(requestBodyContent).length) {
33+
const requestBodyMimeTypes =
34+
op.requestBody.content && Object.keys(requestBodyContent);
35+
if (!requestBodyContent || !requestBodyMimeTypes.length) {
2836
const checkStatus = config.no_request_body_content;
2937
if (checkStatus !== 'off') {
30-
result.error.push({
38+
result[checkStatus].push({
3139
path: `paths.${pathName}.${opName}.requestBody`,
3240
message: 'Request bodies MUST specify a `content` property'
3341
});
3442
}
43+
} else {
44+
// request body has content
45+
const firstMimeType = requestBodyMimeTypes[0]; // code generation uses the first mime type
46+
const oneContentType = requestBodyMimeTypes.length === 1;
47+
const isJson = firstMimeType === 'application/json';
48+
49+
const hasArraySchema =
50+
requestBodyContent[firstMimeType].schema &&
51+
requestBodyContent[firstMimeType].schema.type === 'array';
52+
53+
const hasRequestBodyName =
54+
op[REQUEST_BODY_NAME] && op[REQUEST_BODY_NAME].trim().length;
55+
56+
// non-array json responses with only one content type will have
57+
// the body exploded in sdk generation, no need for name
58+
const explodingBody = oneContentType && isJson && !hasArraySchema;
59+
60+
// referenced request bodies have names
61+
const referencedRequestBody = Boolean(
62+
at(jsSpec, `paths.${pathName}.${opName}.requestBody`)[0].$ref
63+
);
64+
65+
// form params do not need names
66+
if (
67+
!isFormParameter(firstMimeType) &&
68+
!explodingBody &&
69+
!referencedRequestBody &&
70+
!hasRequestBodyName
71+
) {
72+
const checkStatus = config.no_request_body_name;
73+
if (checkStatus != 'off') {
74+
const message =
75+
'Operations with non-form request bodies should set a name with the x-codegen-request-body-name annotation.';
76+
result[checkStatus].push({
77+
path: `paths.${pathName}.${opName}`,
78+
message
79+
});
80+
}
81+
}
3582
}
3683
}
3784
});
3885
});
3986

4087
return { errors: result.error, warnings: result.warning };
4188
};
89+
90+
function isFormParameter(mimeType) {
91+
const formDataMimeTypes = [
92+
'multipart/form-data',
93+
'application/x-www-form-urlencoded',
94+
'application/octet-stream'
95+
];
96+
return formDataMimeTypes.includes(mimeType);
97+
}

‎test/plugins/validation/oas3/operations.js

+163-7
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,10 @@ const expect = require('expect');
22
const {
33
validate
44
} = require('../../../../src/plugins/validation/oas3/semantic-validators/operations');
5+
const config = require('../../../../src/.defaultsForValidator').defaults.oas3;
56

67
describe('validation plugin - semantic - operations - oas3', function() {
78
it('should complain about a request body not having a content field', function() {
8-
const config = {
9-
operations: {
10-
no_request_body_content: 'error'
11-
}
12-
};
13-
149
const spec = {
1510
paths: {
1611
'/pets': {
@@ -25,12 +20,173 @@ describe('validation plugin - semantic - operations - oas3', function() {
2520
}
2621
};
2722

28-
const res = validate({ resolvedSpec: spec }, config);
23+
const res = validate({ resolvedSpec: spec, jsSpec: spec }, config);
2924
expect(res.errors.length).toEqual(1);
3025
expect(res.errors[0].path).toEqual('paths./pets.post.requestBody');
3126
expect(res.errors[0].message).toEqual(
3227
'Request bodies MUST specify a `content` property'
3328
);
3429
expect(res.warnings.length).toEqual(0);
3530
});
31+
32+
it('should warn about an operation with a non-form, array schema request body that does not set a name', function() {
33+
const spec = {
34+
paths: {
35+
'/pets': {
36+
post: {
37+
summary: 'this is a summary',
38+
operationId: 'operationId',
39+
requestBody: {
40+
description: 'body for request',
41+
content: {
42+
'application/json': {
43+
schema: {
44+
type: 'array',
45+
items: {
46+
type: 'string'
47+
}
48+
}
49+
}
50+
}
51+
}
52+
}
53+
}
54+
}
55+
};
56+
57+
const res = validate({ resolvedSpec: spec, jsSpec: spec }, config);
58+
expect(res.warnings.length).toEqual(1);
59+
expect(res.warnings[0].path).toEqual('paths./pets.post');
60+
expect(res.warnings[0].message).toEqual(
61+
'Operations with non-form request bodies should set a name with the x-codegen-request-body-name annotation.'
62+
);
63+
expect(res.errors.length).toEqual(0);
64+
});
65+
66+
it('should not warn about an operation with a non-array json request body that does not set a name', function() {
67+
const spec = {
68+
paths: {
69+
'/pets': {
70+
post: {
71+
summary: 'this is a summary',
72+
operationId: 'operationId',
73+
requestBody: {
74+
description: 'body for request',
75+
content: {
76+
'application/json': {
77+
schema: {
78+
type: 'string'
79+
}
80+
}
81+
}
82+
}
83+
}
84+
}
85+
}
86+
};
87+
88+
const res = validate({ resolvedSpec: spec, jsSpec: spec }, config);
89+
expect(res.warnings.length).toEqual(0);
90+
expect(res.errors.length).toEqual(0);
91+
});
92+
93+
it('should not warn about an operation with a non-form request body that sets a name', function() {
94+
const spec = {
95+
paths: {
96+
'/pets': {
97+
post: {
98+
'x-codegen-request-body-name': 'goodRequestBody',
99+
summary: 'this is a summary',
100+
operationId: 'operationId',
101+
requestBody: {
102+
description: 'body for request',
103+
content: {
104+
'application/json': {
105+
schema: {
106+
type: 'string'
107+
}
108+
}
109+
}
110+
}
111+
}
112+
}
113+
}
114+
};
115+
116+
const res = validate({ resolvedSpec: spec, jsSpec: spec }, config);
117+
expect(res.warnings.length).toEqual(0);
118+
expect(res.errors.length).toEqual(0);
119+
});
120+
121+
it('should not warn about an operation with a form request body that does not set a name', function() {
122+
const spec = {
123+
paths: {
124+
'/pets': {
125+
post: {
126+
summary: 'this is a summary',
127+
operationId: 'operationId',
128+
requestBody: {
129+
description: 'body for request',
130+
content: {
131+
'multipart/form-data': {
132+
schema: {
133+
type: 'object',
134+
properties: {
135+
name: {
136+
type: 'string'
137+
}
138+
}
139+
}
140+
}
141+
}
142+
}
143+
}
144+
}
145+
}
146+
};
147+
148+
const res = validate({ resolvedSpec: spec, jsSpec: spec }, config);
149+
expect(res.warnings.length).toEqual(0);
150+
expect(res.errors.length).toEqual(0);
151+
});
152+
153+
it('should not warn about an operation with a referenced request body that does not set a name', function() {
154+
const resolvedSpec = {
155+
paths: {
156+
'/pets': {
157+
post: {
158+
summary: 'this is a summary',
159+
operationId: 'operationId',
160+
requestBody: {
161+
content: {
162+
'application/json': {
163+
schema: {
164+
type: 'string'
165+
}
166+
}
167+
}
168+
}
169+
}
170+
}
171+
}
172+
};
173+
174+
const jsSpec = {
175+
paths: {
176+
'/pets': {
177+
post: {
178+
summary: 'this is a summary',
179+
operationId: 'operationId',
180+
requestBody: {
181+
$ref: '#/components/requestBodies/SomeBody'
182+
}
183+
}
184+
}
185+
}
186+
};
187+
188+
const res = validate({ resolvedSpec, jsSpec }, config);
189+
expect(res.warnings.length).toEqual(0);
190+
expect(res.errors.length).toEqual(0);
191+
});
36192
});

0 commit comments

Comments
 (0)