less duplication

SingularityBase/src/main/java/com/hubspot/singularity/SingularityPendingRequestParent.java

@@ -1,7 +1,5 @@
 package com.hubspot.singularity;
 
-import java.util.List;
-
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Optional;
@@ -18,7 +16,7 @@ public static SingularityPendingRequestParent fromSingularityRequestParent(Singu
     return new SingularityPendingRequestParent(singularityRequestParent.getRequest(), singularityRequestParent.getState(), singularityRequestParent.getRequestDeployState(),
         singularityRequestParent.getActiveDeploy(), singularityRequestParent.getPendingDeploy(), singularityRequestParent.getPendingDeployState(), pendingRequest,
         singularityRequestParent.getExpiringBounce(), singularityRequestParent.getExpiringPause(), singularityRequestParent.getExpiringScale(),
-        singularityRequestParent.getExpiringSkipHealthchecks(), singularityRequestParent.getTaskIds(), singularityRequestParent.getRecentHistory());
+        singularityRequestParent.getExpiringSkipHealthchecks(), singularityRequestParent.getTaskIds(), singularityRequestParent.getLastHistory());
   }
 
   @JsonCreator
@@ -34,7 +32,7 @@ public SingularityPendingRequestParent(@JsonProperty("request") SingularityReque
                                          @JsonProperty("expiringScale") Optional<SingularityExpiringScale> expiringScale,
                                          @JsonProperty("expiringSkipHealthchecks") Optional<SingularityExpiringSkipHealthchecks> expiringSkipHealthchecks,
                                          @JsonProperty("taskIds") Optional<SingularityTaskIdsByStatus> taskIds,
-                                         @JsonProperty("recentHistory") List<SingularityRequestHistory> recentHistory) {
+                                         @JsonProperty("recentHistory") Optional<SingularityRequestHistory> recentHistory) {
     super(request, state, requestDeployState, activeDeploy, pendingDeploy, pendingDeployState, expiringBounce, expiringPause, expiringScale, expiringSkipHealthchecks, taskIds, recentHistory);
     this.pendingRequest = pendingRequest;
   }

SingularityService/src/main/java/com/hubspot/singularity/helpers/RequestHelper.java

@@ -176,8 +176,7 @@ public void updateRequest(SingularityRequest request, Optional<SingularityReques
   public List<SingularityRequestParent> fillDataForRequestsAndFilter(List<SingularityRequestWithState> requests,
                                                                      Optional<SingularityUser> user,
                                                                      Boolean filterRelevantForUser,
-                                                                     Boolean includeTaskIds,
-                                                                     Boolean includeLastHistory,
+                                                                     Boolean includeFullRequestData,
                                                                      Optional<Integer> limit) {
     Map<String, Optional<SingularityRequestHistory>> requestIdToLastHistory = new HashMap<>();
     Map<String, Optional<SingularityRequestDeployState>> deployStates = new HashMap<>();
@@ -197,7 +196,7 @@ public List<SingularityRequestParent> fillDataForRequestsAndFilter(List<Singular
             // The user is in the group for this request
             return true;
           }
-          if (includeLastHistory) {
+          if (includeFullRequestData) {
             Optional<SingularityRequestHistory> lastHistory = requestIdToLastHistory.computeIfAbsent(requestId, requestHistoryHelper::getLastHistory);
             if (userModifiedRequestLast(lastHistory, user)) {
               return true;
@@ -222,20 +221,18 @@ public List<SingularityRequestParent> fillDataForRequestsAndFilter(List<Singular
     final List<SingularityRequestParent> parents = Lists.newArrayListWithCapacity(requestIds.size());
 
     for (SingularityRequestWithState requestWithState : requests) {
-      Optional<SingularityRequestHistory> lastHistory = includeLastHistory ? requestIdToLastHistory.computeIfAbsent(requestWithState.getRequest().getId(), requestHistoryHelper::getLastHistory) : Optional.absent();
-      Optional<SingularityRequestDeployState> deployState = deployStates.computeIfAbsent(requestWithState.getRequest().getId(), deployManager::getRequestDeployState);
       parents.add(new SingularityRequestParent(
           requestWithState.getRequest(),
           requestWithState.getState(),
-          deployState,
+          deployStates.computeIfAbsent(requestWithState.getRequest().getId(), deployManager::getRequestDeployState),
           Optional.absent(), // full activeDeploy data not provided
           Optional.absent(), Optional.absent(), // full pendingDeploy data and state not provided
-          requestManager.getExpiringBounce(requestWithState.getRequest().getId()),
-          requestManager.getExpiringPause(requestWithState.getRequest().getId()),
-          requestManager.getExpiringScale(requestWithState.getRequest().getId()),
-          requestManager.getExpiringSkipHealthchecks(requestWithState.getRequest().getId()),
-          includeTaskIds ? getTaskIdsByStatusForRequest(requestWithState.getRequest().getId()) : Optional.absent(),
-          includeLastHistory ? requestIdToLastHistory.computeIfAbsent(requestWithState.getRequest().getId(), requestHistoryHelper::getLastHistory) : Optional.absent()));
+          includeFullRequestData ? requestManager.getExpiringBounce(requestWithState.getRequest().getId()) : Optional.absent(),
+          includeFullRequestData ? requestManager.getExpiringPause(requestWithState.getRequest().getId()) : Optional.absent(),
+          includeFullRequestData ? requestManager.getExpiringScale(requestWithState.getRequest().getId()) : Optional.absent(),
+          includeFullRequestData ? requestManager.getExpiringSkipHealthchecks(requestWithState.getRequest().getId()) : Optional.absent(),
+          includeFullRequestData ? getTaskIdsByStatusForRequest(requestWithState) : Optional.absent(),
+          includeFullRequestData ? requestIdToLastHistory.computeIfAbsent(requestWithState.getRequest().getId(), requestHistoryHelper::getLastHistory) : Optional.absent()));
     }
 
     return parents;
@@ -246,6 +243,12 @@ public Optional<SingularityTaskIdsByStatus> getTaskIdsByStatusForRequest(String
     if (!requestWithState.isPresent()) {
       return Optional.absent();
     }
+
+    return getTaskIdsByStatusForRequest(requestWithState.get());
+  }
+
+  private Optional<SingularityTaskIdsByStatus> getTaskIdsByStatusForRequest(SingularityRequestWithState requestWithState) {
+    String requestId = requestWithState.getRequest().getId();
     Optional<SingularityPendingDeploy> pendingDeploy = deployManager.getPendingDeploy(requestId);
 
     List<SingularityTaskId> cleaningTaskIds = taskManager.getCleanupTaskIds().stream().filter((t) -> t.getRequestId().equals(requestId)).collect(Collectors.toList());
@@ -259,7 +262,7 @@ public Optional<SingularityTaskIdsByStatus> getTaskIdsByStatusForRequest(String
     for (Map.Entry<String, List<SingularityTaskId>> entry : taskIdsByDeployId.entrySet()) {
       Optional<SingularityDeploy> deploy = deployManager.getDeploy(requestId, entry.getKey());
       List<SingularityTaskId> healthyTasksIdsForDeploy = deployHealthHelper.getHealthyTasks(
-          requestWithState.get().getRequest(),
+          requestWithState.getRequest(),
           deploy,
           entry.getValue(),
           pendingDeploy.isPresent() && pendingDeploy.get().getDeployMarker().getDeployId().equals(entry.getKey()));

SingularityService/src/main/java/com/hubspot/singularity/resources/DeployResource.java

@@ -52,6 +52,8 @@
 import com.hubspot.singularity.data.RequestManager;
 import com.hubspot.singularity.data.SingularityValidator;
 import com.hubspot.singularity.data.TaskManager;
+import com.hubspot.singularity.data.history.RequestHistoryHelper;
+import com.hubspot.singularity.helpers.RequestHelper;
 import com.ning.http.client.AsyncHttpClient;
 import com.wordnik.swagger.annotations.Api;
 import com.wordnik.swagger.annotations.ApiOperation;
@@ -69,8 +71,8 @@ public class DeployResource extends AbstractRequestResource {
   @Inject
   public DeployResource(RequestManager requestManager, DeployManager deployManager, SingularityValidator validator, SingularityAuthorizationHelper authorizationHelper, Optional<SingularityUser> user,
                         SingularityConfiguration configuration, TaskManager taskManager, LeaderLatch leaderLatch,
-                        AsyncHttpClient httpClient, ObjectMapper objectMapper) {
-    super(requestManager, deployManager, user, validator, authorizationHelper, httpClient, leaderLatch, objectMapper);
+                        AsyncHttpClient httpClient, ObjectMapper objectMapper, RequestHelper requestHelper, RequestHistoryHelper requestHistoryHelper) {
+    super(requestManager, deployManager, user, validator, authorizationHelper, httpClient, leaderLatch, objectMapper, requestHelper, requestHistoryHelper);
     this.configuration = configuration;
     this.taskManager = taskManager;
   }

SingularityService/src/main/java/com/hubspot/singularity/resources/TaskResource.java

@@ -4,11 +4,8 @@
 import static com.hubspot.singularity.WebExceptions.checkNotFound;
 import static com.hubspot.singularity.WebExceptions.notFound;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
-import java.util.stream.Collectors;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.Consumes;
@@ -44,14 +41,11 @@
 import com.hubspot.singularity.SingularityAction;
 import com.hubspot.singularity.SingularityAuthorizationScope;
 import com.hubspot.singularity.SingularityCreateResult;
-import com.hubspot.singularity.SingularityDeploy;
 import com.hubspot.singularity.SingularityKilledTaskIdRecord;
-import com.hubspot.singularity.SingularityPendingDeploy;
 import com.hubspot.singularity.SingularityPendingRequest;
 import com.hubspot.singularity.SingularityPendingRequest.PendingType;
 import com.hubspot.singularity.SingularityPendingTask;
 import com.hubspot.singularity.SingularityPendingTaskId;
-import com.hubspot.singularity.SingularityRequestWithState;
 import com.hubspot.singularity.SingularityShellCommand;
 import com.hubspot.singularity.SingularitySlave;
 import com.hubspot.singularity.SingularityTask;
@@ -73,14 +67,13 @@
 import com.hubspot.singularity.auth.SingularityAuthorizationHelper;
 import com.hubspot.singularity.config.ApiPaths;
 import com.hubspot.singularity.config.SingularityTaskMetadataConfiguration;
-import com.hubspot.singularity.data.DeployManager;
 import com.hubspot.singularity.data.DisasterManager;
 import com.hubspot.singularity.data.RequestManager;
 import com.hubspot.singularity.data.SingularityValidator;
 import com.hubspot.singularity.data.SlaveManager;
 import com.hubspot.singularity.data.TaskManager;
 import com.hubspot.singularity.data.TaskRequestManager;
-import com.hubspot.singularity.scheduler.SingularityDeployHealthHelper;
+import com.hubspot.singularity.helpers.RequestHelper;
 import com.ning.http.client.AsyncHttpClient;
 import com.wordnik.swagger.annotations.Api;
 import com.wordnik.swagger.annotations.ApiOperation;
@@ -103,13 +96,12 @@ public class TaskResource extends AbstractLeaderAwareResource {
   private final SingularityTaskMetadataConfiguration taskMetadataConfiguration;
   private final SingularityValidator validator;
   private final DisasterManager disasterManager;
-  private final SingularityDeployHealthHelper deployHealthHelper;
-  private final DeployManager deployManager;
+  private final RequestHelper requestHelper;
 
   @Inject
   public TaskResource(TaskRequestManager taskRequestManager, TaskManager taskManager, SlaveManager slaveManager, MesosClient mesosClient, SingularityTaskMetadataConfiguration taskMetadataConfiguration,
                       SingularityAuthorizationHelper authorizationHelper, Optional<SingularityUser> user, RequestManager requestManager, SingularityValidator validator, DisasterManager disasterManager,
-                      AsyncHttpClient httpClient, LeaderLatch leaderLatch, ObjectMapper objectMapper, SingularityDeployHealthHelper deployHealthHelper, DeployManager deployManager) {
+                      AsyncHttpClient httpClient, LeaderLatch leaderLatch, ObjectMapper objectMapper, RequestHelper requestHelper) {
     super(httpClient, leaderLatch, objectMapper);
     this.taskManager = taskManager;
     this.taskRequestManager = taskRequestManager;
@@ -121,8 +113,7 @@ public TaskResource(TaskRequestManager taskRequestManager, TaskManager taskManag
     this.user = user;
     this.validator = validator;
     this.disasterManager = disasterManager;
-    this.deployHealthHelper = deployHealthHelper;
-    this.deployManager = deployManager;
+    this.requestHelper = requestHelper;
   }
 
   @GET
@@ -199,37 +190,7 @@ public List<SingularityTaskRequest> getScheduledTasksForRequest(@PathParam("requ
   public Optional<SingularityTaskIdsByStatus> getTaskIdsByStatusForRequest(@PathParam("requestId") String requestId) {
     authorizationHelper.checkForAuthorizationByRequestId(requestId, user, SingularityAuthorizationScope.READ);
 
-    Optional<SingularityRequestWithState> requestWithState = requestManager.getRequest(requestId);
-    if (!requestWithState.isPresent()) {
-       return Optional.absent();
-    }
-    Optional<SingularityPendingDeploy> pendingDeploy = deployManager.getPendingDeploy(requestId);
-
-    List<SingularityTaskId> cleaningTaskIds = taskManager.getCleanupTaskIds().stream().filter((t) -> t.getRequestId().equals(requestId)).collect(Collectors.toList());
-    List<SingularityPendingTaskId> pendingTaskIds = taskManager.getPendingTaskIdsForRequest(requestId);
-    List<SingularityTaskId> activeTaskIds = taskManager.getActiveTaskIdsForRequest(requestId);
-    activeTaskIds.removeAll(cleaningTaskIds);
-
-    List<SingularityTaskId> healthyTaskIds = new ArrayList<>();
-    List<SingularityTaskId> notYetHealthyTaskIds = new ArrayList<>();
-    Map<String, List<SingularityTaskId>> taskIdsByDeployId = activeTaskIds.stream().collect(Collectors.groupingBy(SingularityTaskId::getDeployId));
-    for (Map.Entry<String, List<SingularityTaskId>> entry : taskIdsByDeployId.entrySet()) {
-      Optional<SingularityDeploy> deploy = deployManager.getDeploy(requestId, entry.getKey());
-      List<SingularityTaskId> healthyTasksIdsForDeploy = deployHealthHelper.getHealthyTasks(
-          requestWithState.get().getRequest(),
-          deploy,
-          entry.getValue(),
-          pendingDeploy.isPresent() && pendingDeploy.get().getDeployMarker().getDeployId().equals(entry.getKey()));
-      for (SingularityTaskId taskId : entry.getValue()) {
-        if (healthyTasksIdsForDeploy.contains(taskId)) {
-          healthyTaskIds.add(taskId);
-        } else {
-          notYetHealthyTaskIds.add(taskId);
-        }
-      }
-    }
-
-    return Optional.of(new SingularityTaskIdsByStatus(healthyTaskIds, notYetHealthyTaskIds, pendingTaskIds, cleaningTaskIds));
+    return requestHelper.getTaskIdsByStatusForRequest(requestId);
   }
 
   @GET