Add CDI annotations to the annotations that need them. Updated the si…

[jamezp]

…gnature test profile to use the new maven plugin for generation.

This bumps CDI to the 4.0.0-M1 version and adds explicit module dependencies on jakarta.cdi and jakarta.inject. Technically we do not need the jakarta.inject dependency as it's transitive in jakarta.cdi. However, I felt we should be explicit.

Note that I've upgraded the sigtest-maven-plugin to use the latest jakarta.tck version. However, there is a bug which blocks it from working. I've manually updated the signature tests for now. We will need to update the signature file once a new release of the plugin is done with the fix.

resolves #952
resolves #556

[jansupol]

Can it possibly clash with @singleton?

[jamezp]

TBH I don't know how that works since @Singleton is not part of CDI. However, resources should be @RequestScoped per the specification documentation. I don't see anything in the CDI specification that clears this up.

[jansupol]

CDI says @Singleton is the pseudoscope. Which I am not sure how it is handled by Weld. Would need to do some testing.

However, the REST spec supports the @jakarta.inject.Singleton resource classes, for instance in Section 9.4.

It also mandates:

In a product that supports EJBs, an implementation MUST support the use of stateless and singleton session beans as root resource classes

Though it is jakarta.ejb.Singleton, I still feel some potential clash with @RequestScoped.

[jansupol]

This is what Weld throws (3.1 @Path - Not modified, yet) :

org.jboss.weld.exceptions.DefinitionException: WELD-000046: At most one scope may be specified on [EnhancedAnnotatedTypeImpl] public @RequestScoped @Singleton @Path class AnnotatedTestResource

[jamezp]

That only happens if you add both @RequestScoped and @Singleton to the resource though. If you only add @Singleton and @Path it will work. A stereotypes scope can be overridden as it's just the default scope. From the end of https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0#stereotype_default_scope

For example, the following stereotype might be used to identify action classes in a web application:

@RequestScoped
@Stereotype
@Target(TYPE)
@Retention(RUNTIME)
public @interface Action {}

Then actions would have scope @RequestScoped unless the scope is explicitly specified by the bean.

[jansupol]

Thank you, I was not aware of it, that resolves my concerns. Keeping unresolved for others should they have the same doubts.

[jansupol]

For the pure client and SE env we should not mandate CDI API. Unlike with classes, JDK ignores annotations when it does not know the dependency.

[jamezp]

For the pure client and SE env we should not mandate CDI API. Unlike with classes, JDK ignores annotations when it does not know the dependency.

Agreed and I made both the jakarta.cdi and jakarta.inject modules optional.

[jansupol]

There is one more catch, each @Provider now becomes a bean. While in 3.1 it was known only when registered, now in CDI the bean is there.

What's the expected behaviour, should the unregistered @Provider be used, or should the implementation keep track of registered providers before allowing the bean to be used?

While the former more comply with the @Provider javadoc

Marks an implementation of an extension interface that should be discoverable by JAX-RS runtime during a provider
scanning phase.

the scanning phase happens just when the @ApplicationPath is used in 3.1 and thus this is a change in the behavior.

The latter on the other hand looks like an unpleasant overhead given all the possibilities of how the provider can be registered.

[jamezp]

There is one more catch, each @Provider now becomes a bean. While in 3.1 it was known only when registered, now in CDI the bean is there.

What's the expected behaviour, should the unregistered @Provider be used, or should the implementation keep track of registered providers before allowing the bean to be used?

While the former more comply with the @Provider javadoc

Marks an implementation of an extension interface that should be discoverable by JAX-RS runtime during a provider
scanning phase.

the scanning phase happens just when the @ApplicationPath is used in 3.1 and thus this is a change in the behavior.

The latter on the other hand looks like an unpleasant overhead given all the possibilities of how the provider can be registered.

I'd need to think more through the lifecycle, but you might be able to veto the types that are not in an Application.getClasses() in a CDI extension. I assume it could also be filtered in the jakarta.ws.rs.ext.Providers lookup methods.

[jansupol]

'd need to think more through the lifecycle, but you might be able to veto the types that are not in an Application.getClasses() in a CDI extension. I assume it could also be filtered in the jakarta.ws.rs.ext.Providers lookup methods.

The problem I see is that during the CDI extension run, there is no Application available yet, as the Application might be both not annotated with a bean-defining annotation or the application might be scanned after the provider being handled in the extension.

Yes, it can be filtered in the Providers, but then the implementation needs to know the list of registered providers (but the provider might be registered directly into the injection framework).

Maybe I miss something. Or perhaps a clarification of provider discovery in a CDI environment can be added to a Spec. I am under the impression that now (at least in Jersey) the providers annotated as CDI beans are used in the CDI-managed environment (such as Helidon).

[jamezp]

'd need to think more through the lifecycle, but you might be able to veto the types that are not in an Application.getClasses() in a CDI extension. I assume it could also be filtered in the jakarta.ws.rs.ext.Providers lookup methods.

The problem I see is that during the CDI extension run, there is no Application available yet, as the Application might be both not annotated with a bean-defining annotation or the application might be scanned after the provider being handled in the extension.

Yes, that's my concern with being able to use a CDI extension as well. I'm not sure, like you say, the Application would be available during CDI processing.

Yes, it can be filtered in the Providers, but then the implementation needs to know the list of registered providers (but the provider might be registered directly into the injection framework).

I do understand this concern as well. It will likely be a bit messy, I haven't thought it fully through yet, but there will probably need to be lookup in the BeanManager to do the filtering.

Maybe I miss something. Or perhaps a clarification of provider discovery in a CDI environment can be added to a Spec. I am under the impression that now (at least in Jersey) the providers annotated as CDI beans are used in the CDI-managed environment (such as Helidon).

I'm all for some clarification in the spec. IMO it's okay to require the Application to be a CDI bean in an environment that supports CDI.

[jansupol]

One of the reasons to come to CDI is that CDI does all the magic which the REST implementation does not need to do. That was why we wanted the CDI to be able to invoke the resource methods so that the implementation does not need to.

And now it looks like we want to go against how CDI works and filter its beans somehow. (We may filter those beans, but the user still would see them all should he used the BeanManager directly).

What's even more important is what the customer want to have. I am in doubt the customer wants to come happily to the CDI environment, create CDI beans, and then have to register those beans in the application to beat our tremendous effort to filter unregistered beans.

Should we not define the Providers to be auto-added in the CDI environment, i.e. to define "discoverable" in the CDI container?

I am trying to find the behaviour that should be expected by the Spec, in the CDI container, and I am trying to see how much incompatible the changes could potentially be.

[jamezp]

What's even more important is what the customer want to have. I am in doubt the customer wants to come happily to the CDI environment, create CDI beans, and then have to register those beans in the application to beat our tremendous effort to filter unregistered beans.

I definitely agree with this and it's one of my primary reasons I like the plan of adding the @Deprecated annotation to the @Context annotation. It really gives users a chance to migrate incrementally instead of all at once.

Should we not define the Providers to be auto-added in the CDI environment, i.e. to define "discoverable" in the CDI container?

I feel it's not that different to what we have now TBH. The spec does require Providers are CDI beans in a CDI environment. That said, I do know in RESTEasy before we make a provider a CDI bean, we do check to see if it CAN be a CDI bean.

That said, with a CDI portable extension this should still work by checking if a bean cannot be a CDI bean and veto it.

I am trying to find the behaviour that should be expected by the Spec, in the CDI container, and I am trying to see how much incompatible the changes could potentially be.

Totally understandable for sure. The language to use in the spec personally seems the trickiest to me. Not that implementing will be easy, but wording it so all implementations at least roughly behave the same is the trick.

[NicoNes]

There is one more catch, each @Provider now becomes a bean. While in 3.1 it was known only when registered, now in CDI the bean is there.

What's the expected behaviour, should the unregistered @Provider be used, or should the implementation keep track of registered providers before allowing the bean to be used?

Same catch for resource type annotated with new CDI stereotype @Path that now becomes a bean right ?

What's even more important is what the customer want to have. I am in doubt the customer wants to come happily to the CDI environment, create CDI beans, and then have to register those beans in the application to beat our tremendous effort to filter unregistered beans.

For what it worth, here are my two cent's as a user:

• both resource types annotated with CDI stereotype @Path and provider types annotated with CDI stereotype @Provider not vetoed, should follow CDI rules and be added to the CDI context with no consideration of how the JAKARTA RS application will use them or not.
• then if I have no jakarta.ws.rs.Application annotated with @ApplicationPath or a jakarta.ws.rs.Application annotated with @ApplicationPath with both methods application .getClasses() and application.getSingletons() that return an empty collection, then all resources types annotated with @Path and all providers annotated with @Provider should be discovered by the JAKARTA RS impl from the CDI context and registered.
• else if I have a jakarta.ws.rs.Application annotated with @ApplicationPath with either method application .getClasses() or application.getSingletons() returns a non empty collection then no discovery is needed and then only resources and providers types registered in application .getClasses() and application.getSingletons() should be registered by the JAKARTA RS impl

I think thats how QUARKUS behave for example, except that @ApplicationPath is not mandatory.

-- Nicolas

[jamezp]

Note that for some reason signature tests are failing for me on two default constants:

Missing Fields
--------------

jakarta.ws.rs.core.Cookie:              field public final static int jakarta.ws.rs.core.Cookie.DEFAULT_VERSION
--- affected jakarta.ws.rs.core.NewCookie
jakarta.ws.rs.core.NewCookie:           field public final static int jakarta.ws.rs.core.NewCookie.DEFAULT_MAX_AGE

Added Fields
------------

jakarta.ws.rs.core.Cookie:              field public final static int jakarta.ws.rs.core.Cookie.DEFAULT_VERSION = 1
--- affected jakarta.ws.rs.core.NewCookie
jakarta.ws.rs.core.NewCookie:           field public final static int jakarta.ws.rs.core.NewCookie.DEFAULT_MAX_AGE = -1

duplicate messages suppressed: 2


02-20-2024 18:14:34:********** Package 'jakarta.ws.rs.core' - FAILED (STATIC MODE) **********

I'm not sure why yet, but I have a feeling it's tooling on the test side, not what was generated by the tool as those lines did not change.

[spericas]

LGTM, thanks.

[jansupol]

I feel it's not that different to what we have now TBH. The spec does require Providers are CDI beans in a CDI environment. That > said, I do know in RESTEasy before we make a provider a CDI bean, we do check to see if it CAN be a CDI bean.

That said, with a CDI portable extension this should still work by checking if a bean cannot be a CDI bean and veto it.

I am sorry, I do not follow. What do you mean by "it CAN be a CDI bean"? When it cannot be a CDI bean?

[jamezp]

That said, with a CDI portable extension this should still work by checking if a bean cannot be a CDI bean and veto it.

I am sorry, I do not follow. What do you mean by "it CAN be a CDI bean"? When it cannot be a CDI bean?

Sorry, what I mean by this is you during processing you could check if the bean is a valid CDI bean and if not veto it. Something like:

public <T> void observeProviders(@WithAnnotations({ Provider.class }) @Observes ProcessAnnotatedType<T> event) {
    AnnotatedType<T> annotatedType = event.getAnnotatedType();

    if (isUnproxyableClass(annotatedType.getJavaClass())) {
            event.veto();
    }
}

The isUnproxyableClass() could check for things like making sure the type is not final. Ensuring there is a usable constructor, etc.

Without making these annotations bean defining annotations the user would be required to use a beans.xml file and explicitly list each type or use bean-discovery-mode="all" which is no longer the default. Or they would need to add a bean defining annotation to each provider and resource.

[jamezp]

• then if I have no jakarta.ws.rs.Application annotated with @ApplicationPath or a jakarta.ws.rs.Application annotated with @ApplicationPath with both methods application .getClasses() and application.getSingletons() that return an empty collection, then all resources types annotated with @Path and all providers annotated with @Provider should be discovered by the JAKARTA RS impl from the CDI context and registered.

This feels backwards to me. CDI would not discover the jakarta.ws.rs.Application if it's not annotated. Maybe you mean if the application is defined in the web.xml, is that correct?

• else if I have a jakarta.ws.rs.Application annotated with @ApplicationPath with either method application .getClasses() or application.getSingletons() returns a non empty collection then no discovery is needed and then only resources and providers types registered in application .getClasses() and application.getSingletons() should be registered by the JAKARTA RS impl

I still thing discovery is needed here. I actually don't really know how we'd determine this because we can't lookup the application and determine this during bean scanning.

The Application.getSingletons() can't be CDI beans as they are already instances. That method is already deprecated anyway.

[NicoNes]

This feels backwards to me. CDI would not discover the jakarta.ws.rs.Application if it's not annotated. Maybe you mean if the application is defined in the web.xml, is that correct?

Maybe I was not clear enough, sorry for that but I think we are saying the same thing.
What I meant was: If CDI did not discover no jakarta.ws.rs.Application, either because it was not annotated with @ApplicationPath or because it does not exist at all then the published JAKARTA RS application should contains all resources types annotated with @path and all providers types annotated with @Provider discovered and present in the CDI context.

I still thing discovery is needed here.

Just to be clear I'm not talking about CDI discovery. CDI discovery is mandatory for me as well.
When I say discovery, I mean "JAKARTA RS auto discovery" , the process by which JAKARTA RS impl finds providers and resources to register.
Well, as a user if I create a jakarta.ws.rs.Application annotated with @ApplicationPath and implement getClasses() its to explicitely declare the specific CDI bean resource types and specific CDI bean provider types from the CDI context that I want to be registered in the JAKARTA RS application. Of course if any of those types are not resolvable from the CDI context an error must be thrown/logged.

I think that mixing both JAKARTA RS auto discovery and explicit declration of providers and resource to register can be a bit misleading for user and will go againts the previous spec (section 2.3.2) where both explicit declaration and auto-discovery where exclusive.

I actually don't really know how we'd determine this because we can't lookup the application and determine this during bean scanning.

Well maybe I'm missing something and do not hesitate to tell me, but why do we want to do this during CDI discovery ?
I mean maybe we can wait for CDI context creation to finish then after that start building the JAKARTA RS application based on the content of the CDI context.

• If the CDI context contains a jakarta.ws.rs.Application beans whose method getClasses() returns a non empty collection then let's use it
• else lets use all provider beans and resource beans from CDI context to publish them in the JAKARTA RS application.

-- Nicolas

[jamezp]

@NicoNes Yes, it appears we were saying the same thing :) I apologize for adding confusion.