revert changes for workerId required in /tasks/ack API (#980)

* revert changes for workerId required in /tasks/ack API * Made validation error logging more verbose
Netflix · Feb 6, 2019 · 7eecae3 · 7eecae3
1 parent 5171593
commit 7eecae3
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 20 deletions.
diff --git a/client/src/main/java/com/netflix/conductor/client/exceptions/ConductorClientException.java b/client/src/main/java/com/netflix/conductor/client/exceptions/ConductorClientException.java
@@ -77,6 +77,10 @@ public String toString() {
             builder.append(", instance: ").append(instance);
         }
 
+        if (this.validationErrors != null) {
+            builder.append(", validationErrors: ").append(validationErrors.toString());
+        }
+
         builder.append("}");
         return builder.toString();
     }

diff --git a/client/src/main/java/com/netflix/conductor/client/exceptions/ErrorResponse.java b/client/src/main/java/com/netflix/conductor/client/exceptions/ErrorResponse.java
@@ -2,6 +2,7 @@
 
 import java.util.List;
 import com.netflix.conductor.common.validation.ValidationError;
+import java.util.StringJoiner;
 
 
 //TODO: Use one from common
@@ -53,4 +54,15 @@ public String getInstance() {
     public void setInstance(String instance) {
         this.instance = instance;
     }
+
+    @Override
+    public String toString() {
+        return new StringJoiner(", ", ErrorResponse.class.getSimpleName() + "[", "]")
+            .add("code='" + code + "'")
+            .add("message='" + message + "'")
+            .add("instance='" + instance + "'")
+            .add("retryable=" + retryable)
+            .add("validationErrors=" + validationErrors)
+            .toString();
+    }
 }
diff --git a/client/src/main/java/com/netflix/conductor/client/http/ClientBase.java b/client/src/main/java/com/netflix/conductor/client/http/ClientBase.java
@@ -255,7 +255,7 @@ private void handleUniformInterfaceException(UniformInterfaceException exception
                 return;
             }
             String errorMessage = clientResponse.getEntity(String.class);
-            logger.error("Unable to invoke Conductor API with uri: {}, unexpected response from server: {}", uri, clientResponseToString(exception.getResponse()), exception);
+            logger.error("Unable to invoke Conductor API with uri: {}, unexpected response from server: statusCode={}, responseBody='{}'.", uri, clientResponse.getStatus(), errorMessage);
             ErrorResponse errorResponse;
             try {
                 errorResponse = objectMapper.readValue(errorMessage, ErrorResponse.class);

diff --git a/common/src/main/java/com/netflix/conductor/common/validation/ValidationError.java b/common/src/main/java/com/netflix/conductor/common/validation/ValidationError.java
@@ -1,6 +1,7 @@
 package com.netflix.conductor.common.validation;
 
 import com.netflix.conductor.common.validation.ErrorResponse;
+import java.util.StringJoiner;
 
 /**
  * Captures a validation error that can be returned in {@link ErrorResponse}.
@@ -42,5 +43,14 @@ public void setMessage(String message) {
     public void setInvalidValue(String invalidValue) {
         this.invalidValue = invalidValue;
     }
+
+    @Override
+    public String toString() {
+        return new StringJoiner(", ", ValidationError.class.getSimpleName() + "[", "]")
+            .add("path='" + path + "'")
+            .add("message='" + message + "'")
+            .add("invalidValue='" + invalidValue + "'")
+            .toString();
+    }
 }
 
diff --git a/core/src/main/java/com/netflix/conductor/service/AdminService.java b/core/src/main/java/com/netflix/conductor/service/AdminService.java
@@ -47,5 +47,5 @@ public interface AdminService {
      * @return list of pending {@link Task}
      */
     List<Task> getListOfPendingTask(@NotEmpty(message = "TaskType cannot be null or empty.") String taskType,
-                                    @NotNull Integer start, @NotNull Integer count);
+                                    Integer start, Integer count);
 }
diff --git a/core/src/main/java/com/netflix/conductor/service/TaskService.java b/core/src/main/java/com/netflix/conductor/service/TaskService.java
@@ -87,8 +87,7 @@ Task getPendingTaskForWorkflow(@NotEmpty(message = "WorkflowId cannot be null or
      * @param workerId Id of the worker
      * @return `true|false` if task if received or not
      */
-    String ackTaskReceived(@NotEmpty(message = "TaskId cannot be null or empty.") String taskId,
-                           @NotEmpty(message = "WorkerID cannot be null or empty.") String workerId);
+    String ackTaskReceived(@NotEmpty(message = "TaskId cannot be null or empty.") String taskId, String workerId);
 
     /**
      * Ack Task is received.
@@ -144,7 +143,7 @@ void removeTaskFromQueue(@NotEmpty(message = "TaskType cannot be null or empty."
      * @param taskTypes List of task types.
      * @return map of task type as Key and queue size as value.
      */
-    Map<String, Integer> getTaskQueueSizes(@NotEmpty(message = "List of taskType cannot be null or empty") List<@NotEmpty String> taskTypes);
+    Map<String, Integer> getTaskQueueSizes(List<String> taskTypes);
 
     /**
      * Get the details about each queue.

diff --git a/core/src/test/java/com/netflix/conductor/service/TaskServiceTest.java b/core/src/test/java/com/netflix/conductor/service/TaskServiceTest.java
@@ -21,6 +21,7 @@
 
 import static com.netflix.conductor.utility.TestUtils.getConstraintViolationMessages;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 public class TaskServiceTest {
@@ -126,18 +127,23 @@ public void testUpdateTaskInValid() {
         }
     }
 
+
     @Test(expected = ConstraintViolationException.class)
     public void testAckTaskReceived() {
         try{
             taskService.ackTaskReceived(null, null);
         } catch (ConstraintViolationException ex){
-            assertEquals(2, ex.getConstraintViolations().size());
+            assertEquals(1, ex.getConstraintViolations().size());
             Set<String> messages = getConstraintViolationMessages(ex.getConstraintViolations());
             assertTrue(messages.contains("TaskId cannot be null or empty."));
-            assertTrue(messages.contains("WorkerID cannot be null or empty."));
             throw ex;
         }
     }
+    @Test
+    public void testAckTaskReceivedMissingWorkerId() {
+        String ack = taskService.ackTaskReceived("abc", null);
+        assertNotNull(ack);
+    }
 
     @Test(expected = ConstraintViolationException.class)
     public void testLog(){
@@ -188,19 +194,6 @@ public void testRemoveTaskFromQueue(){
         }
     }
 
-    @Test(expected = ConstraintViolationException.class)
-    public void testGetTaskQueueSizes(){
-        try{
-            List<String> list = new ArrayList<>();
-            taskService.getTaskQueueSizes(list);
-        } catch (ConstraintViolationException ex){
-            assertEquals(1, ex.getConstraintViolations().size());
-            Set<String> messages = getConstraintViolationMessages(ex.getConstraintViolations());
-            assertTrue(messages.contains("List of taskType cannot be null or empty"));
-            throw ex;
-        }
-    }
-
     @Test(expected = ConstraintViolationException.class)
     public void testGetPollData(){
         try{