chkal / mvc-spec-migration-test

0 stars 0 forks source link

Optional support for the injection of Models with JAX-RS @Context #48

Closed chkal closed 8 years ago

chkal commented 8 years ago

Original issue MVC_SPEC-50 created by beryozkin_sergey:

The spec requires that MVC beans are CDI-managed only.

When talking about Models it says that "support for CDI @Named beans is OPTIONAL" and gives an example of Models being itself CDI-injected with @Inject and says it may be needed to support ViewEngines that are not CDI-enabled. In ViewEngines section it further talks about defaulting to RequestDispatcher if no engines are available.

IMHO this is a good reason to relax a requirement that MVC beans must be CDI-managed only. It appears too strong in a situation where one needs to link such a bean with a non CDI view engine, having to bring a CDI dependency even though the given view engine can not utilize it.

Additionally if it is a hybrid case (and the default forwarding to RequestDispatcher is expected in the deployment) then we can have

@Path("/service")
public class Service {

@Context HttpServletRequest request;
// other JAX-RS contexts
@Inject Models models;

}

{code} 

while  

{code:java}
@Path("/service")
public class Service {

@Context HttpServletRequest request;
// other JAX-RS contexts
@Context Models models;

}

works better in a case where no CDI is expected. The runtime will inject a Models map, then when processing the response will set attributes on HTTP request and forward it to RequestDispatcher or indeed to ViewEngine.

CDI is obviously needed when we have CDI aware ViewEngine so that the model can be named and implicitly linked. In a case where a view engine is not CDI aware all that CDI gives us is the injection into a JAX-RS MVC bean and I think it is too restrictive that CDI is a must requirement in such cases.

Proposal: add a statement that JAX-RS applications MIGHT optionally support injecting Models via @Context and update a CDI binding requirement to mention that it is optional in such cases.

chkal commented 8 years ago

Comment by beryozkin_sergey:

Sorry, would like to assign to Santiago but can not modify it now.

chkal commented 8 years ago

Comment by Santiago Pericas-Geertsen:

CDI is an integral part of the "MVC fabric". MVC was designed to completely rely on it for injection, events, SPI lookup, etc. Making CDI optional means redesigning most of the key parts of the API. Moreover, it means that portability will suffer for apps that use CDI and want to move a platform that supports MVC but not CDI. Sorry, this is a "Won't Fix". If your concern is lightweight servers, people have already tried successfully using Weld in those environments.

chkal commented 8 years ago

Comment by beryozkin_sergey:

Hi Santiago, that is fine, we do have a Weld level integration.

Can you clarify please one point:

How is CDI helping the whole MVC fabric in a case where Models is injected directly ? It is a pure injection, nothing to do with MVC per se ?

Thanks

chkal commented 8 years ago

Comment by Santiago Pericas-Geertsen:

For everything else that I said. Even in that case, the controller/resource class is CDI, the events the implementation generates are CDI events, the view engine lookup relies on injection, and there are injectables in the API like Csrf, Encoders and Validation/BindingResult.

Bear in mind that the @Context injection only exists because CDI did not exist at the time JAX-RS 1.0 was created. IMO, it would be nice for JAX-RS to completely transition to CDI and deprecate its own injection mechanism some day.

chkal commented 8 years ago

Comment by beryozkin_sergey:

OK, thanks, worried about CDI becoming a de-facto requirement for the future work. I'm thinking we (JAX-RS communities) will lose people a lot, migrating to Spring WS/etc, as they will have to choose between CDI and Spring, etc...Thanks anyway, supporting MVC via CDI is doable indeed.

chkal commented 8 years ago

Comment by Santiago Pericas-Geertsen:

Note that CDI is also used to manage application beans. I actually think that JAX-RS moving more and more to CDI will benefit rather than hurt JAX-RS. The current mix of @Context and @Inject injection is confusing for developers, and in that sense JAX-RS does not compare favorably to other similar frameworks.