diff --git a/lib/taskana-core/src/main/java/pro/taskana/common/internal/util/DaysToWorkingDaysConverter.java b/lib/taskana-core/src/main/java/pro/taskana/common/internal/util/DaysToWorkingDaysConverter.java index f0fd6010d..b726d9b61 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/common/internal/util/DaysToWorkingDaysConverter.java +++ b/lib/taskana-core/src/main/java/pro/taskana/common/internal/util/DaysToWorkingDaysConverter.java @@ -3,6 +3,7 @@ package pro.taskana.common.internal.util; import static java.time.temporal.ChronoUnit.DAYS; import java.time.DayOfWeek; +import java.time.Duration; import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; @@ -82,6 +83,16 @@ public final class DaysToWorkingDaysConverter { .orElse(0); } + public Instant addWorkingDaysToInstant(Instant instant, Duration workingDays) { + long days = convertWorkingDaysToDays(instant, workingDays.toDays()); + return instant.plus(Duration.ofDays(days)); + } + + public Instant subtractWorkingDaysFromInstant(Instant instant, Duration workingDays) { + long days = convertWorkingDaysToDays(instant, -workingDays.toDays()); + return instant.plus(Duration.ofDays(days)); + } + public boolean isWorkingDay(long day, Instant referenceDate) { LocalDateTime dateToCheck = LocalDateTime.ofInstant(referenceDate, ZoneId.systemDefault()).plusDays(day); diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/AttachmentHandler.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/AttachmentHandler.java index 6a995f7bc..d438501f8 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/AttachmentHandler.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/AttachmentHandler.java @@ -79,7 +79,7 @@ public class AttachmentHandler { } void insertAndDeleteAttachmentsOnTaskUpdate(TaskImpl newTaskImpl, TaskImpl oldTaskImpl) - throws AttachmentPersistenceException { + throws InvalidArgumentException, AttachmentPersistenceException { if (LOGGER.isDebugEnabled()) { LOGGER.debug( "entry to insertAndDeleteAttachmentsOnTaskUpdate(oldTaskImpl = {}, newTaskImpl = {})", @@ -122,40 +122,44 @@ public class AttachmentHandler { } void insertNewAttachmentsOnTaskUpdate(TaskImpl newTaskImpl, TaskImpl oldTaskImpl) - throws AttachmentPersistenceException { + throws InvalidArgumentException, AttachmentPersistenceException { List oldAttachmentIds = oldTaskImpl.getAttachments().stream() .map(AttachmentSummary::getId) .collect(Collectors.toList()); - List exceptions = new ArrayList<>(); + List invalidArgumentExceptions = new ArrayList<>(); + List attachmentPersistenceExceptions = new ArrayList<>(); newTaskImpl .getAttachments() .forEach( a -> { if (!oldAttachmentIds.contains(a.getId())) { try { - insertNewAttachmentOnTaskUpdate(newTaskImpl, a); + initializeAndInsertAttachment(newTaskImpl, (AttachmentImpl) a); + } catch (InvalidArgumentException excpt) { + invalidArgumentExceptions.add(excpt); + LOGGER.warn("attempted to insert attachment {} and caught exception", a, excpt); } catch (AttachmentPersistenceException excpt) { - exceptions.add(excpt); + attachmentPersistenceExceptions.add(excpt); LOGGER.warn("attempted to insert attachment {} and caught exception", a, excpt); } } }); - if (!exceptions.isEmpty()) { - throw exceptions.get(0); + if (!invalidArgumentExceptions.isEmpty()) { + throw invalidArgumentExceptions.get(0); + } + if (!attachmentPersistenceExceptions.isEmpty()) { + throw attachmentPersistenceExceptions.get(0); } } void insertNewAttachmentsOnTaskCreation(TaskImpl task) - throws InvalidArgumentException { + throws InvalidArgumentException, AttachmentPersistenceException { List attachments = task.getAttachments(); if (attachments != null) { for (Attachment attachment : attachments) { AttachmentImpl attachmentImpl = (AttachmentImpl) attachment; - initAttachment(attachmentImpl, task); - ObjectReference objRef = attachmentImpl.getObjectReference(); - ObjectReference.validate(objRef, "ObjectReference", "Attachment"); - attachmentMapper.insert(attachmentImpl); + initializeAndInsertAttachment(task, attachmentImpl); } } } @@ -191,28 +195,6 @@ public class AttachmentHandler { LOGGER.debug("exit from deleteRemovedAttachmentsOnTaskUpdate()"); } - void insertNewAttachmentOnTaskUpdate(TaskImpl newTaskImpl, Attachment attachment) - throws AttachmentPersistenceException { - LOGGER.debug("entry to insertNewAttachmentOnTaskUpdate()"); - AttachmentImpl attachmentImpl = (AttachmentImpl) attachment; - initAttachment(attachmentImpl, newTaskImpl); - - try { - attachmentMapper.insert(attachmentImpl); - LOGGER.debug( - "TaskService.updateTask() for TaskId={} INSERTED an Attachment={}.", - newTaskImpl.getId(), - attachmentImpl); - } catch (PersistenceException e) { - throw new AttachmentPersistenceException( - String.format( - "Cannot insert the Attachement %s for Task %s because it already exists.", - attachmentImpl.getId(), newTaskImpl.getId()), - e.getCause()); - } - LOGGER.debug("exit from insertNewAttachmentOnTaskUpdate(), returning"); - } - void initAttachment(AttachmentImpl attachment, Task newTask) { LOGGER.debug("entry to initAttachment()"); if (attachment.getId() == null) { @@ -230,4 +212,25 @@ public class AttachmentHandler { LOGGER.debug("exit from initAttachment()"); } + private void initializeAndInsertAttachment(TaskImpl task, AttachmentImpl attachmentImpl) + throws AttachmentPersistenceException, InvalidArgumentException { + LOGGER.debug("entry to initializeAndInsertAttachment()"); + initAttachment(attachmentImpl, task); + ObjectReference objRef = attachmentImpl.getObjectReference(); + ObjectReference.validate(objRef, "ObjectReference", "Attachment"); + try { + attachmentMapper.insert(attachmentImpl); + LOGGER.debug( + "TaskService.updateTask() for TaskId={} INSERTED an Attachment={}.", + task.getId(), + attachmentImpl); + } catch (PersistenceException e) { + throw new AttachmentPersistenceException( + String.format( + "Cannot insert the Attachement %s for Task %s because it already exists.", + attachmentImpl.getId(), task.getId()), + e.getCause()); + } + LOGGER.debug("exit from initializeAndInsertAttachment()"); + } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/ServiceLevelHandler.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/ServiceLevelHandler.java index e561bdcd3..1cb78700c 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/ServiceLevelHandler.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/ServiceLevelHandler.java @@ -68,7 +68,7 @@ class ServiceLevelHandler { // - For each task iterate through all referenced classifications and find minimum ServiceLevel // - collect the results into a map Duration -> List of tasks // - for each duration in this map update due date of all associated tasks - public BulkLog setPlannedPropertyOfTasksImpl(Instant planned, List argTaskIds) { + BulkLog setPlannedPropertyOfTasksImpl(Instant planned, List argTaskIds) { BulkLog bulkLog = new BulkLog(); if (argTaskIds == null || argTaskIds.isEmpty()) { return bulkLog; @@ -131,7 +131,8 @@ class ServiceLevelHandler { } // classification update if (forRefreshOnClassificationUpdate) { - newTaskImpl.setDue(newPlannedDueInstant(newTaskImpl, durationPrioHolder.getDuration(), true)); + newTaskImpl.setDue(converter.addWorkingDaysToInstant(newTaskImpl.getPlanned(), + durationPrioHolder.getDuration())); return newTaskImpl; } // creation of new task @@ -142,16 +143,6 @@ class ServiceLevelHandler { } } - Instant newPlannedDueInstant(TaskImpl task, Duration duration, boolean fromPlannedToDue) { - if (fromPlannedToDue) { - long days = converter.convertWorkingDaysToDays(task.getPlanned(), duration.toDays()); - return task.getPlanned().plus(Duration.ofDays(days)); - } else { - long days = converter.convertWorkingDaysToDays(task.getDue(), -duration.toDays()); - return task.getDue().plus(Duration.ofDays(days)); - } - } - DurationPrioHolder determineTaskPrioDuration(TaskImpl newTaskImpl, boolean onlyPriority) { Set classificationsInvolved = getInvolvedClassifications(newTaskImpl); @@ -220,18 +211,21 @@ class ServiceLevelHandler { // case 1: no change of planned / due, but potentially change of an attachment or classification if (oldTaskImpl.getDue().equals(newTaskImpl.getDue()) && oldTaskImpl.getPlanned().equals(newTaskImpl.getPlanned())) { - newTaskImpl.setDue(newPlannedDueInstant(newTaskImpl, durationPrioHolder.getDuration(), true)); + newTaskImpl.setDue(converter.addWorkingDaysToInstant(newTaskImpl.getPlanned(), + durationPrioHolder.getDuration())); } else if (oldTaskImpl.getDue().equals(newTaskImpl.getDue()) && newTaskImpl.getPlanned() != null) { // case 2: planned was changed - newTaskImpl.setDue(newPlannedDueInstant(newTaskImpl, durationPrioHolder.getDuration(), true)); + newTaskImpl.setDue(converter.addWorkingDaysToInstant(newTaskImpl.getPlanned(), + durationPrioHolder.getDuration())); } else { // case 3: due was changed if (newTaskImpl.getDue() == null) { - newTaskImpl.setDue( - newPlannedDueInstant(newTaskImpl, durationPrioHolder.getDuration(), true)); + newTaskImpl.setDue(converter.addWorkingDaysToInstant(newTaskImpl.getPlanned(), + durationPrioHolder.getDuration())); } else { Instant planned = - newPlannedDueInstant(newTaskImpl, durationPrioHolder.getDuration(), false); + (converter.subtractWorkingDaysFromInstant(newTaskImpl.getDue(), + durationPrioHolder.getDuration())); if (newTaskImpl.getPlanned() != null && !planned.equals(newTaskImpl.getPlanned())) { throw new InvalidArgumentException( "Cannot update a task with given planned " @@ -246,7 +240,8 @@ class ServiceLevelHandler { private TaskImpl updatePlannedDueOnCreationOfNewTask( TaskImpl newTaskImpl, DurationPrioHolder durationPrioHolder) throws InvalidArgumentException { if (newTaskImpl.getDue() != null) { // due is specified: calculate back and check correctnes - Instant planned = newPlannedDueInstant(newTaskImpl, durationPrioHolder.getDuration(), false); + Instant planned = (converter.subtractWorkingDaysFromInstant(newTaskImpl.getDue(), + durationPrioHolder.getDuration())); if (newTaskImpl.getPlanned() != null && !planned.equals(newTaskImpl.getPlanned())) { throw new InvalidArgumentException( "Cannot create a task with given planned " @@ -254,7 +249,8 @@ class ServiceLevelHandler { } newTaskImpl.setPlanned(planned); } else { // task.due is null: calculate forward from planned - newTaskImpl.setDue(newPlannedDueInstant(newTaskImpl, durationPrioHolder.getDuration(), true)); + newTaskImpl.setDue(converter.addWorkingDaysToInstant(newTaskImpl.getPlanned(), + durationPrioHolder.getDuration())); } return newTaskImpl; } 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 9eef1501b..4daf5b739 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 @@ -413,6 +413,8 @@ public class TaskServiceImpl implements TaskService { newTaskImpl = checkConcurrencyAndSetModified(newTaskImpl, oldTaskImpl); attachmentHandler.insertAndDeleteAttachmentsOnTaskUpdate(newTaskImpl, oldTaskImpl); + ObjectReference.validate(newTaskImpl.getPrimaryObjRef(), "primary ObjectReference", TASK); + standardUpdateActions(oldTaskImpl, newTaskImpl); taskMapper.update(newTaskImpl); @@ -1191,7 +1193,12 @@ public class TaskServiceImpl implements TaskService { if (task.getDescription() == null && classification != null) { task.setDescription(classification.getDescription()); } - attachmentHandler.insertNewAttachmentsOnTaskCreation(task); + try { + attachmentHandler.insertNewAttachmentsOnTaskCreation(task); + } catch (AttachmentPersistenceException e) { + throw new SystemException( + "Internal error when trying to insert new Attachments on Task Creation.", e); + } LOGGER.debug("exit from standardSettings()"); } @@ -1565,8 +1572,6 @@ public class TaskServiceImpl implements TaskService { private void standardUpdateActions(TaskImpl oldTaskImpl, TaskImpl newTaskImpl) throws InvalidArgumentException, InvalidStateException, ClassificationNotFoundException { - ObjectReference.validate(newTaskImpl.getPrimaryObjRef(), "primary ObjectReference", TASK); - if (oldTaskImpl.getExternalId() == null || !(oldTaskImpl.getExternalId().equals(newTaskImpl.getExternalId()))) { throw new InvalidArgumentException( diff --git a/lib/taskana-core/src/test/java/acceptance/task/ServiceLevelPriorityAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/ServiceLevelPriorityAccTest.java index 4dc847d42..864f45d9d 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/ServiceLevelPriorityAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/ServiceLevelPriorityAccTest.java @@ -50,10 +50,7 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { userName = "user_3_2", groupNames = {"group_2"}) @Test - void testSetPlannedWithDuplicatesSucceeds() - throws NotAuthorizedException, TaskNotFoundException, ClassificationNotFoundException, - InvalidArgumentException, InvalidStateException, ConcurrencyException, - AttachmentPersistenceException { + void testSetPlannedWithDuplicatesSucceeds() throws NotAuthorizedException, TaskNotFoundException { // This test works with the following tasks (w/o attachments) and classifications // @@ -124,7 +121,7 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { void testSetPlannedForTasksWithAttachmentsSucceeds() throws NotAuthorizedException, TaskNotFoundException, ClassificationNotFoundException, InvalidArgumentException, InvalidStateException, ConcurrencyException, - AttachmentPersistenceException, SQLException { + AttachmentPersistenceException { // This test works with the following tasks, attachments and classifications // @@ -179,7 +176,6 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { assertThat(dueBulk0).isEqualTo(planned.plus(1, ChronoUnit.DAYS)); assertThat(dueBulk1).isEqualTo(planned.plus(2, ChronoUnit.DAYS)); assertThat(dueBulk2).isEqualTo(planned.plus(1, ChronoUnit.DAYS)); - long delta = Duration.between(planned, due1).toDays(); assertThat(results.containsErrors()).isFalse(); assertThat(dueBulk0).isEqualTo(due0); @@ -253,8 +249,7 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { userName = "admin", groupNames = {"group_2"}) @Test - void testSetPlannedPropertyOnEmptyTasksList() - throws NotAuthorizedException, TaskNotFoundException { + void testSetPlannedPropertyOnEmptyTasksList() { Instant planned = getInstant("2020-05-03T07:00:00"); BulkOperationResults results = taskanaEngine.getTaskService().setPlannedPropertyOfTasks(planned, new ArrayList<>()); @@ -274,9 +269,7 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { groupNames = {"group_2"}) @Test void testSetPlannedPropertyOnSingleTaskWithBulkUpdate() - throws NotAuthorizedException, TaskNotFoundException, InvalidArgumentException, - ConcurrencyException, InvalidStateException, ClassificationNotFoundException, - AttachmentPersistenceException { + throws NotAuthorizedException, TaskNotFoundException, InvalidArgumentException { String taskId = "TKI:000000000000000000000000000000000002"; Instant planned = getInstant("2020-05-03T07:00:00"); // test bulk operation setPlanned... @@ -293,12 +286,11 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { userName = "admin", groupNames = {"group_2"}) @Test - void testSetPlannedPropertyOnSingleTaskWitHTaskUpdate() + void testSetPlannedPropertyOnSingleTaskWithTaskUpdate() throws NotAuthorizedException, TaskNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidStateException, ClassificationNotFoundException, AttachmentPersistenceException { String taskId = "TKI:000000000000000000000000000000000002"; - Instant planned = getInstant("2020-05-03T07:00:00"); DaysToWorkingDaysConverter converter = DaysToWorkingDaysConverter.initialize(); Task task = taskService.getTask(taskId); // test update of planned date via updateTask() @@ -324,10 +316,9 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { task.setDue(planned.plus(Duration.ofDays(8))); Task finalTask = task; assertThatThrownBy( - () -> { - taskService.updateTask(finalTask); - }) - .isInstanceOf(InvalidArgumentException.class); + () -> { + taskService.updateTask(finalTask); + }).isInstanceOf(InvalidArgumentException.class); // update due and planned as expected. task = taskService.getTask(taskId); @@ -359,7 +350,7 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { String taskId = "TKI:000000000000000000000000000000000002"; final Instant planned = getInstant("2020-05-03T07:00:00"); Task task = taskService.getTask(taskId); - + task.setPlanned(null); task = taskService.updateTask(task); DaysToWorkingDaysConverter converter = DaysToWorkingDaysConverter.initialize(); @@ -389,8 +380,7 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { userName = "admin", groupNames = {"group_2"}) @Test - void testSetPlannedPropertyOnAllTasks() - throws NotAuthorizedException, TaskNotFoundException, SQLException { + void testSetPlannedPropertyOnAllTasks() throws SQLException { Instant planned = getInstant("2020-05-03T07:00:00"); List allTasks = taskService.createTaskQuery().list(); // Now update each task with updateTask() and new planned @@ -410,6 +400,19 @@ public class ServiceLevelPriorityAccTest extends AbstractAccTest { t -> assertThat(t.getDue().equals(bulkUpdatedTaskMap.get(t.getId())))); } + @WithAccessId( + userName = "user_1_2", + groupNames = {"group_1"}) + @Test + void testUpdatePlannedAndDue() throws NotAuthorizedException, TaskNotFoundException { + TaskService taskService = taskanaEngine.getTaskService(); + Task task = taskService.getTask("TKI:000000000000000000000000000000000030"); + task.setPlanned(getInstant("2020-04-21T07:00:00")); + task.setDue(getInstant("2020-04-21T10:00:00")); + assertThatThrownBy(() -> taskService.updateTask(task)) + .isInstanceOf(InvalidArgumentException.class); + } + private TaskSummary getUpdatedTaskSummary(TaskSummary t, Instant planned) { try { Task task = taskService.getTask(t.getId()); diff --git a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAccTest.java index 0e6f2b519..ba185c730 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAccTest.java @@ -74,8 +74,7 @@ class UpdateTaskAccTest extends AbstractAccTest { groupNames = {"group_1"}) @Test void testThrowsExceptionIfMandatoryPrimaryObjectReferenceIsNotSetOrIncomplete() - throws NotAuthorizedException, ClassificationNotFoundException, TaskNotFoundException, - ConcurrencyException, AttachmentPersistenceException { + throws NotAuthorizedException, TaskNotFoundException { TaskService taskService = taskanaEngine.getTaskService(); Task task = taskService.getTask("TKI:000000000000000000000000000000000000"); @@ -343,19 +342,4 @@ class UpdateTaskAccTest extends AbstractAccTest { assertThat(retrievedUpdatedTask.getCallbackInfo()).isEqualTo(callbackInfo); } - @WithAccessId( - userName = "user_1_2", - groupNames = {"group_1"}) - @Test - void testUpdatePlannedAndDue() - throws NotAuthorizedException, TaskNotFoundException, ClassificationNotFoundException, - InvalidArgumentException, InvalidStateException, ConcurrencyException, - AttachmentPersistenceException { - TaskService taskService = taskanaEngine.getTaskService(); - Task task = taskService.getTask("TKI:000000000000000000000000000000000030"); - task.setPlanned(getInstant("2020-04-21T07:00:00")); - task.setDue(getInstant("2020-04-21T10:00:00")); - assertThatThrownBy(() -> taskService.updateTask(task)) - .isInstanceOf(InvalidArgumentException.class); - } }