nosu
/
taskana
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

TSK-1131 updateOwner Bulk - comments from Holger

This commit is contained in:
BerndBreier 2020-02-25 09:19:31 +01:00
parent 20bd0922db
commit 7f93dd048e
7 changed files with 163 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
lib/taskana-core/src/main/java/pro/taskana/common/internal/jobs/TaskCleanupJob.java
View File

@ -184,10 +184,12 @@ public class TaskCleanupJob extends AbstractTaskanaJob {
taskanaEngineImpl.getTaskService().deleteTasks(tasksIdsToBeDeleted);
LOGGER.debug("{} tasks deleted.", tasksIdsToBeDeleted.size() - results.getFailedIds().size());
for (String failedId : results.getFailedIds()) {
LOGGER.warn(
"Task with id {} could not be deleted. Reason: {}",
LogSanitizer.stripLineBreakingChars(failedId),
LogSanitizer.stripLineBreakingChars(results.getErrorForId(failedId)));
if (LOGGER.isWarnEnabled()) {
LOGGER.warn(
"Task with id {} could not be deleted. Reason: {}",
LogSanitizer.stripLineBreakingChars(failedId),
LogSanitizer.stripLineBreakingChars(results.getErrorForId(failedId)));
}
}
LOGGER.debug(
"exit from deleteTasks(), returning {}",

16
lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskRoutingManager.java
View File

@ -76,13 +76,17 @@ public final class TaskRoutingManager {
.filter(Objects::nonNull)
.collect(Collectors.toSet());
if (workbasketIds.isEmpty()) {
LOGGER.error(
"No TaskRouter determined a workbasket for task {}.",
LogSanitizer.stripLineBreakingChars(task));
if (LOGGER.isErrorEnabled()) {
LOGGER.error(
"No TaskRouter determined a workbasket for task {}.",
LogSanitizer.stripLineBreakingChars(task));
}
} else if (workbasketIds.size() > 1) {
LOGGER.error(
"The TaskRouters determined more than one workbasket for task {}",
LogSanitizer.stripLineBreakingChars(task));
if (LOGGER.isErrorEnabled()) {
LOGGER.error(
"The TaskRouters determined more than one workbasket for task {}",
LogSanitizer.stripLineBreakingChars(task));
}
} else {
workbasketId = workbasketIds.stream().findFirst().orElse(null);
}

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

@ -16,7 +16,6 @@ import org.apache.ibatis.exceptions.PersistenceException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import pro.taskana.TaskanaEngineConfiguration;
import pro.taskana.classification.api.ClassificationService;
import pro.taskana.classification.api.exceptions.ClassificationNotFoundException;
import pro.taskana.classification.api.models.Classification;
@ -73,12 +72,17 @@ public class TaskServiceImpl implements TaskService {
private static final String IS_ALREADY_CLAIMED_BY = " is already claimed by ";
private static final String IS_ALREADY_COMPLETED = " is already completed.";
private static final String TASK_WITH_ID_IS_NOT_READY = "Task with id %s is not in state ready.";
private static final String TASK_WITH_ID_IS_NOT_READY =
"Task with id %s is in state %s and not in state ready.";
private static final String TASK_WITH_ID_WAS_NOT_FOUND = "Task with id %s was not found.";
private static final String TASK_WITH_ID_CALLBACK_NOT_PROCESSED =
"Task wit Id %s cannot be deleted because its callback is not yet processed";
private static final String WAS_NOT_FOUND2 = " was not found.";
private static final String WAS_NOT_FOUND = " was not found";
private static final String TASK_WITH_ID = "Task with id ";
private static final String WAS_MARKED_FOR_DELETION = " was marked for deletion";
private static final String THE_WORKBASKET = "The workbasket ";
private static final String TASK = "Task";
private static final Logger LOGGER = LoggerFactory.getLogger(TaskServiceImpl.class);
private static final String ID_PREFIX_ATTACHMENT = "TAI";
private static final String ID_PREFIX_TASK = "TKI";
@ -211,7 +215,7 @@ public class TaskServiceImpl implements TaskService {
Classification classification =
this.classificationService.getClassification(classificationKey, workbasket.getDomain());
task.setClassificationSummary(classification.asSummary());
validateObjectReference(task.getPrimaryObjRef(), "primary ObjectReference", "Task");
validateObjectReference(task.getPrimaryObjRef(), "primary ObjectReference", TASK);
PrioDurationHolder prioDurationFromAttachments = handleAttachments(task);
standardSettings(task, classification, prioDurationFromAttachments);
setCallbackStateOnTaskCreation(task);
@ -628,39 +632,47 @@ public class TaskServiceImpl implements TaskService {
owner,
LoggerUtils.listToString(argTaskIds));
}
BulkOperationResults<String, TaskanaException> bulkLog = new BulkOperationResults<>();
if (argTaskIds == null || argTaskIds.isEmpty()) {
return bulkLog;
}
// remove duplicates
List<String> taskIds = argTaskIds.stream().distinct().collect(Collectors.toList());
final int requestSize = taskIds.size();
try {
taskanaEngine.openConnection();
BulkOperationResults<String, TaskanaException> bulkLog = new BulkOperationResults<>();
if (argTaskIds == null || argTaskIds.isEmpty()) {
return bulkLog;
}
// remove duplicates
List<String> taskIds = argTaskIds.stream().sorted().distinct().collect(Collectors.toList());
final int requestSize = taskIds.size();
// use only elements we are authorized for
taskIds = filterForAuthorized(taskIds, bulkLog);
Pair<List<String>, BulkOperationResults<String, TaskanaException>> resultsPair =
filterForAuthorizedTasks(taskIds);
// set the Owner of these tasks we are authorized for
taskIds = resultsPair.getLeft();
bulkLog.addAllErrors(resultsPair.getRight());
if (taskIds.isEmpty()) {
return bulkLog;
} else {
final int numberOfAffectedTasks = taskMapper.setOwnerOfTasks(owner, taskIds, Instant.now());
// check the outcome
List<MinimalTaskSummary> existingMinimalTaskSummaries =
taskMapper.findExistingTasks(taskIds, null);
// check for tasks that don't exist
handleNonExistingTasks(taskIds, existingMinimalTaskSummaries, bulkLog);
// add all errors that occured to bulkLog
addErrorsToResultObject(owner, bulkLog, existingMinimalTaskSummaries);
LOGGER.debug(
"Received the Request to set owner on "
+ requestSize
+ " tasks, "
+ "actually modified tasks = "
+ numberOfAffectedTasks
+ ", could not set owner on "
+ bulkLog.getFailedIds().size()
+ " tasks.");
return bulkLog;
if (numberOfAffectedTasks == taskIds.size()) { // all tasks were updated
return bulkLog;
} else {
// check the outcome
List<MinimalTaskSummary> existingMinimalTaskSummaries =
taskMapper.findExistingTasks(taskIds, null);
// add exceptions for non existing tasks
bulkLog.addAllErrors(
addExceptionsForNonExistingTasks(taskIds, existingMinimalTaskSummaries));
// add exceptions of all remaining tasks whose owners were not set
bulkLog.addAllErrors(
addExceptionsForTasksWhoseOwnerWasNotSet(owner, existingMinimalTaskSummaries));
if (LOGGER.isDebugEnabled()) {
LOGGER.debug(
"Received the Request to set owner on {} tasks, actually modified tasks = {}, "
+ "could not set owner on {} tasks",
requestSize,
numberOfAffectedTasks,
bulkLog.getFailedIds().size());
}
return bulkLog;
}
}
} finally {
LOGGER.debug("exit from setOwnerOfTasks()");
@ -808,20 +820,20 @@ public class TaskServiceImpl implements TaskService {
return result;
}
private void addErrorsToResultObject(
String owner,
BulkOperationResults<String, TaskanaException> bulkLog,
List<MinimalTaskSummary> existingMinimalTaskSummaries) {
private BulkOperationResults<String, TaskanaException> addExceptionsForTasksWhoseOwnerWasNotSet(
String owner, List<MinimalTaskSummary> existingMinimalTaskSummaries) {
BulkOperationResults<String, TaskanaException> bulkLog = new BulkOperationResults<>();
for (MinimalTaskSummary taskSummary : existingMinimalTaskSummaries) {
if (!owner.equals(taskSummary.getOwner())) { // owner was not set
if (!taskSummary.getTaskState().equals(TaskState.READY)) { // due to invalid state
bulkLog.addError(
taskSummary.getTaskId(),
new InvalidStateException(
"Task "
+ taskSummary.getTaskId()
+ " is in state "
+ taskSummary.getTaskState()));
String.format(
TASK_WITH_ID_IS_NOT_READY,
taskSummary.getTaskId(),
taskSummary.getTaskState())));
} else { // due to unknown reason
bulkLog.addError(
taskSummary.getTaskId(),
@ -830,12 +842,12 @@ public class TaskServiceImpl implements TaskService {
}
}
}
return bulkLog;
}
private void handleNonExistingTasks(
List<String> taskIds,
List<MinimalTaskSummary> existingMinimalTaskSummaries,
BulkOperationResults<String, TaskanaException> bulkLog) {
private BulkOperationResults<String, TaskanaException> addExceptionsForNonExistingTasks(
List<String> taskIds, List<MinimalTaskSummary> existingMinimalTaskSummaries) {
BulkOperationResults<String, TaskanaException> bulkLog = new BulkOperationResults<>();
List<String> nonExistingTaskIds = new ArrayList<>(taskIds);
List<String> existingTaskIds =
existingMinimalTaskSummaries.stream()
@ -843,15 +855,20 @@ public class TaskServiceImpl implements TaskService {
.collect(Collectors.toList());
nonExistingTaskIds.removeAll(existingTaskIds);
for (String taskId : nonExistingTaskIds) {
bulkLog.addError(taskId, new TaskNotFoundException(taskId, "Task was not found"));
bulkLog.addError(
taskId,
new TaskNotFoundException(taskId, String.format(TASK_WITH_ID_WAS_NOT_FOUND, taskId)));
}
return bulkLog;
}
private List<String> filterForAuthorized(
List<String> taskIds, BulkOperationResults<String, TaskanaException> bulkLog) {
List<String> accessIds = getAccessIds();
private Pair<List<String>, BulkOperationResults<String, TaskanaException>>
filterForAuthorizedTasks(List<String> taskIds) {
BulkOperationResults<String, TaskanaException> bulkLog = new BulkOperationResults<>();
List<String> accessIds = CurrentUserContext.getAccessIds();
List<String> tasksAuthorizedFor = new ArrayList<>(taskIds);
if (accessIds != null) {
// check authorization only for non-admin users
if (!taskanaEngine.getEngine().isUserInRole(TaskanaRole.ADMIN)) {
List<String> tasksNotAuthorizedFor =
taskMapper.filterTaskIdsNotAuthorizedFor(taskIds, accessIds);
tasksAuthorizedFor.removeAll(tasksNotAuthorizedFor);
@ -863,20 +880,7 @@ public class TaskServiceImpl implements TaskService {
"Current user not authorized for task " + taskId, currentUserId));
}
}
return tasksAuthorizedFor;
}
private List<String> getAccessIds() {
List<String> accessIds;
if (taskanaEngine.getEngine().isUserInRole(TaskanaRole.ADMIN)) {
accessIds = null;
} else {
accessIds = CurrentUserContext.getAccessIds();
if (TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds()) {
accessIds = accessIds.stream().map(String::toLowerCase).collect(Collectors.toList());
}
}
return accessIds;
return new Pair<>(tasksAuthorizedFor, bulkLog);
}
private Task claim(String taskId, boolean forceClaim)
@ -1020,8 +1024,7 @@ public class TaskServiceImpl implements TaskService {
"Cannot delete Task " + taskId + " because it is not completed.");
}
if (CallbackState.CALLBACK_PROCESSING_REQUIRED.equals(task.getCallbackState())) {
throw new InvalidStateException(
"Task " + taskId + " cannot be deleted because its callback is not yet processed");
throw new InvalidStateException(String.format(TASK_WITH_ID_CALLBACK_NOT_PROCESSED, taskId));
}
taskMapper.delete(taskId);
@ -1062,7 +1065,7 @@ public class TaskServiceImpl implements TaskService {
bulkLog.addError(
currentTaskId,
new InvalidStateException(
"Task " + currentTaskId + " cannot be deleted before callback is processed"));
String.format(TASK_WITH_ID_CALLBACK_NOT_PROCESSED, currentTaskId)));
taskIdIterator.remove();
}
}
@ -1257,7 +1260,7 @@ public class TaskServiceImpl implements TaskService {
bulkLog.addError(
currentTaskId,
new TaskNotFoundException(
currentTaskId, "task with id " + currentTaskId + WAS_NOT_FOUND2));
currentTaskId, String.format(TASK_WITH_ID_WAS_NOT_FOUND, currentTaskId)));
taskIdIterator.remove();
} else if (taskSummary.getClaimed() == null || taskSummary.getState() != TaskState.CLAIMED) {
bulkLog.addError(currentTaskId, new InvalidStateException(currentTaskId));
@ -1266,10 +1269,9 @@ public class TaskServiceImpl implements TaskService {
bulkLog.addError(
currentTaskId,
new InvalidOwnerException(
"TaskOwner is"
+ taskSummary.getOwner()
+ ", but current User is "
+ CurrentUserContext.getUserid()));
String.format(
"TaskOwner is %s, but currentUser is %s.",
taskSummary.getOwner(), CurrentUserContext.getUserid())));
taskIdIterator.remove();
} else {
taskSummary.setCompleted(now);
@ -1659,7 +1661,7 @@ public class TaskServiceImpl implements TaskService {
TaskImpl oldTaskImpl, TaskImpl newTaskImpl, PrioDurationHolder prioDurationFromAttachments)
throws InvalidArgumentException, ConcurrencyException, ClassificationNotFoundException,
InvalidStateException {
validateObjectReference(newTaskImpl.getPrimaryObjRef(), "primary ObjectReference", "Task");
validateObjectReference(newTaskImpl.getPrimaryObjRef(), "primary ObjectReference", TASK);
// TODO: not safe to rely only on different timestamps.
// With fast execution below 1ms there will be no concurrencyException
if (oldTaskImpl.getModified() != null
@ -1696,7 +1698,7 @@ public class TaskServiceImpl implements TaskService {
boolean isOwnerChanged = !Objects.equals(newTaskImpl.getOwner(), oldTaskImpl.getOwner());
if (isOwnerChanged && oldTaskImpl.getState() != TaskState.READY) {
throw new InvalidStateException(
String.format(TASK_WITH_ID_IS_NOT_READY, oldTaskImpl.getId()));
String.format(TASK_WITH_ID_IS_NOT_READY, oldTaskImpl.getId(), oldTaskImpl.getState()));
}
updateClassificationRelatedProperties(oldTaskImpl, newTaskImpl, prioDurationFromAttachments);
@ -2076,8 +2078,8 @@ public class TaskServiceImpl implements TaskService {
id,
String.format(
"When processing task updates due to change "
+ "of classification, the classification with id %s%s",
id, WAS_NOT_FOUND2)));
+ "of classification, the classification with id %s was not found",
id)));
} else {
att.setClassificationSummary(classificationSummary);
result.add(att);

46
lib/taskana-core/src/main/java/pro/taskana/task/internal/models/TaskIdOwnerState.java
View File

@ -1,46 +0,0 @@
package pro.taskana.task.internal.models;
import pro.taskana.task.api.TaskState;
public class TaskIdOwnerState {
private String taskId;
private String owner;
private TaskState taskState;
TaskIdOwnerState() {}
public String getTaskId() {
return taskId;
}
public void setTaskId(String taskId) {
this.taskId = taskId;
}
public String getOwner() {
return owner;
}
public void setOwner(String owner) {
this.owner = owner;
}
public TaskState getTaskState() {
return taskState;
}
public void setTaskState(TaskState taskState) {
this.taskState = taskState;
}
@Override
public String toString() {
return "TaskIdOwnerState [taskId="
+ taskId
+ ", owner="
+ owner
+ ", taskState="
+ taskState
+ "]";
}
}

6
lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAccTest.java
View File

@ -113,15 +113,15 @@ class UpdateTaskAccTest extends AbstractAccTest {
void testThrowsExceptionIfTaskHasAlreadyBeenUpdated()
throws NotAuthorizedException, InvalidArgumentException, ClassificationNotFoundException,
TaskNotFoundException, ConcurrencyException, AttachmentPersistenceException,
InvalidStateException {
InvalidStateException, InterruptedException {
TaskService taskService = taskanaEngine.getTaskService();
Task task = taskService.getTask("TKI:000000000000000000000000000000000000");
Task task2 = taskService.getTask("TKI:000000000000000000000000000000000000");
final Task task2 = taskService.getTask("TKI:000000000000000000000000000000000000");
task.setCustomAttribute("1", "willi");
Thread.sleep(10);
taskService.updateTask(task);
task2.setCustomAttribute("2", "Walter");
// TODO flaky test ... if speed is too high,
assertThatThrownBy(() -> taskService.updateTask(task2))

63
lib/taskana-core/src/test/java/acceptance/taskrouting/TaskRoutingAccTest.java
View File

@ -1,9 +1,9 @@
package acceptance.taskrouting;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import acceptance.AbstractAccTest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@ -37,15 +37,15 @@ class TaskRoutingAccTest extends AbstractAccTest {
newTask.setPrimaryObjRef(
createObjectReference("COMPANY_A", "SYSTEM_A", "INSTANCE_A", "VNR", "1234567"));
final Task taskToCreate = newTask;
Assertions.assertThrows(
InvalidArgumentException.class, () -> taskService.createTask(taskToCreate));
assertThatThrownBy(() -> taskService.createTask(taskToCreate))
.isInstanceOf(InvalidArgumentException.class);
((TaskImpl) taskToCreate).setDomain("DOMAIN_C");
Assertions.assertThrows(
InvalidArgumentException.class, () -> taskService.createTask(taskToCreate));
assertThatThrownBy(() -> taskService.createTask(taskToCreate))
.isInstanceOf(InvalidArgumentException.class);
((TaskImpl) taskToCreate).setDomain("DOMAIN_B");
Task createdTask = taskService.createTask(taskToCreate);
assertEquals(
"WBI:100000000000000000000000000000000011", createdTask.getWorkbasketSummary().getId());
assertThat("WBI:100000000000000000000000000000000011")
.isEqualTo(createdTask.getWorkbasketSummary().getId());
}
@WithAccessId(
@ -56,12 +56,49 @@ class TaskRoutingAccTest extends AbstractAccTest {
throws WorkbasketNotFoundException, ClassificationNotFoundException, NotAuthorizedException,
TaskAlreadyExistException, InvalidArgumentException, TaskNotFoundException {
TaskImpl createdTaskA = createTask("DOMAIN_A", "L12010");
assertEquals(
"WBI:100000000000000000000000000000000001", createdTaskA.getWorkbasketSummary().getId());
assertThat("WBI:100000000000000000000000000000000001")
.isEqualTo(createdTaskA.getWorkbasketSummary().getId());
TaskImpl createdTaskB = createTask("DOMAIN_B", "T21001");
assertEquals(
"WBI:100000000000000000000000000000000011", createdTaskB.getWorkbasketSummary().getId());
Assertions.assertThrows(InvalidArgumentException.class, () -> createTask(null, "L12010"));
assertThat("WBI:100000000000000000000000000000000011")
.isEqualTo(createdTaskB.getWorkbasketSummary().getId());
assertThatThrownBy(() -> createTask(null, "L12010"))
.isInstanceOf(InvalidArgumentException.class);
}
@WithAccessId(
userName = "admin",
groupNames = {"group_1"})
@Test
void testCreateTaskWithNullRouting()
throws WorkbasketNotFoundException, ClassificationNotFoundException, NotAuthorizedException,
TaskAlreadyExistException, InvalidArgumentException, TaskNotFoundException {
TaskService taskService = taskanaEngine.getTaskService();
Task newTask = taskService.newTask(null, "DOMAIN_A");
newTask.setClassificationKey("L12010");
newTask.setPrimaryObjRef(
createObjectReference("COMPANY_A", "SYSTEM_A", "INSTANCE_A", "VNR", "1234567"));
newTask.setCustomAttribute("7", "noRouting");
assertThatThrownBy(() -> taskService.createTask(newTask))
.isInstanceOf(InvalidArgumentException.class);
}
@WithAccessId(
userName = "admin",
groupNames = {"group_1"})
@Test
void testCreateTaskWithRoutingToMultipleWorkbaskets()
throws WorkbasketNotFoundException, ClassificationNotFoundException, NotAuthorizedException,
TaskAlreadyExistException, InvalidArgumentException, TaskNotFoundException {
TaskService taskService = taskanaEngine.getTaskService();
Task newTask = taskService.newTask(null, "DOMAIN_B");
newTask.setClassificationKey("L12010");
newTask.setPrimaryObjRef(
createObjectReference("COMPANY_A", "SYSTEM_A", "INSTANCE_A", "VNR", "1234567"));
newTask.setCustomAttribute("7", "multipleWorkbaskets");
assertThatThrownBy(() -> taskService.createTask(newTask))
.isInstanceOf(InvalidArgumentException.class);
}
private TaskImpl createTask(String domain, String classificationKey)

24
lib/taskana-core/src/test/java/acceptance/taskrouting/TestTaskRoutingProviderForDomainA.java
View File

@ -1,12 +1,17 @@
package acceptance.taskrouting;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import pro.taskana.common.api.TaskanaEngine;
import pro.taskana.common.api.exceptions.InvalidArgumentException;
import pro.taskana.spi.routing.api.TaskRoutingProvider;
import pro.taskana.task.api.models.Task;
/** This is a sample implementation of TaskRouter. */
public class TestTaskRoutingProviderForDomainA implements TaskRoutingProvider {
private static final Logger LOGGER =
LoggerFactory.getLogger(TestTaskRoutingProviderForDomainA.class);
TaskanaEngine theEngine;
@Override
@ -16,8 +21,21 @@ public class TestTaskRoutingProviderForDomainA implements TaskRoutingProvider {
@Override
public String determineWorkbasketId(Task task) {
if ("DOMAIN_A".equals(task.getDomain())) {
return "WBI:100000000000000000000000000000000001";
String att7 = "";
try {
att7 = task.getCustomAttribute("7");
} catch (InvalidArgumentException ex) {
LOGGER.warn("caught exception ", ex);
}
if (att7 != null && att7.equals("multipleWorkbaskets")) {
return "WBI:100000000000000000000000000000000005";
} else if ("DOMAIN_A".equals(task.getDomain())) {
if (att7 != null && att7.equals("noRouting")) {
return null;
} else {
return "WBI:100000000000000000000000000000000001";
}
} else {
return null;
}