Skip to content

Commit

Permalink
fix: Only check operationId naming convention for resource-oriented o…
Browse files Browse the repository at this point in the history 
…perations
  • Loading branch information
Mike Kistler committed Apr 11, 2020
1 parent 80f2302 commit 8da9d37
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 43 deletions.
46 changes: 30 additions & 16 deletions src/plugins/validation/2and3/semantic-validators/operation-ids.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,24 +146,38 @@ module.exports.validate = function({ resolvedSpec }, config) {
);
} else {
// Assertation 2: OperationId must conform to naming conventions
const regex = RegExp(/{[a-zA-Z0-9_-]+\}$/m);

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

if (checkPassed === false) {
messages.addMessage(
op.path + '.operationId',
`operationIds should follow naming convention: operationId verb should be ${verbs}`.replace(
',',
' or '
),
config.operation_id_naming_convention
// We'll use a heuristic to decide if this path is part of a resource oriented API.
// If path ends in path param, look for corresponding create/list path
// Conversely, if no path param, look for path with path param

const pathEndsWithParam = op.pathKey.endsWith('}');
const isResourceOriented = pathEndsWithParam
? Object.keys(resolvedSpec.paths).includes(
op.pathKey.replace('/\\{[A-Za-z0-9-_]+\\}$', '')
)
: Object.keys(resolvedSpec.paths).some(p =>
p.startsWith(op.pathKey + '/{')
);

if (isResourceOriented) {
const { checkPassed, verbs } = operationIdPassedConventionCheck(
op['opKey'],
op.operationId,
op.allPathOperations,
pathEndsWithParam
);

if (checkPassed === false) {
messages.addMessage(
op.path + '.operationId',
`operationIds should follow naming convention: operationId verb should be ${verbs}`.replace(
',',
' or '
),
config.operation_id_naming_convention
);
}
}
}
}
Expand Down
14 changes: 6 additions & 8 deletions test/cli-validator/tests/expectedOutput.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,12 @@ describe('cli tool - test expected output - Swagger 2', function() {

// warnings
expect(capturedText[25].match(/\S+/g)[2]).toEqual('36');
expect(capturedText[29].match(/\S+/g)[2]).toEqual('87');
expect(capturedText[33].match(/\S+/g)[2]).toEqual('36');
expect(capturedText[37].match(/\S+/g)[2]).toEqual('59');
expect(capturedText[41].match(/\S+/g)[2]).toEqual('197');
expect(capturedText[45].match(/\S+/g)[2]).toEqual('108');
expect(capturedText[49].match(/\S+/g)[2]).toEqual('131');
expect(capturedText[53].match(/\S+/g)[2]).toEqual('134');
expect(capturedText[57].match(/\S+/g)[2]).toEqual('126');
expect(capturedText[29].match(/\S+/g)[2]).toEqual('59');
expect(capturedText[33].match(/\S+/g)[2]).toEqual('197');
expect(capturedText[37].match(/\S+/g)[2]).toEqual('108');
expect(capturedText[41].match(/\S+/g)[2]).toEqual('131');
expect(capturedText[45].match(/\S+/g)[2]).toEqual('134');
expect(capturedText[49].match(/\S+/g)[2]).toEqual('126');
});

it('should return exit code of 0 if there are only warnings', async function() {
Expand Down
26 changes: 10 additions & 16 deletions test/cli-validator/tests/optionHandling.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ describe('cli tool - test option handling', function() {

// totals
expect(capturedText[statsSection + 1].match(/\S+/g)[5]).toEqual('5');
expect(capturedText[statsSection + 2].match(/\S+/g)[5]).toEqual('9');
expect(capturedText[statsSection + 2].match(/\S+/g)[5]).toEqual('7');

// errors
expect(capturedText[statsSection + 4].match(/\S+/g)[0]).toEqual('2');
Expand All @@ -168,29 +168,23 @@ 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('1');
expect(capturedText[statsSection + 10].match(/\S+/g)[1]).toEqual('(11%)');
expect(capturedText[statsSection + 10].match(/\S+/g)[0]).toEqual('2');
expect(capturedText[statsSection + 10].match(/\S+/g)[1]).toEqual('(29%)');

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

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

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

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

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

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%)');
expect(capturedText[statsSection + 15].match(/\S+/g)[1]).toEqual('(14%)');
});

it('should not print statistics report by default', async function() {
Expand Down Expand Up @@ -315,7 +309,7 @@ describe('cli tool - test option handling', function() {
}
}
});
expect(warningCount).toEqual(3); // without the config this value is 5
expect(warningCount).toEqual(1); // without the config this value is 5
expect(errorCount).toEqual(3); // without the config this value is 0
});

Expand Down
6 changes: 3 additions & 3 deletions test/plugins/validation/2and3/operation-ids.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('validation plugin - semantic - operation-ids', function() {
expect(res.errors.length).toEqual(1);
expect(res.errors[0].path).toEqual('paths./coolPath.get.operationId');
expect(res.errors[0].message).toEqual('operationIds must be unique');
expect(res.warnings.length).toEqual(1);
expect(res.warnings.length).toEqual(0);
});

it('should complain about a repeated operationId in a different path', function() {
Expand Down Expand Up @@ -55,7 +55,7 @@ describe('validation plugin - semantic - operation-ids', function() {
expect(res.errors.length).toEqual(1);
expect(res.errors[0].path).toEqual('paths./greatPath.put.operationId');
expect(res.errors[0].message).toEqual('operationIds must be unique');
expect(res.warnings.length).toEqual(1);
expect(res.warnings.length).toEqual(0);
});

it('should complain about a repeated operationId in a shared path item', async function() {
Expand Down Expand Up @@ -96,7 +96,7 @@ describe('validation plugin - semantic - operation-ids', function() {
expect(res.errors.length).toEqual(1);
expect(res.errors[0].path).toEqual('paths./greatPath.get.operationId');
expect(res.errors[0].message).toEqual('operationIds must be unique');
expect(res.warnings.length).toEqual(1);
expect(res.warnings.length).toEqual(0);
});

it('should complain about operationId naming convention', async function() {
Expand Down

0 comments on commit 8da9d37

Please sign in to comment.