Deprecating JAX-RS-specific annotations in favor of CDI

[glassfishrobot]

Latest CDI specifications allows to use CDI with Java SE. Some JAX-RS specific annotations duplicate ideas also covered by CDI annotations.

For long term, it makes sense to use CDI instead of proprietary annotations. To start this process, the JAX-RS EG should identify which annotations can be replaced by CDI, and mark them as deprecated.

[glassfishrobot]

• Issue Imported From: https://github.com/jax-rs/api/issues/569
• Original Issue Raised By:@mkarg
• Original Issue Assigned To: Unassigned

[andymc12]

@mkarg, per our discussion on PR #608, I think it might be good to change this issue's milestone to 2.2. The idea is to deprecate the some of the JAX-RS-specific annotations and replace them with optional CDI annotations. That would allow removal of the JAX-RS-specific annotations in the 3.0 release.

So in the 2.2 release the JAX-RS spec would support both @Context and @Inject - CDI is optional in the sense that users would not need to use CDI at all, but vendors would be required to support CDI if the vendor also makes CDI available. The JAX-RS 2.1 spec does something similar regarding JSON-B providers (vendors are required to supply a JSON-B provider if they already supply a JSON-B implementation). This allows JAX-RS-only vendors (like Jersey, CXF, RestEasy, etc.) to continue operating like 2.1 (no need to recognize CDI annotations), but full Java EE / Jakarta EE vendors (like Liberty, WildFly, etc.) would need to support both injection mechanisms.

[mkarg]

@andymc12 I tend to +1 for this but I am not fully conviced, because @spericas wrote that @Context and @Inject are not fully compatible. We should give it a deeper thought before pulling this into 2.2. Particularly I would be more happy if all vendors would give a short statement if they really want to do this in short term, because I do not want to delay 2.2 just for the sake of some troubles with CDI discovered too late.

[arjantijms]

@andymc12

I think it might be good to change this issue's milestone to 2.2. The idea is to deprecate the some of the JAX-RS-specific annotations and replace them with optional CDI annotations. That would allow removal of the JAX-RS-specific annotations in the 3.0 release.

+1

A CDI first approach for all EE specs (thus including JAX-RS) is favoured by Payara.

[chkal]

I'm also +1 for this. But I agree with @mkarg, that the incompatibilities between @Context and @Inject mentioned by @spericas should be discussed in detail.

[ggam]

+1. This was my goal for JAX-RS 2.1 but unfortunately the concern was raised too late to fit the schedule.

At that time I emailed Antoine Sabot Durand to ask some questions regarding this CDI integration and he seemed to be interested in helping. I think we could count with him now again.

El jue., 31 may. 2018 16:31, Andy McCright [email protected] escribió:

@mkarg https://github.com/mkarg, per our discussion on PR #608 https://github.com/eclipse-ee4j/jaxrs-api/pull/608, I think it might be good to change this issue's milestone to 2.2. The idea is to deprecate the some of the JAX-RS-specific annotations and replace them with optional CDI annotations. That would allow removal of the JAX-RS-specific annotations in the 3.0 release.

So in the 2.2 release the JAX-RS spec would support both @Context and @Inject - CDI is optional in the sense that users would not need to use CDI at all, but vendors would be required to support CDI if the vendor also makes CDI available. The JAX-RS 2.1 spec does something similar regarding JSON-B providers (vendors are required to supply a JSON-B provider if they already supply a JSON-B implementation). This allows JAX-RS-only vendors (like Jersey, CXF, RestEasy, etc.) to continue operating like 2.1 (no need to recognize CDI annotations), but full Java EE / Jakarta EE vendors (like Liberty, WildFly, etc.) would need to support both injection mechanisms.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/eclipse-ee4j/jaxrs-api/issues/569#issuecomment-393549787, or mute the thread https://github.com/notifications/unsubscribe-auth/ACAucEMDA039wSPsTxUBwSiZNn88l_FIks5t3_7BgaJpZM4UVJ2E .

[arjantijms]

Hi,

On Thu, May 31, 2018 at 7:47 PM, Christian Kaltepoth < [email protected]> wrote:

I'm also +1 for this. But I agree with @mkarg https://github.com/mkarg, that the incompatibilities between @Context and @Inject mentioned by @spericas https://github.com/spericas should be discussed in detail.

It would be good to discuss indeed what @Context can do (if anything) that CDI can't.

In JSF we're busy with a similar switchover. We introduced CDI versions for existing JSF DI/Managed Bean constructs, and right away deprecated these JSF Managed Beans. In JSF 2.3 one can technically still use the native JSF Managed Beans and DI, but users are strongly encouraged to use the CDI based ones. I intend to remove the native managed beans system fully in JSF 2.3 (if, of course, enough of the other committers agree).

So for JAX-RS, let's see how we can practically best approach this.

Kind regards, Arjan Tijms

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eclipse-ee4j/jaxrs-api/issues/569#issuecomment-393616581, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5XTjfBmF3dM2y8GeM_mWsVu7xO4tYgks5t4Cy4gaJpZM4UVJ2E .

[jansupol]

Jersey made some experiments in making CDI the primary injection framework, but it was not 100% successful. I do not know the details right now, but Jersey allows for registering additional injectables in runtime, and we had an issue to tell CDI while the application already runs: look cdi, here is another class that from now on you start to inject. So this dynamically added injectables are injected via @Context and hk2 and @Inject is handled (more or less) by CDI. Replacing @Context with @Inject seems like large refactoring that we prefer to do in 3.0

[arjantijms]

Hi,

I'm indeed well aware of Jersey using HK2 internally and the step it took to experiment with CDI there (at Payara I also come into contact with that on almost a daily basis). I've even worked directly with the code I think you're referring to:

https://github.com/payara/patched-src-jersey-old/blob/jersey-2.26.payara-maintenance/ext/cdi/jersey-cdi1x/src/main/java/org/glassfish/jersey/ext/cdi1x/internal/ProcessAllAnnotatedTypes.java

and

https://github.com/payara/patched-src-jersey-old/blob/jersey-2.26.payara-maintenance/ext/cdi/jersey-cdi1x/src/main/java/org/glassfish/jersey/ext/cdi1x/internal/ProcessJAXRSAnnotatedTypes.java#L56

Which is related to org.glassfish.jersey.ext.cdi1x.spi.Hk2CustomBoundTypesProvider being present or not.

But these may be orthogonal concerns, in theory at least. At first it's perhaps not so much about replacing the implementation of @Context, but just making sure that all artefacts that currently can be injected using @Context can be injected using @Inject instead(in so far not already possible), and/or there being CDI alternatives.

For instance, in JSF we opted to add a small number of new names for existing native managed bean features, instead of letting CDI take over these in some way.

E.g.

See https://github.com/eclipse-ee4j/mojarra/blob/master/impl/src/main/java/javax/faces/annotation/ManagedProperty.java

which is the CDI replacement for;

https://github.com/eclipse-ee4j/mojarra/blob/master/impl/src/main/java/javax/faces/bean/ManagedProperty.java

Kind regards, Arjan Tijms

On Fri, Jun 1, 2018 at 11:44 AM, jansupol [email protected] wrote:

Jersey made some experiments in making CDI the primary injection framework, but it was not 100% successful. I do not know the details right now, but Jersey allows for registering additional injectables in runtime, and we had an issue to tell CDI while the application already runs: look cdi, here is another class that from now on you start to inject. So this dynamically added injectables are injected via @context https://github.com/context and hk2 and @Inject https://github.com/Inject is handled (more or less) by CDI. Replacing @context https://github.com/context with @Inject https://github.com/Inject seems like large refactoring that we prefer to do in 3.0

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eclipse-ee4j/jaxrs-api/issues/569#issuecomment-393830305, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5XTnKa41uVfHIjvkt5CskG8TdU_iH0ks5t4Q0DgaJpZM4UVJ2E .

[spericas]

As I stated in the PR, @Inject and @Context have different targets. @Inject cannot be used for params and @Context cannot be used in constructors. A very popular use case for @Context is indeed in param position.

The majority of JAX-RS resource methods use @Context in param position, except for the entity. This would need to be converted to an @Inject at method level, but would make things less clear regarding the entity param. There's also the issue of ContextResolver's which are somewhat similar to CDI producers and would need tweaking.

Again, the idea of replacing JAX-RS's DI framework with CDI is very sound, but I still think this is an all-or-nothing exercise that will necessarily break backward compatibility, and as such should be out of scope for 2.2.

[chkal]

@spericas To be honest, I'm surprised to hear that using @Context on parameters is a popular choice. I actually never saw this in any JAX-RS code. Of course @*Param usually is used on parameters. But if you want to get UriInfo and friends, using fields usually makes more sense. Or am I missing something here?

[spericas]

@chkal You're right, in thinking about using @Inject at method level (as it cannot be used at param level), I was thinking about @Context and @*Param together. Clearly, @*Param is far more popular at that level. Having said that, @Context isn't that uncommon in param position.

If it is just about field injection, then it shouldn't be that different. Differences in annotations would be in constructors (@Context again here), resource methods and perhaps even in properties.

[ggam]

I understand the all-or-nothing approach but we have to define what's that "all". My personal opinion: since CDI doesn't support parameter injection, people using CDI won't really miss it on JAX-RS. Moreover, they could even wonder why they can inject dependencies on JAX-RS and nowhere else.

Developers are currently using @Context at parameter level, and it will still work (for now). But it's possible that supporting it on CDI is not worth the effort, which I don't say it's not, but should be discussed anyway.

Just my 2 cents.

[arjantijms]

Hi,

Providing values for @*Param annotated parameters is a JAX-RS functionality, that would keep working regardless of what the bean model and DI system is.

As an @Context replacement for method parameters there may be a couple of solutions:

1. For CDI based endpoints, just don't support injection of arbitrary beans
2. Any method parameter not having a native JAX-RS @*Param annotation is taken to be injected via CDI. Such parameter may optionally be qualified.

@RequestScoped @Produces(TEXT_PLAIN) public class ProtectedResource {

@Inject
private SecurityContext securityContext;

@GET
@Path("sayHi")
public String sayHi(@NotNull @FormParam("name") String name, Foo foo,

@Red Bar bar) { return "saying hi " + "name"; } }

In the example above "name" is provided by the JAX-RS implementation.

---> param0Value = getInternally();

Where for "foo" the JAX-RS implementation would consult CDI:

---> param1Value = CDI.current().select(param1.getClass());

And for "bar" the JAX-RS implementation would consult with CDI as well, but passing in the qualifier:

---> param2Value = CDI.current().select(param2.getClass(), getQualifiers(param2));

Kind regards, Arjan Tijms

On Fri, Jun 1, 2018 at 5:41 PM, Santiago Pericas-Geertsen < [email protected]> wrote:

@chkal https://github.com/chkal You're right, in thinking about using @Inject https://github.com/Inject at method level (as it cannot be used at param level), I was thinking about @context https://github.com/context and @*Param together. Clearly, @*Param is far more popular at that level. Having said that, @context https://github.com/context isn't that uncommon in param position.

If it is just about field injection, then it shouldn't be that different. Differences in annotations would be in constructors (@context https://github.com/context again here), resource methods and perhaps even in properties.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eclipse-ee4j/jaxrs-api/issues/569#issuecomment-393920254, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5XTrvdmF0SUe8JsPqf8BzgOjjaL6MNks5t4WC0gaJpZM4UVJ2E .

[spericas]

Seems that we may have digressed into 3.0 discussions (which is great, but likely for another issue). This issue is about deprecating annotations in 2.2 and potentially providing a transition path to 3.0, and maybe allowing CDI annotations in certain locations in 2.2. Needless to say, JAX-RS 2.2 is intended to be backward compatible.

I have argued that the differences are large enough that people would need to change their applications quite significantly when moving to 3.0 that I don't see the point of more intermediary steps and new rules for developers to learn during the transition.

I'd much rather start the discussion on 3.0, but let's close the loop on this issue first. So, should we do anything related to DI and CDI in 2.2?

[arjantijms]

Needless to say, JAX-RS 2.2 is intended to be backward compatible.

I hear you, though I think the proposed changes till so far do ensure that.

Deprecating @Context is just that, deprecation.

While public String sayHi(@NotNull @FormParam("name") String name, Foo foo is backwards compatible, unless I'm missing something. As far as I know this syntax wasn't allowed in 2.1, so adding it to 2.2 constitutes a new feature. It's not an intermediate step as this would be in 3.0 as well.

Comparing to JSF again, between between 2.2 and 2.3 we made a couple of larger additions to support CDI than what is currently proposed here, so my gut feeling is this all could be for JAX-RS 2.2.

[arjantijms]

p.s. one question is where the work of saying that public String sayHi(@NotNull @FormParam("name") String name, Foo foo) is supported can be done.

I can do a PR for Jersey to support this, but where do we document it? There is no spec project yet for JAX-RS.

[spericas]

@arjantijms I understand that deprecation does not change semantics. However, it does result in annoying warning messages which would be tolerable if the version after 2.2 would only require replacing @Context by @Inject. If such a version is 3.0, which is likely, that would hardly be the only requirement.

As for the sayHi() method. If it was an @PUT/@POST and you didn't use @Context, how would distinguish the entity among all the injected bean params?

[arjantijms]

I understand that deprecation does not change semantics. However, it does result in annoying warning messages which would be tolerable if the version after 2.2 would only require replacing @context by @Inject.

I'm not sure if it should be the version after 2.2. If there's an actual @Deprecated annotation, there should be a replacement right away.

I might still be missing something, but it may not be that difficult.

It concerns the following artefacts / types that are now injectable via @Context:

1. SecurityContext
2. Request
3. Application,
4. Configuration
5. Providers
6. ResourceContext
7. ServletConfig
8. ServletContext
9. HttpServletRequest
10. HttpServletResponse
11. HttpHeaders
12. UriInfo

HttpServletRequest and ServletContext are already injectable via @Inject (built-in CDI beans). To make the others injectable should only be a matter of shipping a CDI extension that produces these, although JAX-RS should perhaps not define producers for the Servlet types.

As for the sayHi() method. If it was an @PUT/@post and you didn't use @context, how would distinguish the entity among all the injected bean params?

You're right, that was the part I'm missing there. JAX-RS already has a non-annotated parameter for the incoming entity. I typically used @FormParam or @BeanParam, but you're referring to something like this right?

@Consumes(APPLICATION_JSON)
public void method(MyEntity entity) {
    ...
}

With MyEntity

public Class MyEntity {
    int age;
    String name;
}

There's still a couple of possible solutions I can think of it. Perhaps others can think of some additional options.

1. Qualify the entity with a JAX-RS specific annotation
2. Qualify the injected bean parameters
3. Adopt a convention; first non annotated parameter is the JAX-RS entity, next ones are bean parameters.

For example:

1 entity and 1 bean parameter:

@Consumes(APPLICATION_JSON)
public void method(@EntityParam MyEntity entity, Foo foo) {
    ...
}

1 entity with annotation

@Consumes(APPLICATION_JSON)
public void method(@EntityParam MyEntity entity) {
    ...
}

1 entity without annotation (for backwards compatibility)

@Consumes(APPLICATION_JSON)
public void method(MyEntity entity) {
    ...
}

[spericas]

@arjantijms Yes, there are indeed solutions, but those that are backward compatible aren't very elegant IMO (not just because of additional conventions, but also for the use of ContextResolver's as "producers" for non-Context injection points) and the other more elegant ones are for a 3.0 discussion ;)

[arjantijms]

Yes, there are indeed solutions, but those that are backward compatible aren't very elegant IMO

Which ones do you exactly mean? (several where mention above ;))

@Inject ResourceContext resourceContext

That's probably as elegant and as backwards compatible as you can get, I guess. It's something that should be easy to implement, easy to specify, and won't need changing in a 3.0.

The other one:

@Consumes(APPLICATION_JSON)
public void method(@EntityParam MyEntity entity, Foo foo) {
    ...
}

Is that the elegant one for the 3.0 discussion, or the not-very elegantly one?

[chkal]

@arjantijms

HttpServletRequest and ServletContext are already injectable via @Inject (built-in CDI beans). To make the others injectable should only be a matter of shipping a CDI extension that produces these, although JAX-RS should perhaps not define producers for the Servlet types.

I don't think it will be that easy. Actually the HttpServletRequest injectable via CDI has one major issue. It only provides the original HttpServletRequest without any wrappers applied via Servlet filters, which can lead to surprising results (see CDI-709).

And I can imagine that JAX-RS implementations also use wrappers which are then injected via @Context into the corresponding resource. At least CXF is doing this. So maybe JAX-RS implementations must produce the Servlet objects them self?

[arjantijms]

I don't think it will be that easy. Actually the HttpServletRequest injectable via CDI has one major issue. It only provides the original HttpServletRequest without any wrappers applied via Servlet filters, which can lead to surprising results (see CDI-709).

You're right, I mentioned that issue a couple of times before. In theory it's even a bit worse as you could even get a random HttpServletRequest even (the CDI spec doesn't say anything about which version you should get).

And I can imagine that JAX-RS implementations also use wrappers which are then injected via @Context into the corresponding resource. At least CXF is doing this.

Interesting. If that happens after the JAX-RS Servlet is called then the Servlet implementation wouldn't know about them.

The simplest way out for now may be to only provide producers for the JAX-RS owned types, then keep using @Context for the other ones.

Another option may be to use qualifiers, so it's clear code is asking for the JAX-RS version of these Servlet owned types.

It's also possible to create an alternative producer, which can return the originally produced HttpServletRequest when not in a JAX-RS context, and otherwise the version that's potentially wrapped by JAX-RS.

Ultimately the Servlet owned types should be produced by the Servlet spec, but unfortunately Greg from Jetty is always blocking that, but that's just because of not understanding what's being asked. See https://issues.jboss.org/browse/CDI-492

[chkal]

Interesting. If that happens after the JAX-RS Servlet is called then the Servlet implementation wouldn't know about them.

Correct. I don't remember all the details anymore, but if you use @Context to get the HttpServletRequest from CXF, you will get a ThreadLocalHttpServletRequest. And IIRC this one is even wrapping another CXF request wrapper.

The simplest way out for now may be to only provide producers for the JAX-RS owned types, then keep using @context for the other ones.

I don't like this idea. Actually we want to get rid of @Context in the long term, correct?

Another option may be to use qualifiers, so it's clear code is asking for the JAX-RS version of these Servlet owned types.

This was also my first idea. But it may be confusing for developers that there are multiple "versions" of the Servlet objects available for injection.

Ultimately the Servlet owned types should be produced by the Servlet spec, but unfortunately Greg from Jetty is always blocking that

Yeah, unfortunately it looks like there won't be much progress on this issue in the near future.

The only way to work around these issues could be to provide some JAX-RS owned class that provides access to the Servlet objects. Something like:

@Path("foo")
public class MyResource {

  @Inject
  private ServletEnvironment servletEnv;

  @GET
  public String get() {
    HttpServletRequest request = servletEnv.getRequest();
    HttpServletResponse response = servletEnv.getResponse();
    // ...
  }

}

Maybe this approach could also be used to make it more explicit that JAX-RS actually doesn't require an Servlet environment? So there could be something like the ExternalContext in JSF. Just thinking out loud. Not sure I really like this idea.

[arjantijms]

I don't like this idea. Actually we want to get rid of @Context in the long term, correct?

I'm not particularly fond about that idea either, but just wanted to mention it for completeness. We absolutely should get rid of @Context. It's one of the things that's very confusing, specifically to people who are new to Java EE and JAX-RS. Sometimes needing @Context to inject things, and sometimes needing @Inject is hard to explain.

This was also my first idea. But it may be confusing for developers that there are multiple "versions" of the Servlet objects available for injection.

It can be, although it does represent reality a little. At some point we could potentially have @Inject @Original HttpServletRequest if you wanted the original, and @Inject @Current HttpServletRequest for well, the current one. But "current" (last known one by Servlet) vs "jaxrs" might not be intuitive.

Yeah, unfortunately it looks like there won't be much progress on this issue in the near future.

Perhaps not. We'll have to see how things move forward when the Servlet group starts again though. Maybe splitting up the Servlet spec (as proposed on the EE4J list before) is an option.

The only way to work around these issues could be to provide some JAX-RS owned class that provides access to the Servlet objects.

Hmmm, might not be a bad idea indeed. Although not exactly the same, EE Security has something similar:

https://javaee.github.io/javaee-spec/javadocs/javax/security/enterprise/authentication/mechanism/http/HttpMessageContext.html

[ggam]

I'm not particularly fond about that idea either, but just wanted to mention it for completeness. We absolutely should get rid of @context. It's one of the things that's very confusing, specifically to people who are new to Java EE and JAX-RS. Sometimes needing @context to inject things, and sometimes needing @Inject is hard to explain.

Totally agree. And then, having parameter injection support ONLY on JAX-RS and nowhere else isn't a bit confusing too? ;)

It can be, although it does represent reality a little. At some point we could potentially have @Inject @Original HttpServletRequest if you wanted the original, and @Inject @Current HttpServletRequest for well, the current one. But "current" (last known one by Servlet) vs "jaxrs" might not be intuitive.

Is is intended by the Servlet spec that you have a way to access the original request? Using the original request can be problematic as the request might have even been forwarded, which changes some attributes.

For me, the injected request should be the one that the dispatched (JSF, JAX-RS, etc) Servlet got. Getting the original request doesn't feel safe to me and is not something we should encourage. People who need the original request can easily register a filter as the first one on web.xml, store it on a request attribute and read it from a @Producer afterwards.

[chkal]

Is is intended by the Servlet spec that you have a way to access the original request? Using the original request can be problematic as the request might have even been forwarded, which changes some attributes.

For me, the injected request should be the one that the dispatched (JSF, JAX-RS, etc) Servlet got. Getting the original request doesn't feel safe to me and is not something we should encourage. People who need the original request can easily register a filter as the first one on web.xml, store it on a request attribute and read it from a @Producer afterwards.

I absolutely agree on this one. The only problem is what @arjantijms already mentioned: You could also inject the Servlet objects (HttpServletRequest and friends) into a filter or into a bean which is used as part of the filter processing. So at this point the request didn't even reach the Servlet yet. And in each filter the "current" request/response could be a completely different one, because every filter could wrap it again.

[ggam]

I absolutely agree on this one. The only problem is what @arjantijms already mentioned: You could also inject the Servlet objects (HttpServletRequest and friends) into a filter or into a bean which is used as part of the filter processing. So at this point the request didn't even reach the Servlet yet. And in each filter the "current" request/response could be a completely different one, because every filter could wrap it again.

True. But even in that case, there's already a "current request", which incidentally won't be request scoped, but @Dependent. This might seem strange, but is similar to the problem with the authenticated caller and JASPIC, which Arjan also discovered on MicroProfile: the request can be initially unauthenticated, authenticated at some point and unauthenticated again before the request ends. Which is the caller principal? It depends on when you ask for it.

I can think of two solutions for that:

• @Inject HttpServletRequest injects the non-proxied current request, wherever you are asking for it. @Proxied @Inject HttpServletRequest injects a proxy which will always delegate to the real request.
• @Inject HttpServletRequest injects the proxied/delegator view. @Inject Supplier<HttpServletRequest> provides you the current request without wrappers.

Either way provides users enough power to create their own producers to support more sophisticated configurations.

[mkarg]

@chkal As you more actively contribute in this thread as I currently do, I would like you to co-assign this issue to you. Agreed?

[chkal]

@ggam Thanks for the summary. I would actually prefer the second option. IMO injecting a proxied view should be the default. Not sure if we really need the Supplier<...> version to get an request without wrapper though. However, I guess all this should go into the Servlet or CDI spec. I don't think that JAX-RS should provide these mechanisms. So perhaps creating some JAX-RS owned class which is directly injectable like I proposed above would be a good option for JAX-RS?

@mkarg Sure, I just so-assigned it to me.