From a64fb47aa0858e2cf68cc9ec9a0838284ff92e41 Mon Sep 17 00:00:00 2001 From: BerndBreier <33351391+BerndBreier@users.noreply.github.com> Date: Thu, 21 Jun 2018 15:29:49 +0200 Subject: [PATCH] TSK-587 Improve Job Support --- .../TaskanaEngineConfiguration.java | 5 +- .../ClassificationChangedJobExecutor.java | 1 - .../pro/taskana/impl/TaskServiceImpl.java | 69 +++++++++++++++---- .../taskana/impl/TaskUpdateJobExecutor.java | 4 +- .../UpdateClassificationAccTest.java | 8 ++- .../src/test/resources/taskana.properties | 4 +- .../java/pro/taskana/rest/JobScheduler.java | 2 +- .../src/main/resources/application.properties | 2 + .../src/main/resources/taskana.properties | 4 +- .../rest/ClassificationControllerIntTest.java | 42 +++++++++-- 10 files changed, 108 insertions(+), 33 deletions(-) diff --git a/lib/taskana-core/src/main/java/pro/taskana/configuration/TaskanaEngineConfiguration.java b/lib/taskana-core/src/main/java/pro/taskana/configuration/TaskanaEngineConfiguration.java index ec4997726..aa63f3f0b 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/configuration/TaskanaEngineConfiguration.java +++ b/lib/taskana-core/src/main/java/pro/taskana/configuration/TaskanaEngineConfiguration.java @@ -44,8 +44,9 @@ public class TaskanaEngineConfiguration { private static final String H2_DRIVER = "org.h2.Driver"; private static final String TASKANA_PROPERTIES = "/taskana.properties"; private static final String TASKANA_ROLES_SEPARATOR = "|"; - private static final String TASKANA_JOB_TASK_UPDATES_PER_TRANSACTION = "taskana.job.max.task.updates.per.transaction"; - private static final String TASKANA_JOB_RETRIES_FOR_FAILED_TASK_UPDATES = "taskana.job.max.retries.for.failed.task.updates"; + private static final String TASKANA_JOB_TASK_UPDATES_PER_TRANSACTION = "taskana.jobs.taskupdate.batchSize"; + private static final String TASKANA_JOB_RETRIES_FOR_FAILED_TASK_UPDATES = "taskana.jobs.taskupdate.maxRetries"; + private static final String TASKANA_DOMAINS_PROPERTY = "taskana.domains"; private static final String TASKANA_CLASSIFICATION_TYPES_PROPERTY = "taskana.classification.types"; private static final String TASKANA_CLASSIFICATION_CATEGORIES_PROPERTY = "taskana.classification.categories"; diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationChangedJobExecutor.java b/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationChangedJobExecutor.java index e1cbe09b7..44bdfd15e 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationChangedJobExecutor.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationChangedJobExecutor.java @@ -79,7 +79,6 @@ public class ClassificationChangedJobExecutor implements SingleJobExecutor { if (!taskIdBatch.isEmpty()) { String taskIds = String.join(",", taskIdBatch); args.put(ClassificationChangedJobExecutor.TASKIDS, taskIds); - args.put(CLASSIFICATION_ID, classificationId); args.put(PRIORITY_CHANGED, new Boolean(priorityChanged).toString()); args.put(SERVICE_LEVEL_CHANGED, new Boolean(serviceLevelChanged).toString()); Job job = new Job(); diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java index 92ce5554f..070fd8a7d 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java @@ -1187,15 +1187,7 @@ public class TaskServiceImpl implements TaskService { } if (newClassificationSummary == null) { // newClassification is null -> take prio and duration from attachments - if (prioDurationFromAttachments.getDuration() != null) { - long days = converter.convertWorkingDaysToDays(newTaskImpl.getPlanned(), - prioDurationFromAttachments.getDuration().toDays()); - Instant due = newTaskImpl.getPlanned().plus(Duration.ofDays(days)); - newTaskImpl.setDue(due); - } - if (prioDurationFromAttachments.getPrio() > Integer.MIN_VALUE) { - newTaskImpl.setPriority(prioDurationFromAttachments.getPrio()); - } + updateTaskPrioDurationFromAttachments(newTaskImpl, prioDurationFromAttachments); } else { Classification newClassification = null; if (!oldClassificationSummary.getKey().equals(newClassificationSummary.getKey())) { @@ -1394,14 +1386,14 @@ public class TaskServiceImpl implements TaskService { } } - BulkOperationResults classificationChanged(String taskId, String classificationId) + BulkOperationResults refreshPriorityAndDueDate(String taskId) throws ClassificationNotFoundException { - LOGGER.debug("entry to classificationChanged(taskId = {} , classificationId = {} )", taskId, classificationId); + LOGGER.debug("entry to classificationChanged(taskId = {})", taskId); TaskImpl task = null; BulkOperationResults bulkLog = new BulkOperationResults<>(); try { taskanaEngine.openConnection(); - if (taskId == null || taskId.isEmpty() || classificationId == null || classificationId.isEmpty()) { + if (taskId == null || taskId.isEmpty()) { return bulkLog; } @@ -1414,11 +1406,12 @@ public class TaskServiceImpl implements TaskService { List attachments = augmentAttachmentsByClassification(attachmentImpls, bulkLog); task.setAttachments(attachments); - Classification classification = classificationService.getClassification(classificationId); + Classification classification = classificationService + .getClassification(task.getClassificationSummary().getId()); task.setClassificationSummary(classification.asSummary()); PrioDurationHolder prioDurationFromAttachments = handleAttachmentsOnClassificationUpdate(task); - updateClassificationRelatedProperties(task, task, prioDurationFromAttachments); + updatePrioDueDateOnClassificationUpdate(task, prioDurationFromAttachments); task.setModified(Instant.now()); taskMapper.update(task); @@ -1430,6 +1423,54 @@ public class TaskServiceImpl implements TaskService { } + private void updatePrioDueDateOnClassificationUpdate(TaskImpl task, + PrioDurationHolder prioDurationFromAttachments) { + ClassificationSummary classificationSummary = task.getClassificationSummary(); + + if (classificationSummary == null) { // classification is null -> take prio and duration from attachments + updateTaskPrioDurationFromAttachments(task, prioDurationFromAttachments); + } else { + updateTaskPrioDurationFromClassificationAndAttachments(task, prioDurationFromAttachments, + classificationSummary); + } + + } + + private void updateTaskPrioDurationFromClassificationAndAttachments(TaskImpl task, + PrioDurationHolder prioDurationFromAttachments, ClassificationSummary classificationSummary) { + if (classificationSummary.getServiceLevel() != null) { + Duration durationFromClassification = Duration.parse(classificationSummary.getServiceLevel()); + Duration minDuration = prioDurationFromAttachments.getDuration(); + if (minDuration != null) { + if (minDuration.compareTo(durationFromClassification) > 0) { + minDuration = durationFromClassification; + } + } else { + minDuration = durationFromClassification; + } + + long days = converter.convertWorkingDaysToDays(task.getPlanned(), minDuration.toDays()); + Instant due = task.getPlanned().plus(Duration.ofDays(days)); + + task.setDue(due); + } + + int newPriority = Math.max(classificationSummary.getPriority(), prioDurationFromAttachments.getPrio()); + task.setPriority(newPriority); + } + + private void updateTaskPrioDurationFromAttachments(TaskImpl task, PrioDurationHolder prioDurationFromAttachments) { + if (prioDurationFromAttachments.getDuration() != null) { + long days = converter.convertWorkingDaysToDays(task.getPlanned(), + prioDurationFromAttachments.getDuration().toDays()); + Instant due = task.getPlanned().plus(Duration.ofDays(days)); + task.setDue(due); + } + if (prioDurationFromAttachments.getPrio() > Integer.MIN_VALUE) { + task.setPriority(prioDurationFromAttachments.getPrio()); + } + } + private List augmentAttachmentsByClassification(List attachmentImpls, BulkOperationResults bulkLog) { List result = new ArrayList<>(); diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskUpdateJobExecutor.java b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskUpdateJobExecutor.java index 25f16a03d..a95914a52 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskUpdateJobExecutor.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskUpdateJobExecutor.java @@ -20,7 +20,6 @@ public class TaskUpdateJobExecutor implements SingleJobExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(TaskUpdateJobExecutor.class); private TaskanaEngineImpl taskanaEngine; - private String classificationId; private List affectedTaskIds; public TaskUpdateJobExecutor() { @@ -33,7 +32,6 @@ public class TaskUpdateJobExecutor implements SingleJobExecutor { String taskIdsString = args.get(TASKIDS); affectedTaskIds = Arrays.asList(taskIdsString.split(",")); - classificationId = args.get(CLASSIFICATION_ID); BulkOperationResults bulkLog = new BulkOperationResults<>(); bulkLog.addAllErrors(handleAffectedTasks(job)); @@ -49,7 +47,7 @@ public class TaskUpdateJobExecutor implements SingleJobExecutor { BulkOperationResults bulkLog = new BulkOperationResults<>(); for (String taskId : affectedTaskIds) { try { - bulkLog.addAllErrors(taskService.classificationChanged(taskId, classificationId)); + bulkLog.addAllErrors(taskService.refreshPriorityAndDueDate(taskId)); } catch (Exception e) { bulkLog.addError(taskId, e); } diff --git a/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java b/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java index 5570a1dea..df03a8b37 100644 --- a/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java @@ -255,8 +255,14 @@ public class UpdateClassificationAccTest extends AbstractAccTest { assertTrue(task.getModified().isAfter(before)); assertTrue(task.getPriority() == 1000); long calendarDays = converter.convertWorkingDaysToDays(task.getPlanned(), 15); + // the following excluded tasks are affected via attachments. The task or an attachment still has a service + // level below 15 days + // therefore, we cannot compare the due date of these tasks to an SL of 15 days. if (!taskId.equals("TKI:000000000000000000000000000000000008") - && !taskId.equals("TKI:000000000000000000000000000000000053")) { + && !taskId.equals("TKI:000000000000000000000000000000000000") + && !taskId.equals("TKI:000000000000000000000000000000000053") + && !taskId.equals("TKI:000000000000000000000000000000000054") + && !taskId.equals("TKI:000000000000000000000000000000000055")) { assertTrue(task.getDue().equals(task.getPlanned().plus(Duration.ofDays(calendarDays)))); } diff --git a/lib/taskana-core/src/test/resources/taskana.properties b/lib/taskana-core/src/test/resources/taskana.properties index 198f402f5..3d2ef4796 100644 --- a/lib/taskana-core/src/test/resources/taskana.properties +++ b/lib/taskana-core/src/test/resources/taskana.properties @@ -8,5 +8,5 @@ taskana.domains= Domain_A , DOMAIN_B taskana.classification.types= TASK , document taskana.classification.categories= EXTERNAL , manual, autoMAtic ,Process -taskana.job.max.task.updates.per.transaction=50 -taskana.job.max.retries.for.failed.task.updates=3 +taskana.jobs.taskupdate.maxRetries=3 +taskana.jobs.taskupdate.batchSize=50 diff --git a/rest/taskana-rest-spring-example/src/main/java/pro/taskana/rest/JobScheduler.java b/rest/taskana-rest-spring-example/src/main/java/pro/taskana/rest/JobScheduler.java index 2d818fb1b..9e69e0c38 100644 --- a/rest/taskana-rest-spring-example/src/main/java/pro/taskana/rest/JobScheduler.java +++ b/rest/taskana-rest-spring-example/src/main/java/pro/taskana/rest/JobScheduler.java @@ -32,7 +32,7 @@ public class JobScheduler { @Autowired TaskanaTransactionProvider> springTransactionProvider; - @Scheduled(fixedRate = 60000) + @Scheduled(cron = "${taskana.jobscheduler.cron}") public void triggerJobs() { boolean otherJobActive = jobRunning.getAndSet(true); if (!otherJobActive) { // only one job should be active at any time diff --git a/rest/taskana-rest-spring-example/src/main/resources/application.properties b/rest/taskana-rest-spring-example/src/main/resources/application.properties index 39904c200..0823434b2 100644 --- a/rest/taskana-rest-spring-example/src/main/resources/application.properties +++ b/rest/taskana-rest-spring-example/src/main/resources/application.properties @@ -26,5 +26,7 @@ taskana.ldap.groupSearchFilterValue=groupOfUniqueNames taskana.ldap.groupNameAttribute=cn taskana.ldap.minSearchForLength=3 taskana.ldap.maxNumberOfReturnedAccessIds=50 +####### JobScheduler cron expression that specifies when the JobSchedler runs +taskana.jobscheduler.cron=0 * * * * * ####### cache static resources properties spring.resources.cache.cachecontrol.cache-private=true diff --git a/rest/taskana-rest-spring-example/src/main/resources/taskana.properties b/rest/taskana-rest-spring-example/src/main/resources/taskana.properties index 19f0a8b6b..2f88ccccd 100644 --- a/rest/taskana-rest-spring-example/src/main/resources/taskana.properties +++ b/rest/taskana-rest-spring-example/src/main/resources/taskana.properties @@ -6,5 +6,5 @@ taskana.domains=DOMAIN_A,DOMAIN_B,DOMAIN_C taskana.classification.types=TASK,DOCUMENT taskana.classification.categories= EXTERNAL , manual, autoMAtic ,Process -taskana.job.max.task.updates.per.transaction=5 -taskana.job.max.retries.for.failed.task.updates=3 +taskana.jobs.taskupdate.maxRetries=3 +taskana.jobs.taskupdate.batchSize=50 diff --git a/rest/taskana-rest-spring-example/src/test/java/pro/taskana/rest/ClassificationControllerIntTest.java b/rest/taskana-rest-spring-example/src/test/java/pro/taskana/rest/ClassificationControllerIntTest.java index bd4d2fa26..7f8b52a88 100644 --- a/rest/taskana-rest-spring-example/src/test/java/pro/taskana/rest/ClassificationControllerIntTest.java +++ b/rest/taskana-rest-spring-example/src/test/java/pro/taskana/rest/ClassificationControllerIntTest.java @@ -13,6 +13,7 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Date; import java.util.List; import org.json.JSONObject; @@ -25,8 +26,8 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.boot.web.server.LocalServerPort; -import org.springframework.context.annotation.Import; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.core.env.Environment; import org.springframework.hateoas.Link; import org.springframework.hateoas.PagedResources; import org.springframework.hateoas.hal.Jackson2HalModule; @@ -38,6 +39,8 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; +import org.springframework.scheduling.TriggerContext; +import org.springframework.scheduling.support.CronTrigger; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; @@ -48,7 +51,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import pro.taskana.Classification; import pro.taskana.Task; import pro.taskana.exceptions.InvalidArgumentException; -import pro.taskana.exceptions.NotAuthorizedException; import pro.taskana.rest.resource.ClassificationResource; import pro.taskana.rest.resource.ClassificationSummaryResource; import pro.taskana.rest.resource.TaskResource; @@ -56,7 +58,8 @@ import pro.taskana.rest.resource.assembler.ClassificationResourceAssembler; import pro.taskana.rest.resource.assembler.TaskResourceAssembler; @RunWith(SpringRunner.class) -@SpringBootTest(classes = RestConfiguration.class, webEnvironment = WebEnvironment.RANDOM_PORT, properties = {"devMode=true"}) +@SpringBootTest(classes = RestConfiguration.class, webEnvironment = WebEnvironment.RANDOM_PORT, + properties = {"devMode=true"}) public class ClassificationControllerIntTest { @Autowired @@ -65,6 +68,9 @@ public class ClassificationControllerIntTest { @Autowired private TaskResourceAssembler taskResourceAssembler; + @Autowired + Environment env; + private static final Logger LOGGER = LoggerFactory.getLogger(ClassificationControllerIntTest.class); String server = "http://127.0.0.1:"; RestTemplate template; @@ -237,7 +243,7 @@ public class ClassificationControllerIntTest { @Test public void testUpdateClassificationPrioServiceLevel() - throws IOException, InterruptedException, NotAuthorizedException, InvalidArgumentException { + throws IOException, InterruptedException, InvalidArgumentException { // 1st step: get old classification : Instant before = Instant.now(); @@ -273,9 +279,31 @@ public class ClassificationControllerIntTest { assertEquals(200, con.getResponseCode()); con.disconnect(); - // wait a minute to give JobScheduler a chance to run - LOGGER.info("About to sleep for 70 seconds to give JobScheduler a chance to process the classification change"); - Thread.sleep(70000); + // wait taskana.jobscheduler.delay + 5 seconds to give JobScheduler a chance to run + String cron = env.getProperty("taskana.jobscheduler.cron"); + CronTrigger trigger = new CronTrigger(cron); + TriggerContext context = new TriggerContext() { + + @Override + public Date lastScheduledExecutionTime() { + return null; + } + + @Override + public Date lastActualExecutionTime() { + return null; + } + + @Override + public Date lastCompletionTime() { + return null; + } + }; + long delay = trigger.nextExecutionTime(context).getTime() - (new Date()).getTime() + 2000; + + LOGGER.info("About to sleep for " + delay / 1000 + + " seconds to give JobScheduler a chance to process the classification change"); + Thread.sleep(delay); LOGGER.info("Sleeping ended. Continuing .... "); // verify the classification modified timestamp is after 'before'