From 93a94f0458979e35cb92ee682be312532d1e5210 Mon Sep 17 00:00:00 2001 From: Mustapha Zorgati <15628173+mustaphazorgati@users.noreply.github.com> Date: Wed, 18 Mar 2020 20:52:19 +0100 Subject: [PATCH] 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. --- .../pro/taskana/task/api/TaskService.java | 6 ++-- .../task/internal/TaskCommentMapper.java | 4 ++- .../task/internal/TaskCommentServiceImpl.java | 30 +++++++++++-------- .../task/internal/TaskServiceImpl.java | 14 ++++----- .../task/DeleteTaskCommentAccTest.java | 29 +++++++++++------- .../task/GetTaskCommentAccTest.java | 18 +++++++---- .../task/UpdateTaskCommentAccTest.java | 12 +++++--- .../taskana/rest/TaskCommentController.java | 16 +++++----- .../TaskCommentResourceAssembler.java | 7 +++-- ...askCommentControllerRestDocumentation.java | 7 ++--- 10 files changed, 86 insertions(+), 57 deletions(-) diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskService.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskService.java index a07c9a1cc..73ae3be10 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskService.java +++ b/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; diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentMapper.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentMapper.java index ca39f0e8b..0a35d13e7 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentMapper.java @@ -49,6 +49,7 @@ public interface TaskCommentMapper { "") @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); } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java index 0c05e0a52..cb70f813f 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java +++ b/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 taskComments = - taskCommentMapper.findByTaskId(taskId).stream().collect(Collectors.toList()); + List taskComments = new ArrayList<>(taskCommentMapper.findByTaskId(taskId)); 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()); diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskServiceImpl.java index fba7860b2..a1193b627 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskServiceImpl.java +++ b/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 - return bulkLog; - } else { + if (numberOfAffectedTasks != taskIds.size()) { // all tasks were updated // 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()"); diff --git a/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskCommentAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskCommentAccTest.java index d2678cc42..70466585b 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskCommentAccTest.java +++ b/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 taskCommentsAfterDeletion = @@ -60,11 +61,12 @@ public class DeleteTaskCommentAccTest extends AbstractAccTest { taskService.getTaskComments("TKI:000000000000000000000000000000000002"); assertThat(taskComments).hasSize(2); - ThrowingCallable httpCall = - () -> { - taskService.deleteTaskComment("TCI:000000000000000000000000000000000005"); - }; - assertThatThrownBy(httpCall).isInstanceOf(NotAuthorizedException.class); + ThrowingCallable lambda = + () -> + taskService.deleteTaskComment( + "TKI:000000000000000000000000000000000001", + "TCI:000000000000000000000000000000000005"); + assertThatThrownBy(lambda).isInstanceOf(NotAuthorizedException.class); // make sure the task comment was not deleted List 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")) - .isInstanceOf(TaskCommentNotFoundException.class); + ThrowingCallable lambda = + () -> taskService.deleteTaskComment("invalidTaskId", "non existing task comment id"); + assertThatThrownBy(lambda).isInstanceOf(TaskCommentNotFoundException.class); // make sure the task comment was not deleted List taskCommentsAfterDeletion = diff --git a/lib/taskana-core/src/test/java/acceptance/task/GetTaskCommentAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/GetTaskCommentAccTest.java index 6b45e6ba1..23a9aba5d 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/GetTaskCommentAccTest.java +++ b/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")) - .isInstanceOf(TaskCommentNotFoundException.class); + ThrowingCallable lambda = + () -> + 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")) - .isInstanceOf(NotAuthorizedException.class); + ThrowingCallable lambda = + () -> + taskService.getTaskComment( + "TKI:000000000000000000000000000000000004", + "TCI:000000000000000000000000000000000012"); + assertThatThrownBy(lambda).isInstanceOf(NotAuthorizedException.class); } } diff --git a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskCommentAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskCommentAccTest.java index f5c5df792..984d816d1 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskCommentAccTest.java +++ b/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); diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskCommentController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskCommentController.java index 79dab09de..b8268fd1b 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskCommentController.java +++ b/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 getTaskComment(@PathVariable String taskCommentId) + public ResponseEntity 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 deleteTaskComment(@PathVariable String taskCommentId) + public ResponseEntity 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 result = ResponseEntity.noContent().build(); diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/resource/TaskCommentResourceAssembler.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/resource/TaskCommentResourceAssembler.java index 9891a3016..a414f3f6c 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/resource/TaskCommentResourceAssembler.java +++ b/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( - List taskComments) { + public TaskCommentListResource toListResource(List taskComments) { return new TaskCommentListResource(toResources(taskComments)); } diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/doc/api/TaskCommentControllerRestDocumentation.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/doc/api/TaskCommentControllerRestDocumentation.java index f1668a24d..4d8d05239 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/doc/api/TaskCommentControllerRestDocumentation.java +++ b/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 taskCommentFieldDescriptionsMap = new HashMap(); + private HashMap 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.templated").ignored() + fieldWithPath("_links.self.href").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="))