fix @ProvidedBy circular dependencies.
git-svn-id: https://google-guice.googlecode.com/svn/trunk@1546 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/core/src/com/google/inject/internal/ConstructorBindingImpl.java b/core/src/com/google/inject/internal/ConstructorBindingImpl.java
index fcb2ea5..fdd6756 100644
--- a/core/src/com/google/inject/internal/ConstructorBindingImpl.java
+++ b/core/src/com/google/inject/internal/ConstructorBindingImpl.java
@@ -38,7 +38,8 @@
import java.util.Map;
import java.util.Set;
-final class ConstructorBindingImpl<T> extends BindingImpl<T> implements ConstructorBinding<T> {
+final class ConstructorBindingImpl<T> extends BindingImpl<T>
+ implements ConstructorBinding<T>, DelayedInitialize {
private final Factory<T> factory;
private final InjectionPoint constructorInjectionPoint;
diff --git a/core/src/com/google/inject/internal/DelayedInitialize.java b/core/src/com/google/inject/internal/DelayedInitialize.java
new file mode 100644
index 0000000..80e52d1
--- /dev/null
+++ b/core/src/com/google/inject/internal/DelayedInitialize.java
@@ -0,0 +1,17 @@
+// Copyright 2011 Google Inc. All Rights Reserved.
+
+package com.google.inject.internal;
+
+/**
+ * Something that needs some delayed initialization, typically
+ * a binding or internal factory that needs to be created & put
+ * into the bindings map & then initialized later.
+ *
+ * @author sameb@google.com (Sam Berlin)
+ */
+interface DelayedInitialize {
+
+ /** Initializes this binding, throwing any errors if necessary. */
+ void initialize(InjectorImpl injector, Errors errors) throws ErrorsException;
+
+}
diff --git a/core/src/com/google/inject/internal/InjectorImpl.java b/core/src/com/google/inject/internal/InjectorImpl.java
index 715f933..bfc7b10 100644
--- a/core/src/com/google/inject/internal/InjectorImpl.java
+++ b/core/src/com/google/inject/internal/InjectorImpl.java
@@ -503,22 +503,22 @@
}
<T> void initializeBinding(BindingImpl<T> binding, Errors errors) throws ErrorsException {
- if (binding instanceof ConstructorBindingImpl<?>) {
- ((ConstructorBindingImpl) binding).initialize(this, errors);
+ if (binding instanceof DelayedInitialize) {
+ ((DelayedInitialize) binding).initialize(this, errors);
}
}
<T> void initializeJitBinding(BindingImpl<T> binding, Errors errors) throws ErrorsException {
// Put the partially constructed binding in the map a little early. This enables us to handle
// circular dependencies. Example: FooImpl -> BarImpl -> FooImpl.
- // Note: We don't need to synchronize on state.lock() during injector creation.
- if (binding instanceof ConstructorBindingImpl<?>) {
+ // Note: We don't need to synchronize on state.lock() during injector creation.
+ if (binding instanceof DelayedInitialize) {
Key<T> key = binding.getKey();
jitBindings.put(key, binding);
boolean successful = false;
- ConstructorBindingImpl cb = (ConstructorBindingImpl)binding;
+ DelayedInitialize delayed = (DelayedInitialize)binding;
try {
- cb.initialize(this, errors);
+ delayed.initialize(this, errors);
successful = true;
} finally {
if (!successful) {
@@ -662,8 +662,8 @@
/** Creates a binding for a type annotated with @ProvidedBy. */
<T> BindingImpl<T> createProvidedByBinding(Key<T> key, Scoping scoping,
ProvidedBy providedBy, Errors errors) throws ErrorsException {
- final Class<?> rawType = key.getTypeLiteral().getRawType();
- final Class<? extends Provider<?>> providerType = providedBy.value();
+ Class<?> rawType = key.getTypeLiteral().getRawType();
+ Class<? extends Provider<?>> providerType = providedBy.value();
// Make sure it's not the same type. TODO: Can we check for deeper loops?
if (providerType == rawType) {
@@ -672,41 +672,20 @@
// Assume the provider provides an appropriate type. We double check at runtime.
@SuppressWarnings("unchecked")
- final Key<? extends Provider<T>> providerKey
- = (Key<? extends Provider<T>>) Key.get(providerType);
- final BindingImpl<? extends Provider<?>> providerBinding
- = getBindingOrThrow(providerKey, errors, JitLimitation.NEW_OR_EXISTING_JIT);
-
- InternalFactory<T> internalFactory =
- new ProviderInternalFactory<T>(providerKey, !options.disableCircularProxies) {
- public T get(Errors errors, InternalContext context, Dependency dependency, boolean linked)
- throws ErrorsException {
- errors = errors.withSource(providerKey);
- Provider provider = providerBinding.getInternalFactory().get(
- errors, context, dependency, true);
- try {
- @SuppressWarnings("unchecked") // type is not checked within circularGet
- Object o = circularGet(provider, errors, context, dependency, linked);
- if (o != null && !rawType.isInstance(o)) {
- throw errors.subtypeNotProvided(providerType, rawType).toException();
- }
- @SuppressWarnings("unchecked") // protected by isInstance() check above
- T t = (T) o;
- return t;
- } catch (RuntimeException e) {
- throw errors.errorInProvider(e).toException();
- }
- }
- };
+ Key<? extends Provider<T>> providerKey = (Key<? extends Provider<T>>) Key.get(providerType);
+ ProvidedByInternalFactory<T> internalFactory =
+ new ProvidedByInternalFactory<T>(rawType, providerType,
+ providerKey, !options.disableCircularProxies);
Object source = rawType;
- return new LinkedProviderBindingImpl<T>(
+ return LinkedProviderBindingImpl.createWithInitializer(
this,
key,
source,
Scoping.<T>scope(key, this, internalFactory, source, scoping),
scoping,
- providerKey);
+ providerKey,
+ internalFactory);
}
/** Creates a binding for a type annotated with @ImplementedBy. */
diff --git a/core/src/com/google/inject/internal/LinkedProviderBindingImpl.java b/core/src/com/google/inject/internal/LinkedProviderBindingImpl.java
index ccbdf9e..d7a8599 100644
--- a/core/src/com/google/inject/internal/LinkedProviderBindingImpl.java
+++ b/core/src/com/google/inject/internal/LinkedProviderBindingImpl.java
@@ -28,21 +28,39 @@
import java.util.Set;
final class LinkedProviderBindingImpl<T>
- extends BindingImpl<T> implements ProviderKeyBinding<T>, HasDependencies {
+ extends BindingImpl<T> implements ProviderKeyBinding<T>, HasDependencies, DelayedInitialize {
final Key<? extends javax.inject.Provider<? extends T>> providerKey;
+ final DelayedInitialize delayedInitializer;
+
+ private LinkedProviderBindingImpl(InjectorImpl injector, Key<T> key, Object source,
+ InternalFactory<? extends T> internalFactory, Scoping scoping,
+ Key<? extends javax.inject.Provider<? extends T>> providerKey,
+ DelayedInitialize delayedInitializer) {
+ super(injector, key, source, internalFactory, scoping);
+ this.providerKey = providerKey;
+ this.delayedInitializer = delayedInitializer;
+ }
public LinkedProviderBindingImpl(InjectorImpl injector, Key<T> key, Object source,
InternalFactory<? extends T> internalFactory, Scoping scoping,
Key<? extends javax.inject.Provider<? extends T>> providerKey) {
- super(injector, key, source, internalFactory, scoping);
- this.providerKey = providerKey;
+ this(injector, key, source, internalFactory, scoping, providerKey, null);
}
LinkedProviderBindingImpl(Object source, Key<T> key, Scoping scoping,
Key<? extends javax.inject.Provider<? extends T>> providerKey) {
super(source, key, scoping);
this.providerKey = providerKey;
+ this.delayedInitializer = null;
+ }
+
+ static <T> LinkedProviderBindingImpl<T> createWithInitializer(InjectorImpl injector, Key<T> key,
+ Object source, InternalFactory<? extends T> internalFactory, Scoping scoping,
+ Key<? extends javax.inject.Provider<? extends T>> providerKey,
+ DelayedInitialize delayedInitializer) {
+ return new LinkedProviderBindingImpl<T>(injector, key, source, internalFactory, scoping,
+ providerKey, delayedInitializer);
}
public <V> V acceptTargetVisitor(BindingTargetVisitor<? super T, V> visitor) {
@@ -53,6 +71,12 @@
return providerKey;
}
+ public void initialize(InjectorImpl injector, Errors errors) throws ErrorsException {
+ if (delayedInitializer != null) {
+ delayedInitializer.initialize(injector, errors);
+ }
+ }
+
public Set<Dependency<?>> getDependencies() {
return ImmutableSet.<Dependency<?>>of(Dependency.get(providerKey));
}
diff --git a/core/src/com/google/inject/internal/ProvidedByInternalFactory.java b/core/src/com/google/inject/internal/ProvidedByInternalFactory.java
new file mode 100644
index 0000000..3ca75e5
--- /dev/null
+++ b/core/src/com/google/inject/internal/ProvidedByInternalFactory.java
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2011 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+package com.google.inject.internal;
+
+import static com.google.inject.internal.util.Preconditions.checkState;
+
+import com.google.inject.Key;
+import com.google.inject.ProvidedBy;
+import com.google.inject.Provider;
+import com.google.inject.internal.InjectorImpl.JitLimitation;
+import com.google.inject.spi.Dependency;
+
+/**
+ * An {@link InternalFactory} for {@literal @}{@link ProvidedBy} bindings.
+ *
+ * @author sameb@google.com (Sam Berlin)
+ */
+class ProvidedByInternalFactory<T> extends ProviderInternalFactory<T>
+ implements DelayedInitialize {
+
+ private final Class<?> rawType;
+ private final Class<? extends Provider<?>> providerType;
+ private final Key<? extends Provider<T>> providerKey;
+ private BindingImpl<? extends Provider<T>> providerBinding;
+
+ ProvidedByInternalFactory(
+ Class<?> rawType,
+ Class<? extends Provider<?>> providerType,
+ Key<? extends Provider<T>> providerKey,
+ boolean allowProxy) {
+ super(providerKey, allowProxy);
+ this.rawType = rawType;
+ this.providerType = providerType;
+ this.providerKey = providerKey;
+ }
+
+ public void initialize(InjectorImpl injector, Errors errors) throws ErrorsException {
+ providerBinding =
+ injector.getBindingOrThrow(providerKey, errors, JitLimitation.NEW_OR_EXISTING_JIT);
+ }
+
+ public T get(Errors errors, InternalContext context, Dependency dependency, boolean linked)
+ throws ErrorsException {
+ checkState(providerBinding != null, "not initialized");
+
+ errors = errors.withSource(providerKey);
+ Provider provider = providerBinding.getInternalFactory().get(
+ errors, context, dependency, true);
+ try {
+ @SuppressWarnings("unchecked") // type is not checked within circularGet
+ Object o = circularGet(provider, errors, context, dependency, linked);
+ if (o != null && !rawType.isInstance(o)) {
+ throw errors.subtypeNotProvided(providerType, rawType).toException();
+ }
+ @SuppressWarnings("unchecked") // protected by isInstance() check above
+ T t = (T) o;
+ return t;
+ } catch (RuntimeException e) {
+ throw errors.errorInProvider(e).toException();
+ }
+ }
+}
diff --git a/core/test/com/google/inject/CircularDependencyTest.java b/core/test/com/google/inject/CircularDependencyTest.java
index 72f020b..28e5272 100644
--- a/core/test/com/google/inject/CircularDependencyTest.java
+++ b/core/test/com/google/inject/CircularDependencyTest.java
@@ -88,21 +88,11 @@
assertCircularDependencies(injector);
}
- // TODO: This creates two As because circular dependencies between @ProvidedBy
- // Providers aren't handled well right now. When we bind A, it looks for its
- // Provider, which goes into InjectorImpl.createProvidedByBinding, which
- // goes into getBindingOrThrow for AutoAP. That creates a ConstructorBinding
- // for AutoAP and looks up its dependency for Provider<B>, which ends up
- // in creativeProvidedByBinding for BP, which creates a ConstructorBinding
- // for BP and looks up the dependency for Provider<A>. That ends up creating
- // another providedByBinding for AutoAP, because the first one hasn't been stored
- // anywhere yet. The solution is to initialize the dependency early, similar
- // to what's done with ConstructorBindings.
-// public void testCircularlyDependentConstructorsWithProvidedBy()
-// throws CreationException {
-// Injector injector = Guice.createInjector();
-// assertCircularDependencies(injector);
-// }
+ public void testCircularlyDependentConstructorsWithProvidedBy()
+ throws CreationException {
+ Injector injector = Guice.createInjector();
+ assertCircularDependencies(injector);
+ }
private void assertCircularDependencies(Injector injector) {
A a = injector.getInstance(A.class);
@@ -238,9 +228,7 @@
fail();
} catch (ProvisionException expected) {
assertContains(expected.getMessage(),
- // TODO: this should fail at C2, but because of strangeness with @ProvidedBy,
- // it fails in the wrong place right now.
- "Tried proxying " + D2.class.getName() + " to support a circular dependency, ",
+ "Tried proxying " + C2.class.getName() + " to support a circular dependency, ",
"but it is not an interface.");
}
}
@@ -336,9 +324,7 @@
fail();
} catch (ProvisionException expected) {
assertContains(expected.getMessage(),
- // TODO: this should fail at C2, but because of strangeness with @ProvidedBy,
- // it fails in the wrong place right now.
- "Tried proxying " + D2.class.getName() + " to support a circular dependency, ",
+ "Tried proxying " + C2.class.getName() + " to support a circular dependency, ",
"but circular proxies are disabled.");
}
}
