| Re: [hudson-dev] Supporting Java EE 7 (CDI 1.1) and later |
I learned unknown annotations are silently ignored if not available on the classpath as Stuart described, so we won't need to include any new jars. It should be OK to just add dependency on cdi-api with scope 'provided' so that we can use CDI annotations only at compile-time. I hope I can make a new patch in a few days.
Hi Kaz,
I have created this wiki page (just cut and paste some info from this e-mail). Can you please update it with all the findings and detail the solution we are going to use.
We have to get approval from Eclipse to add any new jar we bundle. So list the new jars that will be included in the war.
Thanks for all the work.
- Winston
If we try to resolve this issue without CDI compatibility, we will have to use Guice-equivalent of javax.inject.Inject, if any.Just FYI, I got an error from Weld even for org.hudsonci.service.internal.ServiceSupport.setHudson that it has unsatisfied dependency. As you can see, class ServiceSupport has no annotations, just its method setHudson has @Inject.Accoding to the CDI 1.1 spec, in the default 'annotated' bean discovery mode, ServiceSupport should not be discovered as a bean since it has no annotations. So it appears that Weld is trying to process all @Inject annotations even for classes that have no bean defining annotations, as long as they have constructors without parameters, with which they are considered as managed beans. It is unsolicited in my opinion, but it actually works as such.
On Wed, Jan 28, 2015 at 9:38 AM, Kaz Nishimura <kazssym@xxxxxxxxx> wrote:
Thank you for the description. I had believed annotations must be on the classpath if not provided by the container.
2015/01/28 9:26 "Stuart McCulloch" <mcculls@xxxxxxxxx>:
On Wednesday, 28 January 2015 at 00:22, Kaz Nishimura wrote:If we ship Hudson with a cdi-api jar, compatibility with Java EE 5 servers will not be a problem, I believe.
I wouldn’t ship it with the cdi-api jar, instead make the dependency scope provided as the web container would be expected to provide it. That way the helper will activate on CDI capable web containers, and will be ignore elsewhere.On the other hand, if we ship Hudson with the cdi-api 1.1 jar, does it break compatibility with Java EE 6? I hope it doesn't but I am not a Java expert, so I chose to depend on the latest official release of CDI 1.0 API, cdi-api 1.0-SP4 for the moment.
FYI, as far as I know, any cdi-api jars just contain annotations (and necessary classes and interfaces, if any) without any CDI implementation.
2015/01/28 2:04 "Stuart McCulloch" <mcculls@xxxxxxxxx>:
On Tuesday, 27 January 2015 at 16:53, Winston Prakash wrote:Hi Kaz/Sturat/Bob,
Let me try to re-iterate and understand the situation.
Hudson works in EE containers 5.x and below because there is no CDI. It also works in EE 6 because CDI is not enabled by default. However DI works in Hudson because Guice used in Hudson properly takes care of DI annotations (@Inject etc).
In EE7 CDI is enabled. Since CDI finds DI annotations in the code, it thinks it service is required. However, since it finds the wrong Injection Helpers (Specified as a Singleton), which is really meant for Guice, it throws error.
Three suggestions so far:
1. Add some dummy implementation for CDI Injection Helpers, so CDI will be happy, though it does nothing. But still Guice takes care of the DI annotations
2. Change all javax.inject.Singleton to com.google.inject.Singleton. Since CDI will not understand this annotation, it will not try to incorrectly use the Injection Helpers and throw exception. However, Guice understands this annotation and works a normal.
3. Replace Guice with CDI for a cleaner core.
Option 3 is the cleaner solution and is the most difficult choice.
Option 1 satisfies CDI and everything works in EE 6 & 7. But now Hudson stops working in EE5 and below.
If the injection helpers in 1) only use CDI annotations then there’s no reason that won’t work in EE5, since the JVM will silently ignore annotations that aren’t on the classpathOptions 2 is not cleaner. Because we will have some java.inject.* annotations and com.google.inject.Singleton. Also java.inject.Singleton will be totally forbidden. However Hudson will work on EE 5 and below as well 6 & 7.
For an interim solution, I'm kind of leaning towards option 2 because we will not be sidelining EE 5 below. Lots of App servers may be still be in EE5.
My above understand may be still wrong. If so please educate me :-)
- WInston
According to Oracle's definition, CDI (JSR 346) is a standard part of the Java EE 7: http://www.oracle.com/technetwork/java/javaee/tech/index-jsp-142185.html even with the web profile.
Weld is well known as the reference implementation of CDI and used by several products but another implementation that conforms to JSR 346 can also be used instead, if any. Other DI-only implementation that can coexist with CDI may be OK as well. To use CDI or other DI technology is what each application chooses but CDI 1.1 (or Weld only?) is too strict about unsatisfied CDI dependencies in my opinion.
So if an application chooses not to use CDI, it can either just satisfy CDI dependencies without real implementations as I am trying to do now OR instruct users to disable CDI at deployment-time. I don't think runtime switching is a good idea. It would make QA complicated and could make the application less reliable.
Also note that GlassFish includes both Weld and HK2 as far as I know. Since HK2 requires its own annotations to activate it, they can coexist without conflicts, I think. Do you know any Guice update (or its plan) regarding conflict avoidance?
I agree with Bob. One important difference we make in Hudson is, rather than supporting 1000's of plugins by keeping backward compatibilities with outdated technologies, we take bold steps to move to new technologies and try to keep the core more stable, scalable and modern. On the process we may keep only few 100s of most important plugins working better on the modern core. But that is sufficient for enterprises with huge Hudson installation looking for a much stabler CI server.
Having said that, we have a bug existing for more than a year stating Hudson does not work when deployed to Glassfish 4.x. I'm wondering if this simple workaround is stable enough to include it in the next bugfix release and then take up the task of moving to the standard DI implementation.
Now comes the question, which is the standard DI ?
I have very limited knowledge of CDI. Based on my understanding (through limited reading)
CDI = DI + additional contexts. In Hudson we use only DI (Guice). There are three DI implementations (Guice, HK2, Spring). Weld which is a CDI reference implementation, has its own DI (I think it is simplified spring DI) and additional contexts.
Theoretically I would think the DI in CDI would be switchable (or play nicely with other DI). That means either you use the default DI in Weld or plug in Guice/HK2 as DI. Looks like Glassfish 4.x is making HK2 work with CDI.
Do we need to have the full CDI (which is a superset) bundled in Hudson or use only the subset but standard DI in Hudson so that when we run on EE7 containers with CDI already in place, the bundled DI becomes "inactive" allowing the container CDI to take over (or am I just being ignorant here :-) ).
So which one is the standard DI -- HK2, Guice, Spring, Weld DI?
- Winston
I think the future is upon is. My humble opinion is we need to find a CDI-compatible implementation that will work fine with Tomcat and Jetty (and I guess Glassfish for the diehards).
Editorial comment: Hudson can't continue to be a graveyard for old technologies.
Whatever we choose, thanks, Kaz, for digging into this!
On Fri, Jan 23, 2015 at 11:31 PM, Kaz Nishimura <kazssym@xxxxxxxxx> wrote:
It would be a future option to rely on CDI entirely, but Hudson currently uses Guice for dependency injection (as far as I know) and it actually works now, so such a switch will require some rewrites. And CDI is not available on all Java web servers yet, for example, Tomcat and Jetty do not include it by default.
So I cannot tell whether such rewrites deserve to do or not.
On Sat, Jan 24, 2015 at 11:01 AM, Bob Foster <bobfoster@xxxxxxxxx> wrote:
Why not make it work with CDI by using CDI?
On Fri, Jan 23, 2015 at 5:50 PM, Kaz Nishimura <kazssym@xxxxxxxxx> wrote:
It might better to change the package and class names, though.I submitted a patch to satisfy all unsatisfied dependencies only on the form as https://git.eclipse.org/r/#/c/40294/The advantage of this approach would be it does not change any existing classes and all the producer methods are placed inside a small number of unused classes. In addtion to this patch, a rename of the @Named annotation of ExtensionLocator actually allowed me to deploy Hudson 3.2.2-SNAPSHOT on GlassFish 4.1 without changing any deployment options.
On Sat, Jan 24, 2015 at 9:24 AM, Kaz Nishimura <kazssym@xxxxxxxxx> wrote:
In addition, Bug 458002 must also be resolved to deploy Hudson on a Java EE 7 server with its default settings.As one of the first steps, I made classes like the following just to satisfy unsatisfied CDI dependencies, assuming these methods are never called:Each producer method might throw an UnsupportedException instead of just returning null to indicate more clearly that it shall not be used. What do you think?
@Singleton
public class InjectionHelper {
@Produces
private Injector getInjector() {
return null;
}
@Produces
private SmoothieContainer getSmoothieContainer() {
return null;
}
@Produces
@Named("default")
private ExtensionLocator getDefaultExtensionLocator() {
return null;
}
@Produces
private List<ExtensionFinder> getExtensionFinders() {
return null;
}
@Produces
@Parameters
private Map<String, String> getParametersMap() {
return null;
}
}
On Fri, Jan 23, 2015 at 5:59 PM, Kaz Nishimura <kazssym@xxxxxxxxx> wrote:
Oops, s/following test/following text/
On Fri, Jan 23, 2015 at 5:57 PM, Kaz Nishimura <kazssym@xxxxxxxxx> wrote:
As Winston suggested, I want to start discussion about Java EE 7 server support (or CDI 1.1 support) in Hudson. The following test is my understanding of the current situation (mostly taken from my forum post).My proposal is to add some bean classes to Hudson that have producer methods solely for satisfying dependencies on the form. I believe the producer methods need not to do anything useful since Hudson does not actually use CDI. If those methods are known not to be called, they can just either return null or throw an exception always.
(Basic assumption)
As far as I know, Hudson uses dependency injection based on Guice by Google and does not use CDI that is now a standard part of Java EE (6 and later).
(No problem before Java EE 7)
Java EE 6 included CDI 1.0 but it is not automatically enabled if beans.xml is not included, so Hudson is happy with any Java EE 6 servers.
(Problem with Java EE 7)
To resolve issues that many users forget to include beans.xml (I guess), CDI 1.1 introduced bean discovery modes and CDI is automatically enabled in the 'annotated' mode even if there is no beans.xml. At least one (and the reference) CDI implementation, Weld by JBoss, tries to discover a bean that can be injected to any method and field annotated with @Inject even if it will not actually use CDI for injection, which causes conflicts to the use of @Inject in Hudson.
(Workaround)
Most (or all) Java EE 7 servers has an option to disable the automatic CDI when deploying an application. By disabling the automatic CDI, Hudson can be deployed successfully on such servers. A problem is the way to activate the option differs by servers.
(No simple resolution)
Although CDI 1.1 introduced bean discovery modes and one of which is 'none', CDI 1.0 implementations will not see the value and might enable CDI by the existence of beans.xml contrary to the intent to disable it. So including beans.xml is not an option to resolve this problem at all.
_______________________________________________
hudson-dev mailing list
hudson-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/hudson-dev