PR #582 in combination with @JsonTypeInfo.defaultImpl generates an "incomplete" Tagged Union

[robertgartman]

We ran into breaking changes when upgrading to v2.28.785, due to the new feature in #582. When using JsonTypeInfo in combination with a default class implementation (defaultImpl), then the resulting Tagged Union will capture all the @JsonSubTypes (as a result of #582) but miss the ones covered by the fallback defaultImpl. Not a bug, but perhaps a missing feature?

The short version

In v2.27.744, the provided example generates:

export interface INamedEntity {
    type?: INamedEntityType;
    entity?: string;
}

export enum INamedEntityType {
    O = "O",
    NORP = "NORP",
    ORGANIZATION = "ORGANIZATION",
    FACILITY = "FACILITY",
}

In v2.28.785, the provided example generates:

export interface INamedEntity {
    type: "DefaultEntity" | "FACILITY";
    entity?: string;
}

export enum INamedEntityType {
    O = "O",
    NORP = "NORP",
    ORGANIZATION = "ORGANIZATION",
    FACILITY = "FACILITY",
}

Issue: Valid values for INamedEntity.type are the enums in INamedEntityType, not "DefaultEntity" | "FACILITY"

The verbose version

The issue is a result of having a default fallback implementation (defaultImpl = DefaultEntity.class in the example) in combination with JsonTypeInfo.Id.NAME and JsonTypeInfo.As.EXISTING_PROPERTY. The example has an enum as the name property but I assume the issue will be present also with a String.

Is there a way of keeping the pre-2.28.785 behaviour, or perhaps have the type attribute include all the enum values? Creating an object in the TypeScript domain that implements INamedEntity will, for obvious reasons fail, when setting type='NORP'.

A complete example is provided here: https://github.com/avtalsbanken/typescript-generator

Code from the example repo included below:

package my.example.app;

import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;

@JsonTypeInfo(
    use = JsonTypeInfo.Id.NAME,
    include = JsonTypeInfo.As.EXISTING_PROPERTY,
    property = "type",
    defaultImpl = DefaultEntity.class
)
@JsonSubTypes({
    @JsonSubTypes.Type(
        value = FacilityEntity.class,
        name = "FACILITY"
    )
})
abstract class NamedEntity {

    public final NamedEntityType type;

    private String entity;

    public NamedEntity(NamedEntityType type) {
        this.type = type;
    }

    public String getEntity() {
        return entity;
    }

    public void setEntity(String entity) {
        this.entity = entity;
    }
}

package my.example.app;

/**
 * A tiny subset of Named Entities
 */
public enum NamedEntityType {
    O /* Other */,
    NORP /* Nationalities or religious or political groups */,
    ORGANIZATION /* Companies, agencies, institutions, etc */,
    FACILITY /* Buildings, airports, highways, bridges, etc. */
}

package my.example.app;

public class DefaultEntity extends NamedEntity{

    public DefaultEntity(NamedEntityType type) {
        super(type);
    }
}

package my.example.app;

public class FacilityEntity extends NamedEntity {

    private String geoCoordinates;

    public FacilityEntity() {
        super(NamedEntityType.FACILITY);
    }

    public String getGeoCoordinates() {
        return geoCoordinates;
    }

    public void setGeoCoordinates(String geoCoordinates) {
        this.geoCoordinates = geoCoordinates;
    }
}

[vojtechhabarta]

Thanks for precise description.

I think easiest fix in typescript-generator would be to disable this new feature when defaultImpl element is used. But this element can also be used with PROPERTY and I would like to keep the behaviour consistent in PROPERTY and EXISTING_PROPERTY cases. And it cannot be changed for PROPERTY case because it would be also breaking change.

Moreover users can have explicit sub-classes for all types (unlike you) but still have also defaultImpl to be able to handle any new types (not yet supported).

So I am thinking about adding some configuration parameter which would disable this feature globally for EXISTING_PROPERTY and/or PROPERTY cases or adding possibility to use specified annotation to disable this feature for annotated class. What is your situation? Would you prefer to disable this completely or just for some classes?

[robertgartman]

Without considering the complexity and maintainability of your proposed solutions, I think the "for some classes" could be a better way. It removes one decision point when integrating typescript-generator. My guesstimation is that overall use of defaultImpl is marginal and could be treated on a case by case, leaving the Tagged Union feature as the default approach.

Many kudos for all work with this fantastic tooling!

[vojtechhabarta]

In a9bf79c I added disableTaggedUnionAnnotations parameter which can be used to disable tagged union for a class annotated with specified annotation.

[vojtechhabarta]

Released in 2.29.814.