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

Fixing multiple oneOfs #263

Merged
merged 4 commits into from Jun 20, 2018
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: 4 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@
"stopOnEntry": false,
"args": [
"--no-timeouts",
"test/modelValidatorTests.js"
"test/modelValidatorTests.ts",
"-r",
"ts-node/register",
"--no-timeouts"
],
"env": {}
},
Expand Down
4 changes: 4 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
### 06/19/2018 0.4.49
- Bug fix: Data is valid against more than one schema from `oneOf` [#248](https://github.com/Azure/oav/pull/248)
The problem occurs when referenced model may also accept `null`. The fix is replacing `oneOf` with `anyOf`.

### 05/14/2018 0.4.38
- Bug fix: `oav extract-xmsexamples` also extracts request headers. [#241](https://github.com/Azure/oav/pull/241)
- Bug fix: x-ms-parametrized-host is not validated correctly [#240](https://github.com/Azure/oav/issues/240)
Expand Down
8 changes: 5 additions & 3 deletions lib/util/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -559,11 +559,12 @@ export interface Model {
additionalProperties?: Model|false
"x-nullable"?: Unknown
in?: Unknown
oneOf?: Unknown
oneOf?: Model[]
$ref?: string
required?: Unknown[]|false
schema?: Model
allOf?: Ref[]
anyOf?: Model[]
description?: Unknown
discriminator?: string
"x-ms-discriminator-value"?: string
Expand Down Expand Up @@ -698,8 +699,9 @@ export function allowNullType(entity: Model, isPropRequired?: boolean|{}): Model
&& entity.$ref
&& shouldAcceptNullValue(entity["x-nullable"], isPropRequired)) {
const savedEntity = entity
entity = {}
entity.oneOf = [savedEntity, { type: "null" }]
entity = {
anyOf: [savedEntity, { type: "null" }]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyOf [](start = 6, length = 5)

is this the only place where we used to make a oneOf between the type and null ?

Copy link
Contributor Author

@sergey-shandar sergey-shandar Jun 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one that I'm aware of that causes problems. It looks like other cases where we use oneOf should be good.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I was wondering, are we ready to enable tslint on the project


In reply to: 196611051 [](ancestors = 196611051)

Copy link
Contributor Author

@sergey-shandar sergey-shandar Jun 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vladbarosan tslint is enabled already

"tslint": "tslint ./*.ts ./lib/**/*.ts ./test/**/*.ts ./types/**/*.ts",
"test": "npm run tslint && nyc mocha ./test/**/*.ts -r ts-node/register -t 10000",

}
}
return entity
}
Expand Down
3 changes: 0 additions & 3 deletions lib/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@

import * as fs from "fs"
import * as path from "path"
import * as msrest from "ms-rest"
import * as msrestazure from "ms-rest-azure"
import { ResourceManagementClient } from "azure-arm-resource"
import { log } from "./util/logging"
import * as utils from "./util/utils"
import { SpecValidator, SpecValidationResult } from "./validators/specValidator"
Expand Down
10 changes: 5 additions & 5 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "oav",
"version": "0.4.48",
"version": "0.4.49",
"author": {
"name": "Microsoft Corporation",
"email": "[email protected]",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"parameters": {},
"responses": {
"200": {
"headers": {},
"body": {
"namedProp": null
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,29 @@
}
}
},
"/foo/definitionWithReferenceNullOperation": {
"get": {
"tags": [
"getTag"
],
"operationId": "definitionWithReferenceNull_Get",
"description": "Operation to test invalid_type for nulls",
"x-ms-examples": {
"required prop operation": {
"$ref": "./examples/definitionWithReferenceNullOperation.json"
}
},
"parameters": [],
"responses": {
"200": {
"description": "success response",
"schema": {
"$ref": "#/definitions/DefinitionWithReference"
}
}
}
}
},
"/foo/definitionWithReferenceNotNullableOperation": {
"get": {
"tags": [
Expand Down Expand Up @@ -389,7 +412,7 @@
},
"definitions": {
"Result": {
"description": "Result decription",
"description": "Result description",
"properties": {
"prop1": {
"type": "string",
Expand Down
12 changes: 12 additions & 0 deletions test/modelValidatorTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,18 @@ describe("Model Validation", () => {
console.log(result)
})

it("should pass for definitionWithReferenceNull_Get", async () => {
const specPath2 =
`${__dirname}/modelValidation/swaggers/specification/nullableTypes/invalid_type_test.json`
const operationIds = "definitionWithReferenceNull_Get"
const result = await validate.validateExamples(
specPath2, operationIds, { consoleLogLevel: "off" })
assert(
result.validityStatus === true,
`swagger "${specPath2}" with operation "${operationIds}" contains model validation errors.`)
console.log(result)
})

it("should pass for definitionWithReferenceNotNullableOperation_Get", async () => {
const specPath2 =
`${__dirname}/modelValidation/swaggers/specification/nullableTypes/invalid_type_test.json`
Expand Down