Skip to content

Commit 0066f1c

Browse files
lukaszzazulakrobert-hebel-sb
lukaszzazulak
and
robert-hebel-sb
authored
feat(ls): migrate disallowing equivalent paths in OAS2 (#4987)
* feat(ls): migrate disallowing equivalent paths in OAS2 Co-authored-by: robert-hebel-sb <[email protected]>
1 parent ef012f1 commit 0066f1c

File tree

9 files changed

+208
-20
lines changed

9 files changed

+208
-20
lines changed

‎packages/apidom-ls/src/config/codes.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,7 @@ enum ApilintCodes {
679679
OPENAPI2_PATH_TEMPLATE = 3040000,
680680
OPENAPI2_PATH_TEMPLATE_VALUE_WELL_FORMED = 3040100,
681681
OPENAPI2_PATH_TEMPLATE_VALUE_VALID,
682+
OPENAPI2_PATH_TEMPLATE_VALUE_EQUIVALENT_NOT_ALLOWED,
682683

683684
OPENAPI2_LICENSE = 3050000,
684685
OPENAPI2_LICENSE_FIELD_NAME_TYPE = 3050100,

‎packages/apidom-ls/src/config/openapi/path-template/lint/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import valueWellFormedLint from './value--well-formed.ts';
22
import valueValidLint from './value--valid.ts';
3+
import valueEquivalentNotAllowedLint from './value--equivalent-not-allowed.ts';
34

4-
const lints = [valueWellFormedLint, valueValidLint];
5+
const lints = [valueWellFormedLint, valueValidLint, valueEquivalentNotAllowedLint];
56

67
export default lints;

‎packages/apidom-ls/src/config/openapi/path-template/lint/value--equivalent-not-allowed.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { DiagnosticSeverity } from 'vscode-languageserver-types';
2+
3+
import ApilintCodes from '../../../codes.ts';
4+
import { LinterMeta } from '../../../../apidom-language-types.ts';
5+
import { OpenAPI2 } from '../../target-specs.ts';
6+
7+
const valueEquivalentNotAllowedLint: LinterMeta = {
8+
code: ApilintCodes.OPENAPI2_PATH_TEMPLATE_VALUE_EQUIVALENT_NOT_ALLOWED,
9+
source: 'apilint',
10+
message: 'Equivalent paths are not allowed',
11+
severity: DiagnosticSeverity.Error,
12+
linterFunction: 'apilintOpenAPIPathTemplateNoEquivalent',
13+
marker: 'value',
14+
targetSpecs: OpenAPI2,
15+
};
16+
17+
export default valueEquivalentNotAllowedLint;

‎packages/apidom-ls/src/services/validation/linter-functions.ts

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
test as testPathTemplate,
1818
resolve as resolvePathTemplate,
1919
parse as parsePathTemplate,
20+
isIdentical,
2021
} from 'openapi-path-templating';
2122

2223
// eslint-disable-next-line import/no-cycle
@@ -1258,52 +1259,66 @@ export const standardLinterfunctions: FunctionItem[] = [
12581259
}
12591260

12601261
let oneOfParametersIsReferenceObject = false;
1261-
const parameterElements: Element[] = [];
12621262
const isParameterElement = (el: Element): boolean => el.element === 'parameter';
12631263
const isReferenceElement = (el: Element): boolean => el.element === 'reference';
12641264

1265+
const pathLevelParameterElements: ObjectElement[] = [];
12651266
const pathItemParameterElements = pathItemElement.get('parameters');
12661267
if (isArrayElement(pathItemParameterElements)) {
12671268
pathItemParameterElements.forEach((parameter) => {
12681269
if (isReferenceElement(parameter) && !oneOfParametersIsReferenceObject) {
12691270
oneOfParametersIsReferenceObject = true;
12701271
}
1271-
if (isParameterElement(parameter)) {
1272-
parameterElements.push(parameter);
1272+
if (isParameterElement(parameter) && isObject(parameter)) {
1273+
pathLevelParameterElements.push(parameter);
12731274
}
12741275
});
12751276
}
12761277

1278+
const includesTemplateExpression: boolean[] = [];
12771279
pathItemElement.forEach((el) => {
12781280
if (el.element === 'operation') {
1279-
const operationParameterElements = (el as ObjectElement).get('parameters');
1281+
const operationParameterElements = isObject(el) && el.get('parameters');
1282+
const currentOperationLevelParameterElements: ObjectElement[] = [];
12801283
if (isArrayElement(operationParameterElements)) {
12811284
operationParameterElements.forEach((parameter) => {
12821285
if (isReferenceElement(parameter) && !oneOfParametersIsReferenceObject) {
12831286
oneOfParametersIsReferenceObject = true;
12841287
}
1285-
if (isParameterElement(parameter)) {
1286-
parameterElements.push(parameter);
1288+
if (isParameterElement(parameter) && isObject(parameter)) {
1289+
currentOperationLevelParameterElements.push(parameter);
12871290
}
12881291
});
12891292
}
1290-
}
1291-
});
1292-
1293-
const pathTemplateResolveParams: { [key: string]: 'placeholder' } = {};
1293+
const pathTemplateResolveParams: { [key: string]: 'placeholder' } = {};
1294+
pathLevelParameterElements
1295+
.concat(currentOperationLevelParameterElements)
1296+
.forEach((parameter) => {
1297+
const inParam = parameter.get('in');
1298+
const nameParam = parameter.get('name');
1299+
if (inParam && inParam.content === 'path' && nameParam && nameParam.content) {
1300+
pathTemplateResolveParams[nameParam.content] = 'placeholder';
1301+
}
1302+
});
12941303

1295-
parameterElements.forEach((parameter) => {
1296-
if (toValue((parameter as ObjectElement).get('in')) === 'path') {
1297-
pathTemplateResolveParams[toValue((parameter as ObjectElement).get('name'))] =
1298-
'placeholder';
1304+
const pathTemplate = toValue(element);
1305+
const resolvedPathTemplate = resolvePathTemplate(
1306+
pathTemplate,
1307+
pathTemplateResolveParams,
1308+
);
1309+
includesTemplateExpression.push(
1310+
testPathTemplate(resolvedPathTemplate, {
1311+
strict: true,
1312+
}),
1313+
);
12991314
}
13001315
});
13011316

1302-
const pathTemplate = toValue(element);
1303-
const resolvedPathTemplate = resolvePathTemplate(pathTemplate, pathTemplateResolveParams);
1304-
const includesTemplateExpression = testPathTemplate(resolvedPathTemplate, { strict: true });
1305-
1306-
return !includesTemplateExpression || oneOfParametersIsReferenceObject;
1317+
return (
1318+
(includesTemplateExpression.length > 0 &&
1319+
includesTemplateExpression.every((bool) => !bool)) ||
1320+
oneOfParametersIsReferenceObject
1321+
);
13071322
}
13081323

13091324
return true;
@@ -1447,4 +1462,21 @@ export const standardLinterfunctions: FunctionItem[] = [
14471462
return true;
14481463
},
14491464
},
1465+
{
1466+
functionName: 'apilintOpenAPIPathTemplateNoEquivalent',
1467+
function: (element: Element): boolean => {
1468+
const isFirstOccurrence = (currentKey: string, allKeys: unknown[]) => {
1469+
const firstIndex = allKeys.findIndex(
1470+
(e) => typeof e === 'string' && isIdentical(e, currentKey),
1471+
);
1472+
1473+
return allKeys[firstIndex] === currentKey;
1474+
};
1475+
const paths = element.parent?.parent;
1476+
1477+
return isStringElement(element) && isObject(paths)
1478+
? isFirstOccurrence(element.toValue(), paths.keys())
1479+
: true;
1480+
},
1481+
},
14501482
];

‎packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-2-0.yaml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,20 @@ paths:
7676
required: true
7777
type: string
7878
format: uuid
79+
/users/{id}:
80+
get:
81+
summary: Get user by ID
82+
parameters:
83+
- name: id
84+
in: path
85+
required: true
86+
type: string
87+
responses:
88+
'200':
89+
description: OK
90+
91+
delete:
92+
summary: No parameters here (specified in get only)
93+
responses:
94+
'204':
95+
description: No Content

‎packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-0.yaml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,21 @@ paths:
103103
type: string
104104
format: uuid
105105
title: A Id
106+
/users/{id}:
107+
get:
108+
summary: Get user by ID
109+
parameters:
110+
- name: id
111+
in: path
112+
required: true
113+
schema:
114+
type: string
115+
responses:
116+
'200':
117+
description: OK
118+
119+
delete:
120+
summary: No parameters here (specified in get only)
121+
responses:
122+
'204':
123+
description: No Content

‎packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-1.yaml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,21 @@ paths:
103103
type: string
104104
format: uuid
105105
title: A Id
106+
/users/{id}:
107+
get:
108+
summary: Get user by ID
109+
parameters:
110+
- name: id
111+
in: path
112+
required: true
113+
schema:
114+
type: string
115+
responses:
116+
'200':
117+
description: OK
118+
119+
delete:
120+
summary: No parameters here (specified in get only)
121+
responses:
122+
'204':
123+
description: No Content

‎packages/apidom-ls/test/fixtures/validation/oas/path-template-equivalent-not-allowed.yaml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
swagger: '2.0'
2+
info:
3+
title: Test API
4+
version: 1.0.0
5+
paths:
6+
/items/{id}:
7+
get:
8+
summary: Get item by ID
9+
parameters:
10+
- name: id
11+
in: path
12+
required: true
13+
type: string
14+
responses:
15+
200:
16+
description: OK
17+
/items/{itemId}:
18+
get:
19+
summary: Get item by itemId
20+
parameters:
21+
- name: itemId
22+
in: path
23+
required: true
24+
type: string
25+
responses:
26+
200:
27+
description: OK
28+

‎packages/apidom-ls/test/validate.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3788,6 +3788,13 @@ describe('apidom-ls-validate', function () {
37883788
code: 3040101,
37893789
source: 'apilint',
37903790
},
3791+
{
3792+
range: { start: { line: 78, character: 2 }, end: { line: 78, character: 13 } },
3793+
message: 'path template expressions is not matched with Parameter Object(s)',
3794+
severity: 1,
3795+
code: 3040101,
3796+
source: 'apilint',
3797+
},
37913798
];
37923799
assert.deepEqual(result, expected as Diagnostic[]);
37933800

@@ -3845,6 +3852,13 @@ describe('apidom-ls-validate', function () {
38453852
code: 3040101,
38463853
source: 'apilint',
38473854
},
3855+
{
3856+
range: { start: { line: 105, character: 2 }, end: { line: 105, character: 13 } },
3857+
message: 'path template expressions is not matched with Parameter Object(s)',
3858+
severity: 1,
3859+
code: 3040101,
3860+
source: 'apilint',
3861+
},
38483862
];
38493863
assert.deepEqual(result, expected as Diagnostic[]);
38503864

@@ -3902,6 +3916,13 @@ describe('apidom-ls-validate', function () {
39023916
code: 3040101,
39033917
source: 'apilint',
39043918
},
3919+
{
3920+
range: { start: { line: 105, character: 2 }, end: { line: 105, character: 13 } },
3921+
message: 'path template expressions is not matched with Parameter Object(s)',
3922+
severity: 1,
3923+
code: 3040101,
3924+
source: 'apilint',
3925+
},
39053926
];
39063927
assert.deepEqual(result, expected as Diagnostic[]);
39073928

@@ -5893,6 +5914,41 @@ describe('apidom-ls-validate', function () {
58935914
const expected: Diagnostic[] = [];
58945915
assert.deepEqual(result, expected);
58955916

5917+
languageService.terminate();
5918+
});
5919+
it('oas 2.0 should not allow equivalent paths', async function () {
5920+
const spec = fs
5921+
.readFileSync(
5922+
path.join(
5923+
__dirname,
5924+
'fixtures',
5925+
'validation',
5926+
'oas',
5927+
'path-template-equivalent-not-allowed.yaml',
5928+
),
5929+
)
5930+
.toString();
5931+
const doc: TextDocument = TextDocument.create(
5932+
'path-template-equivalent-not-allowed.yaml',
5933+
'yaml',
5934+
0,
5935+
spec,
5936+
);
5937+
5938+
const languageService: LanguageService = getLanguageService(contextNoSchema);
5939+
5940+
const result = await languageService.doValidation(doc);
5941+
const expected: Diagnostic[] = [
5942+
{
5943+
message: 'Equivalent paths are not allowed',
5944+
severity: 1,
5945+
code: 3040102,
5946+
source: 'apilint',
5947+
range: { start: { line: 16, character: 2 }, end: { line: 16, character: 17 } },
5948+
},
5949+
];
5950+
assert.deepEqual(result, expected);
5951+
58965952
languageService.terminate();
58975953
});
58985954
});

0 commit comments

Comments
 (0)