From ec4547768eec13daa5f34c0f3644e71adfbc6920 Mon Sep 17 00:00:00 2001
From: Mike Kistler <mikekistler@microsoft.com>
Date: Sun, 19 Nov 2023 09:36:02 -0600
Subject: [PATCH] Add function to check properties of LRO response schema

---
 functions/lro-response-schema.js | 104 ++++++++++++++++++++++++
 spectral.yaml                    |   6 +-
 test/lro-response-schema.test.js | 135 +++++++++++++++++++++++++++++++
 3 files changed, 242 insertions(+), 3 deletions(-)
 create mode 100644 functions/lro-response-schema.js
 create mode 100644 test/lro-response-schema.test.js

diff --git a/functions/lro-response-schema.js b/functions/lro-response-schema.js
new file mode 100644
index 0000000000..8d218b8fb6
--- /dev/null
+++ b/functions/lro-response-schema.js
@@ -0,0 +1,104 @@
+// Check conformance to Azure guidelines for 202 responses:
+// - A 202 response should have a response body schema
+// - The response body schema should contain `id`, `status`, and `error` properties.
+// - The `id`, `status`, and `error` properties should be required.
+// - The `id` property should be type: string.
+// - The `status` property should be type: string and enum with values:
+//   - "Running", "Succeeded", "Failed", "Cancelled".
+// - The `error` property should be type: object and not required.
+
+// Rule target is a 202 response
+module.exports = (lroResponse, _opts, context) => {
+  // defensive programming - make sure we have an object
+  if (lroResponse === null || typeof lroResponse !== 'object') {
+    return [];
+  }
+
+  const lroResponseSchema = lroResponse.schema;
+
+  // A 202 response should include a schema for the operation status monitor.
+  if (!lroResponseSchema) {
+    return [{
+      message: 'A 202 response should include a schema for the operation status monitor.',
+      path: context.path || [],
+    }];
+  }
+
+  const path = [...(context.path || []), 'schema'];
+
+  const errors = [];
+
+  // - The `id`, `status`, and `error` properties should be required.
+  const requiredProperties = new Set(lroResponseSchema.required || []);
+  const checkRequiredProperty = (prop) => {
+    if (!requiredProperties.has(prop)) {
+      errors.push({
+        message: `\`${prop}\` property in LRO response should be required`,
+        path: [...path, 'required'],
+      });
+    }
+  };
+
+  // Check id property
+  if (lroResponseSchema.properties && 'id' in lroResponseSchema.properties) {
+    if (lroResponseSchema.properties.id.type !== 'string') {
+      errors.push({
+        message: '\'id\' property in LRO response should be type: string',
+        path: [...path, 'properties', 'id', 'type'],
+      });
+    }
+    checkRequiredProperty('id');
+  } else {
+    errors.push({
+      message: 'LRO response should contain top-level property `id`',
+      path: [...path, 'properties'],
+    });
+  }
+
+  // Check status property
+  if (lroResponseSchema.properties && 'status' in lroResponseSchema.properties) {
+    if (lroResponseSchema.properties.status.type !== 'string') {
+      errors.push({
+        message: '`status` property in LRO response should be type: string',
+        path: [...path, 'properties', 'status', 'type'],
+      });
+    }
+    checkRequiredProperty('status');
+    const statusValues = new Set(lroResponseSchema.properties.status.enum || []);
+    const requiredStatusValues = ['Running', 'Succeeded', 'Failed', 'Canceled'];
+    if (!requiredStatusValues.every((value) => statusValues.has(value))) {
+      errors.push({
+        message: `'status' property enum in LRO response should contain values: ${requiredStatusValues.join(', ')}`,
+        path: [...path, 'properties', 'status', 'enum'],
+      });
+    }
+  } else {
+    errors.push({
+      message: 'LRO response should contain top-level property `status`',
+      path: [...path, 'properties'],
+    });
+  }
+
+  // Check error property
+  if (lroResponseSchema.properties && 'error' in lroResponseSchema.properties) {
+    if (lroResponseSchema.properties.error.type !== 'object') {
+      errors.push({
+        message: '`error` property in LRO response should be type: object',
+        path: [...path, 'properties', 'error', 'type'],
+      });
+    }
+    if (requiredProperties.has('error')) {
+      errors.push({
+        message: '`error` property in LRO response should not be required',
+        path: [...path, 'required'],
+      });
+    }
+  } else {
+    errors.push({
+      message: 'LRO response should contain top-level property `error`',
+      path: [...path, 'properties'],
+    });
+  }
+
+  return errors;
+};
diff --git a/spectral.yaml b/spectral.yaml
index bcda1b4d5e..deadd89b98 100644
--- a/spectral.yaml
+++ b/spectral.yaml
@@ -4,6 +4,7 @@ functions:
   - consistent-response-body
   - error-response
   - has-header
+  - lro-response-schema
   - naming-convention
   - operation-id
   - pagination-parameters
@@ -236,13 +237,12 @@ rules:
 
   az-lro-response-schema:
     description: A 202 response should include a schema for the operation status monitor.
-    message: A 202 response should include a schema for the operation status monitor.
+    message: '{{error}}'
     severity: warn
     formats: ['oas2']
     given: $.paths[*][*].responses[?(@property == '202')]
     then:
-      field: schema
-      function: truthy
+      function: lro-response-schema
 
   az-204-no-response-body:
     description: A 204 response should have no response body.
diff --git a/test/lro-response-schema.test.js b/test/lro-response-schema.test.js
new file mode 100644
index 0000000000..5e64c80a01
--- /dev/null
+++ b/test/lro-response-schema.test.js
@@ -0,0 +1,135 @@
+const { linterForRule } = require('./utils');
+
+let linter;
+
+beforeAll(async () => {
+  linter = await linterForRule('az-lro-response-schema');
+  return linter;
+});
+
+test('az-lro-response-schema should find errors', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test1': {
+        post: {
+          responses: {
+            202: {
+              description: 'Accepted',
+            },
+          },
+        },
+      },
+      '/test2': {
+        post: {
+          responses: {
+            202: {
+              description: 'Accepted',
+              schema: {
+                type: 'object',
+                properties: {
+                  id: {
+                    type: 'string',
+                  },
+                  status: {
+                    type: 'string',
+                    enum: ['Running', 'Succeeded', 'Failed', 'Canceled'],
+                  },
+                },
+              },
+            },
+          },
+        },
+      },
+      '/test3': {
+        post: {
+          responses: {
+            202: {
+              description: 'Accepted',
+              schema: {
+                type: 'object',
+                properties: {
+                  id: {
+                    type: 'uuid',
+                  },
+                  status: {
+                    type: 'string',
+                    enum: ['InProgress', 'Succeeded', 'Failed', 'Canceled'],
+                  },
+                  error: {
+                    type: 'string',
+                  },
+                },
+                required: ['id', 'status', 'error'],
+              },
+            },
+          },
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results).toHaveLength(8);
+    expect(results[0].path.join('.')).toBe('paths./test1.post.responses.202');
+    expect(results[0].message).toBe('A 202 response should include a schema for the operation status monitor.');
+    expect(results[1].path.join('.')).toBe('paths./test2.post.responses.202.schema');
+    expect(results[1].message).toBe('`id` property in LRO response should be required');
+    expect(results[2].path.join('.')).toBe('paths./test2.post.responses.202.schema');
+    expect(results[2].message).toBe('`status` property in LRO response should be required');
+    expect(results[3].path.join('.')).toBe('paths./test2.post.responses.202.schema.properties');
+    expect(results[3].message).toBe('LRO response should contain top-level property `error`');
+    expect(results[4].path.join('.')).toBe('paths./test3.post.responses.202.schema.properties.id.type');
+    expect(results[4].message).toBe('\'id\' property in LRO response should be type: string');
+    expect(results[5].path.join('.')).toBe('paths./test3.post.responses.202.schema.properties.status.enum');
+    expect(results[5].message).toBe('\'status\' property enum in LRO response should contain values: Running, Succeeded, Failed, Canceled');
+    expect(results[6].path.join('.')).toBe('paths./test3.post.responses.202.schema.properties.error.type');
+    expect(results[6].message).toBe('`error` property in LRO response should be type: object');
+    expect(results[7].path.join('.')).toBe('paths./test3.post.responses.202.schema.required');
+    expect(results[7].message).toBe('`error` property in LRO response should not be required');
+  });
+});
+
+test('az-lro-response-schema should find no errors', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test1': {
+        post: {
+          responses: {
+            202: {
+              description: 'Accepted',
+              schema: {
+                type: 'object',
+                properties: {
+                  id: {
+                    type: 'string',
+                  },
+                  status: {
+                    type: 'string',
+                    enum: ['Running', 'Succeeded', 'Failed', 'Canceled'],
+                  },
+                  error: {
+                    type: 'object',
+                    properties: {
+                      code: {
+                        type: 'string',
+                      },
+                      message: {
+                        type: 'string',
+                      },
+                    },
+                    required: ['code', 'message'],
+                  },
+                },
+                required: ['id', 'status'],
+              },
+            },
+          },
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results).toHaveLength(0);
+  });
+});