fix provider circular dependency detection to use the Key it is creating, not the Key it is fulfulling, to catch errors sooner.
git-svn-id: https://google-guice.googlecode.com/svn/trunk@1544 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/core/src/com/google/inject/internal/BindingProcessor.java b/core/src/com/google/inject/internal/BindingProcessor.java
index 4b57f6a..524230e 100644
--- a/core/src/com/google/inject/internal/BindingProcessor.java
+++ b/core/src/com/google/inject/internal/BindingProcessor.java
@@ -100,7 +100,7 @@
Set<InjectionPoint> injectionPoints = binding.getInjectionPoints();
Initializable<Provider<? extends T>> initializable = initializer
.<Provider<? extends T>>requestInjection(injector, provider, source, injectionPoints);
- InternalFactory<T> factory = new InternalFactoryToInitializableAdapter<T>(
+ InternalFactory<T> factory = new InternalFactoryToInitializableAdapter<T>(key,
initializable, source, !injector.options.disableCircularProxies);
InternalFactory<? extends T> scopedFactory
= Scoping.scope(key, injector, factory, source, scoping);
@@ -112,7 +112,7 @@
public Boolean visit(ProviderKeyBinding<? extends T> binding) {
prepareBinding();
Key<? extends javax.inject.Provider<? extends T>> providerKey = binding.getProviderKey();
- BoundProviderFactory<T> boundProviderFactory = new BoundProviderFactory<T>(
+ BoundProviderFactory<T> boundProviderFactory = new BoundProviderFactory<T>(key,
injector, providerKey, source, !injector.options.disableCircularProxies);
bindingData.addCreationListener(boundProviderFactory);
InternalFactory<? extends T> scopedFactory = Scoping.scope(
diff --git a/core/src/com/google/inject/internal/BoundProviderFactory.java b/core/src/com/google/inject/internal/BoundProviderFactory.java
index 2a3d790..2988f16 100644
--- a/core/src/com/google/inject/internal/BoundProviderFactory.java
+++ b/core/src/com/google/inject/internal/BoundProviderFactory.java
@@ -30,11 +30,12 @@
private InternalFactory<? extends javax.inject.Provider<? extends T>> providerFactory;
BoundProviderFactory(
+ Key<T> key,
InjectorImpl injector,
Key<? extends javax.inject.Provider<? extends T>> providerKey,
Object source,
boolean allowProxy) {
- super(source, allowProxy);
+ super(key, source, allowProxy);
this.injector = injector;
this.providerKey = providerKey;
}
diff --git a/core/src/com/google/inject/internal/InjectorImpl.java b/core/src/com/google/inject/internal/InjectorImpl.java
index 715f933..e0361b4 100644
--- a/core/src/com/google/inject/internal/InjectorImpl.java
+++ b/core/src/com/google/inject/internal/InjectorImpl.java
@@ -678,7 +678,7 @@
= getBindingOrThrow(providerKey, errors, JitLimitation.NEW_OR_EXISTING_JIT);
InternalFactory<T> internalFactory =
- new ProviderInternalFactory<T>(providerKey, !options.disableCircularProxies) {
+ new ProviderInternalFactory<T>(key, providerKey, !options.disableCircularProxies) {
public T get(Errors errors, InternalContext context, Dependency dependency, boolean linked)
throws ErrorsException {
errors = errors.withSource(providerKey);
diff --git a/core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java b/core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java
index af3c470..eeb35a7 100644
--- a/core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java
+++ b/core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java
@@ -16,6 +16,7 @@
package com.google.inject.internal;
+import com.google.inject.Key;
import com.google.inject.Provider;
import static com.google.inject.internal.util.Preconditions.checkNotNull;
import com.google.inject.spi.Dependency;
@@ -32,8 +33,9 @@
private final Initializable<Provider<? extends T>> initializable;
public InternalFactoryToInitializableAdapter(
- Initializable<Provider<? extends T>> initializable, Object source, boolean allowProxy) {
- super(source, allowProxy);
+ Key<T> key, Initializable<Provider<? extends T>> initializable,
+ Object source, boolean allowProxy) {
+ super(key, source, allowProxy);
this.initializable = checkNotNull(initializable, "provider");
}
diff --git a/core/src/com/google/inject/internal/ProviderInternalFactory.java b/core/src/com/google/inject/internal/ProviderInternalFactory.java
index aba14d9..7ed741c 100644
--- a/core/src/com/google/inject/internal/ProviderInternalFactory.java
+++ b/core/src/com/google/inject/internal/ProviderInternalFactory.java
@@ -21,6 +21,7 @@
import javax.inject.Provider;
+import com.google.inject.Key;
import com.google.inject.spi.Dependency;
/**
@@ -31,10 +32,12 @@
*/
abstract class ProviderInternalFactory<T> implements InternalFactory<T> {
+ private final Key<T> key;
protected final Object source;
private final boolean allowProxy;
- ProviderInternalFactory(Object source, boolean allowProxy) {
+ ProviderInternalFactory(Key<T> key, Object source, boolean allowProxy) {
+ this.key = key;
this.source = checkNotNull(source, "source");
this.allowProxy = allowProxy;
}
@@ -43,9 +46,13 @@
InternalContext context, Dependency<?> dependency, boolean linked)
throws ErrorsException {
Class<?> expectedType = dependency.getKey().getTypeLiteral().getRawType();
- // we do not use 'this' as the key because for JIT provider bindings,
- // 'this' is recreated on failures... the key suffices.
- ConstructionContext<T> constructionContext = context.getConstructionContext(dependency.getKey());
+
+ // Use the Key we are providing for as a unique key to locate the context.
+ // We cannot use dependency.getKey() because that is the Key of the type
+ // we are trying to fulfill (which may be different among different
+ // calls to us). We also cannot use 'this', because the factory can
+ // be recreated different times during @ProvidedBy creations.
+ ConstructionContext<T> constructionContext = context.getConstructionContext(key);
// We have a circular reference between constructors. Return a proxy.
if (constructionContext.isConstructing()) {
diff --git a/core/test/com/google/inject/CircularDependencyTest.java b/core/test/com/google/inject/CircularDependencyTest.java
index 3fbfc83..0a185bb 100644
--- a/core/test/com/google/inject/CircularDependencyTest.java
+++ b/core/test/com/google/inject/CircularDependencyTest.java
@@ -16,6 +16,9 @@
package com.google.inject;
+import java.util.ArrayList;
+import java.util.List;
+
import junit.framework.TestCase;
import static com.google.inject.Asserts.assertContains;
@@ -405,4 +408,35 @@
return "G";
}
}
+
+ /**
+ * Tests that ProviderInternalFactory can detect circular dependencies
+ * before it gets to Scopes.SINGLETON. This is especially important
+ * because the failure in Scopes.SINGLETON doesn't have enough context to
+ * provide a decent error message.
+ */
+ public void testCircularDependenciesDetectedEarlyWhenDependenciesHaveDifferentTypes() {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(Number.class).to(Integer.class);
+ }
+
+ @Provides @Singleton Integer provideInteger(List list) {
+ return new Integer(2);
+ }
+
+ @Provides List provideList(Integer integer) {
+ return new ArrayList();
+ }
+ });
+ try {
+ injector.getInstance(Number.class);
+ fail();
+ } catch(ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "Tried proxying " + Integer.class.getName() + " to support a circular dependency, ",
+ "but it is not an interface.");
+ }
+ }
}
