Merge "Fix bugs related to type-matching for shared Maps, Collections, Enums"
diff --git a/src/com/android/tradefed/config/OptionSetter.java b/src/com/android/tradefed/config/OptionSetter.java
index 90eb747..d0efbcc 100644
--- a/src/com/android/tradefed/config/OptionSetter.java
+++ b/src/com/android/tradefed/config/OptionSetter.java
@@ -19,6 +19,8 @@
import com.android.ddmlib.Log;
import com.android.tradefed.util.ArrayUtil;
+import com.google.common.base.Objects;
+
import java.io.File;
import java.lang.reflect.Field;
import java.lang.reflect.ParameterizedType;
@@ -116,15 +118,23 @@
}
if (type instanceof Class) {
Class<?> cType = (Class<?>) type;
- if (Collection.class.isAssignableFrom(cType)) {
+
+ if (cType.isEnum()) {
+ return new EnumHandler(cType);
+ } else if (Collection.class.isAssignableFrom(cType)) {
// could handle by just having a default of treating
// contents as String but consciously decided this
// should be an error
- throw new ConfigurationException(
- String.format("Cannot handle non-parameterized collection %s. %s", type,
- "use a generic Collection to specify a desired element type"));
- } else if (cType.isEnum()) {
- return new EnumHandler(cType);
+ throw new ConfigurationException(String.format(
+ "Cannot handle non-parameterized collection %s. Use a generic Collection "
+ + "to specify a desired element type.", type));
+ } else if (Map.class.isAssignableFrom(cType)) {
+ // could handle by just having a default of treating
+ // contents as String but consciously decided this
+ // should be an error
+ throw new ConfigurationException(String.format(
+ "Cannot handle non-parameterized map %s. Use a generic Map to specify "
+ + "desired element types.", type));
}
return handlers.get(cType);
}
@@ -147,8 +157,8 @@
void addField(String name, Object source, Field field) throws ConfigurationException {
if (size() > 0) {
- Handler existingFieldHandler = getHandler(getFirstField().getType());
- Handler newFieldHandler = getHandler(field.getType());
+ Handler existingFieldHandler = getHandler(getFirstField().getGenericType());
+ Handler newFieldHandler = getHandler(field.getGenericType());
if (!existingFieldHandler.equals(newFieldHandler)) {
throw new ConfigurationException(String.format(
"@Option field with name '%s' in class '%s' is defined with a " +
@@ -564,6 +574,7 @@
fields = new OptionFieldsForName();
optionMap.put(name, fields);
}
+
fields.addField(name, optionSource, field);
if (getHandler(field.getGenericType()) == null) {
throw new ConfigurationException(String.format(
@@ -731,11 +742,53 @@
mValueHandler = valueHandler;
}
+ Handler getKeyHandler() {
+ return mKeyHandler;
+ }
+
+ Handler getValueHandler() {
+ return mValueHandler;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
@Override
boolean isMap() {
return true;
}
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(MapHandler.class, mKeyHandler, mValueHandler);
+ }
+
+ /**
+ * Define two {@link MapHandler}s as equivalent if their key and value Handlers are
+ * respectively equivalent.
+ * <p />
+ * {@inheritDoc}
+ */
+ @Override
+ public boolean equals(Object otherObj) {
+ if ((otherObj != null) && (otherObj instanceof MapHandler)) {
+ MapHandler other = (MapHandler) otherObj;
+ Handler otherKeyHandler = other.getKeyHandler();
+ Handler otherValueHandler = other.getValueHandler();
+
+ return mKeyHandler.equals(otherKeyHandler)
+ && mValueHandler.equals(otherValueHandler);
+ }
+
+ return false;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
@Override
Object translate(String valueText) {
return null;
@@ -755,16 +808,48 @@
}
/**
- * @ {@link Handler} to handle values for {@link Enum} fields.
+ * A {@link Handler} to handle values for {@link Enum} fields.
*/
private static class EnumHandler extends Handler {
- private final Class/*<?>*/ mEnumType;
+ private final Class mEnumType;
EnumHandler(Class<?> enumType) {
mEnumType = enumType;
}
+ Class getEnumType() {
+ return mEnumType;
+ }
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(EnumHandler.class, mEnumType);
+ }
+
+ /**
+ * Define two EnumHandlers as equivalent if their EnumTypes are mutually assignable
+ * <p />
+ * {@inheritDoc}
+ */
+ @Override
+ public boolean equals(Object otherObj) {
+ if ((otherObj != null) && (otherObj instanceof EnumHandler)) {
+ EnumHandler other = (EnumHandler) otherObj;
+ Class otherType = other.getEnumType();
+
+ return mEnumType.isAssignableFrom(otherType)
+ && otherType.isAssignableFrom(mEnumType);
+ }
+
+ return false;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
@Override
Object translate(String valueText) {
return translate(valueText, true);
diff --git a/tests/src/com/android/tradefed/config/OptionSetterTest.java b/tests/src/com/android/tradefed/config/OptionSetterTest.java
index 14ddc26..786ade7 100644
--- a/tests/src/com/android/tradefed/config/OptionSetterTest.java
+++ b/tests/src/com/android/tradefed/config/OptionSetterTest.java
@@ -76,6 +76,20 @@
@SuppressWarnings("unused")
@Option(name = "string", shortName = 's')
private String mMyOption;
+
+ @Option(name = "enum")
+ private DefaultEnumClass mEnum = null;
+
+ @Option(name = "string_collection")
+ private Collection<String> mStringCollection = new ArrayList<String>();
+
+ @Option(name = "enumMap")
+ private Map<DefaultEnumClass, CustomEnumClass> mEnumMap =
+ new HashMap<DefaultEnumClass, CustomEnumClass>();
+
+ @Option(name = "enumCollection")
+ private Collection<DefaultEnumClass> mEnumCollection =
+ new ArrayList<DefaultEnumClass>();
}
/**
@@ -149,6 +163,14 @@
@Option(name = "customEnum")
private CustomEnumClass mCustomEnum = null;
+
+ @Option(name = "enumMap")
+ private Map<DefaultEnumClass, CustomEnumClass> mEnumMap =
+ new HashMap<DefaultEnumClass, CustomEnumClass>();
+
+ @Option(name = "enumCollection")
+ private Collection<DefaultEnumClass> mEnumCollection =
+ new ArrayList<DefaultEnumClass>();
}
private static class ParentOptionSource {
@@ -229,6 +251,60 @@
}
/**
+ * Test {@link OptionSetter#setOptionValue(String, String)} for Enums used as the key and value
+ * of a {@link Map}.
+ */
+ public void testOptionSetter_sharedEnumMap() throws ConfigurationException {
+ AllTypesOptionSource object1 = new AllTypesOptionSource();
+ SharedOptionSource object2 = new SharedOptionSource();
+
+ final String key = "VAL1";
+ final String value = "VAL1";
+ final DefaultEnumClass expectedKey = DefaultEnumClass.VAL1;
+ final CustomEnumClass expectedValue = CustomEnumClass.VAL1;
+
+ // Actually set the key/value pair
+ OptionSetter parser = new OptionSetter(object1, object2);
+ parser.setOptionMapValue("enumMap", key, value);
+
+ // verify object1
+ assertEquals(1, object1.mEnumMap.size());
+ assertNotNull(object1.mEnumMap.get(expectedKey));
+ assertEquals(expectedValue, object1.mEnumMap.get(expectedKey));
+
+ // verify object2
+ assertEquals(1, object2.mEnumMap.size());
+ assertNotNull(object2.mEnumMap.get(expectedKey));
+ assertEquals(expectedValue, object2.mEnumMap.get(expectedKey));
+ }
+
+
+ /**
+ * Test {@link OptionSetter#setOptionValue(String, String)} for Enums used as the key and value
+ * of a {@link Map}.
+ */
+ public void testOptionSetter_sharedEnumCollection() throws ConfigurationException {
+ AllTypesOptionSource object1 = new AllTypesOptionSource();
+ SharedOptionSource object2 = new SharedOptionSource();
+
+ final String value = "VAL1";
+ final DefaultEnumClass expectedValue = DefaultEnumClass.VAL1;
+
+ // Actually add the element
+ OptionSetter parser = new OptionSetter(object1, object2);
+ parser.setOptionValue("enumCollection", value);
+
+ // verify object1
+ assertEquals(1, object1.mEnumCollection.size());
+ assertTrue(object1.mEnumCollection.contains(expectedValue));
+
+ // verify object2
+ assertEquals(1, object2.mEnumCollection.size());
+ assertTrue(object2.mEnumCollection.contains(expectedValue));
+ }
+
+
+ /**
* Test that multiple options with same name must have the same type.
*/
public void testOptionSetter_sharedOptionsDiffType() throws ConfigurationException {
@@ -657,6 +733,45 @@
}
/**
+ * Test {@link OptionSetter#setOptionValue(String, String)} for Enums used as the key and value
+ * of a {@link Map}.
+ */
+ public void testSetOptionValue_enumMap() throws ConfigurationException {
+ AllTypesOptionSource optionSource = new AllTypesOptionSource();
+
+ final String key = "VAL1";
+ final String value = "VAL1";
+ final DefaultEnumClass expectedKey = DefaultEnumClass.VAL1;
+ final CustomEnumClass expectedValue = CustomEnumClass.VAL1;
+
+ // Actually set the key/value pair
+ OptionSetter parser = new OptionSetter(optionSource);
+ parser.setOptionMapValue("enumMap", key, value);
+
+ assertEquals(1, optionSource.mEnumMap.size());
+ assertNotNull(optionSource.mEnumMap.get(expectedKey));
+ assertEquals(expectedValue, optionSource.mEnumMap.get(expectedKey));
+ }
+
+
+ /**
+ * Test {@link OptionSetter#setOptionValue(String, String)} for Enums used as the key and value
+ * of a {@link Map}.
+ */
+ public void testSetOptionValue_enumCollection() throws ConfigurationException {
+ AllTypesOptionSource optionSource = new AllTypesOptionSource();
+
+ final String value = "VAL1";
+ final DefaultEnumClass expectedValue = DefaultEnumClass.VAL1;
+
+ assertSetOptionValue(optionSource, "enumCollection", value);
+
+ assertEquals(1, optionSource.mEnumCollection.size());
+ assertTrue(optionSource.mEnumCollection.contains(expectedValue));
+ }
+
+
+ /**
* Test {@link OptionSetter#setOptionValue(String, String)} for an Enum.
*/
public void testSetOptionValue_enumBadValue() throws ConfigurationException {