Skip to content

Commit 8e399c7

Browse files
authored
Refactor ESLint rules and remove unnecessary one-var disables; enhance schema resolution logic for composite parameters in conversion tests (#868)
Refactor ESLint rules and remove unnecessary one-var disables; enhance schema resolution logic for composite parameters in conversion tests (#868)
1 parent 5d99774 commit 8e399c7

File tree

4 files changed

+267
-20
lines changed

4 files changed

+267
-20
lines changed

.eslintrc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,6 @@
195195
"no-unneeded-ternary": "error",
196196
"no-whitespace-before-property": "error",
197197
"object-curly-spacing": ["error", "always"],
198-
"one-var": ["error", "always"],
199198
"one-var-declaration-per-line": "error",
200199
"operator-assignment": "error",
201200
"operator-linebreak": ["error", "after"],

libV2/index.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
/* eslint-disable one-var */
21
const _ = require('lodash'),
32
{ Collection } = require('postman-collection/lib/collection/collection'),
43
GraphLib = require('graphlib'),

libV2/schemaUtils.js

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,21 @@ let QUERYPARAM = 'query',
494494
* @returns {Object} Resolved schema
495495
*/
496496
resolveAllOfSchema = (context, schema, stack = 0, resolveFor = CONVERSION, seenRef = {}, currentPath = '') => {
497+
/*
498+
For TYPES_GENERATION, we do not want to merge the allOf schemas
499+
instead we want to keep them separate so that we can generate types like:
500+
allOf: [
501+
{ $ref: '#/components/schemas/User' },
502+
{
503+
type: 'object',
504+
properties: {
505+
timestamp: { type: 'string', format: 'date-time' }
506+
}
507+
}
508+
]
509+
If we merge the schemas, we will loose the information that the schema was
510+
a combination of multiple schemas
511+
*/
497512
if (resolveFor === TYPES_GENERATION) {
498513
return {
499514
allOf: _.map(schema.allOf, (schema) => {
@@ -553,7 +568,6 @@ let QUERYPARAM = 'query',
553568

554569
stack++;
555570

556-
// eslint-disable-next-line one-var
557571
const compositeKeyword = schema.anyOf ? 'anyOf' : 'oneOf',
558572
{ concreteUtils } = context;
559573

@@ -628,7 +642,6 @@ let QUERYPARAM = 'query',
628642
writeOnlyPropCache: context.writeOnlyPropCache
629643
};
630644

631-
// eslint-disable-next-line one-var
632645
const newReadPropCache = context.readOnlyPropCache,
633646
newWritePropCache = context.writeOnlyPropCache;
634647

@@ -1471,7 +1484,6 @@ let QUERYPARAM = 'query',
14711484
});
14721485
});
14731486

1474-
// eslint-disable-next-line one-var
14751487
let responseExample,
14761488
responseExampleData;
14771489

@@ -2114,14 +2126,18 @@ let QUERYPARAM = 'query',
21142126
param = resolveSchema(context, param);
21152127
}
21162128

2117-
if (_.has(param.schema, '$ref')) {
2118-
param.schema = resolveSchema(context, param.schema);
2119-
}
2120-
21212129
if (param.in !== QUERYPARAM || (!includeDeprecated && param.deprecated)) {
21222130
return;
21232131
}
21242132

2133+
const shouldResolveSchema = _.has(param, 'schema') &&
2134+
(_.has(param.schema, '$ref') || _.has(param.schema, 'anyOf') ||
2135+
_.has(param.schema, 'oneOf') || _.has(param.schema, 'allOf'));
2136+
2137+
if (shouldResolveSchema) {
2138+
param.schema = resolveSchema(context, param.schema);
2139+
}
2140+
21252141
let queryParamTypeInfo = {},
21262142
properties = {},
21272143
paramValue = resolveValueOfParameter(context, param);
@@ -2163,14 +2179,19 @@ let QUERYPARAM = 'query',
21632179
param = resolveSchema(context, param);
21642180
}
21652181

2166-
if (_.has(param.schema, '$ref')) {
2167-
param.schema = resolveSchema(context, param.schema);
2168-
}
2169-
21702182
if (param.in !== PATHPARAM) {
21712183
return;
21722184
}
21732185

2186+
2187+
const shouldResolveSchema = _.has(param, 'schema') &&
2188+
(_.has(param.schema, '$ref') || _.has(param.schema, 'anyOf') ||
2189+
_.has(param.schema, 'oneOf') || _.has(param.schema, 'allOf'));
2190+
2191+
if (shouldResolveSchema) {
2192+
param.schema = resolveSchema(context, param.schema);
2193+
}
2194+
21742195
let pathParamTypeInfo = {},
21752196
properties = {},
21762197
paramValue = resolveValueOfParameter(context, param);
@@ -2240,14 +2261,18 @@ let QUERYPARAM = 'query',
22402261
param = resolveSchema(context, param);
22412262
}
22422263

2243-
if (_.has(param.schema, '$ref')) {
2244-
param.schema = resolveSchema(context, param.schema);
2245-
}
2246-
22472264
if (param.in !== HEADER || (!includeDeprecated && param.deprecated)) {
22482265
return;
22492266
}
22502267

2268+
const shouldResolveSchema = _.has(param, 'schema') &&
2269+
(_.has(param.schema, '$ref') || _.has(param.schema, 'anyOf') ||
2270+
_.has(param.schema, 'oneOf') || _.has(param.schema, 'allOf'));
2271+
2272+
if (shouldResolveSchema) {
2273+
param.schema = resolveSchema(context, param.schema);
2274+
}
2275+
22512276
if (!keepImplicitHeaders && _.includes(IMPLICIT_HEADERS, _.toLower(_.get(param, 'name')))) {
22522277
return;
22532278
}
@@ -2400,8 +2425,19 @@ let QUERYPARAM = 'query',
24002425

24012426
headers.push(...serialisedHeader);
24022427

2403-
if (headerData && headerData.name && headerData.schema && headerData.schema.type) {
2404-
const { schema } = headerData;
2428+
if (headerData && headerData.name && headerData.schema) {
2429+
let { schema } = headerData;
2430+
const shouldResolveSchema = _.has(schema, '$ref') || _.has(schema, 'anyOf') ||
2431+
_.has(schema, 'oneOf') || _.has(schema, 'allOf');
2432+
2433+
if (shouldResolveSchema) {
2434+
schema = resolveSchema(context, schema);
2435+
}
2436+
2437+
if (!schema.type) {
2438+
return;
2439+
}
2440+
24052441
properties = {
24062442
type: schema.type,
24072443
format: schema.format,

test/unit/convertV2WithTypes.test.js

Lines changed: 214 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/* eslint-disable max-len */
22
// Disabling max Length for better visibility of the expectedExtractedTypes
33

4-
/* eslint-disable one-var */
54
/* Disabling as we want the checks to run in order of their declaration as declaring everything as once
65
even though initial declarations fails with test won't do any good */
76

@@ -764,6 +763,220 @@ describe('convertV2WithTypes', function() {
764763
done();
765764
});
766765
});
766+
767+
it('should extract only first option from composite query parameters, path parameters, request headers, and response headers', function(done) {
768+
const openApiWithCompositeParams = {
769+
openapi: '3.0.0',
770+
info: { title: 'Test API', version: '1.0.0' },
771+
paths: {
772+
'/test/{pathParam}': {
773+
get: {
774+
parameters: [
775+
// Query parameters with composite schemas
776+
{
777+
name: 'status',
778+
in: 'query',
779+
schema: {
780+
oneOf: [
781+
{ type: 'string', enum: ['active', 'inactive'] },
782+
{ type: 'integer', minimum: 1, maximum: 10 }
783+
]
784+
}
785+
},
786+
{
787+
name: 'category',
788+
in: 'query',
789+
schema: {
790+
anyOf: [
791+
{ type: 'string' },
792+
{ type: 'number' }
793+
]
794+
}
795+
},
796+
{
797+
name: 'priority',
798+
in: 'query',
799+
schema: {
800+
allOf: [
801+
{ type: 'string' },
802+
{ minLength: 3 }
803+
]
804+
}
805+
},
806+
// Path parameter with composite schema
807+
{
808+
name: 'pathParam',
809+
in: 'path',
810+
required: true,
811+
schema: {
812+
oneOf: [
813+
{ type: 'string', pattern: '^[a-z]+$' },
814+
{ type: 'integer', minimum: 100 }
815+
]
816+
}
817+
},
818+
// Header parameters with composite schemas
819+
{
820+
name: 'X-Custom-Header',
821+
in: 'header',
822+
schema: {
823+
anyOf: [
824+
{ type: 'string', format: 'uuid' },
825+
{ type: 'string', enum: ['default', 'custom'] }
826+
]
827+
}
828+
},
829+
{
830+
name: 'X-Version',
831+
in: 'header',
832+
schema: {
833+
allOf: [
834+
{ type: 'string' },
835+
{ pattern: '^v\\d+\\.\\d+$' }
836+
]
837+
}
838+
}
839+
],
840+
responses: {
841+
'200': {
842+
description: 'Success',
843+
headers: {
844+
'X-Rate-Limit': {
845+
description: 'Rate limit header with composite schema',
846+
schema: {
847+
oneOf: [
848+
{ type: 'integer', minimum: 1, maximum: 1000 },
849+
{ type: 'string', enum: ['unlimited', 'blocked'] }
850+
]
851+
}
852+
},
853+
'X-Response-Type': {
854+
description: 'Response type header with composite schema',
855+
schema: {
856+
anyOf: [
857+
{ type: 'string', format: 'uri' },
858+
{ type: 'string', pattern: '^[A-Z_]+$' }
859+
]
860+
}
861+
},
862+
'X-Content-Version': {
863+
description: 'Content version header with composite schema',
864+
schema: {
865+
allOf: [
866+
{ type: 'string' },
867+
{ pattern: '^v\\d+\\.\\d+\\.\\d+$' },
868+
{ minLength: 5 }
869+
]
870+
}
871+
}
872+
},
873+
content: {
874+
'application/json': {
875+
schema: {
876+
anyOf: [
877+
{ type: 'string' },
878+
{ type: 'object', properties: { message: { type: 'string' } } }
879+
]
880+
}
881+
}
882+
}
883+
}
884+
}
885+
}
886+
}
887+
}
888+
};
889+
890+
Converter.convertV2WithTypes({ type: 'json', data: openApiWithCompositeParams }, {}, (err, conversionResult) => {
891+
expect(err).to.be.null;
892+
expect(conversionResult.extractedTypes).to.be.an('object').that.is.not.empty;
893+
894+
const extractedTypes = conversionResult.extractedTypes['get/test/{pathParam}'];
895+
896+
// Verify query parameters extract only first option
897+
const queryParams = JSON.parse(extractedTypes.request.queryParam);
898+
expect(queryParams).to.be.an('array').with.length(3);
899+
900+
// Check oneOf query parameter - should extract first option (string with enum)
901+
expect(queryParams[0]).to.have.property('keyName', 'status');
902+
expect(queryParams[0].properties).to.have.property('type', 'string');
903+
expect(queryParams[0].properties).to.have.property('enum');
904+
expect(queryParams[0].properties.enum).to.deep.equal(['active', 'inactive']);
905+
expect(queryParams[0].properties).to.not.have.property('oneOf');
906+
907+
// Check anyOf query parameter - should extract first option (string)
908+
expect(queryParams[1]).to.have.property('keyName', 'category');
909+
expect(queryParams[1].properties).to.have.property('type', 'string');
910+
expect(queryParams[1].properties).to.not.have.property('anyOf');
911+
912+
// Check allOf query parameter - should merge constraints (string with minLength)
913+
expect(queryParams[2]).to.have.property('keyName', 'priority');
914+
expect(queryParams[2].properties).to.have.property('type', 'string');
915+
expect(queryParams[2].properties).to.have.property('minLength', 3);
916+
expect(queryParams[2].properties).to.not.have.property('allOf');
917+
918+
// Verify path parameters extract only first option
919+
const pathParams = JSON.parse(extractedTypes.request.pathParam);
920+
expect(pathParams).to.be.an('array').with.length(1);
921+
922+
// Check oneOf path parameter - should extract first option (string with pattern)
923+
expect(pathParams[0]).to.have.property('keyName', 'pathParam');
924+
expect(pathParams[0].properties).to.have.property('type', 'string');
925+
expect(pathParams[0].properties).to.have.property('pattern', '^[a-z]+$');
926+
expect(pathParams[0].properties).to.not.have.property('oneOf');
927+
928+
// Verify headers extract only first option
929+
const headers = JSON.parse(extractedTypes.request.headers);
930+
expect(headers).to.be.an('array').with.length(2);
931+
932+
// Check anyOf header - should extract first option (string with format)
933+
expect(headers[0]).to.have.property('keyName', 'X-Custom-Header');
934+
expect(headers[0].properties).to.have.property('type', 'string');
935+
expect(headers[0].properties).to.have.property('format', 'uuid');
936+
expect(headers[0].properties).to.not.have.property('anyOf');
937+
938+
// Check allOf header - should merge constraints (string with pattern)
939+
expect(headers[1]).to.have.property('keyName', 'X-Version');
940+
expect(headers[1].properties).to.have.property('type', 'string');
941+
expect(headers[1].properties).to.have.property('pattern', '^v\\d+\\.\\d+$');
942+
expect(headers[1].properties).to.not.have.property('allOf');
943+
944+
// Verify response body preserves full composite schema
945+
const responseBody = conversionResult.extractedTypes['get/test/{pathParam}'].response['200'].body;
946+
const parsedResponseBody = JSON.parse(responseBody);
947+
948+
expect(parsedResponseBody).to.have.property('anyOf');
949+
expect(parsedResponseBody.anyOf).to.be.an('array').with.length(2);
950+
expect(parsedResponseBody.anyOf[0]).to.have.property('type', 'string');
951+
expect(parsedResponseBody.anyOf[1]).to.have.property('type', 'object');
952+
953+
// Verify response headers extract only first option
954+
const responseHeaders = JSON.parse(extractedTypes.response['200'].headers);
955+
expect(responseHeaders).to.be.an('array').with.length(3);
956+
957+
// Check oneOf response header - should extract first option (integer with constraints)
958+
expect(responseHeaders[0]).to.have.property('keyName', 'X-Rate-Limit');
959+
expect(responseHeaders[0].properties).to.have.property('type', 'integer');
960+
expect(responseHeaders[0].properties).to.have.property('minimum', 1);
961+
expect(responseHeaders[0].properties).to.have.property('maximum', 1000);
962+
expect(responseHeaders[0].properties).to.not.have.property('oneOf');
963+
964+
// Check anyOf response header - should extract first option (string with format)
965+
expect(responseHeaders[1]).to.have.property('keyName', 'X-Response-Type');
966+
expect(responseHeaders[1].properties).to.have.property('type', 'string');
967+
expect(responseHeaders[1].properties).to.have.property('format', 'uri');
968+
expect(responseHeaders[1].properties).to.not.have.property('anyOf');
969+
970+
// Check allOf response header - should merge constraints (string with pattern and minLength)
971+
expect(responseHeaders[2]).to.have.property('keyName', 'X-Content-Version');
972+
expect(responseHeaders[2].properties).to.have.property('type', 'string');
973+
expect(responseHeaders[2].properties).to.have.property('pattern', '^v\\d+\\.\\d+\\.\\d+$');
974+
expect(responseHeaders[2].properties).to.have.property('minLength', 5);
975+
expect(responseHeaders[2].properties).to.not.have.property('allOf');
976+
977+
done();
978+
});
979+
});
767980
});
768981

769982
});

0 commit comments

Comments
 (0)