Skip to content

Commit 792d3b4

Browse files
barshan23barshan23
authored
Refactor schema handling to preserve order of required properties (#903)
* Refactor schema handling to preserve order of required properties and improve handling of schemas without required properties * Remove redundant comment about defaulting to empty required array in request body validation * Enhance schema processing in tests to ensure properties are preserved and required arrays are handled correctly
1 parent 0d8d3b8 commit 792d3b4

File tree

2 files changed

+224
-79
lines changed

2 files changed

+224
-79
lines changed

‎libV2/schemaUtils.js‎

Lines changed: 80 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -691,33 +691,37 @@ let QUERYPARAM = 'query',
691691
let resolvedSchemaProps = {},
692692
{ includeDeprecated } = context.computedOptions;
693693

694-
_.forOwn(schema.properties, (property, propertyName) => {
695-
// Skip property resolution if it's not schema object
696-
if (!_.isObject(property)) {
697-
return;
698-
}
694+
// Only process properties if the schema has a properties field
695+
if (schema.hasOwnProperty('properties')) {
696+
_.forOwn(schema.properties, (property, propertyName) => {
697+
// Skip property resolution if it's not schema object
698+
if (!_.isObject(property)) {
699+
return;
700+
}
699701

700-
if (
701-
property.format === 'decimal' ||
702-
property.format === 'byte' ||
703-
property.format === 'password' ||
704-
property.format === 'unix-time'
705-
) {
706-
delete property.format;
707-
}
702+
if (
703+
property.format === 'decimal' ||
704+
property.format === 'byte' ||
705+
property.format === 'password' ||
706+
property.format === 'unix-time'
707+
) {
708+
delete property.format;
709+
}
708710

709-
// Skip addition of deprecated properties based on provided options
710-
if (!includeDeprecated && property.deprecated) {
711-
return;
712-
}
711+
// Skip addition of deprecated properties based on provided options
712+
if (!includeDeprecated && property.deprecated) {
713+
return;
714+
}
713715

714-
const currentPropPath = utils.addToJsonPath(currentPath, ['properties', propertyName]);
716+
const currentPropPath = utils.addToJsonPath(currentPath, ['properties', propertyName]);
715717

716-
resolvedSchemaProps[propertyName] = _resolveSchema(context, property, stack, resolveFor,
717-
_.cloneDeep(seenRef), currentPropPath);
718-
});
718+
resolvedSchemaProps[propertyName] = _resolveSchema(context, property, stack, resolveFor,
719+
_.cloneDeep(seenRef), currentPropPath);
720+
});
719721

720-
schema.properties = resolvedSchemaProps;
722+
schema.properties = resolvedSchemaProps;
723+
}
724+
721725
schema.type = schema.type || SCHEMA_TYPES.object;
722726
}
723727
// If schema is of type array
@@ -827,73 +831,70 @@ let QUERYPARAM = 'query',
827831
};
828832
}
829833

830-
if (resolvedSchema.type === 'object' && resolvedSchema.properties) {
834+
if (resolvedSchema.type === 'object') {
831835
const schemaDetails = {
832836
description: resolvedSchema.description,
833837
title: resolvedSchema.title,
834-
type: resolvedSchema.type,
835-
properties: {},
836-
required: []
837-
},
838-
requiredProperties = new Set(resolvedSchema.required || []);
839-
840-
for (let [propName, propValue] of Object.entries(resolvedSchema.properties)) {
841-
if (!propValue.type && !propValue.anyOf && !propValue.oneOf && !propValue.allOf) {
842-
continue;
843-
}
844-
const propertyDetails = {
845-
type: propValue.type,
846-
deprecated: propValue.deprecated,
847-
enum: propValue.enum || undefined,
848-
minLength: propValue.minLength,
849-
maxLength: propValue.maxLength,
850-
minimum: propValue.minimum,
851-
maximum: propValue.maximum,
852-
pattern: propValue.pattern,
853-
example: propValue.example,
854-
title: propValue.title,
855-
description: propValue.description,
856-
format: propValue.format
838+
type: resolvedSchema.type
857839
};
858840

859-
if (requiredProperties.has(propName)) {
860-
schemaDetails.required.push(propName);
861-
}
841+
// Only include properties if they exist in the original schema
842+
if (resolvedSchema.hasOwnProperty('properties')) {
843+
schemaDetails.properties = {};
862844

863-
if (propValue.anyOf) {
864-
propertyDetails.anyOf = propValue.anyOf.map((schema) => {
865-
return processSchema(schema);
866-
});
867-
}
868-
else if (propValue.oneOf) {
869-
propertyDetails.oneOf = propValue.oneOf.map((schema) => {
870-
return processSchema(schema);
871-
});
872-
}
873-
else if (propValue.allOf) {
874-
propertyDetails.allOf = propValue.allOf.map((schema) => {
875-
return processSchema(schema);
876-
});
877-
}
878-
else if (propValue.properties) {
879-
let processedProperties = processSchema(propValue);
880-
propertyDetails.properties = processedProperties.properties;
881-
if (processedProperties.required) {
882-
propertyDetails.required = processedProperties.required;
845+
for (let [propName, propValue] of Object.entries(resolvedSchema.properties)) {
846+
if (!propValue.type && !propValue.anyOf && !propValue.oneOf && !propValue.allOf) {
847+
continue;
883848
}
884-
}
885-
else if (propValue.type === 'array' && propValue.items) {
886-
propertyDetails.items = processSchema(propValue.items);
887-
}
849+
const propertyDetails = {
850+
type: propValue.type,
851+
deprecated: propValue.deprecated,
852+
enum: propValue.enum || undefined,
853+
minLength: propValue.minLength,
854+
maxLength: propValue.maxLength,
855+
minimum: propValue.minimum,
856+
maximum: propValue.maximum,
857+
pattern: propValue.pattern,
858+
example: propValue.example,
859+
title: propValue.title,
860+
description: propValue.description,
861+
format: propValue.format
862+
};
888863

889-
schemaDetails.properties[propName] = propertyDetails;
890-
}
891-
if (schemaDetails.required && schemaDetails.required.length === 0) {
892-
schemaDetails.required = undefined;
864+
if (propValue.anyOf) {
865+
propertyDetails.anyOf = propValue.anyOf.map((schema) => {
866+
return processSchema(schema);
867+
});
868+
}
869+
else if (propValue.oneOf) {
870+
propertyDetails.oneOf = propValue.oneOf.map((schema) => {
871+
return processSchema(schema);
872+
});
873+
}
874+
else if (propValue.allOf) {
875+
propertyDetails.allOf = propValue.allOf.map((schema) => {
876+
return processSchema(schema);
877+
});
878+
}
879+
else if (propValue.properties) {
880+
let processedProperties = processSchema(propValue);
881+
propertyDetails.properties = processedProperties.properties;
882+
if (processedProperties.required) {
883+
propertyDetails.required = processedProperties.required;
884+
}
885+
}
886+
else if (propValue.type === 'array' && propValue.items) {
887+
propertyDetails.items = processSchema(propValue.items);
888+
}
889+
890+
schemaDetails.properties[propName] = propertyDetails;
891+
}
893892
}
894-
if (schemaDetails.properties && Object.keys(schemaDetails.properties).length === 0) {
895-
schemaDetails.properties = undefined;
893+
894+
if (resolvedSchema.required) {
895+
schemaDetails.required = resolvedSchema.required;
896896
}
897+
897898
return schemaDetails;
898899
}
899900
else if (resolvedSchema.type === 'array' && resolvedSchema.items) {

‎test/unit/convertV2WithTypes.test.js‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,6 +1611,150 @@ describe('convertV2WithTypes', function() {
16111611
});
16121612
});
16131613

1614+
it('should preserve the original order of required properties from schema', function(done) {
1615+
const oas = {
1616+
openapi: '3.0.0',
1617+
info: { title: 'Required Properties Order Test', version: '1.0.0' },
1618+
paths: {
1619+
'/users': {
1620+
post: {
1621+
requestBody: {
1622+
content: {
1623+
'application/json': {
1624+
schema: {
1625+
type: 'object',
1626+
properties: {
1627+
id: { type: 'string' },
1628+
name: { type: 'string' },
1629+
email: { type: 'string' },
1630+
age: { type: 'integer' },
1631+
address: { type: 'string' }
1632+
},
1633+
// Intentionally define required properties in a different order than properties
1634+
required: ['email', 'name', 'id', 'address']
1635+
}
1636+
}
1637+
}
1638+
},
1639+
responses: {
1640+
'201': {
1641+
description: 'User created',
1642+
content: {
1643+
'application/json': {
1644+
schema: {
1645+
type: 'object',
1646+
properties: {
1647+
userId: { type: 'string' },
1648+
username: { type: 'string' },
1649+
createdAt: { type: 'string', format: 'date-time' },
1650+
profile: { type: 'object' }
1651+
},
1652+
// Different order for response schema as well
1653+
required: ['createdAt', 'userId', 'username']
1654+
}
1655+
}
1656+
}
1657+
}
1658+
}
1659+
}
1660+
}
1661+
}
1662+
};
1663+
1664+
Converter.convertV2WithTypes({ type: 'json', data: oas }, {}, (err, conversionResult) => {
1665+
expect(err).to.be.null;
1666+
expect(conversionResult.result).to.equal(true);
1667+
expect(conversionResult.extractedTypes).to.be.an('object').that.is.not.empty;
1668+
1669+
// Check request body required properties order
1670+
const requestBody = conversionResult.extractedTypes['post/users'].request.body;
1671+
const parsedRequestBody = JSON.parse(requestBody);
1672+
1673+
expect(parsedRequestBody).to.have.property('required');
1674+
expect(parsedRequestBody.required).to.deep.equal(['email', 'name', 'id', 'address']);
1675+
1676+
// Check response body required properties order
1677+
const responseBody = conversionResult.extractedTypes['post/users'].response['201'].body;
1678+
const parsedResponseBody = JSON.parse(responseBody);
1679+
1680+
expect(parsedResponseBody).to.have.property('required');
1681+
expect(parsedResponseBody.required).to.deep.equal(['createdAt', 'userId', 'username']);
1682+
1683+
done();
1684+
});
1685+
});
1686+
1687+
it('should handle schemas without required properties correctly', function(done) {
1688+
const oas = {
1689+
openapi: '3.0.0',
1690+
info: { title: 'No Required Properties Test', version: '1.0.0' },
1691+
paths: {
1692+
'/optional': {
1693+
post: {
1694+
requestBody: {
1695+
content: {
1696+
'application/json': {
1697+
schema: {
1698+
type: 'object',
1699+
properties: {
1700+
name: { type: 'string' },
1701+
age: { type: 'integer' }
1702+
}
1703+
// No required array defined
1704+
}
1705+
}
1706+
}
1707+
},
1708+
responses: {
1709+
'200': {
1710+
description: 'Success',
1711+
content: {
1712+
'application/json': {
1713+
schema: {
1714+
type: 'object',
1715+
properties: {
1716+
result: { type: 'string' }
1717+
},
1718+
required: [] // Empty required array
1719+
}
1720+
}
1721+
}
1722+
}
1723+
}
1724+
}
1725+
}
1726+
}
1727+
};
1728+
1729+
Converter.convertV2WithTypes({ type: 'json', data: oas }, {}, (err, conversionResult) => {
1730+
expect(err).to.be.null;
1731+
expect(conversionResult.result).to.equal(true);
1732+
expect(conversionResult.extractedTypes).to.be.an('object').that.is.not.empty;
1733+
1734+
// Check request body (schema with no required array)
1735+
const requestBody = conversionResult.extractedTypes['post/optional'].request.body;
1736+
const parsedRequestBody = JSON.parse(requestBody);
1737+
1738+
// Schema has properties object, so it should be preserved
1739+
expect(parsedRequestBody).to.have.property('properties');
1740+
expect(parsedRequestBody.properties).to.have.property('name');
1741+
expect(parsedRequestBody.properties).to.have.property('age');
1742+
// When no required array is defined in original schema, it should not be present in the output
1743+
expect(parsedRequestBody).to.not.have.property('required');
1744+
1745+
// Check response body (schema with empty required array)
1746+
const responseBody = conversionResult.extractedTypes['post/optional'].response['200'].body;
1747+
const parsedResponseBody = JSON.parse(responseBody);
1748+
1749+
expect(parsedResponseBody).to.have.property('properties');
1750+
expect(parsedResponseBody.properties).to.have.property('result');
1751+
expect(parsedResponseBody).to.have.property('required');
1752+
expect(parsedResponseBody.required).to.deep.equal([]);
1753+
1754+
done();
1755+
});
1756+
});
1757+
16141758
describe('4xx and 5xx response code normalization', function() {
16151759
it('should convert 4xx wildcard response code to 400 in collection while preserving type data', function(done) {
16161760
const oas = {

0 commit comments

Comments
 (0)