TSK-23: Complete a Task with force-flag and user-check. Added unit test for this function. Added completeTask(id) without flag, improved unit-test, reacted to TSK-22.

This commit is contained in:
Marcel Lengl 2017-12-12 17:14:05 +01:00 committed by Holger Hagen
parent ea60136855
commit f1ddf6780c
8 changed files with 221 additions and 55 deletions

View File

@ -14,6 +14,8 @@ import org.slf4j.LoggerFactory;
import pro.taskana.exceptions.ClassificationAlreadyExistException;
import pro.taskana.exceptions.ClassificationNotFoundException;
import pro.taskana.exceptions.InvalidOwnerException;
import pro.taskana.exceptions.InvalidStateException;
import pro.taskana.exceptions.NotAuthorizedException;
import pro.taskana.exceptions.TaskNotFoundException;
import pro.taskana.exceptions.WorkbasketNotFoundException;
@ -58,9 +60,9 @@ public class TaskanaRestTest {
@DELETE
@Path("{id}")
public void completeTask(@PathParam("id") String id) throws TaskNotFoundException {
public void completeTask(@PathParam("id") String id) throws TaskNotFoundException, InvalidOwnerException, InvalidStateException {
logger.info(id);
taskanaEjb.getTaskService().complete(id);
taskanaEjb.getTaskService().completeTask(id, true);
}
}

View File

@ -53,15 +53,31 @@ public interface TaskService {
throws TaskNotFoundException, InvalidStateException, InvalidOwnerException;
/**
* Set task to completed.
* Complete a claimed Task as owner/admin and update State and Timestamps.
*
* @param taskId
* the task id
* @return changed Task after update.
* @throws TaskNotFoundException
* thrown if the task with taskId is not found.
* @param taskId - Id of the Task which should be completed.
*
* @return Task - updated task after completion.
*
* @throws InvalidStateException when Task wasn´t claimed before.
* @throws TaskNotFoundException if the given Task can´t be found in DB.
* @throws InvalidOwnerException if current user is not the task-owner or administrator.
*/
Task complete(String taskId) throws TaskNotFoundException;
Task completeTask(String taskId) throws TaskNotFoundException, InvalidOwnerException, InvalidStateException;
/**
* Complete a claimed Task and update State and Timestamps.
*
* @param taskId - Id of the Task which should be completed.
* @param isForced - Flag which can complete a Task in every case if Task does exist.
*
* @return Task - updated task after completion.
*
* @throws InvalidStateException when Task wasn´t claimed before.
* @throws TaskNotFoundException if the given Task can´t be found in DB.
* @throws InvalidOwnerException if current user is not the task-owner or administrator.
*/
Task completeTask(String taskId, boolean isForced) throws TaskNotFoundException, InvalidOwnerException, InvalidStateException;
/**
* Create and persist a task.

View File

@ -111,26 +111,41 @@ public class TaskServiceImpl implements TaskService {
}
@Override
public Task complete(String id) throws TaskNotFoundException {
LOGGER.debug("entry to complete(id = {})", id);
public Task completeTask(String taskId) throws TaskNotFoundException, InvalidOwnerException, InvalidStateException {
return completeTask(taskId, false);
}
@Override
public Task completeTask(String taskId, boolean isForced) throws TaskNotFoundException, InvalidOwnerException, InvalidStateException {
LOGGER.debug("entry to completeTask(id = {}, isForced {})", taskId, isForced);
Task task = null;
try {
taskanaEngineImpl.openConnection();
task = taskMapper.findById(id);
if (task != null) {
Timestamp now = new Timestamp(System.currentTimeMillis());
task.setCompleted(now);
task.setModified(now);
task.setState(TaskState.COMPLETED);
taskMapper.update(task);
LOGGER.debug("Method complete() completed Task '{}'.", id);
task = this.getTaskById(taskId);
// check pre-conditions for non-forced invocation
if (!isForced) {
if (task.getClaimed() == null || task.getState() != TaskState.CLAIMED) {
LOGGER.warn("Method completeTask() does expect a task which need to be CLAIMED before. TaskId={}", taskId);
throw new InvalidStateException(taskId);
} else if (CurrentUserContext.getUserid() != task.getOwner()) {
LOGGER.warn("Method completeTask() does expect to be invoced by the task-owner or a administrator. TaskId={}, TaskOwner={}, CurrentUser={}", taskId, task.getOwner(), CurrentUserContext.getUserid());
throw new InvalidOwnerException("TaskOwner is" + task.getOwner() + ", but current User is " + CurrentUserContext.getUserid());
}
} else {
LOGGER.warn("Method complete() didn't find task with id {}. Throwing TaskNotFoundException", id);
throw new TaskNotFoundException(id);
// CLAIM-forced, if task was not already claimed before.
if (task.getClaimed() == null || task.getState() != TaskState.CLAIMED) {
task = this.claim(taskId, true);
}
}
Timestamp now = new Timestamp(System.currentTimeMillis());
task.setCompleted(now);
task.setModified(now);
task.setState(TaskState.COMPLETED);
taskMapper.update(task);
LOGGER.debug("Method completeTask() completed Task '{}'.", taskId);
} finally {
taskanaEngineImpl.returnConnection();
LOGGER.debug("exit from complete()");
LOGGER.debug("exit from completeTask()");
}
return task;
}

View File

@ -40,8 +40,6 @@ public class ClassificationServiceImplTest {
private final Date today = Date.valueOf(LocalDate.now());
private final String idPrefixClassification = "CLI";
@Spy
@InjectMocks
private ClassificationServiceImpl cutSpy;

View File

@ -38,6 +38,8 @@ import pro.taskana.WorkbasketService;
import pro.taskana.configuration.TaskanaEngineConfiguration;
import pro.taskana.exceptions.ClassificationAlreadyExistException;
import pro.taskana.exceptions.ClassificationNotFoundException;
import pro.taskana.exceptions.InvalidOwnerException;
import pro.taskana.exceptions.InvalidStateException;
import pro.taskana.exceptions.NotAuthorizedException;
import pro.taskana.exceptions.TaskNotFoundException;
import pro.taskana.exceptions.WorkbasketNotFoundException;
@ -123,7 +125,6 @@ public class TaskServiceImplTest {
verify(workbasketServiceMock, times(1)).getWorkbasket(any());
verify(taskMapperMock, times(1)).insert(expectedTask);
verify(taskanaEngineImpl, times(1)).returnConnection();
verify(classificationServiceMock, times(1)).createClassification(any());
verify(classificationServiceMock, times(1)).getClassification(any(), any());
verifyNoMoreInteractions(taskanaEngineConfigurationMock, taskanaEngineMock, taskanaEngineImpl,
taskMapperMock, objectReferenceMapperMock, workbasketServiceMock,
@ -164,7 +165,6 @@ public class TaskServiceImplTest {
verify(objectReferenceMapperMock, times(1)).findByObjectReference(any());
verify(taskMapperMock, times(1)).insert(expectedTask);
verify(taskanaEngineImpl, times(1)).returnConnection();
verify(classificationServiceMock, times(1)).createClassification(any());
verify(classificationServiceMock, times(1)).getClassification(any(),
any());
verifyNoMoreInteractions(taskanaEngineConfigurationMock, taskanaEngineMock, taskanaEngineImpl,
@ -184,6 +184,7 @@ public class TaskServiceImplTest {
@Test
public void testCreateSimpleTaskWithObjectReferenceIsNull() throws NotAuthorizedException,
WorkbasketNotFoundException, ClassificationNotFoundException, ClassificationAlreadyExistException {
TaskServiceImpl cutSpy = Mockito.spy(cut);
ObjectReference expectedObjectReference = new ObjectReference();
expectedObjectReference.setId("1");
expectedObjectReference.setType("DUMMY");
@ -198,14 +199,13 @@ public class TaskServiceImplTest {
Mockito.doNothing().when(objectReferenceMapperMock).insert(any());
Mockito.doReturn(null).when(objectReferenceMapperMock).findByObjectReference(any());
Task actualTask = cut.createTask(expectedTask);
Task actualTask = cutSpy.createTask(expectedTask);
expectedTask.getPrimaryObjRef().setId(actualTask.getPrimaryObjRef().getId()); // get only new ID
verify(taskanaEngineImpl, times(1)).openConnection();
verify(taskanaEngineMock, times(1)).getClassificationService();
verify(workbasketServiceMock, times(1)).checkAuthorization(any(), any());
verify(workbasketServiceMock, times(1)).getWorkbasket(any());
verify(classificationServiceMock, times(1)).createClassification(any());
verify(classificationServiceMock, times(1)).getClassification(any(), any());
verify(objectReferenceMapperMock, times(1)).findByObjectReference(any());
verify(objectReferenceMapperMock, times(1)).insert(any());
@ -378,34 +378,90 @@ public class TaskServiceImplTest {
}
@Test
public void testCompleteTask()
throws TaskNotFoundException, InterruptedException, ClassificationAlreadyExistException {
Task expectedTask = createUnitTestTask("1", "Unit Test Task 1", "1");
Thread.sleep(SLEEP_TIME); // to have different timestamps
Mockito.doReturn(expectedTask).when(taskMapperMock).findById(expectedTask.getId());
public void testCompleteTaskDefault() throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, InterruptedException {
TaskServiceImpl cutSpy = Mockito.spy(cut);
final long sleepTime = 100L;
final boolean isForced = false;
Task task = createUnitTestTask("1", "Unit Test Task 1", "1");
Thread.sleep(sleepTime);
task.setState(TaskState.CLAIMED);
task.setClaimed(new Timestamp(System.currentTimeMillis()));
task.setOwner(CurrentUserContext.getUserid());
doReturn(task).when(cutSpy).completeTask(task.getId(), isForced);
Task actualTask = cut.complete(expectedTask.getId());
cutSpy.completeTask(task.getId());
// Just Verify unforced call of complex-complete()
verify(cutSpy, times(1)).completeTask(task.getId(), isForced);
verifyNoMoreInteractions(taskanaEngineConfigurationMock, taskanaEngineMock, taskanaEngineImpl,
taskMapperMock, objectReferenceMapperMock, workbasketServiceMock);
}
@Test
public void testCompleteTaskNotForcedWorking() throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, InterruptedException {
TaskServiceImpl cutSpy = Mockito.spy(cut);
final long sleepTime = 100L;
final boolean isForced = false;
Task task = createUnitTestTask("1", "Unit Test Task 1", "1");
// created and modify should be able to be different.
Thread.sleep(sleepTime);
task.setState(TaskState.CLAIMED);
task.setClaimed(new Timestamp(System.currentTimeMillis()));
task.setOwner(CurrentUserContext.getUserid());
doReturn(task).when(cutSpy).getTaskById(task.getId());
doNothing().when(taskMapperMock).update(task);
Task actualTask = cutSpy.completeTask(task.getId(), isForced);
verify(taskanaEngineImpl, times(1)).openConnection();
verify(taskMapperMock, times(1)).findById(expectedTask.getId());
verify(taskMapperMock, times(1)).update(any());
verify(cutSpy, times(1)).getTaskById(task.getId());
verify(taskMapperMock, times(1)).update(task);
verify(taskanaEngineImpl, times(1)).returnConnection();
verifyNoMoreInteractions(taskanaEngineConfigurationMock, taskanaEngineMock, taskanaEngineImpl,
taskMapperMock, objectReferenceMapperMock, workbasketServiceMock);
assertThat(actualTask.getState(), equalTo(TaskState.COMPLETED));
assertThat(actualTask.getCreated(), not(equalTo(expectedTask.getModified())));
assertThat(actualTask.getCreated(), not(equalTo(task.getModified())));
assertThat(actualTask.getCompleted(), not(equalTo(null)));
assertThat(actualTask.getCompleted(), equalTo(actualTask.getModified()));
}
@Test(expected = TaskNotFoundException.class)
public void testCompleteFailsWithNonExistingTaskId() throws TaskNotFoundException {
String invalidTaskId = "";
@Test(expected = InvalidStateException.class)
public void testCompleteTaskNotForcedNotClaimedBefore() throws TaskNotFoundException, InvalidStateException, InvalidOwnerException {
final boolean isForced = false;
TaskServiceImpl cutSpy = Mockito.spy(cut);
Task task = createUnitTestTask("1", "Unit Test Task 1", "1");
task.setState(TaskState.READY);
task.setClaimed(null);
doReturn(task).when(cutSpy).getTaskById(task.getId());
try {
cut.complete(invalidTaskId);
} catch (Exception e) {
cutSpy.completeTask(task.getId(), isForced);
} catch (InvalidStateException e) {
verify(taskanaEngineImpl, times(1)).openConnection();
verify(taskMapperMock, times(1)).findById(invalidTaskId);
verify(cutSpy, times(1)).getTaskById(task.getId());
verify(taskanaEngineImpl, times(1)).returnConnection();
verifyNoMoreInteractions(taskanaEngineConfigurationMock, taskanaEngineMock, taskanaEngineImpl,
taskMapperMock, objectReferenceMapperMock, workbasketServiceMock);
throw e;
}
}
@Test(expected = InvalidOwnerException.class)
public void testCompleteTaskNotForcedInvalidOwnerException() throws TaskNotFoundException, InvalidStateException, InvalidOwnerException {
final boolean isForced = false;
TaskServiceImpl cutSpy = Mockito.spy(cut);
Task task = createUnitTestTask("1", "Unit Test Task 1", "1");
task.setOwner("Dummy-Owner-ID: 10");
task.setState(TaskState.CLAIMED);
task.setClaimed(new Timestamp(System.currentTimeMillis()));
doReturn(task).when(cutSpy).getTaskById(task.getId());
try {
cutSpy.completeTask(task.getId(), isForced);
} catch (InvalidOwnerException e) {
verify(taskanaEngineImpl, times(1)).openConnection();
verify(cutSpy, times(1)).getTaskById(task.getId());
verify(taskanaEngineImpl, times(1)).returnConnection();
verifyNoMoreInteractions(taskanaEngineConfigurationMock, taskanaEngineMock, taskanaEngineImpl,
taskMapperMock, objectReferenceMapperMock, workbasketServiceMock);
@ -413,6 +469,84 @@ public class TaskServiceImplTest {
}
}
@Test(expected = TaskNotFoundException.class)
public void testCompleteTaskTaskNotFound() throws TaskNotFoundException, InvalidStateException, InvalidOwnerException {
TaskServiceImpl cutSpy = Mockito.spy(cut);
final boolean isForced = false;
String taskId = "1";
doThrow(TaskNotFoundException.class).when(cutSpy).getTaskById(taskId);
try {
cutSpy.completeTask(taskId, isForced);
} catch (InvalidOwnerException e) {
verify(taskanaEngineImpl, times(1)).openConnection();
verify(cutSpy, times(1)).getTaskById(taskId);
verify(taskanaEngineImpl, times(1)).returnConnection();
verifyNoMoreInteractions(taskanaEngineConfigurationMock, taskanaEngineMock, taskanaEngineImpl,
taskMapperMock, objectReferenceMapperMock, workbasketServiceMock);
throw e;
}
}
@Test
public void testCompleteForcedAndAlreadyClaimed() throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, InterruptedException {
final boolean isForced = true;
final long sleepTime = 100L;
TaskServiceImpl cutSpy = Mockito.spy(cut);
Task task = createUnitTestTask("1", "Unit Test Task 1", "1");
// created and modify should be able to be different.
Thread.sleep(sleepTime);
task.setState(TaskState.CLAIMED);
task.setClaimed(new Timestamp(System.currentTimeMillis()));
doReturn(task).when(cutSpy).getTaskById(task.getId());
doNothing().when(taskMapperMock).update(task);
Task actualTask = cutSpy.completeTask(task.getId(), isForced);
verify(taskanaEngineImpl, times(1)).openConnection();
verify(cutSpy, times(1)).getTaskById(task.getId());
verify(taskMapperMock, times(1)).update(task);
verify(taskanaEngineImpl, times(1)).returnConnection();
verifyNoMoreInteractions(taskanaEngineConfigurationMock, taskanaEngineMock, taskanaEngineImpl,
taskMapperMock, objectReferenceMapperMock, workbasketServiceMock);
assertThat(actualTask.getState(), equalTo(TaskState.COMPLETED));
assertThat(actualTask.getCreated(), not(equalTo(task.getModified())));
assertThat(actualTask.getCompleted(), not(equalTo(null)));
assertThat(actualTask.getCompleted(), equalTo(actualTask.getModified()));
}
@Test
public void testCompleteForcedNotClaimed() throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, InterruptedException {
TaskServiceImpl cutSpy = Mockito.spy(cut);
final boolean isForced = true;
final long sleepTime = 100L;
Task task = createUnitTestTask("1", "Unit Test Task 1", "1");
task.setState(TaskState.READY);
task.setClaimed(null);
doReturn(task).when(cutSpy).getTaskById(task.getId());
Task claimedTask = createUnitTestTask("1", "Unit Test Task 1", "1");
// created and modify should be able to be different.
Thread.sleep(sleepTime);
claimedTask.setState(TaskState.CLAIMED);
claimedTask.setClaimed(new Timestamp(System.currentTimeMillis()));
doReturn(claimedTask).when(cutSpy).claim(task.getId(), isForced);
doNothing().when(taskMapperMock).update(claimedTask);
Task actualTask = cutSpy.completeTask(task.getId(), isForced);
verify(taskanaEngineImpl, times(1)).openConnection();
verify(cutSpy, times(1)).getTaskById(task.getId());
verify(cutSpy, times(1)).claim(task.getId(), isForced);
verify(taskMapperMock, times(1)).update(claimedTask);
verify(taskanaEngineImpl, times(1)).returnConnection();
verifyNoMoreInteractions(taskanaEngineConfigurationMock, taskanaEngineMock, taskanaEngineImpl,
taskMapperMock, objectReferenceMapperMock, workbasketServiceMock);
assertThat(actualTask.getState(), equalTo(TaskState.COMPLETED));
assertThat(actualTask.getCreated(), not(equalTo(claimedTask.getModified())));
assertThat(actualTask.getCompleted(), not(equalTo(null)));
assertThat(actualTask.getCompleted(), equalTo(actualTask.getModified()));
}
@Test
public void testTransferTaskToDestinationWorkbasketWithoutSecurity()
throws TaskNotFoundException, WorkbasketNotFoundException, NotAuthorizedException,
@ -420,7 +554,6 @@ public class TaskServiceImplTest {
TaskServiceImpl cutSpy = Mockito.spy(cut);
Workbasket destinationWorkbasket = createWorkbasket("2");
Task task = createUnitTestTask("1", "Unit Test Task 1", "1");
final int workServiceMockCalls = 3;
task.setRead(true);
doReturn(destinationWorkbasket).when(workbasketServiceMock).getWorkbasket(destinationWorkbasket.getId());
doReturn(taskanaEngineConfigurationMock).when(taskanaEngineMock).getConfiguration();
@ -582,11 +715,12 @@ public class TaskServiceImplTest {
@Test
public void testGetTaskCountForState() {
TaskServiceImpl cutSpy = Mockito.spy(cut);
List<TaskState> taskStates = Arrays.asList(TaskState.CLAIMED, TaskState.COMPLETED);
List<TaskStateCounter> expectedResult = new ArrayList<>();
doReturn(expectedResult).when(taskMapperMock).getTaskCountForState(taskStates);
List<TaskStateCounter> actualResult = cut.getTaskCountForState(taskStates);
List<TaskStateCounter> actualResult = cutSpy.getTaskCountForState(taskStates);
verify(taskanaEngineImpl, times(1)).openConnection();
verify(taskMapperMock, times(1)).getTaskCountForState(taskStates);
@ -768,8 +902,7 @@ public class TaskServiceImplTest {
assertThat(actualResultList.size(), equalTo(expectedResultList.size()));
}
private Task createUnitTestTask(String id, String name, String workbasketId)
throws ClassificationAlreadyExistException {
private Task createUnitTestTask(String id, String name, String workbasketId) {
Task task = new Task();
task.setId(id);
task.setName(name);
@ -778,7 +911,6 @@ public class TaskServiceImplTest {
task.setCreated(now);
task.setModified(now);
Classification classification = (Classification) new ClassificationImpl();
classificationServiceMock.createClassification(classification);
task.setClassification(classification);
return task;
}

View File

@ -248,7 +248,7 @@ public class WorkbasketServiceImplIntAutocommitTest {
WorkbasketQuery query2 = workBasketService.createWorkbasketQuery().access(WorkbasketAuthorization.OPEN, "Bernd", "Konstantin");
List<Workbasket> result2 = query2.list();
Assert.assertEquals(1, result1.size());
Assert.assertEquals(1, result2.size());
WorkbasketQuery query3 = workBasketService.createWorkbasketQuery().access(WorkbasketAuthorization.CUSTOM_5, "Bernd", "Konstantin");
List<Workbasket> result3 = query3.list();
@ -256,7 +256,7 @@ public class WorkbasketServiceImplIntAutocommitTest {
WorkbasketQuery query4 = workBasketService.createWorkbasketQuery().access(WorkbasketAuthorization.CUSTOM_1, "Bernd");
List<Workbasket> result4 = query4.list();
Assert.assertEquals(0, result3.size());
Assert.assertEquals(0, result4.size());
}

View File

@ -1,8 +1,11 @@
package pro.taskana;
import javax.annotation.PostConstruct;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;
import pro.taskana.exceptions.ClassificationNotFoundException;
import pro.taskana.exceptions.InvalidOwnerException;
import pro.taskana.exceptions.InvalidStateException;
@ -11,8 +14,6 @@ import pro.taskana.exceptions.TaskNotFoundException;
import pro.taskana.exceptions.WorkbasketNotFoundException;
import pro.taskana.model.Task;
import javax.annotation.PostConstruct;
@Component
@Transactional
public class ExampleBootstrap {
@ -31,8 +32,8 @@ public class ExampleBootstrap {
taskService.claim(task.getId());
System.out.println(
"---------------------------> Task claimed: " + taskService.getTaskById(task.getId()).getOwner());
// taskService.complete(task.getId());
// System.out.println("---------------------------> Task completed");
taskService.completeTask(task.getId(), true);
System.out.println("---------------------------> Task completed");
}
}

View File

@ -105,11 +105,13 @@ public class TaskController {
@RequestMapping(method = RequestMethod.POST, value = "/{taskId}/complete")
public ResponseEntity<Task> completeTask(@PathVariable String taskId) {
try {
taskService.complete(taskId);
taskService.completeTask(taskId, true);
Task updatedTask = taskService.getTaskById(taskId);
return ResponseEntity.status(HttpStatus.OK).body(updatedTask);
} catch (TaskNotFoundException e) {
return ResponseEntity.status(HttpStatus.NOT_FOUND).build();
} catch(InvalidStateException | InvalidOwnerException e) {
return ResponseEntity.status(HttpStatus.PRECONDITION_FAILED).build();
}
}