[JAVA] discriminator.mapping is not supported (in generated model)

[0v1se]

Description

spec: https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/
mapping can be specified in discriminator:

discriminator:
  propertyName: petType
  mapping:
    dogggg: '#/components/schemas/Dog'
    catttt: '#/components/schemas/Cat'

Currently it's not taken in account when generating model (it's not for Java, may be for every other language too)

openapi-generator version

3.1.0-SNAPSHOT

OpenAPI declaration file content or url

openapi: "3.0.0"
info:
  version: 1.0.0
  title: Swagger Petstore
  license:
    name: MIT
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets:
    get:
      summary: List all pets
      operationId: listPets
      tags:
        - pets
      parameters:
        - name: limit
          in: query
          description: How many items to return at one time (max 100)
          required: false
          schema:
            type: integer
            format: int32
      responses:
        '200':
          description: A paged array of pets
          headers:
            x-next:
              description: A link to the next page of responses
              schema:
                type: string
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pets"
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
    post:
      summary: Create a pet
      operationId: createPets
      tags:
        - pets
      responses:
        '201':
          description: Null response
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
  /pets/{petId}:
    get:
      summary: Info for a specific pet
      operationId: showPetById
      tags:
        - pets
      parameters:
        - name: petId
          in: path
          required: true
          description: The id of the pet to retrieve
          schema:
            type: string
      responses:
        '200':
          description: Expected response to a valid request
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pets"
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
components:
  schemas:
    Pet:
      type: object
      discriminator:
        propertyName: petType
        mapping:
          dogggg: '#/components/schemas/Dog'
          catttt: '#/components/schemas/Cat'
      properties:
        name:
          type: string
        petType:
          type: string
      required:
      - name
      - petType
    Cat:
      description: A representation of a cat
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        properties:
          huntingSkill:
            type: string
            description: The measured skill for hunting
            enum:
            - clueless
            - lazy
            - adventurous
            - aggressive
        required:
        - huntingSkill
    Dog:
      description: A representation of a dog
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        properties:
          packSize:
            type: integer
            format: int32
            description: the size of the pack the dog is from
            default: 0
            minimum: 0
        required:
        - packSize
    Pets:
      type: array
      items:
        $ref: "#/components/schemas/Pet"
    Error:
      required:
        - code
        - message
      properties:
        code:
          type: integer
          format: int32
        message:
          type: string

Command line used for generation

java -jar ... generate -i api.yaml -g java --library resttemplate

Steps to reproduce

Generate model
Here is what's generated in Pet.java:

@JsonSubTypes({
  @JsonSubTypes.Type(value = Cat.class, name = "Cat"),
  @JsonSubTypes.Type(value = Dog.class, name = "Dog"),
})

Related issues/PRs

Related to #197

Suggest a fix/enhancement

Should generate model like this:

@JsonSubTypes({
  @JsonSubTypes.Type(value = Cat.class, name = "catttt"),
  @JsonSubTypes.Type(value = Dog.class, name = "dogggg"),
})

[jmini]

According to you having the name in the @JsonSubTypes.Type annotation should be sufficient to make the JSON-Deserializer work?

Where is the information about the field name (petType in your example) that is used to determinate which kind of object should be created?

[0v1se]

@jmini no, it's not sufficient
Also @JsonTypeInfo should be present:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "petType", visible = true )

I didn't notice it because it's generated correctly.

[0v1se]

One more thing, petType property is also generated in the Pet model.

  @JsonProperty("petType")
  private String petType = null;

This way wrong json is produced:

{"petType":"Dog","name":null,"petType":null,"packSize":10}

[0v1se]

@jmini I can fix this and change jackson annotations to

@JsonSubTypes({
  @JsonSubTypes.Type(value = Cat.class, name = "catttt"),
  @JsonSubTypes.Type(value = Dog.class, name = "dogggg"),
})

Also, I can remove property identifying type (in my example is 'petType') from the pojo.
I think it won't break anything.

What do you think?
This way basic polymorphism will be supported at least for Java clients

[jmini]

Yes, I have started to work on it:
fix_discriminator. I have introduced a DiscriminatorCodegen used in CodegenModel and in CodegenOperation. This is a drop in replacement for io.swagger.v3.oas.models.media.Discriminator that was used before.

This is work in progress, but my Idea was to edit the templates so that the cases described here are covered.

[0v1se]

https://github.com/jmini/openapi-generator/blob/b90c53deb6d90a5b9995cd8aa1d1786eacaa2f90/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java#L565

So, this method (modelInheritanceSupportInGson) won't be needed? Am I right?
There will support for inheritance for every generator.

And, mappedModels will be used insted of children in (https://github.com/jmini/openapi-generator/blob/6e2ca294b52e55c381eb8dae1fb01ccf86acb754/modules/openapi-generator/src/main/resources/Java/typeInfoAnnotation.mustache#L4)

[0v1se]

I know it's WIP, but a small note:
equals should be implemented for DiscriminatorCodegen, it's used in the code

[jmini]

Thank you a lot for this feedback.

And, mappedModels will be used insted of children

Yes this was my idea.

And an other template change is needed to remove the creation of the discriminator as field:

  @JsonProperty("petType")
  private String petType = null;

(did not investigate yet)

So, this method (modelInheritanceSupportInGson) won't be needed?

Nice catch, I did not analyze who was computing the "children" map.

equals should be implemented for DiscriminatorCodegen, it's used in the code

Nice catch!

---

Do you want to take over the PR? I have not that much time right now.
If you finish it, I will review it.

---

There is also a small spec modules/openapi-generator/src/test/resources/3_0/allOf.yaml to test that everything is working well. A small unit test in DefaultCodegenTest would be great. Minimum would be to parse the spec and to ensure that the discriminator is set in the CodegenOperation.

[0v1se]

I will try, but I'm not very comfortable with the code right now (I mean, not very familiar with).
Also, I won't be able to implement this feature for other languages/libraries.

Will get back if I have questions.

[jmini]

I won't be able to implement this feature for other languages/libraries.

This is always the case. I just fix stuff in for the Java generators and I help at model level (the codegen classes that are exposed in the templates). Then I consider that people with knowledge of other languages needs to consume the elements in their models.

I can help you if you need to know something, but from the feedback you gave me on my WIP commit, I think you know enough to create a great PR.

[0v1se]

I almost finished. Will submit PR soon (I had only to change templates and do some minor stuff).
But it won't cover one aspect: discriminator field won't be removed from the model.

It is a little bit hard to do this. I will write some thoughts after I submit PR

[jmini]

Sure feel free to do stuff in multiple small steps...

[0v1se]

@jmini
submitted #536

some concerns about the PR:

1. Deleted modelInheritanceSupportInGson cause it doesn't do anything besides calculating children (for annotation) - not 100% sure about this. Pls, review
2. Removing discriminator field requires more information. Will add later

[0v1se]

Regarding not including discriminator field from model (petType from this issue)

1. According to https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/ this property should be defined for every type (Pet, Dog, Cat). Why do we need it in the first place? Is it code duplication? If I define discriminator property, it's obvious I have this property. No need to add it to the model again. Don't you think?
2. If we want to not include this property in generated classes, then there are some strategies:
   2.1 remove it from CodegenModel.vars (currently I think it's best solution, but I can't see all the consequences of this).
   2.2 flag the property with isDiscriminator and filter it out in templates
   2.3 create one more collection with properties and use it in pojo.mustache (and create hasMore, hasVars analogs for this collection)

I tried to implement 2.2, but it's not that easy: I needed to change hasMore, hasVars and possibly other auxiliary fields from CodegenModel to correctly generate java files.

What do you think about this?

[jmini]

I will come back to you with the problem I see with the discriminator in the properties. I am no longer sure what we need.

I will give you an example that is not working at all (using getClass().getSimpleName() as value). We need to fix it.

---

In the mean time, I think it is important to document changes like this in the migration guide, I have opened #587 for that.

[jmini]

I have some cases that produce compile error when prefixes are used (all my models are using a common prefix). I will document this.

Other interesting case, @yuanpli reported a compile error when an enum is used: #806

[nickshoe]

The fix for this may also be needed for Java Spring target?
Maybe related to #3796

[ivarreukers]

According to the status of the PR, I assume that this has been fixed since 3.2.x correct?

Because I'm still facing the same issue when generating from swagger. Although one difference is that my discriminator is in the definition. (And in the definitions, it is only allowed to be a string value)

I have:

definitions:
  Instruction:
    type: "object"
    discriminator: "instructionType"
    properties:
      instructionType:
        description: "Type of instruction"
        enum:
          - "EMAIL"
          - "SOME_OTHER"
  EmailInstruction:
    allOf:
      - $ref: "#/definitions/Instruction"
      - type: "object"
        properties:
          name:
            type: "string"
            example: "Name of reciever"
            description: "Name of the receiver"
          emailAddress:
            type: "string"
            example: "info@stackoverflow.com"
            description: "Email address of the receiver"
  SomeOtherInstruction:
    allOf:
      - $ref: "#/definitions/Instruction"
      - type: "object"
        properties:
          anotherone:
            type: "string"
            example: "Name of reciever"
            description: "Name of the receiver"

And in turn the Instruction is wrapped in another object. When generating, it still uses the class name for the JsonSubType annotation:

Actual:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "instructionType", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = SomeOtherInstruction.class, name = "SomeOtherInstruction"),
  @JsonSubTypes.Type(value = EmailInstruction.class, name = "EmailInstruction"),
})

Expected:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "instructionType", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = SomeOtherInstruction.class, name = "SOME_OTHER"),
  @JsonSubTypes.Type(value = EmailInstruction.class, name = "EMAIL"),
})

[davinchia]

This is also biting me with the following spec and 4.2.2:

OutputFormat:
      type: object
      required:
        - type
      properties:
        type:
          type: string
      discriminator:
        propertyName: type
        mapping:
          outputKeyValueLabel: '#/components/schemas/OutputKeyValueLabel'
          outputIdLabel: '#/components/schemas/OutputIdLabel'
    OutputKeyValueLabel:
      allOf:
        - $ref: '#/components/schemas/OutputFormat'
        - type: object
          properties:
            keyLabel:
              type: string
              example: PETS
            valueLabel:
              type: string
              example: DOGS
          required:
            - keyLabel
    OutputIdLabel:
      allOf:
        - $ref: '#/components/schemas/OutputFormat'
        - type: object
          properties:
            idLabel:
              type: string
              example: MM1234
          required:
            - idLabel

this gives

public class OutputFormat {
  public static final String SERIALIZED_NAME_TYPE = "type";
  @SerializedName(SERIALIZED_NAME_TYPE)
  private String type;

  public OutputFormat() {
    this.type = this.getClass().getSimpleName();
  }

  public OutputFormat type(String type) {
    
    this.type = type;
    return this;
  }
...

and the discriminator is set to the class's name instead of the specified mapping. There also isn't any JsonTypeInfo annotations on the classes.

[ivarreukers]

Note that if you upgrade to an OpenAPI specification instead of swagger 2.0 and use f.e. the <library>jersey2</library> then it does work. Also check out this stackoverflow post: https://stackoverflow.com/questions/59252417/generate-a-property-as-schema-definition-in-openapi-3-0

[davinchia]

Might be mistaken - this should be on OpenApi 3.0.

Silly me. The JsonTypeInfo annotations are Jackson, while the default uses Gson, which has terrible polymorphic support and probably explains why this is misbehaving. Confirm changing to Jersey2 fixed it. Thanks @ivarreukers!

[alqh]

Just looking at some examples in this conversation that worked (e.g @ivarreukers example), do we really need a self reference from the child to the parent (thus creating that dependency between discriminator and its implementation classes)

According to the example of discriminator in inheritance and polymorphism, the following should work (without cyclic reference between child back to parent)

parent:
      oneOf:
        - $ref: '#/components/schemas/simpleObject'
        - $ref: '#/components/schemas/complexObject'
      discriminator:
        propertyName: objectType
        mapping:
          Simple: '#/components/schemas/simpleObject'
          Complex: '#/components/schemas/complexObject'
    
simpleObject:
      type: object
      required:
        - objectType
      properties:
        objectType:
          type: string

complexObject:
      type: object
      required:
        - objectType
      properties:
        objectType:
          type: string

This generated the right JsonSubTypes on the parent

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "objectType", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = SimpleObjectDto.class, name = "Simple"),
  @JsonSubTypes.Type(value = ComplexObjectDto.class, name = "Complex"),
})
@com.fasterxml.jackson.annotation.JsonIgnoreProperties(ignoreUnknown=true)

public class ParentDto   { .... }

but the child does not have the extension to the parent

public class SimpleObjectDto { ... }  // missing extends ParentDto

[lostiniceland]

but the child does not have the extension to the parent

Thats because of #8495