diff --git a/core/src/com/google/inject/internal/InjectorImpl.java b/core/src/com/google/inject/internal/InjectorImpl.java index 9d0d271790..c870c36eb5 100644 --- a/core/src/com/google/inject/internal/InjectorImpl.java +++ b/core/src/com/google/inject/internal/InjectorImpl.java @@ -56,7 +56,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.Collections; -import java.util.HashSet; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -610,7 +610,7 @@ void initializeJitBinding(BindingImpl binding, Errors errors) throws Erro // so that cached exceptions while constructing it get stored. // See TypeListenerTest#testTypeListenerThrows removeFailedJitBinding(binding, null); - cleanup(binding, new HashSet>()); + cleanup(binding, new HashMap<>()); } } } @@ -622,13 +622,15 @@ void initializeJitBinding(BindingImpl binding, Errors errors) throws Erro * added to allow circular dependency support, so dependencies may pass where they should have * failed. */ - private boolean cleanup(BindingImpl binding, Set> encountered) { + private boolean cleanup(BindingImpl binding, Map, Boolean> encountered) { boolean bindingFailed = false; Set> deps = getInternalDependencies(binding); for (Dependency dep : deps) { Key depKey = dep.getKey(); InjectionPoint ip = dep.getInjectionPoint(); - if (encountered.add(depKey)) { // only check if we haven't looked at this key yet + Boolean failedBefore = encountered.get(depKey); + if (failedBefore == null) { // only check if we haven't looked at this key yet + encountered.put(depKey, false); BindingImpl depBinding = jitBindingData.getJitBinding(depKey); if (depBinding != null) { // if the binding still exists, validate boolean failed = cleanup(depBinding, encountered); // if children fail, we fail @@ -640,6 +642,7 @@ private boolean cleanup(BindingImpl binding, Set> encountered) { } } if (failed) { + encountered.put(depKey, true); removeFailedJitBinding(depBinding, ip); bindingFailed = true; } @@ -649,6 +652,9 @@ private boolean cleanup(BindingImpl binding, Set> encountered) { bindingFailed = true; } } + else if (Boolean.TRUE.equals(failedBefore)) { + bindingFailed = true; + } } return bindingFailed; } @@ -812,14 +818,16 @@ private BindingImpl createImplementedByBinding( final Key targetKey = Key.get(subclass); Object source = rawType; FactoryProxy factory = new FactoryProxy<>(this, key, targetKey, source); - factory.notify(errors); // causes the factory to initialize itself internally + // factory.notify(...) causes the factory to initialize itself internally + DelayedInitialize initialize = (injector, errors1) -> factory.notify(errors1); return new LinkedBindingImpl( this, key, source, Scoping.scope(key, this, factory, source, scoping), scoping, - targetKey); + targetKey, + initialize); } /** @@ -941,6 +949,7 @@ private BindingImpl createJustInTimeBinding( createUninitializedBinding(key, Scoping.UNSCOPED, source, errors, true); errors.throwIfNewErrors(numErrorsBefore); initializeJitBinding(binding, errors); + errors.throwIfNewErrors(numErrorsBefore); return binding; } diff --git a/core/src/com/google/inject/internal/LinkedBindingImpl.java b/core/src/com/google/inject/internal/LinkedBindingImpl.java index 87ead562d7..c41c6baa36 100644 --- a/core/src/com/google/inject/internal/LinkedBindingImpl.java +++ b/core/src/com/google/inject/internal/LinkedBindingImpl.java @@ -31,9 +31,10 @@ import java.util.Set; final class LinkedBindingImpl extends BindingImpl - implements LinkedKeyBinding, HasDependencies { + implements LinkedKeyBinding, HasDependencies, DelayedInitialize { final Key targetKey; + private DelayedInitialize delayedInitialize; LinkedBindingImpl( InjectorImpl injector, @@ -42,8 +43,20 @@ final class LinkedBindingImpl extends BindingImpl InternalFactory internalFactory, Scoping scoping, Key targetKey) { + this(injector, key, source, internalFactory, scoping, targetKey, null); + } + + LinkedBindingImpl( + InjectorImpl injector, + Key key, + Object source, + InternalFactory internalFactory, + Scoping scoping, + Key targetKey, + DelayedInitialize delayedInitialize) { super(injector, key, source, internalFactory, scoping); this.targetKey = targetKey; + this.delayedInitialize = delayedInitialize; } LinkedBindingImpl(Object source, Key key, Scoping scoping, Key targetKey) { @@ -51,6 +64,13 @@ final class LinkedBindingImpl extends BindingImpl this.targetKey = targetKey; } + @Override + public void initialize(InjectorImpl injector, Errors errors) throws ErrorsException { + if (this.delayedInitialize != null) { + this.delayedInitialize.initialize(injector, errors); + } + } + @Override public V acceptTargetVisitor(BindingTargetVisitor visitor) { return visitor.visit(this); diff --git a/core/test/com/google/inject/BinderTest.java b/core/test/com/google/inject/BinderTest.java index 2379e4c7c3..9b1cebd346 100644 --- a/core/test/com/google/inject/BinderTest.java +++ b/core/test/com/google/inject/BinderTest.java @@ -439,6 +439,27 @@ protected void configure() { }); } + /** + * Test bugfix of #700.
+ * Test, that {@code JustAClass} is not bound with JIT binding + * when {@code bind(HasImplementedByThatWantsExplicitClass.class}} is called before {@code bind(JustAClass.class)}. + *

+ * The error before the fix was: + *
+   * 1) [Guice/JitBindingAlreadySet]: A just-in-time binding to BinderTest$JustAClass was already configured on a parent injector.
+   *   at BinderTest$25.configure(BinderTest.java:458)
+   * 
+ */ + public void testJitDependencyCanUseExplicitDependenciesInAnyOrder() { + Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + bind(HasImplementedByThatWantsExplicitClass.class); + bind(JustAClass.class); + } + }); + } + /** * Untargetted bindings should follow @ImplementedBy and @ProvidedBy annotations if they exist. * Otherwise the class should be constructed directly. @@ -515,20 +536,23 @@ protected void configure() { } public void testBindingToProvider() { + String simpleName = null; try { - Guice.createInjector( - new AbstractModule() { + Module module = new AbstractModule() { @Override protected void configure() { bind(new TypeLiteral>() {}).toInstance(Providers.of("A")); } - }); + }; + String className = module.getClass().getName(); + simpleName = className.substring(className.lastIndexOf('.') + 1); + Guice.createInjector(module); fail(); } catch (CreationException expected) { assertContains( expected.getMessage(), "Binding to Provider is not allowed.", - "at BinderTest$28.configure"); + String.format("at %s.configure", simpleName)); } } @@ -662,6 +686,15 @@ static class ImplementsHasImplementedByThatWantsExplicit ImplementsHasImplementedByThatWantsExplicit(JustAnInterface jai) {} } + @ImplementedBy(ImplementsHasImplementedByThatWantsExplicitClass.class) + static interface HasImplementedByThatWantsExplicitClass {} + + static class ImplementsHasImplementedByThatWantsExplicitClass + implements HasImplementedByThatWantsExplicitClass { + @Inject + ImplementsHasImplementedByThatWantsExplicitClass(JustAClass jac) {} + } + static interface JustAnInterface {} // public void testBindInterfaceWithoutImplementation() {