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

Add the message body serialize integration with reactive-rest #31

Merged
merged 2 commits into from
Nov 16, 2024

Conversation

zhfeng
Copy link
Contributor

@zhfeng zhfeng commented Nov 15, 2024

No description provided.

the tests into two modules since we can not test quarkus rest and
reactive-rest at the same time
@zhfeng zhfeng requested a review from a team as a code owner November 15, 2024 06:52
@zhfeng zhfeng requested review from gastaldi and removed request for a team November 15, 2024 06:52
@Path("/fury")
@ApplicationScoped
@RegisterForReflection
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gastaldi I have to add @RegisterForReflection here to make FuryResource working in native mode. It looks like we need it when the JAX-RS reources are from the dependencies?

Copy link
Member

Choose a reason for hiding this comment

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

Probably because it was removed from ArC? Tried with @Unremovable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error message in native tests looks like

2024-11-15 14:02:44,802 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-1) HTTP Request to /fury/test failed, error id: 9d9ff831-cab9-46ad-ab65-87ff019e74e4-1: java.lang.RuntimeException: java.lang.NoSuchMethodException: io.quarkiverse.fury.it.FuryResources.testBar(io.quarkiverse.fury.it.Bar)
	at org.jboss.resteasy.reactive.server.spi.ResteasyReactiveResourceInfo.getMethod(ResteasyReactiveResourceInfo.java:84)
	at org.jboss.resteasy.reactive.server.spi.ResteasyReactiveResourceInfo.getParameterAnnotations(ResteasyReactiveResourceInfo.java:125)
	at org.jboss.resteasy.reactive.server.handlers.RequestDeserializeHandler.getAnnotations(RequestDeserializeHandler.java:133)
	at org.jboss.resteasy.reactive.server.handlers.RequestDeserializeHandler.isReadable(RequestDeserializeHandler.java:118)
	at org.jboss.resteasy.reactive.server.handlers.RequestDeserializeHandler.handle(RequestDeserializeHandler.java:78)
	at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:135)
	at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)
	at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:627)
	at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
	at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:11)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:11)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at [email protected]/java.lang.Thread.runWith(Thread.java:1596)
	at [email protected]/java.lang.Thread.run(Thread.java:1583)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.thread.PlatformThreads.threadStartRoutine(PlatformThreads.java:896)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.thread.PlatformThreads.threadStartRoutine(PlatformThreads.java:872)
Caused by: java.lang.NoSuchMethodException: io.quarkiverse.fury.it.FuryResources.testBar(io.quarkiverse.fury.it.Bar)
	at [email protected]/java.lang.Class.checkMethod(DynamicHub.java:1078)
	at [email protected]/java.lang.Class.getMethod(DynamicHub.java:1063)
	at org.jboss.resteasy.reactive.server.spi.ResteasyReactiveResourceInfo.getMethod(ResteasyReactiveResourceInfo.java:79)
	... 18 more


I suppose it miss the reflection information. Will try @Unremovable

Copy link
Member

Choose a reason for hiding this comment

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

Another shot is to add an empty beans.xml in the project to hint Jandex to scan it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I added jandex-maven-plugin. so it should be good.

Copy link

Choose a reason for hiding this comment

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

I can look into it when there is a reproducer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it only fails to find the method below, the other metheds all look good. It's weird and any hint?

    @POST
    @Path("/test")
    @Produces("application/fury")
    @Consumes("application/fury")
    public Bar testBar(Bar obj) {
        ...
   }

Copy link
Member

@gastaldi gastaldi Nov 16, 2024

Choose a reason for hiding this comment

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

Weird, could it be because it starts with test?

Nvm, the other methods have too.Hm, could be something about having @Produces and @Consumes for a type that isn't handled by Quarkus

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other fact is this test works with quarkus-resteasy which is classic rest. But fails with quarkus-rest which is reactive rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I have a reproducer with quarkus-fury and quarkus-rest. But I need to do a new temporary release of quarkus-fury to include this PR.

@zhfeng
Copy link
Contributor Author

zhfeng commented Nov 16, 2024

Sorry, I need to do a temporary release to include this PR to have a handy reproducer about investigating

@zhfeng zhfeng merged commit 0a39d91 into quarkiverse:main Nov 16, 2024
1 check passed
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