[Typescript] Generate oneOf schemas as type unions #19027

ksvirkou-hubspot · 2024-06-27T09:24:22Z

Description of the PR

I have implemented oneOf schemas as a type union and a oneOf class with a discriminator and mapping, similar to PR 2617 and PR 2647, but for the TypeScript generator.

The type union is used for methods like request/response types and for models as a type of parameters. The oneOf class is used for the deserialization process to determine the type of an object from the type union. It checks the discriminator in the mapping list, then in the list of all classes.

Before this PR, the TypeScript generator created a new model with all fields from all oneOf models. The created model is not described in the swagger files. There are the following problems:

It doesn't work if oneOf models have fields with the same name but different types.
It doesn't work if oneOf models have enum fields with the same name but different values in the enum.
It doesn't work if oneOf has a mapping list.

If oneOf models contain required fields, a user has to set all required fields, even if they are not needed for the request.
example:
Dog's required fields:

openapi-generator/samples/openapi3/client/petstore/typescript/builds/composed-schemas/models/Dog.ts

Lines 15 to 17 in daf5222

    
           export class Dog { 
        
               'bark'?: boolean; 
        
               'breed'?: DogBreedEnum;

Cat's required fields:

openapi-generator/samples/openapi3/client/petstore/typescript/builds/composed-schemas/models/Cat.ts

Lines 15 to 17 in daf5222

    
           export class Cat { 
        
               'hunts'?: boolean; 
        
               'age'?: number;

oneOf model required fields :

openapi-generator/samples/openapi3/client/petstore/typescript/builds/composed-schemas/models/PetsPatchRequest.ts

Lines 17 to 21 in daf5222

    
           export class PetsPatchRequest { 
        
               'hunts'?: boolean; 
        
               'age'?: number; 
        
               'bark'?: boolean; 
        
               'breed'?: PetsPatchRequestBreedEnum;

if we sent Dog model we have to set all required fields to PetsPatchRequest(even cat's ones)

It doesn't look like oneOf. If generate docs by the swagger with "oneOf", it won’t match the generated docs.

The implementation "oneOf" schemas as a type union resolve all these problems, making the generated code accurate with the swagger specification.

Sample input:

"inputFieldDependencies" : {
    "type" : "array",
    "items" : {
        "oneOf" : [ {
            "$ref" : "#/components/schemas/PublicSingleFieldDependency"
        }, {
            "$ref" : "#/components/schemas/PublicConditionalSingleFieldDependency"
        } ],
        "discriminator": {
            "propertyName": "dependencyType",
            "mapping": {
                "SINGLE_FIELD": "#/components/schemas/PublicSingleFieldDependency",
                "CONDITIONAL_SINGLE_FIELD": "#/components/schemas/PublicConditionalSingleFieldDependency"
            }
        }
    }
},

Sample output:

import { PublicConditionalSingleFieldDependency } from '../models/PublicConditionalSingleFieldDependency';
import { PublicSingleFieldDependency } from '../models/PublicSingleFieldDependency';

export type PublicActionDefinitionInputFieldDependenciesInner = PublicConditionalSingleFieldDependency | PublicSingleFieldDependency;

export class PublicActionDefinitionInputFieldDependenciesInnerClass {
    static readonly discriminator: string | undefined = "dependencyType";

    static readonly mapping: {[index: string]: string} | undefined = {
        "CONDITIONAL_SINGLE_FIELD": "PublicConditionalSingleFieldDependency",
        "SINGLE_FIELD": "PublicSingleFieldDependency",
    };
}

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.

Run the following to build the project and update samples:

./mvnw clean package 
./bin/generate-samples.sh ./bin/configs/*.yaml
./bin/utils/export_docs_generators.sh

File the PR against the correct branch: master (upcoming 7.6.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Related issues:

#9305

ksvirkou-hubspot · 2024-06-27T09:25:31Z

@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @topce @akehir @petejohansonxo @amakhrov @davidgamero @mkusaka

macjohnny · 2024-07-02T09:23:22Z

modules/openapi-generator/src/main/resources/typescript/model/model.mustache

@@ -12,7 +12,7 @@ import { HttpFile } from '../http/http{{importFileExtension}}';
 */
 {{/description}}
 {{^isEnum}}
-export class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{
+{{#oneOf}}{{#-first}}{{>model/modelOneOf}}{{/-first}}{{/oneOf}}{{^oneOf}}export class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{


Suggested change

{{#oneOf}}{{#-first}}{{>model/modelOneOf}}{{/-first}}{{/oneOf}}{{^oneOf}}export class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{

{{#oneOf}}

{{#-first}}{{>model/modelOneOf}}{{/-first}}

{{/oneOf}}

{{^oneOf}}

export class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{

macjohnny · 2024-07-02T09:27:11Z

samples/openapi3/client/petstore/typescript/builds/composed-schemas/models/PetsPatchRequest.ts

-    'bark'?: boolean;
-    'breed'?: PetsPatchRequestBreedEnum;
-
-    static readonly discriminator: string | undefined = "petType";


this changes the discriminator value to undefined, since we have

openapi-generator/samples/openapi3/client/petstore/typescript/builds/composed-schemas/models/Dog.ts

Line 19 in 8473d47

static readonly discriminator: string | undefined = undefined;

is this intentional?

Yes, intentional
I've created type and class:

/** * @type PetsPatchRequest * Type * @export */ export type PetsPatchRequest = Cat | Dog; /** * @type PetsPatchRequestClass * @export */ export class PetsPatchRequestClass { static readonly discriminator: string | undefined = "petType"; static readonly mapping: {[index: string]: string} | undefined = undefined; }

https://github.com/OpenAPITools/openapi-generator/pull/19027/files#diff-531a756a09a9bb02d3916047a7b0c39dc8caf49c638c9c74391ad432166cd579R25

this does not seem to be backwards compatible, is it?

I'm not sure, because now it isn't work at all
Should I make a PR to 8.0.x?

so you are saying the currently generated classes (before this PR) for the typescript generator dont work? what does not work?

just so i correctly understand, can you update the PR description with the motivation why this change is needed, what use case it solves, and why there are two types generated (the union and the class), and why the class does not have any fields besides a mapping and discriminator?

I've updated the PR description.
Could you check it, please?

Any updates?
@macjohnny

Any updates?
@macjohnny @wing328

Will check soon. Sorry, for the delay

wing328 · 2024-07-10T14:56:02Z

Generate oneOf schemas as type unions

@ksvirkou-hubspot given that this implementation is for oneOf, will this implementation throw an error if the JSON payload for example matches both models/schemes (e.g. Pet, Animal) defined in oneOf?

ksvirkou-hubspot · 2024-07-25T10:39:10Z

Generate oneOf schemas as type unions

@ksvirkou-hubspot given that this implementation is for oneOf, will this implementation throw an error if the JSON payload for example matches both models/schemes (e.g. Pet, Animal) defined in oneOf?

Let me clarify the way ... should work.
Discriminator is a field by which the generated code can determine what model is in the JSON payload.
Example of swagger:

oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  discriminator:
    propertyName: petType

Example of payload:

{
  "id": 123,
  "petType": "Cat",
}

In this case : Cat model.
Considering what I've described, I don't think I fully understand the scenario you've meant. Could you share the swagger and the json payload to reproduce the case?

ksvirkou-hubspot · 2024-07-25T11:19:46Z

Could you check it, please?
@macjohnny @wing328

joscha · 2024-08-30T13:50:44Z

I ran into a similar issue in planet-a-ventures/affinity-node#35 and was passed on this PR by @macjohnny.

I tried regenerating with the changes in this PR and it changes a broken implementation of https://github.com/planet-a-ventures/affinity-node/blob/32fd4f2deea3cb7723fca17353bb35d6b2d19bda/openapi/2024-08-28.json

into a working one with correct discrimination of oneOfs.

Here is the runtime output of the code generated from current master:

ListEntryWithEntityPaged {
  data: [
    ListEntryWithEntity {
      type: "company",
      id: 167627852,
      createdAt: 2024-08-30T10:37:56.000Z,
      creatorId: 54576635,
      entity: Person {
        id: 297186243,
        firstName: undefined,
        lastName: undefined,
        primaryEmailAddress: undefined,
        emailAddresses: undefined,
        type: undefined,
        fields: [ [Field], [Field], [Field], [Field], [Field] ]
      }
    }
  ],
  pagination: Pagination { prevUrl: null, nextUrl: null }
}

please note how the entity has been deserialized into a class Person, albeit the discriminator (type) clearly states it has to be of type "company".

With the changes in this PR, this becomes:

ListEntryWithEntityPaged {
  data: [
    CompanyListEntry {
      id: 167627852,
      type: "company",
      createdAt: 2024-08-30T10:37:56.000Z,
      creatorId: 54576635,
      entity: Company {
        id: 297186243,
        name: "joschas testdealflow company",
        domain: "joscha.io",
        domains: [ "joscha.io" ],
        isGlobal: false,
        fields: [ [Field], [Field], [Field], [Field], [Field] ]
      }
    }
  ],
  pagination: Pagination { prevUrl: null, nextUrl: null }
}

which is correct (see how the entity is correctly of class Company).

I also noticed that this PR does make the changes in #19481 superfluous. E.g. if this PR is merged, #19481 (4238f17) can be reverted.

joscha · 2024-08-30T13:51:53Z

I created #19494 that has the necessary revert, upstream changes from master and regeneration of fixtures that have recently been added since this PR was opened.

joscha · 2024-08-30T13:57:27Z

This pull request either needs all commits from #19494 or #19494 can be merged and it will automatically close this one as well, as it contains all the commits. I tried opening #19494 directly against your fork @ksvirkou-hubspot, however because of the commits that have made it to master in the last month since this PR was last rebased, there are dozens of commits in the diff, so I opted to instead create it against origin.

@macjohnny I can confirm that the generated code in this PR fixes the issue discussed before, both from a semantic perspective when looking at the code as well as in runtime (tried it against the live API, see above), so we should try to get this PR in as soon as possible, if we can, I think it will help anyone with swagger definitions that contain oneOfs.

macjohnny · 2024-08-30T14:20:49Z

@ksvirkou-hubspot thanks for the fix! @joscha thanks for investigating and preparing the final PR. all merged.

joscha · 2024-09-06T10:43:20Z

@macjohnny I just noticed that this change seems to generate nullable enums with an extra 'null' string value.

E.g:

          "enrichmentSource": {
            "description": "The source of the data in this Field (if it is enriched)",
            "enum": [
              "affinity-data",
              "dealroom",
              null
            ],
            "example": "affinity-data",
            "type": "string",
            "nullable": true
          },

becomes

export enum FieldEnrichmentSourceEnum {
    AffinityData = 'affinity-data',
    Dealroom = 'dealroom',
    Null = 'null'
}

which is incorrect as 'null' as a string is not a valid enum value. I will try and produce a fix for this.

macjohnny · 2024-09-06T11:10:10Z

thanks for pointing this out and fixing!

joscha · 2024-09-06T13:27:22Z

thanks for pointing this out and fixing!

fix is in #19540

ksvirkou-hubspot added 2 commits June 25, 2024 12:36

Add oneOf model for Typescript generator

20d9f67

Update import procces: For oneOfs only import $refs within the oneOf

93a1693

ksvirkou-hubspot added 2 commits June 27, 2024 15:15

Remove new line after description

c36d1b3

Update samples

8473d47

macjohnny reviewed Jul 2, 2024

View reviewed changes

Typescript: Update model.mustache

8330958

ksvirkou-hubspot added 10 commits July 16, 2024 12:06

Typescript: Remove emun from oneOf models

b3c8ad7

Update samples

86730d6

Typescript oneOf: add discriminator and update deserialize procces

5e054ae

Typescript: update tests

04f0c94

Typescript oneOf: move type mapping to models

38e9852

Typescript: Update samples

5f5cfbb

Typescript: Update type of mappig

a9564d8

Typescript: Update type of mappig

e17cb91

Typescript oneOf: update deserializing logic

5e67395

Typescript: Update samples

885b776

joscha mentioned this pull request Aug 30, 2024

v2 initial implementation planet-a-ventures/affinity-node#35

Merged

joscha mentioned this pull request Aug 30, 2024

[Typescript] Generate oneOf schemas as type unions #19494

Merged

macjohnny closed this in #19494 Aug 30, 2024

macjohnny added the Client: TypeScript label Sep 3, 2024

joscha mentioned this pull request Sep 6, 2024

[typescript] fix: nullable enums should not serialize a null value to a string #19540

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Typescript] Generate oneOf schemas as type unions #19027

[Typescript] Generate oneOf schemas as type unions #19027

ksvirkou-hubspot commented Jun 27, 2024 •

edited

Loading

ksvirkou-hubspot commented Jun 27, 2024

macjohnny Jul 2, 2024

macjohnny Jul 2, 2024

ksvirkou-hubspot Jul 24, 2024 •

edited

Loading

macjohnny Jul 25, 2024

ksvirkou-hubspot Jul 25, 2024

macjohnny Jul 26, 2024

macjohnny Aug 2, 2024

ksvirkou-hubspot Aug 6, 2024

ksvirkou-hubspot Aug 14, 2024

ksvirkou-hubspot Aug 21, 2024

macjohnny Aug 23, 2024 •

edited

Loading

wing328 commented Jul 10, 2024

ksvirkou-hubspot commented Jul 25, 2024 •

edited

Loading

ksvirkou-hubspot commented Jul 25, 2024

joscha commented Aug 30, 2024

joscha commented Aug 30, 2024

joscha commented Aug 30, 2024

macjohnny commented Aug 30, 2024

joscha commented Sep 6, 2024 •

edited

Loading

macjohnny commented Sep 6, 2024

joscha commented Sep 6, 2024

	export class PetsPatchRequest {
	'hunts'?: boolean;
	'age'?: number;
	'bark'?: boolean;
	'breed'?: PetsPatchRequestBreedEnum;

[Typescript] Generate oneOf schemas as type unions #19027

[Typescript] Generate oneOf schemas as type unions #19027

Conversation

ksvirkou-hubspot commented Jun 27, 2024 • edited Loading

Description of the PR

PR checklist

Related issues:

ksvirkou-hubspot commented Jun 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksvirkou-hubspot Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macjohnny Aug 23, 2024 • edited Loading

Choose a reason for hiding this comment

wing328 commented Jul 10, 2024

ksvirkou-hubspot commented Jul 25, 2024 • edited Loading

ksvirkou-hubspot commented Jul 25, 2024

joscha commented Aug 30, 2024

joscha commented Aug 30, 2024

joscha commented Aug 30, 2024

macjohnny commented Aug 30, 2024

joscha commented Sep 6, 2024 • edited Loading

macjohnny commented Sep 6, 2024

joscha commented Sep 6, 2024

ksvirkou-hubspot commented Jun 27, 2024 •

edited

Loading

ksvirkou-hubspot Jul 24, 2024 •

edited

Loading

macjohnny Aug 23, 2024 •

edited

Loading

ksvirkou-hubspot commented Jul 25, 2024 •

edited

Loading

joscha commented Sep 6, 2024 •

edited

Loading