Bugzilla – Full Text Bug Listing
| Summary: | JSR 330 @Inject in e4 | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Sven Efftinge <sven.efftinge> |
| Component: | Runtime | Assignee: | platform-runtime-inbox <platform-runtime-inbox> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | bokowski, eclipse, eclipse, felix, gunnar, jason, ob1.eclipse, pwebster, sebastian.zarnekow, tom.schindl |
| Version: | 4.0 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Mac OS X - Carbon (unsup.) | ||
| See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=500688 | ||
| Whiteboard: | stalebug | ||
| Bug Depends on: | 331746 | ||
| Bug Blocks: | |||
|
Description
Sven Efftinge
(In reply to comment #0) > Follow up to one aspect of my dependency injection talk at ESE and Boris' > friendly reminder on twitter I'ld like to raise a discussion about the usage of > the JSR330 @Inject annotation in e4. Somehow related to bug 331235. (In reply to comment #0) > The problematic situation is, that whenever such a bundle contributes a > component to the e4 workbench , > the e4 "dependency injection" will think that all @Inject annotations are meant > to be resolved. But that is not the case, they have been defined to construct > and compose the object in the first place (duty of the contributor). When it is > handed over to e4 it should only add the workbench specific things, which are > usually dynamic aspects, such as listeners. > > The situation could be easily solved, if e4 would use a different annotation. > Boris suggested @Dynamic, but any other annotation would solve the issue. At work, to get dynamic bundles working, we did the opposite: what we contributed (dynamic OSGi services that were inappropriately handled by the Eclipse Context's DI) was annotated with @Dynamic (see bug 317706) so that our injectors could match it. The current debate is whether there should be annotations or not and one of the arguments for the absence of annotations is the compatibility with JSR330 frameworks so that one could run injected POJOS wherever they like. If e4 uses the @Inject annotation, DI containers like Guice would refuse to instantiate the object in the first place since it cannot resolve all the @Inject annotated values that are meant to be "injected" by the eclipse context in a subsequent step. For me that's the worst aspect of using the @Inject annotation. Instead of @Dynamic I'd prefer something that describes what this thing actually is, e.g. @SelectionListener or @FromEclipseContext ;-) (In reply to comment #2) > > Instead of @Dynamic I'd prefer something that describes what this thing > actually is, e.g. @SelectionListener or @FromEclipseContext ;-) But most of what is injected is simply information available from the DI container (in our case the IEclipseContext). @Inject private MApplication app; @Inject private MPart myPart; PW The purpose of us conforming to a standard such as JSR 330 was so that the same POJOs could be used across multiple injection frameworks with consistent behaviour. If we used non-standard annotations that would seem to defeat the value of having a standard in the first place. Put another way, you're suggesting there can only be one framework implementing JSR 330 (Guice in your case). I have trouble finding concrete information on it, but can't the JSR 330 @Qualifier annotation be used to help here? I.e., when a client provides an object to be injected, they could specify a qualifier, so that each injector only injects fields/methods with a matching qualifier. I might be misunderstanding the purpose of @qualifier though. I guess the other approach is that clients avoid presenting the same object for injection to multiple injection frameworks. This could be just good separation of concerns - i.e., the same object that represents a view/editor perhaps shouldn't also represent something else injected elsewhere. If I understand the comment 0, the core question is how two different JSR330-compliant DI frameworks should (can?) co-exist. One answer would be, as John suggested, not to mix them in the same class. That might be the safest answer as the JSR does not define lifecycle issues and we undoubtedly going to be bitten them should we start mixing frameworks. A more interesting solution would be to see if we can provide a DI supplier that would be wrapping (or based on Guice injector) to be used as a secondary supplier, similar to ContextInjectionFactory#make(clazz, context, => staticContext <= ) To me, this is potentially more rewarding approach; the downside is that it is unlikely that we'll be able to match all corner cases. At any rate, a very good question, IMO worthy of another JSR. Doesn't the URL used defining which injection framework is used? Currently platform:/pluginid/class (which is wrong anyways) is handled by our DI-Framework. I think we anyways want to make this system more flexible e.g. to make things work for contributions written e.g. in JavaScript. So my proposal would be that we make the URL system extensible and people can then contribute their own DI implementation. So we'd have URLs like this in our model: * guice://mybundle/myguiceclass * e4di://mybundle/mye4class * e4js://mybundle/mye4script.js (In reply to comment #6) This would be a good first step, but I think the request was to allow a different from @Inject (for things that are being set dynamically as they changed over time) so that Guice could be used for creating the objects, but then things like the current selection could still be coming from e4 if it called the @Dynamic methods. Sven, please correct me if I am wrong, or if I understood this correctly, maybe you could go into more detail on how this could work? (In reply to comment #4) > The purpose of us conforming to a standard such as JSR 330 was so that the same > POJOs could be used across multiple injection frameworks with consistent > behaviour. If we used non-standard annotations that would seem to defeat the > value of having a standard in the first place. Put another way, you're > suggesting there can only be one framework implementing JSR 330 (Guice in your > case). The intend behind JSR 330 is not to use multiple dependeny injection frameworks at the same time, but to be able to reuse a certain class, which only relies on annotations from JSR330 to be used in with a different DI container. For every object only one DI container is responsibly for the creation and initialization. e4 assumes that it is always responsible for construction of any contributions. I've filed bug 331746 describing that issue. e4 can't configure my objects, because it doesn't and shouldn't know about the internals of my contributions. The way we use DI is to have one container per bundle, which knows a lot of internals you wouldn't want to expose into a global IEclipseContext. > > I have trouble finding concrete information on it, but can't the JSR 330 > @Qualifier annotation be used to help here? I.e., when a client provides an > object to be injected, they could specify a qualifier, so that each injector > only injects fields/methods with a matching qualifier. I might be > misunderstanding the purpose of @qualifier though. No that wouldn't work. @Qualifier is just a means to create new keys. A DI Container would complain if it cannot find a binding for a certain key and you don't want to declare all your dependencies optional. > I guess the other approach is that clients avoid presenting the same object for > injection to multiple injection frameworks. This could be just good separation > of concerns - i.e., the same object that represents a view/editor perhaps > shouldn't also represent something else injected elsewhere. I think e4 shouldn't force every contributor to give up control over construction and initialization of objects. (In reply to comment #7) > (In reply to comment #6) > > This would be a good first step, but I think the request was to allow a > different from @Inject (for things that are being set dynamically as they > changed over time) so that Guice could be used for creating the objects, but > then things like the current selection could still be coming from e4 if it > called the @Dynamic methods. Yes, exactly. I really think the core question is what component is responsible for constructing objects. The approach in Equinox works quite well. If I write the contribution like platform:/my.bundle/my.Factory#my.Part which would be the equivalent to my.Factory:my.Part in plugin.xml. And the Object created by e4 does have a method annotated with @Create (or whatever you like better), than e4 should call that method and take the return value as the contribution. The fragment (my.Part) could be passed as an additional argument, or you separate the factory and the setting of configuration data like in Equinox. In that case an additional annotation like @ConfigurationData might be the e4 style. E4 should of course ignore any @Inject annotations and only invoke the callbacks (lifecycle, listeners, actions, etc.) if the object was constructed through an @Create method. In that case one might stick to the JSR 330 annotation, although I really think the way e4 does DI is not what the people behind JSR330 had in mind. Also e4 classes will never work with other DI frameworks, because there are so many additional annotations and the @Inject is used so differently (dynamically). IMHO it would be best to also change the annotation. I don't think we should encode anything like Guice into the scheme of the URI, since the Equinox approach is much more flexible and doesn't expose such implementation details. Also you would have to worry about different URIHandlers which again would be global. Could the Guice JSR-330 implementation not be used in e4? Is there a specific reason it was eliminated for consideration? I sincerely believe that in this regard I can speak from hard-won experience about the issues that may arise trying to write a DI mechanism. There's the community aspect and the technical aspects. I believe that the Guice SPIs would allow for anything you might want. At Sonatype we were able to entirely replace a pre-existing DI framework and mimic its behavior so that all existing component implementations and metadata worked without change when the container was swapped out. I really think you could add more value to e4 if you didn't have to worry about the DI mechanism. Some food for thought: http://www.sonatype.com/people/2010/01/from-plexus-to-guice-1-why-guice/ http://www.sonatype.com/people/2010/01/from-plexus-to-guice-2-the-guiceplexus-bridge-and-custom-bean-injection/ http://www.sonatype.com/people/2010/01/from-plexus-to-guice-3-creating-a-guice-bean-extension-layer/ (In reply to comment #10) > And the Object created by e4 does have a method annotated with @Create (or > whatever you like better), > than e4 should call that method and take the return value as the contribution. > The fragment (my.Part) could be passed as an additional argument, or you > separate the factory and the setting of configuration data like in Equinox. In > that case an additional annotation like @ConfigurationData might be the e4 > style. I really like this approach (or the way Tom proposed it). It's true it's better if we don't have to register an URI handler for an injector factory to E4, so I like specifying a factory very much. This factory could delegate the creation to a 3rd party bundle so we can reuse the same Injector factory in different bundles. > E4 should of course ignore any @Inject annotations and only invoke the > callbacks (lifecycle, listeners, actions, etc.) if the object was constructed > through an @Create method. I'm not sure about that. E4 should not manage/call methods not created by its own "stock" DI factory (at least not the annotations). This way, when you want to create a part or an addon with E4, you would specify a factory: - the default factory does things like today. (and maybe in tooling it could be the "regular" mode). - user-supplied factories are charged with creating the object *AND* managing its lifecycle (and maybe in tooling custom factories would be in an "advanced mode") Of course, those factories would probably delegate the work to JSR330 compliant frameworks like Guice with Peaberry or Spring. In order to get E4 events and context-based injection into your custom-factory managed object, it would be up to YOUR custom factory to set-up Context RunAndTracks to notify changes. E4 should not go out of its way to do this. This way, it's also up to the custom-factory to decide WHICH annotations specify E4 specific elements (e.g, @E4, @Dynamic or whatnots). We could provide standard set of annotations so that different custom-factories using different JSR330 containers can be made to work on the same POJO. Of course, if we were to do this, I'm sure someone (like me?) would make a Guice+Peaberry E4 compatibility mode with context values injections available and one would just have to get it :). > In that case one might stick to the JSR 330 annotation, although I really think > the way e4 does DI is not what the people behind JSR330 had in mind. Also e4 > classes will never work with other DI frameworks, because there are so many > additional annotations and the @Inject is used so differently (dynamically). > IMHO it would be best to also change the annotation. Peaberry does call @Inject dynamically on ServiceEvents so I'm not sure this is that different; it's only that a lot of JSR330 code seems to be about dealing with static stuff. > I don't think we should encode anything like Guice into the scheme of the URI, > since the Equinox approach is much more flexible and doesn't expose such > implementation details. Also you would have to worry about different > URIHandlers which again would be global. Agreed to avoid having to register anything with the framework. This setup could be painful (especially ordering). === Summary === - Allow to delegate object construction *and* management to custom factories - Provide default factory that behaves like we have currently - A factory should be passed the EclipseContext so that it can register runAndTracks - The EclipseContext#get method should only look *into* the context (and not go out of its way) to allow this kind of composition. == Open questions == - Would the custom factory have everything it needs to operate? (In reply to comment #11) > Could the Guice JSR-330 implementation not be used in e4? Does Guice support changes in the injected data? Say, I had a binding (key, ObjectA) which was injected into MyObject. When the binding was updated to be (key, ObjectB). Would injected value be propagated into MyObject? (In reply to comment #13) > Does Guice support changes in the injected data? > > Say, I had a binding > (key, ObjectA) > which was injected into MyObject. When the binding was updated to be > (key, ObjectB). > Would injected value be propagated into MyObject? Yes. Guice doesn't support it directly, but Peaberry is a Guice extension that provides this support for all kinds of dynamic behaviors. It also provides the support for OSGi services but Peaberry is agnostic in term of what dynamic data it will inject. (In reply to comment #9) > The intend behind JSR 330 is not to use multiple dependeny injection > frameworks at the same time, but to be able to reuse a certain class, > which only relies on annotations from JSR330 to be used in with a > different DI container. Yes! So if e4 used different annotations, then a POJO using JSR-330 annotations could not be injected by Eclipse 4 contexts. > I really think the core question is what component is responsible for > constructing objects. The approach in Equinox works quite well. It would be great if we could find a similar way for e4 contributions to specify what their factory is, like we do with the extension registry. However when you throw in injection I'm not sure how this will work. The container needs to have access to all the values the need to be supplied to the client object. For example a part contribution needs a Composite injected that indicates the part's parent. The only container that will know the appropriate parent is the e4 container, so if construction was delegated to an alternate container I don't know how such values would be connected through to the client object. > In that case one might stick to the JSR 330 annotation, although I really > think the way e4 does DI is not what the people behind JSR330 had in mind. The initial implementation of e4's injection was written by a member of the JSR 330 expert group, so I think he knew what he had in mind ;) However we're all aware that JSR 330 falls short since it does not specify anything related to dynamic behaviour, uninjection, etc. So, all containers that perform dynamic injection are forced to fill in the gaps here. (In reply to comment #14) > (In reply to comment #13) > > Does Guice support changes in the injected data? > Yes. > Guice doesn't support it directly, but Peaberry is a Guice extension that > provides this support for all kinds of dynamic behaviors. Hmm... Peaberry went through some design changes and I might be out of synch with it; the last time I looked, it used Provider<T> to do that - effectively adding a level of indirection. If you were to get a dynamic service, you were getting a provider for that service that you can query to get the current value of the service. Actually, from its design page http://code.google.com/p/peaberry/wiki/DetailedDesign: "... we won't be using Guice to inject the service itself, but rather Guice will inject a proxy that uses the lookup API to delegate calls to actual services." In any case, I am sorry that I took the bait and tried to answer the question "why did not you used project ABC instead"? :-). (In reply to comment #15) > > I really think the core question is what component is responsible for > > constructing objects. The approach in Equinox works quite well. > > It would be great if we could find a similar way for e4 contributions to > specify what their factory is, like we do with the extension registry. However > when you throw in injection I'm not sure how this will work. The container > needs to have access to all the values the need to be supplied to the client > object. For example a part contribution needs a Composite injected that > indicates the part's parent. The only container that will know the appropriate > parent is the e4 container, so if construction was delegated to an alternate > container I don't know how such values would be connected through to the client > object. My suggestion was to have an annotated factory method, where the parameters are provided by the EclipseContext. Which could in the extreme case be the EclipseContext itself. E.g. @Create MyPart createPart(IEclipseContext ctx) { ... } Shouldn't that be enough? (In reply to comment #16) > (In reply to comment #14) > > (In reply to comment #13) > > > Does Guice support changes in the injected data? > > Yes. > > Guice doesn't support it directly, but Peaberry is a Guice extension that > > provides this support for all kinds of dynamic behaviors. > > Hmm... Peaberry went through some design changes and I might be out of synch > with it; the last time I looked, it used Provider<T> to do that - effectively > adding a level of indirection. If you were to get a dynamic service, you were > getting a provider for that service that you can query to get the current value > of the service. Which is much nicer than being called occasionally and getting null pointers where just a statement before a value was set. Dynamic unset and reset seems pretty invasive. Instead a provider is super simple and one naturally obtains an instance checks it for null and afterwards work with the local variable. In addition to providers one could use proxies, which would still not change the reference, but would cause the same surprises a dynamic unset or reset could cause. Two additional remarks about dynamic injection and unjection: Listeners are not Dependency Injection. That is I really want to listen for an event, i.e. the callback, therefore I *want* dynamic invocation and am not necessarily interested in storing the event data locally. Although I really like the programming model we have for listeners in e4 it is mixed up with DI. Also all the other callbacks like the lifecycle stuff or the action handlers are not dependency injection. They are just call backs identified using annotations. Regarding dynamic OSGi services: If my code needs some OSGi service it should obtain and track it through my bundle context and not a third, unrelated, global context (i.e. e4). There might be services and implementations which don't have anything to do with UI or e4 at all. I have to admit that I don't know e4 enough, so I don't know i what other situations the dynamic injection is important. (In reply to comment #17) > My suggestion was to have an annotated factory method, where the parameters are > provided by the EclipseContext. Which could in the extreme case be the > EclipseContext itself. I see you've raised bug 331746 for this, so I'll follow up there. It sounds promising. I orgiginally searched for the "ExtensionFactory" mechanism known from E3 for parts configured in the E4 application model. I currently use this to create views and command handlers using spring and to inject for example remote services or proxied objects with a security aspect wrapped around the "original" bean. Reading through the comments of this bug I like to comment that I find the possibility to inject part of the attributes from one DI container (for example spring) and anothert part of attributes from another container (for example E4) quite confusing. What might help at least me is to delegate or enhance the resolution of "dependencies" needed for creation and configuration of a bean from one container to a second one. The E4 container can be responsible for creation and lifecycle of parts but inject beans created/provided by another DI container, for example a spring application context. That might be achieved in a perhaps limited way by simply injecting all beans from spring application context to the eclipse context after creation of the spring context. Although I am not sure in which order things are done. The spring osgi extender listens for bundle lifecycle events and creates the app context whenever a "spring bundle" reaches the resolved (?) state. When I remember it correctly the app context is published as an osgi service. So a second "spring E4 extender" could detect the creation of application context osgi services and inject the beans in the E4 context so the spring beans can be injected in by the normal E4 injection into parts and handler and whatever. This would lack the possibility to use the bean created by E4 (for example the handler or view) as a spring bean and, for example, to decorate it with proxy or to inject it elsewhere but thats not a common usecase anyway - at least for me. I am not sure about the "context" of the E4 context and the hierarchy and as I mentioned about the order of execution. Is it possible to get the E4 context for a bundle using some framework methods. I think it should. But will the listener for the osgi service that detects the context and injects spring beans into the E4 context early enough so the E4 DI can use the beans for part creation? Or is there a better hook? This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |