TSK-1150: added taskId as pathvariable to getTaskComment and deleteTaskComment

this is necessary since the path variable {taskId} is not used otherwise. Furthermore this will require that the search for a comment must match a given taskId.

lib/taskana-core/src/main/java/pro/taskana/task/api/TaskService.java

@@ -414,6 +414,7 @@ public interface TaskService {
   /**
    * Deletes the task comment with the given Id.
    *
+   * @param taskId The task id where the comment is supposed to be attached to
    * @param taskCommentId The id of the task comment to delete.
    * @throws NotAuthorizedException If the current user has no authorization to delete a task
    *     comment or is not authorized to access the task.
@@ -424,13 +425,14 @@ public interface TaskService {
    *     existing task.
    * @throws InvalidArgumentException If the given taskCommentId is null or empty
    */
-  void deleteTaskComment(String taskCommentId)
+  void deleteTaskComment(String taskId, String taskCommentId)
       throws NotAuthorizedException, TaskCommentNotFoundException, TaskNotFoundException,
           InvalidArgumentException;
 
   /**
    * Retrieves a task comment for a given taskCommentId.
    *
+   * @param taskId The task id where the comment is supposed to be attached to
    * @param taskCommentId The id of the task comment which should be retrieved
    * @return the task comment identified by taskCommentId
    * @throws TaskCommentNotFoundException If the given taskCommentId in the TaskComment does not
@@ -441,7 +443,7 @@ public interface TaskService {
    *     existing task.
    * @throws InvalidArgumentException If the given taskCommentId is null or empty
    */
-  TaskComment getTaskComment(String taskCommentId)
+  TaskComment getTaskComment(String taskId, String taskCommentId)
       throws TaskCommentNotFoundException, NotAuthorizedException, TaskNotFoundException,
           InvalidArgumentException;
 

lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentMapper.java

@@ -49,6 +49,7 @@ public interface TaskCommentMapper {
       "<script> SELECT ID, TASK_ID, TEXT_FIELD, CREATOR, CREATED, MODIFIED"
           + " FROM TASK_COMMENT "
           + "WHERE ID = #{taskCommentId} "
+          + "AND TASK_ID = #{taskId} "
           + "<if test=\"_databaseId == 'db2'\">with UR </if> "
           + "</script>")
   @Results(
@@ -60,5 +61,6 @@ public interface TaskCommentMapper {
         @Result(property = "created", column = "CREATED"),
         @Result(property = "modified", column = "MODIFIED"),
       })
-  TaskCommentImpl findById(@Param("taskCommentId") String taskCommentId);
+  TaskCommentImpl findById(
+      @Param("taskId") String taskId, @Param("taskCommentId") String taskCommentId);
 }

lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java

@@ -1,8 +1,8 @@
 package pro.taskana.task.internal;
 
 import java.time.Instant;
+import java.util.ArrayList;
 import java.util.List;
-import java.util.stream.Collectors;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -55,7 +55,7 @@ class TaskCommentServiceImpl {
 
   TaskComment updateTaskComment(TaskComment taskCommentToUpdate)
       throws NotAuthorizedException, ConcurrencyException, TaskCommentNotFoundException,
-                 TaskNotFoundException, InvalidArgumentException {
+          TaskNotFoundException, InvalidArgumentException {
 
     LOGGER.debug("entry to updateTaskComment (taskComment = {})", taskCommentToUpdate);
 
@@ -72,7 +72,8 @@ class TaskCommentServiceImpl {
       if (taskCommentToUpdate.getCreator().equals(userId)
           || taskanaEngine.getEngine().isUserInRole(TaskanaRole.ADMIN)) {
 
-        TaskComment oldTaskComment = getTaskComment(taskCommentImplToUpdate.getId());
+        TaskComment oldTaskComment =
+            getTaskComment(taskCommentImplToUpdate.getTaskId(), taskCommentImplToUpdate.getId());
 
         checkModifiedHasNotChanged(oldTaskComment, taskCommentImplToUpdate);
 
@@ -127,11 +128,12 @@ class TaskCommentServiceImpl {
     return taskCommentImplToCreate;
   }
 
-  void deleteTaskComment(String taskCommentId)
+  void deleteTaskComment(String taskId, String taskCommentId)
       throws NotAuthorizedException, TaskCommentNotFoundException, TaskNotFoundException,
           InvalidArgumentException {
 
-    LOGGER.debug("entry to deleteTaskComment (taskComment = {}", taskCommentId);
+    LOGGER.debug(
+        "entry to deleteTaskComment (taskId = {}, taskComment = {}", taskId, taskCommentId);
 
     String userId = CurrentUserContext.getUserid();
 
@@ -139,7 +141,7 @@ class TaskCommentServiceImpl {
 
       taskanaEngine.openConnection();
 
-      TaskComment taskCommentToDelete = getTaskComment(taskCommentId);
+      TaskComment taskCommentToDelete = getTaskComment(taskId, taskCommentId);
 
       if (taskCommentToDelete.getCreator().equals(userId)
           || taskanaEngine.getEngine().isUserInRole(TaskanaRole.ADMIN)) {
@@ -171,8 +173,7 @@ class TaskCommentServiceImpl {
 
       taskService.getTask(taskId);
 
-      List<TaskComment> taskComments =
+      List<TaskComment> taskComments = new ArrayList<>(taskCommentMapper.findByTaskId(taskId));
-          taskCommentMapper.findByTaskId(taskId).stream().collect(Collectors.toList());
 
       if (taskComments.isEmpty()) {
         LOGGER.debug("getTaskComments() found no comments for the provided taskId");
@@ -188,13 +189,13 @@ class TaskCommentServiceImpl {
     }
   }
 
-  TaskComment getTaskComment(String taskCommentId)
+  TaskComment getTaskComment(String taskId, String taskCommentId)
       throws TaskCommentNotFoundException, NotAuthorizedException, TaskNotFoundException,
           InvalidArgumentException {
 
-    LOGGER.debug("entry to getTaskComment (taskCommentId = {})", taskCommentId);
+    LOGGER.debug("entry to getTaskComment (taskId= {}, taskCommentId = {})", taskId, taskCommentId);
 
-    TaskCommentImpl result = null;
+    TaskCommentImpl result;
 
     verifyTaskCommentIdIsNotNullOrEmpty(taskCommentId);
 
@@ -202,11 +203,14 @@ class TaskCommentServiceImpl {
 
       taskanaEngine.openConnection();
 
-      result = taskCommentMapper.findById(taskCommentId);
+      result = taskCommentMapper.findById(taskId, taskCommentId);
 
       if (result == null) {
         throw new TaskCommentNotFoundException(
-            taskCommentId, "TaskComment for id " + taskCommentId + " was not found");
+            taskCommentId,
+            String.format(
+                "TaskComment for taskIid '%s' and taskCommentId '%s' was not found",
+                taskId, taskCommentId));
       }
 
       taskService.getTask(result.getTaskId());

lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskServiceImpl.java

@@ -614,17 +614,17 @@ public class TaskServiceImpl implements TaskService {
   }
 
   @Override
-  public void deleteTaskComment(String taskCommentId)
+  public void deleteTaskComment(String taskId, String taskCommentId)
       throws NotAuthorizedException, TaskCommentNotFoundException, TaskNotFoundException,
           InvalidArgumentException {
-    taskCommentService.deleteTaskComment(taskCommentId);
+    taskCommentService.deleteTaskComment(taskId, taskCommentId);
   }
 
   @Override
-  public TaskComment getTaskComment(String taskCommentid)
+  public TaskComment getTaskComment(String taskId, String taskCommentid)
       throws TaskCommentNotFoundException, NotAuthorizedException, TaskNotFoundException,
           InvalidArgumentException {
-    return taskCommentService.getTaskComment(taskCommentid);
+    return taskCommentService.getTaskComment(taskId, taskCommentid);
   }
 
   @Override
@@ -698,9 +698,7 @@ public class TaskServiceImpl implements TaskService {
         return bulkLog;
       } else {
         final int numberOfAffectedTasks = taskMapper.setOwnerOfTasks(owner, taskIds, Instant.now());
-        if (numberOfAffectedTasks == taskIds.size()) { // all tasks were updated
+        if (numberOfAffectedTasks != taskIds.size()) { // all tasks were updated
-          return bulkLog;
-        } else {
           // check the outcome
           existingMinimalTaskSummaries = taskMapper.findExistingTasks(taskIds, null);
           bulkLog.addAllErrors(
@@ -713,8 +711,8 @@ public class TaskServiceImpl implements TaskService {
                 numberOfAffectedTasks,
                 bulkLog.getFailedIds().size());
           }
-          return bulkLog;
         }
+        return bulkLog;
       }
     } finally {
       LOGGER.debug("exit from setOwnerOfTasks()");

lib/taskana-core/src/test/java/acceptance/task/DeleteTaskCommentAccTest.java

@@ -39,7 +39,8 @@ public class DeleteTaskCommentAccTest extends AbstractAccTest {
         taskService.getTaskComments("TKI:000000000000000000000000000000000001");
     assertThat(taskComments).hasSize(2);
 
-    taskService.deleteTaskComment("TCI:000000000000000000000000000000000004");
+    taskService.deleteTaskComment(
+        "TKI:000000000000000000000000000000000001", "TCI:000000000000000000000000000000000004");
 
     // make sure the task comment was deleted
     List<TaskComment> taskCommentsAfterDeletion =
@@ -60,11 +61,12 @@ public class DeleteTaskCommentAccTest extends AbstractAccTest {
         taskService.getTaskComments("TKI:000000000000000000000000000000000002");
     assertThat(taskComments).hasSize(2);
 
-    ThrowingCallable httpCall =
+    ThrowingCallable lambda =
-        () -> {
+        () ->
-          taskService.deleteTaskComment("TCI:000000000000000000000000000000000005");
+            taskService.deleteTaskComment(
-        };
+                "TKI:000000000000000000000000000000000001",
-    assertThatThrownBy(httpCall).isInstanceOf(NotAuthorizedException.class);
+                "TCI:000000000000000000000000000000000005");
+    assertThatThrownBy(lambda).isInstanceOf(NotAuthorizedException.class);
 
     // make sure the task comment was not deleted
     List<TaskComment> taskCommentsAfterDeletion =
@@ -85,10 +87,16 @@ public class DeleteTaskCommentAccTest extends AbstractAccTest {
         taskService.getTaskComments("TKI:000000000000000000000000000000000002");
     assertThat(taskComments).hasSize(2);
 
-    assertThatThrownBy(() -> taskService.deleteTaskComment(""))
+    assertThatThrownBy(() -> taskService.deleteTaskComment("", ""))
         .isInstanceOf(InvalidArgumentException.class);
 
-    assertThatThrownBy(() -> taskService.deleteTaskComment(null))
+    assertThatThrownBy(() -> taskService.deleteTaskComment("", null))
+        .isInstanceOf(InvalidArgumentException.class);
+
+    assertThatThrownBy(() -> taskService.deleteTaskComment(null, ""))
+        .isInstanceOf(InvalidArgumentException.class);
+
+    assertThatThrownBy(() -> taskService.deleteTaskComment(null, null))
         .isInstanceOf(InvalidArgumentException.class);
 
     // make sure that no task comment was deleted
@@ -110,8 +118,9 @@ public class DeleteTaskCommentAccTest extends AbstractAccTest {
         taskService.getTaskComments("TKI:000000000000000000000000000000000002");
     assertThat(taskComments).hasSize(2);
 
-    assertThatThrownBy(() -> taskService.deleteTaskComment("non existing task comment id"))
+    ThrowingCallable lambda =
-        .isInstanceOf(TaskCommentNotFoundException.class);
+        () -> taskService.deleteTaskComment("invalidTaskId", "non existing task comment id");
+    assertThatThrownBy(lambda).isInstanceOf(TaskCommentNotFoundException.class);
 
     // make sure the task comment was not deleted
     List<TaskComment> taskCommentsAfterDeletion =

lib/taskana-core/src/test/java/acceptance/task/GetTaskCommentAccTest.java

@@ -77,7 +77,8 @@ public class GetTaskCommentAccTest extends AbstractAccTest {
     TaskService taskService = taskanaEngine.getTaskService();
 
     TaskComment taskComment =
-        taskService.getTaskComment("TCI:000000000000000000000000000000000007");
+        taskService.getTaskComment(
+            "TKI:000000000000000000000000000000000002", "TCI:000000000000000000000000000000000007");
     assertThat(taskComment.getCreator()).isEqualTo("user_1_1");
   }
 
@@ -89,8 +90,11 @@ public class GetTaskCommentAccTest extends AbstractAccTest {
 
     TaskService taskService = taskanaEngine.getTaskService();
 
-    assertThatThrownBy(() -> taskService.getTaskComment("Definately Non Existing Task Comment Id"))
+    ThrowingCallable lambda =
-        .isInstanceOf(TaskCommentNotFoundException.class);
+        () ->
+            taskService.getTaskComment(
+                "Invalid Task Id", "Definately Non Existing Task Comment Id");
+    assertThatThrownBy(lambda).isInstanceOf(TaskCommentNotFoundException.class);
   }
 
   @WithAccessId(
@@ -101,7 +105,11 @@ public class GetTaskCommentAccTest extends AbstractAccTest {
 
     TaskService taskService = taskanaEngine.getTaskService();
 
-    assertThatThrownBy(() -> taskService.getTaskComment("TCI:000000000000000000000000000000000012"))
+    ThrowingCallable lambda =
-        .isInstanceOf(NotAuthorizedException.class);
+        () ->
+            taskService.getTaskComment(
+                "TKI:000000000000000000000000000000000004",
+                "TCI:000000000000000000000000000000000012");
+    assertThatThrownBy(lambda).isInstanceOf(NotAuthorizedException.class);
   }
 }

lib/taskana-core/src/test/java/acceptance/task/UpdateTaskCommentAccTest.java

@@ -41,7 +41,8 @@ public class UpdateTaskCommentAccTest extends AbstractAccTest {
     assertThat(taskComments.get(0).getTextField()).isEqualTo("some text in textfield");
 
     TaskComment taskComment =
-        taskService.getTaskComment("TCI:000000000000000000000000000000000003");
+        taskService.getTaskComment(
+            "TKI:000000000000000000000000000000000025", "TCI:000000000000000000000000000000000003");
     taskComment.setTextField("updated textfield");
 
     taskService.updateTaskComment(taskComment);
@@ -67,7 +68,8 @@ public class UpdateTaskCommentAccTest extends AbstractAccTest {
     assertThat(taskComments.get(1).getTextField()).isEqualTo("some other text in textfield");
 
     TaskComment taskComment =
-        taskService.getTaskComment("TCI:000000000000000000000000000000000001");
+        taskService.getTaskComment(
+            "TKI:000000000000000000000000000000000000", "TCI:000000000000000000000000000000000001");
     taskComment.setTextField("updated textfield");
 
     assertThatThrownBy(() -> taskService.updateTaskComment(taskComment))
@@ -96,11 +98,13 @@ public class UpdateTaskCommentAccTest extends AbstractAccTest {
     assertThat(taskComments.get(2).getTextField()).isEqualTo("some other text in textfield");
 
     TaskComment taskCommentToUpdate =
-        taskService.getTaskComment("TCI:000000000000000000000000000000000002");
+        taskService.getTaskComment(
+            "TKI:000000000000000000000000000000000000", "TCI:000000000000000000000000000000000002");
     taskCommentToUpdate.setTextField("updated textfield");
 
     TaskComment concurrentTaskCommentToUpdate =
-        taskService.getTaskComment("TCI:000000000000000000000000000000000002");
+        taskService.getTaskComment(
+            "TKI:000000000000000000000000000000000000", "TCI:000000000000000000000000000000000002");
     concurrentTaskCommentToUpdate.setTextField("concurrently updated textfield");
 
     taskService.updateTaskComment(taskCommentToUpdate);

rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskCommentController.java

@@ -43,17 +43,17 @@ public class TaskCommentController {
     this.taskCommentResourceAssembler = taskCommentResourceAssembler;
   }
 
-
   @GetMapping(path = Mapping.URL_TASK_COMMENT)
   @Transactional(readOnly = true, rollbackFor = Exception.class)
-  public ResponseEntity<TaskCommentResource> getTaskComment(@PathVariable String taskCommentId)
+  public ResponseEntity<TaskCommentResource> getTaskComment(
+      @PathVariable String taskId, @PathVariable String taskCommentId)
       throws NotAuthorizedException, TaskNotFoundException, TaskCommentNotFoundException,
           InvalidArgumentException {
     if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug("Entry to getTaskComment(taskCommentId= {})", taskCommentId);
+      LOGGER.debug("Entry to getTaskComment(taskId= {}, taskCommentId= {})", taskId, taskCommentId);
     }
 
-    TaskComment taskComment = taskService.getTaskComment(taskCommentId);
+    TaskComment taskComment = taskService.getTaskComment(taskId, taskCommentId);
 
     TaskCommentResource taskCommentResource = taskCommentResourceAssembler.toResource(taskComment);
 
@@ -90,14 +90,16 @@ public class TaskCommentController {
 
   @DeleteMapping(path = Mapping.URL_TASK_COMMENT)
   @Transactional(readOnly = true, rollbackFor = Exception.class)
-  public ResponseEntity<TaskCommentResource> deleteTaskComment(@PathVariable String taskCommentId)
+  public ResponseEntity<TaskCommentResource> deleteTaskComment(
+      @PathVariable String taskId, @PathVariable String taskCommentId)
       throws NotAuthorizedException, TaskNotFoundException, TaskCommentNotFoundException,
           InvalidArgumentException {
     if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug("Entry to deleteTaskComment(taskCommentId= {})", taskCommentId);
+      LOGGER.debug(
+          "Entry to deleteTaskComment(taskId= {}, taskCommentId= {})", taskId, taskCommentId);
     }
 
-    taskService.deleteTaskComment(taskCommentId);
+    taskService.deleteTaskComment(taskId, taskCommentId);
 
     ResponseEntity<TaskCommentResource> result = ResponseEntity.noContent().build();
 

rest/taskana-rest-spring/src/main/java/pro/taskana/rest/resource/TaskCommentResourceAssembler.java

@@ -41,7 +41,9 @@ public class TaskCommentResourceAssembler
     TaskCommentResource taskCommentResource = new TaskCommentResource(taskComment);
     try {
       taskCommentResource.add(
-          linkTo(methodOn(TaskCommentController.class).getTaskComment(taskComment.getId()))
+          linkTo(
+                  methodOn(TaskCommentController.class)
+                      .getTaskComment(taskComment.getTaskId(), taskComment.getId()))
               .withSelfRel());
     } catch (TaskCommentNotFoundException
         | TaskNotFoundException
@@ -54,8 +56,7 @@ public class TaskCommentResourceAssembler
   }
 
   @PageLinks(Mapping.URL_TASK_COMMENTS)
-  public TaskCommentListResource toListResource(
+  public TaskCommentListResource toListResource(List<TaskComment> taskComments) {
-      List<TaskComment> taskComments) {
     return new TaskCommentListResource(toResources(taskComments));
   }
 

rest/taskana-rest-spring/src/test/java/pro/taskana/doc/api/TaskCommentControllerRestDocumentation.java

@@ -24,7 +24,7 @@ import pro.taskana.rest.Mapping;
 
 public class TaskCommentControllerRestDocumentation extends BaseRestDocumentation {
 
-  private HashMap<String, String> taskCommentFieldDescriptionsMap = new HashMap<String, String>();
+  private HashMap<String, String> taskCommentFieldDescriptionsMap = new HashMap<>();
 
   private FieldDescriptor[] allTaskCommentsFieldDescriptors;
   private FieldDescriptor[] taskCommentFieldDescriptors;
@@ -58,8 +58,7 @@ public class TaskCommentControllerRestDocumentation extends BaseRestDocumentatio
               .type("String"),
           fieldWithPath("_links").ignored(),
           fieldWithPath("_links.self").ignored(),
-          fieldWithPath("_links.self.href").ignored(),
+          fieldWithPath("_links.self.href").ignored()
-          fieldWithPath("_links.self.templated").ignored()
         };
 
     createTaskCommentFieldDescriptors =
@@ -110,7 +109,7 @@ public class TaskCommentControllerRestDocumentation extends BaseRestDocumentatio
             RestDocumentationRequestBuilders.get(
                     restHelper.toUrl(
                         Mapping.URL_TASK_COMMENT,
-                        "TKI:100000000000000000000000000000000000",
+                        "TKI:000000000000000000000000000000000000",
                         "TCI:000000000000000000000000000000000000"))
                 .accept("application/hal+json")
                 .header("Authorization", "Basic YWRtaW46YWRtaW4="))