rest
rest copied to clipboard
jakartaee
CDI constructor injection & the use of multi-arg constructors Resource and Provider classes
One thing that would be very helpful for implementors is providing clarity on how JAX-RS Resource classes and Providers are instantiated when used with CDI. Presently, there's some significant behavioral differences when switching between JAX-RS implementations with CDI. For example, using Jersey 2.27 with JBoss Weld 3.0.4.Final , constructor injection works as you'd expect. With you attempt to do the same thing with JBoss RESTEasy 3.5.1.Final and Weld 3.0.4.Final, you get an error stating that a 0-arg constructor is required. The JBoss team is interpreting this language in section 3.1
By default a new resource class instance is created for each request to that resource
As a zero-arg constructor is required. See this issue here:
https://issues.jboss.org/browse/RESTEASY-1538
That issue is closed and marked as resolve, citing language in the spec.
I think it would be helpful somewhere in section 3 that when using CDI with JAX-RS, constructor injection MUST be supported when a Resource class have a multi-arg constructor annotated with @Inject.
Similarly, it should also be permissible to do something like this:
@Provider
public class JacksonObjectMapperProvider implements ContextResolver<ObjectMapper> {
private ObjectMapper objectMapper;
@Inject
public JacksonObjectMapperProvider(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
}
@Override
public ObjectMapper getContext(Class<?> type) {
return objectMapper;
}
}
Yet again, the above code works fine in Jersey + Weld while RESTEasy + Weld complains about not having a zero-arg constructor. As CDI starts to be folded into JAX-RS, some additional verbiage to make this abundantly clear that constructor injection must be supported in these use cases would be wonderful.
Hi,
The requirement to support the multi-arg constructor for JAX-RS sounds good indeec.
On Sunday, July 1, 2018, Ryan J. McDonough [email protected] wrote:
One thing that would be very helpful for implementors is providing clarity on how JAX-RS Resource classes and Providers are instantiated when used with CDI. Presently, there's some significant behavioral differences when switching between JAX-RS implementations with CDI. For example, using Jersey 2.27 with JBoss Weld 3.0.4.Final , constructor injection works as you'd expect. With you attempt to do the same thing with JBoss RESTEasy 3.5.1.Final and Weld 3.0.4.Final, you get an error stating that a 0-arg constructor is required. The JBoss team is interpreting this language in section 3.1
By default a new resource class instance is created for each request to that resource
As a zero-arg constructor is required. See this issue here:
https://issues.jboss.org/browse/RESTEASY-1538
That issue is closed and marked as resolve, citing language in the spec.
I think it would be helpful somewhere in section 3 that when using CDI with JAX-RS, constructor injection MUST be supported when a Resource class have a multi-arg constructor annotated with @Inject.
Similarly, it should also be permissible to do something like this:
@Providerpublic class JacksonObjectMapperProvider implements ContextResolver<ObjectMapper> {
private ObjectMapper objectMapper; @Inject public JacksonObjectMapperProvider(ObjectMapper objectMapper) { this.objectMapper = objectMapper; } @Override public ObjectMapper getContext(Class<?> type) { return objectMapper; }}
Yet again, the above code works fine in Jersey + Weld while RESTEasy + Weld complains about not having a zero-arg constructor. As CDI starts to be folded into JAX-RS, some additional verbiage to make this abundantly clear that constructor injection must be supported in these use cases would be wonderful.
— 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/633, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5XTlpM7dmyENwMoLAtI_XSJUON-CTLks5uCOu-gaJpZM4U-XX3 .
Hi Ryan,
Section 11.2.8 of the JAX-RS spec states that constructor injection is OPTIONAL with CDI (and other beans). It recommends the use of field injection and @PostConstruct for portability.
— Santiago
On Jul 1, 2018, at 11:33 AM, Ryan J. McDonough [email protected] wrote:
One thing that would be very helpful for implementors is providing clarity on how JAX-RS Resource classes and Providers are instantiated when used with CDI. Presently, there's some significant behavioral differences when switching between JAX-RS implementations with CDI. For example, using Jersey 2.27 with JBoss Weld 3.0.4.Final , constructor injection works as you'd expect. With you attempt to do the same thing with JBoss RESTEasy 3.5.1.Final and Weld 3.0.4.Final, you get an error stating that a 0-arg constructor is required. The JBoss team is interpreting this language in section 3.1
By default a new resource class instance is created for each request to that resource As a zero-arg constructor is required. See this issue here:
https://issues.jboss.org/browse/RESTEASY-1538 https://issues.jboss.org/browse/RESTEASY-1538 That issue is closed and marked as resolve, citing language in the spec.
I think it would be helpful somewhere in section 3 that when using CDI with JAX-RS, constructor injection MUST be supported when a Resource class have a multi-arg constructor annotated with @Inject.
Similarly, it should also be permissible to do something like this:
@Provider public class JacksonObjectMapperProvider implements ContextResolver<ObjectMapper> {
private ObjectMapper objectMapper; @Inject public JacksonObjectMapperProvider(ObjectMapper objectMapper) { this.objectMapper = objectMapper; } @Override public ObjectMapper getContext(Class<?> type) { return objectMapper; }} Yet again, the above code works fine in Jersey + Weld while RESTEasy + Weld complains about not having a zero-arg constructor. As CDI starts to be folded into JAX-RS, some additional verbiage to make this abundantly clear that constructor injection must be supported in these use cases would be wonderful.
— 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/633, or mute the thread https://github.com/notifications/unsubscribe-auth/AFU9N6AO-PpH7xeUxDoDHA9GsNPKwBH1ks5uCOu-gaJpZM4U-XX3.
@spericas
Section 11.2.8 of the JAX-RS spec states that constructor injection is OPTIONAL with CDI (and other beans). It recommends the use of field injection and @PostConstruct for portability.
Maybe it's only a matter of rephrasing that saying that "constructor injection is NON-OPTIONAL (MUST be supported)", and that "field injection and @PostConstruct is a style choice and has no influence on portability"?
@spericas I'm wondering why constructor injection was specified as being OPTIONAL back then. Is there any special reason why it wasn't defined as mandatory.
Christian,
I would need to dig up that discussion to be certain, but I suspect it is related to the usual problem of combining DI systems. JAX-RS constructors much support param injection of @Context, etc. so, from an implementation perspective, it can get tricky to instantiate JAX-RS/CDI resource classes.
We could strengthen this in the spec, but is it really worth adding more implementation complexity? I much rather spend energy moving to a CDI-as-only-DI world.
— Santiago
On Jul 7, 2018, at 2:59 AM, Christian Kaltepoth [email protected] wrote:
@spericas https://github.com/spericas I'm wondering why constructor injection was specified as being OPTIONAL back then. Is there any special reason why it wasn't defined as mandatory.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eclipse-ee4j/jaxrs-api/issues/633#issuecomment-403194205, or mute the thread https://github.com/notifications/unsubscribe-auth/AFU9N4ni4ZruZmb7A03G8M4siiaIdu6Vks5uEFw-gaJpZM4U-XX3.
I would need to dig up that discussion to be certain, but I suspect it is related to the usual problem of combining DI systems.
Very practical speaking we have used something like this in Payara where both HK2 and CDI tried to inject something. By making it a constructor injection which only CDI does, the conflict was "resolved" (although it's indeed not an ideal solution and there should be a veto actually).
much rather spend energy moving to a CDI-as-only-DI world.
Perhaps you're right, and we shouldn't spend energy on this.
Btw, do you remember why CDI was not chosen right away as the only DI when JAX-RS 1.0 or 1.1 was released? I know that JAX-RS 1.0 went final before CDI 1.0, but it was quite clear that CDI was coming.
On Jul 9, 2018, at 10:55 AM, Arjan Tijms [email protected] wrote:
I would need to dig up that discussion to be certain, but I suspect it is related to the usual problem of combining DI systems.
Very practical speaking we have used something like this in Payara where both HK2 and CDI tried to inject something. By making it a constructor injection which only CDI does, the conflict was "resolved" (although it's indeed not an ideal solution and there should be a veto actually).
much rather spend energy moving to a CDI-as-only-DI world.
Perhaps you're right, and we shouldn't spend energy on this.
Btw, do you remember why CDI was not chosen right away as the only DI when JAX-RS 1.0 or 1.1 was released? I know that JAX-RS 1.0 went final before CDI 1.0, but it was quite clear that CDI was coming.
I wasn’t part of that EG. Yes, CDI was coming but also recall that JAX-RS always supported a standalone mode and, at the time, CDI implementations were considered somewhat large and complex (more than pure DI for sure, which was the one part JAX-RS needed) so I think the decision made at the time is perfectly understandable given the constraints.
— Santiago
To second Santiago, as far as I can remember it was not so clear than today that CDI will be embraced such globally. JAX-RS needed a solution that will be there when JAX-RS 1.0 is there, and that is there as long JAX-RS is there. CDI could have failed.
I wasn’t part of that EG. Yes, CDI was coming but also recall that JAX-RS always supported a standalone mode and, at the time, CDI implementations were considered somewhat large and complex
I can appreciate these reasons, although at the time JAX-RS 1.0 was released CDI implementations hadn't been build yet really (OWB, CanDI and Weld were all in progress or even just started), so maybe that couldn't have been the case. Or at the very least, maybe there was the fear that they would eventually get large and complex.
(more than pure DI for sure, which was the one part JAX-RS needed) so I think the decision made at the time is perfectly understandable given the constraints.
There was JSR 330 though at that same time period, which was only DI. In fact that is what HK2 was eventually made compatible with and which is what Jersey had been using.
JAX-RS needed a solution that will be there when JAX-RS 1.0 is there, and that is there as long JAX-RS is there. CDI could have failed.
That sounds a bit more likely. At the time the mailing list wasn't open, but does anyone still have an actual mail were this decision was made? I remember that at some point Bill Burke mentioned some thing about JAX-RS going to use its own DI engine, and then Gavin King was about to talk to the JAX-RS EG about that, but not sure if that ever happened.
On Jul 9, 2018, at 1:23 PM, Arjan Tijms [email protected] wrote:
I wasn’t part of that EG. Yes, CDI was coming but also recall that JAX-RS always supported a standalone mode and, at the time, CDI implementations were considered somewhat large and complex
I can appreciate these reasons, although at the time JAX-RS 1.0 was released CDI implementations hadn't been build yet really (OWB, CanDI and Weld were all in progress or even just started), so maybe that couldn't have been the case. Or at the very least, maybe there was the fear that they would eventually get large and complex.
The CDI spec already included Events and many other features that were not needed (and was likely bigger than JAX-RS at the time).
(more than pure DI for sure, which was the one part JAX-RS needed) so I think the decision made at the time is perfectly understandable given the constraints.
There was JSR 330 though at that same time period, which was only DI. In fact that is what HK2 was eventually made compatible with and which is what Jersey had been using.
That is just a few annotations, would have been very confusing if JAX-RS defined different semantics for them.
JAX-RS needed a solution that will be there when JAX-RS 1.0 is there, and that is there as long JAX-RS is there. CDI could have failed.
That sounds a bit more likely. At the time the mailing list wasn't open, but does anyone still have an actual mail were this decision was made? I remember that at some point Bill Burke mentioned some thing about JAX-RS going to use its own DI engine, and then Gavin King was about to talk to the JAX-RS EG about that, but not sure if that ever happened.
Interesting, didn’t know that.
— Santiago
We could strengthen this in the spec, but is it really worth adding more implementation complexity? I much rather spend energy moving to a CDI-as-only-DI world.
Maybe I'm missing something obvious here, but IMO this issue is actually about improving integration with CDI. So fixing it is an important step into the "CDI-as-only-DI world" direction. Currently implementations are not required to support CDI constructor injection which really feels weird for everyone familiar with CDI.
On Jul 15, 2018, at 2:46 AM, Christian Kaltepoth [email protected] wrote:
We could strengthen this in the spec, but is it really worth adding more implementation complexity? I much rather spend energy moving to a CDI-as-only-DI world.
Maybe I'm missing something obvious here, but IMO this issue is actually about improving integration with CDI. So fixing it is an important step into the "CDI-as-only-DI world" direction. Currently implementations are not required to support CDI constructor injection which really feels weird for everyone familiar with CDI.
Of course, but I think the original request was about the 2.2 spec. Needless to say this needs to be addressed (with all the other issues) as part of 3.0.
— Santiago
Of course, but I think the original request was about the 2.2 spec. Needless to say this needs to be addressed (with all the other issues) as part of 3.0.
If 2.2 already largely moved to CDI, is it still necessary for 3.0 then?
If 2.2 already largely moved to CDI, is it still necessary for 3.0 then?
If the real issue is the implementation complexity of combining CDI's @Inject with JAX-RS's @Context in case of mixed constructor injection, the problem gets automatically solved by eliminating @Context as part of 3.0. At least this is my understanding.
I just ran across this issue after doing a search for the same topic and realized I never followed up on it :) @spericas I was actually considering this for a 3.0 feature. Adding it to the 2.x spec seems rather disruptive. The challenge is that moving between JAX-RS implementations while using the same CDI implementation yield very different results. So yes @chkal, this is indeed about improving integration with CDI as it's not at all consistent across JAX-RS implementations at the moment. If CDI will replace things like @Context, the stance on constructor injection should be addressed head-on to avoid what is happening today.
Making constructor injection optional feels really weird from an end user perspective. Especially when I can have a non JAX-RS class use constructor injection but I am forced into field injection on JAX-RS resources or providers depending on the chosen implementation.
I agree that it is very unintuitive that a JAX-RS controller would not support @Inject on its constructor. It forces me to use mutable fields on the controller and is inconsistent with non JAX-RS classes where Weld constructor wiring is no problem. I second the notion expressed elsewhere in this issue that this needs addressing. For example, constructor wiring is supported consistently in Spring
