Revert "Revert "Add pageable rule (#161)" (#168)"

This reverts commit 5f55f1c.
Azure · May 10, 2018 · ded46ad · ded46ad
1 parent 5f55f1c
commit ded46ad
Show file tree

Hide file tree

Showing 7 changed files with 366 additions and 0 deletions.
diff --git a/docs/pageable-operations.md b/docs/pageable-operations.md
@@ -0,0 +1,134 @@
+# PageableOperations
+
+## Description
+
+This rule appears when you define a get operation, and its response schema has a property that is an array. The operation might be pageable.
+
+## How to fix
+
+Add an `x-ms-pageable` extension to the operation.
+
+## Effect on generated code
+
+### Before
+
+#### Spec
+
+```json5
+…
+    "paths": {
+        "/plants": {
+            "get": {
+                "operationId": "Stuff_List",
+                "responses": {
+                    "200": {
+                        "schema": {
+                            "$ref": "#/definitions/StuffList"
+                        }
+                    }
+                }
+            }
+        }
+    }
+…
+    "definitions": {
+        "StuffList": {
+            "type": "object",
+            "properties": {
+                "value": {
+                    "type": "array",
+                    "items": {
+                        "$ref": "#/definitions/Stuff"
+                    }
+                },
+                "nextLink": {
+                    "type": "string",
+                }
+            }
+        },
+        "Stuff": {
+            "type": "object",
+            "properties": {
+                "name": {
+                    "type": "string",
+                },
+            }
+        }
+    }
+…
+```
+
+#### Generated code
+
+```csharp
+public static ProductResult GetMultiplePages(this IPagingOperations operations, string clientRequestId = default(string), PagingGetMultiplePagesOptions pagingGetMultiplePagesOptions = default(PagingGetMultiplePagesOptions))
+{
+    return operations.GetMultiplePagesAsync(clientRequestId, pagingGetMultiplePagesOptions).GetAwaiter().GetResult();
+}
+```
+
+### After
+
+#### Spec
+
+```json5
+…
+    "paths": {
+        "/plants": {
+            "get": {
+                "operationId": "Stuff_List",
+                "responses": {
+                    "200": {
+                        "schema": {
+                            "$ref": "#/definitions/StuffList"
+                        }
+                    }
+                }
+                "x-ms-pageable": {
+                    "nextLinkName": "nextLink"
+                }
+            }
+        }
+    }
+…
+    "definitions": {
+        "StuffList": {
+            "type": "object",
+            "properties": {
+                "value": {
+                    "type": "array",
+                    "items": {
+                        "$ref": "#/definitions/Stuff"
+                    }
+                },
+                "nextLink": {
+                    "type": "string",
+                }
+            }
+        },
+        "Stuff": {
+            "type": "object",
+            "properties": {
+                "name": {
+                    "type": "string",
+                },
+            }
+        }
+    }
+…
+```
+
+#### Generated code
+
+```csharp
+public static IPage<Product> GetMultiplePages(this IPagingOperations operations, string clientRequestId = default(string), PagingGetMultiplePagesOptions pagingGetMultiplePagesOptions = default(PagingGetMultiplePagesOptions))
+{
+    return operations.GetMultiplePagesAsync(clientRequestId, pagingGetMultiplePagesOptions).GetAwaiter().GetResult();
+}
+…
+public static IPage<Product> GetMultiplePagesNext(this IPagingOperations operations, string nextPageLink, string clientRequestId = default(string), PagingGetMultiplePagesOptions pagingGetMultiplePagesOptions = default(PagingGetMultiplePagesOptions))
+{
+    return operations.GetMultiplePagesNextAsync(nextPageLink, clientRequestId, pagingGetMultiplePagesOptions).GetAwaiter().GetResult();
+}
+}
+```
diff --git a/src/typescript/azure-openapi-validator/index.ts b/src/typescript/azure-openapi-validator/index.ts
@@ -7,6 +7,7 @@ import { Message } from "../jsonrpc/types";
 import { rules, OpenApiTypes, MergeStates } from "./rule";
 
 // register rules
+require("./rules/PageableOperation");
 require("./rules/DescriptionMustNotBeNodeName");
 require("./rules/ControlCharactersAreNotAllowed");
 require("./rules/ArraySchemaMustHaveItems");

diff --git a/src/typescript/azure-openapi-validator/rules/PageableOperation.ts b/src/typescript/azure-openapi-validator/rules/PageableOperation.ts
@@ -0,0 +1,47 @@
+/*---------------------------------------------------------------------------------------------
+ *  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 { getSuccessfulResponseSchema } from './utilities/rules-helper';
+export const PageableOperation: string = "PageableOperation";
+
+const jp = require('jsonpath');
+
+rules.push({
+  id: "R2029",
+  name: PageableOperation,
+  severity: "warning",
+  category: "SDKViolation",
+  mergeState: MergeStates.composed,
+  openapiType: OpenApiTypes.arm | OpenApiTypes.dataplane,
+  appliesTo_JsonQuery: "$.paths[?(@.get)]",
+  run: function* (doc, node, path) {
+    const operations = Object.keys(node);
+    const getKey = operations.find(key => {
+      return key.toLowerCase() === 'get';
+    });
+
+    var schemaProperties = getSuccessfulResponseSchema(node[getKey], doc)
+
+    function hasArrayProperty(schema) {
+      let arrayPresent: boolean = false;
+      Object.keys(schema).forEach(function (key) {
+        if (schema[key].type === 'array') {
+          arrayPresent = true;
+        }
+      });
+      return arrayPresent;
+    }
+
+    // Why length 3?
+    // 1 - Array
+    // 2 - NextLink
+    // 3 - Count
+    if (schemaProperties != undefined && schemaProperties != null && Object.keys(schemaProperties).length <= 3) {
+      if (hasArrayProperty(schemaProperties) && !('x-ms-pageable' in node[getKey])) {
+        yield { message: `Based on the response model schema, operation '${node[getKey].operationId}' might be pageable. Consider adding the x-ms-pageable extension.`, location: path.concat(getKey) };
+      }
+    }
+  }
+});
diff --git a/src/typescript/azure-openapi-validator/rules/utilities/rules-helper.ts b/src/typescript/azure-openapi-validator/rules/utilities/rules-helper.ts
@@ -0,0 +1,42 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+export function getSuccessfulResponseSchema(node, doc): any {
+  const responses = Object.keys(node['responses']);
+  const response = getMostSuccessfulResponseKey(responses)
+  return getResponseSchema(node['responses'][response], doc)
+}
+
+function getMostSuccessfulResponseKey(responses: string[]): string {
+  var response: string = 'default';
+  if (responses.includes('200')) {
+    response = '200'
+  } else {
+    var twoHundreds = [];
+    responses.forEach(function (value) {
+      if (value.startsWith('2')) {
+        twoHundreds.push(value);
+      }
+    })
+    if (twoHundreds.length > 0) {
+      response = twoHundreds[0];
+    }
+  }
+  return response;
+}
+
+function getResponseSchema(response: Object, doc): any {
+  const schema = response['schema'];
+  if (schema === undefined || schema === null) {
+    return;
+  }
+  if ('$ref' in schema) {
+    const schemaRef = response['schema']['$ref'];
+    const schemaPath: string[] = (<string>schemaRef).split('/');
+    const schemaProperties = doc.definitions[schemaPath[2]].properties;
+    return schemaProperties;
+  }
+  return schema.properties;
+}
diff --git a/src/typescript/azure-openapi-validator/tests/composite-azure-test.ts b/src/typescript/azure-openapi-validator/tests/composite-azure-test.ts
@@ -12,11 +12,24 @@ import {
   collectTestMessagesFromValidator
 } from './utilities/tests-helper';
 import { DescriptionMustNotBeNodeName } from '../rules/DescriptionMustNotBeNodeName';
+import { PageableOperation } from '../rules/PageableOperation';
 
 @suite class CompositeAzureTests {
   @test async "description should not be property name"() {
     const fileName: string = 'DescriptionSameAsPropertyName.json';
     const messages: Message[] = await collectTestMessagesFromValidator(fileName, OpenApiTypes.arm, MergeStates.composed);
     assertValidationRuleCount(messages, DescriptionMustNotBeNodeName, 2);
   }
+
+  @test async "operations returning a model including an array might be pageable (sad path)"() {
+    const fileName: string = 'PageableOperation.json';
+    const messages: Message[] = await collectTestMessagesFromValidator(fileName, OpenApiTypes.arm, MergeStates.composed);
+    assertValidationRuleCount(messages, PageableOperation, 1);
+  }
+
+  @test async "operations returning a model including an array might be pageable (happy path)"() {
+    const fileName: string = 'happyPath/PageableOperation.json';
+    const messages: Message[] = await collectTestMessagesFromValidator(fileName, OpenApiTypes.arm, MergeStates.composed);
+    assertValidationRuleCount(messages, PageableOperation, 0);
+  }
 }
diff --git a/src/typescript/azure-openapi-validator/tests/resources/PageableOperation.json b/src/typescript/azure-openapi-validator/tests/resources/PageableOperation.json
@@ -0,0 +1,63 @@
+{
+    "swagger": "2.0",
+    "info": {
+        "title": "LinterTests",
+        "description": "Use this swagger to test the linter to detect response types with arrays as possible pageable operations",
+        "version": "2016-10-10"
+    },
+    "host": "management.azure.com",
+    "paths": {
+        "/plants": {
+            "get": {
+                "description": "Get a list of plants.",
+                "operationId": "Plants_List",
+                "parameters": [],
+                "responses": {
+                    "200": {
+                        "description": "OK.",
+                        "schema": {
+                            "$ref": "#/definitions/PlantsList"
+                        }
+                    }
+                }
+            }
+        }
+    },
+    "definitions": {
+        "PlantsList": {
+            "type": "object",
+            "description": "Response to a Pageable operation. This one lists plants",
+            "readOnly": true,
+            "properties": {
+                "value": {
+                    "type": "array",
+                    "description": "List of plants",
+                    "items": {
+                        "$ref": "#/definitions/Plant"
+                    },
+                    "readOnly": true
+                },
+                "nextLink": {
+                    "type": "string",
+                    "description": "Link to next page of results",
+                    "readOnly": true
+                }
+            }
+        },
+        "Plant": {
+            "type": "object",
+            "description": "A plant",
+            "properties": {
+                "name": {
+                    "type": "string",
+                    "description": "The plant's name"
+                },
+                "edible": {
+                    "type": "boolean",
+                    "description": "Is the plant edible?"
+                }
+            }
+        }
+    },
+    "parameters": {}
+}
diff --git a/src/typescript/azure-openapi-validator/tests/resources/happyPath/PageableOperation.json b/src/typescript/azure-openapi-validator/tests/resources/happyPath/PageableOperation.json
@@ -0,0 +1,66 @@
+{
+    "swagger": "2.0",
+    "info": {
+        "title": "LinterTests",
+        "description": "Use this swagger to test the linter to detect response types with arrays as possible pageable operations",
+        "version": "2016-10-10"
+    },
+    "host": "management.azure.com",
+    "paths": {
+        "/plants": {
+            "get": {
+                "description": "Get a list of plants.",
+                "operationId": "Plants_List",
+                "x-ms-pageable": {
+                    "nextLinkName": "nextLink"
+                },
+                "parameters": [],
+                "responses": {
+                    "200": {
+                        "description": "OK.",
+                        "schema": {
+                            "$ref": "#/definitions/PlantsList"
+                        }
+                    }
+                }
+            }
+        }
+    },
+    "definitions": {
+        "PlantsList": {
+            "type": "object",
+            "description": "Response to a Pageable operation. This one lists plants",
+            "readOnly": true,
+            "properties": {
+                "value": {
+                    "type": "array",
+                    "description": "List of plants",
+                    "items": {
+                        "$ref": "#/definitions/Plant"
+                    },
+                    "readOnly": true
+                },
+                "nextLink": {
+                    "type": "string",
+                    "description": "Link to next page of results",
+                    "readOnly": true
+                }
+            }
+        },
+        "Plant": {
+            "type": "object",
+            "description": "A plant",
+            "properties": {
+                "name": {
+                    "type": "string",
+                    "description": "The plant's name"
+                },
+                "edible": {
+                    "type": "boolean",
+                    "description": "Is the plant edible?"
+                }
+            }
+        }
+    },
+    "parameters": {}
+}