Add az-pagination-parameters rule

docs/azure-ruleset.md

@@ -128,6 +128,10 @@ Operation should have a summary or description.
 Post operations that specify x-ms-pageable are problematic because it is unclear what http method
 should be used with the `nextLink` URL.
 
+### az-pagination-parameters
+
+The `top`, `skip`, `maxpagesize`, `filter`, `orderby`, `select`, and `expand` parameters, if present, must follow Azure conventions.
+
 ### az-pagination-response
 
 If the operation returns a list that is potentially large, it should [support pagination](../opeapi-style-guidelines.md#support-for-pagination).
@@ -224,6 +228,10 @@ All success responses except 202 and 204 should define a response body.
 
 Responses for status codes 202 and 204 should have no response body.
 
+### az-top-default-not-allowed
+
+The `top` query parameter should not have a default value. The service should return all results when `top` is not specified.
+
 ### az-version-convention
 
 API version (`info.version`) should be a date in YYYY-MM-DD format, optionally suffixed with '-preview'.

functions/pagination-parameters.js

@@ -0,0 +1,205 @@
+// Check conformance to Azure guidelines for pagination parameters:
+// - if present, `top` must be an integer, optional, with no default value
+// - if present, `skip` must be an integer, optional, with a default value of 0
+// - if present, `maxpagesize` must be an integer, optional, with no default value
+// - if present, `filter` must be a string and optional
+// - if present, `orderby` should be be an array of strings and optional
+// - if present, `select` should be be an array of strings and optional
+// - if present, `expand` should be be an array of strings and optional
+
+module.exports = (operation, _opts, paths) => {
+  // operation should be a get or post operation
+  if (operation === null || typeof operation !== 'object') {
+    return [];
+  }
+  const path = paths.path || paths.target || [];
+
+  // If the operation has no parameters, there is nothing to check
+  if (!operation.parameters) {
+    return [];
+  }
+
+  const errors = [];
+
+  // Check the top parameter
+  const topIndex = operation.parameters.findIndex((param) => param.name?.toLowerCase() === 'top');
+  if (topIndex !== -1) {
+    const top = operation.parameters[topIndex];
+    // Improper casing of top will be flagged by the az-parameter-names-convention rule
+    // Check that top is an integer
+    if (top.type !== 'integer') {
+      errors.push({
+        message: 'top parameter must be type: integer',
+        path: [...path, 'parameters', topIndex, 'type'],
+      });
+    }
+    // Check that top is optional
+    if (top.required) {
+      errors.push({
+        message: 'top parameter must be optional',
+        path: [...path, 'parameters', topIndex, 'required'],
+      });
+    }
+    // Check that top has no default value
+    if (top.default !== undefined) {
+      errors.push({
+        message: 'top parameter must have no default value',
+        path: [...path, 'parameters', topIndex, 'default'],
+      });
+    }
+  }
+
+  // Check skip parameter
+  const skipIndex = operation.parameters.findIndex((param) => param.name?.toLowerCase() === 'skip');
+  if (skipIndex !== -1) {
+    const skip = operation.parameters[skipIndex];
+    // Improper casing of skip will be flagged by the az-parameter-names-convention rule
+    // Check that skip is an integer
+    if (skip.type !== 'integer') {
+      errors.push({
+        message: 'skip parameter must be type: integer',
+        path: [...path, 'parameters', skipIndex, 'type'],
+      });
+    }
+    // Check that skip is optional
+    if (skip.required) {
+      errors.push({
+        message: 'skip parameter must be optional',
+        path: [...path, 'parameters', skipIndex, 'required'],
+      });
+    }
+    // Check that skip has a default value of 0
+    if (skip.default !== 0) {
+      errors.push({
+        message: 'skip parameter must have a default value of 0',
+        path: [...path, 'parameters', skipIndex, 'default'],
+      });
+    }
+  }
+
+  // Check maxpagesize parameter
+  const maxpagesizeIndex = operation.parameters.findIndex((param) => param.name?.toLowerCase() === 'maxpagesize');
+  if (maxpagesizeIndex !== -1) {
+    const maxpagesize = operation.parameters[maxpagesizeIndex];
+    // Check case convention for maxpagesize
+    if (maxpagesize.name !== 'maxpagesize') {
+      errors.push({
+        message: 'maxpagesize parameter must be named "maxpagesize" (all lowercase)',
+        path: [...path, 'parameters', maxpagesizeIndex, 'name'],
+      });
+    }
+    // Check that maxpagesize is an integer
+    if (maxpagesize.type !== 'integer') {
+      errors.push({
+        message: 'maxpagesize parameter must be type: integer',
+        path: [...path, 'parameters', maxpagesizeIndex, 'type'],
+      });
+    }
+    // Check that maxpagesize is optional
+    if (maxpagesize.required) {
+      errors.push({
+        message: 'maxpagesize parameter must be optional',
+        path: [...path, 'parameters', maxpagesizeIndex, 'required'],
+      });
+    }
+    // Check that maxpagesize has no default value
+    if (maxpagesize.default !== undefined) {
+      errors.push({
+        message: 'maxpagesize parameter must have no default value',
+        path: [...path, 'parameters', maxpagesizeIndex, 'default'],
+      });
+    }
+  }
+
+  // Check filter parameter
+  const filterIndex = operation.parameters.findIndex((param) => param.name?.toLowerCase() === 'filter');
+  if (filterIndex !== -1) {
+    const filter = operation.parameters[filterIndex];
+    // Improper casing of filter will be flagged by the az-parameter-names-convention rule
+    // Check that filter is a string
+    if (filter.type !== 'string') {
+      errors.push({
+        message: 'filter parameter must be type: string',
+        path: [...path, 'parameters', filterIndex, 'type'],
+      });
+    }
+    // Check that filter is optional
+    if (filter.required) {
+      errors.push({
+        message: 'filter parameter must be optional',
+        path: [...path, 'parameters', filterIndex, 'required'],
+      });
+    }
+  }
+
+  // Check orderby parameter
+  const orderbyIndex = operation.parameters.findIndex((param) => param.name?.toLowerCase() === 'orderby');
+  if (orderbyIndex !== -1) {
+    const orderby = operation.parameters[orderbyIndex];
+    // Check case convention for orderby
+    if (orderby.name !== 'orderby') {
+      errors.push({
+        message: 'orderby parameter must be named "orderby" (all lowercase)',
+        path: [...path, 'parameters', orderbyIndex, 'name'],
+      });
+    }
+    // Check that orderby is an array of strings
+    if (orderby.type !== 'array' || orderby.items?.type !== 'string') {
+      errors.push({
+        message: 'orderby parameter must be type: array with items of type: string',
+        path: [...path, 'parameters', orderbyIndex, 'type'],
+      });
+    }
+    // Check that orderby is optional
+    if (orderby.required) {
+      errors.push({
+        message: 'orderby parameter must be optional',
+        path: [...path, 'parameters', orderbyIndex, 'required'],
+      });
+    }
+  }
+
+  // Check select parameter
+  const selectIndex = operation.parameters.findIndex((param) => param.name?.toLowerCase() === 'select');
+  if (selectIndex !== -1) {
+    const select = operation.parameters[selectIndex];
+    // Improper casing of select will be flagged by the az-parameter-names-convention rule
+    // Check that select is an array of strings
+    if (select.type !== 'array' || select.items?.type !== 'string') {
+      errors.push({
+        message: 'select parameter must be type: array with items of type: string',
+        path: [...path, 'parameters', selectIndex, 'type'],
+      });
+    }
+    // Check that select is optional
+    if (select.required) {
+      errors.push({
+        message: 'select parameter must be optional',
+        path: [...path, 'parameters', selectIndex, 'required'],
+      });
+    }
+  }
+
+  // Check expand parameter
+  const expandIndex = operation.parameters.findIndex((param) => param.name?.toLowerCase() === 'expand');
+  if (expandIndex !== -1) {
+    const expand = operation.parameters[expandIndex];
+    // Improper casing of expand will be flagged by the az-parameter-names-convention rule
+    // Check that expand is an array of strings
+    if (expand.type !== 'array' || expand.items?.type !== 'string') {
+      errors.push({
+        message: 'expand parameter must be type: array with items of type: string',
+        path: [...path, 'parameters', expandIndex, 'type'],
+      });
+    }
+    // Check that expand is optional
+    if (expand.required) {
+      errors.push({
+        message: 'expand parameter must be optional',
+        path: [...path, 'parameters', expandIndex, 'required'],
+      });
+    }
+  }
+
+  return errors;
+};

openapi-style-guide.md

@@ -109,9 +109,13 @@ To support pagination:
 - The operation should have the `x-ms-pageable` annotation
 - The operation response should contain a top-level `value` property of type array and required
 - The operation response should contain a top-level `nextLink` property of type string and optional
-- If the operation has a `skip` parameter, it must be an integer and optional
+- If present, the `top` parameter must be an integer, optional, with no default value
-- If the operation has a `top` parameter, it must be an integer, optional, and have a documented default and maximum value
+- If present, the `skip` parameter must be an integer, optional, with a default value of 0
-- If the operation has a `maxpagesize` parameter, it must be an integer, optional, and have a documented default and maximum value
+- If present, the `maxpagesize` parameter must be an integer, optional, with no default value
+- If present, the `filter` parameter must be a string and optional
+- If present, the `orderby` parameter should be be an array of strings and optional
+- If present, the `select` parameter should be be an array of strings and optional
+- If present, the `expand` parameter should be be an array of strings and optional
 
 ### Long-running operations
 

spectral.yaml

@@ -6,6 +6,7 @@ functions:
   - error-response
   - has-header
   - operation-id
+  - pagination-parameters
   - pagination-response
   - param-names
   - param-names-unique
@@ -197,6 +198,16 @@ rules:
     then:
       function: falsy
 
+  az-pagination-parameters:
+    description: Pagination parameters must conform to Azure guidelines.
+    message: '{{error}}'
+    severity: warn
+    formats: ['oas2']
+    given:
+    - $.paths.*[get,post]
+    then:
+      function: pagination-parameters
+
   az-pagination-response:
     description: An operation that returns a list that is potentially large should support pagination.
     message: '{{error}}'

test/pagination-parameters.test.js

@@ -0,0 +1,239 @@
+/* eslint-disable object-curly-newline */
+const { linterForRule } = require('./utils');
+
+let linter;
+
+beforeAll(async () => {
+  linter = await linterForRule('az-pagination-parameters');
+  return linter;
+});
+
+test('az-pagination-parameters should find errors in top parameter', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test1': {
+        get: {
+          parameters: [{ name: 'top', in: 'query', type: 'string' }],
+        },
+      },
+      '/test2': {
+        get: {
+          parameters: [{ name: 'top', in: 'query', type: 'integer', required: true }],
+        },
+      },
+      '/test3': {
+        post: {
+          parameters: [{ name: 'top', in: 'query', type: 'integer', default: 100 }],
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results.length).toBe(3);
+    expect(results[0].path.join('.')).toBe('paths./test1.get.parameters.0.type');
+    expect(results[1].path.join('.')).toBe('paths./test2.get.parameters.0.required');
+    expect(results[2].path.join('.')).toBe('paths./test3.post.parameters.0.default');
+  });
+});
+
+test('az-pagination-parameters should find errors in skip parameter', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test1': {
+        get: {
+          parameters: [{ name: 'skip', in: 'query', type: 'string', default: 0 }],
+        },
+      },
+      '/test2': {
+        get: {
+          parameters: [{ name: 'skip', in: 'query', type: 'integer', default: 0, required: true }],
+        },
+      },
+      '/test3': {
+        post: {
+          parameters: [{ name: 'skip', in: 'query', type: 'integer', default: 100 }],
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results.length).toBe(3);
+    expect(results[0].path.join('.')).toBe('paths./test1.get.parameters.0.type');
+    expect(results[1].path.join('.')).toBe('paths./test2.get.parameters.0.required');
+    expect(results[2].path.join('.')).toBe('paths./test3.post.parameters.0.default');
+  });
+});
+
+test('az-pagination-parameters should find errors in maxpagesize parameter', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test0': {
+        get: {
+          parameters: [{ name: 'maxPageSize', in: 'query', type: 'integer' }],
+        },
+      },
+      '/test1': {
+        get: {
+          parameters: [{ name: 'maxpagesize', in: 'query', type: 'string' }],
+        },
+      },
+      '/test2': {
+        get: {
+          parameters: [{ name: 'maxpagesize', in: 'query', type: 'integer', required: true }],
+        },
+      },
+      '/test3': {
+        post: {
+          parameters: [{ name: 'maxpagesize', in: 'query', type: 'integer', default: 100 }],
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results.length).toBe(4);
+    expect(results[0].path.join('.')).toBe('paths./test0.get.parameters.0.name');
+    expect(results[1].path.join('.')).toBe('paths./test1.get.parameters.0.type');
+    expect(results[2].path.join('.')).toBe('paths./test2.get.parameters.0.required');
+    expect(results[3].path.join('.')).toBe('paths./test3.post.parameters.0.default');
+  });
+});
+
+test('az-pagination-parameters should find errors in filter parameter', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test1': {
+        get: {
+          parameters: [{ name: 'filter', in: 'query', type: 'integer' }],
+        },
+      },
+      '/test2': {
+        get: {
+          parameters: [{ name: 'filter', in: 'query', type: 'string', required: true }],
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results.length).toBe(2);
+    expect(results[0].path.join('.')).toBe('paths./test1.get.parameters.0.type');
+    expect(results[1].path.join('.')).toBe('paths./test2.get.parameters.0.required');
+  });
+});
+
+test('az-pagination-parameters should find errors in orderby parameter', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test0': {
+        get: {
+          parameters: [{ name: 'orderBy', in: 'query', type: 'array', items: { type: 'string' } }],
+        },
+      },
+      '/test1': {
+        get: {
+          parameters: [{ name: 'orderby', in: 'query', type: 'string' }],
+        },
+      },
+      '/test2': {
+        get: {
+          parameters: [{ name: 'orderby', in: 'query', type: 'array', items: { type: 'string' }, required: true }],
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results.length).toBe(3);
+    expect(results[0].path.join('.')).toBe('paths./test0.get.parameters.0.name');
+    expect(results[1].path.join('.')).toBe('paths./test1.get.parameters.0.type');
+    expect(results[2].path.join('.')).toBe('paths./test2.get.parameters.0.required');
+  });
+});
+
+// Test for errors in the select parameter
+test('az-pagination-parameters should find errors in select parameter', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test1': {
+        get: {
+          parameters: [{ name: 'select', in: 'query', type: 'integer' }],
+        },
+      },
+      '/test2': {
+        get: {
+          parameters: [{ name: 'select', in: 'query', type: 'array', items: { type: 'string' }, required: true }],
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results.length).toBe(2);
+    expect(results[0].path.join('.')).toBe('paths./test1.get.parameters.0.type');
+    expect(results[1].path.join('.')).toBe('paths./test2.get.parameters.0.required');
+  });
+});
+
+// Test for errors in the expand parameter
+test('az-pagination-parameters should find errors in expand parameter', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test1': {
+        get: {
+          parameters: [{ name: 'expand', in: 'query', type: 'integer' }],
+        },
+      },
+      '/test2': {
+        get: {
+          parameters: [{ name: 'expand', in: 'query', type: 'array', items: { type: 'string' }, required: true }],
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results.length).toBe(2);
+    expect(results[0].path.join('.')).toBe('paths./test1.get.parameters.0.type');
+    expect(results[1].path.join('.')).toBe('paths./test2.get.parameters.0.required');
+  });
+});
+
+test('az-pagination-parameters should find no errors', () => {
+  const oasDoc = {
+    swagger: '2.0',
+    paths: {
+      '/test1': {
+        get: {
+          parameters: [
+            { name: 'top', in: 'query', type: 'integer' },
+            { name: 'skip', in: 'query', type: 'integer', default: 0 },
+            { name: 'maxpagesize', in: 'query', type: 'integer' },
+            { name: 'filter', in: 'query', type: 'string' },
+            { name: 'select', in: 'query', type: 'array', items: { type: 'string' } },
+            { name: 'expand', in: 'query', type: 'array', items: { type: 'string' } },
+            { name: 'orderby', in: 'query', type: 'array', items: { type: 'string' } },
+          ],
+        },
+      },
+      '/test2': {
+        post: {
+          parameters: [
+            { name: 'top', in: 'query', type: 'integer' },
+            { name: 'skip', in: 'query', type: 'integer', default: 0 },
+            { name: 'maxpagesize', in: 'query', type: 'integer' },
+            { name: 'filter', in: 'query', type: 'string' },
+            { name: 'select', in: 'query', type: 'array', items: { type: 'string' } },
+            { name: 'expand', in: 'query', type: 'array', items: { type: 'string' } },
+            { name: 'orderby', in: 'query', type: 'array', items: { type: 'string' } },
+          ],
+        },
+      },
+    },
+  };
+  return linter.run(oasDoc).then((results) => {
+    expect(results.length).toBe(0);
+  });
+});