swagger-codegen icon indicating copy to clipboard operation
swagger-codegen copied to clipboard

[Java] doesnt generate correct classes for models with same nested properties

Open stevecookform3 opened this issue 8 years ago • 14 comments

Description

If there are two models in the swagger definition whose subproperties are the same, the code generator always uses the first one.

e.g. take the following two definitions

  house:
    type: object
    properties:
      has_walls:
        type: boolean
      attributes:
        type: object
        properties:
          name:
            type: string
  
  person:
    type: object
    properties:
      has_legs:
        type: boolean
      attributes:
        type: object
        properties:
          name:
            type: string

Swagger doesnt generate a PersonAttributes class at all, and instead generates:

public class Person   {
  @JsonProperty("has_legs")
  private Boolean hasLegs = null;

  @JsonProperty("attributes")
  private HouseAttributes attributes = null;
Swagger-codegen version

Latest snapshot 2.2.2

Swagger declaration file content or url
swagger: '2.0'
info:
  version: 0.0.0
  title: Simple API
paths:
  /:
    get:
      responses:
        200:
          description: OK
          schema:
            $ref: "#/definitions/house"
        400:
          description: OK
          schema:
            $ref: "#/definitions/person"

definitions:
  
  house:
    type: object
    properties:
      has_walls:
        type: boolean
      attributes:
        type: object
        properties:
          name:
            type: string
  
  person:
    type: object
    properties:
      has_legs:
        type: boolean
      attributes:
        type: object
        properties:
          name:
            type: string

Command line used for generation

java -jar bin/swagger-codegen-cli.jar generate -v -i ./swagger.yaml -l spring -Dmodels -o .

Steps to reproduce

Copy the above swagger.yaml into http://editor.swagger.io/#!/ or use the latest CLI to build as above.

stevecookform3 avatar Mar 01 '17 14:03 stevecookform3

On second thoughts this might be a parser error, since I've tried generating a few other types of servers and they all have the same issue (e.g. ASPNET, Flask).

stevecookform3 avatar Mar 01 '17 14:03 stevecookform3

@wing328 any idea where the issue might lie?

stevecookform3 avatar Mar 06 '17 17:03 stevecookform3

@stevecookform3 I think that's because you're defining the model inline and swagger codegen will try to generate only 1 model if both inline models appear to be the same.

You can try to add the title attribute to have a better control on the naming of the inline object or try to define the model separately instead of inline.

wing328 avatar Mar 06 '17 17:03 wing328

@wing328 where would the title attribute go? In the spec I can only see the title attribute used as the title of the app, not at individual model levels (title: "Swagger Sample App")..

stevecookform3 avatar Mar 06 '17 18:03 stevecookform3

Something like this:

      attributes:
        title: NewNameHere
        type: object
        properties:
          name:
            type: string

I would recommend defining the model separately instead of inline.

wing328 avatar Mar 06 '17 18:03 wing328

@wing328 thanks for the info. FYI adding the title attribute doesnt make any difference - the model is still not generated.

Using a ref does work as a workaround for the issue though, so the following will generate the correct code:

definitions:
  
  house:
    type: object
    properties:
      has_walls:
        type: boolean
      attributes:
        $ref: "#/definitions/houseattributes"

   houseattributes:
        type: object
        properties:
          name:
            type: string
  
  person:
    type: object
    properties:
      has_legs:
        type: boolean
      attributes:
        $ref: "#/definitions/personattributes"

  personattributes:
        type: object
        properties:
          name:
            type: string

However i'm not sure this is great for a couple of reasons:

  • The whole model is not the same, only a sub-part of it. Its therefore confusing to have some of the model created using subparts of a different model. Its also difficult to anticipate which objects are going to be shared and which aren't.
  • The workaround is more verbose, and more difficult to read

Is there any simple way to disable this behaviour in the codegen, or any plans to change the default to not try to create semi-shared models?

stevecookform3 avatar Mar 07 '17 08:03 stevecookform3

Understood that different developers have different preferences on how to define models.

What about disabling the behaviour if the "title" attribute is present?

wing328 avatar Mar 07 '17 08:03 wing328

IMHO I would personally not go for the shared models at all. If the developer wants to generate shared models, its simple for them to go through the swagger definition and separate out the models in whatever way they want. They will have a much better understanding of what can and cannot logically be shared that you can get from automated parsing. I'd just suggest a simple rule that anything thats defined in a top level 'definition' would be logically separate, and I think that would be the most easily understandable and consistent.

stevecookform3 avatar Mar 07 '17 08:03 stevecookform3

I've been hitting this issue as well. I thought it would get into the 2.2.3 release, why has it been postponed? Is there any chance this will get into a release soon? Cheers!

tanis2000 avatar Aug 03 '17 13:08 tanis2000

@tanis2000 please use the workaround as we've not decided on the "correct" behaviour:

Using a ref does work as a workaround for the issue though, so the following will generate the correct code:

wing328 avatar Aug 03 '17 16:08 wing328

@wing328 thanks for the follow up. I am indeed using a ref for the time being. It's just that avoiding to add refs everywhere would make things easier. Thanks for the great work anyway! :)

tanis2000 avatar Aug 03 '17 16:08 tanis2000

@stevecookform3 @tanis2000 I know this is old but I've been hitting the very same issue and this thread has helped a lot in my research. I will create a pull request for a new setting in the Maven plugin when/if I get a green light for a corresponding swagger-parser issue as this is a dependency here.

hlyakhovich avatar Jul 26 '19 09:07 hlyakhovich

@xnsdns this change would definitely be helpful in future projects as well.

tanis2000 avatar Jul 26 '19 09:07 tanis2000

The best way to avoid this class merging is to explicitly define separate schemas for HouseAttributes and PersonAttributes and reference them using $ref. This way, Swagger will understand that these are two distinct types, even though they have the same structure.

Here’s how you can modify your Swagger definition:

`swagger: '2.0' info: version: 0.0.0 title: Simple API paths: /: get: responses: 200: description: OK schema: $ref: "#/definitions/house" 400: description: OK schema: $ref: "#/definitions/person"

definitions: HouseAttributes: type: object properties: name: type: string

PersonAttributes: type: object properties: name: type: string

house: type: object properties: has_walls: type: boolean attributes: $ref: "#/definitions/HouseAttributes"

person: type: object properties: has_legs: type: boolean attributes: $ref: "#/definitions/PersonAttributes" ` In this example, HouseAttributes and PersonAttributes are explicitly defined as separate schemas. Now, the code generator will generate two distinct classes: HouseAttributes and PersonAttributes.

rudrakshtripathi avatar Oct 10 '24 08:10 rudrakshtripathi