2.14.0.CR1 Class with @Provider and @RequestScoped annotations causes build failure

[bcluap]

When changing from 2.13.3.Final to 2.14.0.CR1 Quarkus build fails on our project:

Failed to execute goal io.quarkus:quarkus-maven-plugin:2.14.0.CR1:build (default) on project eclipse-deployable: Failed to build quarkus application: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.arc.deployment.ArcProcessor#validate threw an exception: javax.enterprise.inject.spi.DeploymentException: Found 304 deployment problems: 
[1] Unsatisfied dependency for type com.ukheshe.arch.rest.RequestContext and qualifiers [@Default]
	- java member: com.ukheshe.services.reporting.aaa.BaseReportingGateKeeper#context
	- declared on CLASS bean [types=[com.ukheshe.arch.aaa.GateKeeper, com.ukheshe.arch.impl.aaa.SourceIpCheckingGateKeeper, com.ukheshe.arch.impl.aaa.RoleBasedGateKeeper, com.ukheshe.services.reporting.aaa.MoneyMessageReportingGateKeeper, com.ukheshe.services.reporting.aaa.BaseReportingGateKeeper, java.lang.Object], qualifiers=[@Default, @Any], target=com.ukheshe.services.reporting.aaa.MoneyMessageReportingGateKeeper]

The full list of errors is huge but all relate to injecting com.ukheshe.arch.rest.RequestContext
The implementation of this class is slightly unique in that it looks like this:

@Provider
@PreMatching
@RequestScoped
@Priority(Priorities.HEADER_DECORATOR + 200)
public class RequestContextImpl implements ContainerRequestFilter, RequestContext {

private ContainerRequestContext containerRequestContext;

@Override
    public void filter(ContainerRequestContext requestContext) throws IOException {
        this.containerRequestContext = requestContext;
    }

If I remove the @Provider annotation then the build succeeds but the application has issues because this class needs to be a ContainerRequestFilter to get the requestContext and use it when this RequestScoped bean is injected.

Not sure if 2.14 has changed something and wont properly process beans that have both @Provider and @RequestScoped

[gsmet]

@bcluap thanks for the report. A reproducer would be welcome.

[bcluap]

I'll see what I can do. A bit swamped today but will get something to you

[gsmet]

/cc @geoand @mkouba for your awareness

[geoand]

I won't be around until Monday to look at this.

Also cc @Sgitario

[geoand]

Also cc @manovotn who might have made a related change

[bcluap]

Reproducer:
git clone [email protected]:bcluap/quarkus-examples.git
cd quarkus-examples/resteasy-reactive
mvn verify

[geoand]

@manovotn this seems to be caused by #28429.

We really need to address this one for 2.14.0.Final, can you please take a look?

Thanks

[gsmet]

I will revert it if we can't find a solution by then.

[geoand]

Yeah, that makes perfect sense.

[manovotn]

@manovotn this seems to be caused by #28429.

I am looking at it now; will report back once I know more.

I will revert it if we can't find a solution by then.

Agreed.

[manovotn]

So, the problem lies with this code - https://github.com/quarkusio/quarkus/blob/main/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java#L730-L790

We limit bean types of RestEasy classes (such as those with @Provider) to that of the implementation bean which meets internal needs for typesafe resolution in BeanContainer#beanInstanceFactory that RR uses.
However, in this reproducer, the class is also a user bean implementing custom interface and after our modification, the bean loses this interface as its type for the sake of typesafe resolution.

@bcluap note that the reproducer app can be made to work if you instead inject the exact class, i.e. instead of @Inject RequestContext, you can do @Inject RequestContextImpl and it will work just fine. Alternatively, annotating RequestContextImpl with @Typed({RequestContextImpl.class, RequestContext.class}) will do the trick as well and you can then perform the injection based on the interface type.

I am not yet sure how to fix this. We need type restriction if the instance factory should behave according to CDI rules but that inevitably breaks scenarios such as the one presented here...
Thoughts @mkouba?

[mkouba]

Well, I'm not sure it's a good idea to use a bean class that implements the ContainerRequestFilter separately, i.e. via @Inject. And the @Typed workaround seems to me reasonable for this particular case.

[manovotn]

Well, I'm not sure it's a good idea to use a bean class that implements the ContainerRequestFilter separately, i.e. via @Inject

I didn't want to judge that because I am not a RE expert so I have no idea if this common case or not; @geoand might know more.
But I do agree this can get weird really fast and not just with types but also with scopes.

As for solutions:

1. I don't think we can fine-grain how we limit types of beans because we cannot reliably tell which types are "good" and which should be removed
2. We can revert back to using non-CDI behavior for instance factories (which a revert of the original PR would do)
3. We can consider this an existing downside of resteasy having providers and resources automatically treated as beans with known workarounds

Personally, I'd lean towards 3. with this case.

[geoand]

Well, I'm not sure it's a good idea to use a bean class that implements the ContainerRequestFilter separately
I didn't want to judge that because I am not a RE expert so I have no idea if this common case or not; @geoand might know more.

Yeah, this is indeed weird - it would only work in Quarkus and not in any other JAX-RS / CDI implementation. It's not something we have ever said we support, it worked previously by accident.

If we are going to disallow this case, #28429 needs to be a breaking change (I added the label)

[manovotn]

With the above comments, unless @geoand or @gsmet are opposed, I'd suggest we close this issue as not a bug with known workaround.

[geoand]

I'd like to hear from @bcluap on the reasoning behind using this particular pattern

[bcluap]

Hi All, So this weird scenario I have actually came from a discussion with most of you a few years back. Check on quarkus-dev for topic "Injecting @context HttpHeaders at instance level on @RequestScoped" I was trying to find a way to be able to inject JAX-RS stuff using Quakus and this solution presented itself and has been working great. In fact if you search quarkus-dev and tulip you will find its been a suggestion for quite a few people who have wanted to inject things based on ContainerRequestContext.

…

On Mon, Oct 31, 2022 at 12:15 PM Georgios Andrianakis < ***@***.***> wrote: I'd like to hear from @bcluap <https://github.com/bcluap> on the reasoning behind using this particular pattern — Reply to this email directly, view it on GitHub <#28889 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGMB23P6ASXEYJXX4G3NZVLWF6L5BANCNFSM6AAAAAARQJXYFY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[geoand]

Thanks for that info!

Given that this kind of thing is no longer necessary (as RR fuses CDI and JAX-RS seamlessly), I am personally in favor of dropping support for this.

[bcluap]

But I cant seem to inject ContainerRequestContext using @Inject nor @context.

…

On Mon, Oct 31, 2022 at 12:50 PM Georgios Andrianakis < ***@***.***> wrote: Thanks for that info! Given that this kind of thing is no longer necessary (as RR fuses CDI and JAX-RS seamlessly), I am personally in favor of dropping support for this. — Reply to this email directly, view it on GitHub <#28889 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGMB23P6BNHHSMLSUBAMTTLWF6P5TANCNFSM6AAAAAARQJXYFY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[geoand]

Indeed ContainerRequestContext is not a bean type, so it's not eligible for injection.

But you essentially want to access HttpHeaders, correct?

[bcluap]

Yea, but a bit more than that. Here are the things we use it for: containerRequestContext.getUriInfo(); containerRequestContext.getHeaderString(name); containerRequestContext.getHeaders(); containerRequestContext.getProperty(name); containerRequestContext.setProperty(name, value); containerRequestContext.getPropertyNames() containerRequestContext.getAcceptableLanguages()

…

On Mon, Oct 31, 2022 at 1:02 PM Georgios Andrianakis < ***@***.***> wrote: Indeed ContainerRequestContext is not a bean type, so it's not eligible for injection. But you essentially want to access HttpHeaders, correct? — Reply to this email directly, view it on GitHub <#28889 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGMB23NYOC4GDIYW42EUUTTWF6RMDANCNFSM6AAAAAARQJXYFY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[bcluap]

I've added @typed({RequestContextImpl.class, RequestContext.class}) and it works and our tests pass on 2.14.0.CR1 so that's great news. As for this ticket, happy for it to be closed but I do just with that there was a clean way of getting hold of RequestContext without passing it around. It just makes so much sense for this to be injectable as a RequestScoped bean, even if its not what the specs say. Maybe RR could have something like this enabled. Dunno just wondering but for us we are sorted. Thanks guys.

…

On Mon, Oct 31, 2022 at 1:07 PM Paul Carter-Brown ***@***.***> wrote: Yea, but a bit more than that. Here are the things we use it for: containerRequestContext.getUriInfo(); containerRequestContext.getHeaderString(name); containerRequestContext.getHeaders(); containerRequestContext.getProperty(name); containerRequestContext.setProperty(name, value); containerRequestContext.getPropertyNames() containerRequestContext.getAcceptableLanguages() On Mon, Oct 31, 2022 at 1:02 PM Georgios Andrianakis < ***@***.***> wrote: > Indeed ContainerRequestContext is not a bean type, so it's not eligible > for injection. > > But you essentially want to access HttpHeaders, correct? > > — > Reply to this email directly, view it on GitHub > <#28889 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AGMB23NYOC4GDIYW42EUUTTWF6RMDANCNFSM6AAAAAARQJXYFY> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> >

[geoand]

#28946 will allow you to inject ContainerRequestContext directly

[bcluap]

Georgios, you really never fail to amaze me :-)

…

On Mon, Oct 31, 2022 at 1:23 PM Georgios Andrianakis < ***@***.***> wrote: #28946 <#28946> will allow you to inject ContainerRequestContext directly — Reply to this email directly, view it on GitHub <#28889 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGMB23N22JPK7DUHOW7N2ADWF6T43ANCNFSM6AAAAAARQJXYFY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[geoand]

Hahaha ❤️

The community makes all this possible :)