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

Bring your own ObjectWriter to serialize the OpenAPI model #1603

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

gastaldi
Copy link
Contributor

@gastaldi gastaldi commented Oct 7, 2023

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@MikeEdgar
Copy link
Member

Unfortunately I don't think this will solve the issue. The main problem is that the scanner is introspecting a POJO with a field of type JsonNullable and it doesn't know how RestEasy will serialize that with a customized ObjectMapper. The scanner is building the OpenAPI model based on the Jandex types and the serialization of that model using a one-off ObjectMapper is separate from the RestEasy serialization of runtime instances of it.

I think a solution will need to either (1) make the annotation scanner aware of how a JsonNullable<T> should look or (2) make it something a user can configure.

For (1), it may just be that JsonNullable behaves like an Optional, which we already support.

For (2), we already have the mp.openapi.schema. config prefix, but I don't think it would work correctly with a generic type argument - that could be changed perhaps.

@gastaldi
Copy link
Contributor Author

gastaldi commented Oct 7, 2023

@MikeEdgar the solution I proposed here is to reuse an ObjectMapper that already has the required modules installed. The next step after this is released would be to change in Quarkus to pass the managed ObjectMapper that in @melloware's scenario is the one to be used.

Feel free to close this PR if that doesn't make sense

Copy link
Member

@MikeEdgar MikeEdgar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gastaldi I think it makes sense to have a way for Quarkus to pass its ObjectMapper like this, so let's include this PR. I'll unlink the issue though because at the point the OpenAPI model is being serialized, it already contains the incorrect schema model introspected from the Jandex structure. In other words, the JsonNullable isn't in the model for serialization at this stage so having the Jackson module present for serialization won't impact the output.

@MikeEdgar MikeEdgar merged commit 40a312c into smallrye:main Oct 7, 2023
8 checks passed
@gastaldi gastaldi deleted the writer branch October 7, 2023 17:56
@phillip-kruger phillip-kruger added this to the 3.7.0 milestone Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants