openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard
verified publisher icon OpenAPITools

[BUG] [TYPESCRIPT-FETCH] Subclassing components using discriminators fails to convert subclasses to JSON

Open toebes opened this issue 2 years ago • 3 comments

Bug Report Checklist

  • [x] Have you provided a full/minimal spec to reproduce the issue?
  • [x] Have you validated the input using an OpenAPI validator (example)?
  • [x] Have you tested with the latest master to confirm the issue still exists?
  • [x] Have you searched for related issues/PRs?
  • [x] What's the actual output vs expected output?
  • [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

When subclassing components using discriminators for mapping, the generator properly generates code to convert the discriminated types from JSON, but does not do the same when converting to JSON. This is present in TYPESCRIPT-FETCH and a quick look at the GO generator also appears to have the same problem.

openapi-generator version

7.0.0-20230527.150314-72

OpenAPI declaration file content or url

https://raw.githubusercontent.com/toebes/onshape-typescript-fetch/main/testing/testdiscrim.json

Generation Details
wget https://oss.sonatype.org/content/repositories/snapshots/org/openapitools/openapi-generator-cli/7.0.0-SNAPSHOT/openapi-generator-cli-7.0.0-20230527.150314-72.jarH
java -jar .\openapi-generator-cli-7.0.0-20230527.150314-72.jar generate -i https://raw.githubusercontent.com/toebes/onshape-typescript-fetch/main/testing/testdiscrim.json -g typescript-fetch --openapi-normalizer REF_AS_PARENT_IN_ALLOF=true
Steps to reproduce

If you look at the generated code for Base.ts in this case, you can see that the conversion BaseFromFromJSONTyped checks the discriminator discrim and calls the appropriate subclasses to serialize the content. However Looking at the BaseToJSON routine, only captures the discriminator and fails to consider any subclass indicated by the discriminator discrim.

export function BaseFromJSONTyped(json: any, ignoreDiscriminator: boolean): Base {
    if ((json === undefined) || (json === null)) {
        return json;
    }
    if (!ignoreDiscriminator) {
        if (json['discrim'] === 'Sub1') {
            return Sub1FromJSONTyped(json, true);
        }
        if (json['discrim'] === 'Sub2') {
            return Sub2FromJSONTyped(json, true);
        }
    }
    return {
        'discrim': !exists(json, 'discrim') ? undefined : json['discrim'],
        'name': !exists(json, 'name') ? undefined : json['name'],
    };
}

export function BaseToJSON(value?: Base | null): any {
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    return {
        'discrim': value.discrim,
        'name': value.name,
    };
}

Note that in the subclass Sub1.ts we have the code which properly calls the base class for serialization.

export function Sub1ToJSON(value?: Sub1 | null): any {
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    return {
        ...BaseToJSON(value),
        'discrim': value.discrim,
        'Sub1Field1': value.sub1Field1,
    };
}
Related issues/PRs

https://github.com/OpenAPITools/openapi-generator/issues/15574 https://github.com/OpenAPITools/openapi-generator/issues/14100

Suggest a fix

The first thought might be to have BaseToJSON call Sub1ToJSON based on the discriminator, but this would quickly lead to an infinite loop. Ideally the generated code would introduce another function to handle the superclass, renaming the previous BaseToJSON to be BaseSuperToJSON including for all the subclasses that call it and creating a new version of the BaseToJSON function that checks for the discriminated subclasses. Note that I have hand edited the output files for my project and this template change does work.

export function BaseFromJSONTyped(json: any, ignoreDiscriminator: boolean): Base {
    if ((json === undefined) || (json === null)) {
        return json;
    }
    if (!ignoreDiscriminator) {
        if (json['discrim'] === 'Sub1') {
            return Sub1FromJSONTyped(json, true);
        }
        if (json['discrim'] === 'Sub2') {
            return Sub2FromJSONTyped(json, true);
        }
    }
    return {
        'discrim': !exists(json, 'discrim') ? undefined : json['discrim'],
        'name': !exists(json, 'name') ? undefined : json['name'],
    };
}

// This is the old BaseToJSON function, just renamed to be BaseSuperToJSON
export function BaseSuperToJSON(value?: Base | null): any {
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    return {
        'discrim': value.discrim,
        'name': value.name,
    };

// This is a new version of BaseToJSON which checks for the subclassing in exactly the same manner as the BaseFromJSONTyped does
export function BaseToJSON(value?: Base | null): any {
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    if (value.discrim === 'Sub1') {
        return Sub1ToJSON(value);
    }
    if (value.discrim === 'Sub2') {
        return Sub2ToJSON(value);
    }
    return BaseSuperToJSON(value);
}

and in the subclass Sub1.ts make a single change, instead of calling BaseToJSON, call BaseSuperToJSON

export function Sub1ToJSON(value?: Sub1 | null): any {
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    return {
        ...BaseRootToJSON(value),   // This was renamed from BaseToJSON
        'discrim': value.discrim,
        'Sub1Field1': value.sub1Field1,
    };
}

toebes avatar Jun 02 '23 19:06 toebes

Same problem I face, but at least you have given me an idea on how to modify the templates.

Here is the modified modelGeneric.mustache template for typescript-fetch generation.

import { exists, mapValues } from '../runtime{{importFileExtension}}';
{{#hasImports}}
{{#imports}}
import type { {{{.}}} } from './{{.}}{{importFileExtension}}';
import {
    {{.}}FromJSON,
    {{.}}FromJSONTyped,
    {{.}}ToJSON,
} from './{{.}}{{importFileExtension}}';
{{/imports}}
{{#parent}}import { {{{.}}}ParentToJSON } from './{{.}}{{importFileExtension}}';{{/parent}}
{{/hasImports}}
{{#discriminator}}
import {
{{#discriminator.mappedModels}}
     {{modelName}},
     {{modelName}}ToJSON,
     {{modelName}}FromJSONTyped{{^-last}},{{/-last}}
{{/discriminator.mappedModels}}
} from './index{{importFileExtension}}';

{{/discriminator}}
{{>modelGenericInterfaces}}

/**
 * Check if a given object implements the {{classname}} interface.
 */
export function instanceOf{{classname}}(value: object): boolean {
    let isInstance = true;
    {{#vars}}
    {{#required}}
    isInstance = isInstance && "{{name}}" in value;
    {{/required}}
    {{/vars}}

    return isInstance;
}

export function {{classname}}FromJSON(json: any): {{classname}} {
    return {{classname}}FromJSONTyped(json, false);
}

export function {{classname}}FromJSONTyped(json: any, ignoreDiscriminator: boolean): {{classname}} {
    {{#hasVars}}
    if ((json === undefined) || (json === null)) {
        return json;
    }
{{#discriminator}}
    if (!ignoreDiscriminator) {
{{#discriminator.mappedModels}}
        if (json['{{discriminator.propertyBaseName}}'] === '{{mappingName}}') {
            return {{modelName}}FromJSONTyped(json, true);
        }
{{/discriminator.mappedModels}}
    }
{{/discriminator}}
    return {
        {{#parent}}...{{{.}}}FromJSONTyped(json, ignoreDiscriminator),{{/parent}}
        {{#additionalPropertiesType}}
            ...json,
        {{/additionalPropertiesType}}
        {{#vars}}
        {{#isPrimitiveType}}
        {{#isDateType}}
        '{{name}}': {{^required}}!exists(json, '{{baseName}}') ? undefined : {{/required}}({{#isNullable}}json['{{baseName}}'] === null ? null : {{/isNullable}}new Date(json['{{baseName}}'])),
        {{/isDateType}}
        {{#isDateTimeType}}
        '{{name}}': {{^required}}!exists(json, '{{baseName}}') ? undefined : {{/required}}({{#isNullable}}json['{{baseName}}'] === null ? null : {{/isNullable}}new Date(json['{{baseName}}'])),
        {{/isDateTimeType}}
        {{^isDateType}}
        {{^isDateTimeType}}
        '{{name}}': {{^required}}!exists(json, '{{baseName}}') ? undefined : {{/required}}json['{{baseName}}'],
        {{/isDateTimeType}}
        {{/isDateType}}
        {{/isPrimitiveType}}
        {{^isPrimitiveType}}
        {{#isArray}}
        {{#uniqueItems}}
        '{{name}}': {{^required}}!exists(json, '{{baseName}}') ? undefined : {{/required}}({{#isNullable}}json['{{baseName}}'] === null ? null : {{/isNullable}}new Set((json['{{baseName}}'] as Array<any>).map({{#items}}{{datatype}}{{/items}}FromJSON))),
        {{/uniqueItems}}
        {{^uniqueItems}}
        '{{name}}': {{^required}}!exists(json, '{{baseName}}') ? undefined : {{/required}}({{#isNullable}}json['{{baseName}}'] === null ? null : {{/isNullable}}(json['{{baseName}}'] as Array<any>).map({{#items}}{{datatype}}{{/items}}FromJSON)),
        {{/uniqueItems}}
        {{/isArray}}
        {{#isMap}}
        '{{name}}': {{^required}}!exists(json, '{{baseName}}') ? undefined : {{/required}}({{#isNullable}}json['{{baseName}}'] === null ? null : {{/isNullable}}mapValues(json['{{baseName}}'], {{#items}}{{datatype}}{{/items}}FromJSON)),
        {{/isMap}}
        {{^isArray}}
        {{^isMap}}
        {{^isFreeFormObject}}
        '{{name}}': {{^required}}!exists(json, '{{baseName}}') ? undefined : {{/required}}{{datatype}}FromJSON(json['{{baseName}}']),
        {{/isFreeFormObject}}
        {{#isFreeFormObject}}
        '{{name}}': {{^required}}!exists(json, '{{baseName}}') ? undefined : {{/required}}json['{{baseName}}'],
        {{/isFreeFormObject}}
        {{/isMap}}
        {{/isArray}}
        {{/isPrimitiveType}}
        {{/vars}}
    };
    {{/hasVars}}
    {{^hasVars}}
    return json;
    {{/hasVars}}
}

{{^parent}}
export function {{classname}}ParentToJSON(value?: {{classname}} | null): any {
    {{#hasVars}}
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }
    return {
        {{#parent}}...{{{.}}}ToJSON(value),{{/parent}}
        {{#additionalPropertiesType}}
            ...value,
        {{/additionalPropertiesType}}
        {{#vars}}
        {{^isReadOnly}}
        {{#isPrimitiveType}}
        {{#isDateType}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}value.{{name}}.toISOString().substring(0,10)),
        {{/isDateType}}
        {{#isDateTimeType}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}value.{{name}}.toISOString()),
        {{/isDateTimeType}}
        {{#isArray}}
        '{{baseName}}': {{#uniqueItems}}{{^required}}value.{{name}} === undefined ? undefined : {{/required}}{{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}Array.from(value.{{name}} as Set<any>){{/uniqueItems}}{{^uniqueItems}}value.{{name}}{{/uniqueItems}},
        {{/isArray}}
        {{^isDateType}}
        {{^isDateTimeType}}
        {{^isArray}}
        '{{baseName}}': value.{{name}},
        {{/isArray}}
        {{/isDateTimeType}}
        {{/isDateType}}
        {{/isPrimitiveType}}
        {{^isPrimitiveType}}
        {{#isArray}}
        {{#uniqueItems}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}Array.from(value.{{name}} as Set<any>).map({{#items}}{{datatype}}{{/items}}ToJSON)),
        {{/uniqueItems}}
        {{^uniqueItems}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}(value.{{name}} as Array<any>).map({{#items}}{{datatype}}{{/items}}ToJSON)),
        {{/uniqueItems}}
        {{/isArray}}
        {{#isMap}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}mapValues(value.{{name}}, {{#items}}{{datatype}}{{/items}}ToJSON)),
        {{/isMap}}
        {{^isArray}}
        {{^isMap}}
        {{^isFreeFormObject}}
        '{{baseName}}': {{datatype}}ToJSON(value.{{name}}),
        {{/isFreeFormObject}}
        {{#isFreeFormObject}}
        '{{baseName}}': value.{{name}},
        {{/isFreeFormObject}}
        {{/isMap}}
        {{/isArray}}
        {{/isPrimitiveType}}
        {{/isReadOnly}}
        {{/vars}}
    };
    {{/hasVars}}
    {{^hasVars}}
    return value;
    {{/hasVars}}
}
{{/parent}}

export function {{classname}}ToJSON(value?: {{classname}} | null): any {
    {{#hasVars}}
    if (value === undefined) {
        return undefined;
    }
    if (value === null) {
        return null;
    }

    {{#discriminator}}
    {{#discriminator.mappedModels}}
    if (value.{{discriminator.propertyBaseName}} === '{{mappingName}}') {
        return {{modelName}}ToJSON(value as {{modelName}});
    }
    {{/discriminator.mappedModels}}
    {{/discriminator}}

    return {
        {{#parent}}...{{{.}}}ParentToJSON(value),{{/parent}}
        {{#additionalPropertiesType}}
            ...value,
        {{/additionalPropertiesType}}
        {{#vars}}
        {{^isReadOnly}}
        {{#isPrimitiveType}}
        {{#isDateType}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}value.{{name}}.toISOString().substring(0,10)),
        {{/isDateType}}
        {{#isDateTimeType}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}value.{{name}}.toISOString()),
        {{/isDateTimeType}}
        {{#isArray}}
        '{{baseName}}': {{#uniqueItems}}{{^required}}value.{{name}} === undefined ? undefined : {{/required}}{{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}Array.from(value.{{name}} as Set<any>){{/uniqueItems}}{{^uniqueItems}}value.{{name}}{{/uniqueItems}},
        {{/isArray}}
        {{^isDateType}}
        {{^isDateTimeType}}
        {{^isArray}}
        '{{baseName}}': value.{{name}},
        {{/isArray}}
        {{/isDateTimeType}}
        {{/isDateType}}
        {{/isPrimitiveType}}
        {{^isPrimitiveType}}
        {{#isArray}}
        {{#uniqueItems}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}Array.from(value.{{name}} as Set<any>).map({{#items}}{{datatype}}{{/items}}ToJSON)),
        {{/uniqueItems}}
        {{^uniqueItems}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}(value.{{name}} as Array<any>).map({{#items}}{{datatype}}{{/items}}ToJSON)),
        {{/uniqueItems}}
        {{/isArray}}
        {{#isMap}}
        '{{baseName}}': {{^required}}value.{{name}} === undefined ? undefined : {{/required}}({{#isNullable}}value.{{name}} === null ? null : {{/isNullable}}mapValues(value.{{name}}, {{#items}}{{datatype}}{{/items}}ToJSON)),
        {{/isMap}}
        {{^isArray}}
        {{^isMap}}
        {{^isFreeFormObject}}
        '{{baseName}}': {{datatype}}ToJSON(value.{{name}}),
        {{/isFreeFormObject}}
        {{#isFreeFormObject}}
        '{{baseName}}': value.{{name}},
        {{/isFreeFormObject}}
        {{/isMap}}
        {{/isArray}}
        {{/isPrimitiveType}}
        {{/isReadOnly}}
        {{/vars}}
    };
    {{/hasVars}}
    {{^hasVars}}
    return value;
    {{/hasVars}}
}

oravecz avatar Dec 31 '23 20:12 oravecz

We are having this same problem and have worked around it with modifying the Mustache templates, but that makes upgrading the openapi-generator a pain.

@leonyu would you welcome a PR on this?

hagis avatar Aug 20 '24 14:08 hagis

Not sure I tagged the right guy there. Maybe @TiFu or @mkusaka can comment whether a PR on this would be welcome?

hagis avatar Aug 23 '24 04:08 hagis

@macjohnny would a small PR with a couple of template modifications be welcome?

hagis avatar Sep 03 '24 05:09 hagis

@macjohnny would a small PR with a couple of template modifications be welcome?

yes! 😊

macjohnny avatar Sep 03 '24 06:09 macjohnny

Thank you @macjohnny !

hagis avatar Sep 05 '24 13:09 hagis