Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add warning, non-204 success response should have response body #127

Merged
merged 1 commit into from
Feb 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ The `-g` flag installs the tool globally so that the validator can be run from a
3. Install the dependencies using `npm install`
4. Build the command line tool by running `npm run link`.

_If you installed the validator using `npm install -g ibm-openapi-validator`, you will need to run `npm uninstall -g ibm-openapi-validator` before running `npm run link`._

### Platform specific binaries
It is possible to build platform specific binaries for Linux, MacOS, and Windows that do not depend on having node.js installed.

Expand Down Expand Up @@ -202,6 +204,7 @@ The supported rules are described below:
| 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|


##### pagination
| Rule | Description | Spec |
| --------------------------- | ------------------------------------------------------------------------ | ------ |
Expand Down Expand Up @@ -237,6 +240,7 @@ The supported rules are described below:
| inline_response_schema | Flag any response object with a schema that doesn't reference a named model. | shared |
| no_response_codes | Flag any response object that has no valid response codes. | oas3 |
| no_success_response_codes | Flag any response object that has no success response codes. | oas3 |
| no_response_body | Flag any non-204 success responses without a response body. | oas3 |

##### schemas
| Rule | Description | Spec |
Expand Down Expand Up @@ -355,6 +359,7 @@ The default values for each rule are described below.
| ------------------------- | ------- |
| no_response_codes | error |
| no_success_response_codes | warning |
| no_response_body | warning |


##### shared
Expand Down
16 changes: 12 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion src/.defaultsForValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ const defaults = {
},
'responses': {
'no_response_codes': 'error',
'no_success_response_codes': 'warning'
'no_success_response_codes': 'warning',
'no_response_body': 'warning'
}
}
};
Expand Down
20 changes: 20 additions & 0 deletions src/plugins/validation/oas3/semantic-validators/responses.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
// A 204 response MUST not define a response body
// https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.2.5

// Assertation 4:
// A non-204 success response MUST define a response body

const { walk } = require('../../../utils');

module.exports.validate = function({ jsSpec }, config) {
Expand Down Expand Up @@ -57,6 +60,23 @@ module.exports.validate = function({ jsSpec }, config) {
message
});
}
} else {
const checkStatus = config.no_response_body;
// if response body rule is on, loops through success codes and issues warning (by default)
// for non-204 success responses without a response body
if (checkStatus !== 'off') {
for (const successCode of successCodes) {
if (successCode != '204' && !obj[successCode].content) {
result[checkStatus].push({
path: path.concat([successCode]),
message:
`A ` +
successCode +
` response should include a response body. Use 204 for responses without content.`
});
}
}
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/cli-validator/mockFiles/oas3/clean.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ paths:
tags:
- pets
responses:
'201':
'204':
description: Null response
default:
description: unexpected error
Expand Down
131 changes: 124 additions & 7 deletions test/plugins/validation/oas3/responses.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
const config = {
responses: {
no_response_codes: 'error',
no_success_response_codes: 'warning'
no_success_response_codes: 'warning',
no_response_body: 'warning'
}
};

Expand Down Expand Up @@ -41,7 +42,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
const config = {
responses: {
no_response_codes: 'error',
no_success_response_codes: 'warning'
no_success_response_codes: 'warning',
no_response_body: 'warning'
}
};

Expand Down Expand Up @@ -74,7 +76,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
const config = {
responses: {
no_response_codes: 'error',
no_success_response_codes: 'warning'
no_success_response_codes: 'warning',
no_response_body: 'warning'
}
};

Expand All @@ -85,8 +88,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
summary: 'this is a summary',
operationId: 'operationId',
responses: {
'200': {
description: 'successful operation call'
'204': {
description: 'successful operation call with no response body'
}
}
}
Expand All @@ -99,11 +102,124 @@ describe('validation plugin - semantic - responses - oas3', function() {
expect(res.warnings.length).toEqual(0);
});

it('should complain when a non-204 success does not have response body', function() {
const config = {
responses: {
no_response_codes: 'error',
no_success_response_codes: 'warning',
no_response_body: 'warning'
}
};

const spec = {
paths: {
'/example': {
get: {
summary: 'this is a summary',
operationId: 'operationId',
responses: {
'200': {
description: 'successful operation call with no response body'
}
}
}
}
}
};

const res = validate({ jsSpec: spec }, config);
expect(res.warnings.length).toEqual(1);
barrett-schonefeld marked this conversation as resolved.
Show resolved Hide resolved
expect(res.warnings[0].path).toEqual([
'paths',
'/example',
'get',
'responses',
'200'
]);
expect(res.warnings[0].message).toEqual(
`A 200 response should include a response body. Use 204 for responses without content.`
);
});

it('should issue multiple warnings when multiple non-204 successes do not have response bodies', function() {
const config = {
responses: {
no_response_codes: 'error',
no_success_response_codes: 'warning',
no_response_body: 'warning'
}
};

const spec = {
paths: {
'/example1': {
get: {
summary: 'this is a summary',
operationId: 'operationId_1',
responses: {
'200': {
description: 'first successful response with no response body'
},
'202': {
description: 'second successful response with no response body'
}
}
}
},
'/example2': {
get: {
summary: 'this is a summary',
operationId: 'operationId_2',
responses: {
'203': {
description: 'third successful response with no response body'
}
}
}
}
}
};

const res = validate({ jsSpec: spec }, config);
expect(res.warnings.length).toEqual(3);
barrett-schonefeld marked this conversation as resolved.
Show resolved Hide resolved
expect(res.warnings[0].path).toEqual([
'paths',
'/example1',
'get',
'responses',
'200'
]);
expect(res.warnings[0].message).toEqual(
`A 200 response should include a response body. Use 204 for responses without content.`
);
expect(res.warnings[1].path).toEqual([
'paths',
'/example1',
'get',
'responses',
'202'
]);
expect(res.warnings[1].message).toEqual(
`A 202 response should include a response body. Use 204 for responses without content.`
);
expect(res.warnings[2].path).toEqual([
'paths',
'/example2',
'get',
'responses',
'203'
]);
expect(res.warnings[2].message).toEqual(
`A 203 response should include a response body. Use 204 for responses without content.`
);
});

it('should complain about having only error responses', function() {
const config = {
responses: {
no_response_codes: 'error',
no_success_response_codes: 'warning'
no_success_response_codes: 'warning',
no_response_body: 'warning'
}
};

Expand Down Expand Up @@ -150,7 +266,8 @@ describe('validation plugin - semantic - responses - oas3', function() {
const config = {
responses: {
no_response_codes: 'error',
no_success_response_codes: 'warning'
no_success_response_codes: 'warning',
no_response_body: 'warning'
}
};

Expand Down