Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SubClass annotations are missing from the base class #2449

Closed
yaelah opened this issue Mar 24, 2016 · 16 comments
Closed

SubClass annotations are missing from the base class #2449

yaelah opened this issue Mar 24, 2016 · 16 comments

Comments

@yaelah
Copy link

yaelah commented Mar 24, 2016

In order for jackson ObjectMapper to deserialize correctly a base class to its subclasses, we need to include annotations defining all expected subclasses in the parent class.
The codegen currently does not generate those annotations.
For this example YAML:

definitions:
  Base:
    type: object
    discriminator: disc
    properties:
      disc:
        type: string
    required:
      - disc
  SubClass:
    allOf:
      - $ref: '#/definitions/Base'
        required:
          - field
        properties:
          field:
            type: string

Jackson expects these annotations to be produced, but they are not:
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "disc")
@JsonSubTypes({ @type(value = SubClass.class, name = "SubClass") })
public class Base {
}

@wing328
Copy link
Contributor

wing328 commented Mar 28, 2016

@yaelah thanks for the detailed above. Would you have cycle to contribute a fix?

Here is a good starting point: https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Java/model.mustache

@wing328
Copy link
Contributor

wing328 commented Mar 28, 2016

Similar ticket: #1253

@yaelah
Copy link
Author

yaelah commented Mar 29, 2016

I have a fix for this. What is the process for submitting patches? I have never submitted anything to this project before.
thanks.

yaelah pushed a commit to yaelah/swagger-codegen that referenced this issue Mar 29, 2016
… base class.

Added a children property to CodegenModel and write out the necessary annotations.
@wing328
Copy link
Contributor

wing328 commented Mar 30, 2016

Please refer to https://github.com/swagger-api/swagger-codegen/blob/master/CONTRIBUTING.md for contributing guidelines.

If you need help, just reply to let us know.

yaelah pushed a commit to yaelah/swagger-codegen that referenced this issue Mar 30, 2016
yaelah added a commit to yaelah/swagger-codegen that referenced this issue Mar 30, 2016
… base class.

Added a children property to CodegenModel and write out the necessary annotations.
@yaelah
Copy link
Author

yaelah commented Mar 30, 2016

Thanks for noticing my wrong email.

yaelah added a commit to yaelah/swagger-codegen that referenced this issue Apr 9, 2016
… base class.

Added a children property to CodegenModel and write out the necessary annotations.
yaelah added a commit to yaelah/swagger-codegen that referenced this issue Apr 9, 2016
yaelah added a commit to yaelah/swagger-codegen that referenced this issue Apr 9, 2016
… base class.

Added a children property to CodegenModel and write out the necessary annotations.
@alex-estela
Copy link
Contributor

Hi guys,
Also currently facing this issue. Any news about the release of this fix ?
Thanks in advance

@triptec
Copy link
Contributor

triptec commented Oct 20, 2016

@wing328 Do you know anything about progress on this?

@wing328
Copy link
Contributor

wing328 commented Oct 21, 2016

I believe @yaelah has fixed the issue in his branch but didn't have time to submit a PR yet.

Can you test his fix via https://github.com/yaelah/swagger-codegen to see if it resolves the issue for you?

If the fix works, I can cherry-pick the commits to submit a PR for the fix.

szakrewsky added a commit to szakrewsky/swagger-codegen that referenced this issue Oct 27, 2016
szakrewsky added a commit to szakrewsky/swagger-codegen that referenced this issue Oct 27, 2016
@szakrewsky
Copy link
Contributor

szakrewsky commented Oct 27, 2016

I modified @yaelah post to work on the latest master branch. I have been using this for a while now, hope it helps. One issue I did notice is that it won't apply to sub-sub classes.

Update: the sub-sub class issue has now been fixed in my latest pr commit.

szakrewsky added a commit to szakrewsky/swagger-codegen that referenced this issue Nov 1, 2016
szakrewsky added a commit to szakrewsky/swagger-codegen that referenced this issue Nov 2, 2016
@wing328
Copy link
Contributor

wing328 commented Nov 11, 2016

@szakrewsky I added a comment to the PR #4085. Please take a look when you've time.

szakrewsky added a commit to szakrewsky/swagger-codegen that referenced this issue Nov 15, 2016
@jeff9finger
Copy link
Contributor

+1

@jeff9finger
Copy link
Contributor

How is the discriminator value determined for the name property of the@JsonSubTypes.@Type annotation. It looks like the best that can be done without more information is to use the class name.

But if a discriminator property value for a sub class is not the name of the class, but another string, how can that be determined? This may need to be addressed in a new issue, but simply wanted to raise it here to see if anyone has ideas.

My idea is to add a codegen vendor extension that can be used to state the discriminator value in a subclass - maybe x-java-discriminator-value

definitions:
  Base:
    type: object
    discriminator: disc
    properties:
      disc:
        type: string
    required:
      - disc
  SubClass:
    x-java-discriminator-value: sub-class
    allOf:
      - $ref: '#/definitions/Base'
        required:
          - field
        properties:
          field:
            type: string 

@szakrewsky
Copy link
Contributor

@jeff9finger I have seen a similar issue. The spec (http://swagger.io/specification/) for the discriminator field says

When used, the value MUST be the name of this schema or any schema that inherits it.

And since the class name is the schema name its all good.

szakrewsky added a commit to szakrewsky/swagger-codegen that referenced this issue Nov 18, 2016
@jeff9finger
Copy link
Contributor

Using the classname is a good default (in fact, it looks to be the only thing that makes sense). But in some cases, the discriminator value needs to be something other than the class name.

I have used a vendor extension "x-discriminator-value" on each subclass. This value contains the value from the enumeration defined for that subclass. Changed the template to handle that value if it exists (otherwise, use name as before). This works perfectly for my. I'd like to see it in the master branch so I don't have to have my own templates.... Anyone else having this issue?

@boldtrn
Copy link

boldtrn commented Jan 23, 2017

Isn't this fixed by #4085?

@wing328 wing328 closed this as completed Feb 16, 2017
@wing328
Copy link
Contributor

wing328 commented Feb 16, 2017

Isn't this fixed by #4085?

I think so.

davidgri pushed a commit to davidgri/swagger-codegen that referenced this issue May 11, 2017
… class (swagger-api#4085)

* petstore up to latest

* Issue swagger-api#2449 SubClass annotations are missing from the base class

* include child in all its super types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants