From eb4eaeaee29fd6632627830d6726083a0f357570 Mon Sep 17 00:00:00 2001 From: Vamshi Gopari Date: Sun, 11 Feb 2024 01:12:55 -0500 Subject: [PATCH 1/6] Fix issue #38: Return 404 instead of 2xx codes for methods handling non-existent tasks --- .../conductor/rest/controllers/TaskResource.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java index bad7968a9..8fcbfdb56 100644 --- a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java +++ b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java @@ -59,7 +59,7 @@ public ResponseEntity poll( // for backwards compatibility with 2.x client which expects a 204 when no Task is found return Optional.ofNullable(taskService.poll(taskType, workerId, domain)) .map(ResponseEntity::ok) - .orElse(ResponseEntity.noContent().build()); + .orElse(ResponseEntity.notFound().build()); } @GetMapping("/poll/batch/{tasktype}") @@ -74,7 +74,7 @@ public ResponseEntity> batchPoll( return Optional.ofNullable( taskService.batchPoll(taskType, workerId, domain, count, timeout)) .map(ResponseEntity::ok) - .orElse(ResponseEntity.noContent().build()); + .orElse(ResponseEntity.notFound().build()); } @PostMapping(produces = TEXT_PLAIN_VALUE) @@ -103,8 +103,11 @@ public void log(@PathVariable("taskId") String taskId, @RequestBody String log) @GetMapping("/{taskId}/log") @Operation(summary = "Get Task Execution Logs") - public List getTaskLogs(@PathVariable("taskId") String taskId) { - return taskService.getTaskLogs(taskId); + public ResponseEntity> getTaskLogs(@PathVariable("taskId") String taskId) { + return Optional.ofNullable(taskService.getTaskLogs(taskId)) + .filter(logs -> !logs.isEmpty()) + .map(ResponseEntity::ok) + .orElse(ResponseEntity.notFound().build()); } @GetMapping("/{taskId}") @@ -113,7 +116,7 @@ public ResponseEntity getTask(@PathVariable("taskId") String taskId) { // for backwards compatibility with 2.x client which expects a 204 when no Task is found return Optional.ofNullable(taskService.getTask(taskId)) .map(ResponseEntity::ok) - .orElse(ResponseEntity.noContent().build()); + .orElse(ResponseEntity.notFound().build()); } @GetMapping("/queue/sizes") From 9f25ce60a3a3131ed2923b783479b7f1ce516de7 Mon Sep 17 00:00:00 2001 From: Vamshi Gopari Date: Sun, 11 Feb 2024 01:26:43 -0500 Subject: [PATCH 2/6] Fix issue #38: Return 404 instead of 2xx codes for methods handling non-existent tasks --- .../com/netflix/conductor/rest/controllers/TaskResource.java | 1 + 1 file changed, 1 insertion(+) diff --git a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java index 8fcbfdb56..72cf1c4fb 100644 --- a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java +++ b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java @@ -73,6 +73,7 @@ public ResponseEntity> batchPoll( // for backwards compatibility with 2.x client which expects a 204 when no Task is found return Optional.ofNullable( taskService.batchPoll(taskType, workerId, domain, count, timeout)) + .filter(logs -> !logs.isEmpty()) .map(ResponseEntity::ok) .orElse(ResponseEntity.notFound().build()); } From 58abdd5509975e309ed177f191d569ea1ad934af Mon Sep 17 00:00:00 2001 From: Vamshi Gopari Date: Sun, 11 Feb 2024 01:28:34 -0500 Subject: [PATCH 3/6] Fix issue #38: Return 404 instead of 2xx codes for methods handling non-existent tasks --- .../com/netflix/conductor/rest/controllers/TaskResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java index 72cf1c4fb..f2bca44a2 100644 --- a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java +++ b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java @@ -73,7 +73,7 @@ public ResponseEntity> batchPoll( // for backwards compatibility with 2.x client which expects a 204 when no Task is found return Optional.ofNullable( taskService.batchPoll(taskType, workerId, domain, count, timeout)) - .filter(logs -> !logs.isEmpty()) + .filter(tasks -> !tasks.isEmpty()) .map(ResponseEntity::ok) .orElse(ResponseEntity.notFound().build()); } From 828bc5a144823302c479599187b31334a598e0ba Mon Sep 17 00:00:00 2001 From: Vamshi Gopari Date: Wed, 14 Feb 2024 23:36:32 -0500 Subject: [PATCH 4/6] Using NotFoundException when entity is not found #38 --- .../com/netflix/conductor/rest/controllers/TaskResource.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java index f2bca44a2..dd956a4ec 100644 --- a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java +++ b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java @@ -16,6 +16,7 @@ import java.util.Map; import java.util.Optional; +import com.netflix.conductor.core.exception.NotFoundException; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; @@ -108,7 +109,7 @@ public ResponseEntity> getTaskLogs(@PathVariable("taskId") Str return Optional.ofNullable(taskService.getTaskLogs(taskId)) .filter(logs -> !logs.isEmpty()) .map(ResponseEntity::ok) - .orElse(ResponseEntity.notFound().build()); + .orElseThrow(() -> new NotFoundException("Task logs not found for taskId: %s", taskId)); } @GetMapping("/{taskId}") From 8bf7e905c9e8090e4c60c9c2f746e5ba0dffea3c Mon Sep 17 00:00:00 2001 From: Vamshi Gopari Date: Wed, 14 Feb 2024 23:48:34 -0500 Subject: [PATCH 5/6] Using NotFoundException when entity is not found --- .../netflix/conductor/rest/controllers/TaskResource.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java index dd956a4ec..e73a87d6b 100644 --- a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java +++ b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java @@ -60,7 +60,7 @@ public ResponseEntity poll( // for backwards compatibility with 2.x client which expects a 204 when no Task is found return Optional.ofNullable(taskService.poll(taskType, workerId, domain)) .map(ResponseEntity::ok) - .orElse(ResponseEntity.notFound().build()); + .orElseThrow(() -> new NotFoundException("Poll not found for Task Type: %s", taskType)); } @GetMapping("/poll/batch/{tasktype}") @@ -76,7 +76,7 @@ public ResponseEntity> batchPoll( taskService.batchPoll(taskType, workerId, domain, count, timeout)) .filter(tasks -> !tasks.isEmpty()) .map(ResponseEntity::ok) - .orElse(ResponseEntity.notFound().build()); + .orElseThrow(() -> new NotFoundException("Batch Poll not found for Task Type: %s", taskType)); } @PostMapping(produces = TEXT_PLAIN_VALUE) @@ -118,7 +118,7 @@ public ResponseEntity getTask(@PathVariable("taskId") String taskId) { // for backwards compatibility with 2.x client which expects a 204 when no Task is found return Optional.ofNullable(taskService.getTask(taskId)) .map(ResponseEntity::ok) - .orElse(ResponseEntity.notFound().build()); + .orElseThrow(() -> new NotFoundException("Task not found for taskId: %s", taskId)); } @GetMapping("/queue/sizes") From b47d6271a751e81e6b1e597ce1cb9fba81bc1aa7 Mon Sep 17 00:00:00 2001 From: Vamshi Gopari Date: Mon, 4 Mar 2024 12:00:43 -0500 Subject: [PATCH 6/6] Fix issue #38 : Return 204 instead of 404 for /poll endpoints handling non-existent tasks --- .../netflix/conductor/rest/controllers/TaskResource.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java index e73a87d6b..d5d80ecf2 100644 --- a/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java +++ b/rest/src/main/java/com/netflix/conductor/rest/controllers/TaskResource.java @@ -60,7 +60,7 @@ public ResponseEntity poll( // for backwards compatibility with 2.x client which expects a 204 when no Task is found return Optional.ofNullable(taskService.poll(taskType, workerId, domain)) .map(ResponseEntity::ok) - .orElseThrow(() -> new NotFoundException("Poll not found for Task Type: %s", taskType)); + .orElse(ResponseEntity.noContent().build()); } @GetMapping("/poll/batch/{tasktype}") @@ -72,11 +72,10 @@ public ResponseEntity> batchPoll( @RequestParam(value = "count", defaultValue = "1") int count, @RequestParam(value = "timeout", defaultValue = "100") int timeout) { // for backwards compatibility with 2.x client which expects a 204 when no Task is found - return Optional.ofNullable( - taskService.batchPoll(taskType, workerId, domain, count, timeout)) + return Optional.ofNullable(taskService.batchPoll(taskType, workerId, domain, count, timeout)) .filter(tasks -> !tasks.isEmpty()) .map(ResponseEntity::ok) - .orElseThrow(() -> new NotFoundException("Batch Poll not found for Task Type: %s", taskType)); + .orElse(ResponseEntity.noContent().build()); } @PostMapping(produces = TEXT_PLAIN_VALUE)