SQOOP-858: Add validation messages on form level

(Jarcec Cecho via Cheolsoo Park)

client/src/main/java/org/apache/sqoop/client/utils/FormFiller.java

@@ -27,6 +27,7 @@
 import org.apache.sqoop.model.MMapInput;
 import org.apache.sqoop.model.MJob;
 import org.apache.sqoop.model.MStringInput;
+import org.apache.sqoop.model.MValidatedElement;
 import org.codehaus.groovy.tools.shell.IO;
 
 import java.io.IOException;
@@ -147,6 +148,11 @@ public static boolean fillForm(IO io,
                                  ResourceBundle bundle) throws IOException {
     io.out.println("");
     io.out.println(bundle.getString(form.getLabelKey()));
+
+    // Print out form validation
+    printValidationMessage(io, form);
+    io.out.println("");
+
     for (MInput input : form.getInputs()) {
       if(!fillInput(io, input, reader, bundle)) {
         return false;
@@ -160,19 +166,8 @@ public static boolean fillInput(IO io,
                                   MInput input,
                                   ConsoleReader reader,
                                   ResourceBundle bundle) throws IOException {
-    // Print out warning or error message in case some validations were already
+    // Print out validation
-    // performed.
+    printValidationMessage(io, input);
-    switch (input.getValidationStatus()) {
-      case UNACCEPTABLE:
-        errorMessage(io, input.getValidationMessage());
-        break;
-      case ACCEPTABLE:
-        warningMessage(io, input.getValidationMessage());
-        break;
-      default:
-        // Simply ignore all other states for the moment
-        break;
-    }
 
     // Based on the input type, let's perform specific load
     switch (input.getType()) {
@@ -486,6 +481,26 @@ public static String getName(IO io, ConsoleReader reader,
     return nameInput.getValue();
   }
 
+  /**
+   * Print validation message in cases that it's not in state "FINE"
+   *
+   * @param io IO object to print out the message
+   * @param element Validated element
+   */
+  public static void printValidationMessage(IO io, MValidatedElement element) {
+    switch (element.getValidationStatus()) {
+      case UNACCEPTABLE:
+        errorMessage(io, element.getValidationMessage());
+        break;
+      case ACCEPTABLE:
+        warningMessage(io, element.getValidationMessage());
+        break;
+      default:
+        // Simply ignore all other states for the moment
+        break;
+    }
+  }
+
   public static void errorMessage(IO io, String message) {
     io.out.println("Error message: @|red " + message + " |@");
   }

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

@@ -256,16 +256,28 @@ public static void applyValidation(List<MForm> forms, Validation validation) {
     Map<Validation.FormInput, Validation.Message> messages = validation.getMessages();
 
     for(MForm form : forms) {
-      for(MInput input : form.getInputs()) {
+      applyValidation(form, messages);
-        Validation.FormInput fi = new Validation.FormInput(input.getName());
-        if(messages.containsKey(fi)) {
-          Validation.Message message = messages.get(fi);
 
-          input.setValidationMessage(message.getStatus(), message.getMessage());
+      for(MInput input : form.getInputs()) {
+        applyValidation(input, messages);
+      }
+    }
+  }
+
+  /**
+   * Apply validation on given validated element.
+   *
+   * @param element 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) {
+    Validation.FormInput name = new Validation.FormInput(element.getName());
+
+    if(messages.containsKey(name)) {
+      Validation.Message message = messages.get(name);
+      element.setValidationMessage(message.getStatus(), message.getMessage());
     } else {
-          input.setValidationMessage(Status.getDefault(), null);
+      element.setValidationMessage(Status.getDefault(), null);
-        }
-      }
     }
   }
 

common/src/main/java/org/apache/sqoop/validation/Validation.java

@@ -24,7 +24,9 @@
 import java.util.Map;
 
 /**
+ * Validation class.
  *
+ * This class represents validations to given configuration object.
  */
 public class Validation {
 
@@ -37,19 +39,13 @@ public class Validation {
   // Status messages for various fields
   Map<FormInput, Message> messages;
 
-  private Validation() {
-    klass = null;
-  }
   public Validation(Class klass) {
-    this();
-
     this.klass = klass;
     status = Status.getDefault();
     messages = new HashMap<FormInput, Message>();
   }
-  public Validation(Status status, Map<FormInput, Message> messages) {
-    this();
 
+  public Validation(Status status, Map<FormInput, Message> messages) {
     this.status = status;
     this.messages = messages;
   }
@@ -62,14 +58,37 @@ public Map<FormInput, Message> getMessages() {
     return messages;
   }
 
-  public void addMessage(Status status, String form, String field, String message ) {
+  /**
+   * Add message to form.
+   *
+   * @param status Severity of the message
+   * @param form Form name, must be defined in the class
+   * @param message Validation message
+   */
+  public void addMessage(Status status, String form, String message) {
+    addMessage(status, form, null, message);
+  }
+
+  /**
+   * Add message to input in one of the forms.
+   *
+   * @param status Severity of the message
+   * @param form Form name, must be defined in the class
+   * @param input Field name, must be defined in the form class
+   * @param message Validation message
+   */
+  public void addMessage(Status status, String form, String input, String message ) {
     if( klass == null) {
       throw new SqoopException(ValidationError.VALIDATION_0001);
     }
 
+    assert form != null;
+    assert message != null;
+
+    // Field for specified form
     Field formField;
 
-    // Verify that such form exists
+    // Load the form field and verify that it exists
     try {
       formField = klass.getDeclaredField(form);
     } catch (NoSuchFieldException e) {
@@ -77,16 +96,26 @@ public void addMessage(Status status, String form, String field, String message
         "Can't get form " + form + " from " + klass.getName(), e);
     }
 
-    // Verify that such input exists on given form
+    // If this is form message, just save the message and continue
-    try {
+    if(input == null) {
-      formField.getType().getDeclaredField(field);
+      setMessage(status, form, input, message);
-    } catch (NoSuchFieldException e) {
+      return;
-      throw new SqoopException(ValidationError.VALIDATION_0002,
-        "Can't get input " + field + " from form" + formField.getType().getName(), e);
     }
 
+    // Verify that specified input exists on the form
+    try {
+      formField.getType().getDeclaredField(input);
+    } catch (NoSuchFieldException e) {
+      throw new SqoopException(ValidationError.VALIDATION_0002,
+        "Can't get input " + input + " from form" + formField.getType().getName(), e);
+    }
+
+    setMessage(status, form, input, message);
+  }
+
+  private void setMessage(Status status, String form, String input, String message) {
     this.status = Status.getWorstStatus(this.status, status);
-    messages.put(new FormInput(form, field), new Message(status, message));
+    messages.put(new FormInput(form, input), new Message(status, message));
   }
 
   public static class Message {
@@ -143,15 +172,19 @@ public FormInput(String form, String input) {
     }
 
     public FormInput(String formInput) {
+      assert formInput != null;
       String []parts = formInput.split("\\.");
-      if(parts.length != 2) {
+
+      if(formInput.isEmpty() || (parts.length != 1 && parts.length != 2)) {
         throw new SqoopException(ValidationError.VALIDATION_0003,
           "Specification " + formInput + " is not in valid format form.input");
       }
 
       this.form = parts[0];
+      if(parts.length == 2) {
         this.input = parts[1];
       }
+    }
 
     public String getForm() {
       return form;
@@ -185,6 +218,10 @@ public int hashCode() {
 
     @Override
     public String toString() {
+      if(input == null) {
+        return form;
+      }
+
       return form + "." + input;
     }
   }

common/src/test/java/org/apache/sqoop/validation/TestValidation.java

@@ -35,19 +35,19 @@ public class TestValidation extends TestCase {
    * Initialization test
    */
   public void testInitialization() {
-    /** Check initialization with class */
+    /* Check initialization with class */
     Validation validation = new Validation(Class.class);
     assertNotNull(validation);
     assertEquals(Status.FINE, validation.getStatus());
     assertEquals(0, validation.getMessages().size());
 
-    /** Check initialization with status and message as null */
+    /* Check initialization with status and message as null */
     Validation validationNull = new Validation(null, null);
     assertNotNull(validationNull);
     assertNull(validationNull.getStatus());
     assertNull(validationNull.getMessages());
 
-    /** Check initialization with status and message with values */
+    /* Check initialization with status and message with values */
     Status s1 = Status.FINE;
     Map<FormInput, Message> msg1 = new HashMap<Validation.FormInput, Validation.Message>();
     Validation validation1 = new Validation(s1, msg1);
@@ -55,7 +55,7 @@ public void testInitialization() {
     assertEquals(Status.FINE, validation1.getStatus());
     assertEquals(0, validation1.getMessages().size());
 
-    /** Check initialization with status and message with values */
+    /* Check initialization with status and message with values */
     Status s2 = Status.ACCEPTABLE;
     Map<FormInput, Message> msg2 = new HashMap<Validation.FormInput, Validation.Message>();
     Validation validation2 = new Validation(s2, msg2);
@@ -63,7 +63,7 @@ public void testInitialization() {
     assertEquals(Status.ACCEPTABLE, validation2.getStatus());
     assertEquals(0, validation2.getMessages().size());
 
-    /** Check initialization with status and message with values */
+    /* Check initialization with status and message with values */
     Status s3 = Status.ACCEPTABLE;
     Map<FormInput, Message> msg3 = new HashMap<Validation.FormInput, Validation.Message>();
     Validation.FormInput fi = new Validation.FormInput("form\\.input");
@@ -84,61 +84,59 @@ public void testFormInput() {
     Validation.FormInput fi = new Validation.FormInput("test\\.test");
     assertNotNull(fi);
 
-    Validation.FormInput fi1;
+    /* Passing null */
-    /** Passing null */
     try {
-      fi1 = new Validation.FormInput(null);
+      new Validation.FormInput(null);
-      fail("Null pointer exception is expected");
+      fail("Assert error is expected");
-    } catch (NullPointerException e) {
+    } catch (AssertionError e) {
       assertTrue(true);
     }
 
-    /** Passing empty string and check for SqoopException */
+    /* Passing empty and check exception messages */
     try {
-      fi1 = new Validation.FormInput("");
+      new Validation.FormInput("");
-      fail("SqoopException is expected");
-    } catch (SqoopException e) {
-      assertTrue(true);
-    }
-
-    /** Passing empty and check exception messages */
-    try {
-      fi1 = new Validation.FormInput("");
       fail("SqoopException is expected");
     } catch (SqoopException e) {
       assertEquals(ValidationError.VALIDATION_0003.getMessage(), e
           .getErrorCode().getMessage());
     }
 
-    /** Passing value and check */
+    /* Passing value and check */
     Validation.FormInput fi2 = new Validation.FormInput("form\\.input");
     assertEquals("form\\", fi2.getForm());
     assertEquals("input", fi2.getInput());
 
-    /** Check equals */
+    /* Check equals */
     Validation.FormInput fiOne = new Validation.FormInput("form\\.input");
     Validation.FormInput fiTwo = new Validation.FormInput("form\\.input");
     assertEquals(fiOne, fiTwo);
 
-    /** toString() method check */
+    /* toString() method check */
     assertEquals("form\\.input", fiOne.toString());
+
+    // Checking null as input field (form validation)
+    Validation.FormInput fi3 = new FormInput("form");
+    assertEquals("form", fi3.getForm());
+    assertNull(fi3.getInput());
+    assertEquals("form", fi3.toString());
+
   }
 
   /**
    * Test for Validation.Message
    */
   public void testMessage() {
-    /** Passing null */
+    /* Passing null */
     Validation.Message msg1 = new Validation.Message(null, null);
     assertNull(msg1.getStatus());
     assertNull(msg1.getMessage());
 
-    /** Passing values */
+    /* Passing values */
     Validation.Message msg2 = new Validation.Message(Status.FINE, "sqoop");
     assertEquals(Status.FINE, msg2.getStatus());
     assertEquals("sqoop", msg2.getMessage());
 
-    /** Check for equal */
+    /* Check for equal */
     Validation.Message msg3 = new Validation.Message(Status.FINE, "sqoop");
     assertEquals(msg2, msg3);
   }

connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java

@@ -25,6 +25,9 @@
 import org.apache.sqoop.validation.Validation;
 import org.apache.sqoop.validation.Validator;
 
+import java.sql.DriverManager;
+import java.sql.SQLException;
+
 /**
  * Validator to ensure that user is supplying valid input
  */
@@ -51,8 +54,15 @@ public Validation validateConnection(Object configuration) {
       validation.addMessage(Status.UNACCEPTABLE, "connection", "connectionString", "This do not seem as a valid JDBC URL");
     }
 
-    // TODO: Try to connect to database when form level validations will be supported
+    // See if we can connect to the database
+    try {
+      DriverManager.getConnection(config.connection.connectionString,
+        config.connection.username, config.connection.password);
+    } catch (SQLException e) {
+      validation.addMessage(Status.ACCEPTABLE, "connection", "Can't connect to the database with given credentials: " + e.getMessage());
+    }
 
+    // Return final validation object
     return validation;
   }
 
@@ -72,14 +82,11 @@ private Validation validateExportJob(Object jobConfiguration) {
     Validation validation = new Validation(ExportJobConfiguration.class);
     ExportJobConfiguration configuration = (ExportJobConfiguration)jobConfiguration;
 
-    // TODO: Move those message to form level when it will be supported
     if(configuration.table.tableName == null && configuration.table.sql == null) {
-      validation.addMessage(Status.UNACCEPTABLE, "table", "tableName", "Either table name or SQL must be specified");
+      validation.addMessage(Status.UNACCEPTABLE, "table", "Either table name or SQL must be specified");
-      validation.addMessage(Status.UNACCEPTABLE, "table", "sql", "Either table name or SQL must be specified");
     }
     if(configuration.table.tableName != null && configuration.table.sql != null) {
-      validation.addMessage(Status.UNACCEPTABLE, "table", "tableName", "Both table name and SQL cannot be specified");
+      validation.addMessage(Status.UNACCEPTABLE, "table", "Both table name and SQL cannot be specified");
-      validation.addMessage(Status.UNACCEPTABLE, "table", "sql", "Both table name and SQL cannot be specified");
     }
 
     return validation;
@@ -89,14 +96,11 @@ private Validation validateImportJob(Object jobConfiguration) {
     Validation validation = new Validation(ImportJobConfiguration.class);
     ImportJobConfiguration configuration = (ImportJobConfiguration)jobConfiguration;
 
-    // TODO: Move those message to form level when it will be supported
     if(configuration.table.tableName == null && configuration.table.sql == null) {
-      validation.addMessage(Status.UNACCEPTABLE, "table", "tableName", "Either table name or SQL must be specified");
+      validation.addMessage(Status.UNACCEPTABLE, "table", "Either table name or SQL must be specified");
-      validation.addMessage(Status.UNACCEPTABLE, "table", "sql", "Either table name or SQL must be specified");
     }
     if(configuration.table.tableName != null && configuration.table.sql != null) {
-      validation.addMessage(Status.UNACCEPTABLE, "table", "tableName", "Both table name and SQL cannot be specified");
+      validation.addMessage(Status.UNACCEPTABLE, "table", "Both table name and SQL cannot be specified");
-      validation.addMessage(Status.UNACCEPTABLE, "table", "sql", "Both table name and SQL cannot be specified");
     }
 
     return validation;

connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties

@@ -18,7 +18,7 @@
 ############################
 # Connection Form
 #
-connection.label = Configuration configuration
+connection.label = Connection configuration
 connection.help = You must supply the information requested in order to \
                    create a connection object.