Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [sisu-dev] [Bug 403108] Support sharing of AOP method interceptors between injectors

On 21 Jan 2014, at 15:34, bugzilla-daemon@xxxxxxxxxxx wrote:

> https://bugs.eclipse.org/bugs/show_bug.cgi?id=403108
> Product/Component: Sisu / Inject
> 
> --- Comment #8 from GianMaria Romanato <gian.maria.romanato@xxxxxxxxxxxx> ---
> (In reply to Stuart Mcculloch from comment #7)
> 
> Thanks once more for your very detailed answers. 
> Hope it is not a problem to continue the discussion here.

I’ve moved it to the dev list for now as I think a wider audience might appreciate the discussion, but feel free to open a new issue to capture your particular use-case

>> However there’s nothing stopping you from using a different mechanism in
>> your own extender that doesn’t require module packages to be exported, like
>> the service registry - or maybe even the locator itself (more on that below).
> 
> My application integrates the Equinox Registry and I thought about 
> using an extension point for declaring shared modules, which would then be
> collected by my custom extender.

BTW, you might be interested in https://bugs.eclipse.org/bugs/show_bug.cgi?id=386434 which will look into integration of extension points into Sisu (work on this will start soon)
The approach will be similar to the technique I used in peaberry (https://code.google.com/p/peaberry/wiki/EclipseRegistry) but updated with some of the lessons learnt in Sisu.

> But if I am not mistaken (and I'd really love to be proven wrong), the above
> would only partially solve the problem.
> My understanding is that a Guice Injector cannot be modified after creation,
> and as such if a bundle 
> contributes a shared module, that module can be used by the extender only for
> bundles that are started later, not
> for those that have been already started.

When it comes to sharing modules, this is true - the bundle (or fragment) sharing the module would have to be installed before the bundle using it starts. But while an Injector cannot be modified after creation, you can build some flexibility into the configuration. For example, the bindings added by the WireModule delegate at injection time to the BeanLocator. Similarly you could conceivably configure a generic type listener that looked up other type listeners from some kind of registry (ditto for method interception). This is not something we’d do by default, because you’d have to aggressively intercept everything upfront - but if you knew specifically what types would be involved then you could do something like this.

However, even if somehow you could add modules later on to an injector, you’d still have an ordering issue because any instances created before your module was installed would not participate in your custom injection scheme.

> Ideally, I would like a more flexible solution where shared modules are shared
> with any bundle,
> more or less the same way a service registration, once registered in the OSGi
> registry, is available to anyone
> regardless of the activation order.

I don’t think there’s a single sharing approach that will work for all Module elements - bindings are the (relatively) simple case, thanks to the lazy Provider API. Other elements such as AOP / custom injection are harder to share by the very nature of how they alter or interact with the classes and injector internals. As mentioned above one technique could be to register a generic type listener (by adapting the example extender code) and then use that to defer lookup of the actual type listeners to the extension or service registry. While not something that would be in the core of Sisu, you could look into making it a re-usable extension or add-on.

I still lean towards sharing modules via the extender/fragment pattern just because it self-documents the ordering/dependency.

>> BTW, another approach could be to use a custom WireModule strategy
>> (configured either in META-INF/services/org.eclipse.sisu.wire.Wiring or in
>> your custom extender) to hook into the wiring process and supply bindings
>> for custom injections. One benefit of the wiring approach is that your
>> client bundles can then use the standard @Inject instead of a custom
>> injection annotation.
> 
> I had a quick look at the WireModule strategy. I guess you are referring to the 
> possibility of using a custom Wiring implementation that will manage some Keys 
> in a custom way and route to the bean locator all other keys. 
> This looks interesting, but I think it is not a good fit for a custom
> injection. 
> In custom injections you often need a bit more context information (think of
> the
> Logger injection example in the Guice documentation, where the logger is named 
> after the containing class) which does not seem to be available in the Key. 
> When using type listeners in Guice you can register into the TypeEncounter 
> your own MembersInjector that is instantiated by your code and can be passed
> extra bits of information.

Correct, if you need context about the injection point then you will need to use custom injection - of course you can still share this setup via the extender pattern (or by customising the extender bundle if you prefer). But when you don’t need the context (or can work around the lack of context) then custom wiring can save you from having to explicitly write boilerplate/template binding statements - and can also help integrate with external systems.

>> We could consider recording any @Named module classes in the injector (in
>> addition to installing them) which would let you query them via the locator.
>> The sample extender still wouldn’t automatically share modules across
>> bundles, but you could use this information in your own custom extender to
>> share modules that had a specific marker interface or marker annotation.
>> 
>> Logged as https://bugs.eclipse.org/bugs/show_bug.cgi?id=426197
> 
> Not sure I understood what you are saying. 
> Does this mean that I could annotate my shared modules to identify them as 
> shared, and in my custom extender bind the shared modules when creating the 
> injector, so that every time a new injector is created at a later time I could
> use the bean locator to query for all the previously bound shared modules, 
> to obtain them all and to pass them all also to the new Injector?

That’s right - to be honest I’m still unsure how useful this might be in practice, but it would provide a record of what modules were installed while scanning for @Named items.

> If so, isn't this also affected by the problem that already started (and 
> already extended) bundles will not know new shared modules that are bound at 
> a later time?

True, but any solution will have some ordering issue whether at injector creation time or instance creation time...

>> Binding publishers can already choose whether to share bindings or not with
>> a given subscriber - if they don't want to share a binding then they can
>> choose to not call subscriber.add(Binding<T> binding, int rank); This
>> decision can be made per-binding, per-subscriber, etc. We could allow
>> injectors to declare their own BindingPublisher implementation if they want
>> rather than assuming it will always be InjectorPublisher (the default would
>> be InjectorPublisher if no publisher was bound in the injector). That should
>> then give you complete control over what bindings get shared with who.
> 
> This is something I could do in my custom extender, but it's not clear to me
> which is the relation between subscribers and previously extended bundles.

Any previously extended bundle will be subscribing to various types (they implicitly subscribe as they call the locate/watch methods of the BeanLocator) and as other extended bundles come and go they will notify these subscribers - which then update the associated component collections, notify mediators, etc. This is how Sisu supports dynamic component collections without requiring polling or back-references to the locator.

>> Thanks for the feedback, this is good stuff
> 
> Thank you for being always available to discuss.



Back to the top