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

JSON object with inheritance is not rendering the parent object correctly #730

Closed
fcmdeveloper1 opened this issue Apr 28, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@fcmdeveloper1
Copy link

Dears,
i am not sure if this is the behavior of the library or not, but i can see that if i have an object that inherits from a parent object, not all the parent object fields are generated in the specs, specially if there is a "discriminatorProperty".

also, the above is not rendered unless i included both the discriminatorProperty = "" and allOf = {ParentEventDto.class} in the @Schema field of the child object, and even if i did this inclusion, the UI is not rendered correctly for the child object.(missing a lot of fields)

for swagger, without adding the allOf and discriminatorProperty attributes, everything is working perfectly, which is the expected behavior for both UI and the generated swagger description.

image

disclaimer, i am using the exact same objects with the same @Schema attributes for both description swagger and Kafkaconsumer

My Best Regards

@fcmdeveloper1 fcmdeveloper1 added the bug Something isn't working label Apr 28, 2024
@timonback
Copy link
Member

Hi @fcmdeveloper1 ,

Can you share the classes that are involved, again so that we can easily reproduce it?
Or are those the same as in #729 ?

An alternative to uploading the code as zip is to fork the repo, create a new branch, edit the example (https://github.com/springwolf/springwolf-core/blob/master/springwolf-examples/springwolf-kafka-example/src/main/java/io/github/springwolf/examples/kafka/dtos/ExamplePayloadDto.java ; no need to fix tests or anything like that) and commit&push + open a pr to the repo (or your fork)

@fcmdeveloper1
Copy link
Author

Dear @timonback,
yes, exact project, you can reproduce the issue, also, you can see the behavior if you omit the "allOf" and "discriminatorProperty" from the LearningEvent object which is the desired setup to match the swagger default behavior, the eventKey property will not appear in the object.

please, let me know if i can help or support.

My Best Regards

timonback added a commit to timonback/springwolf-core that referenced this issue May 3, 2024
timonback added a commit to timonback/springwolf-core that referenced this issue May 3, 2024
@timonback
Copy link
Member

Hi @fcmdeveloper1,

I started looking into and created #745 so that we can look at the same thing together.

The code contains two variations - see the commits in the pr

  1. The discriminator field eventKey is added as own field in the Parent class
  2. LearningEvent has the @Schema#allOf property set
  • Variation 1 works out of the box for me, although requires code changes in your event class.
  • Variation 2 produces already the correct spec document, however springwolf-ui cannot handle the anyOf schemas yet. The required changes are included in the PR and you can verify that in the preview deployment.

for swagger, without adding the allOf and discriminatorProperty attributes, everything is working perfectly, which is the expected behavior for both UI and the generated swagger description.

I debugged the output of swagger-parser (which springwolf uses) (excluding the extra field of variation 1, excluding the property in the schema annotation of variation 1). Then 3 fields (eventKey is missing) show up.

What do you mean with swagger? springdoc and its ui?

@timonback timonback added the waiting for feedback Waiting for user feedback/response label May 3, 2024
@fcmdeveloper1
Copy link
Author

Dear @timonback,
i think i was not clear enough, if you omit the allOf and discriminatoProperty from the LearningEvent object, the fields of the parent will not be rendered or will be ignored (you can try yourself) although the object itself clearly extends the ParentObject, that's why i got confused, because, i expect to have all the fields including the eventKey without writing the allOf or the discriminatoProperty attributes, and this is what happens in swagger. with or without these attributes, the UI and properties are rendered perfectly.

also, i didn't get well what do you mean by code changes for variation1.

and for the swagger, yes, i am using springdoc-openapi-ui for the swagger documentation.

My Best Regards

@timonback
Copy link
Member

Hi @fcmdeveloper1
Lets break it down.
I agree that swagger does not add the field eventKey from the annotations to any object. That might be expected behaviour as the class does not have the field. The field is only used during deserialization and therefore part of the payload, but not within the application.

Therefore, I suggested the alternatives (variations). Both are shown in the reproduce PR in separate commits: d6690f9

Do the alternatives work for you? Or can you investigate what we need to change in springwolf?

@fcmdeveloper1
Copy link
Author

Dear @timonback,
thanks for your suggestions, and this is my comments for both options and i am not sure if it is doable or the changes are huge:

  1. The discriminator field eventKey is added as own field in the Parent class
    i guess this will be difficult and may be misleading to the developers which might at the end lead to having 2 eventKey properties, one from the deserialization and the other from the added class property, which i guess is not best match.
  2. LearningEvent has the @Schema#allOf property set
    i guess this is more suitable especially after you support the allOf for the UI itself, however, it might be more friendly to the developer who is usually using the swagger if you added the same properties from the parent object which is already extended from the object. and i guess we agreed the eventKey is a deserialized property which will appear in all cases.

we can have further discussions if any.

thanks a lot

My Best Regards

@timonback timonback removed the waiting for feedback Waiting for user feedback/response label May 19, 2024
@timonback
Copy link
Member

We discussed, but we do not see another way at this point - besides the 2 offered options.

The library swagger-parser that we use already omits the eventKey field, so Springwolf does not see it either.
You are welcome to suggest a code improvement via a PR.

Apart from that, one improvement regarding the rendering in springwolf-ui has been made that will be part of the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants