Fix issue 670, keep values from MapBinder & Multibinder distinct.
diff --git a/extensions/multibindings/src/com/google/inject/multibindings/Element.java b/extensions/multibindings/src/com/google/inject/multibindings/Element.java
index 3149865..21126ea 100644
--- a/extensions/multibindings/src/com/google/inject/multibindings/Element.java
+++ b/extensions/multibindings/src/com/google/inject/multibindings/Element.java
@@ -32,6 +32,13 @@
*/
@Retention(RUNTIME) @BindingAnnotation
@interface Element {
+
+ enum Type {
+ MAPBINDER,
+ MULTIBINDER;
+ }
+
String setName();
int uniqueId();
+ Type type();
}
diff --git a/extensions/multibindings/src/com/google/inject/multibindings/MapBinder.java b/extensions/multibindings/src/com/google/inject/multibindings/MapBinder.java
index 9a15efd..7c71e75 100644
--- a/extensions/multibindings/src/com/google/inject/multibindings/MapBinder.java
+++ b/extensions/multibindings/src/com/google/inject/multibindings/MapBinder.java
@@ -16,11 +16,20 @@
package com.google.inject.multibindings;
+import static com.google.inject.multibindings.Element.Type.MAPBINDER;
import static com.google.inject.multibindings.Multibinder.checkConfiguration;
import static com.google.inject.multibindings.Multibinder.checkNotNull;
import static com.google.inject.multibindings.Multibinder.setOf;
import static com.google.inject.util.Types.newParameterizedTypeWithOwner;
+import java.lang.annotation.Annotation;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -44,14 +53,6 @@
import com.google.inject.spi.Toolable;
import com.google.inject.util.Types;
-import java.lang.annotation.Annotation;
-import java.util.Collections;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Set;
-
/**
* An API to bind multiple map entries separately, only to later inject them as
* a complete map. MapBinder is intended for use in your application's module:
@@ -333,7 +334,7 @@
checkNotNull(key, "key");
checkConfiguration(!isInitialized(), "MapBinder was already initialized");
- Key<V> valueKey = Key.get(valueType, new RealElement(entrySetBinder.getSetName()));
+ Key<V> valueKey = Key.get(valueType, new RealElement(entrySetBinder.getSetName(), MAPBINDER));
entrySetBinder.addBinding().toInstance(new MapEntry<K, Provider<V>>(key,
binder.getProvider(valueKey), valueKey));
return binder.bind(valueKey);
@@ -465,6 +466,7 @@
private boolean matchesValueKey(Key key) {
return key.getAnnotation() instanceof Element
&& ((Element) key.getAnnotation()).setName().equals(entrySetBinder.getSetName())
+ && ((Element) key.getAnnotation()).type() == MAPBINDER
&& key.getTypeLiteral().equals(valueType);
}
diff --git a/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java b/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java
index a066e33..59fe82d 100644
--- a/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java
+++ b/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java
@@ -16,8 +16,16 @@
package com.google.inject.multibindings;
+import static com.google.inject.multibindings.Element.Type.MULTIBINDER;
import static com.google.inject.name.Names.named;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
@@ -42,13 +50,6 @@
import com.google.inject.spi.Toolable;
import com.google.inject.util.Types;
-import java.lang.annotation.Annotation;
-import java.lang.reflect.Type;
-import java.util.Collections;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Set;
-
/**
* An API to bind multiple values separately, only to later inject them as a
* complete collection. Multibinder is intended for use in your application's
@@ -263,7 +264,6 @@
}
}
- @SuppressWarnings("unchecked")
public void configure(Binder binder) {
checkConfiguration(!isInitialized(), "Multibinder was already initialized");
@@ -279,7 +279,7 @@
@Override public LinkedBindingBuilder<T> addBinding() {
checkConfiguration(!isInitialized(), "Multibinder was already initialized");
- return binder.bind(Key.get(elementType, new RealElement(setName)));
+ return binder.bind(Key.get(elementType, new RealElement(setName, MULTIBINDER)));
}
/**
@@ -312,7 +312,8 @@
private boolean keyMatches(Key<?> key) {
return key.getTypeLiteral().equals(elementType)
&& key.getAnnotation() instanceof Element
- && ((Element) key.getAnnotation()).setName().equals(setName);
+ && ((Element) key.getAnnotation()).setName().equals(setName)
+ && ((Element) key.getAnnotation()).type() == MULTIBINDER;
}
private boolean isInitialized() {
diff --git a/extensions/multibindings/src/com/google/inject/multibindings/RealElement.java b/extensions/multibindings/src/com/google/inject/multibindings/RealElement.java
index d297f90..cf7b382 100644
--- a/extensions/multibindings/src/com/google/inject/multibindings/RealElement.java
+++ b/extensions/multibindings/src/com/google/inject/multibindings/RealElement.java
@@ -27,10 +27,12 @@
private final int uniqueId;
private final String setName;
+ private final Type type;
- RealElement(String setName) {
+ RealElement(String setName, Type type) {
uniqueId = nextUniqueId.getAndIncrement();
this.setName = setName;
+ this.type = type;
}
public String setName() {
@@ -40,6 +42,10 @@
public int uniqueId() {
return uniqueId;
}
+
+ public Type type() {
+ return type;
+ }
public Class<? extends Annotation> annotationType() {
return Element.class;
@@ -47,17 +53,19 @@
@Override public String toString() {
return "@" + Element.class.getName() + "(setName=" + setName
- + ",uniqueId=" + uniqueId + ")";
+ + ",uniqueId=" + uniqueId + ", type=" + type + ")";
}
@Override public boolean equals(Object o) {
return o instanceof Element
&& ((Element) o).setName().equals(setName())
- && ((Element) o).uniqueId() == uniqueId();
+ && ((Element) o).uniqueId() == uniqueId()
+ && ((Element) o).type() == type();
}
@Override public int hashCode() {
return 127 * ("setName".hashCode() ^ setName.hashCode())
- + 127 * ("uniqueId".hashCode() ^ uniqueId);
+ + 127 * ("uniqueId".hashCode() ^ uniqueId)
+ + 127 * ("type".hashCode() ^ type.hashCode());
}
}
diff --git a/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java b/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java
index 652fb89..4e1d3f1 100644
--- a/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java
+++ b/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java
@@ -41,12 +41,14 @@
import com.google.inject.Stage;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Names;
+import com.google.inject.spi.DefaultBindingTargetVisitor;
import com.google.inject.spi.Dependency;
import com.google.inject.spi.HasDependencies;
import com.google.inject.spi.InstanceBinding;
import com.google.inject.spi.LinkedKeyBinding;
import com.google.inject.util.Modules;
import com.google.inject.util.Providers;
+import com.google.inject.util.Types;
import junit.framework.TestCase;
@@ -60,6 +62,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Set;
/**
@@ -67,6 +70,8 @@
*/
public class MultibinderTest extends TestCase {
+ final TypeLiteral<Map<String, String>> mapOfStringString =
+ new TypeLiteral<Map<String, String>>() {};
final TypeLiteral<Set<String>> setOfString = new TypeLiteral<Set<String>>() {};
final TypeLiteral<Set<Integer>> setOfInteger = new TypeLiteral<Set<Integer>>() {};
final TypeLiteral<String> stringType = TypeLiteral.get(String.class);
@@ -600,7 +605,8 @@
assertEquals(expected, s1);
}
- public void failing_testSetAndMapValueConflict() {
+ // See issue 670
+ public void testSetAndMapValueAreDistinct() {
Injector injector = Guice.createInjector(new AbstractModule() {
@Override protected void configure() {
Multibinder.newSetBinder(binder(), String.class)
@@ -613,4 +619,61 @@
assertEquals(ImmutableSet.<String>of("A"), injector.getInstance(Key.get(setOfString)));
}
+
+ // See issue 670
+ public void testSetAndMapValueAreDistinctInSpi() {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override protected void configure() {
+ Multibinder.newSetBinder(binder(), String.class)
+ .addBinding().toInstance("A");
+
+ MapBinder.newMapBinder(binder(), String.class, String.class)
+ .addBinding("B").toInstance("b");
+ }
+ });
+ Collector collector = new Collector();
+ Binding<Map<String, String>> mapbinding = injector.getBinding(Key.get(mapOfStringString));
+ mapbinding.acceptTargetVisitor(collector);
+ assertNotNull(collector.mapbinding);
+
+ Binding<Set<String>> setbinding = injector.getBinding(Key.get(setOfString));
+ setbinding.acceptTargetVisitor(collector);
+ assertNotNull(collector.setbinding);
+
+ // Capture the value bindings and assert we have them right --
+ // they'll always be in the right order because we preserve
+ // binding order.
+ List<Binding<String>> bindings = injector.findBindingsByType(stringType);
+ assertEquals(2, bindings.size());
+ Binding<String> a = bindings.get(0);
+ Binding<String> b = bindings.get(1);
+ assertEquals("A", ((InstanceBinding<String>)a).getInstance());
+ assertEquals("b", ((InstanceBinding<String>)b).getInstance());
+
+ // Now make sure "A" belongs only to the set binding,
+ // and "b" only belongs to the map binding.
+ assertFalse(collector.mapbinding.containsElement(a));
+ assertTrue(collector.mapbinding.containsElement(b));
+
+ assertTrue(collector.setbinding.containsElement(a));
+ assertFalse(collector.setbinding.containsElement(b));
+ }
+
+ private static class Collector extends DefaultBindingTargetVisitor<Object, Object> implements
+ MultibindingsTargetVisitor<Object, Object> {
+ MapBinderBinding<? extends Object> mapbinding;
+ MultibinderBinding<? extends Object> setbinding;
+
+ @Override
+ public Object visit(MapBinderBinding<? extends Object> mapbinding) {
+ this.mapbinding = mapbinding;
+ return null;
+ }
+
+ @Override
+ public Object visit(MultibinderBinding<? extends Object> multibinding) {
+ this.setbinding = multibinding;
+ return null;
+ }
+ }
}
