Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 348123 - Annotation @PostConstruct not found by injector
Summary: Annotation @PostConstruct not found by injector
Status: RESOLVED FIXED
Alias: None
Product: e4
Classification: Eclipse Project
Component: UI (show other bugs)
Version: unspecified   Edit
Hardware: PC Linux
: P3 blocker (vote)
Target Milestone: 4.1 RC4   Edit
Assignee: Paul Webster CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-02 14:05 EDT by Paul Webster CLA
Modified: 2013-07-11 10:31 EDT (History)
11 users (show)

See Also:
remy.suen: review+
tjwatson: review+


Attachments
patch (1.19 KB, patch)
2011-06-02 14:27 EDT, Andrew Niefer CLA
no flags Details | Diff
test case patch (1.26 KB, patch)
2011-06-02 17:26 EDT, Thomas Watson CLA
no flags Details | Diff
import package v01 (22.03 KB, patch)
2011-06-03 10:02 EDT, Paul Webster CLA
no flags Details | Diff
Sample app showing PostConstruct issue (193.46 KB, application/zip)
2013-05-16 16:19 EDT, Patrick Paulin CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Webster CLA 2011-06-02 14:05:13 EDT
This shows up in a couple of places, but is easily reproducible in our MenuTestSuite.

One of our classes created by ContextInjectionFactory.make(CmdService.class, ctx); doesn't get its @PostConstruct method called.

When debugging down, our code in org.eclipse.e4.core.di does (in effect) a method.isAnnotationPresent(@PostConstruct.class)

The method instance has PostConstruct from the Java6 JRE_LIB and our code has PostConstruct from javax.annotation 1.1.0.v201105051105, so our test is failing and we're not initializing our objects.

This is a blocking problem.

PW
Comment 1 Paul Webster CLA 2011-06-02 14:06:39 EDT
The 1.0.0 javax.annotation bundle has "Require-Bundle: system.bundle".  Doesn't the 1.1.0 version require this as well?

Or is this the case where javax.annotation 1.1.0 is not in 1.6, and so it shouldn't?

PW
Comment 3 Paul Webster CLA 2011-06-02 14:11:42 EDT
Andrew, how can I tell where this version is getting into our build?  Our e4-orbit.map lists the correct one.

plugin@javax.annotation,1.0.0=p2IU,id=javax.annotation,version=1.0.0.v20101115-0725

PW
Comment 4 Andrew Niefer CLA 2011-06-02 14:18:20 EDT
The ./org.eclipse.e4.ui.releng/maps/orbit.map is also being consulted here.

One fix would be for the feature to restrict which version of javax.annotation it wants by specifying "1.0.0.qualifier" in the feature.xml
Comment 5 Paul Webster CLA 2011-06-02 14:23:58 EDT
Is Indigo supposed to have javax.annotation 1.1.0 in it?

I don't see it marked as a singleton, which means it's possible to have both versions of the bundle installed at the same time.

But won't this effect anybody else that does use 1.1.0 (sometime it'll pick java 6 JRE_LIB and sometimes it will pick the bundle)?

PW
Comment 6 Andrew Niefer CLA 2011-06-02 14:27:53 EDT
Created attachment 197259 [details]
patch

I believe this is the simplest and most robust method of selecting a specific version of javax.annotation.

Currently the 4.1 SDK build uses map files from both org.eclipse.e4.sdk and org.eclipse.e4.ui.releng.  Any other fix would involve separating the 4.1 map files from e4 map files leading to possibly having duplicate entries.
Comment 7 Thomas Watson CLA 2011-06-02 17:26:47 EDT
Created attachment 197279 [details]
test case patch

The reason the tests are getting wired to the system export of javax.annotation (version 0.0.0) is because of the use of Require-Bundle.  org.eclipse.e4.ui.menu.tests bundle requires org.eclipse.core.runtime.  The org.eclipse.core.runtime bundle requires org.eclipse.osgi and re-exports it!  The org.eclipse.e4.ui.menu.tests also requires javax.annotation (bundle-version>=1.0.0).

The issue is OSGi class loading delegation happens in the order the bundles are required.  So from the org.eclipse.e4.ui.menu.tests we delegate all class loads of javax.annotation first to org.eclipse.osgi and then to the javax.annotation bundle.  Essentially javax.annotation is treated as a split package.  All bundles should use Import-Package for the javax.annotation bundles to avoid this.  This patch moves the require-bundle for javax.annotation to the beginning of the require-bundle list for the org.eclipse.e4.ui.menu.tests bundle.  This allows it to load the annotation class from the bundle first.

Ahhhh, split packages and re-exporting of bundles, two of the more evil things that have been introduced to the modularity world.
Comment 8 Paul Webster CLA 2011-06-02 20:09:53 EDT
I opened bug 348155 to talk to Virgo about the implications of javax.annotation 1.1.0 vs Java 1.6 and Java 1.7 (which includes 1.1.0)

PW
Comment 9 Paul Webster CLA 2011-06-02 21:24:59 EDT
As I see it, we have 2 options for 4.1:

====
Option 1.  specify javax.annotation 1.0 in our  features and in Import-Package in our bundles, excluding 1.1.  This will work on java 1.5 and 1.6.  I'm not sure on java 7

AFAICT Virgo is not in Indigo, so javax.annotation 1.1 is not in the release train.

When installing Virgo it (must) specifically request 1.1, although as it turns out if they depend on org.eclipse.core.runtime they have to make sure they have their dependency order correct as well, or only ever use Import-Package.

====
Option 2:  specify javax.annotation 1.1 in our bundles and features.  We'll never use the system bundle in java 1.5 and 1.6.  I'm not sure about java 7, however, since the system provides the package javax.annotation 1.1 as well and there's some question about how the bundle will interact with java 7.

PW
Comment 10 Paul Webster CLA 2011-06-02 21:50:28 EDT
I'm voting for Option 1:

First we go through and make sure all of our bundles
1) use Import-Package and 
2) use the version range [1.0.0,1.1.0)

Then we specify the version 1.0.0 in our org.eclipse.e4.rcp feature.

PW
Comment 11 Hugues Malphettes CLA 2011-06-02 22:49:52 EDT
Sorry for the trouble Paul and thanks for formulating the issue so clearly.
I volunteered to package javax.annotation for Indigo.

I did not suspect all the subtleties going on behind Require-Bundle:
system.bundle
and singleton.

Looking forward to hear what is the best solution to cope with those packages
that are available in newer JDKs.

Sorry it is too late to change the orbit version of this bundle.
Otherwise I would think that corrections in the way that it is packaged are
most welcome.
Maybe it should look more like the javax.transaction bundle:
Fragment-Host: system.bundle
Export-Package: javax.transaction.xa;version="1.1",
 javax.transaction;version="1.1"

Although that requires everyone to use
Import-Package: javax.transaction;version="1.1.0"

For jetty we consume the javax.annotation package and we need to work both when
the boot-delegation is on and off.
Comment 12 Paul Webster CLA 2011-06-03 07:25:39 EDT
(In reply to comment #11)
> Sorry for the trouble Paul and thanks for formulating the issue so clearly.
> I volunteered to package javax.annotation for Indigo.

It's just one of those OSGi things (modularity is hard :-)

> Otherwise I would think that corrections in the way that it is packaged are
> most welcome.
> Maybe it should look more like the javax.transaction bundle:
> Fragment-Host: system.bundle
> Export-Package: javax.transaction.xa;version="1.1",
>  javax.transaction;version="1.1"
> 


Tom, I see javax.transaction set itself up using "Fragment-Host: system.bundle".  Does that mean even javax.annotation 1.0 is wrong, since it says "Require-Bundle: system.bundle"?

PW
Comment 13 Thomas Watson CLA 2011-06-03 08:38:57 EDT
(In reply to comment #12)
> (In reply to comment #11)
> 
> Tom, I see javax.transaction set itself up using "Fragment-Host:
> system.bundle".  Does that mean even javax.annotation 1.0 is wrong, since it
> says "Require-Bundle: system.bundle"?
> 
> PW

The Fragment-Host system.bundle approach works nicely if all you are doing is adding more types to what is provided by the VM.  The javax.transaction package is the classic case where the VM provides a very limited subset of the package (I think the VM only provides a few exception types that are thrown by other packages in the VM).

I don't think the javax.annotation package falls into this category.  Especially the 1.1 version since it needs add more attributes to an existing type (or annotation class).  The only way it can do that is to be its own bundle.  If it were to be a fragment of the system.bundle then the following delegation would occur:

   javax.annotation class load
               |
               V
    VM boot class path (Java5; nothing found, Java6; find 1.0; Java7; find 1.1)
               |
               V
     Framework class path (find content from javax.annotation fragment)

As you can see this approach would probably be ok for the 1.0 version of javax.annotation because we would either:

1) On Java 5 or less; find the content from the fragment
2) On Java 6; find the content from the VM and use it as 1.0
3) On Java 7; find the content from the VM and use it as 1.0 even though the VM provides 1.1 version of the package.

This would not be a good idea for the 1.1 version of the javax.annotation bundle when on Java 6 because the VM classes would always be loaded first and override what the fragment bundle provides.
Comment 14 Paul Webster CLA 2011-06-03 10:02:39 EDT
Created attachment 197302 [details]
import package v01

This patch switches all uses of javax.annotation and javax.inject to import package, version="1.0.0"

It also sets our org.eclipse.e4.rcp and org.eclipse.e4.ui.feature to include the javax.annotation plugin at 1.0.0.qualifier

PW
Comment 15 Paul Webster CLA 2011-06-03 10:03:36 EDT
Remy, Tom, could you please review this patch?

Thanx,
PW
Comment 16 Remy Suen CLA 2011-06-03 11:05:42 EDT
(In reply to comment #15)
> Remy, Tom, could you please review this patch?

Looks good to me after having picked Tom's brain for classloading intel.

I noticed we didn't change the javax.inject stuff in the feature.xml files but since there's only one lying around in Orbit it seems okay.
Comment 17 Thomas Watson CLA 2011-06-03 11:08:26 EDT
I found missing Import-Package statements for javax.annotation and javax.inject on:

org.eclipse.e4.ui.workbench.renderers.swt.cocoa

Otherwise patch looks good.
Comment 18 Thomas Watson CLA 2011-06-03 11:09:59 EDT
Forget about org.eclipse.e4.ui.workbench.renderers.swt.cocoa it is a fragment of org.eclipse.e4.ui.workbench.renderers.swt which correct imports the packages.  There is no need to add additional imports since they are already imported from the host.

+1 to patch.
Comment 19 Paul Webster CLA 2011-06-03 11:38:12 EDT
Released for I20110603-1145
PW
Comment 20 Patrick Paulin CLA 2013-05-16 16:19:06 EDT
Created attachment 231119 [details]
Sample app showing PostConstruct issue

This problem has reappeared for me on Kepler releases. I had no problems with Juno, but after switching my target to Kepler M7 I need to add an Import-Package statement for  javax.annotation to my bundles. If the import is missing, the PostConstruct annotation cannot be found (isAnnotationPresent returns false).

Running on Java 6, Mac OS X. Also using the e4 Bridge, but I doubt this is relevant to resolving the PostConstruct class. I've uploaded a sample RCP app that causes the problem to occur for me. Remove the javax.annotation import to reproduce.
Comment 21 Paul Webster CLA 2013-05-16 16:28:26 EDT
(In reply to comment #20)
> Created attachment 231119 [details]
> Sample app showing PostConstruct issue

Yes, you must use import-package for javax.annotation and javax.inject or we won't find @PostConstruct.  It's required.

PW
Comment 22 Patrick Paulin CLA 2013-05-16 16:59:08 EDT
(In reply to comment #21)
> Yes, you must use import-package for javax.annotation and javax.inject or we
> won't find @PostConstruct.  It's required.
> 
> PW

Is there a reason this "just worked" in Juno? I'm guessing this is going to come up pretty frequently as people migrate to Kepler.
Comment 23 Olivier Prouvost CLA 2013-05-16 17:04:38 EDT
Thank you for this crucial information ! 

Paul, do you know if there is a documentation that mention this trick ? I could not find it when I wrote my talk about E4 injection or my trainings...
Comment 24 Patrick Paulin CLA 2013-05-16 17:09:15 EDT
(In reply to comment #21)
> Yes, you must use import-package for javax.annotation and javax.inject or we
> won't find @PostConstruct.  It's required.
> 
> PW

FWIW, I didn't have to add an import for javax.inject to get the @PostConstruct working, which makes sense. 

Are you saying that e4 developers should just get used to adding those two imports so that all of the Java annotations are available?
Comment 25 Olivier Prouvost CLA 2013-05-16 17:25:55 EDT
(In reply to comment #21)
> (In reply to comment #20)
> > Created attachment 231119 [details]
> > Sample app showing PostConstruct issue
> 
> Yes, you must use import-package for javax.annotation and javax.inject or we
> won't find @PostConstruct.  It's required.
> 
> PW


If we create an E4 Application Project with some content (using the wizard), only the import-package is generated for javax.annotations and the javax.inject is declared as a required plugin. 

Is that right ?
Comment 27 Olivier Prouvost CLA 2013-07-11 10:31:47 EDT
I fixed the wiki yesterday !! The javax.annotation must be imported as a package and with version 1.1.0. 

With Kepler and the latest E4 tools the default E4 application project import javax.annotation with release 1.0.0 and when you run the application, the @PostConstruct is still no called !  

It is due to this version number !

I will open an bug for that.