Give more information when duplicate elements are found in a multibound set.
Second attempt at [], which was rolled back in [], because it broke serializability. Turns out we were using Collections.immutableSet, not ImmutableSet.copyOf. Grrr.
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=50346773
diff --git a/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java b/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java
index 59fe82d..3b5284d 100644
--- a/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java
+++ b/extensions/multibindings/src/com/google/inject/multibindings/Multibinder.java
@@ -16,16 +16,13 @@
package com.google.inject.multibindings;
+import static com.google.common.base.Predicates.equalTo;
+import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.getOnlyElement;
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.base.Objects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
@@ -50,6 +47,13 @@
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.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+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
@@ -323,14 +327,16 @@
public Set<T> get() {
checkConfiguration(isInitialized(), "Multibinder is not initialized");
- Set<T> result = new LinkedHashSet<T>();
+ Map<T, Binding<T>> result = new LinkedHashMap<T, Binding<T>>();
for (Binding<T> binding : bindings) {
final T newValue = binding.getProvider().get();
checkConfiguration(newValue != null, "Set injection failed due to null element");
- checkConfiguration(result.add(newValue) || permitDuplicates,
- "Set injection failed due to duplicated element \"%s\"", newValue);
+ Binding<T> duplicateBinding = result.put(newValue, binding);
+ if (!permitDuplicates && duplicateBinding != null) {
+ throw newDuplicateValuesException(result, binding, newValue, duplicateBinding);
+ }
}
- return Collections.unmodifiableSet(result);
+ return ImmutableSet.copyOf(result.keySet());
}
@SuppressWarnings("unchecked")
@@ -447,6 +453,35 @@
throw new ConfigurationException(ImmutableSet.of(new Message(Errors.format(format, args))));
}
+ private static <T> ConfigurationException newDuplicateValuesException(
+ Map<T, Binding<T>> existingBindings,
+ Binding<T> binding,
+ final T newValue,
+ Binding<T> duplicateBinding) {
+ T oldValue = getOnlyElement(filter(existingBindings.keySet(), equalTo(newValue)));
+ String oldString = oldValue.toString();
+ String newString = newValue.toString();
+ if (Objects.equal(oldString, newString)) {
+ // When the value strings match, just show the source of the bindings
+ return new ConfigurationException(ImmutableSet.of(new Message(Errors.format(
+ "Set injection failed due to duplicated element \"%s\""
+ + "\n Bound at %s\n Bound at %s",
+ newValue,
+ duplicateBinding.getSource(),
+ binding.getSource()))));
+ } else {
+ // When the value strings don't match, include them both as they may be useful for debugging
+ return new ConfigurationException(ImmutableSet.of(new Message(Errors.format(
+ "Set injection failed due to multiple elements comparing equal:"
+ + "\n \"%s\"\n bound at %s"
+ + "\n \"%s\"\n bound at %s",
+ oldValue,
+ duplicateBinding.getSource(),
+ newValue,
+ binding.getSource()))));
+ }
+ }
+
static <T> T checkNotNull(T reference, String name) {
if (reference != null) {
return reference;
diff --git a/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java b/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java
index 4e1d3f1..28ed1ce 100644
--- a/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java
+++ b/extensions/multibindings/test/com/google/inject/multibindings/MultibinderTest.java
@@ -48,10 +48,14 @@
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;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
import java.lang.annotation.Annotation;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
@@ -254,6 +258,32 @@
}
}
+ public void testMultibinderSetIsSerializable() throws IOException, ClassNotFoundException {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ protected void configure() {
+ Multibinder.newSetBinder(binder(), String.class)
+ .addBinding().toInstance("A");
+ }
+ });
+
+ Set<String> set = injector.getInstance(Key.get(setOfString));
+ ByteArrayOutputStream byteStream = new ByteArrayOutputStream();
+ ObjectOutputStream objectOutputStream = new ObjectOutputStream(byteStream);
+ try {
+ objectOutputStream.writeObject(set);
+ } finally {
+ objectOutputStream.close();
+ }
+ ObjectInputStream objectInputStream = new ObjectInputStream(
+ new ByteArrayInputStream(byteStream.toByteArray()));
+ try {
+ Object setCopy = objectInputStream.readObject();
+ assertEquals(set, setCopy);
+ } finally {
+ objectInputStream.close();
+ }
+ }
+
public void testMultibinderSetIsLazy() {
Module module = new AbstractModule() {
protected void configure() {
@@ -275,28 +305,94 @@
}
public void testMultibinderSetForbidsDuplicateElements() {
- Module module = new AbstractModule() {
+ Module module1 = new AbstractModule() {
protected void configure() {
final Multibinder<String> multibinder = Multibinder.newSetBinder(binder(), String.class);
multibinder.addBinding().toInstance("A");
+ }
+ };
+ Module module2 = new AbstractModule() {
+ protected void configure() {
+ final Multibinder<String> multibinder = Multibinder.newSetBinder(binder(), String.class);
multibinder.addBinding().toInstance("A");
}
};
- Injector injector = Guice.createInjector(module);
+ Injector injector = Guice.createInjector(module1, module2);
try {
injector.getInstance(Key.get(setOfString));
fail();
- } catch(ProvisionException expected) {
+ } catch (ProvisionException expected) {
assertContains(expected.getMessage(),
- "1) Set injection failed due to duplicated element \"A\"");
+ "1) Set injection failed due to duplicated element \"A\"",
+ "Bound at " + module1.getClass().getName(),
+ "Bound at " + module2.getClass().getName());
}
// But we can still visit the module!
- assertSetVisitor(Key.get(setOfString), stringType, setOf(module), MODULE, false, 0,
+ assertSetVisitor(Key.get(setOfString), stringType, setOf(module1, module2), MODULE, false, 0,
instance("A"), instance("A"));
}
+ public void testMultibinderSetShowsBothElementsIfToStringDifferent() {
+ // A simple example of a type whose toString returns more information than its equals method
+ // considers.
+ class ValueType {
+ int a;
+ int b;
+ ValueType(int a, int b) {
+ this.a = a;
+ this.b = b;
+ }
+ @Override
+ public boolean equals(Object obj) {
+ return (obj instanceof ValueType) && (((ValueType) obj).a == a);
+ }
+ @Override
+ public int hashCode() {
+ return a;
+ }
+ @Override
+ public String toString() {
+ return String.format("ValueType(%d,%d)", a, b);
+ }
+ }
+
+ Module module1 = new AbstractModule() {
+ protected void configure() {
+ final Multibinder<ValueType> multibinder =
+ Multibinder.newSetBinder(binder(), ValueType.class);
+ multibinder.addBinding().toInstance(new ValueType(1, 2));
+ }
+ };
+ Module module2 = new AbstractModule() {
+ protected void configure() {
+ final Multibinder<ValueType> multibinder =
+ Multibinder.newSetBinder(binder(), ValueType.class);
+ multibinder.addBinding().toInstance(new ValueType(1, 3));
+ }
+ };
+ Injector injector = Guice.createInjector(module1, module2);
+
+ TypeLiteral<ValueType> valueType = TypeLiteral.get(ValueType.class);
+ TypeLiteral<Set<ValueType>> setOfValueType = new TypeLiteral<Set<ValueType>>() {};
+ try {
+ injector.getInstance(Key.get(setOfValueType));
+ fail();
+ } catch (ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "1) Set injection failed due to multiple elements comparing equal:",
+ "\"ValueType(1,2)\"",
+ "bound at " + module1.getClass().getName(),
+ "\"ValueType(1,3)\"",
+ "bound at " + module2.getClass().getName());
+ }
+
+ // But we can still visit the module!
+ assertSetVisitor(Key.get(setOfValueType), valueType, setOf(module1, module2), MODULE, false, 0,
+ instance(new ValueType(1, 2)), instance(new ValueType(1, 3)));
+ }
+
public void testMultibinderSetPermitDuplicateElements() {
Module ab = new AbstractModule() {
protected void configure() {