fixes issue 490 -- sets using marker annotations don't work properly all the time.
git-svn-id: https://google-guice.googlecode.com/svn/trunk@1173 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java b/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java
index 1ad9b3d..b846e04 100644
--- a/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java
+++ b/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java
@@ -27,6 +27,7 @@
import com.google.inject.Provider;
import com.google.inject.TypeLiteral;
import com.google.inject.binder.LinkedBindingBuilder;
+import com.google.inject.internal.Annotations;
import com.google.inject.internal.Errors;
import com.google.inject.internal.ImmutableList;
import com.google.inject.internal.ImmutableSet;
@@ -103,7 +104,7 @@
*/
public static <T> Multibinder<T> newSetBinder(Binder binder, TypeLiteral<T> type) {
binder = binder.skipSources(RealMultibinder.class, Multibinder.class);
- RealMultibinder<T> result = new RealMultibinder<T>(binder, type, "",
+ RealMultibinder<T> result = new RealMultibinder<T>(binder, type,
Key.get(Multibinder.<T>setOf(type)));
binder.install(result);
return result;
@@ -124,7 +125,7 @@
public static <T> Multibinder<T> newSetBinder(
Binder binder, TypeLiteral<T> type, Annotation annotation) {
binder = binder.skipSources(RealMultibinder.class, Multibinder.class);
- RealMultibinder<T> result = new RealMultibinder<T>(binder, type, annotation.toString(),
+ RealMultibinder<T> result = new RealMultibinder<T>(binder, type,
Key.get(Multibinder.<T>setOf(type), annotation));
binder.install(result);
return result;
@@ -146,7 +147,7 @@
public static <T> Multibinder<T> newSetBinder(Binder binder, TypeLiteral<T> type,
Class<? extends Annotation> annotationType) {
binder = binder.skipSources(RealMultibinder.class, Multibinder.class);
- RealMultibinder<T> result = new RealMultibinder<T>(binder, type, "@" + annotationType.getName(),
+ RealMultibinder<T> result = new RealMultibinder<T>(binder, type,
Key.get(Multibinder.<T>setOf(type), annotationType));
binder.install(result);
return result;
@@ -227,14 +228,32 @@
/** whether duplicates are allowed. Possibly configured by a different instance */
private boolean permitDuplicates;
- private RealMultibinder(Binder binder, TypeLiteral<T> elementType,
- String setName, Key<Set<T>> setKey) {
+ private RealMultibinder(Binder binder, TypeLiteral<T> elementType, Key<Set<T>> setKey) {
this.binder = checkNotNull(binder, "binder");
this.elementType = checkNotNull(elementType, "elementType");
- this.setName = checkNotNull(setName, "setName");
this.setKey = checkNotNull(setKey, "setKey");
+ this.setName = nameOf(setKey);
this.permitDuplicatesKey = Key.get(Boolean.class, named(toString() + " permits duplicates"));
}
+
+ /**
+ * Returns the name the set should use. This is based on the annotation.
+ * If the annotation has an instance and is not a marker annotation,
+ * we ask the annotation for its toString. If it was a marker annotation
+ * or just an annotation type, we use the annotation's name. Otherwise,
+ * the name is the empty string.
+ */
+ private String nameOf(Key<?> key) {
+ Annotation annotation = setKey.getAnnotation();
+ Class<? extends Annotation> annotationType = setKey.getAnnotationType();
+ if(annotation != null && !Annotations.isMarker(annotationType)) {
+ return setKey.getAnnotation().toString();
+ } else if(setKey.getAnnotationType() != null) {
+ return "@" + setKey.getAnnotationType().getName();
+ } else {
+ return "";
+ }
+ }
@SuppressWarnings("unchecked")
public void configure(Binder binder) {
diff --git a/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java b/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java
index e362bd8..3ba1252 100644
--- a/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java
+++ b/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java
@@ -16,8 +16,11 @@
package com.google.inject.multibindings;
-import com.google.inject.AbstractModule;
import static com.google.inject.Asserts.assertContains;
+import static com.google.inject.name.Names.named;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.AbstractModule;
import com.google.inject.Binding;
import com.google.inject.BindingAnnotation;
import com.google.inject.ConfigurationException;
@@ -34,13 +37,19 @@
import com.google.inject.internal.ImmutableSet;
import com.google.inject.internal.Maps;
import com.google.inject.name.Names;
-import static com.google.inject.name.Names.named;
import com.google.inject.spi.Dependency;
import com.google.inject.spi.HasDependencies;
import com.google.inject.util.Modules;
import com.google.inject.util.Providers;
+
+import junit.framework.TestCase;
+
+import java.lang.annotation.Annotation;
+import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
@@ -48,7 +57,6 @@
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
-import junit.framework.TestCase;
/**
* @author dpb@google.com (David P. Baker)
@@ -569,4 +577,46 @@
private <V> Set<V> setOf(Object... elements) {
return new HashSet(Arrays.asList(elements));
}
+
+ @BindingAnnotation
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target({ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD})
+ private static @interface Marker {}
+
+ @Marker
+ public void testMapBinderMatching() throws Exception {
+ Method m = MapBinderTest.class.getDeclaredMethod("testMapBinderMatching");
+ assertNotNull(m);
+ final Annotation marker = m.getAnnotation(Marker.class);
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override public void configure() {
+ MapBinder<Integer, Integer> mb1 =
+ MapBinder.newMapBinder(binder(), Integer.class, Integer.class, Marker.class);
+ MapBinder<Integer, Integer> mb2 =
+ MapBinder.newMapBinder(binder(), Integer.class, Integer.class, marker);
+ mb1.addBinding(1).toInstance(1);
+ mb2.addBinding(2).toInstance(2);
+
+ // This assures us that the two binders are equivalent, so we expect the instance added to
+ // each to have been added to one set.
+ assertEquals(mb1, mb2);
+ }
+ });
+ TypeLiteral<Map<Integer, Integer>> t = new TypeLiteral<Map<Integer, Integer>>() {};
+ Map<Integer, Integer> s1 = injector.getInstance(Key.get(t, Marker.class));
+ Map<Integer, Integer> s2 = injector.getInstance(Key.get(t, marker));
+
+ // This assures us that the two sets are in fact equal. They may not be same set (as in Java
+ // object identical), but we shouldn't expect that, since probably Guice creates the set each
+ // time in case the elements are dependent on scope.
+ assertEquals(s1, s2);
+
+ // This ensures that MultiBinder is internally using the correct set name --
+ // making sure that instances of marker annotations have the same set name as
+ // MarkerAnnotation.class.
+ Map<Integer, Integer> expected = new HashMap<Integer, Integer>();
+ expected.put(1, 1);
+ expected.put(2, 2);
+ assertEquals(expected, s1);
+ }
}
diff --git a/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java b/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java
index 6046152..0cab926 100644
--- a/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java
+++ b/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java
@@ -18,6 +18,8 @@
import com.google.inject.AbstractModule;
import static com.google.inject.Asserts.assertContains;
+
+import com.google.inject.Binder;
import com.google.inject.Binding;
import com.google.inject.BindingAnnotation;
import com.google.inject.CreationException;
@@ -41,10 +43,18 @@
import com.google.inject.spi.LinkedKeyBinding;
import com.google.inject.util.Modules;
import com.google.inject.util.Providers;
+
+import java.lang.annotation.Annotation;
+import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.lang.reflect.Method;
+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import java.util.Arrays;
import java.util.Collections;
+import java.util.HashSet;
import java.util.List;
import java.util.Set;
import junit.framework.TestCase;
@@ -495,5 +505,45 @@
assertEquals(ImmutableSet.of("A", "B", "C", "D", "E", "F"),
injector.getInstance(Key.get(setOfString)));
+ }
+
+ @BindingAnnotation
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target({ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD})
+ private static @interface Marker {}
+
+ @Marker
+ public void testMultibinderMatching() throws Exception {
+ Method m = MultibinderTest.class.getDeclaredMethod("testMultibinderMatching");
+ assertNotNull(m);
+ final Annotation marker = m.getAnnotation(Marker.class);
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override public void configure() {
+ Multibinder<Integer> mb1 = Multibinder.newSetBinder(binder(), Integer.class, Marker.class);
+ Multibinder<Integer> mb2 = Multibinder.newSetBinder(binder(), Integer.class, marker);
+ mb1.addBinding().toInstance(1);
+ mb2.addBinding().toInstance(2);
+
+ // This assures us that the two binders are equivalent, so we expect the instance added to
+ // each to have been added to one set.
+ assertEquals(mb1, mb2);
+ }
+ });
+ TypeLiteral<Set<Integer>> t = new TypeLiteral<Set<Integer>>() {};
+ Set<Integer> s1 = injector.getInstance(Key.get(t, Marker.class));
+ Set<Integer> s2 = injector.getInstance(Key.get(t, marker));
+
+ // This assures us that the two sets are in fact equal. They may not be same set (as in Java
+ // object identical), but we shouldn't expect that, since probably Guice creates the set each
+ // time in case the elements are dependent on scope.
+ assertEquals(s1, s2);
+
+ // This ensures that MultiBinder is internally using the correct set name --
+ // making sure that instances of marker annotations have the same set name as
+ // MarkerAnnotation.class.
+ Set<Integer> expected = new HashSet<Integer>();
+ expected.add(1);
+ expected.add(2);
+ assertEquals(expected, s1);
}
}
diff --git a/src/com/google/inject/Key.java b/src/com/google/inject/Key.java
index 05d7b25..bc78b2c 100644
--- a/src/com/google/inject/Key.java
+++ b/src/com/google/inject/Key.java
@@ -320,13 +320,6 @@
}
/**
- * Returns {@code true} if the given annotation type has no attributes.
- */
- static boolean isMarker(Class<? extends Annotation> annotationType) {
- return annotationType.getDeclaredMethods().length == 0;
- }
-
- /**
* Gets the strategy for an annotation.
*/
static AnnotationStrategy strategyFor(Annotation annotation) {
@@ -335,7 +328,7 @@
ensureRetainedAtRuntime(annotationType);
ensureIsBindingAnnotation(annotationType);
- if (isMarker(annotationType)) {
+ if (Annotations.isMarker(annotationType)) {
return new AnnotationTypeStrategy(annotationType, annotation);
}
diff --git a/src/com/google/inject/internal/Annotations.java b/src/com/google/inject/internal/Annotations.java
index f748aef..d76d32a 100644
--- a/src/com/google/inject/internal/Annotations.java
+++ b/src/com/google/inject/internal/Annotations.java
@@ -37,6 +37,13 @@
public class Annotations {
/**
+ * Returns {@code true} if the given annotation type has no attributes.
+ */
+ public static boolean isMarker(Class<? extends Annotation> annotationType) {
+ return annotationType.getDeclaredMethods().length == 0;
+ }
+
+ /**
* Returns true if the given annotation is retained at runtime.
*/
public static boolean isRetainedAtRuntime(Class<? extends Annotation> annotationType) {
