sqoop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rzh...@apache.org
Subject sqoop git commit: SQOOP-2390: Sqoop2: FindJob will throw exception when job doesn't exist
Date Thu, 11 Jun 2015 06:02:54 GMT
Repository: sqoop
Updated Branches:
  refs/heads/sqoop2 99698deba -> 4042d2df1


SQOOP-2390: Sqoop2: FindJob will throw exception when job doesn't exist

(Dian Fu via Richard Zhou)


Project: http://git-wip-us.apache.org/repos/asf/sqoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/sqoop/commit/4042d2df
Tree: http://git-wip-us.apache.org/repos/asf/sqoop/tree/4042d2df
Diff: http://git-wip-us.apache.org/repos/asf/sqoop/diff/4042d2df

Branch: refs/heads/sqoop2
Commit: 4042d2df1191205ce07ef637b86b27918d09979e
Parents: 99698de
Author: Richard Zhou <rzhou1@apache.org>
Authored: Thu Jun 11 13:51:28 2015 +0800
Committer: Richard Zhou <rzhou1@apache.org>
Committed: Thu Jun 11 13:51:48 2015 +0800

----------------------------------------------------------------------
 .../sqoop/error/code/CommonRepositoryError.java |  8 +-
 .../common/CommonRepositoryHandler.java         | 93 ++++++++++++++------
 .../sqoop/repository/derby/TestJobHandling.java |  7 +-
 .../repository/derby/TestLinkHandling.java      |  7 +-
 .../repository/postgresql/TestJobHandling.java  |  4 +-
 .../repository/postgresql/TestLinkHandling.java |  4 +-
 6 files changed, 75 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sqoop/blob/4042d2df/common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java b/common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java
index 7b8fce5..9f4a0f8 100644
--- a/common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java
+++ b/common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java
@@ -111,8 +111,8 @@ public enum CommonRepositoryError implements ErrorCode {
   /** We can't restore link from repository **/
   COMMON_0020("Unable to load link from repository"),
 
-  /** We can't restore specific link from repository **/
-  COMMON_0021("Unable to load specific link from repository"),
+  /** The repository contains more than one link with same name or id **/
+  COMMON_0021("Invalid entity state - multiple links found"),
 
   /** We're unable to check if given link already exists */
   COMMON_0022("Unable to check if given link exists"),
@@ -129,8 +129,8 @@ public enum CommonRepositoryError implements ErrorCode {
   /** We're unable to check if given job already exists */
   COMMON_0026("Unable to check if given job exists"),
 
-  /** We can't restore specific job from repository **/
-  COMMON_0027("Unable to load specific job from repository"),
+  /** The repository contains more than one job with same name or id **/
+  COMMON_0027("Invalid entity state - multiple jobs found"),
 
   /** We can't restore job from repository **/
   COMMON_0028("Unable to load job from repository"),

http://git-wip-us.apache.org/repos/asf/sqoop/blob/4042d2df/repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
----------------------------------------------------------------------
diff --git a/repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
b/repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
index 774c3b4..c8335c0 100644
--- a/repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
+++ b/repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
@@ -52,6 +52,7 @@ import org.apache.sqoop.model.MStringInput;
 import org.apache.sqoop.model.MSubmission;
 import org.apache.sqoop.model.MToConfig;
 import org.apache.sqoop.repository.JdbcRepositoryHandler;
+import org.apache.sqoop.repository.RepositoryError;
 import org.apache.sqoop.submission.SubmissionStatus;
 import org.apache.sqoop.submission.counter.Counter;
 import org.apache.sqoop.submission.counter.CounterGroup;
@@ -100,7 +101,6 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
     if (LOG.isDebugEnabled()) {
       LOG.debug("Looking up connector: " + shortName);
     }
-    MConnector mc = null;
     PreparedStatement connectorFetchStmt = null;
     try {
       connectorFetchStmt = conn.prepareStatement(crudQueries.getStmtSelectFromConfigurable());
@@ -109,11 +109,12 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
       List<MConnector> connectors = loadConnectors(connectorFetchStmt, conn);
 
       if (connectors.size() == 0) {
-        LOG.debug("No connector found by name: " + shortName);
+        LOG.debug("Looking up connector with name: " + shortName + ", no connector found");
         return null;
       } else if (connectors.size() == 1) {
-        LOG.debug("Looking up connector: " + shortName + ", found: " + mc);
-        return connectors.get(0);
+        MConnector mc = connectors.get(0);
+        LOG.debug("Looking up connector with name: " + shortName + ", found: " + mc);
+        return mc;
       } else {
         throw new SqoopException(CommonRepositoryError.COMMON_0002, shortName);
       }
@@ -578,14 +579,19 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
 
       List<MLink> links = loadLinks(linkFetchStmt, conn);
 
-      if (links.size() != 1) {
-        throw new SqoopException(CommonRepositoryError.COMMON_0021, "Couldn't find"
-            + " link with id " + linkId);
+      if (links.size() == 0) {
+        LOG.debug("Looking up link with id: " + linkId + ", no link found");
+        return null;
+      } else if (links.size() == 1) {
+        // Return the first and only one link object with the given id
+        MLink link = links.get(0);
+        LOG.debug("Looking up link with id: " + linkId + ", found: " + link);
+        return link;
+      } else {
+        throw new SqoopException(CommonRepositoryError.COMMON_0021,
+            String.valueOf(linkId));
       }
 
-      // Return the first and only one link object with the given id
-      return links.get(0);
-
     } catch (SQLException ex) {
       logException(ex, linkId);
       throw new SqoopException(CommonRepositoryError.COMMON_0020, ex);
@@ -606,13 +612,18 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
 
       List<MLink> links = loadLinks(linkFetchStmt, conn);
 
-      if (links.size() != 1) {
+      if (links.size() == 0) {
+        LOG.debug("Looking up link with name: " + linkName + ", no link found");
         return null;
+      } else if (links.size() == 1) {
+        // Return the first and only one link object with the given name
+        MLink link = links.get(0);
+        LOG.debug("Looking up link with name: " + linkName + ", found: " + link);
+        return link;
+      } else {
+        throw new SqoopException(CommonRepositoryError.COMMON_0021, linkName);
       }
 
-      // Return the first and only one link object with the given name
-      return links.get(0);
-
     } catch (SQLException ex) {
       logException(ex, linkName);
       throw new SqoopException(CommonRepositoryError.COMMON_0020, ex);
@@ -873,14 +884,19 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
 
       List<MJob> jobs = loadJobs(stmt, conn);
 
-      if (jobs.size() != 1) {
-        throw new SqoopException(CommonRepositoryError.COMMON_0027, "Couldn't find"
-            + " job with id " + jobId);
+      if (jobs.size() == 0) {
+        LOG.debug("Looking up job with id: " + jobId + ", no job found");
+        return null;
+      } else if (jobs.size() == 1) {
+        // Return the first and only one job object with the given id
+        MJob job = jobs.get(0);
+        LOG.debug("Looking up job with id: " + jobId + ", found: " + job);
+        return job;
+      } else {
+        throw new SqoopException(CommonRepositoryError.COMMON_0027,
+            String.valueOf(jobId));
       }
 
-      // Return the first and only one link object
-      return jobs.get(0);
-
     } catch (SQLException ex) {
       logException(ex, jobId);
       throw new SqoopException(CommonRepositoryError.COMMON_0028, ex);
@@ -901,13 +917,18 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
 
       List<MJob> jobs = loadJobs(stmt, conn);
 
-      if (jobs.size() != 1) {
+      if (jobs.size() == 0) {
+        LOG.debug("Looking up job with name: " + name + ", no job found");
         return null;
+      } else if (jobs.size() == 1) {
+        // Return the first and only one job object with the given id
+        MJob job = jobs.get(0);
+        LOG.debug("Looking up job with name: " + name + ", found: " + job);
+        return job;
+      } else {
+        throw new SqoopException(CommonRepositoryError.COMMON_0027, name);
       }
 
-      // Return the first and only one link object
-      return jobs.get(0);
-
     } catch (SQLException ex) {
       logException(ex, name);
       throw new SqoopException(CommonRepositoryError.COMMON_0028, ex);
@@ -2076,7 +2097,11 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
 
   @Override
   public MConfig findFromJobConfig(long jobId, String configName, Connection conn) {
-    MFromConfig fromConfigs = findJob(jobId, conn).getFromJobConfig();
+    MJob job = findJob(jobId, conn);
+    if (job == null) {
+      throw new SqoopException(RepositoryError.JDBCREPO_0020, "Invalid id: " + jobId);
+    }
+    MFromConfig fromConfigs = job.getFromJobConfig();
     if (fromConfigs != null) {
       MConfig config = fromConfigs.getConfig(configName);
       if (config == null) {
@@ -2089,7 +2114,11 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
 
   @Override
   public MConfig findToJobConfig(long jobId, String configName, Connection conn) {
-    MToConfig toConfigs = findJob(jobId, conn).getToJobConfig();
+    MJob job = findJob(jobId, conn);
+    if (job == null) {
+      throw new SqoopException(RepositoryError.JDBCREPO_0020, "Invalid id: " + jobId);
+    }
+    MToConfig toConfigs = job.getToJobConfig();
     if (toConfigs != null) {
       MConfig config = toConfigs.getConfig(configName);
       if (config == null) {
@@ -2102,7 +2131,11 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
 
   @Override
   public MConfig findDriverJobConfig(long jobId, String configName, Connection conn) {
-    MDriverConfig driverConfigs = findJob(jobId, conn).getDriverConfig();
+    MJob job = findJob(jobId, conn);
+    if (job == null) {
+      throw new SqoopException(RepositoryError.JDBCREPO_0020, "Invalid id: " + jobId);
+    }
+    MDriverConfig driverConfigs = job.getDriverConfig();
     if (driverConfigs != null) {
       MConfig config = driverConfigs.getConfig(configName);
       if (config == null) {
@@ -2115,7 +2148,11 @@ public abstract class CommonRepositoryHandler extends JdbcRepositoryHandler
{
 
   @Override
   public MConfig findLinkConfig(long linkId, String configName, Connection conn) {
-    MConfig driverConfig = findLink(linkId, conn).getConnectorLinkConfig(configName);
+    MLink link = findLink(linkId, conn);
+    if (link == null) {
+      throw new SqoopException(RepositoryError.JDBCREPO_0017, "Invalid id: " + linkId);
+    }
+    MConfig driverConfig = link.getConnectorLinkConfig(configName);
     if (driverConfig == null) {
       throw new SqoopException(CommonRepositoryError.COMMON_0052, "for configName :" + configName);
     }

http://git-wip-us.apache.org/repos/asf/sqoop/blob/4042d2df/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
----------------------------------------------------------------------
diff --git a/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
b/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
index 6f975a6..de5e3c8 100644
--- a/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
+++ b/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
@@ -66,12 +66,7 @@ public class TestJobHandling extends DerbyTestCase {
   @Test
   public void testFindJob() throws Exception {
     // Let's try to find non existing job
-    try {
-      handler.findJob(1, derbyConnection);
-      fail();
-    } catch(SqoopException ex) {
-      assertEquals(CommonRepositoryError.COMMON_0027, ex.getErrorCode());
-    }
+    assertNull(handler.findJob(1, derbyConnection));
 
     loadJobsForLatestVersion();
 

http://git-wip-us.apache.org/repos/asf/sqoop/blob/4042d2df/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
----------------------------------------------------------------------
diff --git a/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
b/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
index 1ee7996..2a36bd4 100644
--- a/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
+++ b/repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
@@ -58,12 +58,7 @@ public class TestLinkHandling extends DerbyTestCase {
   @Test
   public void testFindLink() throws Exception {
     // Let's try to find non existing link
-    try {
-      handler.findLink(1, getDerbyDatabaseConnection());
-      fail();
-    } catch(SqoopException ex) {
-      assertEquals(CommonRepositoryError.COMMON_0021, ex.getErrorCode());
-    }
+    assertNull(handler.findLink(1, getDerbyDatabaseConnection()));
 
     // Load prepared links into database
     loadLinksForLatestVersion();

http://git-wip-us.apache.org/repos/asf/sqoop/blob/4042d2df/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java
----------------------------------------------------------------------
diff --git a/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java
b/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java
index ad601b4..cf8c1a3 100644
--- a/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java
+++ b/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java
@@ -76,14 +76,14 @@ public class TestJobHandling extends PostgresqlTestCase {
     handler.createJob(getJob(JOB_B_NAME, connectorB, connectorA, linkB, linkA), provider.getConnection());
   }
 
-  @Test(expectedExceptions = SqoopException.class)
+  @Test
   public void testFindJobFail() throws Exception {
     for (MJob job : handler.findJobs(provider.getConnection())) {
       handler.deleteJob(job.getPersistenceId(), provider.getConnection());
     }
 
     // Let's try to find non existing job
-    handler.findJob(1, provider.getConnection());
+    assertNull(handler.findJob(1, provider.getConnection()));
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/sqoop/blob/4042d2df/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java
----------------------------------------------------------------------
diff --git a/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java
b/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java
index 35736d4..699fc42 100644
--- a/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java
+++ b/repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java
@@ -69,14 +69,14 @@ public class TestLinkHandling extends PostgresqlTestCase {
     handler.createLink(linkB, provider.getConnection());
   }
 
-  @Test(expectedExceptions = SqoopException.class)
+  @Test
   public void testFindLinkFail() {
     // Delete links
     for (MLink link : handler.findLinks(provider.getConnection())) {
       handler.deleteLink(link.getPersistenceId(), provider.getConnection());
     }
 
-    handler.findLink(1, provider.getConnection());
+    assertNull(handler.findLink(1, provider.getConnection()));
   }
 
   @Test


Mime
View raw message