Swagger-codegen: [Java] Useless generation of fields when using allOf

Created on 7 Oct 2015  路  9Comments  路  Source: swagger-api/swagger-codegen

When using composition with allOf, the child class contains un-needed attributes which are already in the parent class.

Java Composition / Inheritance Bug

All 9 comments

It even leads to an exception : class io.swagger.client.model.Bar declares multiple JSON fields named foo when using Gson (in okhttp-gson and retrofit libraries)

@cbornet my understanding of allOf (without discriminator) is that child should "copy" all the parent properties to itself (child) since child does not inherit (extend) parent.

Ref: https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#composition-and-inheritance-polymorphism

Do you have a sample spec so that I can repeat the issue you mentioned above?

@wing328 Well, it seems to me that implementing swagger composition with class extension would be fine if there was only one object possible in the composition since the classes are just mere containers for the Json mapping. But since there can be several objects to compose, it looks preferable to just copy the attributes for clarity and remove the class inheritance in the generated code.

A spec to reproduce here: https://raw.githubusercontent.com/swagger-api/swagger-spec/07cce43535e90815737e7258bb4d51c0c76999d6/examples/v2.0/json/petstore-expanded.json

public class Pet extends NewPet  {

  @SerializedName("name")
  private String name = null;

  @SerializedName("tag")
  private String tag = null;

  @SerializedName("id")
  private Long id = null;
public class NewPet   {

  @SerializedName("name")
  private String name = null;

  @SerializedName("tag")
  private String tag = null;

The issue is also debated in #1120 . We need to have class extension or copy but not both as it fails with gson.

I have tested that a definition with multiple allOf gets the copies of all the elements so composition is working fine if we remove the class extension. As per #1120, there seems to be an issue when the parent holds an enum. I think the solution there would be to also copy the enum to the child class (or import the one from the parent class if it is visible).
I agree with @wing328 that class extension should be reserved for when we have discriminator support.
So what should be done ? I have removed the class extension on my fork and I can pull a PR.

@wing328 I'm facing the problem. What is the final decision about it ?

I was going to open a separate issue (and I still can if you find that more appropriate), but I just wanted mention other issues with model generation. It seems that model generation where inheritance/composition is involved is severely broken.

Basically:
It should support the same inheritance hierarchy as specified in the Swagger model.

Also, this generates no extra properties at all (it is the same as Page class, with no extra properties):

PageBatchOrder:
    allOf: 
    - $ref: '#/definitions/Page'
    type: object
    properties: 
      content: 
        type: array
        items: 
          $ref: '#/definitions/BatchOrder'

So my question is, is this an issue that people are actively working on, or should I have a look?

@cbornet PR (#3393) has been merged into master. Please pull the latest to give it a try.

Was this page helpful?
0 / 5 - 0 ratings