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

Added one more rule for response validation in model validation. #436

Merged
merged 10 commits into from
Jul 8, 2019
Merged
Show file tree
Hide file tree
Changes from 8 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
8 changes: 6 additions & 2 deletions ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
# Changelog
### 07/02/2019 0.18.6
- Fail the validation when there's schema defined for response body in spec but the example response doesn't provide the body.

### 06/12/2019 0.18.5

### 05/30/2019 0.18.4
- Better error reporting for spec reference pointing to invalid files
- Better error reporting for spec reference pointing to invalid files.

# Changelog
### 05/29/2019 0.18.3
- Update cache initialization in live validator to `Promise.all()`.
- Update package version of `yuml2svg` dependency (since it contains typescript definitions).
Expand Down
8 changes: 8 additions & 0 deletions lib/util/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ export const ErrorCodes = {
DoubleForwardSlashesInUrl: {
name: "DOUBLE_FORWARD_SLASHES_IN_URL",
id: "OAV129"
},
ResponseBodyNotInExample: {
name: "RESPONSE_BODY_NOT_IN_EXAMPLE",
id: "OAV130"
},
ExtraResponseBodyInExample: {
name: "EXTRA_RESPONSE_BODY_IN_EXAMPLE",
id: "OAV131"
}
}

Expand Down
21 changes: 19 additions & 2 deletions lib/validators/modelValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -558,20 +558,37 @@ export class ModelValidator extends SpecValidator<SpecValidationResult> {

const exampleResponseHeaders = exampleResponseValue[exampleResponseStatusCode].headers || {}
const exampleResponseBody = exampleResponseValue[exampleResponseStatusCode].body
if (exampleResponseBody && !response.schema) {

// Fail when example provides the response body but the swagger spec doesn't define the schema for the response.
if (exampleResponseBody !== undefined && !response.schema) {
const msg =
`Response statusCode "${exampleResponseStatusCode}" for operation ` +
`"${operation.operationId}" has response body provided in the example, ` +
`however the response does not have a "schema" defined in the swagger spec.`
const e = this.constructErrorObject<Sway.ValidationEntry>({
code: C.ErrorCodes.ResponseSchemaNotInSpec,
code: C.ErrorCodes.ExtraResponseBodyInExample,
message: msg,
source: operation.definition
})
validationResults.errors.push(e)
log.error(e as any)
continue
} else if (exampleResponseBody === undefined && response.schema) {
// Fail when example doesn't provide the response body but the swagger spec define the schema for the response.
const msg =
`Response statusCode "${exampleResponseStatusCode}" for operation ` +
`"${operation.operationId}" has no response body provided in the example, ` +
`however the response does have a "schema" defined in the swagger spec.`
const e = this.constructErrorObject<Sway.ValidationEntry>({
code: C.ErrorCodes.ResponseBodyNotInExample,
message: msg,
source: operation.definition
})
validationResults.errors.push(e)
log.error(e as any)
continue
}

// ensure content-type header is present
if (!(exampleResponseHeaders["content-type"] || exampleResponseHeaders["Content-Type"])) {
exampleResponseHeaders["content-type"] = utils.getJsonContentType(operation.produces)
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "oav",
"version": "0.18.5",
"version": "0.18.6",
"author": {
"name": "Microsoft Corporation",
"email": "azsdkteam@microsoft.com",
Expand Down Expand Up @@ -36,7 +36,7 @@
"vscode-jsonrpc": "^3.6.2",
"winston": "^3.2.1",
"yargs": "^6.6.0",
"yasway": "^1.8.1",
"yasway": "^1.8.3",
"yuml2svg": "^4.2.1",
"z-schema": "^4.1.0"
},
Expand Down
15,605 changes: 10,480 additions & 5,125 deletions regression/__snapshots__/validateExamplesRegression_latestOnly.test.ts.snap

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
{
"parameters":{},
"responses":{
"200": {},
"203": {}
"200": {
"body": {}
},
"203": {
"body": {}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"parameters": {},
"responses": {
"200": {
"body": {}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"swagger": "2.0",
"info": {
"title": "sometitle",
"version": "2018-01-01"
},
"consumes": [
"application/json"
],
"produces": [
"application/json"
],
"paths": {
"/somepath": {
"get": {
"operationId": "op",
"responses": {
"200": {
"description": "somedescription"
}
},
"x-ms-examples": {
"Fail": {
"$ref": "./examples/fail.json"
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"parameters": {},
"responses": {
"200": {}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"swagger": "2.0",
"info": {
"title": "sometitle",
"version": "2018-01-01"
},
"consumes": [
"application/json"
],
"produces": [
"application/json"
],
"paths": {
"/somepath": {
"get": {
"operationId": "op",
"responses": {
"200": {
"description": "somedescription",
"schema": {}
}
},
"x-ms-examples": {
"Fail": {
"$ref": "./examples/fail.json"
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
{
"parameters":{},
"responses":{
"200": {},
"200": {
"body": {}
},
"404": {
"body": {
"nameAvailable": "string instead of bool"
Expand Down
22 changes: 22 additions & 0 deletions test/modelValidatorTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,28 @@ describe("Model Validation", () => {
})
})

describe("No body in response if schema is defined for response", () => {
it("should fail on example without body defined in response", async () => {
const specPath2 = `${testPath}/modelValidation/swaggers/specification/noBodyInResponse/test.json`
const result = await validate.validateExamples(specPath2, undefined, {
consoleLogLevel: "off"
})
assert(result.length === 1)
assert.strictEqual(result[0].code, "RESPONSE_BODY_NOT_IN_EXAMPLE")
})
})

describe("Extra body in response even it's not defined in schema", () => {
it("should fail on example when extra body defined in response", async () => {
const specPath2 = `${testPath}/modelValidation/swaggers/specification/extraBodyInResponse/test.json`
const result = await validate.validateExamples(specPath2, undefined, {
consoleLogLevel: "off"
})
assert(result.length === 1)
assert.strictEqual(result[0].code, "EXTRA_RESPONSE_BODY_IN_EXAMPLE")
})
})

describe("Default doesn't cover non-error responses", () => {
it("should fail on example with unrecognized status code", async () => {
const specPath2 = `${testPath}/modelValidation/swaggers/specification/defaultIsErrorOnly/test.json`
Expand Down