Revision: 1546
Author: sberlin
Date: Mon May 2 11:03:35 2011
Log: fix @ProvidedBy circular dependencies.
http://code.google.com/p/google-guice/source/detail?r=1546
Added:
/trunk/core/src/com/google/inject/internal/DelayedInitialize.java
/trunk/core/src/com/google/inject/internal/ProvidedByInternalFactory.java
Modified:
/trunk/core/src/com/google/inject/internal/ConstructorBindingImpl.java
/trunk/core/src/com/google/inject/internal/InjectorImpl.java
/trunk/core/src/com/google/inject/internal/LinkedProviderBindingImpl.java
/trunk/core/test/com/google/inject/CircularDependencyTest.java
=======================================
--- /dev/null
+++ /trunk/core/src/com/google/inject/internal/DelayedInitialize.java Mon
May 2 11:03:35 2011
@@ -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 [email protected] (Sam Berlin)
+ */
+interface DelayedInitialize {
+
+ /** Initializes this binding, throwing any errors if necessary. */
+ void initialize(InjectorImpl injector, Errors errors) throws
ErrorsException;
+
+}
=======================================
--- /dev/null
+++
/trunk/core/src/com/google/inject/internal/ProvidedByInternalFactory.java
Mon May 2 11:03:35 2011
@@ -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 [email protected] (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();
+ }
+ }
+}
=======================================
--- /trunk/core/src/com/google/inject/internal/ConstructorBindingImpl.java
Tue Dec 14 21:38:37 2010
+++ /trunk/core/src/com/google/inject/internal/ConstructorBindingImpl.java
Mon May 2 11:03:35 2011
@@ -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;
=======================================
--- /trunk/core/src/com/google/inject/internal/InjectorImpl.java Mon May 2
06:30:37 2011
+++ /trunk/core/src/com/google/inject/internal/InjectorImpl.java Mon May 2
11:03:35 2011
@@ -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. */
=======================================
---
/trunk/core/src/com/google/inject/internal/LinkedProviderBindingImpl.java
Sat Jul 3 08:51:31 2010
+++
/trunk/core/src/com/google/inject/internal/LinkedProviderBindingImpl.java
Mon May 2 11:03:35 2011
@@ -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) {
@@ -52,6 +70,12 @@
public Key<? extends javax.inject.Provider<? extends T>>
getProviderKey() {
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));
=======================================
--- /trunk/core/test/com/google/inject/CircularDependencyTest.java Mon May
2 06:30:37 2011
+++ /trunk/core/test/com/google/inject/CircularDependencyTest.java Mon May
2 11:03:35 2011
@@ -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.");
}
}
--
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.
