Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] typescript-fetch oneOf doesn't handle discrimination by property. #6513

Open
keean opened this issue Jun 1, 2020 · 5 comments
Open

Comments

@keean
Copy link

keean commented Jun 1, 2020

Description

OpenApi generator does not generate valid code for unions discriminated by property name. Unions (oneOf) work correctly when discriminated by a property value, however the generated code seems to try and combine the property lists from all members of the union. If each member of the union have disjoint sets of required properties, then the union is discriminated, and should be generated as such.

openapi-generator version

openapi-generator 4.3.1

OpenAPI declaration file content or url
{
    "openapi": "3.0",
    "info": {
        "title": "Test",
        "version": "0.1"
    },
    "servers": [{
        "url": "https://localhost"
    }],
    "paths": {
        "/": {
            "get": {
                "operationId": "dummyOperation",
                "responses": {
                    "200": {
                        "description": "Successful operaion",
                        "content": {
                            "application/json": {
                                "schema": {"$ref": "#/components/schemas/aOrB"}
                            }
                        }
                    }
                }
            }
        }
    },
    "components": {
        "schemas": {
            "a": {
                "type": "object",
                "required": ["a", "b"],
                "properties": {
                    "a": {
                        "type": "string"
                    },
                    "b": {
                        "type": "string"
                    }
                }
            },
            "b": {
                "type": "object",
                "required": ["b", "c"],
                "properties": {
                    "b": {
                        "type": "string"
                    },
                    "c": {
                        "type": "string"
                    }
                }
            },
            "aOrB": {
                "oneOf": [
                    {"$ref": "#/components/schemas/a"},
                    {"$ref": "#/components/schemas/b"}
                ]
            }
        }
    }
}
Command line used for generation

openapi-generator generate -i test.json -g typescript-fetch --additional-properties=typescriptThreePlus --generate-alias-as-model -o test

Steps to reproduce

After generation, the model file for aOrB is:

export function AOrBFromJSONTyped(json: any, ignoreDiscriminator: boolean): AOrB {
    if ((json === undefined) || (json === null)) {
        return json;
    }
    return { ...AFromJSONTyped(json, true), ...BFromJSONTyped(json, true) };
}

export function AOrBToJSON(value?: AOrB | null): any {
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    return { ...AToJSON(value), ...BToJSON(value) };
}

This fails because A and B both contain 'required' parameters, so they cannot be combined by spreading as above.

Related issues/PRs

There are similar PR, but they don't seem to address exactly the same issue:
#6376 #5202 #4626 #1563 #927

Suggest a fix

The generated functions need to discriminate on properties as well as values, something like:

export function AOrBFromJSONTyped(json: any, ignoreDiscriminator: boolean): AOrB {
    if ((json === undefined) || (json === null)) {
        return json;
    }
    if (json['a'] != undefined && json['b'] != undefined) {
        return AFromJSONTyped(json, true);
    } else if (json['b'] != undefined && json['c'] != undefined) {
        return BFromJSONTyped(json, true);
    }
}

export function AOrBToJSON(value?: AOrB | null): any {
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    if (value.a != undefined && value.b != undefined) {
        return AToJSON(value);
    } else if (value.b != undefined && value.c != undefined) {
        return BToJSON(value);
    }
}

In the 'if' statements it would test all the required parameters for that member object in the union.

@auto-labeler
Copy link

auto-labeler bot commented Jun 1, 2020

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@keean
Copy link
Author

keean commented Jun 26, 2020

Is this issue going to be addressed or is it a won't fix? I have to decide whether to push an API compatibility breaking change for my application, or if I should wait for openapi-generator to fix this before using the generator.

@keean keean changed the title [BUG] typescript-fetch allOf doesn't handle discrimination by property. [BUG] typescript-fetch oneOf doesn't handle discrimination by property. Jun 26, 2020
@macjohnny
Copy link
Member

@keean would you like to implement a fix?

@macjohnny
Copy link
Member

moreover, you could try the newly refactored https://openapi-generator.tech/docs/generators/typescript

@protoEvangelion
Copy link

@macjohnny I'm still running into this issue amongst several others with oneOf in the typescript-fetch package. Is there another generator that you are aware that supports oneOf?

Using the latest open-api generator as of today: 7.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants