From 46fe48fd36aab1310d788ead96a8574a2a5ed609 Mon Sep 17 00:00:00 2001 From: BerndBreier <33351391+BerndBreier@users.noreply.github.com> Date: Thu, 11 Jan 2018 08:46:56 +0100 Subject: [PATCH] TSK-121 Implement Holgers comments - make SystemException unchecked and map ClassificationNotFoundException to SystemException when working with existing tasks. --- .../src/main/java/pro/taskana/BaseQuery.java | 8 +- .../pro/taskana/ClassificationService.java | 6 +- .../main/java/pro/taskana/TaskService.java | 29 ++---- .../java/pro/taskana/impl/TaskQueryImpl.java | 91 +------------------ .../pro/taskana/impl/TaskServiceImpl.java | 30 ++++-- .../taskana/model/mappings/QueryMapper.java | 2 +- .../taskana/model/mappings/TaskMapper.java | 7 +- 7 files changed, 43 insertions(+), 130 deletions(-) diff --git a/lib/taskana-core/src/main/java/pro/taskana/BaseQuery.java b/lib/taskana-core/src/main/java/pro/taskana/BaseQuery.java index 621843ac6..f3865c21b 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/BaseQuery.java +++ b/lib/taskana-core/src/main/java/pro/taskana/BaseQuery.java @@ -18,7 +18,7 @@ public interface BaseQuery { * * @return List containing elements of type T * @throws NotAuthorizedException - * if the permissions are not granted for this specific interaction with query. + * if the user is not authorized to perform this query */ List list() throws NotAuthorizedException; @@ -28,10 +28,10 @@ public interface BaseQuery { * @param offset * index of the first element which should be returned. * @param limit - * amount of elements which should be returned beginning on offset. + * number of elements which should be returned beginning with offset. * @return List containing elements of type T * @throws NotAuthorizedException - * if the permissions are not granted for this specific interaction with query. + * if the user is not authorized to perform this query */ List list(int offset, int limit) throws NotAuthorizedException; @@ -40,7 +40,7 @@ public interface BaseQuery { * * @return T a single object of given Type. * @throws NotAuthorizedException - * if the permissions for interactions are not granted. + * if the user is not authorized to perform this query */ T single() throws NotAuthorizedException; diff --git a/lib/taskana-core/src/main/java/pro/taskana/ClassificationService.java b/lib/taskana-core/src/main/java/pro/taskana/ClassificationService.java index 40659b4a6..11a48bcd6 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/ClassificationService.java +++ b/lib/taskana-core/src/main/java/pro/taskana/ClassificationService.java @@ -36,8 +36,8 @@ public interface ClassificationService { List getAllClassificationsWithKey(String key, String domain); /** - * Get the Classification for key and domain. If there's no specification for the given domain, it returns the root - * domain. + * Get the Classification for key and domain. If there's no Classification in the given domain, return the + * Classification from the root domain. * * @param key * the key of the searched-for classifications @@ -45,7 +45,7 @@ public interface ClassificationService { * the domain of the searched-for classifications * @return If exist: domain-specific classification, else root classification * @throws ClassificationNotFoundException - * if no classification is found that matches key and domain. + * if no classification is found that matches the key either in domain or in the root domain. */ Classification getClassification(String key, String domain) throws ClassificationNotFoundException; diff --git a/lib/taskana-core/src/main/java/pro/taskana/TaskService.java b/lib/taskana-core/src/main/java/pro/taskana/TaskService.java index e3a4befc2..6900365d4 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/TaskService.java +++ b/lib/taskana-core/src/main/java/pro/taskana/TaskService.java @@ -32,11 +32,9 @@ public interface TaskService { * if the state of the task with taskId is not {@link TaskState#READY} * @throws InvalidOwnerException * if the task with taskId is claimed by some else - * @throws ClassificationNotFoundException - * if the task refers to a classification that cannot be found */ Task claim(String taskId) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException; + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException; /** * Claim an existing task for the current user. Enable forced claim. @@ -52,11 +50,9 @@ public interface TaskService { * if the state of the task with taskId is not {@link TaskState#READY} * @throws InvalidOwnerException * if the task with taskId is claimed by someone else - * @throws ClassificationNotFoundException - * if the task refers to a classification that cannot be found */ Task claim(String taskId, boolean forceClaim) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException; + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException; /** * Complete a claimed Task as owner/admin and update State and Timestamps. @@ -70,11 +66,9 @@ public interface TaskService { * if the given Task can´t be found in DB. * @throws InvalidOwnerException * if current user is not the task-owner or administrator. - * @throws ClassificationNotFoundException - * if the task refers to a classification that cannot be found */ Task completeTask(String taskId) - throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, ClassificationNotFoundException; + throws TaskNotFoundException, InvalidOwnerException, InvalidStateException; /** * Complete a claimed Task and update State and Timestamps. @@ -90,11 +84,9 @@ public interface TaskService { * if the given Task can´t be found in DB. * @throws InvalidOwnerException * if current user is not the task-owner or administrator. - * @throws ClassificationNotFoundException - * if the task refers to a classification that cannot be found */ Task completeTask(String taskId, boolean isForced) - throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, ClassificationNotFoundException; + throws TaskNotFoundException, InvalidOwnerException, InvalidStateException; /** * Persists a not persisted Task which does not exist already. @@ -127,10 +119,8 @@ public interface TaskService { * @return the Task * @throws TaskNotFoundException * thrown of the {@link Task} with taskId is not found - * @throws ClassificationNotFoundException - * if the classification associated to the task is not found */ - Task getTask(String taskId) throws TaskNotFoundException, ClassificationNotFoundException; + Task getTask(String taskId) throws TaskNotFoundException; /** * Transfer a task to another work basket. The transfer sets the transferred flag and resets the read flag. @@ -148,12 +138,9 @@ public interface TaskService { * Thrown if the current user is not authorized to transfer this {@link Task} to the target work basket * @throws InvalidWorkbasketException * Thrown if either the source or the target workbasket has a missing required property - * @throws ClassificationNotFoundException - * if the task refers to a classification that cannot be found */ Task transfer(String taskId, String workbasketKey) - throws TaskNotFoundException, WorkbasketNotFoundException, NotAuthorizedException, InvalidWorkbasketException, - ClassificationNotFoundException; + throws TaskNotFoundException, WorkbasketNotFoundException, NotAuthorizedException, InvalidWorkbasketException; /** * Marks a task as read. @@ -165,10 +152,8 @@ public interface TaskService { * @return the updated Task * @throws TaskNotFoundException * Thrown if the {@link Task} with taskId was not found - * @throws ClassificationNotFoundException - * if the task refers to a classification that cannot be found */ - Task setTaskRead(String taskId, boolean isRead) throws TaskNotFoundException, ClassificationNotFoundException; + Task setTaskRead(String taskId, boolean isRead) throws TaskNotFoundException; /** * This method provides a query builder for quering the database. diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskQueryImpl.java index b7ab15698..f8476ce4c 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskQueryImpl.java @@ -188,7 +188,7 @@ public class TaskQueryImpl implements TaskQuery { } @Override - public List list() throws NotAuthorizedException, SystemException { + public List list() throws NotAuthorizedException { LOGGER.debug("entry to list(), this = {}", this); List result = new ArrayList<>(); try { @@ -218,7 +218,7 @@ public class TaskQueryImpl implements TaskQuery { } @Override - public List list(int offset, int limit) throws NotAuthorizedException, SystemException { + public List list(int offset, int limit) throws NotAuthorizedException { LOGGER.debug("entry to list(offset = {}, limit = {}), this = {}", offset, limit, this); List result = new ArrayList<>(); try { @@ -249,7 +249,7 @@ public class TaskQueryImpl implements TaskQuery { } @Override - public TaskImpl single() throws NotAuthorizedException, SystemException { + public TaskImpl single() throws NotAuthorizedException { LOGGER.debug("entry to single(), this = {}", this); TaskImpl taskImpl = null; try { @@ -291,170 +291,85 @@ public class TaskQueryImpl implements TaskQuery { return name; } - public void setName(String[] name) { - this.name = name; - } - public String getDescription() { return description; } - - public void setDescription(String description) { - this.description = description; - } - public int[] getPriority() { return priority; } - public void setPriority(int[] priority) { - this.priority = priority; - } - public TaskState[] getStates() { return states; } - public void setStates(TaskState[] states) { - this.states = states; - } - public String[] getClassificationKey() { return classificationKey; } - public void setClassificationKey(String[] classificationKey) { - this.classificationKey = classificationKey; - } - public String[] getWorkbasketKey() { return workbasketKey; } - public void setWorkbasketKey(String[] workbasketKey) { - this.workbasketKey = workbasketKey; - } - public String[] getDomain() { return domain; } - public void setDomain(String[] domain) { - this.domain = domain; - } - public String[] getOwner() { return owner; } - public void setOwner(String[] owner) { - this.owner = owner; - } - public String[] getCustomFields() { return customFields; } - public void setCustomFields(String[] customFields) { - this.customFields = customFields; - } - public Boolean getIsRead() { return isRead; } - public void setIsRead(Boolean isRead) { - this.isRead = isRead; - } - public Boolean getIsTransferred() { return isTransferred; } - public void setIsTransferred(Boolean isTransferred) { - this.isTransferred = isTransferred; - } - public String[] getPorCompanyIn() { return porCompanyIn; } - public void setPorCompanyIn(String[] porCompanyIn) { - this.porCompanyIn = porCompanyIn; - } - public String getPorCompanyLike() { return porCompanyLike; } - public void setPorCompanyLike(String porCompanyLike) { - this.porCompanyLike = porCompanyLike; - } - public String[] getPorSystemIn() { return porSystemIn; } - public void setPorSystemIn(String[] porSystemIn) { - this.porSystemIn = porSystemIn; - } - public String getPorSystemLike() { return porSystemLike; } - public void setPorSystemLike(String porSystemLike) { - this.porSystemLike = porSystemLike; - } - public String[] getPorSystemInstanceIn() { return porSystemInstanceIn; } - public void setPorSystemInstanceIn(String[] porSystemInstanceIn) { - this.porSystemInstanceIn = porSystemInstanceIn; - } - public String getPorSystemInstanceLike() { return porSystemInstanceLike; } - public void setPorSystemInstanceLike(String porSystemInstanceLike) { - this.porSystemInstanceLike = porSystemInstanceLike; - } - public String[] getPorTypeIn() { return porTypeIn; } - public void setPorTypeIn(String[] porTypeIn) { - this.porTypeIn = porTypeIn; - } - public String getPorTypeLike() { return porTypeLike; } - public void setPorTypeLike(String porTypeLike) { - this.porTypeLike = porTypeLike; - } - public String[] getPorValueIn() { return porValueIn; } - public void setPorValueIn(String[] porValueIn) { - this.porValueIn = porValueIn; - } - public String getPorValueLike() { return porValueLike; } - public void setPorValueLike(String porValueLike) { - this.porValueLike = porValueLike; - } - @Override public String toString() { StringBuilder builder = new StringBuilder(); 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 d11143587..8696f0a28 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 @@ -24,6 +24,7 @@ import pro.taskana.exceptions.InvalidOwnerException; import pro.taskana.exceptions.InvalidStateException; import pro.taskana.exceptions.InvalidWorkbasketException; import pro.taskana.exceptions.NotAuthorizedException; +import pro.taskana.exceptions.SystemException; import pro.taskana.exceptions.TaskAlreadyExistException; import pro.taskana.exceptions.TaskNotFoundException; import pro.taskana.exceptions.WorkbasketNotFoundException; @@ -70,13 +71,13 @@ public class TaskServiceImpl implements TaskService { @Override public Task claim(String taskId) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException { return claim(taskId, false); } @Override public Task claim(String taskId, boolean forceClaim) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException { String userId = CurrentUserContext.getUserid(); LOGGER.debug("entry to claim(id = {}, forceClaim = {}, userId = {})", taskId, forceClaim, userId); TaskImpl task = null; @@ -113,18 +114,19 @@ public class TaskServiceImpl implements TaskService { @Override public Task completeTask(String taskId) - throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, ClassificationNotFoundException { + throws TaskNotFoundException, InvalidOwnerException, InvalidStateException { return completeTask(taskId, false); } @Override public Task completeTask(String taskId, boolean isForced) - throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, ClassificationNotFoundException { + throws TaskNotFoundException, InvalidOwnerException, InvalidStateException { LOGGER.debug("entry to completeTask(id = {}, isForced {})", taskId, isForced); TaskImpl task = null; try { taskanaEngineImpl.openConnection(); task = (TaskImpl) this.getTask(taskId); + // check pre-conditions for non-forced invocation if (!isForced) { if (task.getClaimed() == null || task.getState() != TaskState.CLAIMED) { @@ -193,7 +195,7 @@ public class TaskServiceImpl implements TaskService { } @Override - public Task getTask(String id) throws TaskNotFoundException, ClassificationNotFoundException { + public Task getTask(String id) throws TaskNotFoundException { LOGGER.debug("entry to getTaskById(id = {})", id); TaskImpl result = null; try { @@ -204,7 +206,18 @@ public class TaskServiceImpl implements TaskService { List attachments = setAttachmentObjRef( attachmentMapper.findAttachmentsByTaskId(result.getId())); result.setAttachments(attachments); - Classification classification = this.classificationService.getClassificationByTask(result); + Classification classification; + try { + classification = this.classificationService.getClassificationByTask(result); + } catch (ClassificationNotFoundException e) { + LOGGER.debug( + "getTask(taskId = {}) caught a ClassificationNotFoundException when attemptin to get " + + "the classification for the task. Throwing a SystemException ", + id); + throw new SystemException( + "TaskService.getTask could not find the classification associated to " + id); + } + result.setClassification(classification); return result; } else { @@ -219,8 +232,7 @@ public class TaskServiceImpl implements TaskService { @Override public Task transfer(String taskId, String destinationWorkbasketKey) - throws TaskNotFoundException, WorkbasketNotFoundException, NotAuthorizedException, InvalidWorkbasketException, - ClassificationNotFoundException { + throws TaskNotFoundException, WorkbasketNotFoundException, NotAuthorizedException, InvalidWorkbasketException { LOGGER.debug("entry to transfer(taskId = {}, destinationWorkbasketKey = {})", taskId, destinationWorkbasketKey); Task result = null; try { @@ -259,7 +271,7 @@ public class TaskServiceImpl implements TaskService { @Override public Task setTaskRead(String taskId, boolean isRead) - throws TaskNotFoundException, ClassificationNotFoundException { + throws TaskNotFoundException { LOGGER.debug("entry to setTaskRead(taskId = {}, isRead = {})", taskId, isRead); Task result = null; try { diff --git a/lib/taskana-core/src/main/java/pro/taskana/model/mappings/QueryMapper.java b/lib/taskana-core/src/main/java/pro/taskana/model/mappings/QueryMapper.java index 83e9773a3..242e781cd 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/model/mappings/QueryMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/model/mappings/QueryMapper.java @@ -25,7 +25,7 @@ public interface QueryMapper { String CLASSIFICATION_FINDBYKEYANDDOMAIN = "pro.taskana.model.mappings.ClassificationMapper.findByKeyAndDomain"; String CLASSIFICATION_FINDBYID = "pro.taskana.model.mappings.ClassificationMapper.findById"; - @Select("