Skip to content

Commit 9212668

Browse files
mrlubosmrlubos
authored
Merge pull request #3490 from pgraug/fix/discriminator-allof-inline-mapping
fix: explicit discriminator mapping wins over fallback in nested `allOf`
2 parents ef871e4 + a67d589 commit 9212668

11 files changed

Lines changed: 373 additions & 55 deletions

File tree

‎.changeset/curly-wombats-deliver.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@hey-api/openapi-ts": patch
3+
"@hey-api/shared": patch
4+
---
5+
6+
**parser**: fix: explicit discriminator mapping wins over fallback in nested `allOf`

‎packages/openapi-ts-tests/main/test/3.0.x.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,14 @@ describe(`OpenAPI ${version}`, () => {
217217
}),
218218
description: 'handles nested allOf with discriminators',
219219
},
220+
{
221+
config: createConfig({
222+
input: 'discriminator-allof-inline.json',
223+
output: 'discriminator-allof-inline',
224+
}),
225+
description:
226+
'handles allOf where inline schema discriminator mapping should take priority over $ref discriminator fallback',
227+
},
220228
{
221229
config: createConfig({
222230
input: 'discriminator-non-string.yaml',

‎packages/openapi-ts-tests/main/test/3.1.x.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,14 @@ describe(`OpenAPI ${version}`, () => {
243243
}),
244244
description: 'handles nested allOf with discriminators',
245245
},
246+
{
247+
config: createConfig({
248+
input: 'discriminator-allof-inline.json',
249+
output: 'discriminator-allof-inline',
250+
}),
251+
description:
252+
'handles allOf where inline schema discriminator mapping should take priority over $ref discriminator fallback',
253+
},
246254
{
247255
config: createConfig({
248256
input: 'discriminator-non-string.yaml',

‎packages/openapi-ts-tests/main/test/__snapshots__/3.0.x/discriminator-allof-inline/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// This file is auto-generated by @hey-api/openapi-ts
2+
3+
export type { Bar, Baz, ClientOptions, Foo, GetFoosData, GetFoosResponse, GetFoosResponses, Qux } from './types.gen';

‎packages/openapi-ts-tests/main/test/__snapshots__/3.0.x/discriminator-allof-inline/types.gen.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// This file is auto-generated by @hey-api/openapi-ts
2+
3+
export type ClientOptions = {
4+
baseUrl: `${string}://${string}` | (string & {});
5+
};
6+
7+
export type Foo = {
8+
$type: string;
9+
foo?: string;
10+
};
11+
12+
export type Bar = Omit<Foo, '$type'> & {
13+
$type: 'FooBar';
14+
bar?: string;
15+
};
16+
17+
export type Baz = Omit<Foo, '$type'> & {
18+
baz?: string;
19+
$type: 'FooBaz';
20+
};
21+
22+
export type Qux = Omit<Bar, '$type'> & {
23+
qux?: string;
24+
$type: 'BarQux';
25+
};
26+
27+
export type GetFoosData = {
28+
body?: never;
29+
path?: never;
30+
query?: never;
31+
url: '/foos';
32+
};
33+
34+
export type GetFoosResponses = {
35+
/**
36+
* OK
37+
*/
38+
200: Bar | Baz | Qux;
39+
};
40+
41+
export type GetFoosResponse = GetFoosResponses[keyof GetFoosResponses];

‎packages/openapi-ts-tests/main/test/__snapshots__/3.1.x/discriminator-allof-inline/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// This file is auto-generated by @hey-api/openapi-ts
2+
3+
export type { Bar, Baz, ClientOptions, Foo, GetFoosData, GetFoosResponse, GetFoosResponses, Qux } from './types.gen';

‎packages/openapi-ts-tests/main/test/__snapshots__/3.1.x/discriminator-allof-inline/types.gen.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// This file is auto-generated by @hey-api/openapi-ts
2+
3+
export type ClientOptions = {
4+
baseUrl: `${string}://${string}` | (string & {});
5+
};
6+
7+
export type Foo = {
8+
$type: string;
9+
foo?: string;
10+
};
11+
12+
export type Bar = Omit<Foo, '$type'> & {
13+
$type: 'FooBar';
14+
bar?: string;
15+
};
16+
17+
export type Baz = Omit<Foo, '$type'> & {
18+
baz?: string;
19+
$type: 'FooBaz';
20+
};
21+
22+
export type Qux = Omit<Bar, '$type'> & {
23+
qux?: string;
24+
$type: 'BarQux';
25+
};
26+
27+
export type GetFoosData = {
28+
body?: never;
29+
path?: never;
30+
query?: never;
31+
url: '/foos';
32+
};
33+
34+
export type GetFoosResponses = {
35+
/**
36+
* OK
37+
*/
38+
200: Bar | Baz | Qux;
39+
};
40+
41+
export type GetFoosResponse = GetFoosResponses[keyof GetFoosResponses];

‎packages/shared/src/openApi/3.0.x/parser/schema.ts‎

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -436,11 +436,11 @@ const parseAllOf = ({
436436
// Collect discriminator information to add after all compositions are processed
437437
type DiscriminatorInfo = {
438438
discriminator: NonNullable<SchemaObject['discriminator']>;
439+
isExplicitMapping: boolean;
439440
isRequired: boolean;
440441
values: ReadonlyArray<string>;
441442
};
442443
const discriminatorsToAdd: Array<DiscriminatorInfo> = [];
443-
const addedDiscriminators = new Set<string>();
444444

445445
for (const compositionSchema of compositionSchemas) {
446446
const originalInAllOf = state.inAllOf;
@@ -478,13 +478,7 @@ const parseAllOf = ({
478478
schema: ref,
479479
});
480480

481-
// Process each discriminator found
482481
for (const { discriminator, oneOf } of discriminators) {
483-
// Skip if we've already collected this discriminator property
484-
if (addedDiscriminators.has(discriminator.propertyName)) {
485-
continue;
486-
}
487-
488482
const values = discriminatorValues(
489483
state.$ref,
490484
discriminator.mapping,
@@ -494,28 +488,48 @@ const parseAllOf = ({
494488
oneOf ? () => oneOf.some((o) => '$ref' in o && o.$ref === state.$ref) : undefined,
495489
);
496490

497-
if (values.length > 0) {
498-
// Check if the discriminator property is required in any of the discriminator schemas
499-
const isRequired = discriminators.some(
500-
(d) =>
501-
d.discriminator.propertyName === discriminator.propertyName &&
502-
// Check in the ref's required array or in the allOf components
503-
(ref.required?.includes(d.discriminator.propertyName) ||
504-
(ref.allOf &&
505-
ref.allOf.some((item) => {
506-
const resolvedItem =
507-
'$ref' in item ? context.resolveRef<SchemaObject>(item.$ref) : item;
508-
return resolvedItem.required?.includes(d.discriminator.propertyName);
509-
}))),
510-
);
491+
if (values.length === 0) {
492+
continue;
493+
}
511494

512-
discriminatorsToAdd.push({
513-
discriminator,
514-
isRequired,
515-
values,
516-
});
517-
addedDiscriminators.add(discriminator.propertyName);
495+
// True when state.$ref appears directly in the mapping; false when the
496+
// value fell back to the schema name because no mapping entry matched.
497+
const isExplicitMapping =
498+
discriminator.mapping !== undefined &&
499+
Object.values(discriminator.mapping).includes(state.$ref);
500+
501+
// An explicit mapping always beats a same-property fallback collected
502+
// earlier (e.g. from a grandparent discriminator that doesn't list this
503+
// schema). Replace it; otherwise skip the duplicate.
504+
const existingIndex = discriminatorsToAdd.findIndex(
505+
(d) => d.discriminator.propertyName === discriminator.propertyName,
506+
);
507+
if (existingIndex !== -1) {
508+
if (isExplicitMapping && !discriminatorsToAdd[existingIndex]!.isExplicitMapping) {
509+
discriminatorsToAdd.splice(existingIndex, 1);
510+
} else {
511+
continue;
512+
}
518513
}
514+
515+
const isRequired = discriminators.some(
516+
(d) =>
517+
d.discriminator.propertyName === discriminator.propertyName &&
518+
(ref.required?.includes(d.discriminator.propertyName) ||
519+
(ref.allOf &&
520+
ref.allOf.some((item) => {
521+
const resolvedItem =
522+
'$ref' in item ? context.resolveRef<SchemaObject>(item.$ref) : item;
523+
return resolvedItem.required?.includes(d.discriminator.propertyName);
524+
}))),
525+
);
526+
527+
discriminatorsToAdd.push({
528+
discriminator,
529+
isExplicitMapping,
530+
isRequired,
531+
values,
532+
});
519533
}
520534
}
521535
}

‎packages/shared/src/openApi/3.1.x/parser/schema.ts‎

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -531,11 +531,11 @@ const parseAllOf = ({
531531
// Collect discriminator information to add after all compositions are processed
532532
type DiscriminatorInfo = {
533533
discriminator: NonNullable<SchemaObject['discriminator']>;
534+
isExplicitMapping: boolean;
534535
isRequired: boolean;
535536
values: ReadonlyArray<string>;
536537
};
537538
const discriminatorsToAdd: Array<DiscriminatorInfo> = [];
538-
const addedDiscriminators = new Set<string>();
539539

540540
for (const compositionSchema of compositionSchemas) {
541541
const originalInAllOf = state.inAllOf;
@@ -573,13 +573,7 @@ const parseAllOf = ({
573573
schema: ref,
574574
});
575575

576-
// Process each discriminator found
577576
for (const { discriminator, oneOf } of discriminators) {
578-
// Skip if we've already collected this discriminator property
579-
if (addedDiscriminators.has(discriminator.propertyName)) {
580-
continue;
581-
}
582-
583577
const values = discriminatorValues(
584578
state.$ref,
585579
discriminator.mapping,
@@ -589,29 +583,49 @@ const parseAllOf = ({
589583
oneOf ? () => oneOf.some((o) => '$ref' in o && o.$ref === state.$ref) : undefined,
590584
);
591585

592-
if (values.length > 0) {
593-
// Check if the discriminator property is required in any of the discriminator schemas
594-
const isRequired = discriminators.some(
595-
(d) =>
596-
d.discriminator.propertyName === discriminator.propertyName &&
597-
// Check in the ref's required array or in the allOf components
598-
(ref.required?.includes(d.discriminator.propertyName) ||
599-
(ref.allOf &&
600-
ref.allOf.some((item) => {
601-
const resolvedItem = item.$ref
602-
? context.resolveRef<SchemaObject>(item.$ref)
603-
: item;
604-
return resolvedItem.required?.includes(d.discriminator.propertyName);
605-
}))),
606-
);
586+
if (values.length === 0) {
587+
continue;
588+
}
607589

608-
discriminatorsToAdd.push({
609-
discriminator,
610-
isRequired,
611-
values,
612-
});
613-
addedDiscriminators.add(discriminator.propertyName);
590+
// True when state.$ref appears directly in the mapping; false when the
591+
// value fell back to the schema name because no mapping entry matched.
592+
const isExplicitMapping =
593+
discriminator.mapping !== undefined &&
594+
Object.values(discriminator.mapping).includes(state.$ref);
595+
596+
// An explicit mapping always beats a same-property fallback collected
597+
// earlier (e.g. from a grandparent discriminator that doesn't list this
598+
// schema). Replace it; otherwise skip the duplicate.
599+
const existingIndex = discriminatorsToAdd.findIndex(
600+
(d) => d.discriminator.propertyName === discriminator.propertyName,
601+
);
602+
if (existingIndex !== -1) {
603+
if (isExplicitMapping && !discriminatorsToAdd[existingIndex]!.isExplicitMapping) {
604+
discriminatorsToAdd.splice(existingIndex, 1);
605+
} else {
606+
continue;
607+
}
614608
}
609+
610+
const isRequired = discriminators.some(
611+
(d) =>
612+
d.discriminator.propertyName === discriminator.propertyName &&
613+
(ref.required?.includes(d.discriminator.propertyName) ||
614+
(ref.allOf &&
615+
ref.allOf.some((item) => {
616+
const resolvedItem = item.$ref
617+
? context.resolveRef<SchemaObject>(item.$ref)
618+
: item;
619+
return resolvedItem.required?.includes(d.discriminator.propertyName);
620+
}))),
621+
);
622+
623+
discriminatorsToAdd.push({
624+
discriminator,
625+
isExplicitMapping,
626+
isRequired,
627+
values,
628+
});
615629
}
616630
}
617631
}

0 commit comments

Comments
 (0)