google-code-export / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
2 stars 1 forks source link

Document optional dependencies and child injectors #282

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I've found a case where optional dependencies (@Inject(optional = true))
may not be satisfied if the class with the dependencies is built with a JIT
binding and there's a parent injector.

What seems to happen is that the just-in-time binding bubbles up to the
parent module, but the class to satisfy the optional dependency is bound in
the child. The parent can't get to the dependency in the child (the
state#isBlacklisted check fails) and an error occurs.

Unfortunately, since the dependency is optional, the error is ignored and
the SingleMethodInjector is then discarded. If the dependency is required,
things succeed (I haven't tracked this further, but it seems it triggers a
retry?).

Attached some simple test cases that show that the problem occurs when
child injectors and optional dependencies come together.

Found in the snapshot20081123 release.

Btw, if you can advise on a strategy for solving this (doing the JIT in the
child? propogating the optional error further rather than ignoring it?) I'd
be happy to take a stab at a patch.

Original issue reported on code.google.com by phopkins on 13 Dec 2008 at 7:28

Attachments:

GoogleCodeExporter commented 9 years ago
Unfortunate as it is, this is the intended behaviour. You can work around this 
by adding the following binding 
to the child module:
  bind(MyTypeWithOptionalInjections.class);

From the Injector.createChildInjector() Javadoc:
Just-in-time bindings created for child injectors will be created in an 
ancestor injector whenever possible. 
This allows for scoped instances to be shared between injectors. Use explicit 
bindings to prevent bindings 
from being shared with the parent injector.

No key may be bound by both an injector and one of its ancestors. This includes 
just-in-time bindings. The 
lone exception is the key for Injector.class, which is bound by each injector 
to itself.

I'll add to the Javadoc to explicitly mention optional injection points, which 
are naturally surprising here.

But I'm not going to change the behaviour. If I did so, you'd get different 
behaviour depending on which 
injector you asked first - the child or the parent. Suppose we had this code:
  public class HanSolo {
    @Inject(optional=true) Wookie wookie;
  }
  interface Wookie { ... }

When we ask the parent injector first, we get null wookies from both parent and 
child injectors. This is 
because we'd create the binding in the parent, and Guice only allows one 
binding for each type in the injector 
hierarchy:
  Injector parent = Guice.createInjector();
  Injector child = parent.createChildInjector(new ChewbaccaModule());
  parent.getInstance(HanSolo.class); // yields a HanSolo with a null wookie
  child.getInstance(HanSolo.class); // yields a HanSolo with a null wookie

If we were to treat optional injection points as special when child injectors 
exist, we run into problems when 
we ask the child first. 
  Injector parent = Guice.createInjector();
  Injector child = parent.createChildInjector(new ChewbaccaModule());
  child.getInstance(HanSolo.class); // yields a HanSolo with a non-null wookie
  parent.getInstance(HanSolo.class); // throws, since parent cannot clobber a child's binding

To avoid this situation where the order in which requests are made impacts 
their results, we have to create the 
binding in the topmost injector. For this example, what the workaround does is 
it blocks the parent injector 
from creating the HanSolo binding in the first place:
  Injector parent = Guice.createInjector();
  Injector child = parent.createChildInjector(new ChewbaccaModule(), new HanSoloModule());
  parent.getInstance(HanSolo.class); // throws, since parent cannot clobber a child's binding
  child.getInstance(HanSolo.class); // returns a HanSolo with a non-null wookie

I'm sorry that this behaviour is frustrating, but I believe the alternative is 
worse.

Original comment by limpbizkit on 15 Dec 2008 at 10:14

GoogleCodeExporter commented 9 years ago

Original comment by limpbizkit on 26 Apr 2009 at 9:51

GoogleCodeExporter commented 9 years ago
Issue 392 has been merged into this issue.

Original comment by limpbizkit on 17 Jun 2009 at 4:13

GoogleCodeExporter commented 9 years ago
r1502 adds a sentence to the javadoc mentioning odd things may be afoot.  

Original comment by sberlin on 20 Feb 2011 at 10:09