Skip to content

Commit

Permalink
fix: updated code and test to reflect updated naming convention (#133)
Browse files Browse the repository at this point in the history 
* updated code and test to reflect updated naming convention
* post operationIds should not be checked when patch is present
  • Loading branch information
@SaltedCaramelCoffee
SaltedCaramelCoffee authored Feb 11, 2020
1 parent cd9484c commit c44f2fa
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 33 deletions.
63 changes: 49 additions & 14 deletions src/plugins/validation/2and3/semantic-validators/operation-ids.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ module.exports.validate = function({ resolvedSpec }) {
const pathOps = pickBy(path, (obj, k) => {
return validOperationKeys.indexOf(k) > -1;
});
const allPathOperations = Object.keys(pathOps);
each(pathOps, (op, opKey) =>
arr.push(
merge(
{
pathKey: `${pathKey}`,
opKey: `${opKey}`,
path: `paths.${pathKey}.${opKey}`
path: `paths.${pathKey}.${opKey}`,
allPathOperations
},
op
)
Expand All @@ -57,47 +59,76 @@ module.exports.validate = function({ resolvedSpec }) {
const operationIdPassedConventionCheck = (
opKey,
operationId,
hasPathParam
allPathOperations,
pathEndsWithParam
) => {
// Only consider paths for which
// - paths with no path param has a GET and POST path
// - paths with path param has a GET, a DELETE, and a POST or PUT or PATCH.
// - paths that do not end with param has a GET and POST operation
// - paths that end with param has a GET, DELETE, POST, PUT or PATCH.

let checkPassed = true;
const verbs = [];

if (!hasPathParam) {
if (!pathEndsWithParam) {
// operationId for GET should starts with "list"
if (opKey === 'get' && !operationId.match(/^list[a-zA-Z0-9_]+/m)) {
checkPassed = false;
verbs.push('list');
}

// operationId for POST should starts with "create" or "add"
if (
else if (
opKey === 'post' &&
!operationId.match(/^(add|create)[a-zA-Z0-9_]+/m)
) {
checkPassed = false;
verbs.push('add');
verbs.push('create');
}
} else {
// operationId for GET should starts with "get"
if (opKey === 'get' && !operationId.match(/^get[a-zA-Z0-9_]+/m)) {
checkPassed = false;
verbs.push('get');
}

// operationId for DELETE should starts with "delete"
if (opKey === 'delete' && !operationId.match(/^delete[a-zA-Z0-9_]+/m)) {
else if (
opKey === 'delete' &&
!operationId.match(/^delete[a-zA-Z0-9_]+/m)
) {
checkPassed = false;
verbs.push('delete');
}

// operationId for POST/PUT/PATCH should starts with "update"
if (
['put', 'post', 'patch'].includes(opKey) &&
// operationId for PATCH should starts with "update"
else if (
opKey === 'patch' &&
!operationId.match(/^update[a-zA-Z0-9_]+/m)
) {
checkPassed = false;
verbs.push('update');
} else if (opKey === 'post') {
// If PATCH operation doesn't exist for path, POST operationId should start with "update"
if (
!allPathOperations.includes('patch') &&
!operationId.match(/^(update[a-zA-Z0-9_]+/m)
) {
checkPassed = false;
verbs.push('update');
}
}

// operationId for PUT should starts with "replace"
else if (
opKey === 'put' &&
!operationId.match(/^replace[a-zA-Z0-9_]+/m)
) {
checkPassed = false;
verbs.push('replace');
}
}
return checkPassed;
return { checkPassed, verbs };
};

operations.forEach(op => {
Expand All @@ -112,18 +143,22 @@ module.exports.validate = function({ resolvedSpec }) {
});
} else {
// Assertation 2: OperationId must conform to naming conventions
const regex = RegExp(/{[a-zA-Z0-9_-]+\}/m);
const regex = RegExp(/{[a-zA-Z0-9_-]+\}$/m);

const checkPassed = operationIdPassedConventionCheck(
const { checkPassed, verbs } = operationIdPassedConventionCheck(
op['opKey'],
op.operationId,
op.allPathOperations,
regex.test(op['pathKey'])
);

if (checkPassed === false) {
warnings.push({
path: op.path + '.operationId',
message: `operationIds should follow consistent naming convention`
message: `operationIds should follow consistent naming convention. operationId verb should be ${verbs}`.replace(
',',
' or '
)
});
}
}
Expand Down
15 changes: 9 additions & 6 deletions test/cli-validator/tests/optionHandling.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,14 @@ describe('cli tool - test option handling', function() {
expect(capturedText[statsSection + 7].match(/\S+/g)[1]).toEqual('(20%)');

// warnings
expect(capturedText[statsSection + 10].match(/\S+/g)[0]).toEqual('2');
expect(capturedText[statsSection + 10].match(/\S+/g)[1]).toEqual('(22%)');
expect(capturedText[statsSection + 10].match(/\S+/g)[0]).toEqual('1');
expect(capturedText[statsSection + 10].match(/\S+/g)[1]).toEqual('(11%)');

expect(capturedText[statsSection + 11].match(/\S+/g)[0]).toEqual('2');
expect(capturedText[statsSection + 11].match(/\S+/g)[1]).toEqual('(22%)');
expect(capturedText[statsSection + 11].match(/\S+/g)[0]).toEqual('1');
expect(capturedText[statsSection + 11].match(/\S+/g)[1]).toEqual('(11%)');

expect(capturedText[statsSection + 12].match(/\S+/g)[0]).toEqual('1');
expect(capturedText[statsSection + 12].match(/\S+/g)[1]).toEqual('(11%)');
expect(capturedText[statsSection + 12].match(/\S+/g)[0]).toEqual('2');
expect(capturedText[statsSection + 12].match(/\S+/g)[1]).toEqual('(22%)');

expect(capturedText[statsSection + 13].match(/\S+/g)[0]).toEqual('1');
expect(capturedText[statsSection + 13].match(/\S+/g)[1]).toEqual('(11%)');
Expand All @@ -186,6 +186,9 @@ describe('cli tool - test option handling', function() {

expect(capturedText[statsSection + 16].match(/\S+/g)[0]).toEqual('1');
expect(capturedText[statsSection + 16].match(/\S+/g)[1]).toEqual('(11%)');

expect(capturedText[statsSection + 17].match(/\S+/g)[0]).toEqual('1');
expect(capturedText[statsSection + 17].match(/\S+/g)[1]).toEqual('(11%)');
});

it('should not print statistics report by default', async function() {
Expand Down
49 changes: 36 additions & 13 deletions test/plugins/validation/2and3/operation-ids.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,39 @@ describe('validation plugin - semantic - operation-ids', function() {
operationId: 'listBooks'
},
delete: {
operationId: 'removeBooks'
operationId: 'removeBook'
},
post: {
operationId: 'changeBooks1'
operationId: 'updatesBook'
},
put: {
operationId: 'changeBooks2'
operationId: 'changeBook'
},
patch: {
operationId: 'changeBooks3'
operationId: 'changeBook2'
}
},
'/coffee/{id}': {
parameters: [
{
name: 'id',
in: 'path'
}
],
get: {
operationId: 'listCoffee'
},
delete: {
operationId: 'removeCoffee'
},
post: {
operationId: 'changeCoffee'
},
put: {
operationId: 'changeCoffee2'
},
patch: {
operationId: 'changeCoffee3'
}
}
}
Expand All @@ -147,10 +170,10 @@ describe('validation plugin - semantic - operation-ids', function() {
const res = validate({ resolvedSpec });

expect(res.errors.length).toEqual(0);
expect(res.warnings.length).toEqual(9);
expect(res.warnings.length).toEqual(12);
expect(res.warnings[0].path).toEqual('paths./books.get.operationId');
expect(res.warnings[0].message).toEqual(
'operationIds should follow consistent naming convention'
'operationIds should follow consistent naming convention. operationId verb should be list'
);
});

Expand Down Expand Up @@ -187,13 +210,13 @@ describe('validation plugin - semantic - operation-ids', function() {
operationId: 'deleteBook'
},
post: {
operationId: 'updateBook1'
operationId: 'addBook'
},
put: {
operationId: 'updateBook2'
operationId: 'replaceBook'
},
patch: {
operationId: 'updateBook3'
operationId: 'updateBook'
}
},
'/coffee/{id}': {
Expand All @@ -207,16 +230,16 @@ describe('validation plugin - semantic - operation-ids', function() {
operationId: 'get_coffee'
},
delete: {
operationId: 'delete_book'
operationId: 'delete_coffee'
},
post: {
operationId: 'update_book_1'
operationId: 'create_coffee'
},
put: {
operationId: 'update_book_2'
operationId: 'replace_coffee'
},
patch: {
operationId: 'update_book_3'
operationId: 'update_coffee'
}
}
}
Expand Down

0 comments on commit c44f2fa

Please sign in to comment.