Skip to content

Commit da1262c

Browse files
committed
Updated deduplicateOneOfWithArrayType to preserve oneOf variants when they have different title attributes
Signed-off-by: xil <fridalu66@gmail.com>
1 parent 2f80528 commit da1262c

3 files changed

Lines changed: 134 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
1010
- Change vendorExtension protobuf type handling to use protobuf type instead of openApi type ([#409](https://github.com/opensearch-project/opensearch-protobufs/pull/409))
1111
- Normalize mixed oneOf patterns ([#416](https://github.com/opensearch-project/opensearch-protobufs/pull/416))
1212
- Add normalizeAnyOfInAllOf transformation to prevent protobuf generator from flattening allOf+anyOf structures ([#425](https://github.com/opensearch-project/opensearch-protobufs/pull/425))
13+
- Updated deduplicateOneOfWithArrayType to preserve oneOf variants when they have different title attributes
1314
### Removed
1415

1516
### Fixed

tools/proto-convert/src/SchemaModifier.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,24 +105,41 @@ export class SchemaModifier {
105105
// Simplify schemas with `oneOf` by aggregating items.
106106
// If there are only two `oneOf` items and one matches an array schema, remove oneOf type and set type to array.
107107
// If there are more than two `oneOf` items and one matches an array schema, remove that item from `oneOf`.
108+
// IMPORTANT: Skips deduplication if BOTH single and array items have titles AND they're different.
109+
// This preserves semantically different variants (e.g., 'regexp' vs 'terms', 'single' vs 'array').
108110
deduplicateOneOfWithArrayType(schema: OpenAPIV3.SchemaObject): void{
109111
if (!('$ref' in schema) && Array.isArray(schema.oneOf)) {
110112
const oneOfs = schema.oneOf;
111113

112-
const arraySet = new Set<string>();
114+
const arrayMap = new Map<string, any>(); // signature -> array item
113115
var deleteIndx = -1;
114116

117+
// Collect all array items
115118
for (const oneOf of oneOfs) {
116119
if (this.isArraySchemaObject(oneOf)) {
117120
const { type, $ref, additionalProperties} = oneOf.items as any;
118121
const oneOfStr = JSON.stringify({ type, $ref, additionalProperties});
119-
arraySet.add(oneOfStr)
122+
arrayMap.set(oneOfStr, oneOf);
120123
}
121124
}
125+
126+
// Find matching single items and check titles before removing
122127
for (const oneOf of oneOfs) {
123128
const { type, $ref, additionalProperties} = oneOf as any;
124129
const oneOfStr = JSON.stringify({ type, $ref, additionalProperties});
125-
if (arraySet.has(oneOfStr)) {
130+
131+
if (arrayMap.has(oneOfStr)) {
132+
const arrayItem = arrayMap.get(oneOfStr);
133+
const singleTitle = (oneOf as any).title;
134+
const arrayTitle = arrayItem.title;
135+
136+
// Only skip if BOTH have titles AND they're different
137+
if (singleTitle && arrayTitle && singleTitle !== arrayTitle) {
138+
logger.info(`Skipping deduplicateOneOfWithArrayType: different titles '${singleTitle}' vs '${arrayTitle}'`);
139+
continue;
140+
}
141+
142+
// Same title or at least one has no title -> proceed with deduplication
126143
deleteIndx = oneOfs.findIndex(item => isEqual(item, oneOf));
127144
oneOfs.splice(deleteIndx, 1);
128145
}

tools/proto-convert/test/SchemaModifier.test.ts

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,119 @@ describe('SchemaModifier', () => {
416416
expect(schema.oneOf).toContainEqual({ type: 'boolean' });
417417
expect(schema.oneOf).toContainEqual({ type: 'array', items: { type: 'string' } });
418418
});
419+
420+
it('should NOT remove single item when it has different title from array item', () => {
421+
const doc = createDocument();
422+
const modifier = new SchemaModifier(doc);
423+
424+
const schema: any = {
425+
oneOf: [
426+
{ title: 'regexp', type: 'string' },
427+
{ title: 'terms', type: 'array', items: { type: 'string' } }
428+
]
429+
};
430+
431+
modifier.deduplicateOneOfWithArrayType(schema);
432+
433+
// Both items should be preserved because titles differ
434+
expect(schema.oneOf).toHaveLength(2);
435+
expect(schema.oneOf[0]).toEqual({ title: 'regexp', type: 'string' });
436+
expect(schema.oneOf[1]).toEqual({ title: 'terms', type: 'array', items: { type: 'string' } });
437+
});
438+
439+
it('should NOT remove single item when titles differ (BucketsPath pattern)', () => {
440+
const doc = createDocument();
441+
const modifier = new SchemaModifier(doc);
442+
443+
const schema: any = {
444+
oneOf: [
445+
{ title: 'single', type: 'string' },
446+
{ title: 'array', type: 'array', items: { type: 'string' } },
447+
{ title: 'dict', type: 'object', additionalProperties: { type: 'string' } }
448+
]
449+
};
450+
451+
modifier.deduplicateOneOfWithArrayType(schema);
452+
453+
// All items should be preserved because single and array have different titles
454+
expect(schema.oneOf).toHaveLength(3);
455+
expect(schema.oneOf).toContainEqual({ title: 'single', type: 'string' });
456+
expect(schema.oneOf).toContainEqual({ title: 'array', type: 'array', items: { type: 'string' } });
457+
expect(schema.oneOf).toContainEqual({ title: 'dict', type: 'object', additionalProperties: { type: 'string' } });
458+
});
459+
460+
it('should remove single item when both have the same title', () => {
461+
const doc = createDocument();
462+
const modifier = new SchemaModifier(doc);
463+
464+
const schema: any = {
465+
oneOf: [
466+
{ title: 'value', type: 'string' },
467+
{ title: 'value', type: 'array', items: { type: 'string' } }
468+
]
469+
};
470+
471+
modifier.deduplicateOneOfWithArrayType(schema);
472+
473+
// Single item should be removed since titles match
474+
expect(schema.oneOf).toHaveLength(1);
475+
expect(schema.oneOf[0]).toEqual({ title: 'value', type: 'array', items: { type: 'string' } });
476+
});
477+
478+
it('should remove single item when only single item has title', () => {
479+
const doc = createDocument();
480+
const modifier = new SchemaModifier(doc);
481+
482+
const schema: any = {
483+
oneOf: [
484+
{ title: 'value', type: 'string' },
485+
{ type: 'array', items: { type: 'string' } }
486+
]
487+
};
488+
489+
modifier.deduplicateOneOfWithArrayType(schema);
490+
491+
// Single item should be removed (no conflict since array has no title)
492+
expect(schema.oneOf).toHaveLength(1);
493+
expect(schema.oneOf[0]).toEqual({ type: 'array', items: { type: 'string' } });
494+
});
495+
496+
it('should remove single item when only array item has title', () => {
497+
const doc = createDocument();
498+
const modifier = new SchemaModifier(doc);
499+
500+
const schema: any = {
501+
oneOf: [
502+
{ type: 'string' },
503+
{ title: 'array', type: 'array', items: { type: 'string' } }
504+
]
505+
};
506+
507+
modifier.deduplicateOneOfWithArrayType(schema);
508+
509+
// Single item should be removed (no conflict since single has no title)
510+
expect(schema.oneOf).toHaveLength(1);
511+
expect(schema.oneOf[0]).toEqual({ title: 'array', type: 'array', items: { type: 'string' } });
512+
});
513+
514+
it('should NOT remove when different titles with $ref types', () => {
515+
const doc = createDocument();
516+
const modifier = new SchemaModifier(doc);
517+
518+
const schema: any = {
519+
oneOf: [
520+
{ title: 'inline', $ref: '#/components/schemas/InlineScript' },
521+
{ title: 'stored', type: 'array', items: { $ref: '#/components/schemas/InlineScript' } }
522+
]
523+
};
524+
525+
modifier.deduplicateOneOfWithArrayType(schema);
526+
527+
// Both should be preserved because titles differ
528+
expect(schema.oneOf).toHaveLength(2);
529+
expect(schema.oneOf[0]).toEqual({ title: 'inline', $ref: '#/components/schemas/InlineScript' });
530+
expect(schema.oneOf[1]).toEqual({ title: 'stored', type: 'array', items: { $ref: '#/components/schemas/InlineScript' } });
531+
});
419532
});
420533

421534
describe('collapseSingleItemComposite', () => {

0 commit comments

Comments
 (0)