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

OpenAPI model not considering mixins in natibe build #5891

Closed
ivivanov-bg opened this issue Mar 18, 2024 · 8 comments · Fixed by #5898
Closed

OpenAPI model not considering mixins in natibe build #5891

ivivanov-bg opened this issue Mar 18, 2024 · 8 comments · Fixed by #5898
Assignees
Milestone

Comments

@ivivanov-bg
Copy link

Bug description

When using camel-quarkus-openapi-java generation of the OpenAPI specification in native mode doesn't take into account the mixins specified in the swagger-core library:

https://github.com/swagger-api/swagger-core/blob/3fcc473c33c56f306902853a451020926ad691ea/modules/swagger-core/src/main/java/io/swagger/v3/core/util/ObjectMapperFactory.java#L244

In JVM mode - everything works as expected.

The root cause of the issue is that the mixins are abstract classes without any children, so they get stripped during native build.
As a result - the OpenAPI document might become very big (in case of complex data model) and make swagger UI completely unresponsive.

The extension should register the mixins for reflection to prevent flooding openapi json with unnecessary data.

Attached is sample reproduces: [sample-app.zip](https://github.com/apache/camel-quarkus/files/14634597/sample-app.zip)

In JVM mode openapi size (accessible on /v1/openapi) is 1502 bytes
In Native mode openapi size (accessible on /v1/openapi) is 3653 bytes
(The difference comes mostly from Components -> Schemas -> <DataType> -> jsonSchema section)

@zhfeng
Copy link
Contributor

zhfeng commented Mar 18, 2024

Thanks @ivivanov-bg - I will take a look.

@zhfeng
Copy link
Contributor

zhfeng commented Mar 19, 2024

@ivivanov-bg what the mixin classes should be registered for reflection? I tried all class from io.swagger.v3.core.jackson.mixin but the native runner is producing an empty response now.

@ivivanov-bg
Copy link
Author

That's exacrly what I observed too and the reason to not privide a PR directly.

My cussrent issue (with the jsonSchema) gets resolved if the Schema31 mixin and the inner type serializer are registered, but to me this looked more like a workaround for particular situation

@zhfeng
Copy link
Contributor

zhfeng commented Mar 19, 2024

io.swagger.v3.core.jackson.mixin.Schema31Mixin and Schema31Mixin.TypeSerializer ?

@ivivanov-bg
Copy link
Author

ivivanov-bg commented Mar 19, 2024

Yes
Attached is updated sample-app
sample-app.zip

But again - this only fixes my problem (and only partially)
I think all those mixins needs to be analyzed and decide how to proceed in native mode.

@zhfeng
Copy link
Contributor

zhfeng commented Mar 19, 2024

Thanks @ivivanov-bg - yeah, it works by adding all the mixin classes and relavant JsonSerializer. I will prepare a PR with an integration test.

@zhfeng
Copy link
Contributor

zhfeng commented Mar 20, 2024

@ivivanov-bg can you check the fix with your reproducer? If it works, I will merge the PR.

@ivivanov-bg
Copy link
Author

I just tested it - it's working as expected now
Thanks

@jamesnetherton jamesnetherton added this to the 3.9.0 milestone Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants