Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Ignore compiled files
src/**/*.js
regression/**/*.js
src/**/*.map
src/**/*.d.ts
node_modules/
Expand Down
1 change: 1 addition & 0 deletions src/typescript/azure-openapi-validator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ require("./rules/PreviewVersionOverOneYear")
require("./rules/UniqueXmsExample")
require("./rules/UniqueClientParameterName")
require("./rules/ValidResponseCodeRequired")
require("./rules/Rpaas_ResourceProvisioningState")
export async function run(
document: string,
openapiDefinition: any,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ rules.push({
openapiType: OpenApiTypes.arm,
appliesTo_JsonQuery: "$.paths.*.*.responses.*.schema",
*run(doc, node, path) {
const msg: string = "Response schema must not be empty"
const msg: string = "Response schema must not be empty."
if (!Object.keys(node).length) {
yield { message: `${msg}'`, location: path }
yield { message: `${msg}`, location: path }
}
}
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { MergeStates, OpenApiTypes, rules } from "../rule"
import { ResourceUtils } from "./utilities/resourceUtils"
import { JsonPath } from "../../jsonrpc/types"
export const Rpaas_ResourceProvisioningState: string = "Rpaas_ResourceProvisioningState"

rules.push({
Comment thread
jianyexi marked this conversation as resolved.
id: "R4031",
name: Rpaas_ResourceProvisioningState,
severity: "error",
category: "RPaaSViolation",
mergeState: MergeStates.individual,
openapiType: OpenApiTypes.rpaas,
appliesTo_JsonQuery: "$",
*run(doc, node, path) {
const msg = `The resource {0} is defined without 'provisioningState' in properties bag, consider adding the provisioningState for it.`
const utils = new ResourceUtils(doc)
const allResources = utils.getAllResource()
for (const resource of allResources) {
const model = utils.getResourceByName(resource)
const properties = utils.getPropertyOfModel(model,"properties")
let hasProvisioningState = false
if (properties && (!properties.type || properties.type === "object")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If type undefined, is it valid?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, undefined type equals to "object"

if (utils.getPropertyOfModel(properties,"provisioningState")) {
hasProvisioningState = true
}

}
if (!hasProvisioningState) {
yield { message: msg.replace("{0}", resource),
location: ["$", "definitions", resource] as JsonPath}
}
}
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ rules.push({
*run(doc, node, path) {
const msg: string = 'The top-level resource "{0}" does not have list by resource group operation, please add it.'
const utils = new ResourceUtils(doc)
const topLevelResources = utils.getAllTopLevelResources()
const topLevelResources = utils.getTopLevelResourcesByRG()
const allCollectionPath = utils.getCollectionApiInfo()
for (const resource of topLevelResources) {
const hasMatched = allCollectionPath.some(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class ResourceUtils {
}
}

private getResourceByName(modelName: string) {
public getResourceByName(modelName: string) {
if (!modelName) {
return undefined
}
Expand Down Expand Up @@ -132,6 +132,10 @@ export class ResourceUtils {
}
}


/**
* Get all resources which allOf a x-ms-resource
*/
public getAllOfResources() {
const keys = Object.keys(this.innerDoc.definitions as object)
const AllOfResources = keys.reduce((pre, cur) => {
Expand Down Expand Up @@ -195,6 +199,42 @@ export class ResourceUtils {
}
return topLevelModels
}

public getTopLevelResourcesByRG() {
const fullModels = this.getOperationGetResponseResources()
const topLevelModels = new Set<string>()
for (const entry of fullModels.entries()) {
const paths = entry[1]
Comment thread
jianyexi marked this conversation as resolved.
paths
.filter(p => this.isPathByResourceGroup(p))
.some(p => {
const hierarchy = this.getResourcesTypeHierarchy(p)
if (hierarchy.length === 1) {
topLevelModels.add(entry[0])
return true
}
})
}
return topLevelModels
}

public getAllResource() {
const fullModels = this.getOperationGetResponseResources()
const resources = new Set<string>()
for (const entry of fullModels.entries()) {
const paths = entry[1]
paths
.some(p => {
const hierarchy = this.getResourcesTypeHierarchy(p)
if (hierarchy.length > 0) {
resources.add(entry[0])
return true
}
})
}
return resources.values()
}

/**
*
* @param path
Expand Down Expand Up @@ -249,8 +289,25 @@ export class ResourceUtils {
return hierarchy
}

private dereference(ref: string) {
return this.getResourceByName(this.stripDefinitionPath(ref))
private dereference(ref: string, visited:Set<string>) {
if (visited.has(ref)) {
return undefined
}
visited.add(ref)
const model = this.getResourceByName(this.stripDefinitionPath(ref))
if (model && model.$ref) {
return this.dereference(model.$ref, visited)
}
else {
return model
}
}

private getUnwrappedModel(property:any) {
if (property) {
const ref = property.$ref
return ref ? this.dereference(ref, new Set<string>()) : property
}
}

public containsDiscriminator(modelName: string) {
Expand Down Expand Up @@ -432,21 +489,7 @@ export class ResourceUtils {
if (!model) {
return undefined
}
if (model.properties) {
if (model.properties[propertyName]) {
const ref = model.properties[propertyName].$ref
return ref ? this.dereference(ref) : model.properties[propertyName]
}
}
if (model.allOf) {
for (const element of model.allOf) {
const property = this.getPropertyOfModelName(this.stripDefinitionPath(element.$ref), propertyName)
if (property) {
const ref = property.$ref
return ref ? this.dereference(ref) : property
}
}
}
return this.getPropertyOfModel(model,propertyName)
}

public getPropertyOfModel(sourceModel, propertyName: string) {
Expand All @@ -455,20 +498,29 @@ export class ResourceUtils {
}
let model = sourceModel
if (sourceModel.$ref) {
model = this.getResourceByName(this.stripDefinitionPath(sourceModel))
model = this.getUnwrappedModel(sourceModel.$ref)
}
if (!model) {
return undefined
}
if (model.properties) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

model.properties?.propertyName

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, 'propertyName' is variable

if (model.properties[propertyName]) {
const ref = model.properties[propertyName].$ref
return ref ? this.dereference(ref) : model.properties[propertyName]
return this.getUnwrappedModel(model.properties[propertyName])
}
}
if (model.allOf) {
for (const element of model.allOf) {
const property = this.getPropertyOfModelName(this.stripDefinitionPath(element.$ref), propertyName)
if (property) {
const ref = property.$ref
return ref ? this.dereference(ref) : property
if (element.$ref) {
const property = this.getPropertyOfModelName(this.stripDefinitionPath(element.$ref), propertyName)
if (property) {
return this.getUnwrappedModel(property)
}
}
else {
const property = this.getPropertyOfModel(element,propertyName)
if (property) {
Comment thread
jianyexi marked this conversation as resolved.
Outdated
return property
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { PathResourceTypeNameCamelCase } from "./../rules/PathResourceTypeNameCa
import { assertValidationRuleCount, collectTestMessagesFromValidator } from "./utilities/tests-helper"
import { Rpaas_DeleteOperationAsyncResponseValidation } from "../rules/Rpaas_DeleteOperationAsyncResponseValidation"
import { Rpaas_PostOperationAsyncResponseValidation } from "../rules/Rpaas_PostOperationAsyncResponseValidation"
import { Rpaas_ResourceProvisioningState } from "../rules/Rpaas_ResourceProvisioningState"
@suite
class IndividualAzureTests {
@test public async "control characters not allowed test"() {
Expand Down Expand Up @@ -245,6 +246,17 @@ class IndividualAzureTests {
assertValidationRuleCount(messages, Rpaas_PostOperationAsyncResponseValidation, 0)
}

@test public async "Raas resource is defined with empty properties"() {
const fileName = "RpaasResourceWithEmptyPropertiesBag.json"
const messages: Message[] = await collectTestMessagesFromValidator(fileName, OpenApiTypes.rpaas, MergeStates.individual)
assertValidationRuleCount(messages, Rpaas_ResourceProvisioningState, 1)
}

@test public async "Raas resource is defined with provisioning properties"() {
const fileName = "RpaasResourceWithProvisioningState.json"
const messages: Message[] = await collectTestMessagesFromValidator(fileName, OpenApiTypes.rpaas, MergeStates.individual)
assertValidationRuleCount(messages, Rpaas_ResourceProvisioningState, 0)
}

@test public async "Unique x-ms-examples"() {
const fileName: string = "UniqueXmsExample.json"
Expand Down
20 changes: 20 additions & 0 deletions src/typescript/azure-openapi-validator/tests/resourceUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,24 @@ class ResourceUtilsTests {
assert.equal(allCollectionInfo.length, 22)
assert.equal(allCollectionModel.size, 21)
}

@test public "test get properties"() {
const swagger = readObjectFromFile(getFilePath("armResource/test_get_properties.json"))
const util = new ResourceUtils(swagger)
const bar = util.getResourceByName("A")
assert.deepEqual(
{
type: "string",
description: "p1"
},
util.getPropertyOfModel(bar, "p1")
)
const foo = util.getResourceByName("B")
assert.deepEqual(
{
description: "a ref"
},
util.getPropertyOfModel(foo, "display")
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
{
"swagger": "2.0",
"info": {
"title": "Network interfaces have unallowed types",
"description": "Some documentation.",
"version": "2014-04-01-preview"
},
"host": "management.azure.com",
"schemes": [
"https"
],
"basePath": "/",
"produces": [
"application/json"
],
"consumes": [
"application/json"
],
"paths": {
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/applicationGateways/{applicationGatewayName}": {
"get": {
"operationId": "getTestResource",
"summary": "Foo path",
"description": "Foo operation",
"parameters": [
{
"$ref": "#/parameters/SubscriptionIdParameter"
},
{
"$ref": "#/parameters/ApiVersion"
}
],
"responses": {
"200": {
"description": "Created",
"schema":{
"$ref": "#/definitions/TestResource"
}
},
"default": {
"description": "Unexpected error"
}
}
}
}
},
"parameters": {
"SubscriptionIdParameter": {
"name": "subscriptionId",
"in": "path",
"required": true,
"type": "string",
"description": "test subscription id"
},
"ApiVersion": {
"name": "api-version",
"in": "path",
"required": true,
"type": "string",
"description": "test api version"
}
},
"definitions": {
"Resource": {
"properties": {
"id": {
"type": "string",
"description": "Resource ID."
},
"name": {
"readOnly": true,
"type": "string",
"description": "Resource name."
},
"type": {
"readOnly": true,
"type": "string",
"description": "Resource type."
}
},
"description": "Common resource representation.",
"x-ms-azure-resource": true
},
"TestResource": {
"description": "test",
"allOf": [
{"$ref": "#/definitions/Resource"}
],
"properties": {
"properties":{
"type":"object",
"description": "properties"
}
}
}
}
}
Loading