Skip to content

Commit 8169480

Browse files
authored
Fix simplifySingleMapSchema to generate named wrapper schemas (opensearch-project#406)
* Fix simplifySingleMapSchema to generate named wrapper schemas Signed-off-by: xil <fridalu66@gmail.com> * Add old behavior Signed-off-by: xil <fridalu66@gmail.com> * add ut Signed-off-by: xil <fridalu66@gmail.com> --------- Signed-off-by: xil <fridalu66@gmail.com>
1 parent 203886f commit 8169480

10 files changed

Lines changed: 1179 additions & 157 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
66
### Added
77

88
### Changed
9-
9+
- Fix simplifySingleMapSchema to generate named wrapper schemas. ([#406](https://github.com/opensearch-project/opensearch-protobufs/pull/406))
1010
### Removed
1111

1212
### Fixed

DEVELOPER_GUIDE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,8 @@ npm config set registry https://registry.npmjs.org/
234234
3. **Generate Protobuf**
235235

236236
```bash
237-
export OPENAPI_GENERATOR_VERSION=7.19.0
238-
npx @openapitools/openapi-generator-cli generate \
237+
export OPENAPI_GENERATOR_VERSION=7.19.0
238+
npx @openapitools/openapi-generator-cli generate \
239239
-c tools/proto-convert/src/config/protobuf-generator-config.yaml
240240
```
241241

package-lock.json

Lines changed: 18 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
"ajv": "^8.18.0",
3030
"ajv-errors": "^3.0.0",
3131
"ajv-formats": "^3.0.1",
32-
"aws4-axios": "^3.3.7",
32+
"aws4-axios": "^3.4.0",
3333
"axios": "^1.13.5",
3434
"axios-mock-adapter": "^2.1.0",
3535
"cbor": "^9.0.2",
@@ -81,6 +81,7 @@
8181
"eslint": "^9.39.1",
8282
"eslint-plugin-jest": "^29.2.1",
8383
"@apidevtools/swagger-parser": "^12.1.0",
84-
"json-schema-to-typescript": "^15.0.4"
84+
"json-schema-to-typescript": "^15.0.4",
85+
"fast-xml-parser": ">=5.3.8"
8586
}
8687
}

tools/proto-convert/src/SchemaModifier.ts

Lines changed: 101 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import type {OpenAPIV3} from "openapi-types";
22
import {traverse} from './utils/OpenApiTraverser';
33
import isEqual from 'lodash.isequal';
4-
import {compressMultipleUnderscores, isPrimitiveType, resolveObj, isReferenceObject, isEmptyObjectSchema, is_simple_ref} from './utils/helper';
4+
import {compressMultipleUnderscores, isPrimitiveType, resolveObj, isReferenceObject, isEmptyObjectSchema, is_simple_ref, toSnakeCase} from './utils/helper';
55
import logger from "./utils/logger";
66

77

88
const DEFAULT_MAP_KEY = 'field' // default key for simplified additionalProperties
99
const DEFAULT_MAP_VALUE = 'value' // default value for simplified additionalProperties
10+
const QUERY_CONTAINER_SCHEMA_NAME = 'QueryContainer' // schema name that requires special inline handling.
1011

1112
export class SchemaModifier {
1213
root: OpenAPIV3.Document;
@@ -21,7 +22,6 @@ export class SchemaModifier {
2122
this.convertNullTypeToNullValue(schema)
2223
this.deduplicateOneOfWithArrayType(schema)
2324
this.collapseSingleItemComposite(schema);
24-
this.removeArrayOfMapWrapper(schema)
2525
},
2626
onSchema: (schema, schemaName) => {
2727
if (!schema || isReferenceObject(schema)) return;
@@ -33,14 +33,13 @@ export class SchemaModifier {
3333
this.deduplicateOneOfWithArrayType(schema)
3434
this.collapseSingleItemComposite(schema);
3535
this.collapseOneOfObjectPropContainsTitleSchema(schema)
36-
this.removeArrayOfMapWrapper(schema)
3736
this.convertOneOfToMinMaxProperties(schema)
3837
},
3938
});
4039
const visit = new Set();
4140
traverse(this.root, {
42-
onSchemaProperty: (schema) => {
43-
this.simplifySingleMapSchema(schema, visit);
41+
onSchemaProperty: (schema, propertyName, parentSchemaName) => {
42+
this.simplifySingleMapSchema(schema, visit, parentSchemaName);
4443
this.handleAdditionalPropertiesUndefined(schema)
4544

4645
},
@@ -280,53 +279,120 @@ export class SchemaModifier {
280279
}
281280

282281
/**
283-
* Transforms SchemaObject that single-key maps (`minProperties = 1` and `maxProperties = 1`) into standard schema by reconstructing
284-
* the additional property definitions.
285-
* Example:
286-
* Input:
282+
* Extracts type name from $ref or title
283+
**/
284+
private getTypeName(schema: OpenAPIV3.SchemaObject | OpenAPIV3.ReferenceObject): string | null {
285+
if ('$ref' in schema) {
286+
const parts = schema.$ref.split('/');
287+
return parts[parts.length - 1];
288+
}
289+
if ('title' in schema && schema.title) {
290+
return schema.title;
291+
}
292+
return null;
293+
}
294+
295+
/**
296+
* Transforms SchemaObject that single-key maps (`minProperties = 1` and `maxProperties = 1`) into a $ref to a new map schema.
297+
*
298+
* Input (items of DecayFunction):
287299
* {
288300
* type: "object",
301+
* propertyNames: { title: "field", type: "string" },
289302
* additionalProperties: {
290-
* - ref: "#/components/schemas/Model"
303+
* title: "placement",
304+
* $ref: "#/components/schemas/DecayPlacement"
291305
* },
292306
* minProperties: 1,
293307
* maxProperties: 1,
294-
* };
295-
* Model:
296-
* properties: {
297-
* properties1: string
298-
* properties2: string
299-
* }
300-
*
308+
* }
301309
*
302-
*Output:
303-
* {
304-
* ref: "#/components/schemas/Example
305-
* }
306-
*
307-
* Model:
308-
* properties: {
309-
* field: string
310-
* properties1: string
311-
* properties2: string
312-
* }
310+
* Output:
311+
* - Replaces with: { $ref: "#/components/schemas/DecayPlacementSingleMap" }
312+
* - Creates new schema DecayPlacementSingleMap:
313+
* {
314+
* type: "object",
315+
* properties: {
316+
* field: { type: "string" },
317+
* decay_placement: {
318+
* title: "placement",
319+
* $ref: "#/components/schemas/DecayPlacement"
320+
* }
321+
* },
322+
* required: ["field", "decay_placement"]
323+
* }
313324
*
314325
**/
315-
simplifySingleMapSchema(schema: OpenAPIV3.SchemaObject, visit: Set<any>): void {
326+
simplifySingleMapSchema(schema: OpenAPIV3.SchemaObject, visit: Set<any>, parentSchemaName?: string): void {
316327
if (schema.type === 'object' && typeof schema.additionalProperties === 'object' &&
317328
!Array.isArray(schema.additionalProperties) && schema.minProperties === 1 && schema.maxProperties === 1){
318329

319-
const reconstructAdditionalPropertySchema = this.reconstructAdditionalPropertySchema(schema.additionalProperties, visit);
330+
// Check if this is a QueryContainer property
331+
// If so, use the old inline behavior to avoid breaking QueryContainer structure
332+
// TODO: Remove this special case in next major release and use SingleMap wrapper for all schemas
333+
if (parentSchemaName === QUERY_CONTAINER_SCHEMA_NAME) {
334+
logger.info(`Using inline modification for ${QUERY_CONTAINER_SCHEMA_NAME} property (legacy behavior)`+JSON.stringify(schema));
335+
// Old behavior: inline modification
336+
const reconstructAdditionalPropertySchema = this.reconstructAdditionalPropertySchema(schema.additionalProperties, visit);
337+
Object.assign(schema, reconstructAdditionalPropertySchema);
338+
} else {
339+
// New behavior: create intermediate SingleMap schema
340+
const valueSchema = schema.additionalProperties;
341+
const typeName = this.getTypeName(valueSchema) || 'Value';
342+
343+
// Extract field property name and schema from propertyNames
344+
let fieldPropertyName = 'field';
345+
let fieldPropertySchema: any = { type: 'string' as const };
346+
347+
if ((schema as any).propertyNames) {
348+
const propertyNames = (schema as any).propertyNames;
349+
350+
// If propertyNames has a title, use it as the property name
351+
if (propertyNames.title && typeof propertyNames.title === 'string') {
352+
fieldPropertyName = propertyNames.title;
353+
354+
fieldPropertySchema = { ...propertyNames };
355+
delete fieldPropertySchema.title;
356+
} else {
357+
// Use propertyNames as-is for the field schema
358+
fieldPropertySchema = propertyNames;
359+
}
360+
}
361+
362+
// Create new map schema name
363+
const mapSchemaName = `${typeName}SingleMap`;
364+
365+
// Create the new map schema
366+
const newMapSchema: OpenAPIV3.SchemaObject = {
367+
type: 'object',
368+
properties: {
369+
[fieldPropertyName]: fieldPropertySchema,
370+
[toSnakeCase(typeName)]: valueSchema
371+
},
372+
required: [fieldPropertyName, toSnakeCase(typeName)]
373+
};
374+
375+
// Add to components.schemas if not already exists
376+
if (!this.root.components) {
377+
this.root.components = {};
378+
}
379+
if (!this.root.components.schemas) {
380+
this.root.components.schemas = {};
381+
}
382+
if (!this.root.components.schemas[mapSchemaName]) {
383+
this.root.components.schemas[mapSchemaName] = newMapSchema;
384+
}
320385

321-
Object.assign(schema, reconstructAdditionalPropertySchema)
386+
// Replace current schema with $ref to the new map schema
387+
(schema as any).$ref = `#/components/schemas/${mapSchemaName}`;
388+
}
322389

390+
// Cleanup properties for both paths (keep vendor extensions like x-*)
391+
delete schema.type;
323392
delete schema.additionalProperties;
324393
delete schema.minProperties;
325394
delete schema.maxProperties;
326-
delete schema.type
327-
if ('propertyNames' in schema) {
328-
delete schema.propertyNames;
329-
}
395+
delete (schema as any).propertyNames
330396
}
331397
}
332398

@@ -449,44 +515,6 @@ export class SchemaModifier {
449515
logger.info(`Converted additionalProperties to named property '${propertyName}' with type: object`);
450516
}
451517

452-
/**
453-
* Removes the array wrapper if the schema is an array of maps (additionalProperties).
454-
* Converts array of objects with only additionalProperties into just the additionalProperties schema.
455-
*
456-
* Example:
457-
* Input:
458-
* {
459-
* type: "array",
460-
* items: {
461-
* type: "object",
462-
* additionalProperties: {
463-
* $ref: "#/components/schemas/Value"
464-
* }
465-
* }
466-
* }
467-
*
468-
* Output:
469-
* {
470-
* type: "object",
471-
* additionalProperties: {
472-
* $ref: "#/components/schemas/Value"
473-
* }
474-
* }
475-
**/
476-
removeArrayOfMapWrapper(schema: OpenAPIV3.SchemaObject): void {
477-
if (schema.type === 'array' && schema.items && typeof schema.items === 'object' && !('$ref' in schema.items)) {
478-
const items = schema.items as OpenAPIV3.SchemaObject;
479-
480-
if (items.type === 'object' && items.additionalProperties && !items.properties) {
481-
(schema as any).type = 'object';
482-
schema.additionalProperties = items.additionalProperties;
483-
delete (schema as any).items;
484-
485-
logger.info(`Removed array wrapper from array of maps schema`);
486-
}
487-
}
488-
}
489-
490518
/**
491519
* Converts oneOf pattern with single-property objects into minProperties/maxProperties pattern.
492520
* For AggregationContainer

tools/proto-convert/src/utils/OpenApiTraverser.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ export type SchemaVisitorSet = {
55
onResponseSchema?: (schema: OpenAPIV3.SchemaObject, name: string) => void;
66
onRequestSchema?: (schema: OpenAPIV3.SchemaObject, name: string) => void;
77
onParameter?: (param: OpenAPIV3.ParameterObject, name: string) => void;
8-
onSchemaProperty?: (schema: OpenAPIV3.SchemaObject, propertyName: string) => void;
8+
onSchemaProperty?: (schema: OpenAPIV3.SchemaObject, propertyName: string, parentSchemaName?: string) => void;
99
};
1010

1111
/**
@@ -24,7 +24,7 @@ export function traverse(
2424
const schema = components.schemas[schemaName];
2525
visitors.onSchema?.(schema, schemaName);
2626
if (!('$ref' in schema)) {
27-
traverseSchema(schema, visitors);
27+
traverseSchema(schema, visitors, schemaName);
2828
}
2929
}
3030
}
@@ -79,7 +79,8 @@ export function traverse(
7979
*/
8080
export function traverseSchema(
8181
schema: OpenAPIV3.SchemaObject,
82-
visitors: SchemaVisitorSet
82+
visitors: SchemaVisitorSet,
83+
parentSchemaName?: string
8384
): void {
8485
if (!schema) return;
8586

@@ -88,21 +89,23 @@ export function traverseSchema(
8889
for (const propName in schema.properties) {
8990
const propSchema = schema.properties[propName];
9091
if (!('$ref' in propSchema)) {
91-
visitors.onSchemaProperty?.(propSchema, propName);
92-
traverseSchema(propSchema, visitors); // recurse into nested properties
92+
visitors.onSchemaProperty?.(propSchema, propName, parentSchemaName);
93+
traverseSchema(propSchema, visitors, parentSchemaName); // recurse into nested properties
9394
}
9495
}
9596
}
9697

9798
// Visit items (for array types)
9899
if (schema.type === 'array' && schema.items && typeof schema.items === 'object' && !('$ref' in schema.items)) {
99-
traverseSchema(schema.items as OpenAPIV3.SchemaObject, visitors);
100+
const itemsSchema = schema.items as OpenAPIV3.SchemaObject;
101+
visitors.onSchemaProperty?.(itemsSchema, 'items', parentSchemaName);
102+
traverseSchema(itemsSchema, visitors, parentSchemaName);
100103
}
101104

102105
// Visit additionalProperties
103106
if (schema.additionalProperties && typeof schema.additionalProperties === 'object' && !('$ref' in schema.additionalProperties)
104107
) {
105-
traverseSchema(schema.additionalProperties as OpenAPIV3.SchemaObject, visitors);
108+
traverseSchema(schema.additionalProperties as OpenAPIV3.SchemaObject, visitors, parentSchemaName);
106109
}
107110

108111
// Visit composed schemas
@@ -113,7 +116,7 @@ export function traverseSchema(
113116
for (const sub of subschemas) {
114117
if (!('$ref' in sub)) {
115118
visitors.onSchema?.(sub, `${key}`);
116-
traverseSchema(sub, visitors);
119+
traverseSchema(sub, visitors, parentSchemaName);
117120
}
118121
}
119122
}

0 commit comments

Comments
 (0)