rest
rest copied to clipboard
Drop support for @Context injection and related artifacts
Note that this issue is not about deprecation, but about dropping support, and it is targeted for the next major release. We should review other artifacts related to @Context
that need removal.
What is the intention for something like:
@GET
@Path("eventStream")
@Produces(MediaType.SERVER_SENT_EVENTS)
public void eventStream(@Context SseEventSink eventSink,
@Context Sse sse) {
executor.execute(() -> {
try (SseEventSink sink = eventSink) {
eventSink.send(sse.newEvent("event1"));
eventSink.send(sse.newEvent("event2"));
eventSink.send(sse.newEvent("event3"));
}
});
}
Injecting the SseEventSink
as a parameter is used in all the TCK tests and examples. I've personally only ever seen SseEventSink
injected as a method parameter.
It would likely require large refactoring for users to remove parameter injection of @Context
. Not that I'm opposed to using CDI and deprecating @Context
and ContextResolver
. I just worry about the removal for the migration issue.
P.S. I wasn't sure if I should have commented this here or on #569, but I opted for here only because it's newer :)
@jamezp It would be injectable as a normal CDI bean without the need to use @Context
. Any CDI bean shall be injectable in param position on a resource method, in fact.
@spericas Kind of what I assumed, but just wanted to verify thank you. I'm working on this for RESTEasy which seems like for full support is going to take some refactoring. However, that is not a spec issue :)
Can you confirm you intend to break existing applications at two levels:
- Force users to move from context to inject
- Prevent users to distinguish between CDI and JAXRS beans without a custom qualifier (since currently context is considered as a classifier in several/most applications so you can have
@Inject Foo foo;
and@Inject @Context Foo foo
- with implicit or not@Inject
) - Not be able to distinguish anymore what is a context from a payload in POST/PUT/... methods - guess you intend to fail somehow?
On my side I don't understand the gain to break something which works so I would request to maybe not do it to avoid people to have to rewrite their apps.
On my side I don't understand the gain
The gain is to go to a single component model, and not have 6 different ones, which all have their quirks in interaction and all have to be maintained separately. Faces moved to CDI in this release, and there's a long standing wish toi phase out EJB beans in favour of CDI compatible alternatives.
@arjantijms I get this point but can't you get it without any breaking change defining @Context
as a qualifier? Will keep existing code working and unify the model anyway. So proposal is to:
- deprecate
@Context
injection wtihout@Inject
for fields/methods, - define
@Context
as being a qualifier when ran in a CDI container (SE or EE) - keep parameter injections as it is
This makes everyone happy (no code to rewrite, CDI fully embraced - actually it is what most JAXRS containers already do in CDI integrations).
Redefining Context
as qualifier might be an option to consider.
We did do this once with jakarta.faces.convert.FacesConverter
, which was made retroactively a qualifier. It wasn't without its own issues though, although the mistake we made there was perhaps not switching over completely (it supports converters being CDI and old-style at the same time).
A side note is that CDI embracement is also about not keeping SeBootstrap
unreleased class since defining CDI integration in JAXRS (scanning etc) solves wat SeBootstrap tries to do in a clumsy way since it requires users to either be fully explicit or reimplement the scanning + it does not promote what all implementations had been doing (cause it fits user requests) plus it has some other pitfalls like not being well integrated with the container since it is not linked to servlet container (only http layer in EE) to control the bindings. SE API already failed in other specs (thinking strongly to JAXWS Endpoint
) so not sure current API SNAPSHOT is great so I think this issue should be part of an epic "CDI integration" which would mean:
- Ensure it is integrated with CDI at API level - previous proposal,
- Ensure it is integrated with CDI at scanning level (mainly define the CDI extension behavior to find providers, endpoints, features, application, ...) and start the server. Here it should work well and transparently with CDI SE API as well as CDI EE API when in an EE container (which would just get more scanning rules to stay backward compat but the minimum should work in both env making code way more reusable).
- Ensure beans can be CDI instances and not just injectable instances (it is not well defined currently IIRC), including the scope delegation (whatever scope should work, not only requetscoped or singleton/appscoped)
- (optional) define that an event is fired when the server starts/stops (to know endpoints are available or not)
Guess it would be the main integrations which would enable to run JAXRS in all env without learning a new programming model. Not sure how epics are managed for this project but feel free to promote as such that if it makes sense for you.
Redefining @Context
as a qualifier isn't going to make the new Jakarta REST compatible. There are other significant differences (resource method parameters, context resolvers, entity params, etc.) that have been proposed and discussed. Moreover, making something that looks syntactically compatible but is semantically different is even worse (e.g, @Context
can not be used in parameter position).
I understand that dropping backward compatibility has some cost, but Jakarta REST has been compatible for a decade and some of the initial decisions would have been different had CDI been matured at the time. Implementations that support the existing API will remain available for a long time for those that don't want to migrate.
@rmannibucau On the topic of scanning, absolutely, CDI scanning should be part of the SE proposal.
@spericas well, if you want to enforce all JAXRS component to be CDI beans then you break 100% of JAXRS applications (most of features, body readers/writers etc are standalone instances and not even managed by CDI - which is good in general in terms of design for the application but also for libraries writers).
To make it CDI compatible you don't have to break anything, just keep the integration with 3rd party libraries smooth and easy so probably don't drop @Context
support without @Inject
for not managed beans - this is the part to spec properly since it is undefined. Supporting @Inject @Context
is a good move forward for CDI beans but this one requires @Context
to be a qualifier.
So overall the logic would be:
- if the instance is a cdi bean, only
@Inject @Context
are respected (which implies CDI bean instances are explicitly supported, yeah :)) - if the instance is a standalone bean only
@Context
are respected
This is exactly what jbatch does with quite some success. Moving to a full CDI model had been evaluated but was not judged realistic for end users and 3rd parties and I tend to think it would be the same for JAXRS.
if the instance is a standalone bean only @Context are respected
It's best not to have that confusing difference.
People also thought in 2003 not everything could be a spring bean in Spring, but they hold on to that concept and it worked quite well for them.
For Faces, people also thought it could not use CDI exclusively, and there we are; using CDI exclusively. And for Jakarta Security, the same was thought; invent a semi component model that abstracts from CDI, since who knows, some person, somewhere out there, might want to use Jakarta Security with Python, and doesn't want to use CDI. But it too uses CDI exclusively making everything a lot simpler.
Don't forget that Jakarta EE doesn't have even close to the resources it had before. Maintaining large abstraction framework over things that are essentially abstract themselves, who is going to maintain that?
Not sure it is true, most jsf components are still not cdi beans and making it beans slows down things and makes the app consuming way more mem. Same for spring. Whatever you want, the way jaxrs is defined it has its own ioc so handling standalone is cheap and easy to understand for the consumers i mentionned. If you want to align it on cdi you should drop jaxrs filters, interceptors etc for the same argument/point so jaxrs becomes cdi centric and way lighter by dripping redundant concept. While I can see benefit to that it would be a huge breaking change but not doing it does not solve the duplication and ambiguities IMHO.
Not sure it is true, most jsf components are still not cdi beans
Faces UIComponents are not CDI beans, but I was referring to the managed beans (which are mostly used for backing beans). The own native managed bean / IOC facility of Faces has just been pruned and it's gone.
Internally several things are managed by CDI already, and there's more on the agenda. See e.g. https://arjan-tijms.omnifaces.org/p/jsf-23.html#cdi
and making it beans slows down things and makes the app consuming way more mem.
What is the slow down exactly? And how much more memory does it consume? Do you have any benchmarks related to this?
Whatever you want, the way jaxrs is defined it has its own ioc so handling standalone is cheap and easy to understand for the consumers i mentionned.
Is that really the case? Now Jersey for instance uses HK2 internally to handle the bean model and injection. Going forward that could be e.g. Weld. Why is Jersey using HK2 internally easier to understand than using Weld Internally?
I'd like to propose to separate discussions into several issues, hence discuss replacing the annotation @Context by the annotation @Inject separately from enforcing the use of CDI, separately from how to deal with SeBootstrap shortcomings. There might be interrelations, but I assume it makes finding a consensus much easier.
Agree but it also needs an "epic" to coordinate all of them with consistency otherwise we would end up with local solutions and no global one IMHO.
Thank you for this idea, but actually I think the JAX-RS Committers have proven since years that they are pretty able to decide on their own how they organize their project.
@mkarg exactly, this is why I asked how governance would be handled to avoid errors on this topic but it is really up to you to make it specified (my only request is no breaking change nor new useless api as an user).
Remaining backward compatible is definitely not a goal for 4.0. In fact, we have been trying to better align with CDI for a long time. Keeping support for @Context
and all that implies would be for a new 3.X release, should the need for that arise.
@spericas well since alignment is not done it can be added but the questions are "does it need to break" on one side and on ther other side "is the standalone feature used and needed". All applications I used were answering "no" and "yes" and with some good background so I think it can just be a clarification of CDI integration and not a restatement of the basis which would better fit a new spec where half of JAXRS concepts are dropped cause overlapping with CDI. To be super clear: you can break context support but if you really base JAXRS on CDI you drop all the JAXRS chains and half of the components/models/API so I humbly request a quick and dirty integration is not done if the goal is a deep integration but a full restart in a new spec to avoid ambiguity OR keep it compatible and aligned on the mainstream usage (way cheaper and likely sufficient for 99% of apps IMHO).
side note: microprofile played this "backward compatible is not a goal" but they lost a lot of users after very few years due to that and one of the biggest quality of JavaEE was to be backward compatible long enough regarding project life time so I hope it stays a fundation of JakartaEE too.
@rmannibucau I'm sorry but I cannot disagree more with your assessment of MP. In my view, Jakarta EE is the one at risk here if it continues to rely on old specs and does not bring better integration and innovations into the platform soon. As I stated before, if backward compatibility is essential, 3.X will continue to be supported by vendors for a long time.
@spericas you assume moving forward means breaking. It implies that you take microprofile path, ie break each year applications enforcing coding for just upgrades (including security upgrade). This is a very bad experience for end user IMHO and technically not justifiable. Here, the topic is to specify the CDI integration in JAXRS spec, it is a relatively easy task - since all vendors did it almost the same way - and does not require any breaking change. If you want to break, you can indeed, but as explained, you will either take the MP path and make your user paying for no feature gain OR you will create a new spec so incrementing the version is irrelevant. The issue of EE is not relying on old specs (@Inject
will likely stay for way more years without any issue) but to keep embracing new features properly - here I agree with you EE should keep up, for example defining what happens on a record, a better integration of Flow
etc...nothing related to any breaking change, debt in JAXRS is not yet high enough to justify to break from the ground and I suspect it would go through another spec as explained.
Hi, my view on this is sort of mixed but practical. I've always had problems with @Context
injection, which works completely on its own and doesn't even rely on plain @Inject
like Batch injection does. On the other hand, I don't see much value in dropping @Context
if it only means more work on the spec to remove it from the API, docs and tests. The only benefit for removing I see is that new potential implementations wouldn't need to implement it. For that sake, we can just make it optional and deprecated.
I would rather first focus on the replacement - to fully specify how CDI works in JAX-RS, how method arguments are injected (AFAIK this isn't supported by @Inject
and CDI), and everything that would replace @Context
.
Once we're done with that, we can drop @Context
. Batch did it this way, improved CDI integration without dropping anything and left all removals for later. Faces defined CDI integration first with @Named
and CDI scopes and only later dropped managed beans. But Faces now suffer a bit because a lot of tests will need to be removed or modified in the Jakarta TCK to align it.
Last comment sounds like a good idea. Maybe stretch dropping over a few releases. Yes, this means YEARS. Maybe like this?
- the next major version should issue a warning
- the major version after that should rework context to be a qualifier
- only then in the again next major version remove it completely.
Just my 2ยข because I agree with all of you: why break something for users which has been working like this forever? But then, don't hold on to old standards?
From a user's perspective (I use OpenLiberty a lot), jaxrs-x.0 will pull in servlet-y.0. Maybe some day servlet 3.0 will not be available anymore. Or I want to update step by step because (you got it:) big companies. And at some point you have to update.
So, if you need to break it, either give it a new name and artifact, or do it over multiple releases/years as gracefully as possible.
Thanks. ๐
Oh, and thanks everyone for the good work and being a part of this. Even comments and opinions in the discussion matter a lot! ๐๐ป
how method arguments are injected (AFAIK this isn't suppoeted by @Inject and CDI),
Well... actually it is. It's just that the parameters don't need to have the @Inject annotation applied to it. Producers and constructors use this all the time.
For instance:
@ApplicationScoped
public class ApplicationInit {
private static final Logger logger = Logger.getLogger(ApplicationInit.class.getName());
@Produces
public HttpAuthenticationMechanism produce(InterceptionFactory<HttpAuthenticationMechanismWrapper> interceptionFactory, BeanManager beanManager) {
..
}
}
Here, the interceptionFactory
and beanManager
parameters are not part of any method signature and can be declared in any order, and any amount of parameters can be specified here. CDI will inject them as required, exactly as Jakarta REST does.
That said, we do have an issue open in CDI to make this a bit more convenient to use for arbitrary methods:
https://github.com/jakartaee/cdi/issues/460
Maybe stretch dropping over a few releases. Yes, this means YEARS.
In a way dropping @Context
has already been in progress for give or take 13 to 14 years. Yes, that process already started before JAX-RS was released in 2009.
Somewhere around 2008 @gavinking noticed that the JAX-RS team was duplicating the efforts of the CDI team and went out to talk to them. It has been lost in history what exactly transpired in that talk, but shortly before the release of EE 6 a couple of amendments were made to provide some level of integration between the JAX-RS component model and the CDI one.
@arjantijms your example is great to complete the reaons why @Context
is needed. You example shows that @Inject
can be omitted when implicit - your example is a CDI managed method where all parameters are CDI beans. JAX-RS is not that since parameters could come from CDI but they can also come from JAX-RS so it is not comparable, in particular with subresources or context instances which can be different from CDI instance (think to JSON-B instance for a trivial case but security and other transversal cases will get more complex usages of such a thing - multiple instances of the same bean "typed" in DI only through a qualifier).
The reason JAX-RS does not use CDI is because making it easy to run with another IoC always had been a goal which also led to creating the JAX-RS chains (filters, interceptors, etc...). All that is useless if you are based on CDI except @Context
to remove the injection ambiguity as explained earlier so really think context is not the right fight for any coming release since best case it would be replaced by @Qualifier @interface Context {}
which would be 1-1 for end users in all cases but the field/setter injections where @Inject
would become required until CDI supports implicit inject for qualifiers - AFAIK it is more or less pushed by quarkus to become standard but some implementations already support it - quarkus/openwebbeans AFAIK.
CDI was fairly new and "unproven" at the time and it did not support SE in the early days. JAX-RS had other requirements that were difficult to achieve with a direct dependency on CDI. Times have changed, CDI is a core component in EE and MP, so there is no need to duplicate efforts any longer and make JAX-RS implementations complicated.
The reason JAX-RS does not use CDI is because making it easy to run with another IoC always had been a goal which also led to creating the JAX-RS chains (filters, interceptors, etc...). All that is useless if you are based on CDI
They are not useless. You could argue entity interceptors could be replaced (they don't really have to), but most of the other JAX-RS providers are far from useless.
The reason JAX-RS does not use CDI is because making it easy to run with another IoC always had been a goal
Are you sure? I can't remember that being stated anywhere at the time (or currently). Do you have any historical evidence for this? (not trying to be smart here, I spend a great amount of time finding out about the history of bean models and injection and writing about it for my CDI book).
At any length, CDI can be bridged to another component model / IoC just as well. Perhaps better even.