Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ibm-request-and-response-content): exempt minimally represented resource PUTs #659

Merged
merged 1 commit into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/ruleset/src/functions/api-symmetry.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2023 IBM Corporation.
* Copyright 2023 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

Expand Down
1 change: 1 addition & 0 deletions packages/ruleset/src/functions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ module.exports = {
propertyNameCollision: require('./property-name-collision'),
refPattern: require('./ref-pattern'),
refSiblingDuplicateDescription: require('./ref-sibling-duplicate-description'),
requestAndResponseContent: require('./request-and-response-content'),
requestBodyNameExists: require('./requestbody-name-exists'),
requiredProperty: require('./required-property'),
resourceResponseConsistency: require('./resource-response-consistency'),
Expand Down
93 changes: 93 additions & 0 deletions packages/ruleset/src/functions/request-and-response-content.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* Copyright 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const { isObject } = require('@ibm-cloud/openapi-ruleset-utilities');
const {
LoggerFactory,
pathHasMinimallyRepresentedResource,
} = require('../utils');

let ruleId;
let logger;

/**
* The implementation for this rule makes assumptions that are dependent on the
* presence of the following other rules:
*
* ibm-operation-responses - all operations define a response
*
*/

module.exports = function requestAndResponseContent(
operation,
options,
context
) {
if (!logger) {
ruleId = context.rule.name;
logger = LoggerFactory.getInstance().getLogger(ruleId);
}
return checkForContent(
operation,
context.path,
context.documentInventory.resolved
);
};

/**
* This function checks to ensure an operation defines "content" on its
* requestBody and responses, when appropriate.
*
* @param {*} operation - openapi operation object
* @param {*} path - path to current openapi artifact, as a list
* @param {*} apidef - resolved api spec
* @returns an array containing the violations found or [] if no violations
*/
function checkForContent(operation, path, apidef) {
const errors = [];

// Check request body for every operation.
if (
isObject(operation.requestBody) &&
!isObject(operation.requestBody.content)
) {
errors.push({
message: '', // Use rule-defined message.
path: [...path, 'requestBody'],
});
}

// Check response bodies for the following operations.
if (
['get', 'post', 'put', 'patch', 'delete'].includes(path.at(-1)) &&
isObject(operation.responses)
) {
for (const [statusCode, response] of Object.entries(operation.responses)) {
// Skip exempt status codes.
if (['204', '202', '101', '304'].includes(statusCode)) {
continue;
}

// A PUT operation for a minimally represented resource (i.e. GET returns a 204)
// is allowed to define a 201 response with no content.
if (
path.at(-1) === 'put' &&
statusCode === '201' &&
pathHasMinimallyRepresentedResource(path.at(-2), apidef, logger, ruleId)
) {
continue;
}

if (!isObject(response.content)) {
errors.push({
message: '',
path: [...path, 'responses', statusCode],
});
}
}
}

return errors;
}
33 changes: 7 additions & 26 deletions packages/ruleset/src/functions/response-status-codes.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

Expand All @@ -9,6 +9,7 @@ const {
isOperationOfType,
getResourceSpecificSiblingPath,
getResponseCodes,
pathHasMinimallyRepresentedResource,
} = require('../utils');

let ruleId;
Expand Down Expand Up @@ -184,30 +185,10 @@ function hasBodyRepresentation(path, apidef) {
return;
}

const resourceGetOperation = apidef.paths[resourceSpecificPath].get;
if (!resourceGetOperation) {
logger.debug(
`${ruleId}: no GET operation found at path "${resourceSpecificPath}"`
);
return;
}

if (!resourceGetOperation.responses) {
logger.debug(
`${ruleId}: no responses defined on GET operation at path "${resourceSpecificPath}"`
);
return;
}

const [, getOpSuccessCodes] = getResponseCodes(
resourceGetOperation.responses
return !pathHasMinimallyRepresentedResource(
resourceSpecificPath,
apidef,
logger,
ruleId
);

logger.debug(
`${ruleId}: corresponding GET operation has the following status codes: ${getOpSuccessCodes.join(
', '
)}`
);

return !getOpSuccessCodes.includes('204');
}
2 changes: 1 addition & 1 deletion packages/ruleset/src/ibm-oas.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ module.exports = {
ibmRules.propertyConsistentNameAndType,
'ibm-property-description': ibmRules.propertyDescriptionExists,
'ibm-ref-pattern': ibmRules.refPattern,
'ibm-request-and-response-content': ibmRules.contentExists,
'ibm-request-and-response-content': ibmRules.requestAndResponseContent,
'ibm-requestbody-is-object': ibmRules.requestBodyIsObject,
'ibm-requestbody-name': ibmRules.requestBodyNameExists,
'ibm-resource-response-consistency': ibmRules.resourceResponseConsistency,
Expand Down
23 changes: 0 additions & 23 deletions packages/ruleset/src/rules/content-exists.js

This file was deleted.

2 changes: 1 addition & 1 deletion packages/ruleset/src/rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ module.exports = {
collectionArrayProperty: require('./collection-array-property'),
consecutivePathSegments: require('./consecutive-path-segments'),
contentContainsSchema: require('./content-contains-schema'),
contentExists: require('./content-exists'),
contentTypeHeader: require('./content-type-header'),
contentTypeIsSpecific: require('./content-type-is-specific'),
deleteBody: require('./delete-body'),
Expand Down Expand Up @@ -61,6 +60,7 @@ module.exports = {
propertyNameCollision: require('./property-name-collision'),
refPattern: require('./ref-pattern'),
refSiblingDuplicateDescription: require('./ref-sibling-duplicate-description'),
requestAndResponseContent: require('./request-and-response-content'),
requestBodyIsObject: require('./requestbody-is-object'),
requestBodyNameExists: require('./requestbody-name-exists'),
requiredPropertyMissing: require('./required-property-missing'),
Expand Down
22 changes: 22 additions & 0 deletions packages/ruleset/src/rules/request-and-response-content.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const {
operations,
} = require('@ibm-cloud/openapi-ruleset-utilities/src/collections');
const { oas3 } = require('@stoplight/spectral-formats');
const { requestAndResponseContent } = require('../functions');

module.exports = {
description:
'Request bodies and non-204 responses should define a content object',
given: operations,
severity: 'warn',
formats: [oas3],
resolved: true,
then: {
function: requestAndResponseContent,
},
};
3 changes: 2 additions & 1 deletion packages/ruleset/src/utils/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

Expand All @@ -19,6 +19,7 @@ module.exports = {
LoggerFactory: require('./logger-factory'),
mergeAllOfSchemaProperties: require('./merge-allof-schema-properties'),
operationMethods: require('./constants'),
pathHasMinimallyRepresentedResource: require('./path-has-minimally-represented-resource'),
pathMatchesRegexp: require('./path-matches-regexp'),
...require('./mimetype-utils'),
...require('./pagination-utils'),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* Copyright 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const getResponseCodes = require('./get-response-codes');

/**
* This function checks for a "minimally represented resource" (i.e. a resource
* with no body representation) path. The path is considered to have a minimally
* represented resource if the GET request on the path returns a 204 response.
*
* @param {string} path - the resource-specific path to check
* @param {*} apidef - resolved api spec
* @param {*} logger - logger object passed from rule, to maintain scope
* @param {string} ruleId - name of current rule, to maintain scope
* @returns {bool} true if the resource on this path is minimally represented
*/
function pathHasMinimallyRepresentedResource(path, apidef, logger, ruleId) {
// This should only be true when the path is resource-specific
// (i.e. ends with a path parameter).
if (!path.endsWith('}')) {
logger.debug(`${ruleId}: path "${path}" is not for a specific resource`);
return false;
}

const pathItem = apidef.paths[path];
if (!pathItem) {
logger.debug(`${ruleId}: path "${path}" does not exist`);
return false;
}

const resourceGetOperation = pathItem.get;
if (!resourceGetOperation) {
logger.debug(`${ruleId}: no GET operation found at path "${path}"`);
return false;
}

if (!resourceGetOperation.responses) {
logger.debug(
`${ruleId}: no responses defined on GET operation at path "${path}"`
);
return false;
}

const [, getOpSuccessCodes] = getResponseCodes(
resourceGetOperation.responses
);

logger.debug(
`${ruleId}: corresponding GET operation has the following status codes: ${getOpSuccessCodes.join(
', '
)}`
);

return getOpSuccessCodes.includes('204');
}

module.exports = pathHasMinimallyRepresentedResource;
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const { contentExists } = require('../src/rules');
const { requestAndResponseContent } = require('../src/rules');
const { makeCopy, rootDocument, testRule, severityCodes } = require('./utils');

const ruleId = 'ibm-request-and-response-content';
const rule = contentExists;
const rule = requestAndResponseContent;

describe(`Spectral rule: ${ruleId}`, () => {
it('should not error with a clean spec', async () => {
Expand Down Expand Up @@ -121,6 +121,31 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(0);
});

it('should not error if 201 response for PUT on minimally represented resource is missing content', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.paths['/v1/movies/{id}/genres/{genre}'] = {
get: {
operationId: 'check_movie_genre',
responses: {
204: {
description: 'No content - checked',
},
},
},
put: {
operationId: 'add_movie_genre',
responses: {
201: {
description: 'No content - created',
},
},
},
};

const results = await testRule(ruleId, rule, testDocument);
expect(results).toHaveLength(0);
});

it('should error if 201 response is missing content', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.paths['/v1/movies'].post = {
Expand Down