From 10bc766c617ed4b42f83eedc716184deb73e2376 Mon Sep 17 00:00:00 2001 From: Mike Kistler Date: Tue, 10 May 2022 15:03:33 -0500 Subject: [PATCH] Add az-pagination-parameters rule --- docs/azure-ruleset.md | 8 + functions/pagination-parameters.js | 205 +++++++++++++++++++++++++ openapi-style-guide.md | 10 +- spectral.yaml | 11 ++ test/pagination-parameters.test.js | 239 +++++++++++++++++++++++++++++ 5 files changed, 470 insertions(+), 3 deletions(-) create mode 100644 functions/pagination-parameters.js create mode 100644 test/pagination-parameters.test.js diff --git a/docs/azure-ruleset.md b/docs/azure-ruleset.md index 78e1011..1ce62b0 100644 --- a/docs/azure-ruleset.md +++ b/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'. diff --git a/functions/pagination-parameters.js b/functions/pagination-parameters.js new file mode 100644 index 0000000..dc08470 --- /dev/null +++ b/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; +}; diff --git a/openapi-style-guide.md b/openapi-style-guide.md index 53e1624..c56a7d3 100644 --- a/openapi-style-guide.md +++ b/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 the operation has a `top` parameter, it must be an integer, optional, and have a documented default and maximum value -- If the operation has a `maxpagesize` parameter, it must be an integer, optional, and have a documented default and maximum value +- If present, the `top` parameter must be an integer, optional, with no default value +- If present, the `skip` parameter must be an integer, optional, with a default value of 0 +- 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 diff --git a/spectral.yaml b/spectral.yaml index 041124d..a4eb624 100644 --- a/spectral.yaml +++ b/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}}' diff --git a/test/pagination-parameters.test.js b/test/pagination-parameters.test.js new file mode 100644 index 0000000..42f5216 --- /dev/null +++ b/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); + }); +});