chore(engine): Allow Null Values to TaskService Operations

* chore(engine): Allow Null Values to TaskService Operations - refactor(engine): AbstractSetTaskPropertyCmd Does Only Value Validation - Methods: * Description * DueDate * FollowUpDate Related to: camunda#3491, camunda#3154
vivekchand · Jun 15, 2023 · c9497b1 · c9497b1
1 parent b9b9f3f
commit c9497b1
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 42 deletions.
diff --git a/engine/src/main/java/org/camunda/bpm/engine/TaskService.java b/engine/src/main/java/org/camunda/bpm/engine/TaskService.java
@@ -507,7 +507,7 @@ public interface TaskService {
    *
    * @param taskId id of the task, not null
    * @param name   the new task name, not null
-   * @throws NullValueException     in case the given arguments are null.
+   * @throws NullValueException     in case the taskId is null.
    * @throws NotFoundException      when the task doesn't exist.
    * @throws AuthorizationException If the user hasn't any of {@link Permissions#UPDATE}, {@link Permissions#TASK_ASSIGN} permissions on {@link Resources#TASK}
    *                                or no {@link Permissions#UPDATE_TASK}, {@link Permissions#TASK_ASSIGN} permissions on {@link Resources#PROCESS_DEFINITION}
@@ -520,7 +520,7 @@ public interface TaskService {
    *
    * @param taskId      id of the task, not null
    * @param description the new task description, not null
-   * @throws NullValueException     in case the given arguments are null.
+   * @throws NullValueException     in case the taskId is null.
    * @throws NotFoundException      when the task doesn't exist.
    * @throws AuthorizationException If the user hasn't any of {@link Permissions#UPDATE}, {@link Permissions#TASK_ASSIGN} permissions on {@link Resources#TASK}
    *                                or no {@link Permissions#UPDATE_TASK}, {@link Permissions#TASK_ASSIGN} permissions on {@link Resources#PROCESS_DEFINITION}
@@ -533,7 +533,7 @@ public interface TaskService {
    *
    * @param taskId  id of the task, not null
    * @param dueDate the new task dueDate, not null
-   * @throws NullValueException     in case the given arguments are null.
+   * @throws NullValueException     in case the taskId is null.
    * @throws NotFoundException      when the task doesn't exist.
    * @throws AuthorizationException If the user hasn't any of {@link Permissions#UPDATE}, {@link Permissions#TASK_ASSIGN} permissions on {@link Resources#TASK}
    *                                or no {@link Permissions#UPDATE_TASK}, {@link Permissions#TASK_ASSIGN} permissions on {@link Resources#PROCESS_DEFINITION}
@@ -546,7 +546,7 @@ public interface TaskService {
    *
    * @param taskId       id of the task, not null
    * @param followUpDate the new task followUpDate, not null
-   * @throws NullValueException     in case the given arguments are null.
+   * @throws NullValueException     in case the taskId is null.
    * @throws NotFoundException      when the task doesn't exist.
    * @throws AuthorizationException If the user hasn't any of {@link Permissions#UPDATE}, {@link Permissions#TASK_ASSIGN} permissions on {@link Resources#TASK}
    *                                or no {@link Permissions#UPDATE_TASK}, {@link Permissions#TASK_ASSIGN} permissions on {@link Resources#PROCESS_DEFINITION}

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/AbstractSetTaskPropertyCmd.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/AbstractSetTaskPropertyCmd.java
@@ -57,18 +57,11 @@ public AbstractSetTaskPropertyCmd(String taskId, T value) {
    *
    * @param taskId                 the id of the task whose property should be changed
    * @param value                  the new value to set to the referenced task
-   * @param excludeInputValidation if true, the validation of the input will be excluded
+   * @param skipValueValidation if true, the validation of the value will be excluded
    */
-  protected AbstractSetTaskPropertyCmd(String taskId, T value, boolean excludeInputValidation) {
-
-    if (excludeInputValidation) {
-      this.taskId = taskId;
-      this.value = value;
-      return;
-    }
-
+  protected AbstractSetTaskPropertyCmd(String taskId, T value, boolean skipValueValidation) {
     this.taskId = ensureNotNullAndGet("taskId", taskId);
-    this.value = ensureNotNullAndGet("value", value);
+    this.value = skipValueValidation ? value : ensureNotNullAndGet("value", value);
   }
 
   /**

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/SetTaskDescriptionCmd.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/SetTaskDescriptionCmd.java
@@ -34,7 +34,7 @@ public class SetTaskDescriptionCmd extends AbstractSetTaskPropertyCmd<String> {
    * @throws NullValueException in case the given taskId or the given description are null
    */
   public SetTaskDescriptionCmd(String taskId, String description) {
-    super(taskId, description);
+    super(taskId, description, true);
   }
 
   @Override

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/SetTaskDueDateCmd.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/SetTaskDueDateCmd.java
@@ -35,7 +35,7 @@ public class SetTaskDueDateCmd extends AbstractSetTaskPropertyCmd<Date> {
    * @throws NullValueException in case the given taskId or the given dueDate value are null
    */
   public SetTaskDueDateCmd(String taskId, Date value) {
-    super(taskId, value);
+    super(taskId, value, true);
   }
 
   @Override

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/SetTaskFollowUpDateCmd.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/SetTaskFollowUpDateCmd.java
@@ -35,7 +35,7 @@ public class SetTaskFollowUpDateCmd extends AbstractSetTaskPropertyCmd<Date> {
    * @throws NullValueException in case the given taskId or the given followUpDate value are null
    */
   public SetTaskFollowUpDateCmd(String taskId, Date value) {
-    super(taskId, value);
+    super(taskId, value, true);
   }
 
   @Override

diff --git a/engine/src/test/java/org/camunda/bpm/engine/test/api/task/TaskServiceTest.java b/engine/src/test/java/org/camunda/bpm/engine/test/api/task/TaskServiceTest.java
@@ -1162,7 +1162,7 @@ public void testSetAssigneeNullTaskId() {
       taskService.setAssignee(null, "userId");
       fail("ProcessEngineException expected");
     } catch (NullValueException ae) {
-      testRule.assertTextPresent("Invalid task id: id is null", ae.getMessage());
+      testRule.assertTextPresent("taskId is null", ae.getMessage());
     }
   }
 
@@ -1187,7 +1187,7 @@ public void testSetOwnerNullTaskId() {
       taskService.setOwner(null, "userId");
       fail("ProcessEngineException expected");
     } catch (NullValueException ae) {
-      testRule.assertTextPresent("Invalid task id: id is null", ae.getMessage());
+      testRule.assertTextPresent("taskId is null", ae.getMessage());
     }
   }
 
@@ -1245,7 +1245,7 @@ public void testAddCandidateUserNullTaskId() {
       taskService.addCandidateUser(null, "userId");
       fail("ProcessEngineException expected");
     } catch (NullValueException ae) {
-      testRule.assertTextPresent("Invalid task id: id is null", ae.getMessage());
+      testRule.assertTextPresent("taskId is null", ae.getMessage());
     }
   }
 
@@ -1280,7 +1280,7 @@ public void testAddCandidateGroupNullTaskId() {
       taskService.addCandidateGroup(null, "groupId");
       fail("ProcessEngineException expected");
     } catch (NullValueException ae) {
-      testRule.assertTextPresent("Invalid task id: id is null", ae.getMessage());
+      testRule.assertTextPresent("taskId is null", ae.getMessage());
     }
   }
 
@@ -1313,7 +1313,7 @@ public void testAddGroupIdentityLinkNullTaskId() {
       taskService.addGroupIdentityLink(null, "groupId", IdentityLinkType.CANDIDATE);
       fail("ProcessEngineException expected");
     } catch (NullValueException ae) {
-      testRule.assertTextPresent("Invalid task id: id is null", ae.getMessage());
+      testRule.assertTextPresent("taskId is null", ae.getMessage());
     }
   }
 
@@ -1348,7 +1348,7 @@ public void testAddUserIdentityLinkNullTaskId() {
       taskService.addUserIdentityLink(null, "userId", IdentityLinkType.CANDIDATE);
       fail("ProcessEngineException expected");
     } catch (NullValueException ae) {
-      testRule.assertTextPresent("Invalid task id: id is null", ae.getMessage());
+      testRule.assertTextPresent("taskId is null", ae.getMessage());
     }
   }
 
@@ -1540,7 +1540,7 @@ public void testSetPriorityNullTaskId() {
       taskService.setPriority(null, 12345);
       fail("ProcessEngineException expected");
     } catch (NullValueException ae) {
-      testRule.assertTextPresent("Invalid task id: id is null", ae.getMessage());
+      testRule.assertTextPresent("taskId is null", ae.getMessage());
     }
   }
 
@@ -1601,15 +1601,17 @@ public void testSetDescriptionNullTaskId() {
 
   @Test
   public void testSetDescriptionNullDescription() {
+    // given
     Task task = taskService.newTask();
     taskService.saveTask(task);
 
-    try {
-      taskService.setDescription(task.getId(), null);
-      fail("ProcessEngineException expected");
-    } catch (NullValueException ae) {
-      testRule.assertTextPresent("value is null", ae.getMessage());
-    }
+    //when
+    taskService.setDescription(task.getId(), null);
+
+    // then
+    task = taskService.createTaskQuery().singleResult();
+
+    assertThat(task.getDescription()).isNull();
 
     taskService.deleteTask(task.getId(), true);
   }
@@ -1636,15 +1638,17 @@ public void testSetDueDateNullTaskId() {
 
   @Test
   public void testSetDueDateNullDueDate() {
+    // given
     Task task = taskService.newTask();
     taskService.saveTask(task);
 
-    try {
-      taskService.setDueDate(task.getId(), null);
-      fail("ProcessEngineException expected");
-    } catch (NullValueException ae) {
-      testRule.assertTextPresent("value is null", ae.getMessage());
-    }
+    // when
+    taskService.setDueDate(task.getId(), null);
+
+    // then
+    task = taskService.createTaskQuery().singleResult();
+
+    assertThat(task.getDescription()).isNull();
 
     taskService.deleteTask(task.getId(), true);
   }
@@ -1671,15 +1675,17 @@ public void testSetFollowUpDateNullTaskId() {
 
   @Test
   public void testSetFollowUpDateNullFollowUpDate() {
+    // given
     Task task = taskService.newTask();
     taskService.saveTask(task);
 
-    try {
-      taskService.setFollowUpDate(task.getId(), null);
-      fail("ProcessEngineException expected");
-    } catch (NullValueException ae) {
-      testRule.assertTextPresent("value is null", ae.getMessage());
-    }
+    // when
+    taskService.setFollowUpDate(task.getId(), null);
+
+    // then
+    task = taskService.createTaskQuery().singleResult();
+
+    assertThat(task.getDescription()).isNull();
 
     taskService.deleteTask(task.getId(), true);
   }