beam-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lc...@apache.org
Subject [1/3] incubator-beam git commit: Allow empty string value for ValueProvider types.
Date Wed, 14 Dec 2016 06:48:02 GMT
Repository: incubator-beam
Updated Branches:
  refs/heads/master f516627e0 -> 5a51ace8d


Allow empty string value for ValueProvider types.


Project: http://git-wip-us.apache.org/repos/asf/incubator-beam/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-beam/commit/ae1d2a30
Tree: http://git-wip-us.apache.org/repos/asf/incubator-beam/tree/ae1d2a30
Diff: http://git-wip-us.apache.org/repos/asf/incubator-beam/diff/ae1d2a30

Branch: refs/heads/master
Commit: ae1d2a308f5867d0a0dad536a79b605a7d590ec3
Parents: f516627
Author: Vikas Kedigehalli <vikasrk@google.com>
Authored: Mon Dec 12 12:29:54 2016 -0800
Committer: Luke Cwik <lcwik@google.com>
Committed: Tue Dec 13 22:37:22 2016 -0800

----------------------------------------------------------------------
 .../sdk/options/PipelineOptionsFactory.java     | 108 ++++++---
 .../sdk/options/PipelineOptionsFactoryTest.java | 223 ++++++++++++++++---
 2 files changed, 278 insertions(+), 53 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/ae1d2a30/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
----------------------------------------------------------------------
diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
index 9805489..2d013fd 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
@@ -55,7 +55,6 @@ import java.io.PrintStream;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
-import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Proxy;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
@@ -1610,13 +1609,7 @@ public class PipelineOptionsFactory {
               throw new IllegalArgumentException(msg, e);
             }
           }
-        } else if ((returnType.isArray() && (SIMPLE_TYPES.contains(returnType.getComponentType())
-                   || returnType.getComponentType().isEnum()))
-                   || Collection.class.isAssignableFrom(returnType)
-                   || (returnType.equals(ValueProvider.class)
-                       && MAPPER.getTypeFactory().constructType(
-                         ((ParameterizedType) method.getGenericReturnType())
-                         .getActualTypeArguments()[0]).isCollectionLikeType())) {
+        } else if (isCollectionOrArrayOfAllowedTypes(returnType, type)) {
           // Split any strings with ","
           List<String> values = FluentIterable.from(entry.getValue())
               .transformAndConcat(new Function<String, Iterable<String>>() {
@@ -1626,31 +1619,21 @@ public class PipelineOptionsFactory {
                 }
           }).toList();
 
-          if (returnType.isArray() && !returnType.getComponentType().equals(String.class)
-              || Collection.class.isAssignableFrom(returnType)
-              || returnType.equals(ValueProvider.class)) {
-            for (String value : values) {
-              checkArgument(!value.isEmpty(),
-                  "Empty argument value is only allowed for String, String Array, "
-                            + "and Collections of Strings, but received: %s",
-                            method.getGenericReturnType());
-            }
+          if (values.contains("")) {
+            checkEmptyStringAllowed(returnType, type, method.getGenericReturnType().toString());
           }
           convertedOptions.put(entry.getKey(), MAPPER.convertValue(values, type));
-        } else if (SIMPLE_TYPES.contains(returnType) || returnType.isEnum()
-                   || returnType.equals(ValueProvider.class)) {
+        } else if (isSimpleType(returnType, type)) {
           String value = Iterables.getOnlyElement(entry.getValue());
-          checkArgument(returnType.equals(String.class) || !value.isEmpty(),
-               "Empty argument value is only allowed for String, String Array, "
-                        + "and Collections of Strings, but received: %s",
-                        method.getGenericReturnType());
+          if (value.isEmpty()) {
+            checkEmptyStringAllowed(returnType, type, method.getGenericReturnType().toString());
+          }
           convertedOptions.put(entry.getKey(), MAPPER.convertValue(value, type));
         } else {
           String value = Iterables.getOnlyElement(entry.getValue());
-          checkArgument(returnType.equals(String.class) || !value.isEmpty(),
-                "Empty argument value is only allowed for String, String Array, "
-                        + "and Collections of Strings, but received: %s",
-                        method.getGenericReturnType());
+          if (value.isEmpty()) {
+            checkEmptyStringAllowed(returnType, type, method.getGenericReturnType().toString());
+          }
           try {
             convertedOptions.put(entry.getKey(), MAPPER.readValue(value, type));
           } catch (IOException e) {
@@ -1669,6 +1652,77 @@ public class PipelineOptionsFactory {
     return convertedOptions;
   }
 
+  /**
+   * Returns true if the given type is a SIMPLE_TYPES, enum or any of these types in a
+   * parameterized ValueProvider.
+   */
+  private static boolean isSimpleType(Class<?> type, JavaType genericType) {
+    Class<?> unwrappedType = type.equals(ValueProvider.class)
+        ? genericType.containedType(0).getRawClass() : type;
+    return SIMPLE_TYPES.contains(unwrappedType) || unwrappedType.isEnum();
+  }
+
+  /**
+   * Returns true if the given type is a any Array or Collection of SIMPLE_TYPES or enum,
or
+   * any of these types in a parameterized ValueProvider.
+   */
+  private static boolean isCollectionOrArrayOfAllowedTypes(Class<?> type, JavaType
genericType) {
+    Class<?> containerType = type.equals(ValueProvider.class)
+        ? genericType.containedType(0).getRawClass() : type;
+
+    // Check if it is an array of simple types or enum.
+    if (containerType.isArray() && (SIMPLE_TYPES.contains(containerType.getComponentType())
+        || containerType.getComponentType().isEnum())) {
+        return true;
+    }
+    // Check if it is Collection of simple types or enum.
+    if (Collection.class.isAssignableFrom(containerType)) {
+      JavaType innerType = type.equals(ValueProvider.class)
+          ? genericType.containedType(0).containedType(0)
+          : genericType.containedType(0);
+      // Note that raw types are allowed, hence the null check.
+      if (innerType == null || SIMPLE_TYPES.contains(innerType.getRawClass())
+          || innerType.getRawClass().isEnum()) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
+   * Ensures that empty string value is allowed for a given type.
+   *
+   * <p>Empty strings are only allowed for String, String Array, Collection of Strings
or any of
+   * these types in a parameterized ValueProvider.
+   *
+   * @param type class object for the type under check.
+   * @param genericType complete type information for the type under check.
+   * @param genericTypeName a string representation of the complete type information.
+   */
+  private static void checkEmptyStringAllowed(Class<?> type, JavaType genericType,
+      String genericTypeName) {
+    Class<?> unwrappedType = type.equals(ValueProvider.class)
+        ? genericType.containedType(0).getRawClass() : type;
+
+    Class<?> containedType = unwrappedType;
+    if (unwrappedType.isArray()) {
+      containedType = unwrappedType.getComponentType();
+    } else if (Collection.class.isAssignableFrom(unwrappedType)) {
+      JavaType innerType = type.equals(ValueProvider.class)
+          ? genericType.containedType(0).containedType(0)
+          : genericType.containedType(0);
+      // Note that raw types are allowed, hence the null check.
+      containedType = innerType == null ? String.class : innerType.getRawClass();
+    }
+    if (!containedType.equals(String.class)) {
+      String msg = String.format("Empty argument value is only allowed for String, String
Array, "
+              + "Collections of Strings or any of these types in a parameterized ValueProvider,
"
+              + "but received: %s",
+          genericTypeName);
+      throw new IllegalArgumentException(msg);
+    }
+  }
+
   @VisibleForTesting
   static Set<String> getSupportedRunners() {
     ImmutableSortedSet.Builder<String> supportedRunners = ImmutableSortedSet.naturalOrder();

http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/ae1d2a30/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java
----------------------------------------------------------------------
diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java
index 7ff4a92..525b937 100644
--- a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java
+++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java
@@ -27,6 +27,7 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
+import com.fasterxml.jackson.annotation.JsonFormat.Value;
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.auto.service.AutoService;
@@ -752,9 +753,7 @@ public class PipelineOptionsFactoryTest {
     String[] args = new String[] {
         "--byte="};
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage(
-        "Empty argument value is only allowed for String, String Array, and Collections"
-        + " of Strings");
+    expectedException.expectMessage(emptyStringErrorMessage());
     PipelineOptionsFactory.fromArgs(args).as(Primitives.class);
   }
 
@@ -789,6 +788,12 @@ public class PipelineOptionsFactoryTest {
     void setClassValue(Class<?> value);
     TestEnum getEnum();
     void setEnum(TestEnum value);
+    ValueProvider<String> getStringValue();
+    void setStringValue(ValueProvider<String> value);
+    ValueProvider<Long> getLongValue();
+    void setLongValue(ValueProvider<Long> value);
+    ValueProvider<TestEnum> getEnumValue();
+    void setEnumValue(ValueProvider<TestEnum> value);
   }
 
   @Test
@@ -805,7 +810,10 @@ public class PipelineOptionsFactoryTest {
         "--string=stringValue",
         "--emptyString=",
         "--classValue=" + PipelineOptionsFactoryTest.class.getName(),
-        "--enum=" + TestEnum.Value};
+        "--enum=" + TestEnum.Value,
+        "--stringValue=beam",
+        "--longValue=12389049585840",
+        "--enumValue=" + TestEnum.Value};
 
     Objects options = PipelineOptionsFactory.fromArgs(args).as(Objects.class);
     assertTrue(options.getBoolean());
@@ -820,6 +828,42 @@ public class PipelineOptionsFactoryTest {
     assertTrue(options.getEmptyString().isEmpty());
     assertEquals(PipelineOptionsFactoryTest.class, options.getClassValue());
     assertEquals(TestEnum.Value, options.getEnum());
+    assertEquals("beam", options.getStringValue().get());
+    assertEquals(Long.valueOf(12389049585840L), options.getLongValue().get());
+    assertEquals(TestEnum.Value, options.getEnumValue().get());
+  }
+
+
+  @Test
+  public void testStringValueProvider() {
+    String[] args = new String[] {"--stringValue=beam"};
+    String[] emptyArgs = new String[] { "--stringValue="};
+    Objects options = PipelineOptionsFactory.fromArgs(args).as(Objects.class);
+    assertEquals("beam", options.getStringValue().get());
+    options =  PipelineOptionsFactory.fromArgs(emptyArgs).as(Objects.class);
+    assertEquals("", options.getStringValue().get());
+  }
+
+  @Test
+  public void testLongValueProvider() {
+    String[] args = new String[] {"--longValue=12345678762"};
+    String[] emptyArgs = new String[] {"--longValue="};
+    Objects options = PipelineOptionsFactory.fromArgs(args).as(Objects.class);
+    assertEquals(Long.valueOf(12345678762L), options.getLongValue().get());
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage(emptyStringErrorMessage());
+    PipelineOptionsFactory.fromArgs(emptyArgs).as(Objects.class);
+  }
+
+  @Test
+  public void testEnumValueProvider() {
+    String[] args = new String[] {"--enumValue=" + TestEnum.Value};
+    String[] emptyArgs = new String[] {"--enumValue="};
+    Objects options = PipelineOptionsFactory.fromArgs(args).as(Objects.class);
+    assertEquals(TestEnum.Value, options.getEnumValue().get());
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage(emptyStringErrorMessage());
+    PipelineOptionsFactory.fromArgs(emptyArgs).as(Objects.class);
   }
 
   /** A test class for verifying JSON -> Object conversion. */
@@ -839,17 +883,23 @@ public class PipelineOptionsFactoryTest {
 
     ComplexType getObject();
     void setObject(ComplexType value);
+
+    ValueProvider<ComplexType> getObjectValue();
+    void setObjectValue(ValueProvider<ComplexType> value);
   }
 
   @Test
   public void testComplexTypes() {
     String[] args = new String[] {
         "--map={\"key\":\"value\",\"key2\":\"value2\"}",
-        "--object={\"key\":\"value\",\"key2\":\"value2\"}"};
+        "--object={\"key\":\"value\",\"key2\":\"value2\"}",
+        "--objectValue={\"key\":\"value\",\"key2\":\"value2\"}"};
     ComplexTypes options = PipelineOptionsFactory.fromArgs(args).as(ComplexTypes.class);
     assertEquals(ImmutableMap.of("key", "value", "key2", "value2"), options.getMap());
     assertEquals("value", options.getObject().value);
     assertEquals("value2", options.getObject().value2);
+    assertEquals("value", options.getObjectValue().get().value);
+    assertEquals("value2", options.getObjectValue().get().value2);
   }
 
   @Test
@@ -882,6 +932,12 @@ public class PipelineOptionsFactoryTest {
     void setClassValue(Class<?>[] value);
     TestEnum[] getEnum();
     void setEnum(TestEnum[] value);
+    ValueProvider<String[]> getStringValue();
+    void setStringValue(ValueProvider<String[]> value);
+    ValueProvider<Long[]> getLongValue();
+    void setLongValue(ValueProvider<Long[]> value);
+    ValueProvider<TestEnum[]> getEnumValue();
+    void setEnumValue(ValueProvider<TestEnum[]> value);
   }
 
   @Test
@@ -915,7 +971,13 @@ public class PipelineOptionsFactoryTest {
         "--classValue=" + PipelineOptionsFactory.class.getName(),
         "--classValue=" + PipelineOptionsFactoryTest.class.getName(),
         "--enum=" + TestEnum.Value,
-        "--enum=" + TestEnum.Value2};
+        "--enum=" + TestEnum.Value2,
+        "--stringValue=abc",
+        "--stringValue=beam",
+        "--longValue=123890123890",
+        "--longValue=123890123891",
+        "--enumValue=" + TestEnum.Value,
+        "--enumValue=" + TestEnum.Value2};
 
     Arrays options = PipelineOptionsFactory.fromArgs(args).as(Arrays.class);
     boolean[] bools = options.getBoolean();
@@ -932,6 +994,10 @@ public class PipelineOptionsFactoryTest {
                                    PipelineOptionsFactoryTest.class},
         options.getClassValue());
     assertArrayEquals(new TestEnum[] {TestEnum.Value, TestEnum.Value2}, options.getEnum());
+    assertArrayEquals(new String[] {"abc", "beam"}, options.getStringValue().get());
+    assertArrayEquals(new Long[] {123890123890L, 123890123891L}, options.getLongValue().get());
+    assertArrayEquals(new TestEnum[] {TestEnum.Value, TestEnum.Value2},
+        options.getEnumValue().get());
   }
 
   @Test
@@ -961,9 +1027,7 @@ public class PipelineOptionsFactoryTest {
   @Test
   public void testEmptyInNonStringArrays() {
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage(
-        "Empty argument value is only allowed for String, String Array, and Collections"
-        + " of Strings");
+    expectedException.expectMessage(emptyStringErrorMessage());
 
     String[] args = new String[] {
         "--boolean=true",
@@ -976,9 +1040,7 @@ public class PipelineOptionsFactoryTest {
   @Test
   public void testEmptyInNonStringArraysWithCommaList() {
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage(
-        "Empty argument value is only allowed for String, String Array, and Collections"
-        + " of Strings");
+    expectedException.expectMessage(emptyStringErrorMessage());
 
     String[] args = new String[] {
         "--int=1,,9"};
@@ -986,6 +1048,50 @@ public class PipelineOptionsFactoryTest {
   }
 
   @Test
+  public void testStringArrayValueProvider() {
+    String[] args = new String[] {"--stringValue=abc", "--stringValue=xyz"};
+    String[] commaArgs = new String[]{"--stringValue=abc,xyz"};
+    String[] emptyArgs = new String[] { "--stringValue=", "--stringValue="};
+    Arrays options = PipelineOptionsFactory.fromArgs(args).as(Arrays.class);
+    assertArrayEquals(new String[]{"abc", "xyz"}, options.getStringValue().get());
+    options = PipelineOptionsFactory.fromArgs(commaArgs).as(Arrays.class);
+    assertArrayEquals(new String[]{"abc", "xyz"}, options.getStringValue().get());
+    options = PipelineOptionsFactory.fromArgs(emptyArgs).as(Arrays.class);
+    assertArrayEquals(new String[]{"", ""}, options.getStringValue().get());
+  }
+
+  @Test
+  public void testLongArrayValueProvider() {
+    String[] args = new String[] {"--longValue=12345678762", "--longValue=12345678763"};
+    String[] commaArgs = new String[] {"--longValue=12345678762,12345678763"};
+    String[] emptyArgs = new String[] {"--longValue=", "--longValue="};
+    Arrays options = PipelineOptionsFactory.fromArgs(args).as(Arrays.class);
+    assertArrayEquals(new Long[] {12345678762L, 12345678763L}, options.getLongValue().get());
+    options = PipelineOptionsFactory.fromArgs(commaArgs).as(Arrays.class);
+    assertArrayEquals(new Long[] {12345678762L, 12345678763L}, options.getLongValue().get());
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage(emptyStringErrorMessage());
+    PipelineOptionsFactory.fromArgs(emptyArgs).as(Arrays.class);
+  }
+
+  @Test
+  public void testEnumArrayValueProvider() {
+    String[] args = new String[] {"--enumValue=" + TestEnum.Value,
+        "--enumValue=" + TestEnum.Value2};
+    String[] commaArgs = new String[] {"--enumValue=" + TestEnum.Value + "," + TestEnum.Value2};
+    String[] emptyArgs = new String[] {"--enumValue="};
+    Arrays options = PipelineOptionsFactory.fromArgs(args).as(Arrays.class);
+    assertArrayEquals(new TestEnum[] {TestEnum.Value, TestEnum.Value2},
+        options.getEnumValue().get());
+    options = PipelineOptionsFactory.fromArgs(commaArgs).as(Arrays.class);
+    assertArrayEquals(new TestEnum[] {TestEnum.Value, TestEnum.Value2},
+        options.getEnumValue().get());
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage(emptyStringErrorMessage());
+    PipelineOptionsFactory.fromArgs(emptyArgs).as(Arrays.class);
+  }
+
+  @Test
   public void testOutOfOrderArrays() {
     String[] args = new String[] {
         "--char=d",
@@ -1011,6 +1117,12 @@ public class PipelineOptionsFactoryTest {
     List getList();
     @SuppressWarnings("rawtypes")
     void setList(List value);
+    ValueProvider<List<String>> getStringValue();
+    void setStringValue(ValueProvider<List<String>> value);
+    ValueProvider<List<Long>> getLongValue();
+    void setLongValue(ValueProvider<List<Long>> value);
+    ValueProvider<List<TestEnum>> getEnumValue();
+    void setEnumValue(ValueProvider<List<TestEnum>> value);
   }
 
   @Test
@@ -1018,9 +1130,15 @@ public class PipelineOptionsFactoryTest {
     String[] manyArgs =
         new String[] {"--list=stringValue1", "--list=stringValue2", "--list=stringValue3"};
 
+    String[] manyArgsWithEmptyString =
+        new String[] {"--list=stringValue1", "--list=", "--list=stringValue3"};
+
     Lists options = PipelineOptionsFactory.fromArgs(manyArgs).as(Lists.class);
     assertEquals(ImmutableList.of("stringValue1", "stringValue2", "stringValue3"),
         options.getList());
+    options = PipelineOptionsFactory.fromArgs(manyArgsWithEmptyString).as(Lists.class);
+    assertEquals(ImmutableList.of("stringValue1", "", "stringValue3"),
+        options.getList());
   }
 
   @Test
@@ -1028,6 +1146,7 @@ public class PipelineOptionsFactoryTest {
     String[] manyArgs =
         new String[] {"--string=stringValue1", "--string=stringValue2", "--string=stringValue3"};
     String[] oneArg = new String[] {"--string=stringValue1"};
+    String[] emptyArg = new String[] {"--string="};
 
     Lists options = PipelineOptionsFactory.fromArgs(manyArgs).as(Lists.class);
     assertEquals(ImmutableList.of("stringValue1", "stringValue2", "stringValue3"),
@@ -1035,6 +1154,9 @@ public class PipelineOptionsFactoryTest {
 
     options = PipelineOptionsFactory.fromArgs(oneArg).as(Lists.class);
     assertEquals(ImmutableList.of("stringValue1"), options.getString());
+
+    options = PipelineOptionsFactory.fromArgs(emptyArg).as(Lists.class);
+    assertEquals(ImmutableList.of(""), options.getString());
   }
 
   @Test
@@ -1054,10 +1176,8 @@ public class PipelineOptionsFactoryTest {
     assertEquals(ImmutableList.of(1), options.getInteger());
 
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage(
-      "Empty argument value is only allowed for String, String Array, and Collections of
Strings,"
-      + " but received: java.util.List<java.lang.Integer>");
-    options = PipelineOptionsFactory.fromArgs(missingArg).as(Lists.class);
+    expectedException.expectMessage(emptyStringErrorMessage("java.util.List<java.lang.Integer>"));
+    PipelineOptionsFactory.fromArgs(missingArg).as(Lists.class);
   }
 
   @Test
@@ -1087,6 +1207,48 @@ public class PipelineOptionsFactoryTest {
   }
 
   @Test
+  public void testStringListValueProvider() {
+    String[] args = new String[] {"--stringValue=abc", "--stringValue=xyz"};
+    String[] commaArgs = new String[]{"--stringValue=abc,xyz"};
+    String[] emptyArgs = new String[] { "--stringValue=", "--stringValue="};
+    Lists options = PipelineOptionsFactory.fromArgs(args).as(Lists.class);
+    assertEquals(ImmutableList.of("abc", "xyz"), options.getStringValue().get());
+    options = PipelineOptionsFactory.fromArgs(commaArgs).as(Lists.class);
+    assertEquals(ImmutableList.of("abc", "xyz"), options.getStringValue().get());
+    options = PipelineOptionsFactory.fromArgs(emptyArgs).as(Lists.class);
+    assertEquals(ImmutableList.of("", ""), options.getStringValue().get());
+  }
+
+  @Test
+  public void testLongListValueProvider() {
+    String[] args = new String[] {"--longValue=12345678762", "--longValue=12345678763"};
+    String[] commaArgs = new String[] {"--longValue=12345678762,12345678763"};
+    String[] emptyArgs = new String[] {"--longValue=", "--longValue="};
+    Lists options = PipelineOptionsFactory.fromArgs(args).as(Lists.class);
+    assertEquals(ImmutableList.of(12345678762L, 12345678763L), options.getLongValue().get());
+    options = PipelineOptionsFactory.fromArgs(commaArgs).as(Lists.class);
+    assertEquals(ImmutableList.of(12345678762L, 12345678763L), options.getLongValue().get());
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage(emptyStringErrorMessage());
+    PipelineOptionsFactory.fromArgs(emptyArgs).as(Lists.class);
+  }
+
+  @Test
+  public void testEnumListValueProvider() {
+    String[] args = new String[] {"--enumValue=" + TestEnum.Value,
+        "--enumValue=" + TestEnum.Value2};
+    String[] commaArgs = new String[] {"--enumValue=" + TestEnum.Value + "," + TestEnum.Value2};
+    String[] emptyArgs = new String[] {"--enumValue="};
+    Lists options = PipelineOptionsFactory.fromArgs(args).as(Lists.class);
+    assertEquals(ImmutableList.of(TestEnum.Value, TestEnum.Value2), options.getEnumValue().get());
+    options = PipelineOptionsFactory.fromArgs(commaArgs).as(Lists.class);
+    assertEquals(ImmutableList.of(TestEnum.Value, TestEnum.Value2), options.getEnumValue().get());
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage(emptyStringErrorMessage());
+    PipelineOptionsFactory.fromArgs(emptyArgs).as(Lists.class);
+  }
+
+  @Test
   public void testSetASingularAttributeUsingAListThrowsAnError() {
     String[] args = new String[] {
         "--string=100",
@@ -1127,11 +1289,9 @@ public class PipelineOptionsFactoryTest {
     assertEquals(ImmutableMap.of(1, 1), options.getMap());
 
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage(
-      "Empty argument value is only allowed for String, String Array, and "
-      + "Collections of Strings, but received: java.util.Map<java.lang.Integer, "
-      + "java.lang.Integer>");
-    options = PipelineOptionsFactory.fromArgs(missingArg).as(Maps.class);
+    expectedException.expectMessage(emptyStringErrorMessage(
+        "java.util.Map<java.lang.Integer, java.lang.Integer>"));
+    PipelineOptionsFactory.fromArgs(missingArg).as(Maps.class);
   }
 
   @Test
@@ -1150,11 +1310,9 @@ public class PipelineOptionsFactoryTest {
                  options.getNestedMap());
 
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage(
-      "Empty argument value is only allowed for String, String Array, and Collections of
"
-      + "Strings, but received: java.util.Map<java.lang.Integer, "
-      + "java.util.Map<java.lang.Integer, java.lang.Integer>>");
-    options = PipelineOptionsFactory.fromArgs(missingArg).as(Maps.class);
+    expectedException.expectMessage(emptyStringErrorMessage(
+        "java.util.Map<java.lang.Integer, java.util.Map<java.lang.Integer, java.lang.Integer>>"));
+    PipelineOptionsFactory.fromArgs(missingArg).as(Maps.class);
   }
 
   @Test
@@ -1461,6 +1619,19 @@ public class PipelineOptionsFactoryTest {
         containsString("The pipeline runner that will be used to execute the pipeline."));
   }
 
+  private String emptyStringErrorMessage() {
+    return emptyStringErrorMessage(null);
+  }
+  private String emptyStringErrorMessage(String type) {
+    String msg = "Empty argument value is only allowed for String, String Array, "
+        + "Collections of Strings or any of these types in a parameterized ValueProvider";
+    if (type != null) {
+      return String.format("%s, but received: %s", msg, type);
+    } else {
+      return msg;
+    }
+  }
+
   private static class RegisteredTestRunner extends PipelineRunner<PipelineResult>
{
     public static PipelineRunner<PipelineResult> fromOptions(PipelineOptions options)
{
       return new RegisteredTestRunner();


Mime
View raw message