Merge pull request #912 from google/moe-changes
Merge internal changes
diff --git a/core/src/com/google/inject/internal/InjectorImpl.java b/core/src/com/google/inject/internal/InjectorImpl.java
index d1d028f..3d868eb 100644
--- a/core/src/com/google/inject/internal/InjectorImpl.java
+++ b/core/src/com/google/inject/internal/InjectorImpl.java
@@ -801,6 +801,7 @@
throw errors.childBindingAlreadySet(key, sources).toException();
}
+ key = MoreTypes.canonicalizeKey(key); // before storing the key long-term, canonicalize it.
BindingImpl<T> binding = createJustInTimeBinding(key, errors, jitDisabled, jitType);
state.parent().blacklist(key, state, binding.getSource());
jitBindings.put(key, binding);
diff --git a/core/src/com/google/inject/internal/MoreTypes.java b/core/src/com/google/inject/internal/MoreTypes.java
index 12a7625..bdf6029 100644
--- a/core/src/com/google/inject/internal/MoreTypes.java
+++ b/core/src/com/google/inject/internal/MoreTypes.java
@@ -23,6 +23,7 @@
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableMap;
import com.google.inject.ConfigurationException;
+import com.google.inject.Key;
import com.google.inject.TypeLiteral;
import com.google.inject.util.Types;
@@ -64,6 +65,25 @@
.build();
/**
+ * Returns a key that doesn't hold any references to parent classes.
+ * This is necessary for anonymous keys, so ensure we don't hold a ref
+ * to the containing module (or class) forever.
+ */
+ public static <T> Key<T> canonicalizeKey(Key<T> key) {
+ // If we know this isn't a subclass, return as-is.
+ // Otherwise, recreate the key to avoid the subclass
+ if (key.getClass() == Key.class) {
+ return key;
+ } else if (key.getAnnotation() != null) {
+ return Key.get(key.getTypeLiteral(), key.getAnnotation());
+ } else if (key.getAnnotationType() != null) {
+ return Key.get(key.getTypeLiteral(), key.getAnnotationType());
+ } else {
+ return Key.get(key.getTypeLiteral());
+ }
+ }
+
+ /**
* Returns an type that's appropriate for use in a key.
*
* <p>If the raw type of {@code typeLiteral} is a {@code javax.inject.Provider}, this returns a
@@ -93,9 +113,20 @@
@SuppressWarnings("unchecked")
TypeLiteral<T> wrappedPrimitives = (TypeLiteral<T>) PRIMITIVE_TO_WRAPPER.get(typeLiteral);
- return wrappedPrimitives != null
- ? wrappedPrimitives
- : typeLiteral;
+ if (wrappedPrimitives != null) {
+ return wrappedPrimitives;
+ }
+
+ // If we know this isn't a subclass, return as-is.
+ if (typeLiteral.getClass() == TypeLiteral.class) {
+ return typeLiteral;
+ }
+
+ // recreate the TypeLiteral to avoid anonymous TypeLiterals from holding refs to their
+ // surrounding classes.
+ @SuppressWarnings("unchecked")
+ TypeLiteral<T> recreated = (TypeLiteral<T>) TypeLiteral.get(typeLiteral.getType());
+ return recreated;
}
/**
diff --git a/core/src/com/google/inject/spi/Dependency.java b/core/src/com/google/inject/spi/Dependency.java
index c51d87c..f86e255 100644
--- a/core/src/com/google/inject/spi/Dependency.java
+++ b/core/src/com/google/inject/spi/Dependency.java
@@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.inject.Key;
+import com.google.inject.internal.MoreTypes;
import java.util.List;
import java.util.Set;
@@ -54,7 +55,7 @@
* nullable.
*/
public static <T> Dependency<T> get(Key<T> key) {
- return new Dependency<T>(null, key, true, -1);
+ return new Dependency<T>(null, MoreTypes.canonicalizeKey(key), true, -1);
}
/**
diff --git a/core/src/com/google/inject/spi/Elements.java b/core/src/com/google/inject/spi/Elements.java
index 46072e3..9348276 100644
--- a/core/src/com/google/inject/spi/Elements.java
+++ b/core/src/com/google/inject/spi/Elements.java
@@ -44,6 +44,7 @@
import com.google.inject.internal.Errors;
import com.google.inject.internal.ExposureBuilder;
import com.google.inject.internal.InternalFlags.IncludeStackTraceOption;
+import com.google.inject.internal.MoreTypes;
import com.google.inject.internal.PrivateElementsImpl;
import com.google.inject.internal.ProviderMethodsModule;
import com.google.inject.internal.util.SourceProvider;
@@ -147,10 +148,12 @@
private static class ModuleInfo {
private final Binder binder;
private final ModuleSource moduleSource;
+ private final boolean skipScanning;
- private ModuleInfo(Binder binder, ModuleSource moduleSource) {
+ private ModuleInfo(Binder binder, ModuleSource moduleSource, boolean skipScanning) {
this.binder = binder;
this.moduleSource = moduleSource;
+ this.skipScanning = skipScanning;
}
}
@@ -240,13 +243,14 @@
@Override
public <T> void requestInjection(TypeLiteral<T> type, T instance) {
- elements.add(new InjectionRequest<T>(getElementSource(), type, instance));
+ elements.add(new InjectionRequest<T>(getElementSource(), MoreTypes.canonicalizeForKey(type),
+ instance));
}
@Override
public <T> MembersInjector<T> getMembersInjector(final TypeLiteral<T> typeLiteral) {
- final MembersInjectorLookup<T> element
- = new MembersInjectorLookup<T>(getElementSource(), typeLiteral);
+ final MembersInjectorLookup<T> element = new MembersInjectorLookup<T>(getElementSource(),
+ MoreTypes.canonicalizeForKey(typeLiteral));
elements.add(element);
return element.getMembersInjector();
}
@@ -270,19 +274,25 @@
}
}
+ /**
+ * Applies all scanners to the modules we've installed. We skip certain
+ * PrivateModules because store them in more than one Modules map and only
+ * want to process them through one of the maps. (They're stored in both
+ * maps to prevent a module from being installed more than once.)
+ */
void scanForAnnotatedMethods() {
for (ModuleAnnotatedMethodScanner scanner : scanners) {
// Note: we must iterate over a copy of the modules because calling install(..)
// will mutate modules, otherwise causing a ConcurrentModificationException.
for (Map.Entry<Module, ModuleInfo> entry : Maps.newLinkedHashMap(modules).entrySet()) {
Module module = entry.getKey();
- // If this was from a child private binder, skip it... we'll process it later.
- if (entry.getValue().binder != this) {
+ ModuleInfo info = entry.getValue();
+ if (info.skipScanning) {
continue;
}
moduleSource = entry.getValue().moduleSource;
try {
- install(ProviderMethodsModule.forModule(module, scanner));
+ info.binder.install(ProviderMethodsModule.forModule(module, scanner));
} catch(RuntimeException e) {
Collection<Message> messages = Errors.getMessagesFromThrowable(e);
if (!messages.isEmpty()) {
@@ -298,7 +308,7 @@
public void install(Module module) {
if (!modules.containsKey(module)) {
- Binder binder = this;
+ RecordingBinder binder = this;
boolean unwrapModuleSource = false;
// Update the module source for the new module
if (module instanceof ProviderMethodsModule) {
@@ -316,14 +326,16 @@
moduleSource = getModuleSource(module);
unwrapModuleSource = true;
}
+ boolean skipScanning = false;
if (module instanceof PrivateModule) {
- binder = binder.newPrivateBinder();
- // Store the module in the private binder too.
- ((RecordingBinder) binder).modules.put(module, new ModuleInfo(binder, moduleSource));
+ binder = (RecordingBinder) binder.newPrivateBinder();
+ // Store the module in the private binder too so we scan for it.
+ binder.modules.put(module, new ModuleInfo(binder, moduleSource, false));
+ skipScanning = true; // don't scan this module in the parent's module set.
}
// Always store this in the parent binder (even if it was a private module)
// so that we know not to process it again, and so that scanners inherit down.
- modules.put(module, new ModuleInfo(binder, moduleSource));
+ modules.put(module, new ModuleInfo(binder, moduleSource, skipScanning));
try {
module.configure(binder);
} catch (RuntimeException e) {
@@ -360,7 +372,8 @@
}
public <T> AnnotatedBindingBuilder<T> bind(Key<T> key) {
- BindingBuilder<T> builder = new BindingBuilder<T>(this, elements, getElementSource(), key);
+ BindingBuilder<T> builder =
+ new BindingBuilder<T>(this, elements, getElementSource(), MoreTypes.canonicalizeKey(key));
return builder;
}
@@ -470,7 +483,8 @@
};
}
- ExposureBuilder<T> builder = new ExposureBuilder<T>(this, getElementSource(), key);
+ ExposureBuilder<T> builder =
+ new ExposureBuilder<T>(this, getElementSource(), MoreTypes.canonicalizeKey(key));
privateElements.addExposureBuilder(builder);
return builder;
}
diff --git a/core/test/com/google/inject/Asserts.java b/core/test/com/google/inject/Asserts.java
index 363e161..6c63158 100644
--- a/core/test/com/google/inject/Asserts.java
+++ b/core/test/com/google/inject/Asserts.java
@@ -21,12 +21,14 @@
import static com.google.inject.internal.InternalFlags.getIncludeStackTraceOption;
import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNotNull;
+import static junit.framework.Assert.assertSame;
import static junit.framework.Assert.assertTrue;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
+import com.google.common.testing.GcFinalization;
import junit.framework.Assert;
@@ -36,6 +38,8 @@
import java.io.NotSerializableException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
/**
* @author jessewilson@google.com (Jesse Wilson)
@@ -159,4 +163,42 @@
} catch (NotSerializableException expected) {
}
}
+
+ public static void awaitFullGc() {
+ // GcFinalization *should* do it, but doesn't work well in practice...
+ // so we put a second latch and wait for a ReferenceQueue to tell us.
+ ReferenceQueue<Object> queue = new ReferenceQueue<Object>();
+ WeakReference ref = new WeakReference<Object>(new Object(), queue);
+ GcFinalization.awaitFullGc();
+ try {
+ assertSame("queue didn't return ref in time", ref, queue.remove(5000));
+ } catch (IllegalArgumentException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ public static void awaitClear(WeakReference<?> ref) {
+ // GcFinalization *should* do it, but doesn't work well in practice...
+ // so we put a second latch and wait for a ReferenceQueue to tell us.
+ Object data = ref.get();
+ ReferenceQueue<Object> queue = null;
+ WeakReference extraRef = null;
+ if (data != null) {
+ queue = new ReferenceQueue<Object>();
+ extraRef = new WeakReference<Object>(data, queue);
+ data = null;
+ }
+ GcFinalization.awaitClear(ref);
+ if (queue != null) {
+ try {
+ assertSame("queue didn't return ref in time", extraRef, queue.remove(5000));
+ } catch (IllegalArgumentException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
}
diff --git a/core/test/com/google/inject/KeyTest.java b/core/test/com/google/inject/KeyTest.java
index c0401e0..d9dd943 100644
--- a/core/test/com/google/inject/KeyTest.java
+++ b/core/test/com/google/inject/KeyTest.java
@@ -19,10 +19,12 @@
import static com.google.inject.Asserts.assertContains;
import static com.google.inject.Asserts.assertEqualsBothWays;
import static com.google.inject.Asserts.assertNotSerializable;
+import static com.google.inject.Asserts.awaitClear;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import com.google.inject.name.Named;
import com.google.inject.name.Names;
+import com.google.inject.spi.Dependency;
import com.google.inject.util.Types;
import junit.framework.TestCase;
@@ -31,12 +33,15 @@
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
+import java.lang.ref.WeakReference;
import java.lang.reflect.Method;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
+import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
/**
* @author crazybob@google.com (Bob Lee)
@@ -275,4 +280,42 @@
@Marker
class HasAnnotations {}
+ public void testAnonymousClassesDontHoldRefs() {
+ final AtomicReference<Provider<List<String>>> stringProvider =
+ new AtomicReference<Provider<List<String>>>();
+ final AtomicReference<Provider<List<Integer>>> intProvider =
+ new AtomicReference<Provider<List<Integer>>>();
+ final Object foo = new Object() {
+ @SuppressWarnings("unused") @Inject List<String> list;
+ };
+ Module module = new AbstractModule() {
+ @Override protected void configure() {
+ bind(new Key<List<String>>() {}).toInstance(new ArrayList<String>());
+ bind(new TypeLiteral<List<Integer>>() {}).toInstance(new ArrayList<Integer>());
+
+ stringProvider.set(getProvider(new Key<List<String>>() {}));
+ intProvider.set(binder().getProvider(Dependency.get(new Key<List<Integer>>() {})));
+
+ binder().requestInjection(new TypeLiteral<Object>() {}, foo);
+ }
+ };
+ WeakReference<Module> moduleRef = new WeakReference<Module>(module);
+ final Injector injector = Guice.createInjector(module);
+ module = null;
+ awaitClear(moduleRef); // Make sure anonymous keys & typeliterals don't hold the module.
+
+ Runnable runner = new Runnable() {
+ @Override public void run() {
+ injector.getInstance(new Key<Typed<String>>() {});
+ injector.getInstance(Key.get(new TypeLiteral<Typed<Integer>>() {}));
+ }
+ };
+ WeakReference<Runnable> runnerRef = new WeakReference<Runnable>(runner);
+ runner.run();
+ runner = null;
+ awaitClear(runnerRef); // also make sure anonymous keys & typeliterals don't hold for JITs
+ }
+
+ static class Typed<T> {}
+
}
diff --git a/core/test/com/google/inject/internal/WeakKeySetTest.java b/core/test/com/google/inject/internal/WeakKeySetTest.java
index 3797d88..4a81ebb 100644
--- a/core/test/com/google/inject/internal/WeakKeySetTest.java
+++ b/core/test/com/google/inject/internal/WeakKeySetTest.java
@@ -16,13 +16,13 @@
package com.google.inject.internal;
+import static com.google.inject.Asserts.awaitClear;
+import static com.google.inject.Asserts.awaitFullGc;
import static com.google.inject.internal.WeakKeySetUtils.assertBlacklisted;
import static com.google.inject.internal.WeakKeySetUtils.assertInSet;
import static com.google.inject.internal.WeakKeySetUtils.assertNotBlacklisted;
import static com.google.inject.internal.WeakKeySetUtils.assertNotInSet;
import static com.google.inject.internal.WeakKeySetUtils.assertSourceNotInSet;
-import static com.google.inject.internal.WeakKeySetUtils.awaitClear;
-import static com.google.inject.internal.WeakKeySetUtils.awaitFullGc;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -34,13 +34,6 @@
import com.google.inject.Key;
import com.google.inject.Scope;
import com.google.inject.TypeLiteral;
-import com.google.inject.internal.BindingImpl;
-import com.google.inject.internal.Errors;
-/*if[AOP]*/
-import com.google.inject.internal.MethodAspect;
-/*end[AOP]*/
-import com.google.inject.internal.State;
-import com.google.inject.internal.WeakKeySet;
import com.google.inject.spi.ModuleAnnotatedMethodScannerBinding;
import com.google.inject.spi.ProvisionListenerBinding;
import com.google.inject.spi.ScopeBinding;
diff --git a/core/test/com/google/inject/internal/WeakKeySetUtils.java b/core/test/com/google/inject/internal/WeakKeySetUtils.java
index bab9e92..b023aa1 100644
--- a/core/test/com/google/inject/internal/WeakKeySetUtils.java
+++ b/core/test/com/google/inject/internal/WeakKeySetUtils.java
@@ -18,15 +18,11 @@
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertNotNull;
import static junit.framework.Assert.assertNull;
-import static junit.framework.Assert.assertSame;
import static junit.framework.Assert.assertTrue;
-import com.google.common.testing.GcFinalization;
import com.google.inject.Injector;
import com.google.inject.Key;
-import java.lang.ref.ReferenceQueue;
-import java.lang.ref.WeakReference;
import java.util.Set;
/**
@@ -38,44 +34,6 @@
private WeakKeySetUtils() {}
- public static void awaitFullGc() {
- // GcFinalization *should* do it, but doesn't work well in practice...
- // so we put a second latch and wait for a ReferenceQueue to tell us.
- ReferenceQueue<Object> queue = new ReferenceQueue<Object>();
- WeakReference ref = new WeakReference<Object>(new Object(), queue);
- GcFinalization.awaitFullGc();
- try {
- assertSame("queue didn't return ref in time", ref, queue.remove(5000));
- } catch (IllegalArgumentException e) {
- throw new RuntimeException(e);
- } catch (InterruptedException e) {
- throw new RuntimeException(e);
- }
- }
-
- public static void awaitClear(WeakReference<?> ref) {
- // GcFinalization *should* do it, but doesn't work well in practice...
- // so we put a second latch and wait for a ReferenceQueue to tell us.
- Object data = ref.get();
- ReferenceQueue<Object> queue = null;
- WeakReference extraRef = null;
- if (data != null) {
- queue = new ReferenceQueue<Object>();
- extraRef = new WeakReference<Object>(data, queue);
- data = null;
- }
- GcFinalization.awaitClear(ref);
- if (queue != null) {
- try {
- assertSame("queue didn't return ref in time", extraRef, queue.remove(5000));
- } catch (IllegalArgumentException e) {
- throw new RuntimeException(e);
- } catch (InterruptedException e) {
- throw new RuntimeException(e);
- }
- }
- }
-
public static void assertBlacklisted(Injector injector, Key<?> key) {
assertBlacklistState(injector, key, true);
}
diff --git a/core/test/com/google/inject/spi/ModuleAnnotatedMethodScannerTest.java b/core/test/com/google/inject/spi/ModuleAnnotatedMethodScannerTest.java
index e73a9af..6c797b1 100644
--- a/core/test/com/google/inject/spi/ModuleAnnotatedMethodScannerTest.java
+++ b/core/test/com/google/inject/spi/ModuleAnnotatedMethodScannerTest.java
@@ -75,6 +75,34 @@
foo2Binding.getProvider().toString());
}
+ public void testSkipSources() throws Exception {
+ Module module = new AbstractModule() {
+ @Override protected void configure() {
+ binder().skipSources(getClass()).install(new AbstractModule() {
+ @Override protected void configure() {}
+
+ @TestProvides @Named("foo") String foo() { return "foo"; }
+ });
+ }
+ };
+ Injector injector = Guice.createInjector(module, NamedMunger.module());
+ assertMungedBinding(injector, String.class, "foo", "foo");
+ }
+
+ public void testWithSource() throws Exception {
+ Module module = new AbstractModule() {
+ @Override protected void configure() {
+ binder().withSource("source").install(new AbstractModule() {
+ @Override protected void configure() {}
+
+ @TestProvides @Named("foo") String foo() { return "foo"; }
+ });
+ }
+ };
+ Injector injector = Guice.createInjector(module, NamedMunger.module());
+ assertMungedBinding(injector, String.class, "foo", "foo");
+ }
+
public void testMoreThanOneClaimedAnnotationFails() throws Exception {
Module module = new AbstractModule() {
@Override protected void configure() {}
@@ -228,6 +256,34 @@
assertMungedBinding(injector, String.class, "foo", "foo");
}
+ public void testPrivateModule_skipSourcesWithinPrivateModule() {
+ Injector injector = Guice.createInjector(NamedMunger.module(), new PrivateModule() {
+ @Override protected void configure() {
+ binder().skipSources(getClass()).install(new AbstractModule() {
+ @Override protected void configure() {}
+ @Exposed @TestProvides @Named("foo") String foo() {
+ return "foo";
+ }
+ });
+ }
+ });
+ assertMungedBinding(injector, String.class, "foo", "foo");
+ }
+
+ public void testPrivateModule_skipSourcesForPrivateModule() {
+ Injector injector = Guice.createInjector(NamedMunger.module(), new AbstractModule() {
+ @Override protected void configure() {
+ binder().skipSources(getClass()).install(new PrivateModule() {
+ @Override protected void configure() {}
+
+ @Exposed @TestProvides @Named("foo") String foo() {
+ return "foo";
+ }
+ });
+ }});
+ assertMungedBinding(injector, String.class, "foo", "foo");
+ }
+
public void testPrivateModuleInheritScanner_usingPrivateBinder() {
Injector injector = Guice.createInjector(NamedMunger.module(), new AbstractModule() {
@Override protected void configure() {
@@ -243,6 +299,36 @@
assertMungedBinding(injector, String.class, "foo", "foo");
}
+ public void testPrivateModuleInheritScanner_skipSourcesFromPrivateBinder() {
+ Injector injector = Guice.createInjector(NamedMunger.module(), new AbstractModule() {
+ @Override protected void configure() {
+ binder().newPrivateBinder().skipSources(getClass()).install(new AbstractModule() {
+ @Override protected void configure() {}
+
+ @Exposed @TestProvides @Named("foo") String foo() {
+ return "foo";
+ }
+ });
+ }
+ });
+ assertMungedBinding(injector, String.class, "foo", "foo");
+ }
+
+ public void testPrivateModuleInheritScanner_skipSourcesFromPrivateBinder2() {
+ Injector injector = Guice.createInjector(NamedMunger.module(), new AbstractModule() {
+ @Override protected void configure() {
+ binder().skipSources(getClass()).newPrivateBinder().install(new AbstractModule() {
+ @Override protected void configure() {}
+
+ @Exposed @TestProvides @Named("foo") String foo() {
+ return "foo";
+ }
+ });
+ }
+ });
+ assertMungedBinding(injector, String.class, "foo", "foo");
+ }
+
public void testPrivateModuleScannersDontImpactSiblings_usingPrivateModule() {
Injector injector = Guice.createInjector(new PrivateModule() {
@Override protected void configure() {
@@ -289,4 +375,20 @@
}});
assertMungedBinding(injector, String.class, "foo", "foo");
}
+
+ public void testPrivateModuleWithinPrivateModule() {
+ Injector injector = Guice.createInjector(NamedMunger.module(), new PrivateModule() {
+ @Override protected void configure() {
+ expose(Key.get(String.class, named("foo-munged")));
+ install(new PrivateModule() {
+ @Override protected void configure() {}
+
+ @Exposed @TestProvides @Named("foo") String foo() {
+ return "foo";
+ }
+ });
+ }
+ });
+ assertMungedBinding(injector, String.class, "foo", "foo");
+ }
}
diff --git a/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java b/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java
index 849993f..4206521 100644
--- a/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java
+++ b/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java
@@ -32,6 +32,7 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.inject.AbstractModule;
+import com.google.inject.Asserts;
import com.google.inject.Binding;
import com.google.inject.BindingAnnotation;
import com.google.inject.ConfigurationException;
@@ -1025,7 +1026,7 @@
// Clear the ref, GC, and ensure that we are no longer blacklisting.
childInjector = null;
- WeakKeySetUtils.awaitClear(weakRef);
+ Asserts.awaitClear(weakRef);
WeakKeySetUtils.assertNotBlacklisted(parentInjector, mapKey);
}
}
diff --git a/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java b/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java
index d0a239a..f3c9f63 100644
--- a/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java
+++ b/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java
@@ -30,6 +30,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.inject.AbstractModule;
+import com.google.inject.Asserts;
import com.google.inject.Binding;
import com.google.inject.BindingAnnotation;
import com.google.inject.CreationException;
@@ -1203,7 +1204,7 @@
// Clear the ref, GC, and ensure that we are no longer blacklisting.
childInjector = null;
- WeakKeySetUtils.awaitClear(weakRef);
+ Asserts.awaitClear(weakRef);
WeakKeySetUtils.assertNotBlacklisted(parentInjector, Key.get(Integer.class));
}
diff --git a/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProviderMethod.java b/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProviderMethod.java
index f65b6d2..a76d64c 100644
--- a/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProviderMethod.java
+++ b/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProviderMethod.java
@@ -50,6 +50,7 @@
private final boolean exposed;
private final Class<? extends CheckedProvider> checkedProvider;
private final List<TypeLiteral<?>> exceptionTypes;
+ private final boolean scopeExceptions;
CheckedProviderMethod(
Key<T> key,
@@ -59,7 +60,8 @@
List<Provider<?>> parameterProviders,
Class<? extends Annotation> scopeAnnotation,
Class<? extends CheckedProvider> checkedProvider,
- List<TypeLiteral<?>> exceptionTypes) {
+ List<TypeLiteral<?>> exceptionTypes,
+ boolean scopeExceptions) {
this.key = key;
this.scopeAnnotation = scopeAnnotation;
this.instance = instance;
@@ -69,6 +71,7 @@
this.exposed = method.isAnnotationPresent(Exposed.class);
this.checkedProvider = checkedProvider;
this.exceptionTypes = exceptionTypes;
+ this.scopeExceptions = scopeExceptions;
method.setAccessible(true);
}
@@ -77,13 +80,14 @@
binder = binder.withSource(method);
SecondaryBinder<?, ?> sbinder =
- ThrowingProviderBinder.create(binder)
- .bind(checkedProvider, key.getTypeLiteral());
+ ThrowingProviderBinder.create(binder)
+ .bind(checkedProvider, key.getTypeLiteral());
if(key.getAnnotation() != null) {
sbinder = sbinder.annotatedWith(key.getAnnotation());
} else if(key.getAnnotationType() != null) {
sbinder = sbinder.annotatedWith(key.getAnnotationType());
- }
+ }
+ sbinder.scopeExceptions(scopeExceptions);
ScopedBindingBuilder sbbuilder = sbinder.toProviderMethod(this);
if(scopeAnnotation != null) {
sbbuilder.in(scopeAnnotation);
diff --git a/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProviderMethodsModule.java b/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProviderMethodsModule.java
index dce2bfa..f88c507 100644
--- a/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProviderMethodsModule.java
+++ b/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProviderMethodsModule.java
@@ -79,7 +79,7 @@
for (Method method : c.getDeclaredMethods()) {
CheckedProvides checkedProvides = method.getAnnotation(CheckedProvides.class);
if(checkedProvides != null) {
- result.add(createProviderMethod(binder, method, checkedProvides.value()));
+ result.add(createProviderMethod(binder, method, checkedProvides));
}
}
}
@@ -87,7 +87,9 @@
}
<T> CheckedProviderMethod<T> createProviderMethod(Binder binder, final Method method,
- Class<? extends CheckedProvider> throwingProvider) {
+ CheckedProvides checkedProvides) {
+ @SuppressWarnings("rawtypes")
+ Class<? extends CheckedProvider> throwingProvider = checkedProvides.value();
binder = binder.withSource(method);
Errors errors = new Errors(method);
@@ -123,7 +125,8 @@
}
return new CheckedProviderMethod<T>(key, method, delegate, ImmutableSet.copyOf(dependencies),
- parameterProviders, scopeAnnotation, throwingProvider, exceptionTypes);
+ parameterProviders, scopeAnnotation, throwingProvider, exceptionTypes,
+ checkedProvides.scopeExceptions());
}
<T> Key<T> getKey(Errors errors, TypeLiteral<T> type, Member member, Annotation[] annotations) {
diff --git a/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProvides.java b/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProvides.java
index b702dcc..c7bfaed 100644
--- a/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProvides.java
+++ b/extensions/throwingproviders/src/com/google/inject/throwingproviders/CheckedProvides.java
@@ -42,5 +42,10 @@
* The interface that provides this value, a subinterface of {@link CheckedProvider}.
*/
Class<? extends CheckedProvider> value();
-
+
+ /**
+ * Whether exceptions should be put into the Guice scope.
+ * Default behavior is that exceptions are scoped.
+ */
+ boolean scopeExceptions() default true;
}
diff --git a/extensions/throwingproviders/src/com/google/inject/throwingproviders/ThrowingProviderBinder.java b/extensions/throwingproviders/src/com/google/inject/throwingproviders/ThrowingProviderBinder.java
index 637099a..de33d2c 100644
--- a/extensions/throwingproviders/src/com/google/inject/throwingproviders/ThrowingProviderBinder.java
+++ b/extensions/throwingproviders/src/com/google/inject/throwingproviders/ThrowingProviderBinder.java
@@ -85,6 +85,12 @@
*/
public class ThrowingProviderBinder {
+ private static final TypeLiteral<CheckedProvider<?>> CHECKED_PROVIDER_TYPE
+ = new TypeLiteral<CheckedProvider<?>>() { };
+
+ private static final TypeLiteral<CheckedProviderMethod<?>> CHECKED_PROVIDER_METHOD_TYPE
+ = new TypeLiteral<CheckedProviderMethod<?>>() { };
+
private final Binder binder;
private ThrowingProviderBinder(Binder binder) {
@@ -134,6 +140,7 @@
private Class<? extends Annotation> annotationType;
private Annotation annotation;
private Key<P> interfaceKey;
+ private boolean scopeExceptions = true;
public SecondaryBinder(Class<P> interfaceType, Type valueType) {
this.interfaceType = checkNotNull(interfaceType, "interfaceType");
@@ -171,6 +178,15 @@
return this;
}
+ /**
+ * Determines if exceptions should be scoped. By default exceptions are scoped.
+ * @param scopeExceptions whether exceptions should be scoped.
+ */
+ public SecondaryBinder<P, T> scopeExceptions(boolean scopeExceptions) {
+ this.scopeExceptions = scopeExceptions;
+ return this;
+ }
+
public ScopedBindingBuilder to(P target) {
Key<P> targetKey = Key.get(interfaceType, UniqueAnnotations.create());
binder.bind(targetKey).toInstance(target);
@@ -236,28 +252,32 @@
}
};
- Key<CheckedProvider> targetKey = Key.get(CheckedProvider.class, UniqueAnnotations.create());
+ Key<CheckedProvider<?>> targetKey = Key.get(CHECKED_PROVIDER_TYPE,
+ UniqueAnnotations.create());
binder.bind(targetKey).toInstance(checkedProvider);
return toInternal(targetKey);
}
ScopedBindingBuilder toProviderMethod(CheckedProviderMethod<?> target) {
- Key<CheckedProviderMethod> targetKey =
- Key.get(CheckedProviderMethod.class, UniqueAnnotations.create());
+ Key<CheckedProviderMethod<?>> targetKey =
+ Key.get(CHECKED_PROVIDER_METHOD_TYPE, UniqueAnnotations.create());
binder.bind(targetKey).toInstance(target);
return toInternal(targetKey);
}
+ @SuppressWarnings("unchecked") // P only extends the raw type of CheckedProvider
public ScopedBindingBuilder to(Key<? extends P> targetKey) {
checkNotNull(targetKey, "targetKey");
- return toInternal(targetKey);
+ return toInternal((Key<? extends CheckedProvider<?>>)targetKey);
}
- private ScopedBindingBuilder toInternal(final Key<? extends CheckedProvider> targetKey) {
+ private ScopedBindingBuilder toInternal(final Key<? extends CheckedProvider<?>> targetKey) {
final Key<Result> resultKey = Key.get(Result.class, UniqueAnnotations.create());
+ // Note that this provider will behave like the final provider Guice creates.
+ // It will especially do scoping if the user adds that.
final Provider<Result> resultProvider = binder.getProvider(resultKey);
- final Provider<? extends CheckedProvider> targetProvider = binder.getProvider(targetKey);
+ final Provider<? extends CheckedProvider<?>> targetProvider = binder.getProvider(targetKey);
interfaceKey = createKey();
// don't bother binding the proxy type if this is in an invalid state.
@@ -272,31 +292,63 @@
if (method.getDeclaringClass() == Object.class) {
return method.invoke(this, args);
}
- return resultProvider.get().getOrThrow();
+
+ if (scopeExceptions) {
+ return resultProvider.get().getOrThrow();
+ } else {
+ Result result;
+ try {
+ result = resultProvider.get();
+ } catch (ProvisionException pe) {
+ Throwable cause = pe.getCause();
+ if (cause instanceof ResultException) {
+ throw ((ResultException)cause).getCause();
+ } else {
+ throw pe;
+ }
+ }
+ return result.getOrThrow();
+ }
}
}));
+ @Override
public P get() {
return instance;
}
-
+
+ @Override
public Set<Dependency<?>> getDependencies() {
return ImmutableSet.<Dependency<?>>of(Dependency.get(resultKey));
}
});
}
- return binder.bind(resultKey).toProvider(new ProviderWithDependencies<Result>() {
+ // The provider is unscoped, but the user may apply a scope to it through the
+ // ScopedBindingBuilder this returns.
+ return binder.bind(resultKey).toProvider(
+ createResultProvider(targetKey, targetProvider));
+ }
+
+ private ProviderWithDependencies<Result> createResultProvider(
+ final Key<? extends CheckedProvider<?>> targetKey,
+ final Provider<? extends CheckedProvider<?>> targetProvider) {
+ return new ProviderWithDependencies<Result>() {
+ @Override
public Result get() {
try {
return Result.forValue(targetProvider.get().get());
} catch (Exception e) {
- for(Class<? extends Throwable> exceptionType : exceptionTypes) {
+ for (Class<? extends Throwable> exceptionType : exceptionTypes) {
if (exceptionType.isInstance(e)) {
- return Result.forException(e);
+ if (scopeExceptions) {
+ return Result.forException(e);
+ } else {
+ throw new ResultException(e);
+ }
}
}
-
+
if (e instanceof RuntimeException) {
throw (RuntimeException) e;
} else {
@@ -305,13 +357,14 @@
}
}
}
-
+
+ @Override
public Set<Dependency<?>> getDependencies() {
return ImmutableSet.<Dependency<?>>of(Dependency.get(targetKey));
}
- });
+ };
}
-
+
/**
* Returns the exception type declared to be thrown by the get method of
* {@code interfaceType}.
@@ -455,10 +508,12 @@
}
/**
- * Represents the returned value from a call to {@link
- * CheckedProvider#get()}. This is the value that will be scoped by Guice.
+ * Represents the returned value from a call to {@link CheckedProvider#get()}. This is the value
+ * that will be scoped by Guice.
*/
static class Result implements Serializable {
+ private static final long serialVersionUID = 0L;
+
private final Object value;
private final Exception exception;
@@ -474,7 +529,7 @@
public static Result forException(Exception e) {
return new Result(null, e);
}
-
+
public Object getOrThrow() throws Exception {
if (exception != null) {
throw exception;
@@ -482,8 +537,17 @@
return value;
}
}
-
- private static final long serialVersionUID = 0L;
+ }
+
+ /**
+ * RuntimeException class to wrap exceptions from the checked provider.
+ * The regular guice provider can throw it and the checked provider proxy extracts
+ * the underlying exception and rethrows it.
+ */
+ private static class ResultException extends RuntimeException {
+ ResultException(Exception cause) {
+ super(cause);
+ }
}
private static class NotSyntheticOrBridgePredicate implements Predicate<Method> {
diff --git a/extensions/throwingproviders/test/com/google/inject/throwingproviders/CheckedProviderTest.java b/extensions/throwingproviders/test/com/google/inject/throwingproviders/CheckedProviderTest.java
index 7a640be..1f4f977 100644
--- a/extensions/throwingproviders/test/com/google/inject/throwingproviders/CheckedProviderTest.java
+++ b/extensions/throwingproviders/test/com/google/inject/throwingproviders/CheckedProviderTest.java
@@ -16,6 +16,9 @@
package com.google.inject.throwingproviders;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -23,6 +26,7 @@
import com.google.common.collect.Lists;
import com.google.inject.AbstractModule;
import com.google.inject.Asserts;
+import com.google.inject.BindingAnnotation;
import com.google.inject.CreationException;
import com.google.inject.Guice;
import com.google.inject.Inject;
@@ -45,6 +49,7 @@
import junit.framework.TestCase;
import java.io.IOException;
+import java.lang.annotation.Annotation;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@@ -64,6 +69,8 @@
* @author sameb@google.com (Sam Berlin)
*/
public class CheckedProviderTest extends TestCase {
+ @Target(METHOD) @Retention(RUNTIME) @BindingAnnotation
+ @interface NotExceptionScoping { };
private static final Function<Dependency<?>, Key<?>> DEPENDENCY_TO_KEY =
new Function<Dependency<?>, Key<?>>() {
@@ -95,6 +102,14 @@
.bind(RemoteProvider.class, Foo.class)
.to(mockRemoteProvider)
.in(testScope);
+
+ ThrowingProviderBinder.create(binder())
+ .bind(RemoteProvider.class, Foo.class)
+ .annotatedWith(NotExceptionScoping.class)
+ .scopeExceptions(false)
+ .to(mockRemoteProvider)
+ .in(testScope);
+
}
});
@@ -111,6 +126,15 @@
Foo throwOrGet() throws RemoteException, BindException {
return mockRemoteProvider.get();
}
+
+ @SuppressWarnings("unused")
+ @CheckedProvides(value = RemoteProvider.class, scopeExceptions = false)
+ @NotExceptionScoping
+ @TestScope.Scoped
+ Foo notExceptionScopingThrowOrGet() throws RemoteException, BindException {
+ return mockRemoteProvider.get();
+ }
+
});
cxtorInjector = Guice.createInjector(new AbstractModule() {
@@ -120,6 +144,14 @@
.bind(RemoteProvider.class, Foo.class)
.providing(MockFoo.class)
.in(testScope);
+
+ ThrowingProviderBinder.create(binder())
+ .bind(RemoteProvider.class, Foo.class)
+ .annotatedWith(NotExceptionScoping.class)
+ .scopeExceptions(false)
+ .providing(MockFoo.class)
+ .in(testScope);
+
}
});
}
@@ -151,17 +183,28 @@
}
public void testValuesScoped_Bind() throws Exception {
- tValuesScoped(bindInjector);
+ tValuesScoped(bindInjector, null);
}
public void testValuesScoped_Provides() throws Exception {
- tValuesScoped(providesInjector);
+ tValuesScoped(providesInjector, null);
}
- private void tValuesScoped(Injector injector) throws Exception {
- RemoteProvider<Foo> remoteProvider =
- injector.getInstance(Key.get(remoteProviderOfFoo));
+ public void testValuesScopedWhenNotExceptionScoping_Bind() throws Exception {
+ tValuesScoped(bindInjector, NotExceptionScoping.class);
+ }
+
+ public void testValuesScopedWhenNotExceptionScoping_Provides() throws Exception {
+ tValuesScoped(providesInjector, NotExceptionScoping.class);
+ }
+ private void tValuesScoped(Injector injector,
+ Class<? extends Annotation> annotation) throws Exception {
+ Key<RemoteProvider<Foo>> key = annotation != null ?
+ Key.get(remoteProviderOfFoo, annotation) :
+ Key.get(remoteProviderOfFoo);
+ RemoteProvider<Foo> remoteProvider = injector.getInstance(key);
+
mockRemoteProvider.setNextToReturn(new SimpleFoo("A"));
assertEquals("A", remoteProvider.get().s());
@@ -218,6 +261,41 @@
}
}
+ public void testExceptionsNotScopedWhenNotExceptionScoping_Bind() throws Exception {
+ tExceptionsNotScopedWhenNotExceptionScoping(bindInjector);
+ }
+
+ public void testExceptionsNotScopedWhenNotExceptionScoping_Provides() throws Exception {
+ tExceptionsNotScopedWhenNotExceptionScoping(providesInjector);
+ }
+
+ public void testExceptionNotScopedWhenNotExceptionScoping_Cxtor() throws Exception {
+ tExceptionsNotScopedWhenNotExceptionScoping(cxtorInjector);
+ }
+
+ private void tExceptionsNotScopedWhenNotExceptionScoping(Injector injector) throws Exception {
+ RemoteProvider<Foo> remoteProvider =
+ injector.getInstance(Key.get(remoteProviderOfFoo, NotExceptionScoping.class));
+
+ mockRemoteProvider.throwOnNextGet(new RemoteException("A"));
+ MockFoo.nextToThrow = new RemoteException("A");
+ try {
+ remoteProvider.get();
+ fail();
+ } catch (RemoteException expected) {
+ assertEquals("A", expected.getMessage());
+ }
+
+ mockRemoteProvider.throwOnNextGet(new RemoteException("B"));
+ MockFoo.nextToThrow = new RemoteException("B");
+ try {
+ remoteProvider.get();
+ fail();
+ } catch (RemoteException expected) {
+ assertEquals("B", expected.getMessage());
+ }
+ }
+
public void testAnnotations_Bind() throws Exception {
final MockRemoteProvider<Foo> mockRemoteProviderA = new MockRemoteProvider<Foo>();
final MockRemoteProvider<Foo> mockRemoteProviderB = new MockRemoteProvider<Foo>();
diff --git a/extensions/throwingproviders/test/com/google/inject/throwingproviders/ThrowingProviderTest.java b/extensions/throwingproviders/test/com/google/inject/throwingproviders/ThrowingProviderTest.java
index d56cb7d..373c68d 100644
--- a/extensions/throwingproviders/test/com/google/inject/throwingproviders/ThrowingProviderTest.java
+++ b/extensions/throwingproviders/test/com/google/inject/throwingproviders/ThrowingProviderTest.java
@@ -16,11 +16,15 @@
package com.google.inject.throwingproviders;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.inject.AbstractModule;
+import com.google.inject.BindingAnnotation;
import com.google.inject.CreationException;
import com.google.inject.Guice;
import com.google.inject.Inject;
@@ -37,6 +41,9 @@
import junit.framework.TestCase;
import java.io.IOException;
+import java.lang.annotation.Annotation;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
import java.rmi.AccessException;
import java.rmi.RemoteException;
import java.util.Arrays;
@@ -50,6 +57,8 @@
*/
@SuppressWarnings("deprecation")
public class ThrowingProviderTest extends TestCase {
+ @Target(METHOD) @Retention(RUNTIME) @BindingAnnotation
+ @interface NotExceptionScoping { };
private final TypeLiteral<RemoteProvider<String>> remoteProviderOfString
= new TypeLiteral<RemoteProvider<String>>() { };
@@ -61,6 +70,13 @@
.bind(RemoteProvider.class, String.class)
.to(mockRemoteProvider)
.in(testScope);
+
+ ThrowingProviderBinder.create(binder())
+ .bind(RemoteProvider.class, String.class)
+ .annotatedWith(NotExceptionScoping.class)
+ .scopeExceptions(false)
+ .to(mockRemoteProvider)
+ .in(testScope);
}
});
private Injector providesInjector = Guice.createInjector(new AbstractModule() {
@@ -75,6 +91,14 @@
String throwOrGet() throws RemoteException {
return mockRemoteProvider.get();
}
+
+ @SuppressWarnings("unused")
+ @CheckedProvides(value = RemoteProvider.class, scopeExceptions = false)
+ @NotExceptionScoping
+ @TestScope.Scoped
+ String notExceptionScopingThrowOrGet() throws RemoteException {
+ return mockRemoteProvider.get();
+ }
});
public void testExceptionsThrown_Bind() {
@@ -99,16 +123,27 @@
}
public void testValuesScoped_Bind() throws RemoteException {
- tValuesScoped(bindInjector);
+ tValuesScoped(bindInjector, null);
}
public void testValuesScoped_Provides() throws RemoteException {
- tValuesScoped(providesInjector);
+ tValuesScoped(providesInjector, null);
}
- private void tValuesScoped(Injector injector) throws RemoteException {
- RemoteProvider<String> remoteProvider =
- injector.getInstance(Key.get(remoteProviderOfString));
+ public void testValuesScopedWhenNotExceptionScoping_Bind() throws RemoteException {
+ tValuesScoped(bindInjector, NotExceptionScoping.class);
+ }
+
+ public void testValuesScopedWhenNotExceptionScoping_Provides() throws RemoteException {
+ tValuesScoped(providesInjector, NotExceptionScoping.class);
+ }
+
+ private void tValuesScoped(Injector injector, Class<? extends Annotation> annotation)
+ throws RemoteException {
+ Key<RemoteProvider<String>> key = annotation != null ?
+ Key.get(remoteProviderOfString, annotation) :
+ Key.get(remoteProviderOfString);
+ RemoteProvider<String> remoteProvider = injector.getInstance(key);
mockRemoteProvider.setNextToReturn("A");
assertEquals("A", remoteProvider.get());
@@ -148,6 +183,35 @@
assertEquals("A", expected.getMessage());
}
}
+
+ public void testExceptionsNotScopedWhenNotExceptionScoping_Bind() {
+ tExceptionsNotScopedWhenNotExceptionScoping(bindInjector);
+ }
+
+ public void testExceptionsNotScopedWhenNotExceptionScoping_Provides() {
+ tExceptionsNotScopedWhenNotExceptionScoping(providesInjector);
+ }
+
+ private void tExceptionsNotScopedWhenNotExceptionScoping(Injector injector) {
+ RemoteProvider<String> remoteProvider =
+ injector.getInstance(Key.get(remoteProviderOfString, NotExceptionScoping.class));
+
+ mockRemoteProvider.throwOnNextGet("A");
+ try {
+ remoteProvider.get();
+ fail();
+ } catch (RemoteException expected) {
+ assertEquals("A", expected.getMessage());
+ }
+
+ mockRemoteProvider.throwOnNextGet("B");
+ try {
+ remoteProvider.get();
+ fail();
+ } catch (RemoteException expected) {
+ assertEquals("B", expected.getMessage());
+ }
+ }
public void testAnnotations_Bind() throws RemoteException {
final MockRemoteProvider<String> mockRemoteProviderA = new MockRemoteProvider<String>();