Skip to content

Commit

Permalink
Merge pull request #1530 from HubSpot/npe_fix
Browse files Browse the repository at this point in the history 
Fix for Optional request bodies
  • Loading branch information
@ssalinas
ssalinas committed May 10, 2017
2 parents 7d7add7 + f5e0a93 commit 78fd166
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 62 deletions.
3 changes: 2 additions & 1 deletion SingularityService/src/main/java/com/hubspot/singularity/resources/DisastersResource.java
View file
Expand Up @@ -107,7 +107,8 @@ public List<SingularityDisabledAction> disabledActions() {
@POST
@Path("/disabled-actions/{action}")
@ApiOperation(value="Disable a specific action")
public void disableAction(@PathParam("action") SingularityAction action, Optional<SingularityDisabledActionRequest> maybeRequest) {
public void disableAction(@PathParam("action") SingularityAction action, SingularityDisabledActionRequest disabledActionRequest) {
final Optional<SingularityDisabledActionRequest> maybeRequest = Optional.fromNullable(disabledActionRequest);
authorizationHelper.checkAdminAuthorization(user);
Optional<String> message = maybeRequest.isPresent() ? maybeRequest.get().getMessage() : Optional.<String>absent();
disasterManager.disable(action, message, user, false, Optional.<Long>absent());
Expand Down
15 changes: 9 additions & 6 deletions SingularityService/src/main/java/com/hubspot/singularity/resources/RackResource.java
View file
Expand Up @@ -71,22 +71,25 @@ public void removeRack(@ApiParam("Rack ID") @PathParam("rackId") String rackId)
@POST
@Path("/rack/{rackId}/decommission")
@ApiOperation("Begin decommissioning a specific active rack")
public void decommissionRack(@ApiParam("Active rack ID") @PathParam("rackId") String rackId, Optional<SingularityMachineChangeRequest> changeRequest) {
super.decommission(rackId, changeRequest, JavaUtils.getUserEmail(user), SingularityAction.DECOMMISSION_RACK);
public void decommissionRack(@ApiParam("Active rack ID") @PathParam("rackId") String rackId, SingularityMachineChangeRequest changeRequest) {
final Optional<SingularityMachineChangeRequest> maybeChangeRequest = Optional.fromNullable(changeRequest);
super.decommission(rackId, maybeChangeRequest, JavaUtils.getUserEmail(user), SingularityAction.DECOMMISSION_RACK);
}

@POST
@Path("/rack/{rackId}/freeze")
@ApiOperation("Freeze a specific rack")
public void freezeRack(@ApiParam("Rack ID") @PathParam("rackId") String rackId, Optional<SingularityMachineChangeRequest> changeRequest) {
super.freeze(rackId, changeRequest, JavaUtils.getUserEmail(user), SingularityAction.FREEZE_RACK);
public void freezeRack(@ApiParam("Rack ID") @PathParam("rackId") String rackId, SingularityMachineChangeRequest changeRequest) {
final Optional<SingularityMachineChangeRequest> maybeChangeRequest = Optional.fromNullable(changeRequest);
super.freeze(rackId, maybeChangeRequest, JavaUtils.getUserEmail(user), SingularityAction.FREEZE_RACK);
}

@POST
@Path("/rack/{rackId}/activate")
@ApiOperation("Activate a decomissioning rack, canceling decomission without erasing history")
public void activateRack(@ApiParam("Active rackId") @PathParam("rackId") String rackId, Optional<SingularityMachineChangeRequest> changeRequest) {
super.activate(rackId, changeRequest, JavaUtils.getUserEmail(user), SingularityAction.ACTIVATE_RACK);
public void activateRack(@ApiParam("Active rackId") @PathParam("rackId") String rackId, SingularityMachineChangeRequest changeRequest) {
final Optional<SingularityMachineChangeRequest> maybeChangeRequest = Optional.fromNullable(changeRequest);
super.activate(rackId, maybeChangeRequest, JavaUtils.getUserEmail(user), SingularityAction.ACTIVATE_RACK);
}

@DELETE
Expand Down
57 changes: 28 additions & 29 deletions SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java
View file
Expand Up @@ -184,7 +184,7 @@ private String getAndCheckDeployId(String requestId) {
@Path("/request/{requestId}/bounce")
public SingularityRequestParent bounce(@PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext) {
return bounce(requestId, requestContext, Optional.absent());
return bounce(requestId, requestContext, null);
}

@POST
Expand All @@ -194,8 +194,9 @@ public SingularityRequestParent bounce(@PathParam("requestId") String requestId,
response=SingularityRequestParent.class)
public SingularityRequestParent bounce(@ApiParam("The request ID to bounce") @PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext,
@ApiParam("Bounce request options") Optional<SingularityBounceRequest> bounceRequest) {
return maybeProxyToLeader(requestContext, SingularityRequestParent.class, bounceRequest.orNull(), () -> bounce(requestId, bounceRequest));
@ApiParam("Bounce request options") SingularityBounceRequest bounceRequest) {
final Optional<SingularityBounceRequest> maybeBounceRequest = Optional.fromNullable(bounceRequest);
return maybeProxyToLeader(requestContext, SingularityRequestParent.class, maybeBounceRequest.orNull(), () -> bounce(requestId, maybeBounceRequest));
}

public SingularityRequestParent bounce(String requestId, Optional<SingularityBounceRequest> bounceRequest) {
Expand Down Expand Up @@ -253,7 +254,7 @@ public SingularityRequestParent bounce(String requestId, Optional<SingularityBou
@POST
@Path("/request/{requestId}/run")
public SingularityPendingRequestParent scheduleImmediately(@PathParam("requestId") String requestId) {
return scheduleImmediately(requestId, Optional.absent());
return scheduleImmediately(requestId, null);
}

@POST
Expand All @@ -264,7 +265,8 @@ public SingularityPendingRequestParent scheduleImmediately(@PathParam("requestId
@ApiResponse(code=400, message="Singularity Request is not scheduled or one-off"),
})
public SingularityPendingRequestParent scheduleImmediately(@ApiParam("The request ID to run") @PathParam("requestId") String requestId,
Optional<SingularityRunNowRequest> runNowRequest) {
SingularityRunNowRequest runNowRequest) {
final Optional<SingularityRunNowRequest> maybeRunNowRequest = Optional.fromNullable(runNowRequest);
SingularityRequestWithState requestWithState = fetchRequestWithState(requestId);

authorizationHelper.checkForAuthorization(requestWithState.getRequest(), user, SingularityAuthorizationScope.WRITE);
Expand Down Expand Up @@ -296,12 +298,12 @@ public SingularityPendingRequestParent scheduleImmediately(@ApiParam("The reques
Optional<List<String>> commandLineArgs = Optional.absent();
Optional<Resources> resources = Optional.absent();

if (runNowRequest.isPresent()) {
message = runNowRequest.get().getMessage();
runId = runNowRequest.get().getRunId();
skipHealthchecks = runNowRequest.get().getSkipHealthchecks();
commandLineArgs = runNowRequest.get().getCommandLineArgs();
resources = runNowRequest.get().getResources();
if (maybeRunNowRequest.isPresent()) {
message = maybeRunNowRequest.get().getMessage();
runId = maybeRunNowRequest.get().getRunId();
skipHealthchecks = maybeRunNowRequest.get().getSkipHealthchecks();
commandLineArgs = maybeRunNowRequest.get().getCommandLineArgs();
resources = maybeRunNowRequest.get().getResources();
}

if (runId.isPresent() && runId.get().length() > 100) {
Expand Down Expand Up @@ -335,7 +337,7 @@ public Optional<SingularityTaskId> getTaskByRunId(@PathParam("requestId") String
@Path("/request/{requestId}/pause")
public SingularityRequestParent pause(@PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext) {
return pause(requestId, requestContext, Optional.absent());
return pause(requestId, requestContext, null);
}

@POST
Expand All @@ -347,8 +349,9 @@ public SingularityRequestParent pause(@PathParam("requestId") String requestId,
})
public SingularityRequestParent pause(@ApiParam("The request ID to pause") @PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext,
@ApiParam("Pause Request Options") Optional<SingularityPauseRequest> pauseRequest) {
return maybeProxyToLeader(requestContext, SingularityRequestParent.class, pauseRequest.orNull(), () -> pause(requestId, pauseRequest));
@ApiParam("Pause Request Options") SingularityPauseRequest pauseRequest) {
final Optional<SingularityPauseRequest> maybePauseRequest = Optional.fromNullable(pauseRequest);
return maybeProxyToLeader(requestContext, SingularityRequestParent.class, maybePauseRequest.orNull(), () -> pause(requestId, maybePauseRequest));
}

public SingularityRequestParent pause(String requestId, Optional<SingularityPauseRequest> pauseRequest) {
Expand Down Expand Up @@ -400,7 +403,7 @@ public SingularityRequestParent pause(String requestId, Optional<SingularityPaus
@Path("/request/{requestId}/unpause")
public SingularityRequestParent unpauseNoBody(@PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext) {
return unpause(requestId, requestContext, Optional.absent());
return unpause(requestId, requestContext, null);
}

@POST
Expand All @@ -412,8 +415,9 @@ public SingularityRequestParent unpauseNoBody(@PathParam("requestId") String req
})
public SingularityRequestParent unpause(@ApiParam("The request ID to unpause") @PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext,
Optional<SingularityUnpauseRequest> unpauseRequest) {
return maybeProxyToLeader(requestContext, SingularityRequestParent.class, unpauseRequest.orNull(), () -> unpause(requestId, unpauseRequest));
SingularityUnpauseRequest unpauseRequest) {
final Optional<SingularityUnpauseRequest> maybeUnpauseRequest = Optional.fromNullable(unpauseRequest);
return maybeProxyToLeader(requestContext, SingularityRequestParent.class, maybeUnpauseRequest.orNull(), () -> unpause(requestId, maybeUnpauseRequest));
}

public SingularityRequestParent unpause(String requestId, Optional<SingularityUnpauseRequest> unpauseRequest) {
Expand Down Expand Up @@ -443,7 +447,7 @@ public SingularityRequestParent unpause(String requestId, Optional<SingularityUn
@Path("/request/{requestId}/exit-cooldown")
public SingularityRequestParent exitCooldown(@PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext) {
return exitCooldown(requestId, requestContext, Optional.absent());
return exitCooldown(requestId, requestContext, null);
}

@POST
Expand All @@ -455,8 +459,9 @@ public SingularityRequestParent exitCooldown(@PathParam("requestId") String requ
})
public SingularityRequestParent exitCooldown(@PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext,
Optional<SingularityExitCooldownRequest> exitCooldownRequest) {
return maybeProxyToLeader(requestContext, SingularityRequestParent.class, exitCooldownRequest.orNull(), () -> exitCooldown(requestId, exitCooldownRequest));
SingularityExitCooldownRequest exitCooldownRequest) {
final Optional<SingularityExitCooldownRequest> maybeExitCooldownRequest = Optional.fromNullable(exitCooldownRequest);
return maybeProxyToLeader(requestContext, SingularityRequestParent.class, maybeExitCooldownRequest.orNull(), () -> exitCooldown(requestId, maybeExitCooldownRequest));
}

public SingularityRequestParent exitCooldown(String requestId, Optional<SingularityExitCooldownRequest> exitCooldownRequest) {
Expand Down Expand Up @@ -589,13 +594,6 @@ public SingularityRequestParent getRequest(String requestId) {
return fillEntireRequest(fetchRequestWithState(requestId, false));
}

@DELETE
@Path("/request/{requestId}")
public SingularityRequest deleteRequest(@PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext) {
return deleteRequest(requestId, requestContext, Optional.absent());
}

@DELETE
@Path("/request/{requestId}")
@Consumes({ MediaType.APPLICATION_JSON })
Expand All @@ -605,8 +603,9 @@ public SingularityRequest deleteRequest(@PathParam("requestId") String requestId
})
public SingularityRequest deleteRequest(@ApiParam("The request ID to delete.") @PathParam("requestId") String requestId,
@Context HttpServletRequest requestContext,
@ApiParam("Delete options") Optional<SingularityDeleteRequestRequest> deleteRequest) {
return maybeProxyToLeader(requestContext, SingularityRequest.class, deleteRequest.orNull(), () -> deleteRequest(requestId, deleteRequest));
@ApiParam("Delete options") SingularityDeleteRequestRequest deleteRequest) {
final Optional<SingularityDeleteRequestRequest> maybeDeleteRequest = Optional.fromNullable(deleteRequest);
return maybeProxyToLeader(requestContext, SingularityRequest.class, maybeDeleteRequest.orNull(), () -> deleteRequest(requestId, maybeDeleteRequest));
}

public SingularityRequest deleteRequest(String requestId, Optional<SingularityDeleteRequestRequest> deleteRequest) {
Expand Down
15 changes: 9 additions & 6 deletions SingularityService/src/main/java/com/hubspot/singularity/resources/SlaveResource.java
View file
Expand Up @@ -76,22 +76,25 @@ public void removeSlave(@ApiParam("Active SlaveId") @PathParam("slaveId") String
@POST
@Path("/slave/{slaveId}/decommission")
@ApiOperation("Begin decommissioning a specific active slave")
public void decommissionSlave(@ApiParam("Active slaveId") @PathParam("slaveId") String slaveId, Optional<SingularityMachineChangeRequest> changeRequest) {
super.decommission(slaveId, changeRequest, JavaUtils.getUserEmail(user), SingularityAction.DECOMMISSION_SLAVE);
public void decommissionSlave(@ApiParam("Active slaveId") @PathParam("slaveId") String slaveId, SingularityMachineChangeRequest changeRequest) {
final Optional<SingularityMachineChangeRequest> maybeChangeRequest = Optional.fromNullable(changeRequest);
super.decommission(slaveId, maybeChangeRequest, JavaUtils.getUserEmail(user), SingularityAction.DECOMMISSION_SLAVE);
}

@POST
@Path("/slave/{slaveId}/freeze")
@ApiOperation("Freeze tasks on a specific slave")
public void freezeSlave(@ApiParam("Slave ID") @PathParam("slaveId") String slaveId, Optional<SingularityMachineChangeRequest> changeRequest) {
super.freeze(slaveId, changeRequest, JavaUtils.getUserEmail(user), SingularityAction.FREEZE_SLAVE);
public void freezeSlave(@ApiParam("Slave ID") @PathParam("slaveId") String slaveId, SingularityMachineChangeRequest changeRequest) {
final Optional<SingularityMachineChangeRequest> maybeChangeRequest = Optional.fromNullable(changeRequest);
super.freeze(slaveId, maybeChangeRequest, JavaUtils.getUserEmail(user), SingularityAction.FREEZE_SLAVE);
}

@POST
@Path("/slave/{slaveId}/activate")
@ApiOperation("Activate a decomissioning slave, canceling decomission without erasing history")
public void activateSlave(@ApiParam("Active slaveId") @PathParam("slaveId") String slaveId, Optional<SingularityMachineChangeRequest> changeRequest) {
super.activate(slaveId, changeRequest, JavaUtils.getUserEmail(user), SingularityAction.ACTIVATE_SLAVE);
public void activateSlave(@ApiParam("Active slaveId") @PathParam("slaveId") String slaveId, SingularityMachineChangeRequest changeRequest) {
final Optional<SingularityMachineChangeRequest> maybeChangeRequest = Optional.fromNullable(changeRequest);
super.activate(slaveId, maybeChangeRequest, JavaUtils.getUserEmail(user), SingularityAction.ACTIVATE_SLAVE);
}

@DELETE
Expand Down
10 changes: 6 additions & 4 deletions SingularityService/src/main/java/com/hubspot/singularity/resources/TaskResource.java
View file
Expand Up @@ -282,7 +282,7 @@ public Optional<SingularityTaskCleanup> getTaskCleanup(@PathParam("taskId") Stri
@DELETE
@Path("/task/{taskId}")
public SingularityTaskCleanup killTask(@PathParam("taskId") String taskId, @Context HttpServletRequest requestContext) {
return killTask(taskId, requestContext, Optional.absent());
return killTask(taskId, requestContext, null);
}

@DELETE
Expand All @@ -292,9 +292,11 @@ public SingularityTaskCleanup killTask(@PathParam("taskId") String taskId, @Cont
@ApiResponses({
@ApiResponse(code=409, message="Task already has a cleanup request (can be overridden with override=true)")
})
public SingularityTaskCleanup killTask(@PathParam("taskId") String taskId, @Context HttpServletRequest requestContext, Optional<SingularityKillTaskRequest> killTaskRequest
) {
return maybeProxyToLeader(requestContext, SingularityTaskCleanup.class, killTaskRequest.orNull(), () -> killTask(taskId, killTaskRequest));
public SingularityTaskCleanup killTask(@PathParam("taskId") String taskId,
@Context HttpServletRequest requestContext,
SingularityKillTaskRequest killTaskRequest) {
final Optional<SingularityKillTaskRequest> maybeKillTaskRequest = Optional.fromNullable(killTaskRequest);
return maybeProxyToLeader(requestContext, SingularityTaskCleanup.class, maybeKillTaskRequest.orNull(), () -> killTask(taskId, maybeKillTaskRequest));
}

public SingularityTaskCleanup killTask(String taskId, Optional<SingularityKillTaskRequest> killTaskRequest) {
Expand Down
2 changes: 1 addition & 1 deletion SingularityService/src/test/java/com/hubspot/singularity/SingularityHistoryTest.java
View file
Expand Up @@ -200,7 +200,7 @@ public void testRunId() {
String runId = "my-run-id";

SingularityPendingRequestParent parent = requestResource.scheduleImmediately(requestId,
Optional.of(new SingularityRunNowRequest(Optional.<String> absent(), Optional.<Boolean> absent(), Optional.of(runId), Optional.<List<String>> absent(), Optional.<Resources>absent())));
new SingularityRunNowRequest(Optional.<String> absent(), Optional.<Boolean> absent(), Optional.of(runId), Optional.<List<String>> absent(), Optional.<Resources>absent()));

Assert.assertEquals(runId, parent.getPendingRequest().getRunId().get());

Expand Down

0 comments on commit 78fd166

Please sign in to comment.