Skip to content

Commit 533e881

Browse files
SahilChoudhary22SahilChoudhary22
authored
Merge pull request #609 from postmanlabs/fix/fixAllOfValidationIssue597
Fix resolveRefs method
2 parents 4836bd5 + d3f1fba commit 533e881

File tree

6 files changed

+345
-8
lines changed

6 files changed

+345
-8
lines changed

‎lib/deref.js

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,33 @@ const _ = require('lodash'),
2727
'float',
2828
'double'
2929
],
30-
DEFAULT_SCHEMA_UTILS = require('./30XUtils/schemaUtils30X');
30+
DEFAULT_SCHEMA_UTILS = require('./30XUtils/schemaUtils30X'),
31+
traverseUtility = require('traverse');
32+
33+
/**
34+
* @param {*} currentNode - the object from which you're trying to find references
35+
* @param {*} seenRef References that are repeated. Used to identify circular references.
36+
* @returns {boolean} - Whether the object has circular references
37+
*/
38+
function hasReference(currentNode, seenRef) {
39+
let hasRef = false;
40+
41+
traverseUtility(currentNode).forEach(function (property) {
42+
if (property) {
43+
let hasReferenceTypeKey;
44+
hasReferenceTypeKey = Object.keys(property)
45+
.find(
46+
(key) => {
47+
return key === '$ref';
48+
}
49+
);
50+
if (hasReferenceTypeKey && seenRef[property.$ref]) {
51+
hasRef = true;
52+
}
53+
}
54+
});
55+
return hasRef;
56+
}
3157

3258
module.exports = {
3359
/**
@@ -167,13 +193,6 @@ module.exports = {
167193
let refKey = schema.$ref,
168194
outerProperties = concreteUtils.getOuterPropsIfIsSupported(schema);
169195

170-
// if this reference is seen before, ignore and move on.
171-
if (seenRef[refKey]) {
172-
return { value: '<Circular reference to ' + refKey + ' detected>' };
173-
}
174-
// add to seen array if not encountered before.
175-
seenRef[refKey] = stack;
176-
177196
// points to an existing location
178197
// .split will return [#, components, schemas, schemaName]
179198
splitRef = refKey.split('/');
@@ -199,6 +218,12 @@ module.exports = {
199218
});
200219

201220
resolvedSchema = this._getEscaped(components, splitRef);
221+
// if this reference is seen before, ignore and move on.
222+
if (seenRef[refKey] && hasReference(resolvedSchema, seenRef)) {
223+
return { value: '<Circular reference to ' + refKey + ' detected>' };
224+
}
225+
// add to seen array if not encountered before.
226+
seenRef[refKey] = stack;
202227
if (outerProperties) {
203228
resolvedSchema = concreteUtils.addOuterPropsToRefSchemaIfIsSupported(resolvedSchema, outerProperties);
204229
}

‎test/data/valid_openapi/all_of_properties.json

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
{
2+
"info": {
3+
"title": "Inheritance test API",
4+
"version": "v1.0"
5+
},
6+
"openapi": "3.0.1",
7+
"paths": {
8+
"/api/inheritancetest": {
9+
"get": {
10+
"responses": {
11+
"200": {
12+
"content": {
13+
"application/json": {
14+
"schema": {
15+
"$ref": "#/components/schemas/Page"
16+
}
17+
}
18+
},
19+
"description": "The page data including metadata and content"
20+
}
21+
}
22+
}
23+
}
24+
},
25+
"components": {
26+
"schemas": {
27+
"SpecificType": {
28+
"allOf": [
29+
{
30+
"$ref": "#/components/schemas/ParentType"
31+
},
32+
{
33+
"properties": {
34+
"specificTypeData": {
35+
"type": "string"
36+
}
37+
},
38+
"required": ["specificTypeData"]
39+
}
40+
]
41+
},
42+
"ParentType": {
43+
"allOf": [
44+
{
45+
"$ref": "#/components/schemas/GrandParentType"
46+
},
47+
{
48+
"properties": {
49+
"parentTypeData": {
50+
"type": "string"
51+
}
52+
},
53+
"required": ["parentTypeData"]
54+
}
55+
]
56+
},
57+
"GrandParentType": {
58+
"properties": {
59+
"grandParentTypeData": {
60+
"type": "string"
61+
}
62+
},
63+
"required": ["grandParentTypeData"]
64+
},
65+
"Page": {
66+
"allOf": [
67+
{
68+
"$ref": "#/components/schemas/GrandParentType"
69+
},
70+
{
71+
"properties": {
72+
"specificType": {
73+
"$ref": "#/components/schemas/SpecificType"
74+
}
75+
},
76+
"required": ["specificType"]
77+
}
78+
]
79+
}
80+
},
81+
"securitySchemes": {
82+
"basic": {
83+
"description": "Basic HTTP Authentication",
84+
"scheme": "basic",
85+
"type": "http"
86+
}
87+
}
88+
}
89+
}

‎test/data/validationData/invalid_response_body_all_of_properties_collection.json

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
{
2+
"item": [
3+
{
4+
"id": "d8bfb088-d251-4ecf-bb48-915eebcccbb7",
5+
"name": "/api/inheritancetest",
6+
"request": {
7+
"name": "/api/inheritancetest",
8+
"description": {},
9+
"url": {
10+
"path": [
11+
"api",
12+
"inheritancetest"
13+
],
14+
"host": [
15+
"{{baseUrl}}"
16+
],
17+
"query": [],
18+
"variable": []
19+
},
20+
"header": [
21+
{
22+
"key": "Accept",
23+
"value": "application/json"
24+
}
25+
],
26+
"method": "GET",
27+
"auth": null
28+
},
29+
"response": [
30+
{
31+
"id": "c1e63b4e-1f73-487f-b214-5962ba3b2164",
32+
"name": "The page data including metadata and content",
33+
"originalRequest": {
34+
"url": {
35+
"path": [
36+
"api",
37+
"inheritancetest"
38+
],
39+
"host": [
40+
"{{baseUrl}}"
41+
],
42+
"query": [],
43+
"variable": []
44+
},
45+
"method": "GET",
46+
"body": {}
47+
},
48+
"status": "OK",
49+
"code": 200,
50+
"header": [
51+
{
52+
"key": "Content-Type",
53+
"value": "application/json"
54+
}
55+
],
56+
"body": "{\n \"grandParentTypeData\": \"repreh\",\n \"specificType\": {\n \"parentTypeData\": \"sunt mollit sed Ut\",\n \"specificTypeData\": \"commod\"\n }\n}",
57+
"cookie": [],
58+
"_postman_previewlanguage": "json"
59+
}
60+
],
61+
"event": [],
62+
"protocolProfileBehavior": {
63+
"disableBodyPruning": true
64+
}
65+
}
66+
],
67+
"event": [],
68+
"variable": [
69+
{
70+
"type": "string",
71+
"value": "/",
72+
"key": "baseUrl"
73+
}
74+
],
75+
"info": {
76+
"_postman_id": "cc7d1e25-6930-4263-8147-2d67439b453b",
77+
"name": "Inheritance test API",
78+
"schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json",
79+
"description": {
80+
"content": "",
81+
"type": "text/plain"
82+
}
83+
}
84+
}

‎test/data/validationData/invalid_response_body_all_of_properties_spec.json

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
{
2+
"info": {
3+
"title": "Inheritance test API",
4+
"version": "v1.0"
5+
},
6+
"openapi": "3.0.1",
7+
"paths": {
8+
"/api/inheritancetest": {
9+
"get": {
10+
"responses": {
11+
"200": {
12+
"content": {
13+
"application/json": {
14+
"schema": {
15+
"$ref": "#/components/schemas/Page"
16+
}
17+
}
18+
},
19+
"description": "The page data including metadata and content"
20+
}
21+
}
22+
}
23+
}
24+
},
25+
"components": {
26+
"schemas": {
27+
"SpecificType": {
28+
"allOf": [
29+
{
30+
"$ref": "#/components/schemas/ParentType"
31+
},
32+
{
33+
"properties": {
34+
"specificTypeData": {
35+
"type": "string"
36+
}
37+
},
38+
"required": ["specificTypeData"]
39+
}
40+
]
41+
},
42+
"ParentType": {
43+
"allOf": [
44+
{
45+
"$ref": "#/components/schemas/GrandParentType"
46+
},
47+
{
48+
"properties": {
49+
"parentTypeData": {
50+
"type": "string"
51+
}
52+
},
53+
"required": ["parentTypeData"]
54+
}
55+
]
56+
},
57+
"GrandParentType": {
58+
"properties": {
59+
"grandParentTypeData": {
60+
"type": "string"
61+
}
62+
},
63+
"required": ["grandParentTypeData"]
64+
},
65+
"Page": {
66+
"allOf": [
67+
{
68+
"$ref": "#/components/schemas/GrandParentType"
69+
},
70+
{
71+
"properties": {
72+
"specificType": {
73+
"$ref": "#/components/schemas/SpecificType"
74+
}
75+
},
76+
"required": ["specificType"]
77+
}
78+
]
79+
}
80+
},
81+
"securitySchemes": {
82+
"basic": {
83+
"description": "Basic HTTP Authentication",
84+
"scheme": "basic",
85+
"type": "http"
86+
}
87+
}
88+
}
89+
}

‎test/unit/base.test.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,30 @@ describe('CONVERT FUNCTION TESTS ', function() {
12161216
done();
12171217
});
12181218
});
1219+
1220+
it('[GITHUB #597] - should convert file with all of merging properties', function() {
1221+
const fileSource = path.join(__dirname, VALID_OPENAPI_PATH, 'all_of_properties.json'),
1222+
fileData = fs.readFileSync(fileSource, 'utf8'),
1223+
input = {
1224+
type: 'string',
1225+
data: fileData
1226+
};
1227+
1228+
Converter.convert(input, { optimizeConversion: false, stackLimit: 50 }, (err, result) => {
1229+
let responseBody = JSON.parse(result.output[0].data.item[0].response[0].body);
1230+
expect(err).to.be.null;
1231+
expect(result.result).to.be.true;
1232+
expect(responseBody)
1233+
.to.have.all.keys('grandParentTypeData', 'specificType');
1234+
expect(responseBody.specificType)
1235+
.to.have.all.keys(
1236+
'grandParentTypeData',
1237+
'parentTypeData',
1238+
'specificTypeData'
1239+
);
1240+
});
1241+
});
1242+
12191243
it('The converter must throw an error for invalid null info', function (done) {
12201244
var openapi = fs.readFileSync(invalidNullInfo, 'utf8');
12211245
Converter.convert({ type: 'string', data: openapi },

‎test/unit/validator.test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,6 +1309,32 @@ describe('VALIDATE FUNCTION TESTS ', function () {
13091309
});
13101310
});
13111311

1312+
it('Should report a mismatch when the response body is not valid', function (done) {
1313+
let allOfExample = fs.readFileSync(path.join(__dirname, VALIDATION_DATA_FOLDER_PATH +
1314+
'/invalid_response_body_all_of_properties_spec.json'), 'utf-8'),
1315+
allOfCollection = fs.readFileSync(path.join(__dirname, VALIDATION_DATA_FOLDER_PATH +
1316+
'/invalid_response_body_all_of_properties_collection.json'), 'utf-8'),
1317+
historyRequest = [],
1318+
schemaPack = new Converter.SchemaPack({ type: 'string', data: allOfExample },
1319+
{ suggestAvailableFixes: true, showMissingInSchemaErrors: true });
1320+
1321+
getAllTransactions(JSON.parse(allOfCollection), historyRequest);
1322+
1323+
schemaPack.validateTransaction(historyRequest, (err, result) => {
1324+
const requestId = historyRequest[0].id,
1325+
request = result.requests[requestId],
1326+
responseId = historyRequest[0].response[0].id,
1327+
response = request.endpoints[0].responses[responseId];
1328+
expect(err).to.be.null;
1329+
expect(request.endpoints[0].matched).to.equal(false);
1330+
expect(response.matched).to.equal(false);
1331+
expect(response.mismatches).to.have.length(1);
1332+
expect(response.mismatches[0].reason)
1333+
.to.equal('The response body didn\'t match the specified schema');
1334+
done();
1335+
});
1336+
});
1337+
13121338
describe('findMatchingRequestFromSchema function', function () {
13131339
it('#GITHUB-9396 Should maintain correct order of matched endpoint', function (done) {
13141340
let schema = {

0 commit comments

Comments
 (0)