Revision: 1544
Author: sberlin
Date: Sun May 1 17:28:59 2011
Log: fix provider circular dependency detection to use the Key it is
creating, not the Key it is fulfulling, to catch errors sooner.
http://code.google.com/p/google-guice/source/detail?r=1544
Modified:
/trunk/core/src/com/google/inject/internal/BindingProcessor.java
/trunk/core/src/com/google/inject/internal/BoundProviderFactory.java
/trunk/core/src/com/google/inject/internal/InjectorImpl.java
/trunk/core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java
/trunk/core/src/com/google/inject/internal/ProviderInternalFactory.java
/trunk/core/test/com/google/inject/CircularDependencyTest.java
=======================================
--- /trunk/core/src/com/google/inject/internal/BindingProcessor.java Sat
Apr 30 08:38:05 2011
+++ /trunk/core/src/com/google/inject/internal/BindingProcessor.java Sun
May 1 17:28:59 2011
@@ -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(
=======================================
--- /trunk/core/src/com/google/inject/internal/BoundProviderFactory.java
Sat Apr 30 08:38:05 2011
+++ /trunk/core/src/com/google/inject/internal/BoundProviderFactory.java
Sun May 1 17:28:59 2011
@@ -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;
}
=======================================
--- /trunk/core/src/com/google/inject/internal/InjectorImpl.java Sat Apr 30
08:38:05 2011
+++ /trunk/core/src/com/google/inject/internal/InjectorImpl.java Sun May 1
17:28:59 2011
@@ -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);
=======================================
---
/trunk/core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java
Sat Apr 30 08:38:05 2011
+++
/trunk/core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java
Sun May 1 17:28:59 2011
@@ -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");
}
=======================================
--- /trunk/core/src/com/google/inject/internal/ProviderInternalFactory.java
Sat Apr 30 08:38:05 2011
+++ /trunk/core/src/com/google/inject/internal/ProviderInternalFactory.java
Sun May 1 17:28:59 2011
@@ -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()) {
=======================================
--- /trunk/core/test/com/google/inject/CircularDependencyTest.java Sat Apr
30 08:38:05 2011
+++ /trunk/core/test/com/google/inject/CircularDependencyTest.java Sun May
1 17:28:59 2011
@@ -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.");
+ }
+ }
+}
--
You received this message because you are subscribed to the Google Groups
"google-guice-dev" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/google-guice-dev?hl=en.
