eclipse-ee4j / jersey

Eclipse Jersey Project - Read our Wiki:
https://github.com/eclipse-ee4j/jersey/wiki
Other
691 stars 352 forks source link

Failed to Inject ResourceContext and UriInfo by @Inject #5100

Open hantsy opened 2 years ago

hantsy commented 2 years ago

Glassfish 7.0.0-M7, Java 17, Windows 10.

As described in Jakarta EE 10 spec, the Jaxrs 3.1 should align to CDI and support to inject Jaxrs resource by general @Inject annotation.

I have tried to use @Inject instead of Jaxrs @Context, all failed.

@Path("todos")
@RequestScoped
public class TodoResources {

    //@Inject
    @Context
    ResourceContext resourceContext;

    //@Inject
    @Context
    UriInfo uriInfo;
jansupol commented 2 years ago
  1. Injecting using @Context used to work at least since 2.0, I am sure there are multiple tests that verify the functionality. Do you have an application that is testing it so we can give it a try?
  2. Support of @Inject is not mandated by the Jakarta REST Spec 3.1, AFAIK. It was the original plan, but I'd say it is not a requirement for 3.1. Can you, please, point to the spec where @Inject support is mentioned?
  3. Jersey has the support of @Inject implemented by org.glassfish.jersey.ext.cdi:jersey-cdi-rs-inject, but that module is not in Glassfish.
hantsy commented 2 years ago
  1. Check my sandbox project: https://github.com/hantsy/jakartaee10-sandbox/tree/master/rest but there is a Datasource issue of the default db to stop the startup of the Ejb @Startup beans, https://github.com/eclipse-ee4j/glassfish/issues/24048
  2. I read this article: https://dev.to/andymc12/what-s-coming-in-jakarta-rest-3-1-ole , in it it mentioned @Context will be deprecated and replaced with @Inject for better alignment with CDI.
  3. ~Can I file an issue on Glassfish?~ filed an issue: https://github.com/eclipse-ee4j/glassfish/issues/24049
jansupol commented 2 years ago
  1. My suspicion is that your resource class is not properly instantiated - it is not referenced from the Application subclass (getClasses). The resource class should not be working at all - I assume it is instantiated by CDI, which does not know @Context, it should have been instantiated by Jersey/HK2. (I have not tried to modify your test case, though).
  2. The article is more than a year old, that time, that was the plan. But EE 10 was planned for September 2021 and the tight time frame did not allow a proper replacement for @Context (@Inject cannot be used in method arguments the way @Context is) and without the replacement, the @Context could not be deprecated.
hantsy commented 2 years ago

My suspicion is that your resource class is not properly instantiated - it is not referenced from the Application subclass (getClasses). The resource class should not be working at all - I assume it is instantiated by CDI, which does not know @Context, it should have been instantiated by Jersey/HK2. (I have not tried to modify your test case, though).

For the Jakarta EE /application server env, it is not required to register resource classes in Application, it will be discovered automatically by container(application servers).

In the former Jaxrs, a Resource class works with a simple @Path, or CDI annotation, such as @RequestScope and @Path, or EJB @Stateless with @Path. In before projects, I would like use EJB form to get transaction automatically.

hantsy commented 2 years ago

(@Inject cannot be used in method arguments the way @Context is) and without the replacement, the @Context could not be deprecated.

But @Inject can be put on the method level for argument injection.

@Inject
public void someMethods(SomeBean bean){}
jansupol commented 2 years ago

For the Jakarta EE /application server env, it is not required to register resource classes in Application, it will be discovered automatically by container(application servers).

Right, I missed the @ApplicationPath annotation, sorry. I was trying to think why the injection is not working, but I run your example and @Context injection works just fine.

But @Inject can be put on the method level for argument injection.

@Inject
public void someMethods(SomeBean bean){}

Correct, the problem is

  1. The backwards incompatibility -> so need for major 4.0 release
  2. method like `post(SomeEntity entity, @Context UriInfo info, @Context Application app, @Context...) where there is not every argument injected, and Jakarta REST 4.0 may have another annotation for the entity (@Entity ?), and that brings another backward incompatibility
hantsy commented 2 years ago

I found this section in Rest spec 3.1 Final Release: https://jakarta.ee/specifications/restful-ws/3.1/jakarta-restful-ws-spec-3.1.html#context-injection

1.1.1. Support for @Context Injection As part of an effort to better align with Jakarta CDI, future versions of this API will no longer support @Context injection and related types such as ContextResolver. As much as possible, all injection tasks will be delegated to Jakarta CDI for a better integration into the Jakarta EE ecosystem.

jansupol commented 2 years ago

Yes, the future version is 4.0, where indeed the plan is to drop @Context. This sentence is a compromise, @Context is not deprecated in 3.1, but will be dropped in 4.0. Currently, I am not aware of any plan for any other 3.x release, 4.0 should be part of EE 11.

hantsy commented 2 years ago

I tried to add org.glassfish.jersey.ext.cdi:jersey-cdi-rs-inject in the project deps and got the following exception while deploying it.

SEVERE: exit_code: FAILURE, message: Error occurred during deployment: Exception while loading the app : CDI definition failure:Exception List with 1 exceptions:
Exception 0 :
org.jboss.weld.exceptions.IllegalArgumentException: WELD-001325: No instance of an extension class org.glassfish.jersey.ext.cdi1x.internal.CdiComponentProvider registered with the depl
oyment
        at org.jboss.weld.manager.BeanManagerImpl.getExtension(BeanManagerImpl.java:1421)
        at org.jboss.weld.util.ForwardingBeanManager.getExtension(ForwardingBeanManager.java:228)
        at org.glassfish.jersey.ext.cdi1x.inject.internal.InjectExtension.beforeDiscoveryObserver(InjectExtension.java:70)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
jansupol commented 2 years ago

Do you have org.glassfish.jersey.ext.cdi:jersey-cdi1x there, too?

hantsy commented 2 years ago

Yes, add a print in the @Deployment war archive preparing stage in my TodoResourceTest , it shows,

/WEB-INF/
/WEB-INF/lib/
/WEB-INF/lib/jersey-cdi-rs-inject-3.1.0-M7.jar
/WEB-INF/lib/jersey-cdi1x-3.1.0-M7.jar
/WEB-INF/lib/assertj-core-3.22.0.jar
/WEB-INF/classes/
/WEB-INF/classes/com/
/WEB-INF/classes/com/example/
/WEB-INF/classes/com/example/TodoResource.class
/WEB-INF/classes/com/example/TodoResources.class
/WEB-INF/classes/com/example/TodoService.class
/WEB-INF/classes/com/example/Todo.class
/WEB-INF/classes/com/example/TodoSamples.class
/WEB-INF/classes/com/example/RestConfig.class
/WEB-INF/classes/META-INF/
/WEB-INF/classes/META-INF/persistence.xml
/WEB-INF/beans.xml
Jul 22, 2022 9:35:38 AM org.jboss.arquillian.container.glassfish.clientutils.GlassFishClientUtil getResponseMap
SEVERE: exit_code: FAILURE, message: Error occurred during deployment: Exception while loading the app : CDI definition failure:Exception List with 1 exceptions:
Exception 0 :
org.jboss.weld.exceptions.IllegalArgumentException: WELD-001325: No instance of an extension class org.glassfish.jersey.ext.cdi1x.internal.CdiComponentProvider registered with the depl
oyment
        at org.jboss.weld.manager.BeanManagerImpl.getExtension(BeanManagerImpl.java:1421)
        at org.jboss.weld.util.ForwardingBeanManager.getExtension(ForwardingBeanManager.java:228)
        at org.glassfish.jersey.ext.cdi1x.inject.internal.InjectExtension.beforeDiscoveryObserver(InjectExtension.java:70)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
...
jansupol commented 2 years ago

Interesting, it looks like CDI Extensions are executed in the wrong order...

hantsy commented 2 years ago

And exploring glassfish7\glassfish\modules, I found there is a jersey-cdi1x existed, no jersey-cdi-rs-inject there.

OndroMih commented 2 years ago

@arjantijms, @dmatej , maybe jersey-cdi-rs-inject should be integrated into GF so that @Inject in resources works? There's an issue for it already: https://github.com/eclipse-ee4j/glassfish/issues/24049

hantsy commented 2 years ago

@OndroMih I've created issue https://github.com/eclipse-ee4j/glassfish/issues/24049, hope it will be integrated in next RC.

hantsy commented 2 years ago

@OndroMih I have tried to copy it to the galssfish/modules folder, still does not work.

https://github.com/hantsy/jakartaee10-sandbox/blob/master/rest/pom.xml#L354-L369

jansupol commented 2 years ago

The jersey-cdi-rs-inject module does not have the OSGi information to be working in GF :( Will need to fix that.

hantsy commented 2 years ago

Glassfish 7.0.0-M8 still does not include it?

jansupol commented 2 years ago

Nope, not yet, sorry.

jansupol commented 2 years ago

Jersey 3.1.0-M8 fixes the OSGi headers and it can be included in GF 7.

hantsy commented 2 years ago

@jansupol Still encountered exception in the latest Glassfish 7.0.0-M9. https://github.com/eclipse-ee4j/glassfish/issues/24049#issuecomment-1264200348