From d5dc3a28b20fdc97fea2f36ee4aaf36189a1a0d6 Mon Sep 17 00:00:00 2001 From: Cheolsoo Park Date: Thu, 31 Jan 2013 09:24:58 -0800 Subject: [PATCH] SQOOP-845: Improve Generic JDBC validator (Jarcec Cecho via Cheolsoo Park) --- .../jdbc/GenericJdbcExportInitializer.java | 13 +--- .../jdbc/GenericJdbcImportInitializer.java | 14 +--- .../connector/jdbc/GenericJdbcValidator.java | 72 +++++++++++++++++-- 3 files changed, 70 insertions(+), 29 deletions(-) diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java index 531dd2e9..06fbc519 100644 --- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java +++ b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java @@ -58,17 +58,8 @@ private void configureJdbcProperties(MutableContext context, ConnectionConfigura String username = connectionConfig.connection.username; String password = connectionConfig.connection.password; - if (driver == null) { - throw new SqoopException( - GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0012, - "JDBC Driver"); - } - - if (url == null) { - throw new SqoopException( - GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0012, - "Connection string"); - } + assert driver != null; + assert url != null; executor = new GenericJdbcExecutor(driver, url, username, password); } diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java index c6336fab..a509e2b6 100644 --- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java +++ b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java @@ -67,18 +67,8 @@ private void configureJdbcProperties(MutableContext context, ConnectionConfigura String username = connectionConfig.connection.username; String password = connectionConfig.connection.password; - // TODO(jarcec): Those checks should be in validator and not here - if (driver == null) { - throw new SqoopException( - GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0012, - "JDBC Driver"); - } - - if (url == null) { - throw new SqoopException( - GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0012, - "Connection string"); - } + assert driver != null; + assert url != null; executor = new GenericJdbcExecutor(driver, url, username, password); } diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java index 5ba1aae1..8980b629 100644 --- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java +++ b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java @@ -18,12 +18,15 @@ package org.apache.sqoop.connector.jdbc; import org.apache.sqoop.connector.jdbc.configuration.ConnectionConfiguration; +import org.apache.sqoop.connector.jdbc.configuration.ExportJobConfiguration; +import org.apache.sqoop.connector.jdbc.configuration.ImportJobConfiguration; +import org.apache.sqoop.model.MJob; import org.apache.sqoop.validation.Status; import org.apache.sqoop.validation.Validation; import org.apache.sqoop.validation.Validator; /** - * + * Validator to ensure that user is supplying valid input */ public class GenericJdbcValidator extends Validator { @@ -32,11 +35,68 @@ public Validation validateConnection(Object configuration) { Validation validation = new Validation(ConnectionConfiguration.class); ConnectionConfiguration config = (ConnectionConfiguration)configuration; - if(config.connection.connectionString == null - || !config.connection.connectionString.startsWith("jdbc:")) { - validation.addMessage(Status.UNACCEPTABLE, - "connection", "connectionString", - "This do not seem as a valid JDBC URL"); + if(config.connection.jdbcDriver == null) { + validation.addMessage(Status.UNACCEPTABLE, "connection", "jdbcDriver", "Driver can't be empty"); + } else { + try { + Class.forName(config.connection.jdbcDriver); + } catch (ClassNotFoundException e) { + validation.addMessage(Status.UNACCEPTABLE, "connection", "jdbcDriver", "Can't load specified driver"); + } + } + + if(config.connection.connectionString == null) { + validation.addMessage(Status.UNACCEPTABLE, "connection", "connectionString", "JDBC URL can't be empty"); + } else if(!config.connection.connectionString.startsWith("jdbc:")) { + 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 + + return validation; + } + + @Override + public Validation validateJob(MJob.Type type, Object jobConfiguration) { + switch(type) { + case IMPORT: + return validateImportJob(jobConfiguration); + case EXPORT: + return validateExportJob(jobConfiguration); + default: + return super.validateJob(type, jobConfiguration); + } + } + + 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", "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", "sql", "Both table name and SQL cannot be specified"); + } + + return validation; + } + + 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", "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", "sql", "Both table name and SQL cannot be specified"); } return validation;