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 <[email protected]> Authored: Thu Jun 11 13:51:28 2015 +0800 Committer: Richard Zhou <[email protected]> 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
