diff --git a/tools/spectral/ipa/__tests__/createMethodShouldNotHaveQueryParameters.test.js b/tools/spectral/ipa/__tests__/createMethodShouldNotHaveQueryParameters.test.js index b6244093ed..59ce2f3081 100644 --- a/tools/spectral/ipa/__tests__/createMethodShouldNotHaveQueryParameters.test.js +++ b/tools/spectral/ipa/__tests__/createMethodShouldNotHaveQueryParameters.test.js @@ -120,20 +120,19 @@ testRule('xgen-IPA-106-create-method-should-not-have-query-parameters', [ errors: [ { code: 'xgen-IPA-106-create-method-should-not-have-query-parameters', - message: 'Input parameter [filter]: Create operations should not have query parameters. http://go/ipa/106', + message: 'Create operations should not have query parameters. Found [filter]. http://go/ipa/106', path: ['paths', '/resource', 'post'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-106-create-method-should-not-have-query-parameters', - message: 'Input parameter [query-param]: Create operations should not have query parameters. http://go/ipa/106', + message: 'Create operations should not have query parameters. Found [query-param]. http://go/ipa/106', path: ['paths', '/resourceTwo', 'post'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-106-create-method-should-not-have-query-parameters', - message: - 'Input parameter [query-param-2]: Create operations should not have query parameters. http://go/ipa/106', + message: 'Create operations should not have query parameters. Found [query-param-2]. http://go/ipa/106', path: ['paths', '/resourceTwo', 'post'], severity: DiagnosticSeverity.Warning, }, diff --git a/tools/spectral/ipa/__tests__/updateMethodMustNotHaveQueryParams.test.js b/tools/spectral/ipa/__tests__/updateMethodMustNotHaveQueryParams.test.js new file mode 100644 index 0000000000..41e53d885f --- /dev/null +++ b/tools/spectral/ipa/__tests__/updateMethodMustNotHaveQueryParams.test.js @@ -0,0 +1,315 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const componentSchemas = { + schemas: { + Schema: { + type: 'object', + }, + }, + parameters: { + QueryParam: { + name: 'query-param', + in: 'query', + schema: { + type: 'string', + }, + }, + QueryParam2: { + name: 'query-param-2', + in: 'query', + schema: { + type: 'string', + }, + }, + PathParam: { + name: 'resource-id', + in: 'path', + schema: { + type: 'string', + }, + }, + envelope: { + name: 'envelope', + in: 'query', + }, + pretty: { + name: 'pretty', + in: 'query', + }, + }, +}; + +testRule('xgen-IPA-107-put-must-not-have-query-params', [ + { + name: 'valid put', + document: { + components: componentSchemas, + paths: { + '/resource/{id}': { + put: { + parameters: [ + { + name: 'header-param', + in: 'header', + schema: { type: 'string' }, + }, + { + name: 'resource-id', + in: 'path', + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + { + $ref: '#/components/parameters/PathParam', + }, + { + $ref: '#/components/parameters/envelope', + }, + { + $ref: '#/components/parameters/pretty', + }, + ], + }, + }, + '/resource/{id}/singleton': { + put: { + parameters: [], + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid put', + document: { + components: componentSchemas, + paths: { + '/resource/{id}': { + put: { + parameters: [ + { + name: 'filter', + in: 'query', + schema: { type: 'string' }, + }, + ], + }, + }, + '/resource/{id}/singleton': { + put: { + parameters: [ + { + name: 'header-param', + in: 'header', + schema: { type: 'string' }, + }, + { + $ref: '#/components/parameters/QueryParam', + }, + { + $ref: '#/components/parameters/QueryParam2', + }, + ], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-107-put-must-not-have-query-params', + message: 'Update operations must not have query parameters. Found [filter]. http://go/ipa-spectral#IPA-107', + path: ['paths', '/resource/{id}', 'put'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-put-must-not-have-query-params', + message: + 'Update operations must not have query parameters. Found [query-param]. http://go/ipa-spectral#IPA-107', + path: ['paths', '/resource/{id}/singleton', 'put'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-put-must-not-have-query-params', + message: + 'Update operations must not have query parameters. Found [query-param-2]. http://go/ipa-spectral#IPA-107', + path: ['paths', '/resource/{id}/singleton', 'put'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid put with exceptions', + document: { + components: componentSchemas, + paths: { + '/resource/{id}': { + put: { + parameters: [ + { + name: 'filter', + in: 'query', + schema: { type: 'string' }, + }, + ], + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-put-must-not-have-query-params': 'Reason', + }, + }, + }, + '/resource/{id}/singleton': { + put: { + parameters: [ + { + $ref: '#/components/parameters/QueryParam', + }, + ], + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-put-must-not-have-query-params': 'Reason', + }, + }, + }, + }, + }, + errors: [], + }, +]); + +testRule('xgen-IPA-107-patch-must-not-have-query-params', [ + { + name: 'valid patch', + document: { + components: componentSchemas, + paths: { + '/resource/{id}': { + patch: { + parameters: [ + { + name: 'header-param', + in: 'header', + schema: { type: 'string' }, + }, + { + name: 'resource-id', + in: 'path', + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + { + $ref: '#/components/parameters/PathParam', + }, + { + $ref: '#/components/parameters/envelope', + }, + { + $ref: '#/components/parameters/pretty', + }, + ], + }, + }, + '/resource/{id}/singleton': { + patch: { + parameters: [], + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid patch', + document: { + components: componentSchemas, + paths: { + '/resource/{id}': { + patch: { + parameters: [ + { + name: 'filter', + in: 'query', + schema: { type: 'string' }, + }, + ], + }, + }, + '/resource/{id}/singleton': { + patch: { + parameters: [ + { + name: 'header-param', + in: 'header', + schema: { type: 'string' }, + }, + { + $ref: '#/components/parameters/QueryParam', + }, + { + $ref: '#/components/parameters/QueryParam2', + }, + ], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-107-patch-must-not-have-query-params', + message: 'Update operations must not have query parameters. Found [filter]. http://go/ipa-spectral#IPA-107', + path: ['paths', '/resource/{id}', 'patch'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-patch-must-not-have-query-params', + message: + 'Update operations must not have query parameters. Found [query-param]. http://go/ipa-spectral#IPA-107', + path: ['paths', '/resource/{id}/singleton', 'patch'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-patch-must-not-have-query-params', + message: + 'Update operations must not have query parameters. Found [query-param-2]. http://go/ipa-spectral#IPA-107', + path: ['paths', '/resource/{id}/singleton', 'patch'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid patch with exceptions', + document: { + components: componentSchemas, + paths: { + '/resource/{id}': { + patch: { + parameters: [ + { + name: 'filter', + in: 'query', + schema: { type: 'string' }, + }, + ], + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-patch-must-not-have-query-params': 'Reason', + }, + }, + }, + '/resource/{id}/singleton': { + patch: { + parameters: [ + { + $ref: '#/components/parameters/QueryParam', + }, + ], + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-patch-must-not-have-query-params': 'Reason', + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/ipa-spectral.yaml b/tools/spectral/ipa/ipa-spectral.yaml index 03bbbbe660..582cf47cef 100644 --- a/tools/spectral/ipa/ipa-spectral.yaml +++ b/tools/spectral/ipa/ipa-spectral.yaml @@ -4,6 +4,7 @@ extends: - ./rulesets/IPA-104.yaml - ./rulesets/IPA-105.yaml - ./rulesets/IPA-106.yaml + - ./rulesets/IPA-107.yaml - ./rulesets/IPA-108.yaml - ./rulesets/IPA-109.yaml - ./rulesets/IPA-113.yaml diff --git a/tools/spectral/ipa/rulesets/IPA-005.yaml b/tools/spectral/ipa/rulesets/IPA-005.yaml index 5bdc7710e1..92d0cc8983 100644 --- a/tools/spectral/ipa/rulesets/IPA-005.yaml +++ b/tools/spectral/ipa/rulesets/IPA-005.yaml @@ -7,7 +7,7 @@ functions: rules: xgen-IPA-005-exception-extension-format: description: | - IPA exception extensions must follow the correct format. http://go/ipa/5 + IPA exception extensions must follow the correct format. ##### Implementation details Rule checks for the following conditions: diff --git a/tools/spectral/ipa/rulesets/IPA-105.yaml b/tools/spectral/ipa/rulesets/IPA-105.yaml index 8e46f25bcb..74b7f417a2 100644 --- a/tools/spectral/ipa/rulesets/IPA-105.yaml +++ b/tools/spectral/ipa/rulesets/IPA-105.yaml @@ -32,7 +32,7 @@ rules: function: 'eachResourceHasListMethod' xgen-IPA-105-list-method-response-is-get-method-response: description: >- - The response body of the List method should consist of the same resource object returned by the Get method. http://go/ipa/105 + The response body of the List method should consist of the same resource object returned by the Get method. ##### Implementation details diff --git a/tools/spectral/ipa/rulesets/IPA-106.yaml b/tools/spectral/ipa/rulesets/IPA-106.yaml index d04cec321d..458c0e93c3 100644 --- a/tools/spectral/ipa/rulesets/IPA-106.yaml +++ b/tools/spectral/ipa/rulesets/IPA-106.yaml @@ -28,6 +28,8 @@ rules: given: '$.paths[*].post' then: function: 'createMethodShouldNotHaveQueryParameters' + functionOptions: + ignoredValues: ['pretty', 'envelope'] xgen-IPA-106-create-method-request-body-is-get-method-response: description: >- Request body content of the Create method and response content of the Get method should refer to the same resource. http://go/ipa/106 diff --git a/tools/spectral/ipa/rulesets/IPA-107.yaml b/tools/spectral/ipa/rulesets/IPA-107.yaml new file mode 100644 index 0000000000..3d14c1b57c --- /dev/null +++ b/tools/spectral/ipa/rulesets/IPA-107.yaml @@ -0,0 +1,41 @@ +# IPA-107: Update +# http://go/ipa/107 + +functions: + - updateMethodMustNotHaveQueryParams + +rules: + xgen-IPA-107-put-must-not-have-query-params: + description: >- + Update operations must not accept query parameters. + + ##### Implementation details + + Validation checks the PUT method for single resource paths and singleton resources. + + - Query parameters `envelope` and `pretty` are exempt from this rule + - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation + message: '{{error}} http://go/ipa-spectral#IPA-107' + severity: warn + given: '$.paths[*].put' + then: + function: 'updateMethodMustNotHaveQueryParams' + functionOptions: + ignoredValues: ['pretty', 'envelope'] + xgen-IPA-107-patch-must-not-have-query-params: + description: >- + Update operations must not accept query parameters. + + ##### Implementation details + + Validation checks the PATCH method for single resource paths and singleton resources. + + - Query parameters `envelope` and `pretty` are exempt from this rule + - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation + message: '{{error}} http://go/ipa-spectral#IPA-107' + severity: warn + given: '$.paths[*].patch' + then: + function: 'updateMethodMustNotHaveQueryParams' + functionOptions: + ignoredValues: ['pretty', 'envelope'] diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 56c1dab17d..eba172975b 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -15,7 +15,7 @@ Rule is based on [http://go/ipa/IPA-5](http://go/ipa/IPA-5). #### xgen-IPA-005-exception-extension-format ![error](https://img.shields.io/badge/error-red) -IPA exception extensions must follow the correct format. http://go/ipa/5 +IPA exception extensions must follow the correct format. ##### Implementation details Rule checks for the following conditions: @@ -101,11 +101,10 @@ The List method request must not include a body. http://go/ipa/105 ![warn](https://img.shields.io/badge/warning-yellow) APIs must provide a List method for resources. http://go/ipa/105 - #### xgen-IPA-105-list-method-response-is-get-method-response ![warn](https://img.shields.io/badge/warning-yellow) -The response body of the List method should consist of the same resource object returned by the Get method. http://go/ipa/105 +The response body of the List method should consist of the same resource object returned by the Get method. ##### Implementation details Validation checks that the List method response contains items property with reference to the same schema as the Get method response. @@ -143,6 +142,30 @@ Create method Request object must not include fields with readOnly:true. http:// Create methods must return a 201 Created response code. http://go/ipa/106 This rule applies only to POST requests targeting resource collection URIs. +### IPA-107 + +Rule is based on [http://go/ipa/IPA-107](http://go/ipa/IPA-107). + +#### xgen-IPA-107-put-must-not-have-query-params + + ![warn](https://img.shields.io/badge/warning-yellow) +Update operations must not accept query parameters. +##### Implementation details +Validation checks the PUT method for single resource paths and singleton resources. + + - Query parameters `envelope` and `pretty` are exempt from this rule + - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation +#### xgen-IPA-107-patch-must-not-have-query-params + + ![warn](https://img.shields.io/badge/warning-yellow) +Update operations must not accept query parameters. +##### Implementation details +Validation checks the PATCH method for single resource paths and singleton resources. + + - Query parameters `envelope` and `pretty` are exempt from this rule + - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation + + ### IPA-108 Rule is based on [http://go/ipa/IPA-108](http://go/ipa/IPA-108). diff --git a/tools/spectral/ipa/rulesets/functions/createMethodShouldNotHaveQueryParameters.js b/tools/spectral/ipa/rulesets/functions/createMethodShouldNotHaveQueryParameters.js index f4229e01de..eac26dc20b 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodShouldNotHaveQueryParameters.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodShouldNotHaveQueryParameters.js @@ -15,9 +15,14 @@ import { const RULE_NAME = 'xgen-IPA-106-create-method-should-not-have-query-parameters'; const ERROR_MESSAGE = 'Create operations should not have query parameters.'; -const ignoredParameters = ['envelope', 'pretty']; - -export default (input, _, { path, documentInventory }) => { +/** + * Create operations should not have query parameters. + * + * @param {object} input - The create operation object + * @param {{ignoredValues: string[]}} opts - Array of ignored query parameters + * @param {object} context - The context object containing the path and document + */ +export default (input, opts, { path, documentInventory }) => { const resourcePath = path[1]; const oas = documentInventory.resolved; const resourcePaths = getResourcePathItems(resourcePath, oas.paths); @@ -37,21 +42,23 @@ export default (input, _, { path, documentInventory }) => { return; } - const errors = checkViolationsAndReturnErrors(postMethod.parameters, path); + const errors = checkViolationsAndReturnErrors(postMethod.parameters, path, opts); if (errors.length !== 0) { return collectAndReturnViolation(path, RULE_NAME, errors); } collectAdoption(path, RULE_NAME); }; -function checkViolationsAndReturnErrors(postMethodParameters, path) { +function checkViolationsAndReturnErrors(postMethodParameters, path, opts) { const errors = []; try { + const ignoredValues = opts?.ignoredValues || []; + for (const parameter of postMethodParameters) { - if (parameter.in === 'query' && !ignoredParameters.includes(parameter.name)) { + if (parameter.in === 'query' && !ignoredValues.includes(parameter.name)) { errors.push({ path: path, - message: `Input parameter [${parameter.name}]: ${ERROR_MESSAGE}`, + message: `${ERROR_MESSAGE} Found [${parameter.name}].`, }); } } diff --git a/tools/spectral/ipa/rulesets/functions/updateMethodMustNotHaveQueryParams.js b/tools/spectral/ipa/rulesets/functions/updateMethodMustNotHaveQueryParams.js new file mode 100644 index 0000000000..35e9a8f557 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/updateMethodMustNotHaveQueryParams.js @@ -0,0 +1,70 @@ +import { hasException } from './utils/exceptions.js'; +import { + collectAdoption, + collectAndReturnViolation, + collectException, + handleInternalError, +} from './utils/collectionUtils.js'; +import { + getResourcePathItems, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, + isSingletonResource, +} from './utils/resourceEvaluation.js'; + +const ERROR_MESSAGE = 'Update operations must not have query parameters.'; + +/** + * Update operations must not have query parameters. + * + * @param {object} input - The update operation object + * @param {{ignoredValues: string[]}} opts - Array of ignored query parameters + * @param {object} context - The context object containing the path, document and rule + */ +export default (input, opts, { path, documentInventory, rule }) => { + const resourcePath = path[1]; + const oas = documentInventory.resolved; + const resourcePathItems = getResourcePathItems(resourcePath, oas.paths); + const ruleName = rule.name; + + if ( + !isSingleResourceIdentifier(resourcePath) && + !(isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePathItems)) + ) { + return; + } + + if (!input.parameters || input.parameters.length === 0) { + return; + } + + if (hasException(input, ruleName)) { + collectException(input, ruleName, path); + return; + } + + const errors = checkViolationsAndReturnErrors(input.parameters, path, ruleName, opts); + if (errors.length !== 0) { + return collectAndReturnViolation(path, ruleName, errors); + } + collectAdoption(path, ruleName); +}; + +function checkViolationsAndReturnErrors(postMethodParameters, path, ruleName, opts) { + const errors = []; + try { + const ignoredValues = opts?.ignoredValues || []; + + for (const parameter of postMethodParameters) { + if (parameter.in === 'query' && !ignoredValues.includes(parameter.name)) { + errors.push({ + path: path, + message: `${ERROR_MESSAGE} Found [${parameter.name}].`, + }); + } + } + return errors; + } catch (e) { + handleInternalError(ruleName, path, e); + } +}