SQOOP-3334: Improve ArgumentArrayBuilder, so arguments are replaceable

(Fero Szabo via Szabolcs Vasas)
diff --git a/src/test/org/apache/sqoop/testutil/ArgumentArrayBuilder.java b/src/test/org/apache/sqoop/testutil/ArgumentArrayBuilder.java
index 00ce4fe..481dba4 100644
--- a/src/test/org/apache/sqoop/testutil/ArgumentArrayBuilder.java
+++ b/src/test/org/apache/sqoop/testutil/ArgumentArrayBuilder.java
@@ -18,11 +18,12 @@
 
 package org.apache.sqoop.testutil;
 
-import org.apache.commons.collections.CollectionUtils;
-
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import static org.apache.commons.lang3.StringUtils.isEmpty;
 
@@ -31,55 +32,57 @@
   private static final String PROPERTY_PREFIX = "-D";
 
   private static final String OPTION_PREFIX = "--";
-  public static final String TOOL_ARG_SEPARATOR = "--";
 
-  private List<Argument> properties;
+  private static final String TOOL_ARG_SEPARATOR = "--";
 
-  private List<Argument> options;
+  private Map<String, Argument> properties;
 
-  private List<Argument> toolOptions;
+  private Map<String, Argument> options;
+
+  private Map<String, Argument> toolOptions;
 
   private boolean withCommonHadoopFlags;
 
   public ArgumentArrayBuilder() {
-    properties = new ArrayList<>();
-    options = new ArrayList<>();
-    toolOptions = new ArrayList<>();
+    properties = new HashMap<>();
+    options = new HashMap<>();
+    toolOptions = new HashMap<>();
   }
 
   public ArgumentArrayBuilder withProperty(String name, String value) {
-    properties.add(new Argument(name, value));
+    properties.put(name, new Argument(name, value));
     return this;
   }
 
   public ArgumentArrayBuilder withProperty(String name) {
-    properties.add(new Argument(name));
+    properties.put(name, new Argument(name));
     return this;
   }
 
   public ArgumentArrayBuilder withOption(String name, String value) {
-    options.add(new Argument(name, value));
+    options.put(name, new Argument(name, value));
     return this;
   }
 
   public ArgumentArrayBuilder withOption(String name) {
-    options.add(new Argument(name));
+    options.put(name, new Argument(name));
     return this;
   }
 
   public ArgumentArrayBuilder withToolOption(String name, String value) {
-    toolOptions.add(new Argument(name, value));
+    toolOptions.put(name, new Argument(name, value));
     return this;
   }
 
   public ArgumentArrayBuilder withToolOption(String name) {
-    toolOptions.add(new Argument(name));
+    toolOptions.put(name, new Argument(name));
     return this;
   }
 
   public ArgumentArrayBuilder with(ArgumentArrayBuilder otherBuilder) {
-    properties.addAll(otherBuilder.properties);
-    options.addAll(otherBuilder.options);
+    properties.putAll(otherBuilder.properties);
+    options.putAll(otherBuilder.options);
+    toolOptions.putAll(otherBuilder.toolOptions);
     return this;
   }
 
@@ -103,20 +106,20 @@
     if (withCommonHadoopFlags) {
       CommonArgs.addHadoopFlags(result);
     }
-    if (CollectionUtils.isNotEmpty(properties)) {
-      Collections.addAll(result, createArgumentArrayFromProperties(properties));
+    if (!properties.isEmpty()) {
+      Collections.addAll(result, createArgumentArrayFromProperties(properties.values()));
     }
-    if (CollectionUtils.isNotEmpty(options)) {
-      Collections.addAll(result, createArgumentArrayFromOptions(options));
+    if (!options.isEmpty()) {
+      Collections.addAll(result, createArgumentArrayFromOptions(options.values()));
     }
-    if (CollectionUtils.isNotEmpty(toolOptions)) {
+    if (!toolOptions.isEmpty()) {
       result.add(TOOL_ARG_SEPARATOR);
-      Collections.addAll(result, createArgumentArrayFromOptions(toolOptions));
+      Collections.addAll(result, createArgumentArrayFromOptions(toolOptions.values()));
     }
     return result.toArray(new String[result.size()]);
   }
 
-  private String[] createArgumentArrayFromProperties(List<Argument> properties) {
+  private String[] createArgumentArrayFromProperties(Iterable<Argument> properties) {
     List<String> result = new ArrayList<>();
     for (Argument property : properties) {
       result.add(PROPERTY_PREFIX);
@@ -125,7 +128,7 @@
     return result.toArray(new String[result.size()]);
   }
 
-  private String[] createArgumentArrayFromOptions(List<Argument> options) {
+  private String[] createArgumentArrayFromOptions(Iterable<Argument> options) {
     List<String> result = new ArrayList<>();
     for (Argument option : options) {
       result.add(OPTION_PREFIX + option.getName());
@@ -135,4 +138,9 @@
     }
     return result.toArray(new String[result.size()]);
   }
+
+  @Override
+  public String toString() {
+    return Arrays.toString(build());
+  }
 }
\ No newline at end of file
diff --git a/src/test/org/apache/sqoop/testutil/TestArgumentArrayBuilder.java b/src/test/org/apache/sqoop/testutil/TestArgumentArrayBuilder.java
new file mode 100644
index 0000000..6d701ab
--- /dev/null
+++ b/src/test/org/apache/sqoop/testutil/TestArgumentArrayBuilder.java
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sqoop.testutil;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertArrayEquals;
+
+public class TestArgumentArrayBuilder {
+
+  @Test
+  public void testArgumentArrayBuilder() {
+    String[] expectedArray = { "-D", "property=value3", "--option", "value2", "--", "--toolOption", "value1" };
+    ArgumentArrayBuilder builder = new ArgumentArrayBuilder();
+    builder.withToolOption("toolOption", "value1")
+        .withOption("option", "value2")
+        .withProperty("property", "value3");
+    String[] argArray = builder.build();
+    assertArrayEquals(expectedArray, argArray);
+  }
+
+  @Test
+  public void testArgumentArrayBuilderOverriddenValues() {
+    String[] expectedArray = { "-D", "property=modifiedProperty", "--option", "modifiedOption", "--", "--toolOption", "modifiedToolOption" };
+
+    ArgumentArrayBuilder builder = new ArgumentArrayBuilder();
+    builder.withToolOption("toolOption", "originalToolOption")
+        .withOption("option", "originalOption")
+        .withProperty("property", "originalProperty");
+    builder.withProperty("property", "modifiedProperty")
+        .withOption("option", "modifiedOption")
+        .withToolOption("toolOption", "modifiedToolOption");
+    String[] argArray = builder.build();
+
+    assertArrayEquals(expectedArray, argArray);
+  }
+
+  @Test
+  public void testArgumentArrayBuilderCopiesEverything() {
+    String[] expectedArray = { "-D", "property=value3", "--option", "value2", "--", "--toolOption", "value1" };
+    ArgumentArrayBuilder builder = new ArgumentArrayBuilder();
+    builder.withToolOption("toolOption", "value1")
+        .withOption("option", "value2")
+        .withProperty("property", "value3");
+
+    ArgumentArrayBuilder otherBuilder = new ArgumentArrayBuilder();
+    otherBuilder.with(builder);
+    String[] argArray = otherBuilder.build();
+    assertArrayEquals(expectedArray, argArray);
+  }
+
+}