Spring-di's @Scope#session throws ContextNotActiveException

[jedla97]

Describe the bug

After the merge of #45170 the @Scope(scopeName = "session") stopped working and throwing the jakarta.enterprise.context.ContextNotActiveException: SessionScoped context was not active when trying to obtain a bean instance for a client proxy of PRODUCER_METHOD bean [class=io.quarkus.ts.spring.data.primitivetypes.configuration.AppConfiguration, id=9767shscEaYLEayHF-_VBq-X1Aw]

Other scopes seems working fine. So maybe with recent change the SpringDIProcessor needs to updated.

Expected behavior

Not getting ContextNotActiveException when using @Scope(scopeName = "session")

Actual behavior

jakarta.enterprise.context.ContextNotActiveException: SessionScoped context was not active when trying to obtain a bean instance for a client proxy of PRODUCER_METHOD bean [class=io.quarkus.ts.spring.data.primitivetypes.configuration.AppConfiguration, id=9767shscEaYLEayHF-_VBq-X1Aw]
        at io.quarkus.arc.impl.ClientProxies.notActive(ClientProxies.java:70)
        at io.quarkus.arc.impl.ClientProxies.getDelegate(ClientProxies.java:58)
        at io.quarkus.ts.spring.data.primitivetypes.configuration.AppConfiguration_ProducerMethod_sessionIdBean_3pCqCWZn6or_axTX0l-b0tdJ7hs_ClientProxy.arc$delegate(Unknown Source)
        at io.quarkus.ts.spring.data.primitivetypes.configuration.AppConfiguration_ProducerMethod_sessionIdBean_3pCqCWZn6or_axTX0l-b0tdJ7hs_ClientProxy.getSessionId(Unknown Source)
        at io.quarkus.ts.spring.data.rest.MagazineResource.test(MagazineResource.java:38)
        at io.quarkus.ts.spring.data.rest.MagazineResource$quarkusrestinvoker$test_056288de5192b9be1916ebef3bf0275a7088b428.invoke(Unknown Source)
        at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29)
        at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:141)
        at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)
        at io.quarkus.vertx.core.runtime.VertxCoreRecorder$15.runWith(VertxCoreRecorder.java:637)
        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 java.base/java.lang.Thread.run(Thread.java:1583)

How to Reproduce?

1. git clone -b spring-session-scoped git@github.com:jedla97/quarkus-reproducers.git
2. cd quarkus-reproducers
3. quarkus dev
4. Visit http://127.0.0.1:8080/magazine-resource or http://127.0.0.1:8080/magazine-resource/1

We mainly propagated it to headers when running test. For simplification I add it as response value on MagazineResource#test endpoint (magazine-resource).

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

main

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

If you want to try it on our TS see https://github.com/quarkus-qe/quarkus-test-suite/tree/main/spring/spring-data

You can run run it by mvn -fae -V -B clean verify -Dreruns=0 -Pspring-modules -pl spring/spring-data

[quarkus-bot]

/cc @aureamunoz (spring), @geoand (spring)

[jedla97]

cc @mkouba as creator of #45170

[mkouba]

AFAIK the session scope was never supported. @geoand do you happen to know how quarkus-spring-di handles the session scope?

[geoand]

It never did. If it worked before, that was purely accidental

[aureamunoz]

From the doc:
Doesn’t have a one-to-one mapping to a CDI annotation. Depending on the value of @scope, one of the @singleton, @ApplicationScoped, @SessionScoped, @RequestScoped, @dependent could be used

[manovotn]

From the doc: Doesn’t have a one-to-one mapping to a CDI annotation. Depending on the value of @scope, one of the @singleton, @ApplicationScoped, @SessionScoped, @RequestScoped, @dependent could be used

Right but what Martin means is that ArC never had an actual session scope impl so we must have ignored it or transformed it into another one whereas now it seems to use the newly added context (which is for tests).

[mkouba]

I believe that the scope was previously simply ignored. The SpringDIProcessor turns @Scope(scopeName = "session") into @jakarta.enterprise.context.SessionScoped but since it was not a bean defining annotation the scope was ignored and @Dependent was used for a producer method.

SpringDIProcessor should probably fail the deployment unless the quarkus-undertow (or any other extension that adds the support for session context) is present.

[manovotn]

I was looking into the Spring processor as well and came to the same conclusion.

SpringDIProcessor should probably fail the deployment unless the quarkus-undertow (or any other extension that adds the support for session context) is present.

While this is the right solution, it might break existing apps - an alternative would be to re-implement the fallback to @Dependent while logging a warning as such behavior is probably unintentional at best.

[mkouba]

While this is the right solution, it might break existing apps - an alternative would be to re-implement the fallback to @Dependent while logging a warning as such behavior is probably unintentional at best.

Good point. Although the fallback should not be used by default because this functionality was unintenional and erroneous. Maybe we should add a config knob, fail by default but enable the fallback if really needed.

[geoand]

Maybe we should add a config knob, fail by default but enable the fallback if really needed.

+1

[rsvoboda]

The fail message could mention the fallback configuration option.