Skip to content

Commit

Permalink
chore: apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Phil Adams <[email protected]>
Signed-off-by: Dustin Popp <[email protected]>
  • Loading branch information
dpopp07 and padamstx committed Sep 8, 2023
1 parent ec5b8c0 commit ac78dfe
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 14 deletions.
14 changes: 5 additions & 9 deletions packages/ruleset/src/functions/schema-naming-convention.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,17 +388,19 @@ function computeRefsAtPaths(nodes) {
* @returns {string} - the name of the referenced schema at a given path, or undefined
*/
function getSchemaNameAtPath(path, pathToReferencesMap) {
if (!path && typeof path !== 'string') {
if (!path || typeof path !== 'string') {
return;
}

// resolve path to reference - add comments explaining
// Build the path, replacing each path that resolves to a reference with the
// referenced path in order to match the expected format in the
// pathToReferencesMap (which comes from graph nodes that Spectral gives us).
// See the function documentation above for more info.
let pathBuilder = '';
for (const pathSegment of path.split('.')) {
if (pathBuilder) {
pathBuilder += '.';
}
pathBuilder += `${pathSegment}`;
const schemaReference = pathToReferencesMap[pathBuilder];

Expand All @@ -408,14 +410,8 @@ function getSchemaNameAtPath(path, pathToReferencesMap) {
if (schemaReference && pathSegment !== path.split('.').at(-1)) {
pathBuilder = schemaReference;
}

pathBuilder += '.';
}

// We lazily add a period character at the end of every segment
// during the loop. Remove the final period here.
pathBuilder = pathBuilder.slice(0, -1);

if (path !== pathBuilder) {
logger.debug(`${ruleId}: resolved path to be ${pathBuilder}`);
}
Expand All @@ -431,7 +427,7 @@ function getSchemaNameAtPath(path, pathToReferencesMap) {
* @returns {string} - the name of the schema (e.g. 'Thing')
*/
function getSchemaNameFromReference(reference) {
if (!reference && typeof reference !== 'string') {
if (!reference || typeof reference !== 'string') {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const { isObject } = require('@ibm-cloud/openapi-ruleset-utilities');
* "specific" resource path that is a sibling (e.g. /foo/{id}); if
* found, return the sibling path string.
*
* @param {*} path the array of path segments indicating the "location" of an operation within the API definition
* @param {*} path the path string (for a generic path e.g. '/foo')
* @param {*} apidef the resolved API spec, as an object
* @returns the specific resource path, as a string
*/
Expand Down
23 changes: 20 additions & 3 deletions packages/ruleset/src/utils/schema-finding-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,24 @@ function getSuccessResponseSchemaForOperation(operation, pathToOp) {

const [, successCodes] = getResponseCodes(operation.responses);

if (!successCodes || !successCodes.length || !successCodes.includes('200')) {
if (!successCodes || !successCodes.length) {
return info;
}

const successResponse = operation.responses['200'];
// Prefer 200 responses if present. If not, find the first response with a JSON schema.
const code = successCodes.includes('200')
? '200'
: successCodes.find(c => responseHasJsonSchema(operation.responses[c]));
if (!code) {
return info;
}

const successResponse = operation.responses[code];
if (!successResponse.content || !isObject(successResponse.content)) {
return info;
}

pathToSchema += '.200.content';
pathToSchema += `.${code}.content`;

// Find the first content object determined to be JSON -
// we are only interested in JSON content.
Expand All @@ -74,6 +82,15 @@ function getSuccessResponseSchemaForOperation(operation, pathToOp) {
return info;
}

function responseHasJsonSchema(response) {
return (
response.content &&
Object.keys(response.content).some(
c => isJsonMimeType(c) && response.content[c].schema
)
);
}

/**
* Takes an operation object/path combo and finds the JSON request body
* schema, if it exists. We're only interested in JSON content.
Expand Down
2 changes: 1 addition & 1 deletion packages/ruleset/test/schema-naming-convention.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(0);
});

it('Prototype schema in PUT request body is correctly named but the path is not resource-oriented', async () => {
it('Prototype schema in PUT request body is incorrectly named but the path is not resource-oriented', async () => {
const testDocument = makeCopy(rootDocument);

testDocument.components.schemas.Mess = {};
Expand Down

0 comments on commit ac78dfe

Please sign in to comment.