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

Vertx RouteFilter not propagating request context to a resteasy-reactive resource #34693

Closed
HerrDerb opened this issue Jul 12, 2023 · 13 comments
Closed
Labels
area/rest area/vertx kind/bug Something isn't working
Milestone

Comments

@HerrDerb
Copy link
Contributor

HerrDerb commented Jul 12, 2023

Describe the bug

When having a @RouteFilter setting information on a @RequestScoped bean, the bean is not being propagated correctly and the information is lost as a new request scope is beeing openend.

Expected behavior

The request scope of an @RouteFilter should be propagated to my jakarta.ws rest controller.

Actual behavior

A new request scope is beeing opened between my @RouteFilter and the invocation of my rest controller.

2023-07-12 10:24:06,341 INFO  [io.quarkus] (main) code-with-quarkus 1.0.0-SNAPSHOT on JVM (powered by Quarkus 3.1.2.Final) started in 2.462s. Listening on: http://localhost:8081
2023-07-12 10:24:06,367 INFO  [io.quarkus] (main) Profile test activated. 
2023-07-12 10:24:06,367 INFO  [io.quarkus] (main) Installed features: [cdi, jacoco, reactive-routes, resteasy-reactive, smallrye-context-propagation, vertx]
2023-07-12 10:24:07,578 WARN  [org.acm.iss.RequestScopedBean] (vert.x-eventloop-thread-0) ## New RequestScopedBean created
2023-07-12 10:24:07,579 WARN  [org.acm.iss.VertxRoutFilter] (vert.x-eventloop-thread-0) ## value from field 'info' of request scoped bean: testParamValue 
2023-07-12 10:24:07,584 WARN  [org.acm.iss.ExampleRestController] (executor-thread-1) ## Handeling getRequetScopedInfo
2023-07-12 10:24:07,584 WARN  [org.acm.iss.RequestScopedBean] (executor-thread-1) ## New RequestScopedBean created
2023-07-12 10:24:07,584 WARN  [org.acm.iss.ExampleRestController] (executor-thread-1) ## value from field 'info' of request scoped bean: null
2023-07-12 10:24:07,587 WARN  [org.acm.iss.RequestScopedBean] (executor-thread-1) ## RequestScopedBean destroyed
2023-07-12 10:24:07,587 WARN  [org.acm.iss.RequestScopedBean] (executor-thread-1) ## RequestScopedBean destroyed
2023-07-12 10:24:07,694 INFO  [io.quarkus] (main) code-with-quarkus stopped in 0.027s

How to Reproduce?

https://github.com/HerrDerb/quarkus-issue/tree/vertx-requestscope-issue

@HerrDerb HerrDerb added the kind/bug Something isn't working label Jul 12, 2023
@HerrDerb
Copy link
Contributor Author

HerrDerb commented Jul 12, 2023

Similar to #13073

@geoand
Copy link
Contributor

geoand commented Jul 12, 2023

cc @mkouba

@mkouba mkouba self-assigned this Jul 12, 2023
@mkouba mkouba changed the title Vertx RouteFilter not propagating @RequestScope Vertx RouteFilter not propagating request context to a resteasy-reactive resource Jul 12, 2023
@mkouba
Copy link
Contributor

mkouba commented Jul 12, 2023

It seems that ArcThreadSetupAction does not check if an existing context exists but always activates a new one.

I can confirm that we only have a test for reactive routes.

@geoand
Copy link
Contributor

geoand commented Jul 12, 2023

It seems that ArcThreadSetupAction does not check if an existing context exists but always activates a new one

Should we be checking if one already exists?

@mkouba
Copy link
Contributor

mkouba commented Jul 13, 2023

It seems that ArcThreadSetupAction does not check if an existing context exists but always activates a new one

Should we be checking if one already exists?

I think so, yes. The whole ArcThreadSetupAction should be a no-op if the context is already active. Hope that does not break anything ;-).

@geoand
Copy link
Contributor

geoand commented Jul 13, 2023

I am sure it will :)

But I'll check it out tomorrow.

@geoand
Copy link
Contributor

geoand commented Jul 13, 2023

Let's see what CI will say for #34724

geoand added a commit to geoand/quarkus that referenced this issue Jul 13, 2023
@HerrDerb
Copy link
Contributor Author

HerrDerb commented Jul 20, 2023

So in the meantime I'll just leave with two mutual exclusive request filters. What definitly feels odd is, that I need to include the reactive-routes dependency to filter vertx requests which are already enabled with resteasy reactive

@mkouba
Copy link
Contributor

mkouba commented Aug 1, 2023

What definitly feels odd is, that I need to include the reactive-routes dependency to filter vertx requests which are already enabled with resteasy reactive

In fact, you only need the reactive-routes for the declarative approach.

You can use the io.quarkus.vertx.http.runtime.filters.Filters as described in the javadoc: https://github.com/quarkusio/quarkus/blob/main/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/filters/Filters.java#L14-L24.

Or even observe and configure the io.vertx.ext.web.Router as described in this example: https://quarkus.io/guides/vertx-reference#bidirectional-communication-with-browsers-by-using-sockjs.

@mkouba mkouba removed their assignment Aug 1, 2023
@HerrDerb
Copy link
Contributor Author

HerrDerb commented Aug 14, 2023

@mkouba interestingly, the request scope seems only managed/ propagated properly when using the declarative approach with quarkus-reactive-routes. When registering the filter over @Observes, as you suggested to avoid the dependency, the request scope is not propagated correctly, more specificaly a new request scope is created after the filter on the executor thread.

@mkouba
Copy link
Contributor

mkouba commented Aug 14, 2023

@mkouba interestingly, the request scope seems only managed/ propagated properly when using the declarative approach with quarkus-reactive-routes. When registering the filter over @Observes, as you suggested to avoid the dependency, the request scope is not propagated correctly, more specificaly a new request scope is created after the filter on the executor thread.

Well, a declarative filter goes through RouteHandler#handle() which among other things only activates the request context if not active. A filter registered through an observer is used as-is.

@Eng-Fouad
Copy link
Contributor

Any updates regarding this issue? Thanks.

@geoand
Copy link
Contributor

geoand commented Jun 4, 2024

Closing as this was implemented in #40408

@geoand geoand closed this as completed Jun 4, 2024
@geoand geoand added this to the 3.8.5 milestone Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest area/vertx kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants