Re: [e4-dev] OSGi, Contexts and DI

[Oleg Besedin <obesedin@xxxxxxxxxx>]

Hi Simon,
Thanks for the very thoughtful post.
As a service to others, I'd like to share my shorter digest of it with
few comments:

====== Digest start ======

Possible improvements to the context
injection story:

(1) create a way to define OSGi services
from e4's DI ("outjection").
(2) improve the consumption of OSGi
services using e4's DI
        (i)
All services injected in UI bundles in an E4 application are bound to only
one bundleContext;
         
      - Issue: OSGi framework can't determine consuming
bundles and can't properly handle dynamic services
         
      - [Oleg] Also issue: might be a problem in
the strict mode - need to check
        (ii)
It is impossible to filter on service properties when we get injected.

Proposed solution for (i):

- Modify context object supplier to
inject "proper" bundle context for
        @Inject
BundleContext bc;
- remove OSGi strategy, use specific
annotation (@Dynamic, or @Service, or @OSGi) to specify OSGi services to
be injected
        -
Issue: a lot of E4 framework code AND user code is currently depending
on OSGi service injection with no annotation
        [Oleg]:
Please, not "@Dynamic". All non-constructor injections in e4
are dynamic. OSGi strategy not dynamic at the moment (bug), but all injections
in methods and fields are supposed to be dynamic.
        [Oleg]:
We did not want users to care where the injected values are coming from.
For instance, if I have "@Inject ILog", I'd prefer not to worry
if the ILog service was provided via OSGi or in some other way.

====== Digest end ======

Those are very good and valid points.
I'd suggest creating two separate enhancement requests for (1) and (2).
For (2) we need to first settle on whether we use specific annotation for
injection of OSGi services.

Couple general things that I wanted
to clarify as they highlight places where e4's dependency injection is
different:

=> "In Peaberry Activation,
every bundle has a "Config" java file referenced in the Manifest
which says which services will be used (and exported)"

That is one of the points we are doing
differently (and, in my opinion, better) in the e4 DI: we removed the need
to have an extra file in custom format that ties values to be injected
with placed where to inject them. As everything in life, it has "pros"
and "cons" and might change in future. (One possibility is that
we'll have a file for "exporters" of services, but require only
annotations for "importers").

=> "The problem is that eclipse
contexts are finite: for a given key, a value should be there or not. The
dynamic nature of OSGi services make the keyset "infinite", because
the key is the interface name and a service may just "not be there
yet"."

I think you've been misled here by the
shortcomings of the current implementation of the OSGi strategy. While
both statements are correct, the combination implies that contexts injection
is not a good fit for OSGi services, which, in my opinion, is not correct.
 

In e4 injected values are dynamic; values
that changed in the context get propagated into injected fields/methods.
So, if OSGi strategy worked properly, the changes in the service implementations
would be propagated. Also, we have the "@Optional" annotation
which allows values not currently in the context to be injected as "null"
and re-injected later when the values are added to the context.


Again, great post, and I am looking
forward to continue specific discussion in the enhancement requests.

Sincerely,
Oleg Besedin

From:	Simon Chemouil <eclipse@xxxxxxxxxxxxxx>
To:	E4 Project developer mailing list <e4-dev@xxxxxxxxxxx>
Date:	10/25/2010 08:39 AM
Subject:	[e4-dev] OSGi, Contexts and DI
Sent by:	e4-dev-bounces@xxxxxxxxxxx

---

Hello,

I have previously discussed my interest in improving OSGi aspects in E4
but since my election as a committer I have been away first for 
holidays, and then with a lot of work on our product which left me 
little time to communicate much about my ideas. I wish to discuss them
before I start prototyping. I apologize for the length of the mail... 
and I'm open (and hoping) for any comment from other committers or OSGi
gurus.

Disclaimer: most of this are my findings on E4 as of late August. Things
have changed in the last two months but I had no time to keep up as much
as I would have wanted to. I believe most of it is still accurate, but
please tell me if it's not!

Currently, in an OSGi and E4 powered RCP application, the most common 
pattern I see is:
- definition of OSGi services through DS / Blueprint or programmatic API
(from an activator for instance)
- consumption of services in the UI using E4's DI support

That seems to leave us with two directions for improvements:
(1) a way to define OSGi services from E4's DI ("outjection").
(2) improving the consumption of OSGi services using E4's DI


I believe we already have many interesting solutions for (1) like 
declarative services, blueprint, or other DI frameworks that support 
outjection for OSGi services (ie, Guice + Peaberry Activation). In a 
future version it might be interesting to provide the functionality 
because some people might be interested in using E4 contexts even for 
bundles that are not related to the modeled workbench UI (we toyed with
that in our project, but another @Inject framework would probably be 
less risky for now).


So, in this mail I want to focus on the current limitations of (2) and
what I'd like to do to solve them:

(i) *All services injected in UI bundles in an E4 application are bound
to only one bundleContext.*

This context is created using the activator of the 
org.eclipse.e4.ui.workbench bundle, which means all services injected 
will be bound using this bundleContext.

I believe this goes against a design rule in OSGi best practices that 
contexts should not "leak" out of their bundle, and that you
should bind 
a service using YOUR context. If a bundle X wants to get a service 
injected, it should be referenced using its own bundleContext so that 
when the bundle is stopped, the service is properly unget'ed.

Of course, it's impossible to know if a user is following this best 
practice, but as a good OSGi citizen the Eclipse 4 framework should do
this in a cleaner way. Currently, it's also impossible to know which UI
bundle is using which service, and it makes it harder to manage the 
dynamic aspects of UI.


(ii) *It is impossible to filter on service properties when we get injected*

When we have more than one service implementing an interface, we can't
filter on properties or have an Iterable<ServicesInterface> to have
all 
the services injected. Peaberry Activation solves this problem ...:

- it provides annotations to filter on the services. Unfortunately this
is limited due to the constraints Java puts on annotation constants.
- it allows multiple services on an interfaces (+ filtering on 
properties using annotations) to be bound if the injected parameter is
of the type Iterable<ServiceInterface>. This is quite practical but
also 
tricky with the dynamics of OSGi services (Peaberry does some magic with
service variables accesses and throws exception when the service is not
bound, I believe this is done using aspects but I didn't look at the code).
- finally, if you can't get the service you want injected using those 
techniques, you can get your BundleContext injected, since it is always
put in the module scope. Then it's just good ol' programming against the
OSGi API.

Obviously E4's DI and Guice+Peaberry Activation have different 
approaches but we can get some inspiration.


So, a good first step that would address (i) and (ii) would be to allow
the current bundle's BundleContext to be injected, and make sure all 
injected services are bound using the appropriate bundleContext. Of 
course, it is already possible to retrieve the bundlecontext using 
FrameworkUtil.getBundle(getClass()).getBundleContext() but I believe 
users would like to avoid writing this if possible.


One approach would be to set the BundleContext, one way or another, in
the Eclipse Context.

Unfortunately, I think some changes are required in Eclipse Contexts to
allow this to be properly implemented.

The main issue is that Eclipse Contexts are hierarchical in "one 
dimension". Children contexts are used for different modeled UI elements
so that an UI part (such as MUILabel) can redefine a value for a key 
only for its "sub" context.

This hierarchy is following the model and thus can be shared between 
different bundles, depending on what bundle is implementing what UI 
part. The bundles may be split differently...

So if we are to put the bundleContext in the eclipse context, we should
somehow create a "child" context every time we are in a different
bundle, and it would unbalance the context tree. Unless we allow a 
secondary hierarchy for meta data on the contexts, this is not a good 
approach.

==== Proposed solution to allow the BundleContext to be injected

A more lightweight approach would be to allow injection of 
BundleContexts through the @Inject annotation (and in @Execute, 
@CanExecute, etc) using an ExtendedObjectSupplier, while not providing
it in the EclipseContext. This way, one could get the bundle context using:

-----------8<------------
@Inject public void setBundleContext(BundleContext bundleContext) {
  // do something
}
-----------8<------------
or simply
-----------8<------------
@Inject BundleContext bc;
-----------8<------------

but this would not work:
-----------8<------------
@Inject public void setBundleContext(IEclipseContext eclipseContext) {
  BundleContext bundleContext = context.get(BundleContext.class);
  // do something
}
-----------8<------------

This doesn't look to me as a big limitation.

The problem, in terms of code, is that (afaik) it's possible to provide
an ExtendedObjectSupplier only if you have a custom annotation as 
well... I'm not sure about that, but it could be changed.

I'm not sure there should be an @OSGi annotation for this, since I 
consider E4 is an OSGi framework and that it's one of its fundamentals.


I'm still investigating the code to get the actual BundleContext without
calling FrameworkUtil.getBundle() everytime. I'm open to suggestion ;)


==== Binding injected services to the proper bundle context

First, let's remember how service injection works in stock E4 currently.
When looking for a key in the context, if there is no value, the 
OSGiContextStrategy (that is provided in core.contexts) will try to find
a service reference for that key (treating it as an interface). If the
service reference is null, the context map will then cache "null"
to 
avoid any further lookup that may be costly. The problem is that dynamic
OSGi services, which were not there at first, will never be injected in
the context later on.

I believe OSGi services should never have been an "included"
strategy 
within core.contexts, which should really contain only stuff that is to
be shared at the "E4-layer" level. The problem is that eclipse
contexts 
are finite: for a given key, a value should be there or not. The dynamic
nature of OSGi services make the keyset "infinite", because the
key is 
the interface name and a service may just "not be there yet".
I'm not 
sure if I'm very clear here. The BundleContext could be at the eclipse
context level, because it's just one interface, though.

To resolve the problem of dynamic services, I have been using a @Dynamic
annotation provided by an ExtendedObjectSupplier that will (after a 
mandatory context lookup, unfortunately, because the context is always
the PrimaryObjectSupplier) only look in the OSGi Service Registry.


I believe the next steps to take are:
- removing the OSGiContextStrategy and most "OSGi" stuff in 
EclipseContexts (we won't need to build the context from an OSGi 
bundleContext either, since we want to bind the services on each 
bundle's bundlecontext). The only thing left should maybe be an implicit
injection of the BundleContext in the ContextObjectSupplier class (ie,
the EclipseContext won't contain the BundleContext but the since the 
PrimaryObjectSupplier is the ContextObjectSupplier for contexts created
by the modeled workbench, it can be interesting to return the 
BundleContext to DI asap).

- providing a @Dynamic or @Service (or @OSGi, even if I prefer @Dynamic
for various reasons) annotation for all OSGi services access (bound to
the proper bundleContext).

A big issue is that a lot of E4 framework code AND usercode is currently
depending on OSGi service injection with no annotation, as if it were 
provided by the context. Unfortunately, if we don't fix this soon (for
0.11) I'm afraid even more code will depend on it and a lot of cool 
stuff won't be easily done.

For instance, in our current project, using our custom @Dynamic 
annotation, we have some views that get enabled only when a service 
comes up (and we use databinding as soon as it comes up to bind it to 
the UI).

Why not keep the current OSGiContextStrategy and provide the annotation
for users who require this feature? Well, apart for some bugs and 
shortcoming, as I wrote, I believe the OSGiContextStrategy truly does 
not belong in Eclipse Contexts due to the fact that we can never know if
a key will exist later on, they are like a "black hole".

In Peaberry Activation, every bundle has a "Config" java file
referenced 
in the Manifest which says which services will be used (and exported),
so the @Inject works only on those services that have been explicitly 
marked as being dynamic/OSGi.

Since we don't have a similar config file, and that it's not really in
E4's philosophy, the @Dynamic annotation does about the same by 
explicitly marking an interface to be injected as an OSGi service.




=== Summary

Sorry if this was a long read (and for any mistake). I hope it's still
relevant.

I propose:
==> Removing a lot of OSGi stuff e4.core.contexts
==> Allowing BundleContexts to be injected "by default"
==> Providing an @Dynamic or @Service annotation to get all services
(and yes, if it's a service, you can't get it without the annotation).
==> Updating the framework so that it still works with the changes

Next:
- annotation properties to filter on OSGI properties
- 0..n service injection when type is Iterable<ServiceInterface>
?


I'll open a bug and start coding if other people are interested.

Please comment!

-- 
Simon

_______________________________________________
e4-dev mailing list
e4-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/e4-dev