Revision: 21a19670ce07
Author:   Sam Berlin <[email protected]>
Date:     Sat Jan 21 08:06:49 2012
Log:      Fix issue 670, keep values from MapBinder & Multibinder distinct.
http://code.google.com/p/google-guice/source/detail?r=21a19670ce07

Modified:
 /extensions/multibindings/src/com/google/inject/multibindings/Element.java
/extensions/multibindings/src/com/google/inject/multibindings/MapBinder.java /extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java /extensions/multibindings/src/com/google/inject/multibindings/RealElement.java /extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java

=======================================
--- /extensions/multibindings/src/com/google/inject/multibindings/Element.java Thu Jul 7 17:34:16 2011 +++ /extensions/multibindings/src/com/google/inject/multibindings/Element.java Sat Jan 21 08:06:49 2012
@@ -32,6 +32,13 @@
  */
 @Retention(RUNTIME) @BindingAnnotation
 @interface Element {
+
+  enum Type {
+    MAPBINDER,
+    MULTIBINDER;
+  }
+
   String setName();
   int uniqueId();
-}
+  Type type();
+}
=======================================
--- /extensions/multibindings/src/com/google/inject/multibindings/MapBinder.java Thu Jul 7 17:34:16 2011 +++ /extensions/multibindings/src/com/google/inject/multibindings/MapBinder.java Sat Jan 21 08:06:49 2012
@@ -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);
     }

=======================================
--- /extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java Wed Nov 9 15:47:57 2011 +++ /extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java Sat Jan 21 08:06:49 2012
@@ -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() {
=======================================
--- /extensions/multibindings/src/com/google/inject/multibindings/RealElement.java Fri May 16 07:53:49 2008 +++ /extensions/multibindings/src/com/google/inject/multibindings/RealElement.java Sat Jan 21 08:06:49 2012
@@ -27,10 +27,12 @@

   private final int uniqueId;
   private final String setName;
-
-  RealElement(String setName) {
+  private final Type type;
+
+  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());
   }
 }
=======================================
--- /extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java Fri Jan 13 15:22:35 2012 +++ /extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java Sat Jan 21 08:06:49 2012
@@ -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;
+    }
+  }
+}

--
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.

Reply via email to