Tracked at MAGNOLIA-6027 - Getting issue details... STATUS
Rationale
Guice (and Magnolia's current usage of it) doesn't allow multi-binding out of the box. Guice forces us to register every component with a unique key (the <type>
element in our module descriptors), and this key is used to resolve injection. If you register FooBar
with the Foo
key, where class FooBar implements Foo
for example, you can only inject FooBar
by declaring a dependency to the Foo
interface.
This is a problem if a component needs to be aware of multiple implementations of a common interface, which is something that's often desirable in a plugin-based environment like Magnolia; there are cases where multiple implementations of an interface should be contributed by additional modules. What we've been doing so far in such cases is either
- ignore pluggability and "hard-code" the list of known implementations
- force configuration of a list of implementations where configuration or order shouldn't matter
- ignore the I in IoC and have each implementation register themselves with an injected "manager"
This means one can't "discover" all instances of a certain type in the container.
Current use-cases and workarounds we've implemented so far
Current non-use-cases
Here are some cases where multi-binding is not what we need and where the current solution actually makes sense. Common reasons are "order matters" (and needs to be controlled/configured) or "further configuration of components is required".
- Filters. Order of filters need to be controlled and predictable. Module dependency order is not enough.
- REST endpoints (this is arguable, because mappings are (or can be) configured via annotations, and further configuration might not be needed or could be delegated to a module's own configuration)
- Traits in Personalization - filters (setting the trait) need configuration and order
- Registries (templates, dialogs, ...) are typically for configured items.
Current use-cases with workarounds
Essentially, this feature is for components which are declared as <component>
in a module descriptor and can be injected.
ConfigurationSourceFactory
: ideally,ConfigurationSource
and ConfigurationSourceBuilder
implementations should not be limited to those in themagnolia-configuration
module. We would like to be able to decouple them and have other modules contribute more or different implementations.RegistryFacade
: the implementations' constructor takes a set ofRegistry
implementation as an argument, but the component itself is not instantiated by Guice ! It is instead instantiated "manually" at its usage point (currently the config overview app), by explicitly calling it with the known registries. This means that the current implementation will never know about registries that are not depended upon by the UI (e.g REST endpoints, Traits, ...)- Note that a) the
Registry
interface is actually new in 5.4, but b) each implementation is still registered with the key they used in prior versions, e.gTemplateDefinitionRegistry
. This is why it's still possible to simply depend on (inject)TemplateDefinitionRegistry
.
- Note that a) the
- Links: there are actually no real work arounds at the moment, but link generators would benefit from this; modules could register their own link generators(e.g content apps could register one that's able to deal with nodes form their own workspace, etc)
Proposed solution
This Guice extension enables multibinding: https://github.com/google/guice/wiki/Multibindings. It still requires code to do the binding, as always with Guice, nothing is magic; one has to explicitly bind each implementation to a common key.
The common type, which is neither the key or the implementation class of the component, could be annotated with @MultiBinding
. When registering components implement such an annotated interface, we would signal to Guice to do such a multiple-binding, using the annotated interface as the additional multi-binding key.
Example
Here's an example that avoids terms like "manager" and "registry", since it tends to make the whole topic confusing (because talking about a RegistryOfRegistriesManager can get confusing).
Given the following interface
public interface Fish { void feed(); }
Say we have module-a declaring this implementation of Fish
public interface PufferFish extends Fish { void pester(); } public class PufferFishImpl implements PufferFish { ... }
and module-b declaring this one:
public class GoldFish implements Fish { ... }
Both modules declare a <component>
in the module descriptor.
If we wanted to have an Aquarium
interface to feed all the fish known by our system, we'd have a dependency problem when trying to implement it, right ?
public interface Aquarium { void feedAll(); } public class AquariumImpl { private Collection<Fish> allTheFish; public AquariumImpl(...???) { .. } }
Unless AquariumImpl was provided by yet another module, which knows specifically about module-a and module-b... but typically, we'll want the Aquarium
in the same module as our Fish
interface. One (ugly) way around it would be to revert the inversion of control in every Fish
implementation:
public class GoldFish implements Fish { public GoldFish(Aquarium aquarium) { aquarium.addFish(this); } }
... this means every fish would be calling the aquarium to signal itself. Every actor would be calling their agent, despite IoC trying to enforce the Hollywood principle: "don't call us, we'll call you".
The suggested solution would enable the following:
@MultiBinding public interface Fish { void feed(); }
And an implementation of aquarium, declared as a component in the same module as the Fish
interface, would be enabled like the following, without having to know about which particular implementations exist in the system:
public class AquariumImpl { private final Set<Fish> allTheFish; public AquariumImpl(Set<Fish> allTheFish) { this.allTheFish = allTheFish; } public void feedAll() { for (Fish f : this.allTheFish) { f.feed(); } }
Notes
It should be noted that use-cases should not encourage depending on Set<Fish>
, but rather on Aquarium
(or some other abstraction around all the fish we know). There is (probably?) no way to prevent abusing the system other than through documentation and education.
The Set<Fish
> doesn't provide any order guarantee. I assumed it'd inherently be ordered by module dependency order, but I can't verify that by looking at the implement code of Multibinder
.
Keep in mind that components still get registered only with the provided key, and optionally-additionally, with a key of Set<WhateverWasAnnotatedWith@MultipleBinding>
; that means that
- if 2 or more
<component>s
are declared with<type>foo.bar.Fish</type>
, the latter still wins. Registration of components is done in module-dependency order, which is how we enable one module to override a component declared by another. - if all our fish is declared with their own keys (e.g
PufferFish
andGoldFish
), and another component declares a dependency to a single instance ofFish
, that dependency won't be resolved, since there is noFish
key known to Guice.
It is currently unclear what the implications are if some of the components are lazy and some not; presumably the "surrounding" component (aquarium) will get all implementations (and thus "wake up" the lazy ones). We also need to verify what happens with components in different scopes. I would however not advise usage of this feature for anything else that system-wide components for now anyway.
5 Comments
Jan Haderka
Re "notes", isn't
Or maybe I misunderstood what you meant by first sentence there.
Set<Fish>
calledschool
rather thenAquarium
?Seriously tho, it's very good write up that should ply end up in documentation and in training if we go for it. And yes I definitively agree it has its use cases.
I'm still bit uncertain about things we don't know for sure, like the order, or what happens when I one has something like
public class FishBowlImpl implements Aquarium
with ctor that takes only single fish sopublic FishBowlImpl(Fish fish)
... what would guice do? Barf that there's too many implementations/instances of fish to choose from or silently pick the first one (I hope not)?I'm sure there's even more questions like this that we would discover only during process of implementing it so I would indeed caution for very step wise approach when introducing this feature.
Magnolia International
What I mean is that we should encourage creation of "wrapping" components (such as the
Aquarium
) which expose functionality that use the set-of-fish, rather than create more components that depended directly onSet<Fish>
. Clearer ?Regarding order, I think we're fine with stating one shouldn't depend on it; that's already conveyed by the fact you're getting a
Set
and not aList
orSortedSet
.Regarding
FishBowlImpl(Fish fish)
it would simply not work (some Guice exception would be thrown) because nothing is registered withFish
as a key.Jan Haderka
Yes, thx, so I got it right first time around ... and I still think S
chool
is better wrapping container for fish thanAqua
, but agreed it's all just semantics and irrelevant to the discussion aboveGood, that's what I was hoping for. But that brings other question: How do you register those components then?
@Multibind
is onFish
so I (wrongly) assumed you would be registeringFish
as a key multiple times with all those different implementations. So what is it that you have to register? Nothing? Does guice automagically discovers all classes that implementFish
because of the@Multibind
annotation?Jan Haderka
And I see you edited the page to pretty much answer the question. Thx.
Magnolia International
See added note for the RegistryFacade example: "Note that a) the Registry interface is actually new in 5.4, but b) each implementation is still registered with the key they used in prior versions, e.g TemplateDefinitionRegistry. This is why it's still possible to simply depend on (inject) TemplateDefinitionRegistry." and added "Keep in mind" in the notes section, too. Also see the patch on MAGNOLIA-6027.
Basically, the registration of the component does not change (so you can still depend on the exact same key as before to get THE instance of that particular component type), but we additionally traverse the class hierarchy of the component-being-registered to see if any of its super types has the
@Multibinding
annotation, and use that type as the additional Set-Key.