Skip to content

Commit d87497e

Browse files
author
Dustin Popp
committed
refactor: change rules a bit for request body names check
1 parent a229fc0 commit d87497e

File tree

3 files changed

+105
-12
lines changed

3 files changed

+105
-12
lines changed

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ The default values for each rule are described below.
308308
| Rule | Default |
309309
| --------------------------- | --------|
310310
| no_request_body_content | error |
311-
| no_request_body_name | error |
311+
| no_request_body_name | warning |
312312

313313
###### parameters
314314
| Rule | Default |

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

+28-4
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66

77
const pick = require('lodash/pick');
88
const each = require('lodash/each');
9+
const at = require('lodash/at');
910

10-
module.exports.validate = function({ resolvedSpec }, config) {
11+
module.exports.validate = function({ resolvedSpec, jsSpec }, config) {
1112
const result = {};
1213
result.error = [];
1314
result.warning = [];
@@ -42,9 +43,32 @@ module.exports.validate = function({ resolvedSpec }, config) {
4243
} else {
4344
// request body has content
4445
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+
4553
const hasRequestBodyName =
4654
op[REQUEST_BODY_NAME] && op[REQUEST_BODY_NAME].trim().length;
47-
if (isBodyParameter(firstMimeType) && !hasRequestBodyName) {
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+
) {
4872
const checkStatus = config.no_request_body_name;
4973
if (checkStatus != 'off') {
5074
const message =
@@ -63,11 +87,11 @@ module.exports.validate = function({ resolvedSpec }, config) {
6387
return { errors: result.error, warnings: result.warning };
6488
};
6589

66-
function isBodyParameter(mimeType) {
90+
function isFormParameter(mimeType) {
6791
const formDataMimeTypes = [
6892
'multipart/form-data',
6993
'application/x-www-form-urlencoded',
7094
'application/octet-stream'
7195
];
72-
return !formDataMimeTypes.includes(mimeType);
96+
return formDataMimeTypes.includes(mimeType);
7397
}

test/plugins/validation/oas3/operations.js

+76-7
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ describe('validation plugin - semantic - operations - oas3', function() {
2020
}
2121
};
2222

23-
const res = validate({ resolvedSpec: spec }, config);
23+
const res = validate({ resolvedSpec: spec, jsSpec: spec }, config);
2424
expect(res.errors.length).toEqual(1);
2525
expect(res.errors[0].path).toEqual('paths./pets.post.requestBody');
2626
expect(res.errors[0].message).toEqual(
@@ -29,7 +29,7 @@ describe('validation plugin - semantic - operations - oas3', function() {
2929
expect(res.warnings.length).toEqual(0);
3030
});
3131

32-
it('should warn about an operation with a non-form request body that does not set a name', function() {
32+
it('should warn about an operation with a non-form, array schema request body that does not set a name', function() {
3333
const spec = {
3434
paths: {
3535
'/pets': {
@@ -41,7 +41,10 @@ describe('validation plugin - semantic - operations - oas3', function() {
4141
content: {
4242
'application/json': {
4343
schema: {
44-
type: 'string'
44+
type: 'array',
45+
items: {
46+
type: 'string'
47+
}
4548
}
4649
}
4750
}
@@ -51,7 +54,7 @@ describe('validation plugin - semantic - operations - oas3', function() {
5154
}
5255
};
5356

54-
const res = validate({ resolvedSpec: spec }, config);
57+
const res = validate({ resolvedSpec: spec, jsSpec: spec }, config);
5558
expect(res.warnings.length).toEqual(1);
5659
expect(res.warnings[0].path).toEqual('paths./pets.post');
5760
expect(res.warnings[0].message).toEqual(
@@ -60,6 +63,33 @@ describe('validation plugin - semantic - operations - oas3', function() {
6063
expect(res.errors.length).toEqual(0);
6164
});
6265

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+
6393
it('should not warn about an operation with a non-form request body that sets a name', function() {
6494
const spec = {
6595
paths: {
@@ -83,12 +113,11 @@ describe('validation plugin - semantic - operations - oas3', function() {
83113
}
84114
};
85115

86-
const res = validate({ resolvedSpec: spec }, config);
116+
const res = validate({ resolvedSpec: spec, jsSpec: spec }, config);
87117
expect(res.warnings.length).toEqual(0);
88118
expect(res.errors.length).toEqual(0);
89119
});
90120

91-
// should not warn about a form request body
92121
it('should not warn about an operation with a form request body that does not set a name', function() {
93122
const spec = {
94123
paths: {
@@ -116,7 +145,47 @@ describe('validation plugin - semantic - operations - oas3', function() {
116145
}
117146
};
118147

119-
const res = validate({ resolvedSpec: spec }, config);
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);
120189
expect(res.warnings.length).toEqual(0);
121190
expect(res.errors.length).toEqual(0);
122191
});

0 commit comments

Comments
 (0)