OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.24k stars 6.43k forks source link

[REQ] [Java] Setting automatically discriminator member at POJO instantiation #10992

Open alexlegay opened 2 years ago

alexlegay commented 2 years ago

Is your feature request related to a problem? Please describe.

More or less a problem but there are workarounds. With the current version of the generator, in an inheritance context, for instance using the unit test example "src/test/resources/3_0/allOf_composition_discriminator.yaml", when instantiating a Snake object, the petType member (discriminator property of Pet) is set to null. This leads to "issues" identifying easily the subtype of Pet actually instantiated. Workarounds are:

One thing that can be confusing & error prone is that nothing prevents to instantiate a Snake but set the petType to "lizard" for instance.

Describe the solution you'd like

The solution would consist of 2 things:

In java, for again the same example, it would result in something like that:

public class Pet {
 ...
 protected Pet petType(String petType) {
    this.petType = petType;
    return this;
 }
 public String getPetType() {
    return petType;
 }
 protected void setPetType(String petType) {
    this.petType = petType;
 }
 ...
}
public class Reptile extends Pet  {
 ...
public Reptile() {
    super();
    this.petType("Reptile");
  }
 ...
}
public class Snake extends Reptile {
 ...
public Snake() {
    super();
    this.petType("Snake");
 }
 ...
}

Additional context

In my fork, I managed to fix it in 2 steps: - Updating only the pojo.mustache as follows:

...
  {{#parentModel.discriminator}}public {{classname}}() {
    super();
    this.{{propertyBaseName}}("{{{name}}}");
  }{{/parentModel.discriminator}}
...
{{#vars}}
...
{{#isDiscriminator}}protected {{/isDiscriminator}}{{^isDiscriminator}}public {{/isDiscriminator}}{{classname}} {{name}}({{{datatypeWithEnum}}} {{name}}) {
    this.{{name}} = {{name}};
    return this;
  }
...
{{#isDiscriminator}}protected{{/isDiscriminator}}{{^isDiscriminator}}public{{/isDiscriminator}} void {{setter}}({{{datatypeWithEnum}}} {{name}}) {
    this.{{name}} = {{name}};
  }
{{/vars}}

It works well but only for one level of inheritance (therefore not with the example Snake -> Reptile -> Pet) and this is obviously not acceptable. Moreover, it's probably not very clean to fix it like this (using #parentModel.discriminator).

- Updating CodegenModel.java, DefaultCodegen.java & pojo.mustache as follows: In CodegenModel.java:

...
    public boolean hasDiscriminatorUpward; // + getter/setter
    public String upwardDiscriminatorPropertyBaseName; // + getter/setter
...

In DefaultCodegen.java:

...
public Map<String, Object> updateAllModels(Map<String, Object> objs) {
...
        for (String name : allModels.keySet()) {
          CodegenModel cm = allModels.get(name);
          CodegenDiscriminator discriminator = cm.getDiscriminator();
          if (discriminator != null) {
            for (MappedModel mappedModel : discriminator.getMappedModels()) {
              CodegenModel model = allModels.get(mappedModel.getModelName());
              model.setHasDiscriminatorUpward(true);
              model.setUpwardDiscriminatorPropertyBaseName(discriminator.getPropertyBaseName());
            }
          }
        }
...
}

In pojo.mustache:

...
  {{#hasDiscriminatorUpward}}public {{classname}}() {
    super();
    this.{{upwardDiscriminatorPropertyBaseName}}("{{{name}}}");
  }{{/hasDiscriminatorUpward}}

Same solution to set protected visibility on both setter methods
...

It generates the expected result for mulitple inheritance levels too. But again, it might not be optimal & very clean.

Final comments: This request is first intended to trigger a discussion around this specifc use of the discriminator in a multiple inheritance context in JAVA. My solutions have been designed with a limited knowledge of the open API generator and there are probably much better ways to fix this issue so feel free to propose something else in case this request brings interest.

argenstijn commented 2 years ago

any idea when to implement this feature?