SQOOP-2848: Sqoop2: RESTiliency: Simplify JobRequestHandler.getJobs similarly as was done for getLinks

(Jarek Jarcec Cecho via Abraham Fine)

server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java

@@ -20,9 +20,11 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Date;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 import org.apache.sqoop.audit.AuditLoggerManager;
 import org.apache.sqoop.common.Direction;
@@ -257,48 +259,35 @@ private JsonBean createUpdateJob(RequestContext ctx, boolean create) {
   }
 
   private JsonBean getJobs(RequestContext ctx) {
-    String connectorIdentifier = ctx.getLastURLElement();
+    String jobName = ctx.getLastURLElement();
-    JobBean jobBean;
+    List<MJob> jobs;
     Locale locale = ctx.getAcceptLanguageHeader();
     Repository repository = RepositoryManager.getInstance().getRepository();
-    // jobs by connector
-    if (ctx.getParameterValue(CONNECTOR_NAME_QUERY_PARAM) != null) {
-      connectorIdentifier = ctx.getParameterValue(CONNECTOR_NAME_QUERY_PARAM);
-      AuditLoggerManager.getInstance().logAuditEvent(ctx.getUserName(),
-          ctx.getRequest().getRemoteAddr(), "get", "jobsByConnector", connectorIdentifier);
-      List<MJob> jobList = repository.findJobsForConnector(connectorIdentifier);
 
-      // Authorization check
+    AuditLoggerManager.getInstance().logAuditEvent(ctx.getUserName(), ctx.getRequest().getRemoteAddr(), "get", "job", jobName);
-      jobList = AuthorizationEngine.filterResource(ctx.getUserName(), MResource.TYPE.JOB, jobList);
 
-      jobBean = createJobBean(jobList, locale);
+    if(jobName.equals("all")) { // Return all links (by perhaps only for given connector)
-    } else
+      String connectorName = ctx.getParameterValue(CONNECTOR_NAME_QUERY_PARAM);
-    // all jobs in the system
-    if (ctx.getPath().contains(JOBS_PATH)
-        || (ctx.getPath().contains(JOB_PATH) && connectorIdentifier.equals("all"))) {
-      AuditLoggerManager.getInstance().logAuditEvent(ctx.getUserName(),
-          ctx.getRequest().getRemoteAddr(), "get", "jobs", "all");
-      List<MJob> jobList = repository.findJobs();
 
-      // Authorization check
+      if(StringUtils.isEmpty(connectorName)) {
-      jobList = AuthorizationEngine.filterResource(ctx.getUserName(), MResource.TYPE.JOB, jobList);
+        jobs = repository.findJobs();
-
+      } else {
-      jobBean = createJobBean(jobList, locale);
+        if(repository.findConnector(connectorName) == null) {
+          throw new SqoopException(ServerError.SERVER_0006, "Invalid connector: " + connectorName);
+        }
+        jobs = repository.findJobsForConnector(connectorName);
+      }
+    } else { // Return one specific job with name or id stored in identifier
+      MJob job = HandlerUtils.getJobFromIdentifier(jobName);
+      jobs = new LinkedList<>();
+      jobs.add(job);
     }
-    // job by Id
-    else {
-      AuditLoggerManager.getInstance().logAuditEvent(ctx.getUserName(),
-          ctx.getRequest().getRemoteAddr(), "get", "job", connectorIdentifier);
 
-      MJob job = HandlerUtils.getJobFromIdentifier(connectorIdentifier);
+    // Authorization check
-      String jobName = job.getName();
+    jobs = AuthorizationEngine.filterResource(ctx.getUserName(), MResource.TYPE.JOB, jobs);
 
-      // Authorization check
+    // And return resulting links
-      AuthorizationEngine.readJob(ctx.getUserName(), jobName);
+    return createJobBean(jobs, locale);
-
-      jobBean = createJobBean(Arrays.asList(job), locale);
-    }
-    return jobBean;
   }
 
   private JobBean createJobBean(List<MJob> jobs, Locale locale) {

test/src/test/java/org/apache/sqoop/integration/server/rest/JobRestTest.java

@@ -0,0 +1,72 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sqoop.integration.server.rest;
+
+import org.apache.sqoop.test.utils.ParametrizedUtils;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Factory;
+
+import java.util.Iterator;
+
+public class JobRestTest extends RestTest {
+
+  @BeforeMethod
+  public void setUp() {
+    createFirstJob();
+  }
+
+  private static Validator firstJob = new Validator() {
+    @Override
+    void validate() throws Exception {
+      assertResponseCode(200);
+      assertContains("first-job");
+    }
+  };
+
+  public static TestDescription[] PROVIDER_DATA = new TestDescription[]{
+    // Get
+    new TestDescription("Get all jobs", "v1/job/all", "GET", null, firstJob),
+    new TestDescription("Get job by name", "v1/job/first-job", "GET", null, firstJob),
+    new TestDescription("Get all jobs for connector", "v1/job/all?cname=hdfs-connector", "GET", null, firstJob),
+    new TestDescription("Get non existing job", "v1/job/i-dont-exists", "GET", null, new Validator() {
+      @Override
+      void validate() throws Exception {
+        assertResponseCode(500);
+        assertServerException("Entity requested doesn't exist", "SERVER_0006");
+        assertContains("Job: i-dont-exists doesn't exist");
+      }}),
+    new TestDescription("Get jobs for non existing connector", "v1/job/all?cname=i-dont-exists", "GET", null, new Validator() {
+      @Override
+      void validate() throws Exception {
+        assertResponseCode(500);
+        assertServerException("Entity requested doesn't exist", "SERVER_0006");
+        assertContains("Invalid connector: i-dont-exists");
+      }}),
+  };
+
+  @DataProvider(name="job-rest-test")
+  public static Iterator<Object[]> data() {
+    return ParametrizedUtils.toArrayOfArrays(PROVIDER_DATA).iterator();
+  }
+
+  @Factory(dataProvider = "job-rest-test")
+  public JobRestTest(TestDescription desc) {
+    super(desc);
+  }
+}

test/src/test/java/org/apache/sqoop/integration/server/rest/RestTest.java

@@ -20,7 +20,9 @@
 import org.apache.log4j.Logger;
 import org.apache.commons.io.IOUtils;
 import org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL;
+import org.apache.sqoop.connector.hdfs.configuration.ToFormat;
 import org.apache.sqoop.model.MConfigList;
+import org.apache.sqoop.model.MJob;
 import org.apache.sqoop.model.MLink;
 import org.apache.sqoop.test.infrastructure.Infrastructure;
 import org.apache.sqoop.test.infrastructure.SqoopTestCase;
@@ -123,6 +125,14 @@ public void createFirstLink() {
     hdfsLinkFrom.setName("first-link");
     saveLink(hdfsLinkFrom);
   }
+  public void createFirstJob() {
+    createFirstLink();
+    MJob job = getClient().createJob("first-link", "first-link");
+    job.setName("first-job");
+    fillHdfsFromConfig(job);
+    fillHdfsToConfig(job, ToFormat.TEXT_FILE);
+    saveJob(job);
+  }
 
   @AfterMethod
   public void dropTestData() {