create_thing would be preferred over manufacture
when deciding on an operationId for the POST /v1/things operation.
Likewise, for the GET /v1/things/{thing_id} operation, we might prefer
get_thing over retrieve_thing for the operationId.
-This rule will analyze the operations, looking for operationId values that are not using the recommended verbs.
+
This rule will analyze the operations, looking for operationId values that are not using the recommended verbs. Furthermore it can also validate the complete name of the operation id by comparing it to the path segments.
+
+
+
+Configuration:
+This rule can be configured to validate the complete name of the operation id, or only the verb it begins with.
+The default configuration object provided in the rule definition is:
+
+{
+ strict: true
+}
+
+To switch off the complete name validation and only validate the verbs the operation ids begin with, you'll need to
+replace this rule with a new rule within your
+custom ruleset and modify the configuration such that the value of the strict field to false
+
+{
+ strict: false
+}
+
@@ -4410,6 +4429,10 @@ paths:
operationId: manufacture_thing
description: Create a new Thing instance.
summary: Create a Thing
+ get:
+ operationId: list_thing
+ description: List all Thing instances.
+ summary: List Thing
'/v1/things/{thing_id}':
get:
operationId: retrieve_thing
@@ -4428,6 +4451,10 @@ paths:
operationId: create_thing
description: Create a new Thing instance.
summary: Create a Thing
+ get:
+ operationId: list_things
+ description: List all Thing instances.
+ summery: List Things
'/v1/things/{thing_id}':
get:
operationId: get_thing
diff --git a/packages/ruleset/src/functions/operationid-naming-convention.js b/packages/ruleset/src/functions/operationid-naming-convention.js
index 6882fd55..e8785e56 100644
--- a/packages/ruleset/src/functions/operationid-naming-convention.js
+++ b/packages/ruleset/src/functions/operationid-naming-convention.js
@@ -7,11 +7,11 @@ const { each, merge, pickBy, reduce } = require('lodash');
const { operationMethods } = require('../utils');
const inflected = require('inflected');
-module.exports = function (rootDocument) {
- return operationIdNamingConvention(rootDocument);
+module.exports = function (rootDocument, options) {
+ return operationIdNamingConvention(rootDocument, options.strict);
};
-function operationIdNamingConvention(resolvedSpec) {
+function operationIdNamingConvention(resolvedSpec, fullNamingCheck) {
const operations = reduce(
resolvedSpec.paths,
(arr, path, pathKey) => {
@@ -59,23 +59,31 @@ function operationIdNamingConvention(resolvedSpec) {
p.startsWith(op.pathKey + '/{')
);
- const { checkPassed, correctIds, operationId } =
+ const { checkPassed, correctIds, operationId, verbs } =
operationIdPassedConventionCheck(
isResourceOriented,
op['opKey'],
op.operationId,
pathEndsWithParam,
numParamRefs,
- op.pathKey
+ op.pathKey,
+ fullNamingCheck
);
if (checkPassed === false) {
- errors.push({
- message: `operationIds should follow naming convention: operationId should be ${correctIds.join(
- ' or '
- )} but it's ${operationId} instead`,
- path: [...op.path, 'operationId'],
- });
+ if (fullNamingCheck) {
+ errors.push({
+ message: `operationIds should follow naming convention: operationId should be ${correctIds.join(
+ ' or '
+ )} but it's ${operationId} instead`,
+ path: [...op.path, 'operationId'],
+ });
+ } else {
+ errors.push({
+ message: `operationIds should follow naming convention: operationId verb should be ${verbs.join(' or ')}`,
+ path: [...op.path, 'operationId'],
+ });
+ }
}
}
});
@@ -139,7 +147,8 @@ function operationIdPassedConventionCheck(
operationId,
pathEndsWithParam,
numParamRefs,
- fullPath
+ fullPath,
+ fullNamingCheck
) {
// Useful for debugging.
// console.log(`Debug: ${httpMethod} ${isResourceOriented} ${pathEndsWithParam} ${numParamRefs} ${operationId}`);
@@ -202,27 +211,36 @@ function operationIdPassedConventionCheck(
// that the operationId starts with that verb
// and that the rest of the operation id matches
// the path according to the naming conventions
- const convertedPath = fullPath
- .replace(/^\/+/, '')
- .split('/')
- .filter(part => !part.startsWith('{') && !part.endsWith('}'))
- .filter(part => !/^v\d+$/.test(part));
-
- const isPlural = pluralVerbs.some(verb => verbs.includes(verb));
-
- // Singularize the words in the path according to the naming conventions.
- for (let i = 0; i < convertedPath.length; i++) {
- if (i !== convertedPath.length - 1 || !isPlural || pathEndsWithParam)
- convertedPath[i] = inflected.singularize(convertedPath[i]);
- }
+ if (fullNamingCheck) {
+ const convertedPath = fullPath
+ .replace(/^\/+/, '')
+ .split('/')
+ .filter(part => !part.startsWith('{') && !part.endsWith('}'))
+ .filter(part => !/^v\d+$/.test(part));
+
+ const isPlural = pluralVerbs.some(verb => verbs.includes(verb));
+
+ // Singularize the words in the path according to the naming conventions.
+ for (let i = 0; i < convertedPath.length; i++) {
+ if (i !== convertedPath.length - 1 || !isPlural || pathEndsWithParam)
+ convertedPath[i] = inflected.singularize(convertedPath[i]);
+ }
- const correctIds = [];
+ const correctIds = [];
- for (let i = 0; i < verbs.length; i++) {
- const correctId = verbs[i] + '_' + convertedPath.join('_');
- if (correctId === operationId) return { checkPassed: true };
- else correctIds.push(correctId);
- }
+ for (let i = 0; i < verbs.length; i++) {
+ const correctId = verbs[i] + '_' + convertedPath.join('_');
+ if (correctId === operationId) return { checkPassed: true };
+ else correctIds.push(correctId);
+ }
- return { checkPassed: false, correctIds, operationId };
+ return { checkPassed: false, correctIds, operationId };
+ } else {
+ if (verbs.length > 0) {
+ const checkPassed = verbs
+ .map(verb => operationId.startsWith(verb))
+ .some(v => v);
+ return { checkPassed, verbs: verbs };
+ }
+ }
}
diff --git a/packages/ruleset/src/rules/operationid-naming-convention.js b/packages/ruleset/src/rules/operationid-naming-convention.js
index a8f355ee..dc33e431 100644
--- a/packages/ruleset/src/rules/operationid-naming-convention.js
+++ b/packages/ruleset/src/rules/operationid-naming-convention.js
@@ -15,5 +15,8 @@ module.exports = {
resolved: true,
then: {
function: operationIdNamingConvention,
+ functionOptions: {
+ strict: true,
+ },
},
};
diff --git a/packages/ruleset/test/rules/operationid-naming-convention.test.js b/packages/ruleset/test/rules/operationid-naming-convention.test.js
index 3268b289..10ab2c9a 100644
--- a/packages/ruleset/test/rules/operationid-naming-convention.test.js
+++ b/packages/ruleset/test/rules/operationid-naming-convention.test.js
@@ -14,8 +14,10 @@ const {
const rule = operationIdNamingConvention;
const ruleId = 'ibm-operation-id-naming-convention';
const expectedSeverity = severityCodes.warning;
-const expectedMsgPrefix =
+const expectedStrictMsgPrefix =
/^operationIds should follow naming convention: operationId should be.*$/;
+const expectedNotStrictMsgPrefix =
+ /^operationIds should follow naming convention: operationId verb should be.*$/;
describe(`Spectral rule: ${ruleId}`, () => {
describe('Should not yield errors', () => {
@@ -204,7 +206,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*list_drinks*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe('paths./v1/drinks.get.operationId');
@@ -220,7 +222,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*get_drink*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe(
@@ -241,7 +243,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*get_drink_glass or check_drink_glass*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe(
@@ -257,7 +259,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*create_drink*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe('paths./v1/drinks.post.operationId');
@@ -274,7 +276,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*create_drink*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe(
@@ -293,7 +295,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*update_drink*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe(
@@ -312,7 +314,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*replace_drink*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe(
@@ -331,7 +333,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*replace_drinks*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe('paths./v1/drinks.put.operationId');
@@ -348,7 +350,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*replace_drink*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe(
@@ -369,7 +371,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(
/^.*replace_drink_glasses or set_drink_glasses*/
);
@@ -392,7 +394,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*replace_drink_glass or add_drink_glass*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe(
@@ -411,7 +413,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*delete_drink*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe(
@@ -432,7 +434,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(
/^.*delete_drink_glasses or unset_drink_glasses*/
);
@@ -455,12 +457,35 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(1);
const r = results[0];
expect(r.code).toBe(ruleId);
- expect(r.message).toMatch(expectedMsgPrefix);
+ expect(r.message).toMatch(expectedStrictMsgPrefix);
expect(r.message).toMatch(/^.*delete_drink_glass or remove_drink_glass*/);
expect(r.severity).toBe(expectedSeverity);
expect(r.path.join('.')).toBe(
'paths./v1/drinks/{drink_id}/glasses/{glass_id}.delete.operationId'
);
});
+
+ it('path has multiple path params, delete, not strict', async () => {
+ const testDocument = makeCopy(rootDocument);
+
+ testDocument.paths['/v1/drinks/{drink_id}/glasses/{glass_id}'] = {
+ delete: {
+ operationId: 'smash_drink_glass',
+ },
+ };
+
+ rule.then.functionOptions.strict = false;
+
+ const results = await testRule(ruleId, rule, testDocument);
+ expect(results).toHaveLength(1);
+ const r = results[0];
+ expect(r.code).toBe(ruleId);
+ expect(r.message).toMatch(expectedNotStrictMsgPrefix);
+ expect(r.message).toMatch(/^.*delete or remove*/);
+ expect(r.severity).toBe(expectedSeverity);
+ expect(r.path.join('.')).toBe(
+ 'paths./v1/drinks/{drink_id}/glasses/{glass_id}.delete.operationId'
+ );
+ });
});
});