SQOOP-1436: Sqoop2: Support custom form name via the Form Annotation

common/src/main/java/org/apache/sqoop/model/Form.java

@@ -25,4 +25,11 @@
  */
 @Retention(RetentionPolicy.RUNTIME)
 public @interface Form {
+
+  /**
+   * Optional name for the form object
+   *
+   * @return
+   */
+  String name() default "";
 }

common/src/main/java/org/apache/sqoop/model/FormUtils.java

@@ -17,6 +17,7 @@
  */
 package org.apache.sqoop.model;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.sqoop.common.SqoopException;
 import org.apache.sqoop.utils.ClassUtils;
 import org.apache.sqoop.validation.Status;
@@ -26,9 +27,12 @@
 
 import java.lang.reflect.Field;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+
 
 /**
  * Util class for transforming data from correctly annotated configuration
@@ -56,6 +60,9 @@ public static List<MForm> toForms(Class klass) {
 
   @SuppressWarnings("unchecked")
   public static List<MForm> toForms(Class klass, Object configuration) {
+
+    Set<String> formNames = new HashSet<String>();
+
     ConfigurationClass global =
       (ConfigurationClass)klass.getAnnotation(ConfigurationClass.class);
 
@@ -68,25 +75,25 @@ public static List<MForm> toForms(Class klass, Object configuration) {
     List<MForm> forms = new LinkedList<MForm>();
 
     // Iterate over all declared fields
-    for (Field field : klass.getDeclaredFields()) {
+    for (Field formField : klass.getDeclaredFields()) {
-      field.setAccessible(true);
+      formField.setAccessible(true);
-
-      String formName = field.getName();
 
       // Each field that should be part of user input should have Input
       // annotation.
-      Form formAnnotation = field.getAnnotation(Form.class);
+      Form formAnnotation = formField.getAnnotation(Form.class);
 
       if (formAnnotation != null) {
-        Class type = field.getType();
+        String formName = getFormName(formField, formAnnotation, formNames);
+
+        Class type = formField.getType();
 
         Object value = null;
         if(configuration != null) {
           try {
-            value = field.get(configuration);
+            value = formField.get(configuration);
           } catch (IllegalAccessException e) {
             throw new SqoopException(ModelError.MODEL_005,
-              "Can't retrieve value from " + field.getName(), e);
+              "Can't retrieve value from " + formField.getName(), e);
           }
         }
 
@@ -98,7 +105,7 @@ public static List<MForm> toForms(Class klass, Object configuration) {
   }
 
   @SuppressWarnings("unchecked")
-  private static MForm toForm(String formName, Class klass, Object object) {
+  private static MForm toForm(String formName, Class<?> klass, Object object) {
      FormClass global =
       (FormClass)klass.getAnnotation(FormClass.class);
 
@@ -125,7 +132,7 @@ private static MForm toForm(String formName, Class klass, Object object) {
       if(inputAnnotation != null) {
         boolean sensitive = inputAnnotation.sensitive();
         short maxLen = inputAnnotation.size();
-        Class type = field.getType();
+        Class<?> type = field.getType();
 
         MInput input;
 
@@ -145,10 +152,11 @@ private static MForm toForm(String formName, Class klass, Object object) {
         } else if (type == Boolean.class) {
           input = new MBooleanInput(inputName, sensitive);
         } else if (type.isEnum()) {
-          input = new MEnumInput(inputName, sensitive, ClassUtils.getEnumStrings(type));
+          input = new MEnumInput(inputName, sensitive,
+              ClassUtils.getEnumStrings(type));
         } else {
-          throw new SqoopException(ModelError.MODEL_004,
+          throw new SqoopException(ModelError.MODEL_004, "Unsupported type "
-            "Unsupported type " + type.getName() + " for input " + fieldName);
+              + type.getName() + " for input " + fieldName);
         }
 
         // Move value if it's present in original configuration object
@@ -174,28 +182,45 @@ private static MForm toForm(String formName, Class klass, Object object) {
     return new MForm(formName, inputs);
   }
 
+  private static Field getFieldFromName(Class<?> klass, String name) {
+    Field formField;
+    try {
+      formField = klass.getDeclaredField(name);
+    } catch (NoSuchFieldException e) {
+      // reverse lookup form field from custom form name
+      if (name != null) {
+        for (Field field : klass.getDeclaredFields()) {
+          Form formAnnotation = field.getAnnotation(Form.class);
+          if (formAnnotation == null) {
+            continue;
+          }
+          if (!StringUtils.isEmpty(formAnnotation.name()) && name.equals(formAnnotation.name())) {
+            return field;
+          }
+        }
+      }
+      throw new SqoopException(ModelError.MODEL_006, "Missing field " + name + " on form class "
+          + klass.getCanonicalName(), e);
+    }
+    return formField;
+  }
+
   /**
    * Move form values from form list into corresponding configuration object.
    *
-   * @param forms Input form list
+   * @param forms
-   * @param configuration Output configuration object
+   *          Input form list
+   * @param configuration
+   *          Output configuration object
    */
   public static void fromForms(List<MForm> forms, Object configuration) {
-    Class klass = configuration.getClass();
+    Class<?> klass = configuration.getClass();
 
     for (MForm form : forms) {
-      Field formField;
+      Field formField = getFieldFromName(klass, form.getName());
-      try {
-        formField = klass.getDeclaredField(form.getName());
-      } catch (NoSuchFieldException e) {
-        throw new SqoopException(ModelError.MODEL_006,
-          "Missing field " + form.getName() + " on form class " + klass.getCanonicalName(), e);
-      }
-
       // We need to access this field even if it would be declared as private
       formField.setAccessible(true);
-
+      Class<?> formClass = formField.getType();
-      Class formClass = formField.getType();
       Object newValue = ClassUtils.instantiate(formClass);
 
       if (newValue == null) {
@@ -206,8 +231,8 @@ public static void fromForms(List<MForm> forms, Object configuration) {
       for (MInput input : form.getInputs()) {
         String[] splitNames = input.getName().split("\\.");
         if (splitNames.length != 2) {
-          throw new SqoopException(ModelError.MODEL_009,
+          throw new SqoopException(ModelError.MODEL_009, "Invalid name: "
-            "Invalid name: " + input.getName());
+              + input.getName());
         }
 
         String inputName = splitNames[1];
@@ -216,8 +241,8 @@ public static void fromForms(List<MForm> forms, Object configuration) {
         try {
           inputField = formClass.getDeclaredField(inputName);
         } catch (NoSuchFieldException e) {
-          throw new SqoopException(ModelError.MODEL_006,
+          throw new SqoopException(ModelError.MODEL_006, "Missing field "
-            "Missing field " + input.getName(), e);
+              + input.getName(), e);
         }
 
         // We need to access this field even if it would be declared as private
@@ -228,22 +253,24 @@ public static void fromForms(List<MForm> forms, Object configuration) {
             inputField.set(newValue, null);
           } else {
             if (input.getType() == MInputType.ENUM) {
-              inputField.set(newValue, Enum.valueOf((Class<? extends Enum>)inputField.getType(), (String) input.getValue()));
+              inputField.set(newValue, Enum.valueOf(
+                  (Class<? extends Enum>) inputField.getType(),
+                  (String) input.getValue()));
             } else {
               inputField.set(newValue, input.getValue());
             }
           }
         } catch (IllegalAccessException e) {
-          throw new SqoopException(ModelError.MODEL_005,
+          throw new SqoopException(ModelError.MODEL_005, "Issue with field "
-            "Issue with field " + inputField.getName(), e);
+              + inputField.getName(), e);
         }
       }
 
       try {
         formField.set(configuration, newValue);
       } catch (IllegalAccessException e) {
-        throw new SqoopException(ModelError.MODEL_005,
+        throw new SqoopException(ModelError.MODEL_005, "Issue with field "
-          "Issue with field " + formField.getName(), e);
+            + formField.getName(), e);
       }
     }
   }
@@ -251,11 +278,14 @@ public static void fromForms(List<MForm> forms, Object configuration) {
   /**
    * Apply validations on the forms.
    *
-   * @param forms Forms that should be updated
+   * @param forms
-   * @param validation Validation that we should apply
+   *          Forms that should be updated
+   * @param validation
+   *          Validation that we should apply
    */
   public static void applyValidation(List<MForm> forms, Validation validation) {
-    Map<Validation.FormInput, Validation.Message> messages = validation.getMessages();
+    Map<Validation.FormInput, Validation.Message> messages = validation
+        .getMessages();
 
     for (MForm form : forms) {
       applyValidation(form, messages);
@@ -269,10 +299,13 @@ public static void applyValidation(List<MForm> forms, Validation validation) {
   /**
    * Apply validation on given validated element.
    *
-   * @param element Element on what we're applying the validations
+   * @param element
-   * @param messages Map of all validation messages
+   *          Element on what we're applying the validations
+   * @param messages
+   *          Map of all validation messages
    */
-  public static void applyValidation(MValidatedElement element, Map<Validation.FormInput, Validation.Message> messages) {
+  public static void applyValidation(MValidatedElement element,
+      Map<Validation.FormInput, Validation.Message> messages) {
     Validation.FormInput name = new Validation.FormInput(element.getName());
 
     if (messages.containsKey(name)) {
@@ -293,9 +326,9 @@ public static void applyValidation(MValidatedElement element, Map<Validation.For
   @SuppressWarnings("unchecked")
   public static String toJson(Object configuration) {
     Class klass = configuration.getClass();
-
+    Set<String> formNames = new HashSet<String>();
-    ConfigurationClass global =
+    ConfigurationClass global = (ConfigurationClass) klass
-      (ConfigurationClass)klass.getAnnotation(ConfigurationClass.class);
+        .getAnnotation(ConfigurationClass.class);
 
     // Each configuration object must have this class annotation
     if (global == null) {
@@ -308,20 +341,20 @@ public static String toJson(Object configuration) {
     // Iterate over all declared fields
     for (Field formField : klass.getDeclaredFields()) {
       formField.setAccessible(true);
-      String formName = formField.getName();
 
       // We're processing only form validations
       Form formAnnotation = formField.getAnnotation(Form.class);
       if (formAnnotation == null) {
         continue;
       }
+      String formName = getFormName(formField, formAnnotation, formNames);
 
       Object formValue;
       try {
         formValue = formField.get(configuration);
       } catch (IllegalAccessException e) {
-        throw new SqoopException(ModelError.MODEL_005,
+        throw new SqoopException(ModelError.MODEL_005, "Issue with field "
-          "Issue with field " + formName, e);
+            + formName, e);
       }
 
       JSONObject jsonForm = new JSONObject();
@@ -335,20 +368,21 @@ public static String toJson(Object configuration) {
         try {
           value = inputField.get(formValue);
         } catch (IllegalAccessException e) {
-          throw new SqoopException(ModelError.MODEL_005,
+          throw new SqoopException(ModelError.MODEL_005, "Issue with field "
-            "Issue with field " + formName + "." + inputName, e);
+              + formName + "." + inputName, e);
         }
 
         Input inputAnnotation = inputField.getAnnotation(Input.class);
 
         // Do not serialize all values
         if (inputAnnotation != null && value != null) {
-          Class type = inputField.getType();
+          Class<?> type = inputField.getType();
 
           // We need to support NULL, so we do not support primitive types
           if (type.isPrimitive()) {
             throw new SqoopException(ModelError.MODEL_007,
-              "Detected primitive type " + type + " for field " + formName + "." + inputName);
+                "Detected primitive type " + type + " for field " + formName
+                    + "." + inputName);
           }
 
           if (type == String.class) {
@@ -366,15 +400,13 @@ public static String toJson(Object configuration) {
           } else if (type == Boolean.class) {
             jsonForm.put(inputName, value);
           } else {
-            throw new SqoopException(ModelError.MODEL_004,
+            throw new SqoopException(ModelError.MODEL_004, "Unsupported type "
-              "Unsupported type " + type.getName() + " for input " + formName + "." + inputName);
+                + type.getName() + " for input " + formName + "." + inputName);
           }
         }
       }
-
       jsonOutput.put(formName, jsonForm);
     }
-
     return jsonOutput.toJSONString();
   }
 
@@ -389,16 +421,17 @@ public static void fillValues(String json, Object configuration) {
     Class klass = configuration.getClass();
 
     JSONObject jsonForms = (JSONObject) JSONValue.parse(json);
+    Set<String> formNames = new HashSet<String>();
 
     for(Field formField : klass.getDeclaredFields()) {
       formField.setAccessible(true);
-      String formName = formField.getName();
 
       // We're processing only form validations
       Form formAnnotation = formField.getAnnotation(Form.class);
       if(formAnnotation == null) {
         continue;
       }
+      String formName = getFormName(formField, formAnnotation, formNames);
 
       try {
         formField.set(configuration, formField.getType().newInstance());
@@ -407,7 +440,7 @@ public static void fillValues(String json, Object configuration) {
           "Issue with field " + formName, e);
       }
 
-      JSONObject jsonInputs = (JSONObject) jsonForms.get(formField.getName());
+      JSONObject jsonInputs = (JSONObject) jsonForms.get(formName);
       if(jsonInputs == null) {
         continue;
       }
@@ -466,4 +499,40 @@ public static void fillValues(String json, Object configuration) {
     }
   }
 
+  private static String getFormName(Field member, Form annotation, Set<String> existingFormNames) {
+    if (StringUtils.isEmpty(annotation.name())) {
+      return member.getName();
+    } else {
+      checkForValidFormName(existingFormNames, annotation.name());
+      existingFormNames.add(annotation.name());
+      return annotation.name();
+    }
+  }
+
+  private static void checkForValidFormName(Set<String> existingFormNames,
+      String customFormName) {
+    // uniqueness across fields check
+    if (existingFormNames.contains(customFormName)) {
+      throw new SqoopException(ModelError.MODEL_012,
+          "Issue with field form name " + customFormName);
+    }
+
+    if (!Character.isJavaIdentifierStart(customFormName.toCharArray()[0])) {
+      throw new SqoopException(ModelError.MODEL_013,
+          "Issue with field form name " + customFormName);
+    }
+    for (Character c : customFormName.toCharArray()) {
+      if (Character.isJavaIdentifierPart(c))
+        continue;
+      throw new SqoopException(ModelError.MODEL_013,
+          "Issue with field form name " + customFormName);
+    }
+
+    if (customFormName.length() > 30) {
+      throw new SqoopException(ModelError.MODEL_014,
+          "Issue with field form name " + customFormName);
+
+    }
+  }
+
 }

common/src/main/java/org/apache/sqoop/model/ModelError.java

@@ -46,6 +46,11 @@ public enum ModelError implements ErrorCode {
 
   MODEL_011("Input do not exist"),
 
+  MODEL_012("Form name attribute should be unique across a configuration object"),
+
+  MODEL_013("Form name attribute should not contain unsupported characters"),
+
+  MODEL_014("Form name attribute cannot be more than 30 characters long")
 ;
 
   private final String message;

common/src/test/java/org/apache/sqoop/model/TestFormUtils.java

@@ -33,24 +33,24 @@
 public class TestFormUtils extends TestCase {
 
   public void testToForms() {
-    Config config = new Config();
+    Configuration Configuration = new Configuration();
-    config.aForm.a1 = "value";
+    Configuration.aForm.a1 = "value";
 
-    List<MForm> formsByInstance = FormUtils.toForms(config);
+    List<MForm> formsByInstance = FormUtils.toForms(Configuration);
     assertEquals(getForms(), formsByInstance);
     assertEquals("value", formsByInstance.get(0).getInputs().get(0).getValue());
 
-    List<MForm> formsByClass = FormUtils.toForms(Config.class);
+    List<MForm> formsByClass = FormUtils.toForms(Configuration.class);
     assertEquals(getForms(), formsByClass);
 
-    List<MForm> formsByBoth = FormUtils.toForms(Config.class, config);
+    List<MForm> formsByBoth = FormUtils.toForms(Configuration.class, Configuration);
     assertEquals(getForms(), formsByBoth);
     assertEquals("value", formsByBoth.get(0).getInputs().get(0).getValue());
   }
 
   public void testToFormsMissingAnnotation() {
     try {
-      FormUtils.toForms(ConfigWithout.class);
+      FormUtils.toForms(ConfigurationWithoutAnnotation.class);
     } catch(SqoopException ex) {
       assertEquals(ModelError.MODEL_003, ex.getErrorCode());
       return;
@@ -59,11 +59,43 @@ public void testToFormsMissingAnnotation() {
     fail("Correct exception wasn't thrown");
   }
 
+  public void testNonUniqueFormNameAttributes() {
+    try {
+      FormUtils.toForms(ConfigurationWithNonUniqueFormNameAttribute.class);
+    } catch (SqoopException ex) {
+      assertEquals(ModelError.MODEL_012, ex.getErrorCode());
+      return;
+    }
+
+    fail("Correct exception wasn't thrown");
+  }
+
+  public void testInvalidFormNameAttribute() {
+    try {
+      FormUtils.toForms(ConfigurationWithInvalidFormNameAttribute.class);
+    } catch (SqoopException ex) {
+      assertEquals(ModelError.MODEL_013, ex.getErrorCode());
+      return;
+    }
+
+    fail("Correct exception wasn't thrown");
+  }
+
+  public void testInvalidFormNameAttributeLength() {
+    try {
+      FormUtils.toForms(ConfigurationWithInvalidFormNameAttributeLength.class);
+    } catch (SqoopException ex) {
+      assertEquals(ModelError.MODEL_014, ex.getErrorCode());
+      return;
+    }
+    fail("Correct exception wasn't thrown");
+  }
+
   public void testFailureOnPrimitiveType() {
-    PrimitiveConfig config = new PrimitiveConfig();
+    PrimitiveConfiguration Configuration = new PrimitiveConfiguration();
 
     try {
-      FormUtils.toForms(config);
+      FormUtils.toForms(Configuration);
       fail("We were expecting exception for unsupported type.");
     } catch(SqoopException ex) {
       assertEquals(ModelError.MODEL_007, ex.getErrorCode());
@@ -72,13 +104,16 @@ public void testFailureOnPrimitiveType() {
 
   public void testFillValues() {
     List<MForm> forms = getForms();
+    assertEquals("test_AForm", forms.get(0).getName());
+    assertEquals("test_BForm", forms.get(1).getName());
+    assertEquals("cForm", forms.get(2).getName());
 
     ((MStringInput)forms.get(0).getInputs().get(0)).setValue("value");
 
-    Config config = new Config();
+    Configuration Configuration = new Configuration();
 
-    FormUtils.fromForms(forms, config);
+    FormUtils.fromForms(forms, Configuration);
-    assertEquals("value", config.aForm.a1);
+    assertEquals("value", Configuration.aForm.a1);
   }
 
   public void testFillValuesObjectReuse() {
@@ -86,15 +121,15 @@ public void testFillValuesObjectReuse() {
 
     ((MStringInput)forms.get(0).getInputs().get(0)).setValue("value");
 
-    Config config = new Config();
+    Configuration Configuration = new Configuration();
-    config.aForm.a2 = "x";
+    Configuration.aForm.a2 = "x";
-    config.bForm.b1 = "y";
+    Configuration.bForm.b1 = "y";
 
-    FormUtils.fromForms(forms, config);
+    FormUtils.fromForms(forms, Configuration);
-    assertEquals("value", config.aForm.a1);
+    assertEquals("value", Configuration.aForm.a1);
-    assertNull(config.aForm.a2);
+    assertNull(Configuration.aForm.a2);
-    assertNull(config.bForm.b2);
+    assertNull(Configuration.bForm.b2);
-    assertNull(config.bForm.b2);
+    assertNull(Configuration.bForm.b2);
   }
 
   public void testApplyValidation() {
@@ -115,16 +150,16 @@ public void testApplyValidation() {
   }
 
   public void testJson() {
-    Config config = new Config();
+    Configuration Configuration = new Configuration();
-    config.aForm.a1 = "A";
+    Configuration.aForm.a1 = "A";
-    config.bForm.b2 = "B";
+    Configuration.bForm.b2 = "B";
-    config.cForm.intValue = 4;
+    Configuration.cForm.intValue = 4;
-    config.cForm.map.put("C", "D");
+    Configuration.cForm.map.put("C", "D");
-    config.cForm.enumeration = Enumeration.X;
+    Configuration.cForm.enumeration = Enumeration.X;
 
-    String json = FormUtils.toJson(config);
+    String json = FormUtils.toJson(Configuration);
 
-    Config targetConfig = new Config();
+    Configuration targetConfig = new Configuration();
 
     // Old values from should be always removed
     targetConfig.aForm.a2 = "X";
@@ -152,17 +187,17 @@ protected Validation getValidation() {
       = new HashMap<Validation.FormInput, Validation.Message>();
 
     messages.put(
-      new Validation.FormInput("aForm", "a1"),
+      new Validation.FormInput("test_AForm", "a1"),
       new Validation.Message(Status.ACCEPTABLE, "e1"));
     messages.put(
-      new Validation.FormInput("aForm", "a2"),
+      new Validation.FormInput("test_AForm", "a2"),
       new Validation.Message(Status.UNACCEPTABLE, "e2"));
 
     return new Validation(Status.UNACCEPTABLE, messages);
   }
 
   /**
-   * Form structure that corresponds to Config class declared below
+   * Form structure that corresponds to Configuration class declared below
    * @return Form structure
    */
   protected List<MForm> getForms() {
@@ -172,15 +207,15 @@ protected List<MForm> getForms() {
 
     // Form A
     inputs = new LinkedList<MInput<?>>();
-    inputs.add(new MStringInput("aForm.a1", false, (short)30));
+    inputs.add(new MStringInput("test_AForm.a1", false, (short)30));
-    inputs.add(new MStringInput("aForm.a2", true, (short)-1));
+    inputs.add(new MStringInput("test_AForm.a2", true, (short)-1));
-    ret.add(new MForm("aForm", inputs));
+    ret.add(new MForm("test_AForm", inputs));
 
     // Form B
     inputs = new LinkedList<MInput<?>>();
-    inputs.add(new MStringInput("bForm.b1", false, (short)2));
+    inputs.add(new MStringInput("test_BForm.b1", false, (short)2));
-    inputs.add(new MStringInput("bForm.b2", false, (short)3));
+    inputs.add(new MStringInput("test_BForm.b2", false, (short)3));
-    ret.add(new MForm("bForm", inputs));
+    ret.add(new MForm("test_BForm", inputs));
 
     // Form C
     inputs = new LinkedList<MInput<?>>();
@@ -193,21 +228,57 @@ protected List<MForm> getForms() {
   }
 
   @ConfigurationClass
-  public static class Config {
+  public static class ConfigurationWithNonUniqueFormNameAttribute {
+    public ConfigurationWithNonUniqueFormNameAttribute() {
+      aForm = new InvalidForm();
+      bForm = new InvalidForm();
+    }
 
-    public Config() {
+    @Form(name = "sameName")
+    InvalidForm aForm;
+    @Form(name = "sameName")
+    InvalidForm bForm;
+  }
+
+  @ConfigurationClass
+  public static class ConfigurationWithInvalidFormNameAttribute {
+    public ConfigurationWithInvalidFormNameAttribute() {
+      invalidForm = new InvalidForm();
+    }
+
+    @Form(name = "#_form")
+    InvalidForm invalidForm;
+  }
+
+  @ConfigurationClass
+  public static class ConfigurationWithInvalidFormNameAttributeLength {
+    public ConfigurationWithInvalidFormNameAttributeLength() {
+      invalidLengthForm = new InvalidForm();
+    }
+
+    @Form(name = "longest_form_more_than_30_characers")
+    InvalidForm invalidLengthForm;
+  }
+
+  @ConfigurationClass
+  public static class Configuration {
+
+    public Configuration() {
       aForm = new AForm();
       bForm = new BForm();
       cForm = new CForm();
     }
 
-    @Form AForm aForm;
+    @Form(name = "test_AForm")
-    @Form BForm bForm;
+    AForm aForm;
-    @Form CForm cForm;
+    @Form(name = "test_BForm")
+    BForm bForm;
+    @Form(name = "")
+    CForm cForm;
   }
 
   @ConfigurationClass
-  public static class PrimitiveConfig {
+  public static class PrimitiveConfiguration {
     @Form DForm dForm;
   }
 
@@ -234,12 +305,16 @@ public CForm() {
     }
   }
 
+  @FormClass
+  public static class InvalidForm {
+
+  }
   @FormClass
   public static class DForm {
     @Input int value;
   }
 
-  public static class ConfigWithout {
+  public static class ConfigurationWithoutAnnotation {
   }
 
   enum Enumeration {