Revision: 1586
Author: [email protected]
Date: Tue Sep 27 08:37:30 2011
Log:
Add Scopes.isCircularProxy, for use by Scope implementations. The basic
problem is that somewhere along the line, Guice is breaking the type-safety
of <T> in the scope(Key<T>, Provider<T>) method. This happens when <T> is
involved in a circular dependency, and the ConstructionContext creates a
circular proxy for a given 'expectedType' of <T> (the type in the parameter
where it's being injected). Expected type is a superclass or
superinterface of <T>, not a subclass or subinterface, so if a Scope caches
the result of Key<T> -> Provider<T>.get(), and then tries to reuse it, it's
possible that we return something that isn't compatible with <T>. This
results in either a ClassCastException (if cglib is involved) or
IllegalArgumentException (java reflection) when trying to construct the
object, because the parameters don't match the arguments.
Revision created by MOE tool push_codebase.
MOE_MIGRATION=3341
http://code.google.com/p/google-guice/source/detail?r=1586
Modified:
/trunk/core/src/com/google/inject/Scopes.java
/trunk/core/test/com/google/inject/CircularDependencyTest.java
/trunk/extensions/servlet/src/com/google/inject/servlet/ServletScopes.java
=======================================
--- /trunk/core/src/com/google/inject/Scopes.java Sun Oct 24 17:41:10 2010
+++ /trunk/core/src/com/google/inject/Scopes.java Tue Sep 27 08:37:30 2011
@@ -65,7 +65,7 @@
T provided = creator.get();
// don't remember proxies; these exist only to serve
circular dependencies
- if (provided instanceof CircularDependencyProxy) {
+ if (isCircularProxy(provided)) {
return provided;
}
@@ -87,6 +87,7 @@
return returnedInstance;
}
+ @Override
public String toString() {
return String.format("%s[%s]", creator, SINGLETON);
}
@@ -154,7 +155,7 @@
if (binding instanceof LinkedBindingImpl) {
LinkedBindingImpl<?> linkedBinding = (LinkedBindingImpl) binding;
- Injector injector = (Injector) linkedBinding.getInjector();
+ Injector injector = linkedBinding.getInjector();
if (injector != null) {
binding = injector.getBinding(linkedBinding.getLinkedKey());
continue;
@@ -171,4 +172,16 @@
return false;
} while (true);
}
-}
+
+ /**
+ * Returns true if the object is a proxy for a circular dependency,
+ * constructed by Guice because it encountered a circular dependency.
Scope
+ * implementations should be careful to <b>not cache circular
proxies</b>,
+ * because the proxies are not intended for general purpose use. (They
are
+ * designed just to fulfill the immediate injection, not all injections.
+ * Caching them can lead to IllegalArgumentExceptions or
ClassCastExceptions.)
+ */
+ public static boolean isCircularProxy(Object object) {
+ return object instanceof CircularDependencyProxy;
+ }
+}
=======================================
--- /trunk/core/test/com/google/inject/CircularDependencyTest.java Thu Jul
7 17:34:16 2011
+++ /trunk/core/test/com/google/inject/CircularDependencyTest.java Tue Sep
27 08:37:30 2011
@@ -17,11 +17,19 @@
package com.google.inject;
import static com.google.inject.Asserts.assertContains;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
import junit.framework.TestCase;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
import java.util.ArrayList;
import java.util.List;
+import java.util.Map;
/**
* @author [email protected] (Bob Lee)
@@ -501,4 +509,90 @@
static class Bar {
@Inject String string;
}
-}
+
+ /**
+ * When Scope Providers call their unscoped Provider's get() methods are
+ * called, it's possible that the result is a circular proxy designed
for one
+ * specific parameter (not for all possible parameters). But custom
scopes
+ * typically cache the results without checking to see if the result is a
+ * proxy. This leads to caching a result that is unsuitable for reuse for
+ * other parameters.
+ *
+ * This means that custom proxies have to do an
+ * {@code if(Scopes.isCircularProxy(..))}
+ * in order to avoid exceptions.
+ */
+ public void testCustomScopeCircularProxies() {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bindScope(SimpleSingleton.class, new BasicSingleton());
+ bind(H.class).to(HImpl.class);
+ bind(I.class).to(IImpl.class);
+ bind(J.class).to(JImpl.class);
+ }
+ });
+
+ // The reason this happens is because the Scope gets these requests,
in order:
+ // entry: Key<IImpl> (1 - from getInstance call)
+ // entry: Key<HImpl>
+ // entry: Key<IImpl> (2 - circular dependency from HImpl)
+ // result of 2nd Key<IImpl> - a com.google.inject.$Proxy, because it's
a circular proxy
+ // result of Key<HImpl> - an HImpl
+ // entry: Key<JImpl>
+ // entry: Key<IImpl> (3 - another circular dependency, this time from
JImpl)
+ // At this point, if the first Key<Impl> result was cached, our cache
would have
+ // Key<IImpl> caching to an instanceof of I, but not an an instanceof
of IImpl.
+ // If returned this, it would result in cglib giving a
ClassCastException or
+ // java reflection giving an IllegalArgumentException when filling in
parameters
+ // for the constructor, because JImpl wants an IImpl, not an I.
+
+ try {
+ injector.getInstance(IImpl.class);
+ fail();
+ } catch(ProvisionException pe) {
+
assertContains(Iterables.getOnlyElement(pe.getErrorMessages()).getMessage(),
+ "Tried proxying " + IImpl.class.getName()
+ + " to support a circular dependency, but it is not an
interface.");
+ }
+ }
+
+ interface H {}
+ interface I {}
+ interface J {}
+ @SimpleSingleton
+ static class HImpl implements H {
+ @Inject HImpl(I i) {}
+ }
+ @SimpleSingleton
+ static class IImpl implements I {
+ @Inject IImpl(HImpl i, J j) {}
+ }
+ @SimpleSingleton
+ static class JImpl implements J {
+ @Inject JImpl(IImpl i) {}
+ }
+
+ @Target({ ElementType.TYPE, ElementType.METHOD })
+ @Retention(RUNTIME)
+ @ScopeAnnotation
+ public @interface SimpleSingleton {}
+ public static class BasicSingleton implements Scope {
+ private static Map<Key, Object> cache = Maps.newHashMap();
+ public <T> Provider<T> scope(final Key<T> key, final Provider<T>
unscoped) {
+ return new Provider<T>() {
+ @SuppressWarnings("unchecked")
+ public T get() {
+ if (!cache.containsKey(key)) {
+ T t = unscoped.get();
+ if (Scopes.isCircularProxy(t)) {
+ return t;
+ }
+ cache.put(key, t);
+ }
+ return (T)cache.get(key);
+ }
+ };
+ }
+ }
+}
=======================================
---
/trunk/extensions/servlet/src/com/google/inject/servlet/ServletScopes.java
Tue Sep 27 08:36:19 2011
+++
/trunk/extensions/servlet/src/com/google/inject/servlet/ServletScopes.java
Tue Sep 27 08:37:30 2011
@@ -23,6 +23,7 @@
import com.google.inject.OutOfScopeException;
import com.google.inject.Provider;
import com.google.inject.Scope;
+import com.google.inject.Scopes;
import java.util.Map;
import java.util.concurrent.Callable;
@@ -76,8 +77,10 @@
if (t == null) {
t = creator.get();
- // Store a sentinel for provider-given null values.
- scopeMap.put(name, t != null ? t : NullObject.INSTANCE);
+ if (!Scopes.isCircularProxy(t)) {
+ // Store a sentinel for provider-given null values.
+ scopeMap.put(name, t != null ? t : NullObject.INSTANCE);
+ }
}
return t;
@@ -105,18 +108,22 @@
T t = (T) obj;
if (t == null) {
t = creator.get();
- request.setAttribute(name, (t != null) ? t :
NullObject.INSTANCE);
+ if (!Scopes.isCircularProxy(t)) {
+ request.setAttribute(name, (t != null) ? t :
NullObject.INSTANCE);
+ }
}
return t;
}
}
+ @Override
public String toString() {
return String.format("%s[%s]", creator, REQUEST);
}
};
}
+ @Override
public String toString() {
return "ServletScopes.REQUEST";
}
@@ -140,17 +147,21 @@
T t = (T) obj;
if (t == null) {
t = creator.get();
- session.setAttribute(name, (t != null) ? t :
NullObject.INSTANCE);
+ if (!Scopes.isCircularProxy(t)) {
+ session.setAttribute(name, (t != null) ? t :
NullObject.INSTANCE);
+ }
}
return t;
}
}
+ @Override
public String toString() {
return String.format("%s[%s]", creator, SESSION);
}
};
}
+ @Override
public String toString() {
return "ServletScopes.SESSION";
}
--
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.
