Introduce CDI requirements for deprecated @Context injection

[jamezp]

This PR introduces the Jakarta Dependency and Context Injection requirements as a replacement for @Context injection. Note that most of this was taken from the original JakartaRest40.pdf. There are a few things I did differently however.

One is I changed the entity parameter annotation from @Entity to @EntityParam. One reason is there is already a jakarta.ws.rs.client.Entity type and that seemed confusing to me. The other is Jakarta Persistence already has a @Entity parameter. Using @EntityParam seemed the most natural with other parameter annotations. However, I'm open to suggestions like @MessageBody or anything else.

The second is I do think we should required the @Inject annotation on methods we want to use CDI injection on. The reason is implementations will need a way to know if we should be using CDI injection or the old way of parameter injection. For example consider a method like:

@GET
@Path("example")
public Response performGet(final MyCdiBean bean) {
    return Response.ok(bean.get()).build();
}

We have no way to know if MyCdiBean is an entity or it should be handled by the CDI container.

I'm opening this as a draft for now for further discussions. There are more tests that are needed for the @EntityParam and we may need to copy some other tests to use CDI instead of @Context injection. I didn't want to spend too much time on that until we agreed on the rest of the PR though.

This PR relates to:

• Deprecating JAX-RS-specific annotations in favor of CDI #569
• Deprecate @Context injection and @Suspended  #1209
• Add CDI annotations to the annotations that need them. Updated the si… #1221

We need to consider what needs to be done for #1214 and #1215 as well. I did not add any specification documentation about those as I wasn't sure where we wanted to go.

Note I added a OpenRewrite recipe here for adding the @EntityParam annotation. However, this might be the wrong place for this. I'd initially added it here to do some testing. It needs some updating though as I changed the requirements a bit. I've left it in the draft for now, but we can remove it before we merge.

[FroMage]

I would like to suggest that the choice of calling this an entity in the first place was not necessarily helpful.

It was called an entity or message body in the first HTTP 1.1 RFC but a resource in REST and in the more recent HTTP RFC it is called a resource, a representation data or a payload or message payload or message body.

A lot of other frameworks, libraries or programming languages also call it a request body (as opposed to a response version).

I think we should consider a better name than entity for this annotation, and not only because it clashes with JPA's @Entity annotation.

Do we expect this annotation to be used to designate the response entity too?

[jamezp]

I would like to suggest that the choice of calling this an entity in the first place was not necessarily helpful.

It was called an entity or message body in the first HTTP 1.1 RFC but a resource in REST and in the more recent HTTP RFC it is called a resource, a representation data or a payload or message payload or message body.

A lot of other frameworks, libraries or programming languages also call it a request body (as opposed to a response version).

Yeah, @RequestBody was another one I thought of. I only went with @EntityParam because it kind of "fit" with the other @*Param annotations. However, this is different and I honestly don't love the name. I just wanted to push something up to start the conversion so thank you :)

I think we should consider a better name than entity for this annotation, and not only because it clashes with JPA's @Entity annotation.

Do we expect this annotation to be used to designate the response entity too?

No, just on a request which is why @RequestyBody, @MessageBody, something like that makes more sense to me personally.

[FroMage]

I think, but this may be just me, I don't have data to back this up, but I think the entity term is confusing for new users. In a way that request body is not: I feel that most people understand what this is. I have some doubts about message body because I suspect fewer people associate HTTP with messages.

[FroMage]

But on the other hand, we already have Entity in several type names in the spec 😥

[jamezp]

Agreed for sure. IMO @Entity so ambiguous and somewhat confusing. I think my personal preference would be @RequestBody because that is exactly what it is.

[mkarg]

My personal preference would be @Entity because it is the word found in all the RFCs, so people dealing with specs will search for exactly that. It is not our job to criticize the IETF. It is our job to introduce CDI, solely.

[WhiteCat22]

I am leaning towards @Entity, although I hate the potential collision with JPA.

My reasoning is that we already have jakarta.ws.rs.client.Entity in the Client and it might be confusing to users if the client and server don't match.

.post(Entity.entity(message, MediaType.TEXT_PLAIN));

...

public Response updateFoo(@Entity Foo foo) {
...
}

If we didn't already use Entity in the client I'd be more inclined towards @RequestBody with a "definition of terms" section in the REST spec to map @RequestBody to HTTP's Entity. However, since Entity is already well established in the REST spec I think we should continue to use it for consistency.

[jamezp]

Another option is to simply not allow CDI for resource method parameters. Then we essentially have what we have now which is no annotation on the entity. The other types that require @Context can just use inference to be resolved. How the @*Param parameters are resolved becomes an implementation detail.

[FroMage]

My personal preference would be @Entity because it is the word found in all the RFCs, so people dealing with specs will search for exactly that. It is not our job to criticize the IETF. It is our job to introduce CDI, solely.

As I said in my comment, the latest RFC removed entity entirely, it's not called that anymore: https://datatracker.ietf.org/doc/html/rfc9110

For some reason, they did keep entity tag but that's probably an oversight.

I am not criticising IETF, I'm pointing out that nobody ever called these entities outside of Jakarta REST, and the latest RFC appears to agree with this observation.

[jamezp]

It's seems like RFC9110 uses "content" so we could use @Content https://www.rfc-editor.org/rfc/rfc9110.html#name-content

HTTP messages often transfer a complete or partial representation as the message "content": a stream of octets sent after the header section, as delineated by the message framing.

If we really want to stick with something around entity I'd prefer @EntityParam to not be confused with or require a FQCN reference, the jakarta.ws.rs.client.Entity.

[FroMage]

It's seems like RFC9110 uses "content" so we could use @Content https://www.rfc-editor.org/rfc/rfc9110.html#name-content

Huh, I did not notice content when I listed all the other terms it uses. That one makes a lot of sense, given content types, content negociation…

Probably not as widespread as request body for a term, but definitely a lot more obvious to anyone than entity.

[mkarg]

It's seems like RFC9110 uses "content" so we could use @Content https://www.rfc-editor.org/rfc/rfc9110.html#name-content

HTTP messages often transfer a complete or partial representation as the message "content": a stream of octets sent after the header section, as delineated by the message framing.

If we really want to stick with something around entity I'd prefer @EntityParam to not be confused with or require a FQCN reference, the jakarta.ws.rs.client.Entity.

@EntityParam seems to be straightforward, as it builds on the "Entity" word used in other parts of Jakarta REST, and extends the Param series of injection markers. +1 from me.

[mkarg]

It's seems like RFC9110 uses "content" so we could use @Content https://www.rfc-editor.org/rfc/rfc9110.html#name-content

Huh, I did not notice content when I listed all the other terms it uses. That one makes a lot of sense, given content types, content negociation…

Probably not as widespread as request body for a term, but definitely a lot more obvious to anyone than entity.

IMHO @Content is far too easy to confuse with @Context.

[jamezp]

I did a short talk about these changes today in this PR on the Jakarta EE Futures call. @jhanders34 had a, very good IMO, idea to not allow arbitrary CDI beans to be injected into resource methods which means we would not need the @EntityParam annotation for 5.0. Then in 6.0 when we remove @Context, we can add the @EntityParam annotation and then allow CDI bean injection into resource methods.

Without this approach we could break compatibility and require users to add @EntityParam to request body content parameters.

[mkarg]

@jamezp I need to confess I somewhat got lost in the discussion. Could you please elaborate how that distinction in 5.0 and 6.0 changes anything? I am no CDI expect, so please bear with me. What problem arises from our current plan and why is that "very good IMO" idea the soltuion? 🤔

[jamezp]

@mkarg That is a fair request and would help others too :) I'll work on something more detailed next week and work on updating this PR to reflect the plan.

REST 5.0

Add CDI full CDI support for resources, providers and applications for field injection and constructor injection. For resource methods, CDI will only replace the usage of @Context injection. Meaning you cannot inject an arbitrary CDI bean into a resource method.

Deprecate @Context, @Suspended annotations and the ContextResolver interface. We may want to deprecate the Providers.getContextResolver() too since we're deprecating the ContextResolver. However, maybe we don't need to deprecate the latter two. I'm open to either option.

Full backward compatibility, meaning @Context injection will still work as will the other API's. There is no need to add the @EntityParam annotation as the request body parameter will still be the "unknown" parameter.

REST 6.0

Remove the deprecated API's and add the @EntityParam (or whatever we decide to call it) and require it on the message body parameter. This will allow us to support full CDI injection for resource methods.

[mkarg]

Thank you, James. As in the end the result will be the same (everbody needs to overhaul his existing applications and put @EntityParam on each single resource method), I do not see the actual benefit of this two-step way. Developers will get confused by a long and ongoing change path. Why not simply doing that cut in 5.0 directly?

[mkarg]

Maybe we should add since 5.0 to the JavaDocs also, so people have a better understanding which version supports what technology?

[jamezp]

We have the version in the @Deprecated annotation. We could add it here too I guess.

[mkarg]

As CDI is an abstract idea but not a concrete product, shouldn't it be "...by CDI prio..." (without "the")?

[mkarg]

Maybe we need another cross reference mentioned, to make clear that these interfaces are injectable?

[mkarg]

Actually I wonder if providers can only be application scoped, or if it was valid to make them request scoped before?

[jamezp]

I don't see why they couldn't be request scoped. I probably wouldn't suggest it, but I don't see why it would be an issue :)

[mkarg]

👍

[mkarg]

"When"? Isn't that mandatory now?

[jamezp]

We haven't defined what needs to happen with SeBoostrap which is why I added this.

[mkarg]

The method MUST not be annotated with @Inject

Why should someone do that? Do we really have to explicitly say that?

[jamezp]

The part of requiring @Inject for resource methods has been removed and we definitely shouldn't do that. My initial thought when I added it was that it would be easy for implementations to know if it was a CDI resource method or a legacy one.

[mkarg]

Is there a technical reason why the entity should not be a CDI bean?

[jamezp]

I don't really see how it could be. You need the request to create the entity. If it was a CDI bean it would mean it could be injected into anything which doesn't have to be a Jakarta REST endpoint.

[mkarg]

I do not understand this separation. Again, I am not a CDI expert, so bear with me. IMHO @EntityParam should be provided by a CDI producer. The runtime should be that CDI producer. To produce the instance, the runtime invokes MBRs. Wouldn't that be straightforward? 🤔

[jamezp]

I thought about this more and I don't think it can be. We need the request to create the entity with a reader. CDI doesn't really need a request, at least not an HTTP one. We can't make it a CDI bean and assume it can just be injected let's say into a scheduled EJB.

[mkarg]

I do not see why we need to deprecate "ContextResolver". Couldn't we instead say that runtime MUST provide CDI producers for these classes, and MUST internally invoke the original context resolver in turn?

[jamezp]

I've been wondering about this one as well. The ContextResolver likely doesn't need to be deprecated, so maybe we just don't deprecate which seems fine to me.

[jamezp]

Thinking more about this, I'm not sure what advantage there is to allowing CDI injection into parameters. CDI beans can be injected at the instance level or in the constructor. I don't really see why we'd need them injected in resource methods.

If we don't allow CDI injection into resource methods, we can keep backwards compatibility much easier.

[jamezp]

What I've done is gone with the approach of removing the @EntityParam annotation. Arbitrary CDI are not allowed to be injected in resource methods. You can inject them via instance fields or constructor parameters though.

The same rules apply for determining the entity in a resource method. If the parameter is not annotated with @*Param, is not a known required context type, e.g. UriInfo, HttpHeaders, etc., or is of type AsyncResponse, then it is assumed to be the entity parameter.

I've removed the deprecation of the ContextResolver as a CDI producer is not really a direct replacement.

I also removed the OpenRewrite recipe and will publish it elsewhere.