From 34d2bbfa929ccaa9e9b93c173b8fa867154330bb Mon Sep 17 00:00:00 2001 From: Mustapha Zorgati <15628173+mustaphazorgati@users.noreply.github.com> Date: Thu, 8 Jul 2021 03:45:15 +0200 Subject: [PATCH] TSK-1647: Implemented an error key for every exception Co-authored-by: Tristan Eisermann<19949441+Tristan2357@users.noreply.github.com> Co-authored-by: Tim Gerversmann<72377965+tge20@users.noreply.github.com> Co-authored-by: Sofie Hofmann<29145005+sofie29@users.noreply.github.com> javaDoc Exceptions blub TSK-1647: now validating existing & not authorized tasks :) --- .../internal/logging/LoggingAspectTest.java | 10 +- .../common/api/BulkOperationResults.java | 4 +- .../exceptions/AutocommitFailedException.java | 8 +- .../api/exceptions/ConcurrencyException.java | 8 +- .../exceptions/ConnectionNotSetException.java | 8 +- .../exceptions/DomainNotFoundException.java | 20 +- .../common/api/exceptions/ErrorCode.java | 57 ++++ .../exceptions/InvalidArgumentException.java | 6 +- .../exceptions/MismatchedRoleException.java | 36 ++ .../exceptions/NotAuthorizedException.java | 14 +- .../api/exceptions/NotFoundException.java | 13 +- .../api/exceptions/SystemException.java | 8 +- .../api/exceptions/TaskanaException.java | 22 +- .../exceptions/TaskanaRuntimeException.java | 29 +- .../UnsupportedDatabaseException.java | 24 +- .../WrongCustomHolidayFormatException.java | 21 +- .../configuration/DbSchemaCreator.java | 4 +- .../internal/util/ComparableVersion.java | 17 +- .../common/internal/util/EnumUtil.java | 22 ++ .../common/internal/util/MapCreator.java | 57 ++++ .../util/ObjectAttributeChangeDetector.java | 4 +- .../impl/LogfileHistoryServiceImpl.java | 1 - .../impl/SimpleHistoryServiceImpl.java | 6 +- .../impl/TaskanaHistoryEngineImpl.java | 5 +- .../impl/jobs/HistoryCleanupJob.java | 2 +- .../TaskHistoryEventControllerIntTest.java | 7 +- .../taskana/TaskanaEngineConfiguration.java | 6 +- .../api/ClassificationService.java | 17 +- .../ClassificationAlreadyExistException.java | 33 +- .../ClassificationInUseException.java | 43 ++- .../ClassificationNotFoundException.java | 40 ++- .../MalformedServiceLevelException.java | 56 ++++ .../internal/ClassificationQueryImpl.java | 2 +- .../internal/ClassificationServiceImpl.java | 90 ++--- .../jobs/ClassificationChangedJob.java | 9 +- .../pro/taskana/common/api/TaskanaEngine.java | 11 +- .../internal/InternalTaskanaEngine.java | 1 - .../common/internal/TaskanaEngineImpl.java | 18 +- .../TaskanaHistoryEventNotFoundException.java | 21 +- .../pro/taskana/task/api/TaskService.java | 12 +- .../java/pro/taskana/task/api/TaskState.java | 2 +- .../AttachmentPersistenceException.java | 35 +- .../InvalidCallbackStateException.java | 53 +++ .../api/exceptions/InvalidOwnerException.java | 24 +- .../api/exceptions/InvalidStateException.java | 7 +- .../exceptions/InvalidTaskStateException.java | 53 +++ ...MismatchedTaskCommentCreatorException.java | 38 +++ .../exceptions/TaskAlreadyExistException.java | 21 +- .../TaskCommentNotFoundException.java | 19 +- .../api/exceptions/TaskNotFoundException.java | 19 +- .../api/exceptions/UpdateFailedException.java | 11 - .../pro/taskana/task/api/models/Task.java | 2 +- .../task/internal/AttachmentHandler.java | 53 +-- .../task/internal/ServiceLevelHandler.java | 80 ++--- .../task/internal/TaskCommentServiceImpl.java | 25 +- .../pro/taskana/task/internal/TaskMapper.java | 33 +- .../taskana/task/internal/TaskQueryImpl.java | 9 +- .../task/internal/TaskServiceImpl.java | 312 +++++++++--------- .../task/internal/TaskTransferrer.java | 26 +- .../task/internal/jobs/TaskCleanupJob.java | 3 +- .../task/internal/jobs/TaskRefreshJob.java | 3 +- .../workbasket/api/WorkbasketService.java | 44 +-- .../InvalidWorkbasketException.java | 14 - ...smatchedWorkbasketPermissionException.java | 96 ++++++ ...tAuthorizedToQueryWorkbasketException.java | 13 +- ...basketAccessItemAlreadyExistException.java | 28 +- .../WorkbasketAlreadyExistException.java | 30 +- .../exceptions/WorkbasketInUseException.java | 24 +- .../WorkbasketMarkedForDeletionException.java | 29 ++ .../WorkbasketNotFoundException.java | 32 +- ...AbstractWorkbasketAccessItemQueryImpl.java | 3 +- .../WorkbasketAccessItemQueryImpl.java | 3 +- .../internal/WorkbasketQueryImpl.java | 6 +- .../internal/WorkbasketServiceImpl.java | 235 ++++++------- .../internal/jobs/WorkbasketCleanupJob.java | 3 +- .../java/acceptance/ArchitectureTest.java | 57 +++- .../test/java/acceptance/TaskTestMapper.java | 1 - .../CreateClassificationAccTest.java | 5 +- .../UpdateClassificationAccTest.java | 2 +- .../acceptance/task/CallbackStateAccTest.java | 47 +-- .../acceptance/task/CompleteTaskAccTest.java | 25 +- .../acceptance/task/CreateTaskAccTest.java | 3 +- .../acceptance/task/DeleteTaskAccTest.java | 106 ++---- .../QueryTasksByObjectReferenceAccTest.java | 3 +- .../java/acceptance/task/SetOwnerAccTest.java | 26 +- .../acceptance/task/TaskEngineAccTest.java | 3 +- .../acceptance/task/TransferTaskAccTest.java | 16 +- .../acceptance/task/UpdateTaskAccTest.java | 3 +- .../task/UpdateTaskAttachmentsAccTest.java | 3 +- .../workbasket/CreateWorkbasketAccTest.java | 14 +- .../workbasket/DeleteWorkbasketAccTest.java | 29 +- .../GetWorkbasketAuthorizationsAccTest.java | 2 +- .../workbasket/UpdateWorkbasketAccTest.java | 22 +- ...UpdateWorkbasketAuthorizationsAccTest.java | 20 +- .../workbasket/WorkbasketServiceImplTest.java | 3 +- .../pro/taskana/example/ExampleBootstrap.java | 12 +- .../example/TaskanaTestController.java | 8 +- .../example/TaskanaTransactionIntTest.java | 9 +- .../taskana/example/jobs/JobScheduler.java | 59 +--- .../ldap/LdapEmptySearchRootsTest.java | 2 +- .../pro/taskana/example/ldap/LdapTest.java | 2 +- .../src/docs/asciidoc/rest-api.adoc | 75 ++++- .../rest/ClassificationController.java | 9 +- .../ClassificationDefinitionController.java | 29 +- .../rest/TaskanaRestExceptionHandler.java | 284 ++++++++++------ .../taskana/common/rest/ldap/LdapClient.java | 13 +- ...java => ExceptionRepresentationModel.java} | 30 +- .../rest/util/QueryParamsValidator.java | 3 +- .../workbasket/rest/WorkbasketController.java | 15 +- .../rest/WorkbasketDefinitionController.java | 46 +-- .../rest/ClassificationControllerIntTest.java | 7 +- .../common/rest/ExceptionErrorKeyTest.java | 104 ++++++ .../rest/GeneralExceptionHandlingTest.java | 192 +++++++++-- .../rest/TaskCommentControllerIntTest.java | 2 +- .../task/rest/TaskControllerIntTest.java | 42 +-- ...WorkbasketAccessItemControllerIntTest.java | 7 +- .../rest/WorkbasketControllerIntTest.java | 7 +- ...WorkbasketDefinitionControllerIntTest.java | 6 +- 118 files changed, 2239 insertions(+), 1274 deletions(-) create mode 100644 common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ErrorCode.java create mode 100644 common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/MismatchedRoleException.java create mode 100644 common/taskana-common/src/main/java/pro/taskana/common/internal/util/EnumUtil.java create mode 100644 common/taskana-common/src/main/java/pro/taskana/common/internal/util/MapCreator.java create mode 100644 lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/MalformedServiceLevelException.java create mode 100644 lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidCallbackStateException.java create mode 100644 lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidTaskStateException.java create mode 100644 lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/MismatchedTaskCommentCreatorException.java delete mode 100644 lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/UpdateFailedException.java delete mode 100644 lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/InvalidWorkbasketException.java create mode 100644 lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/MismatchedWorkbasketPermissionException.java create mode 100644 lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketMarkedForDeletionException.java rename rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/models/{TaskanaErrorData.java => ExceptionRepresentationModel.java} (70%) create mode 100644 rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/ExceptionErrorKeyTest.java diff --git a/common/taskana-common-logging/src/test/java/pro/taskana/common/internal/logging/LoggingAspectTest.java b/common/taskana-common-logging/src/test/java/pro/taskana/common/internal/logging/LoggingAspectTest.java index 5f5340dbe..ffb250254 100644 --- a/common/taskana-common-logging/src/test/java/pro/taskana/common/internal/logging/LoggingAspectTest.java +++ b/common/taskana-common-logging/src/test/java/pro/taskana/common/internal/logging/LoggingAspectTest.java @@ -135,11 +135,6 @@ class LoggingAspectTest { static class LoggingTestClass { public void logInternalMethod() {} - @SuppressWarnings("UnusedReturnValue") - String logInternalMethodWithReturnValue() { - return "test string"; - } - @SuppressWarnings("UnusedReturnValue") public String logInternalMethodWithReturnValueNull() { return null; @@ -165,6 +160,11 @@ class LoggingAspectTest { @NoLogging public void doNotLogInternalMethod() {} + @SuppressWarnings("UnusedReturnValue") + String logInternalMethodWithReturnValue() { + return "test string"; + } + private void logInternalMethodPrivate() {} } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/BulkOperationResults.java b/common/taskana-common/src/main/java/pro/taskana/common/api/BulkOperationResults.java index 6ba18b3e3..67c0603e1 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/BulkOperationResults.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/BulkOperationResults.java @@ -19,7 +19,7 @@ public class BulkOperationResults { /** * Returning a list of current errors as map. If there are no errors the result will be empty. * - * @return map of errors which can´t be null. + * @return map of errors which can't be null. */ public Map getErrorMap() { return this.errorMap; @@ -82,7 +82,7 @@ public class BulkOperationResults { /** * Map from any exception type to Exception. * - * @return map of errors which can´t be null. + * @return map of errors which can't be null. */ public BulkOperationResults mapBulkOperationResults() { BulkOperationResults bulkLogMapped = new BulkOperationResults<>(); diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/AutocommitFailedException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/AutocommitFailedException.java index 5ac61750f..d25ac5f82 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/AutocommitFailedException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/AutocommitFailedException.java @@ -1,9 +1,13 @@ package pro.taskana.common.api.exceptions; -/** Thrown in ConnectionManagementMode AUTOCOMMIT when an attempt to commit fails. */ +/** + * This exception is thrown in ConnectionManagementMode AUTOCOMMIT when an attempt to commit fails. + */ public class AutocommitFailedException extends TaskanaRuntimeException { + public static final String ERROR_KEY = "CONNECTION_AUTOCOMMIT_FAILED"; + public AutocommitFailedException(Throwable cause) { - super("Autocommit failed", cause); + super("Autocommit failed", ErrorCode.of(ERROR_KEY), cause); } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ConcurrencyException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ConcurrencyException.java index d04688d51..e868eec7f 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ConcurrencyException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ConcurrencyException.java @@ -6,7 +6,11 @@ package pro.taskana.common.api.exceptions; */ public class ConcurrencyException extends TaskanaException { - public ConcurrencyException(String msg) { - super(msg); + public static final String ERROR_KEY = "ENTITY_NOT_UP_TO_DATE"; + + public ConcurrencyException() { + super( + "The current entity cannot be updated since it has been modified while editing.", + ErrorCode.of(ERROR_KEY)); } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ConnectionNotSetException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ConnectionNotSetException.java index 67352b502..730f77029 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ConnectionNotSetException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ConnectionNotSetException.java @@ -1,12 +1,14 @@ package pro.taskana.common.api.exceptions; /** - * Thrown if ConnectionManagementMode is CONNECTION_MANAGED_EXTERNALLY and an attempt is made to - * call an API method before the setConnection() method has been called. + * This exception is thrown when ConnectionManagementMode is CONNECTION_MANAGED_EXTERNALLY and an + * attempt is made to call an API method before the setConnection() method has been called. */ public class ConnectionNotSetException extends TaskanaRuntimeException { + public static final String ERROR_KEY = "CONNECTION_NOT_SET"; + public ConnectionNotSetException() { - super("Connection not set"); + super("Connection not set", ErrorCode.of(ERROR_KEY)); } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/DomainNotFoundException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/DomainNotFoundException.java index 91f30dbdb..ae2ecf307 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/DomainNotFoundException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/DomainNotFoundException.java @@ -1,11 +1,21 @@ package pro.taskana.common.api.exceptions; -/** - * This exception is thrown if a domain name is specified which is not found in the configuration. - */ +import pro.taskana.common.internal.util.MapCreator; + +/** This exception is thrown when the specified domain is not found in the configuration. */ public class DomainNotFoundException extends NotFoundException { - public DomainNotFoundException(String domain, String msg) { - super(domain, msg); + public static final String ERROR_KEY = "DOMAIN_NOT_FOUND"; + private final String domain; + + public DomainNotFoundException(String domain) { + super( + String.format("Domain '%s' does not exist in the configuration", domain), + ErrorCode.of(ERROR_KEY, MapCreator.of("domain", domain))); + this.domain = domain; + } + + public String getDomain() { + return domain; } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ErrorCode.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ErrorCode.java new file mode 100644 index 000000000..f02600acd --- /dev/null +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/ErrorCode.java @@ -0,0 +1,57 @@ +package pro.taskana.common.api.exceptions; + +import java.io.Serializable; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +public class ErrorCode implements Serializable { + private final String key; + // Unfortunately this is necessary. The Map interface does not implement Serializable.. + private final HashMap messageVariables; + + private ErrorCode(String key, Map messageVariables) { + this.key = key; + this.messageVariables = new HashMap<>(messageVariables); + } + + public static ErrorCode of(String key, Map messageVariables) { + return new ErrorCode(key, messageVariables); + } + + public static ErrorCode of(String key) { + return new ErrorCode(key, Collections.emptyMap()); + } + + public String getKey() { + return key; + } + + public Map getMessageVariables() { + return messageVariables; + } + + @Override + public int hashCode() { + return Objects.hash(key, messageVariables); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof ErrorCode)) { + return false; + } + ErrorCode other = (ErrorCode) obj; + return Objects.equals(key, other.key) + && Objects.equals(messageVariables, other.messageVariables); + } + + @Override + public String toString() { + return "ErrorCode [key=" + key + ", messageVariables=" + messageVariables + "]"; + } +} diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/InvalidArgumentException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/InvalidArgumentException.java index 3fa028272..88673f781 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/InvalidArgumentException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/InvalidArgumentException.java @@ -1,13 +1,13 @@ package pro.taskana.common.api.exceptions; -/** This exception is thrown when a method is called with invalid argument. */ +/** This exception is thrown when a method is called with an invalid argument. */ public class InvalidArgumentException extends TaskanaException { public InvalidArgumentException(String msg) { - super(msg); + this(msg, null); } public InvalidArgumentException(String msg, Throwable cause) { - super(msg, cause); + super(msg, ErrorCode.of("INVALID_ARGUMENT"), cause); } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/MismatchedRoleException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/MismatchedRoleException.java new file mode 100644 index 000000000..0bdaefa88 --- /dev/null +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/MismatchedRoleException.java @@ -0,0 +1,36 @@ +package pro.taskana.common.api.exceptions; + +import java.util.Arrays; + +import pro.taskana.common.api.TaskanaRole; +import pro.taskana.common.internal.util.MapCreator; + +/** + * This exception is thrown when the current user is not in a certain {@linkplain TaskanaRole role} + * it is supposed to be. + */ +public class MismatchedRoleException extends NotAuthorizedException { + + public static final String ERROR_KEY = "ROLE_MISMATCHED"; + private final String currentUserId; + private final TaskanaRole[] roles; + + public MismatchedRoleException(String currentUserId, TaskanaRole... roles) { + super( + String.format( + "Not authorized. The current user '%s' is not member of role(s) '%s'.", + currentUserId, Arrays.toString(roles)), + ErrorCode.of(ERROR_KEY, MapCreator.of("roles", roles, "currentUserId", currentUserId))); + + this.currentUserId = currentUserId; + this.roles = roles; + } + + public TaskanaRole[] getRoles() { + return roles; + } + + public String getCurrentUserId() { + return currentUserId; + } +} diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/NotAuthorizedException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/NotAuthorizedException.java index 90fc3d2ea..49ddbf23a 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/NotAuthorizedException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/NotAuthorizedException.java @@ -1,16 +1,8 @@ package pro.taskana.common.api.exceptions; -/** This exception is used to communicate a not authorized user. */ +/** This exception is thrown when a user is not authorized. */ public class NotAuthorizedException extends TaskanaException { - - private final String currentUserId; - - public NotAuthorizedException(String msg, String currentUserId) { - super(msg + " - [CURRENT USER: {'" + currentUserId + "'}]"); - this.currentUserId = currentUserId; - } - - public String getCurrentUserId() { - return currentUserId; + protected NotAuthorizedException(String msg, ErrorCode errorCode) { + super(msg, errorCode); } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/NotFoundException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/NotFoundException.java index 451510d84..766ef1e33 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/NotFoundException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/NotFoundException.java @@ -1,16 +1,9 @@ package pro.taskana.common.api.exceptions; -/** This exception will be thrown if a specific object is not in the database. */ +/** This exception is thrown when a specific object is not in the database. */ public class NotFoundException extends TaskanaException { - private final String id; - - public NotFoundException(String id, String message) { - super(message); - this.id = id; - } - - public String getId() { - return id; + protected NotFoundException(String message, ErrorCode errorCode) { + super(message, errorCode); } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/SystemException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/SystemException.java index d9e46ac0c..924be18f4 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/SystemException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/SystemException.java @@ -1,13 +1,15 @@ package pro.taskana.common.api.exceptions; -/** This exception is thrown when a generic taskana problem is encountered. */ +/** This exception is thrown when a generic TASKANA problem is encountered. */ public class SystemException extends TaskanaRuntimeException { + public static final String ERROR_KEY = "CRITICAL_SYSTEM_ERROR"; + public SystemException(String msg) { - super(msg); + this(msg, null); } public SystemException(String msg, Throwable cause) { - super(msg, cause); + super(msg, ErrorCode.of(ERROR_KEY), cause); } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/TaskanaException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/TaskanaException.java index 28c7ef836..d4499599d 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/TaskanaException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/TaskanaException.java @@ -3,24 +3,18 @@ package pro.taskana.common.api.exceptions; /** common base class for TASKANA's checked exceptions. */ public class TaskanaException extends Exception { - public TaskanaException() { - super(); + private final ErrorCode errorCode; + + protected TaskanaException(String message, ErrorCode errorCode) { + this(message, errorCode, null); } - public TaskanaException(String message) { - super(message); - } - - public TaskanaException(Throwable cause) { - super(cause); - } - - public TaskanaException(String message, Throwable cause) { + protected TaskanaException(String message, ErrorCode errorCode, Throwable cause) { super(message, cause); + this.errorCode = errorCode; } - public TaskanaException( - String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) { - super(message, cause, enableSuppression, writableStackTrace); + public ErrorCode getErrorCode() { + return errorCode; } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/TaskanaRuntimeException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/TaskanaRuntimeException.java index 7891b5381..0bbf23ba8 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/TaskanaRuntimeException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/TaskanaRuntimeException.java @@ -1,26 +1,25 @@ package pro.taskana.common.api.exceptions; -/** Common base class for Taskana's runtime exceptions. */ +/** The common base class for TASKANA's runtime exceptions. */ public class TaskanaRuntimeException extends RuntimeException { - public TaskanaRuntimeException() { - super(); + private final ErrorCode errorCode; + + protected TaskanaRuntimeException(String message, ErrorCode errorCode) { + this(message, errorCode, null); } - public TaskanaRuntimeException(String message) { - super(message); - } - - public TaskanaRuntimeException(Throwable cause) { - super(cause); - } - - public TaskanaRuntimeException(String message, Throwable cause) { + protected TaskanaRuntimeException(String message, ErrorCode errorCode, Throwable cause) { super(message, cause); + this.errorCode = errorCode; } - public TaskanaRuntimeException( - String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) { - super(message, cause, enableSuppression, writableStackTrace); + public ErrorCode getErrorCode() { + return errorCode; + } + + @Override + public String toString() { + return "TaskanaRuntimeException [errorCode=" + errorCode + "]"; } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/UnsupportedDatabaseException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/UnsupportedDatabaseException.java index 9f776b571..e16c42422 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/UnsupportedDatabaseException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/UnsupportedDatabaseException.java @@ -1,11 +1,23 @@ package pro.taskana.common.api.exceptions; -/** - * This exception will be thrown if the database name doesn't match to one of the desired databases. - */ -public class UnsupportedDatabaseException extends RuntimeException { +import pro.taskana.common.internal.util.MapCreator; - public UnsupportedDatabaseException(String name) { - super("Database with '" + name + "' not found"); +/** + * This exception is thrown when the database name doesn't match to one of the desired databases. + */ +public class UnsupportedDatabaseException extends TaskanaRuntimeException { + + public static final String ERROR_KEY = "DATABASE_UNSUPPORTED"; + private final String databaseProductName; + + public UnsupportedDatabaseException(String databaseProductName) { + super( + String.format("Database '%s' is not supported", databaseProductName), + ErrorCode.of(ERROR_KEY, MapCreator.of("databaseProductName", databaseProductName))); + this.databaseProductName = databaseProductName; + } + + public String getDatabaseProductName() { + return databaseProductName; } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/WrongCustomHolidayFormatException.java b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/WrongCustomHolidayFormatException.java index 0c5863adc..fc708b27f 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/WrongCustomHolidayFormatException.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/api/exceptions/WrongCustomHolidayFormatException.java @@ -1,8 +1,25 @@ package pro.taskana.common.api.exceptions; +import pro.taskana.common.api.CustomHoliday; +import pro.taskana.common.internal.util.MapCreator; + +/** This exception is thrown when an entry for the {@linkplain CustomHoliday} has a wrong format. */ public class WrongCustomHolidayFormatException extends TaskanaException { - public WrongCustomHolidayFormatException(String message) { - super(message); + public static final String ERROR_KEY = "CUSTOM_HOLIDAY_WRONG_FORMAT"; + private final String customHoliday; + + public WrongCustomHolidayFormatException(String customHoliday) { + super( + String.format( + "Wrong format for custom holiday entry '%s'! The format should be 'dd.MM' " + + "i.e. 01.05 for the first of May.", + customHoliday), + ErrorCode.of(ERROR_KEY, MapCreator.of("customHoliday", customHoliday))); + this.customHoliday = customHoliday; + } + + public String getCustomHoliday() { + return customHoliday; } } diff --git a/common/taskana-common/src/main/java/pro/taskana/common/internal/configuration/DbSchemaCreator.java b/common/taskana-common/src/main/java/pro/taskana/common/internal/configuration/DbSchemaCreator.java index ec0a77498..a16f639fc 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/internal/configuration/DbSchemaCreator.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/internal/configuration/DbSchemaCreator.java @@ -96,8 +96,8 @@ public class DbSchemaCreator { Map queryResult = runner.selectOne(query); - ComparableVersion actualVersion = new ComparableVersion((String) queryResult.get("VERSION")); - ComparableVersion minVersion = new ComparableVersion(expectedMinVersion); + ComparableVersion actualVersion = ComparableVersion.of((String) queryResult.get("VERSION")); + ComparableVersion minVersion = ComparableVersion.of(expectedMinVersion); if (actualVersion.compareTo(minVersion) < 0) { LOGGER.error( diff --git a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ComparableVersion.java b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ComparableVersion.java index 04f35dca7..c07d4c201 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ComparableVersion.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ComparableVersion.java @@ -75,10 +75,14 @@ public class ComparableVersion implements Comparable { private ListItem items; - public ComparableVersion(String version) { + private ComparableVersion(String version) { parseVersion(version); } + public static ComparableVersion of(String version) { + return new ComparableVersion(version); + } + public final void parseVersion(String version) { this.value = version; @@ -226,7 +230,7 @@ public class ComparableVersion implements Comparable { this.value = 0; } - IntItem(String str) { + private IntItem(String str) { this.value = Integer.parseInt(str); } @@ -294,7 +298,7 @@ public class ComparableVersion implements Comparable { private static class LongItem implements Item { private final long value; - LongItem(String str) { + private LongItem(String str) { this.value = Long.parseLong(str); } @@ -363,7 +367,7 @@ public class ComparableVersion implements Comparable { private static class BigIntegerItem implements Item { private final BigInteger value; - BigIntegerItem(String str) { + private BigIntegerItem(String str) { this.value = new BigInteger(str); } @@ -447,7 +451,7 @@ public class ComparableVersion implements Comparable { private final String value; - StringItem(String value, boolean followedByDigit) { + private StringItem(String value, boolean followedByDigit) { if (followedByDigit && value.length() == 1) { // a1 = alpha-1, b1 = beta-1, m1 = milestone-1 switch (value.charAt(0)) { @@ -549,6 +553,9 @@ public class ComparableVersion implements Comparable { * sub-lists (which start with '-(number)' in the version specification). */ private static class ListItem extends ArrayList implements Item { + + private ListItem() {} + @Override public int getType() { return LIST_ITEM; diff --git a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/EnumUtil.java b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/EnumUtil.java new file mode 100644 index 000000000..37578081b --- /dev/null +++ b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/EnumUtil.java @@ -0,0 +1,22 @@ +package pro.taskana.common.internal.util; + +import java.lang.reflect.Array; +import java.util.Arrays; +import java.util.EnumSet; + +public class EnumUtil { + + private EnumUtil() { + throw new IllegalStateException("Utility class"); + } + + @SafeVarargs + public static > E[] allValuesExceptFor(E... values) { + if (values == null || values.length == 0) { + throw new IllegalArgumentException("values must be present"); + } + @SuppressWarnings("unchecked") + E[] array = (E[]) Array.newInstance(values[0].getClass(), 0); + return EnumSet.complementOf(EnumSet.copyOf(Arrays.asList(values))).toArray(array); + } +} diff --git a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/MapCreator.java b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/MapCreator.java new file mode 100644 index 000000000..114e18e91 --- /dev/null +++ b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/MapCreator.java @@ -0,0 +1,57 @@ +package pro.taskana.common.internal.util; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/* This class will be removed once we fully migrate from JDK8*/ +public class MapCreator { + + private MapCreator() { + throw new IllegalStateException("Utility class"); + } + + public static Map of() { + return Collections.emptyMap(); + } + + public static Map of(K k1, V v1) { + Map map = new HashMap<>(); + map.put(k1, v1); + return map; + } + + public static Map of(K k1, V v1, K k2, V v2) { + Map map = new HashMap<>(); + map.put(k1, v1); + map.put(k2, v2); + return map; + } + + public static Map of(K k1, V v1, K k2, V v2, K k3, V v3) { + Map map = new HashMap<>(); + map.put(k1, v1); + map.put(k2, v2); + map.put(k3, v3); + return map; + } + + public static Map of(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4) { + Map map = new HashMap<>(); + map.put(k1, v1); + map.put(k2, v2); + map.put(k3, v3); + map.put(k4, v4); + return map; + } + + public static Map of(K k1, V v1, K k2, V v2, K k3, V v3, K k4, V v4, K k5, V v5) { + Map map = new HashMap<>(); + map.put(k1, v1); + map.put(k2, v2); + map.put(k3, v3); + map.put(k4, v4); + map.put(k5, v5); + return map; + } +} diff --git a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ObjectAttributeChangeDetector.java b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ObjectAttributeChangeDetector.java index 69bf514e6..7ad14abcc 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ObjectAttributeChangeDetector.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ObjectAttributeChangeDetector.java @@ -15,7 +15,9 @@ import pro.taskana.common.api.exceptions.SystemException; public class ObjectAttributeChangeDetector { - private ObjectAttributeChangeDetector() {} + private ObjectAttributeChangeDetector() { + throw new IllegalStateException("Utility class"); + } /** * Determines changes in fields between two objects. diff --git a/history/taskana-loghistory-provider/src/main/java/pro/taskana/loghistory/impl/LogfileHistoryServiceImpl.java b/history/taskana-loghistory-provider/src/main/java/pro/taskana/loghistory/impl/LogfileHistoryServiceImpl.java index f743ce30a..f020cc894 100644 --- a/history/taskana-loghistory-provider/src/main/java/pro/taskana/loghistory/impl/LogfileHistoryServiceImpl.java +++ b/history/taskana-loghistory-provider/src/main/java/pro/taskana/loghistory/impl/LogfileHistoryServiceImpl.java @@ -90,7 +90,6 @@ public class LogfileHistoryServiceImpl implements TaskanaHistory { @Override public void deleteHistoryEventsByTaskIds(List taskIds) { - throw new UnsupportedOperationException("HistoryLogger is not supposed to delete events"); } diff --git a/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/SimpleHistoryServiceImpl.java b/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/SimpleHistoryServiceImpl.java index 1183cebb8..8c0937656 100644 --- a/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/SimpleHistoryServiceImpl.java +++ b/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/SimpleHistoryServiceImpl.java @@ -108,9 +108,7 @@ public class SimpleHistoryServiceImpl implements TaskanaHistory { try { taskanaHistoryEngine.openConnection(); - taskHistoryEventMapper.deleteMultipleByTaskIds(taskIds); - } catch (SQLException e) { LOGGER.error("Caught exception while trying to delete history events", e); } finally { @@ -126,9 +124,7 @@ public class SimpleHistoryServiceImpl implements TaskanaHistory { resultEvent = taskHistoryEventMapper.findById(historyEventId); if (resultEvent == null) { - throw new TaskanaHistoryEventNotFoundException( - historyEventId, - String.format("TaskHistoryEvent for id %s was not found", historyEventId)); + throw new TaskanaHistoryEventNotFoundException(historyEventId); } return resultEvent; diff --git a/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/TaskanaHistoryEngineImpl.java b/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/TaskanaHistoryEngineImpl.java index 8a27943a9..afed17b10 100644 --- a/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/TaskanaHistoryEngineImpl.java +++ b/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/TaskanaHistoryEngineImpl.java @@ -23,6 +23,7 @@ import org.slf4j.LoggerFactory; import pro.taskana.TaskanaEngineConfiguration; import pro.taskana.common.api.TaskanaEngine; import pro.taskana.common.api.TaskanaRole; +import pro.taskana.common.api.exceptions.MismatchedRoleException; import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.internal.persistence.InstantTypeHandler; import pro.taskana.common.internal.persistence.MapTypeHandler; @@ -91,9 +92,7 @@ public class TaskanaHistoryEngineImpl implements TaskanaHistoryEngine { taskanaEngine.getCurrentUserContext().getAccessIds(), Arrays.toString(roles)); } - throw new NotAuthorizedException( - "current user is not member of role(s) " + Arrays.toString(roles), - taskanaEngine.getCurrentUserContext().getUserid()); + throw new MismatchedRoleException(taskanaEngine.getCurrentUserContext().getUserid(), roles); } } diff --git a/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/jobs/HistoryCleanupJob.java b/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/jobs/HistoryCleanupJob.java index 2c3e00560..d67ecbbbc 100644 --- a/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/jobs/HistoryCleanupJob.java +++ b/history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/jobs/HistoryCleanupJob.java @@ -135,7 +135,7 @@ public class HistoryCleanupJob extends AbstractTaskanaJob { LOGGER.info( "Job ended successfully. {} history events deleted.", totalNumberOfHistoryEventsDeleted); } catch (Exception e) { - throw new TaskanaException("Error while processing HistoryCleanupJob.", e); + throw new SystemException("Error while processing HistoryCleanupJob.", e); } finally { scheduleNextCleanupJob(); } diff --git a/history/taskana-simplehistory-rest-spring/src/test/java/pro/taskana/simplehistory/rest/TaskHistoryEventControllerIntTest.java b/history/taskana-simplehistory-rest-spring/src/test/java/pro/taskana/simplehistory/rest/TaskHistoryEventControllerIntTest.java index 9e5264d50..3bc408089 100644 --- a/history/taskana-simplehistory-rest-spring/src/test/java/pro/taskana/simplehistory/rest/TaskHistoryEventControllerIntTest.java +++ b/history/taskana-simplehistory-rest-spring/src/test/java/pro/taskana/simplehistory/rest/TaskHistoryEventControllerIntTest.java @@ -20,6 +20,7 @@ import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.util.UriComponentsBuilder; import pro.taskana.common.rest.models.PageMetadata; @@ -270,11 +271,11 @@ class TaskHistoryEventControllerIntTest { TASK_HISTORY_EVENT_PAGED_REPRESENTATION_MODEL_TYPE); assertThatThrownBy(httpCall) - .isInstanceOf(HttpClientErrorException.class) + .isInstanceOf(HttpServerErrorException.class) .hasMessageContaining( "Unkown request parameters found: [anotherIllegalParam, illegalParam]") - .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) - .isEqualTo(HttpStatus.BAD_REQUEST); + .extracting(ex -> ((HttpServerErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); } // endregion diff --git a/lib/taskana-core/src/main/java/pro/taskana/TaskanaEngineConfiguration.java b/lib/taskana-core/src/main/java/pro/taskana/TaskanaEngineConfiguration.java index 38bf7fd4a..d3d4118be 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/TaskanaEngineConfiguration.java +++ b/lib/taskana-core/src/main/java/pro/taskana/TaskanaEngineConfiguration.java @@ -551,11 +551,7 @@ public class TaskanaEngineConfiguration { if (parts.size() == 2) { return CustomHoliday.of(Integer.valueOf(parts.get(0)), Integer.valueOf(parts.get(1))); } - throw new WrongCustomHolidayFormatException( - String.format( - "Wrong format for custom holiday entry %s! The format should be 'dd.MM' " - + "i.e. 01.05 for the first of may. The value will be ignored!", - customHolidayEntry)); + throw new WrongCustomHolidayFormatException(customHolidayEntry); } private List splitStringAndTrimElements(String str, String separator) { diff --git a/lib/taskana-core/src/main/java/pro/taskana/classification/api/ClassificationService.java b/lib/taskana-core/src/main/java/pro/taskana/classification/api/ClassificationService.java index 33529ba58..9539cc818 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/classification/api/ClassificationService.java +++ b/lib/taskana-core/src/main/java/pro/taskana/classification/api/ClassificationService.java @@ -3,6 +3,7 @@ package pro.taskana.classification.api; import pro.taskana.classification.api.exceptions.ClassificationAlreadyExistException; import pro.taskana.classification.api.exceptions.ClassificationInUseException; import pro.taskana.classification.api.exceptions.ClassificationNotFoundException; +import pro.taskana.classification.api.exceptions.MalformedServiceLevelException; import pro.taskana.classification.api.models.Classification; import pro.taskana.common.api.exceptions.ConcurrencyException; import pro.taskana.common.api.exceptions.DomainNotFoundException; @@ -87,28 +88,30 @@ public interface ClassificationService { * @throws NotAuthorizedException if the current user is not member of role BUSINESS_ADMIN or * ADMIN * @throws DomainNotFoundException if the {@code domain} does not exist in the configuration - * @throws InvalidArgumentException if the {@code serviceLevel} property does not comply with the - * ISO 8601 specification + * @throws MalformedServiceLevelException if the {@code serviceLevel} property does not comply + * with the ISO 8601 specification + * @throws InvalidArgumentException if the {@linkplain Classification} contains invalid properties */ Classification createClassification(Classification classification) throws ClassificationAlreadyExistException, NotAuthorizedException, DomainNotFoundException, - InvalidArgumentException; + InvalidArgumentException, MalformedServiceLevelException; /** * Updates a Classification. * * @param classification the Classification to update * @return the updated Classification. - * @throws ClassificationNotFoundException if the classification OR it´s parent does not exist. + * @throws ClassificationNotFoundException if the classification OR it's parent does not exist. * @throws NotAuthorizedException if the caller got no ADMIN or BUSINESS_ADMIN permissions. * @throws ConcurrencyException If the classification was modified in the meantime and is not the * most up to date anymore. - * @throws InvalidArgumentException if the ServiceLevel property does not comply with the ISO 8601 - * specification + * @throws MalformedServiceLevelException if the {@code serviceLevel} property does not comply + * with the ISO 8601 specification + * @throws InvalidArgumentException if the {@linkplain Classification} contains invalid properties */ Classification updateClassification(Classification classification) throws ClassificationNotFoundException, NotAuthorizedException, ConcurrencyException, - InvalidArgumentException; + InvalidArgumentException, MalformedServiceLevelException; /** * This method provides a query builder for querying the database. diff --git a/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationAlreadyExistException.java b/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationAlreadyExistException.java index 41fd9cbb9..e1760802c 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationAlreadyExistException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationAlreadyExistException.java @@ -1,17 +1,38 @@ package pro.taskana.classification.api.exceptions; import pro.taskana.classification.api.models.Classification; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; -/** Thrown, when a classification does already exits, but wanted to create with same ID+domain. */ +/** + * This exception is thrown when a {@linkplain Classification} does already exits, but was tried to + * be created with the same {@linkplain Classification#getId() id} and {@linkplain + * Classification#getDomain() domain}. + */ public class ClassificationAlreadyExistException extends TaskanaException { + public static final String ERROR_KEY = "CLASSIFICATION_ALREADY_EXISTS"; + private final String domain; + private final String classificationKey; + public ClassificationAlreadyExistException(Classification classification) { + this(classification.getKey(), classification.getDomain()); + } + + public ClassificationAlreadyExistException(String key, String domain) { super( - "A classification with key '" - + classification.getKey() - + "' already exists in domain '" - + classification.getDomain() - + "'."); + String.format("A Classification with key '%s' already exists in domain '%s'.", key, domain), + ErrorCode.of(ERROR_KEY, MapCreator.of("classificationKey", key, "domain", domain))); + classificationKey = key; + this.domain = domain; + } + + public String getDomain() { + return domain; + } + + public String getClassificationKey() { + return classificationKey; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationInUseException.java b/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationInUseException.java index fee163c75..b42e26d0d 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationInUseException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationInUseException.java @@ -1,15 +1,48 @@ package pro.taskana.classification.api.exceptions; +import pro.taskana.classification.api.models.Classification; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.task.api.models.Attachment; +import pro.taskana.task.api.models.Task; -/** Thrown if a specific task is not in the database. */ +/** + * This exception is thrown when a specific {@linkplain Classification} was tried to be deleted + * while still being in use.
+ * This could mean that there are either {@linkplain Task Tasks} or {@linkplain Attachment + * Attachments} associated with it. + */ public class ClassificationInUseException extends TaskanaException { - public ClassificationInUseException(String msg) { - super(msg); + public static final String ERROR_KEY = "CLASSIFICATION_IN_USE"; + private final String classificationKey; + private final String domain; + + public ClassificationInUseException(Classification classification, Throwable cause) { + super( + String.format( + "The Classification with id = '%s' and key = '%s' in domain = '%s' " + + "is in use and cannot be deleted. There are either Tasks or " + + "Attachments associated with the Classification.", + classification.getId(), classification.getKey(), classification.getDomain()), + ErrorCode.of( + ERROR_KEY, + MapCreator.of( + "classificationKey", + classification.getKey(), + "domain", + classification.getDomain())), + cause); + classificationKey = classification.getKey(); + domain = classification.getDomain(); } - public ClassificationInUseException(String msg, Throwable cause) { - super(msg, cause); + public String getClassificationKey() { + return classificationKey; + } + + public String getDomain() { + return domain; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationNotFoundException.java b/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationNotFoundException.java index b4e16c39d..2420c2a69 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationNotFoundException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/ClassificationNotFoundException.java @@ -1,28 +1,48 @@ package pro.taskana.classification.api.exceptions; +import pro.taskana.classification.api.models.Classification; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.NotFoundException; +import pro.taskana.common.internal.util.MapCreator; -/** Thrown if a specific task is not in the database. */ +/** Thrown if a specific {@linkplain Classification} is not in the database. */ public class ClassificationNotFoundException extends NotFoundException { - private String key; - private String domain; + public static final String ERROR_KEY_ID = "CLASSIFICATION_WITH_ID_NOT_FOUND"; + public static final String ERROR_KEY_KEY_DOMAIN = "CLASSIFICATION_WITH_KEY_NOT_FOUND"; + private final String classificationId; + private final String classificationKey; + private final String domain; - public ClassificationNotFoundException(String id, String msg) { - super(id, msg); + public ClassificationNotFoundException(String classificationId) { + super( + String.format("Classification with id '%s' wasn't found", classificationId), + ErrorCode.of(ERROR_KEY_ID, MapCreator.of("classificationId", classificationId))); + this.classificationId = classificationId; + classificationKey = null; + domain = null; } - public ClassificationNotFoundException(String key, String domain, String msg) { - super(null, msg); - this.key = key; + public ClassificationNotFoundException(String key, String domain) { + super( + String.format( + "Classification with key '%s' and domain '%s' could not be found", key, domain), + ErrorCode.of( + ERROR_KEY_KEY_DOMAIN, MapCreator.of("classificationKey", key, "domain", domain))); + this.classificationKey = key; this.domain = domain; + classificationId = null; } - public String getKey() { - return key; + public String getClassificationKey() { + return classificationKey; } public String getDomain() { return domain; } + + public String getClassificationId() { + return classificationId; + } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/MalformedServiceLevelException.java b/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/MalformedServiceLevelException.java new file mode 100644 index 000000000..8b8732f88 --- /dev/null +++ b/lib/taskana-core/src/main/java/pro/taskana/classification/api/exceptions/MalformedServiceLevelException.java @@ -0,0 +1,56 @@ +package pro.taskana.classification.api.exceptions; + +import pro.taskana.classification.api.models.Classification; +import pro.taskana.common.api.exceptions.ErrorCode; +import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; + +/** + * This exception is thrown when the {@linkplain Classification#getServiceLevel() service level} of + * the {@linkplain Classification} has not the required format. The {@linkplain + * Classification#getServiceLevel() service level} has to be a positive ISO-8601 duration format and + * TASKANA only supports whole days. The format must be 'PnD'. + */ +public class MalformedServiceLevelException extends TaskanaException { + + public static final String ERROR_KEY = "CLASSIFICATION_SERVICE_LEVEL_MALFORMED"; + private final String serviceLevel; + private final String classificationKey; + private final String domain; + + public MalformedServiceLevelException( + String serviceLevel, String classificationKey, String domain) { + super( + String.format( + "The provided service level '%s' of the " + + "Classification with key '%s' and domain '%s' is invalid." + + "The service level has to be a positive ISO-8601 duration format. " + + "Furthermore, TASKANA only supports whole days; " + + "the service level must be in the format 'PnD'", + serviceLevel, classificationKey, domain), + ErrorCode.of( + ERROR_KEY, + MapCreator.of( + "classificationKey", + classificationKey, + "domain", + domain, + "serviceLevel", + serviceLevel))); + this.serviceLevel = serviceLevel; + this.classificationKey = classificationKey; + this.domain = domain; + } + + public String getServiceLevel() { + return serviceLevel; + } + + public String getClassificationKey() { + return classificationKey; + } + + public String getDomain() { + return domain; + } +} diff --git a/lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationQueryImpl.java index de015dfbc..124f524e0 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationQueryImpl.java @@ -338,7 +338,7 @@ public class ClassificationQueryImpl implements ClassificationQuery { } catch (PersistenceException e) { if (e.getMessage().contains("ERRORCODE=-4470")) { TaskanaRuntimeException ex = - new TaskanaRuntimeException( + new SystemException( "The offset beginning was set over the amount of result-rows.", e.getCause()); ex.setStackTrace(e.getStackTrace()); throw ex; diff --git a/lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationServiceImpl.java index bafeb5ba2..97b86cf6b 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationServiceImpl.java @@ -18,6 +18,7 @@ import pro.taskana.classification.api.ClassificationService; import pro.taskana.classification.api.exceptions.ClassificationAlreadyExistException; import pro.taskana.classification.api.exceptions.ClassificationInUseException; import pro.taskana.classification.api.exceptions.ClassificationNotFoundException; +import pro.taskana.classification.api.exceptions.MalformedServiceLevelException; import pro.taskana.classification.api.models.Classification; import pro.taskana.classification.api.models.ClassificationSummary; import pro.taskana.classification.internal.jobs.ClassificationChangedJob; @@ -62,19 +63,17 @@ public class ClassificationServiceImpl implements ClassificationService { public Classification getClassification(String key, String domain) throws ClassificationNotFoundException { if (key == null) { - throw new ClassificationNotFoundException( - null, domain, "Classification for null key and domain " + domain + " was not found."); + throw new ClassificationNotFoundException(null, domain); } - Classification result = null; + Classification result; try { taskanaEngine.openConnection(); result = classificationMapper.findByKeyAndDomain(key, domain); if (result == null) { result = classificationMapper.findByKeyAndDomain(key, ""); if (result == null) { - throw new ClassificationNotFoundException( - key, domain, "Classification for key = " + key + " and master domain was not found"); + throw new ClassificationNotFoundException(key, domain); } } return result; @@ -86,15 +85,14 @@ public class ClassificationServiceImpl implements ClassificationService { @Override public Classification getClassification(String id) throws ClassificationNotFoundException { if (id == null) { - throw new ClassificationNotFoundException(null, "Classification for null id is invalid."); + throw new ClassificationNotFoundException(null); } - Classification result = null; + Classification result; try { taskanaEngine.openConnection(); result = classificationMapper.findById(id); if (result == null) { - throw new ClassificationNotFoundException( - id, "Classification for id " + id + " was not found"); + throw new ClassificationNotFoundException(id); } return result; } finally { @@ -110,8 +108,7 @@ public class ClassificationServiceImpl implements ClassificationService { taskanaEngine.openConnection(); Classification classification = this.classificationMapper.findById(classificationId); if (classification == null) { - throw new ClassificationNotFoundException( - classificationId, "The classification \"" + classificationId + "\" wasn't found"); + throw new ClassificationNotFoundException(classificationId); } if (classification.getDomain().equals("")) { @@ -150,13 +147,7 @@ public class ClassificationServiceImpl implements ClassificationService { } catch (PersistenceException e) { if (isReferentialIntegrityConstraintViolation(e)) { - throw new ClassificationInUseException( - String.format( - "The classification id = \"%s\" and key = \"%s\" in domain = \"%s\" " - + "is in use and cannot be deleted. There are either tasks or " - + "attachments associated with the classification.", - classificationId, classification.getKey(), classification.getDomain()), - e.getCause()); + throw new ClassificationInUseException(classification, e); } } } finally { @@ -173,13 +164,7 @@ public class ClassificationServiceImpl implements ClassificationService { Classification classification = this.classificationMapper.findByKeyAndDomain(classificationKey, domain); if (classification == null) { - throw new ClassificationNotFoundException( - classificationKey, - domain, - "The classification \"" - + classificationKey - + "\" wasn't found in the domain " - + domain); + throw new ClassificationNotFoundException(classificationKey, domain); } deleteClassification(classification.getId()); } finally { @@ -190,13 +175,11 @@ public class ClassificationServiceImpl implements ClassificationService { @Override public Classification createClassification(Classification classification) throws ClassificationAlreadyExistException, NotAuthorizedException, DomainNotFoundException, - InvalidArgumentException { + InvalidArgumentException, MalformedServiceLevelException { taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); if (!taskanaEngine.domainExists(classification.getDomain()) && !"".equals(classification.getDomain())) { - throw new DomainNotFoundException( - classification.getDomain(), - "Domain " + classification.getDomain() + " does not exist in the configuration."); + throw new DomainNotFoundException(classification.getDomain()); } ClassificationImpl classificationImpl; final boolean isClassificationExisting; @@ -246,14 +229,16 @@ public class ClassificationServiceImpl implements ClassificationService { @Override public Classification updateClassification(Classification classification) throws NotAuthorizedException, ConcurrencyException, ClassificationNotFoundException, - InvalidArgumentException { + InvalidArgumentException, MalformedServiceLevelException { taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); ClassificationImpl classificationImpl; try { taskanaEngine.openConnection(); if (classification.getKey().equals(classification.getParentKey())) { throw new InvalidArgumentException( - "The classification " + classification.getName() + " has the same key and parentKey"); + String.format( + "The Classification '%s' has the same key and parent key", + classification.getName())); } classificationImpl = (ClassificationImpl) classification; @@ -306,36 +291,26 @@ public class ClassificationServiceImpl implements ClassificationService { return classification; } - private static void validateServiceLevel(String serviceLevel) throws InvalidArgumentException { + private static void validateServiceLevel(Classification classification) + throws MalformedServiceLevelException { + String serviceLevel = classification.getServiceLevel(); Duration duration; try { duration = Duration.parse(serviceLevel); } catch (Exception e) { - throw new InvalidArgumentException( - String.format( - "Invalid service level %s. " - + "The formats accepted are based on the ISO-8601 duration format " - + "PnDTnHnMn.nS with days considered to be exactly 24 hours. " - + "For example: \"P2D\" represents a period of \"two days.\" ", - serviceLevel), - e.getCause()); + throw new MalformedServiceLevelException( + serviceLevel, classification.getKey(), classification.getDomain()); } if (duration.isNegative()) { - throw new InvalidArgumentException( - String.format( - "Invalid service level %s. Taskana does not support a negative service level.", - serviceLevel)); + throw new MalformedServiceLevelException( + serviceLevel, classification.getKey(), classification.getDomain()); } // check that the duration is based on format PnD, i.e. it must start with a P, end with a D if (!serviceLevel.toLowerCase().startsWith("p") || !serviceLevel.toLowerCase().endsWith("d")) { - throw new InvalidArgumentException( - String.format( - "Invalid service level %s. Taskana only supports service " - + "levels that contain a number of whole days " - + "specified according to the format ''PnD'' where n is the number of days", - serviceLevel)); + throw new MalformedServiceLevelException( + serviceLevel, classification.getKey(), classification.getDomain()); } } @@ -423,11 +398,13 @@ public class ClassificationServiceImpl implements ClassificationService { * Fill missing values and validate classification before saving the classification. * * @param classification the classification which will be verified. - * @throws InvalidArgumentException if the given classification has no key, the service level is - * null, the type is not valid or the category for the provided type is invalid. + * @throws InvalidArgumentException if the given classification has no key, the type is not valid + * or the category for the provided type is invalid. + * @throws MalformedServiceLevelException if the given service level of the {@linkplain + * Classification} is invalid */ private void initDefaultClassificationValues(ClassificationImpl classification) - throws InvalidArgumentException { + throws InvalidArgumentException, MalformedServiceLevelException { Instant now = Instant.now(); if (classification.getId() == null || "".equals(classification.getId())) { classification.setId(IdGenerator.generateWithPrefix(IdGenerator.ID_PREFIX_CLASSIFICATION)); @@ -448,7 +425,7 @@ public class ClassificationServiceImpl implements ClassificationService { if (classification.getServiceLevel() == null || classification.getServiceLevel().isEmpty()) { classification.setServiceLevel("P0D"); } else { - validateServiceLevel(classification.getServiceLevel()); + validateServiceLevel(classification); } if (classification.getKey() == null) { @@ -538,10 +515,7 @@ public class ClassificationServiceImpl implements ClassificationService { Classification oldClassification = this.getClassification(classificationImpl.getKey(), classificationImpl.getDomain()); if (!oldClassification.getModified().equals(classificationImpl.getModified())) { - throw new ConcurrencyException( - "The current Classification has been modified while editing. " - + "The values can not be updated. Classification " - + classificationImpl.toString()); + throw new ConcurrencyException(); } return oldClassification; } diff --git a/lib/taskana-core/src/main/java/pro/taskana/classification/internal/jobs/ClassificationChangedJob.java b/lib/taskana-core/src/main/java/pro/taskana/classification/internal/jobs/ClassificationChangedJob.java index 0be127325..8d421b946 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/classification/internal/jobs/ClassificationChangedJob.java +++ b/lib/taskana-core/src/main/java/pro/taskana/classification/internal/jobs/ClassificationChangedJob.java @@ -8,6 +8,7 @@ import org.slf4j.LoggerFactory; import pro.taskana.common.api.ScheduledJob; import pro.taskana.common.api.TaskanaEngine; +import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.common.api.exceptions.TaskanaException; import pro.taskana.common.internal.jobs.AbstractTaskanaJob; import pro.taskana.common.internal.transaction.TaskanaTransactionProvider; @@ -21,9 +22,9 @@ public class ClassificationChangedJob extends AbstractTaskanaJob { public static final String PRIORITY_CHANGED = "priorityChanged"; public static final String SERVICE_LEVEL_CHANGED = "serviceLevelChanged"; private static final Logger LOGGER = LoggerFactory.getLogger(ClassificationChangedJob.class); - private String classificationId; - private boolean priorityChanged; - private boolean serviceLevelChanged; + private final String classificationId; + private final boolean priorityChanged; + private final boolean serviceLevelChanged; public ClassificationChangedJob( TaskanaEngine engine, TaskanaTransactionProvider txProvider, ScheduledJob job) { @@ -46,7 +47,7 @@ public class ClassificationChangedJob extends AbstractTaskanaJob { } LOGGER.info("ClassificationChangedJob ended successfully."); } catch (Exception e) { - throw new TaskanaException("Error while processing ClassificationChangedJob.", e); + throw new SystemException("Error while processing ClassificationChangedJob.", e); } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/common/api/TaskanaEngine.java b/lib/taskana-core/src/main/java/pro/taskana/common/api/TaskanaEngine.java index b2550e6c3..a02e7709c 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/common/api/TaskanaEngine.java +++ b/lib/taskana-core/src/main/java/pro/taskana/common/api/TaskanaEngine.java @@ -83,11 +83,11 @@ public interface TaskanaEngine { void setConnectionManagementMode(ConnectionManagementMode mode); /** - * Set the connection to be used by taskana in mode CONNECTION_MANAGED_EXTERNALLY. If this Api is - * called, taskana uses the connection passed by the client for all subsequent API calls until the - * client resets this connection. Control over commit and rollback of the connection is the - * responsibility of the client. In order to close the connection, closeConnection() or - * setConnection(null) has to be called. + * Set the connection to be used by TASKANA in mode {@linkplain + * ConnectionManagementMode#EXPLICIT}. If this Api is called, taskana uses the connection passed + * by the client for all subsequent API calls until the client resets this connection. Control + * over commit and rollback of the connection is the responsibility of the client. In order to + * close the connection, closeConnection() or setConnection(null) has to be called. * * @param connection - The java.sql.Connection that is controlled by the client * @throws SQLException if a database access error occurs @@ -126,7 +126,6 @@ public interface TaskanaEngine { */ T runAsAdmin(Supplier supplier); - /** * Returns the CurrentUserContext class. * diff --git a/lib/taskana-core/src/main/java/pro/taskana/common/internal/InternalTaskanaEngine.java b/lib/taskana-core/src/main/java/pro/taskana/common/internal/InternalTaskanaEngine.java index e639e49e9..4aaefbc2b 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/common/internal/InternalTaskanaEngine.java +++ b/lib/taskana-core/src/main/java/pro/taskana/common/internal/InternalTaskanaEngine.java @@ -83,5 +83,4 @@ public interface InternalTaskanaEngine { * @return the CreateTaskPreprocessorManager instance. */ CreateTaskPreprocessorManager getCreateTaskPreprocessorManager(); - } diff --git a/lib/taskana-core/src/main/java/pro/taskana/common/internal/TaskanaEngineImpl.java b/lib/taskana-core/src/main/java/pro/taskana/common/internal/TaskanaEngineImpl.java index c5bf05cab..cc9c5a4b7 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/common/internal/TaskanaEngineImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/common/internal/TaskanaEngineImpl.java @@ -36,9 +36,9 @@ import pro.taskana.common.api.TaskanaRole; import pro.taskana.common.api.WorkingDaysToDaysConverter; import pro.taskana.common.api.exceptions.AutocommitFailedException; import pro.taskana.common.api.exceptions.ConnectionNotSetException; +import pro.taskana.common.api.exceptions.MismatchedRoleException; import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.api.exceptions.SystemException; -import pro.taskana.common.api.exceptions.TaskanaRuntimeException; import pro.taskana.common.api.security.CurrentUserContext; import pro.taskana.common.api.security.UserPrincipal; import pro.taskana.common.internal.configuration.DB; @@ -239,23 +239,16 @@ public class TaskanaEngineImpl implements TaskanaEngine { currentUserContext.getAccessIds(), rolesAsString); } - throw new NotAuthorizedException( - "current user is not member of role(s) " + Arrays.toString(roles), - currentUserContext.getUserid()); + throw new MismatchedRoleException(currentUserContext.getUserid(), roles); } } - @Override - public CurrentUserContext getCurrentUserContext() { - return currentUserContext; - } - public T runAsAdmin(Supplier supplier) { String adminName = this.getConfiguration().getRoleMap().get(TaskanaRole.ADMIN).stream() .findFirst() - .orElseThrow(() -> new TaskanaRuntimeException("There is no admin configured")); + .orElseThrow(() -> new SystemException("There is no admin configured")); Subject subject = new Subject(); subject.getPrincipals().add(new UserPrincipal(adminName)); @@ -263,6 +256,11 @@ public class TaskanaEngineImpl implements TaskanaEngine { return Subject.doAs(subject, (PrivilegedAction) supplier::get); } + @Override + public CurrentUserContext getCurrentUserContext() { + return currentUserContext; + } + /** * This method creates the sqlSessionManager of myBatis. It integrates all the SQL mappers and * sets the databaseId attribute. diff --git a/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/exceptions/TaskanaHistoryEventNotFoundException.java b/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/exceptions/TaskanaHistoryEventNotFoundException.java index 69de742f1..83d52388d 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/exceptions/TaskanaHistoryEventNotFoundException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/spi/history/api/exceptions/TaskanaHistoryEventNotFoundException.java @@ -1,10 +1,27 @@ package pro.taskana.spi.history.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.NotFoundException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.spi.history.api.events.task.TaskHistoryEvent; +/** + * This exception is thrown when the {@linkplain TaskHistoryEvent} with the specified {@linkplain + * TaskHistoryEvent#getId() id} was not found. + */ public class TaskanaHistoryEventNotFoundException extends NotFoundException { - public TaskanaHistoryEventNotFoundException(String id, String msg) { - super(id, msg); + public static final String ERROR_KEY = "HISTORY_EVENT_NOT_FOUND"; + private final String historyEventId; + + public TaskanaHistoryEventNotFoundException(String historyEventId) { + super( + String.format("TaskHistoryEvent with id '%s' was not found", historyEventId), + ErrorCode.of(ERROR_KEY, MapCreator.of("historyEventId", historyEventId))); + this.historyEventId = historyEventId; + } + + public String getHistoryEventId() { + return historyEventId; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskService.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskService.java index c39147719..850701bbe 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskService.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskService.java @@ -60,7 +60,7 @@ public interface TaskService { * * @param taskId id of the task which should be unclaimed. * @return updated unclaimed task - * @throws TaskNotFoundException if the task can´t be found or does not exist + * @throws TaskNotFoundException if the task can't be found or does not exist * @throws InvalidStateException if the task is already in an end state. * @throws InvalidOwnerException if the task is claimed by another user. * @throws NotAuthorizedException if the current user has no read permission for the workbasket @@ -75,7 +75,7 @@ public interface TaskService { * * @param taskId id of the task which should be unclaimed. * @return updated unclaimed task - * @throws TaskNotFoundException if the task can´t be found or does not exist + * @throws TaskNotFoundException if the task can't be found or does not exist * @throws InvalidStateException if the task is already in an end state. * @throws InvalidOwnerException if forceCancel is false and the task is claimed by another user. * @throws NotAuthorizedException if the current user has no read permission for the workbasket @@ -91,8 +91,8 @@ public interface TaskService { * * @param taskId - Id of the Task which should be completed. * @return Task - updated task after completion. - * @throws InvalidStateException if Task wasn´t claimed before. - * @throws TaskNotFoundException if the given Task can´t be found in DB. + * @throws InvalidStateException if 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. * @throws NotAuthorizedException if the current user has no read permission for the workbasket * the task is in @@ -107,8 +107,8 @@ public interface TaskService { * * @param taskId - Id of the Task which should be completed. * @return Task - updated task after completion. - * @throws InvalidStateException if Task wasn´t claimed before. - * @throws TaskNotFoundException if the given Task can´t be found in DB. + * @throws InvalidStateException if 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. * @throws NotAuthorizedException if the current user has no read permission for the workbasket * the task is in diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskState.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskState.java index e883a54d0..750fb6a01 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskState.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskState.java @@ -13,7 +13,7 @@ public enum TaskState { public static final TaskState[] END_STATES = {COMPLETED, CANCELLED, TERMINATED}; public boolean in(TaskState... states) { - return Arrays.stream(states).anyMatch(state -> state == this); + return Arrays.asList(states).contains(this); } public boolean isEndState() { diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/AttachmentPersistenceException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/AttachmentPersistenceException.java index 113deed25..0778f6609 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/AttachmentPersistenceException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/AttachmentPersistenceException.java @@ -1,15 +1,40 @@ package pro.taskana.task.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.task.api.models.Attachment; +import pro.taskana.task.api.models.Task; /** - * Thrown, when an attachment should be inserted to the DB, but it does already exist.
- * This may happen when a not inserted attachment with ID will be added twice on a task. This can´t - * be happen it the correct Task-Methods will be used instead the List ones. + * This exception is thrown when an {@linkplain Attachment} should be inserted to the DB, but it + * does already exist.
+ * This may happen when a not inserted {@linkplain Attachment} with the same {@linkplain + * Attachment#getId() id} will be added twice on a {@linkplain Task}. This can't happen if the + * correct {@linkplain Task}-Methods will be used instead of the List ones. */ public class AttachmentPersistenceException extends TaskanaException { + public static final String ERROR_KEY = "ATTACHMENT_ALREADY_EXISTS"; + private final String attachmentId; + private final String taskId; - public AttachmentPersistenceException(String msg, Throwable cause) { - super(msg, cause); + public AttachmentPersistenceException(String attachmentId, String taskId, Throwable cause) { + super( + String.format( + "Cannot insert Attachment with id '%s' for Task with id '%s' " + + "because it already exists.", + attachmentId, taskId), + ErrorCode.of(ERROR_KEY, MapCreator.of("attachmentId", attachmentId, "taskId", taskId)), + cause); + this.attachmentId = attachmentId; + this.taskId = taskId; + } + + public String getAttachmentId() { + return attachmentId; + } + + public String getTaskId() { + return taskId; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidCallbackStateException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidCallbackStateException.java new file mode 100644 index 000000000..93ae2e641 --- /dev/null +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidCallbackStateException.java @@ -0,0 +1,53 @@ +package pro.taskana.task.api.exceptions; + +import java.util.Arrays; + +import pro.taskana.common.api.exceptions.ErrorCode; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.task.api.CallbackState; +import pro.taskana.task.api.models.Task; +import pro.taskana.task.internal.models.MinimalTaskSummary; + +/** + * This exception is thrown when the {@linkplain MinimalTaskSummary#getCallbackState() callback + * state} of the {@linkplain Task} doesn't allow the requested operation. + */ +public class InvalidCallbackStateException extends InvalidStateException { + + public static final String ERROR_KEY = "TASK_INVALID_CALLBACK_STATE"; + private final String taskId; + private final CallbackState taskCallbackState; + private final CallbackState[] requiredCallbackStates; + + public InvalidCallbackStateException( + String taskId, CallbackState taskCallbackState, CallbackState... requiredCallbackStates) { + super( + String.format( + "Expected callback state of Task with id '%s' to be: '%s', but found '%s'", + taskId, Arrays.toString(requiredCallbackStates), taskCallbackState), + ErrorCode.of( + ERROR_KEY, + MapCreator.of( + "taskId", + taskId, + "taskCallbackState", + taskCallbackState, + "requiredCallbackStates", + requiredCallbackStates))); + this.taskId = taskId; + this.taskCallbackState = taskCallbackState; + this.requiredCallbackStates = requiredCallbackStates; + } + + public CallbackState getTaskCallbackState() { + return taskCallbackState; + } + + public CallbackState[] getRequiredCallbackStates() { + return requiredCallbackStates; + } + + public String getTaskId() { + return taskId; + } +} diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidOwnerException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidOwnerException.java index 013c8fc4c..a1ec94ade 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidOwnerException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidOwnerException.java @@ -1,11 +1,29 @@ package pro.taskana.task.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.task.api.models.Task; -/** This exception is thrown when the task state doesn't allow the requested operation. */ +/** This exception is thrown when the current user is not the owner of the {@linkplain Task}. */ public class InvalidOwnerException extends TaskanaException { + public static final String ERROR_KEY = "TASK_INVALID_OWNER"; + private final String taskId; + private final String currentUserId; - public InvalidOwnerException(String msg) { - super(msg); + public InvalidOwnerException(String currentUserId, String taskId) { + super( + String.format("User '%s' is not owner of Task '%s'.", currentUserId, taskId), + ErrorCode.of(ERROR_KEY, MapCreator.of("taskId", taskId, "currentUserId", currentUserId))); + this.taskId = taskId; + this.currentUserId = currentUserId; + } + + public String getTaskId() { + return taskId; + } + + public String getCurrentUserId() { + return currentUserId; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidStateException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidStateException.java index 987da67f1..5580984ca 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidStateException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidStateException.java @@ -1,11 +1,12 @@ package pro.taskana.task.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaException; -/** This exception is thrown when the task state doesn't allow the requested operation. */ +/** This exception is thrown when the current state doesn't allow the requested operation. */ public class InvalidStateException extends TaskanaException { - public InvalidStateException(String msg) { - super(msg); + protected InvalidStateException(String msg, ErrorCode errorCode) { + super(msg, errorCode); } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidTaskStateException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidTaskStateException.java new file mode 100644 index 000000000..564386d74 --- /dev/null +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/InvalidTaskStateException.java @@ -0,0 +1,53 @@ +package pro.taskana.task.api.exceptions; + +import java.util.Arrays; + +import pro.taskana.common.api.exceptions.ErrorCode; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.task.api.TaskState; +import pro.taskana.task.api.models.Task; + +/** + * This exception is thrown when the {@linkplain Task#getState() state} of the {@linkplain Task} + * doesn't allow the requested operation. + */ +public class InvalidTaskStateException extends InvalidStateException { + + public static final String ERROR_KEY = "TASK_INVALID_STATE"; + private final String taskId; + private final TaskState taskState; + private final TaskState[] requiredTaskStates; + + public InvalidTaskStateException( + String taskId, TaskState taskState, TaskState... requiredTaskStates) { + super( + String.format( + "Task with id '%s' is in state: '%s', but must be in one of these states: '%s'", + taskId, taskState, Arrays.toString(requiredTaskStates)), + ErrorCode.of( + ERROR_KEY, + MapCreator.of( + "taskId", + taskId, + "taskState", + taskState, + "requiredTaskStates", + requiredTaskStates))); + + this.taskId = taskId; + this.taskState = taskState; + this.requiredTaskStates = requiredTaskStates; + } + + public String getTaskId() { + return taskId; + } + + public TaskState getTaskState() { + return taskState; + } + + public TaskState[] getRequiredTaskStates() { + return requiredTaskStates; + } +} diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/MismatchedTaskCommentCreatorException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/MismatchedTaskCommentCreatorException.java new file mode 100644 index 000000000..14b5133f3 --- /dev/null +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/MismatchedTaskCommentCreatorException.java @@ -0,0 +1,38 @@ +package pro.taskana.task.api.exceptions; + +import pro.taskana.common.api.exceptions.ErrorCode; +import pro.taskana.common.api.exceptions.NotAuthorizedException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.task.api.models.TaskComment; + +/** + * This exception is thrown when the current user is not the creator of the {@linkplain TaskComment} + * it tries to modify. + */ +public class MismatchedTaskCommentCreatorException extends NotAuthorizedException { + + public static final String ERROR_KEY = "TASK_COMMENT_CREATOR_MISMATCHED"; + private final String currentUserId; + private final String taskCommentId; + + public MismatchedTaskCommentCreatorException(String currentUserId, String taskCommentId) { + super( + String.format( + "Not authorized. Current user '%s' is not the creator of TaskComment with id '%s'.", + currentUserId, taskCommentId), + ErrorCode.of( + ERROR_KEY, + MapCreator.of("currentUserId", currentUserId, "taskCommentId", taskCommentId))); + + this.currentUserId = currentUserId; + this.taskCommentId = taskCommentId; + } + + public String getCurrentUserId() { + return currentUserId; + } + + public String getTaskCommentId() { + return taskCommentId; + } +} diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskAlreadyExistException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskAlreadyExistException.java index 392e01155..645143327 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskAlreadyExistException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskAlreadyExistException.java @@ -1,14 +1,27 @@ package pro.taskana.task.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.task.api.models.Task; /** - * Thrown when a Task is going to be created, but a Task with the same ID does already exist. The - * Task ID should be unique. + * This exception is thrown when a {@linkplain Task} is going to be created, but a {@linkplain Task} + * with the same {@linkplain Task#getExternalId() external id} does already exist. */ public class TaskAlreadyExistException extends TaskanaException { - public TaskAlreadyExistException(String id) { - super(id); + public static final String ERROR_KEY = "TASK_ALREADY_EXISTS"; + private final String externalId; + + public TaskAlreadyExistException(String externalId) { + super( + String.format("Task with external id '%s' already exists", externalId), + ErrorCode.of(ERROR_KEY, MapCreator.of("externalTaskId", externalId))); + this.externalId = externalId; + } + + public String getExternalId() { + return externalId; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskCommentNotFoundException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskCommentNotFoundException.java index eb08cfe8c..f2a5af581 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskCommentNotFoundException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskCommentNotFoundException.java @@ -1,11 +1,24 @@ package pro.taskana.task.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.NotFoundException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.task.api.models.TaskComment; -/** This exception will be thrown if a specific task comment is not in the database. */ +/** This exception is thrown when a specific {@linkplain TaskComment} is not in the database. */ public class TaskCommentNotFoundException extends NotFoundException { - public TaskCommentNotFoundException(String id, String msg) { - super(id, msg); + public static final String ERROR_KEY = "TASK_COMMENT_NOT_FOUND"; + private final String taskCommentId; + + public TaskCommentNotFoundException(String taskCommentId) { + super( + String.format("TaskComment with id '%s' was not found", taskCommentId), + ErrorCode.of(ERROR_KEY, MapCreator.of("taskCommentId", taskCommentId))); + this.taskCommentId = taskCommentId; + } + + public String getTaskCommentId() { + return taskCommentId; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskNotFoundException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskNotFoundException.java index 7ffc49c7f..1e0428c57 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskNotFoundException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/TaskNotFoundException.java @@ -1,11 +1,24 @@ package pro.taskana.task.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.NotFoundException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.task.api.models.Task; -/** This exception will be thrown if a specific task is not in the database. */ +/** This exception is thrown when a specific {@linkplain Task} is not in the database. */ public class TaskNotFoundException extends NotFoundException { - public TaskNotFoundException(String id, String msg) { - super(id, msg); + public static final String ERROR_KEY = "TASK_NOT_FOUND"; + private final String taskId; + + public TaskNotFoundException(String taskId) { + super( + String.format("Task with id '%s' was not found.", taskId), + ErrorCode.of(ERROR_KEY, MapCreator.of("taskId", taskId))); + this.taskId = taskId; + } + + public String getTaskId() { + return taskId; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/UpdateFailedException.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/UpdateFailedException.java deleted file mode 100644 index 5c05a4a5b..000000000 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/exceptions/UpdateFailedException.java +++ /dev/null @@ -1,11 +0,0 @@ -package pro.taskana.task.api.exceptions; - -import pro.taskana.common.api.exceptions.TaskanaException; - -/** This exception will be thrown if a specific task is not in the database. */ -public class UpdateFailedException extends TaskanaException { - - public UpdateFailedException(String msg) { - super(msg); - } -} diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/models/Task.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/models/Task.java index af8c5348a..21f4cdf8f 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/models/Task.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/models/Task.java @@ -119,7 +119,7 @@ public interface Task extends TaskSummary { /** * Return the attachments for this task.
* Do not use List.add()/addAll() for adding Elements, because it can cause redundant data. Use - * addAttachment(). Clear() and remove() can be used, because it´s a controllable change. + * addAttachment(). Clear() and remove() can be used, because it's a controllable change. * * @return the {@link List list} of {@link Attachment attachments} for this task */ 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 ae7ceed48..a645ae187 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 @@ -14,7 +14,6 @@ import org.slf4j.LoggerFactory; import pro.taskana.classification.api.ClassificationService; import pro.taskana.classification.api.exceptions.ClassificationNotFoundException; import pro.taskana.classification.api.models.ClassificationSummary; -import pro.taskana.common.api.BulkOperationResults; import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.internal.util.IdGenerator; import pro.taskana.task.api.exceptions.AttachmentPersistenceException; @@ -36,46 +35,6 @@ public class AttachmentHandler { this.classificationService = classificationService; } - List augmentAttachmentsByClassification( - List attachmentImpls, BulkOperationResults bulkLog) { - List result = new ArrayList<>(); - if (attachmentImpls == null || attachmentImpls.isEmpty()) { - return result; - } - List classifications = - classificationService - .createClassificationQuery() - .idIn( - attachmentImpls.stream() - .map(t -> t.getClassificationSummary().getId()) - .distinct() - .toArray(String[]::new)) - .list(); - for (AttachmentImpl att : attachmentImpls) { - ClassificationSummary classificationSummary = - classifications.stream() - .filter(cl -> cl.getId().equals(att.getClassificationSummary().getId())) - .findFirst() - .orElse(null); - if (classificationSummary == null) { - String id = att.getClassificationSummary().getId(); - bulkLog.addError( - att.getClassificationSummary().getId(), - new ClassificationNotFoundException( - id, - String.format( - "When processing task updates due to change " - + "of classification, the classification with id %s was not found", - id))); - } else { - att.setClassificationSummary(classificationSummary); - result.add(att); - } - } - - return result; - } - void insertAndDeleteAttachmentsOnTaskUpdate(TaskImpl newTaskImpl, TaskImpl oldTaskImpl) throws AttachmentPersistenceException, InvalidArgumentException, ClassificationNotFoundException { @@ -113,11 +72,7 @@ public class AttachmentHandler { 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()); + throw new AttachmentPersistenceException(attachmentImpl.getId(), task.getId(), e); } } } @@ -199,11 +154,7 @@ public class AttachmentHandler { 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()); + throw new AttachmentPersistenceException(attachmentImpl.getId(), newTaskImpl.getId(), e); } } 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 8aeb57d55..365e247db 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 @@ -21,7 +21,7 @@ import pro.taskana.common.api.WorkingDaysToDaysConverter; import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.TaskanaException; import pro.taskana.common.internal.InternalTaskanaEngine; -import pro.taskana.task.api.exceptions.UpdateFailedException; +import pro.taskana.common.internal.util.Pair; import pro.taskana.task.api.models.Attachment; import pro.taskana.task.api.models.AttachmentSummary; import pro.taskana.task.api.models.Task; @@ -38,6 +38,7 @@ class ServiceLevelHandler { private final TaskMapper taskMapper; private final AttachmentMapper attachmentMapper; private final WorkingDaysToDaysConverter converter; + private final TaskServiceImpl taskServiceImpl; ServiceLevelHandler( InternalTaskanaEngine taskanaEngine, @@ -47,6 +48,7 @@ class ServiceLevelHandler { this.taskMapper = taskMapper; this.attachmentMapper = attachmentMapper; converter = taskanaEngine.getEngine().getWorkingDaysToDaysConverter(); + taskServiceImpl = (TaskServiceImpl) taskanaEngine.getEngine().getTaskService(); } // use the same algorithm as setPlannedPropertyOfTasksImpl to refresh @@ -351,7 +353,7 @@ class ServiceLevelHandler { private BulkOperationResults updateDuePropertyOfTasksWithIdenticalDueDate( InstantDurationHolder durationHolder, List taskDurationList) { - BulkLog bulkLog = new BulkLog(); + final BulkLog bulkLog = new BulkLog(); TaskImpl referenceTask = new TaskImpl(); referenceTask.setPlanned(durationHolder.getPlanned()); referenceTask.setModified(Instant.now()); @@ -359,61 +361,30 @@ class ServiceLevelHandler { getFollowingWorkingDays(referenceTask.getPlanned(), durationHolder.getDuration())); List taskIdsToUpdate = taskDurationList.stream().map(TaskDuration::getTaskId).collect(Collectors.toList()); - long numTasksUpdated = taskMapper.updateTaskDueDates(taskIdsToUpdate, referenceTask); - if (numTasksUpdated != taskIdsToUpdate.size()) { - BulkLog checkResult = - checkResultsOfTasksUpdateAndAddErrorsToBulkLog( - taskIdsToUpdate, referenceTask, numTasksUpdated); - bulkLog.addAllErrors(checkResult); - } + Pair, BulkLog> existingAndAuthorizedTasks = + taskServiceImpl.getMinimalTaskSummaries(taskIdsToUpdate); + bulkLog.addAllErrors(existingAndAuthorizedTasks.getRight()); + + taskMapper.updateTaskDueDates(existingAndAuthorizedTasks.getLeft(), referenceTask); return bulkLog; } private BulkLog updatePlannedPropertyOfAffectedTasks( - Instant planned, Map> durationToTaskIdsMap) { - BulkLog bulkLog = new BulkLog(); + Instant planned, Map> taskIdsByDueDuration) { + final BulkLog bulkLog = new BulkLog(); TaskImpl referenceTask = new TaskImpl(); referenceTask.setPlanned(planned); referenceTask.setModified(Instant.now()); - for (Map.Entry> entry : durationToTaskIdsMap.entrySet()) { - List taskIdsToUpdate = entry.getValue(); - referenceTask.setDue(getFollowingWorkingDays(referenceTask.getPlanned(), entry.getKey())); - long numTasksUpdated = taskMapper.updateTaskDueDates(taskIdsToUpdate, referenceTask); - if (numTasksUpdated != taskIdsToUpdate.size()) { - BulkLog checkResult = - checkResultsOfTasksUpdateAndAddErrorsToBulkLog( - taskIdsToUpdate, referenceTask, numTasksUpdated); - bulkLog.addAllErrors(checkResult); - } - } - return bulkLog; - } - private BulkLog checkResultsOfTasksUpdateAndAddErrorsToBulkLog( - List taskIdsToUpdate, TaskImpl referenceTask, long numTasksUpdated) { - BulkLog bulkLog = new BulkLog(); - long numErrors = taskIdsToUpdate.size() - numTasksUpdated; - long numErrorsLogged = 0; - if (numErrors > 0) { - List taskSummaries = taskMapper.findExistingTasks(taskIdsToUpdate, null); - for (MinimalTaskSummary task : taskSummaries) { - if (referenceTask.getDue() != task.getDue()) { - bulkLog.addError( - task.getTaskId(), - new UpdateFailedException( - String.format("Could not set Due Date of Task with Id %s. ", task.getTaskId()))); - numErrorsLogged++; - } - } - long numErrorsNotLogged = numErrors - numErrorsLogged; - if (numErrorsNotLogged != 0) { - for (int i = 1; i <= numErrorsNotLogged; i++) { - bulkLog.addError( - String.format("UnknownTaskId: %d", i), - new UpdateFailedException("Update of unknown task failed")); - } - } - } + taskIdsByDueDuration.forEach( + (duration, taskIds) -> { + referenceTask.setDue(getFollowingWorkingDays(planned, duration)); + Pair, BulkLog> existingAndAuthorizedTasks = + taskServiceImpl.getMinimalTaskSummaries(taskIds); + bulkLog.addAllErrors(existingAndAuthorizedTasks.getRight()); + taskMapper.updateTaskDueDates(existingAndAuthorizedTasks.getLeft(), referenceTask); + }); + return bulkLog; } @@ -641,17 +612,12 @@ class ServiceLevelHandler { && newClassificationIds.containsAll(oldClassificationIds); } - static class BulkLog extends BulkOperationResults { - - BulkLog() { - super(); - } - } + static class BulkLog extends BulkOperationResults {} static final class DurationPrioHolder { - private Duration duration; - private int priority; + private final Duration duration; + private final int priority; DurationPrioHolder(Duration duration, int priority) { this.duration = duration; diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java index 0a703fcb2..4d2ef6b66 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java @@ -13,6 +13,7 @@ import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.common.internal.InternalTaskanaEngine; import pro.taskana.common.internal.util.IdGenerator; +import pro.taskana.task.api.exceptions.MismatchedTaskCommentCreatorException; import pro.taskana.task.api.exceptions.TaskCommentNotFoundException; import pro.taskana.task.api.exceptions.TaskNotFoundException; import pro.taskana.task.api.models.TaskComment; @@ -75,12 +76,7 @@ class TaskCommentServiceImpl { } } else { - throw new NotAuthorizedException( - String.format( - "Not authorized, TaskComment creator and current user must match. " - + "TaskComment creator is %s but current user is %s", - taskCommentImplToUpdate.getCreator(), userId), - userId); + throw new MismatchedTaskCommentCreatorException(userId, taskCommentImplToUpdate.getId()); } } finally { taskanaEngine.returnConnection(); @@ -136,12 +132,7 @@ class TaskCommentServiceImpl { } } else { - throw new NotAuthorizedException( - String.format( - "Not authorized, TaskComment creator and current user must match. " - + "TaskComment creator is %s but current user is %s", - taskCommentToDelete.getCreator(), userId), - userId); + throw new MismatchedTaskCommentCreatorException(userId, taskCommentToDelete.getId()); } } finally { @@ -186,9 +177,7 @@ class TaskCommentServiceImpl { result = taskCommentMapper.findById(taskCommentId); if (result == null) { - throw new TaskCommentNotFoundException( - taskCommentId, - String.format("TaskComment for taskCommentId '%s' was not found", taskCommentId)); + throw new TaskCommentNotFoundException(taskCommentId); } taskService.getTask(result.getTaskId()); @@ -204,11 +193,7 @@ class TaskCommentServiceImpl { TaskComment oldTaskComment, TaskComment taskCommentImplToUpdate) throws ConcurrencyException { if (!oldTaskComment.getModified().equals(taskCommentImplToUpdate.getModified())) { - - throw new ConcurrencyException( - "The current TaskComment has been modified while editing. " - + "The values can not be updated. TaskComment " - + taskCommentImplToUpdate.toString()); + throw new ConcurrencyException(); } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskMapper.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskMapper.java index d784a8b4a..e6714dacd 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskMapper.java @@ -1,6 +1,7 @@ package pro.taskana.task.internal; import java.time.Instant; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; @@ -131,10 +132,9 @@ public interface TaskMapper { @Update( "") - int setOwnerOfTasks( + void setOwnerOfTasks( @Param("owner") String owner, @Param("taskIds") List taskIds, @Param("modified") Instant modified); @@ -184,7 +184,7 @@ public interface TaskMapper { @Result(property = "planned", column = "PLANNED") @Result(property = "callbackState", column = "CALLBACK_STATE") List findExistingTasks( - @Param("taskIds") List taskIds, @Param("externalIds") List externalIds); + @Param("taskIds") Collection taskIds, @Param("externalIds") List externalIds); @Update( "") - long updateTaskDueDates( - @Param("taskIds") List taskIds, @Param("referenceTask") TaskImpl referenceTask); + void updateTaskDueDates( + @Param("taskSummaries") List taskSummaries, + @Param("referenceTask") TaskImpl referenceTask); @Update( "") - long updatePriorityOfTasks( + void updatePriorityOfTasks( @Param("taskIds") List taskIds, @Param("referenceTask") TaskImpl referenceTask); @Select( @@ -261,10 +262,10 @@ public interface TaskMapper { "") - @Result(property = "id", column = "ID") - List filterTaskIdsNotAuthorizedFor( - @Param("taskIds") List taskIds, @Param("accessIds") List accessIds); + @Result(property = "left", column = "ID") + @Result(property = "right", column = "WORKBASKET_ID") + List> getTaskAndWorkbasketIdsNotAuthorizedFor( + @Param("taskSummaries") List taskSummaries, + @Param("accessIds") List accessIds); } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryImpl.java index 5c9bd3341..05528cb57 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskQueryImpl.java @@ -2,7 +2,6 @@ package pro.taskana.task.internal; import java.util.ArrayList; import java.util.Arrays; -import java.util.EnumSet; import java.util.List; import org.apache.ibatis.exceptions.PersistenceException; import org.apache.ibatis.session.RowBounds; @@ -18,6 +17,7 @@ import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.common.api.exceptions.TaskanaRuntimeException; import pro.taskana.common.internal.InternalTaskanaEngine; import pro.taskana.common.internal.configuration.DB; +import pro.taskana.common.internal.util.EnumUtil; import pro.taskana.task.api.CallbackState; import pro.taskana.task.api.ObjectReferenceQuery; import pro.taskana.task.api.TaskCustomField; @@ -231,8 +231,7 @@ public class TaskQueryImpl implements TaskQuery { @Override public TaskQuery stateNotIn(TaskState... states) { - this.stateIn = - EnumSet.complementOf(EnumSet.copyOf(Arrays.asList(states))).toArray(new TaskState[0]); + this.stateIn = EnumUtil.allValuesExceptFor(states); return this; } @@ -947,7 +946,7 @@ public class TaskQueryImpl implements TaskQuery { } catch (PersistenceException e) { if (e.getMessage().contains("ERRORCODE=-4470")) { TaskanaRuntimeException ex = - new TaskanaRuntimeException( + new SystemException( "The offset beginning was set over the amount of result-rows.", e.getCause()); ex.setStackTrace(e.getStackTrace()); throw ex; @@ -1619,7 +1618,7 @@ public class TaskQueryImpl implements TaskQuery { } } } catch (NotAuthorizedException e) { - throw new NotAuthorizedToQueryWorkbasketException(e.getMessage(), e.getCause()); + throw new NotAuthorizedToQueryWorkbasketException(e.getMessage(), e.getErrorCode(), e); } } 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 a9680758a..8795f1402 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 @@ -3,6 +3,7 @@ package pro.taskana.task.internal; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; @@ -34,6 +35,7 @@ import pro.taskana.common.api.exceptions.TaskanaException; import pro.taskana.common.internal.InternalTaskanaEngine; import pro.taskana.common.internal.util.CheckedConsumer; import pro.taskana.common.internal.util.CollectionUtil; +import pro.taskana.common.internal.util.EnumUtil; import pro.taskana.common.internal.util.IdGenerator; import pro.taskana.common.internal.util.ObjectAttributeChangeDetector; import pro.taskana.common.internal.util.Pair; @@ -52,12 +54,13 @@ import pro.taskana.task.api.TaskQuery; import pro.taskana.task.api.TaskService; import pro.taskana.task.api.TaskState; import pro.taskana.task.api.exceptions.AttachmentPersistenceException; +import pro.taskana.task.api.exceptions.InvalidCallbackStateException; import pro.taskana.task.api.exceptions.InvalidOwnerException; import pro.taskana.task.api.exceptions.InvalidStateException; +import pro.taskana.task.api.exceptions.InvalidTaskStateException; import pro.taskana.task.api.exceptions.TaskAlreadyExistException; import pro.taskana.task.api.exceptions.TaskCommentNotFoundException; import pro.taskana.task.api.exceptions.TaskNotFoundException; -import pro.taskana.task.api.exceptions.UpdateFailedException; import pro.taskana.task.api.models.Attachment; import pro.taskana.task.api.models.ObjectReference; import pro.taskana.task.api.models.Task; @@ -71,6 +74,7 @@ import pro.taskana.task.internal.models.TaskImpl; import pro.taskana.task.internal.models.TaskSummaryImpl; import pro.taskana.workbasket.api.WorkbasketPermission; import pro.taskana.workbasket.api.WorkbasketService; +import pro.taskana.workbasket.api.exceptions.MismatchedWorkbasketPermissionException; import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; import pro.taskana.workbasket.api.models.Workbasket; import pro.taskana.workbasket.api.models.WorkbasketSummary; @@ -193,9 +197,7 @@ public class TaskServiceImpl implements TaskService { } if (workbasket.isMarkedForDeletion()) { - throw new WorkbasketNotFoundException( - workbasket.getId(), - "The workbasket " + workbasket.getId() + " was marked for deletion"); + throw new WorkbasketNotFoundException(workbasket.getId()); } task.setWorkbasketSummary(workbasket.asSummary()); @@ -260,8 +262,7 @@ public class TaskServiceImpl implements TaskService { if (msg != null && (msg.contains("violation") || msg.contains("violates") || msg.contains("verletzt")) && msg.contains("external_id")) { - throw new TaskAlreadyExistException( - "Task with external id " + task.getExternalId() + " already exists"); + throw new TaskAlreadyExistException(task.getExternalId()); } else { throw e; } @@ -274,7 +275,7 @@ public class TaskServiceImpl implements TaskService { @Override public Task getTask(String id) throws NotAuthorizedException, TaskNotFoundException { - TaskImpl resultTask = null; + TaskImpl resultTask; try { taskanaEngine.openConnection(); @@ -285,13 +286,10 @@ public class TaskServiceImpl implements TaskService { String workbasketId = resultTask.getWorkbasketSummary().getId(); List workbaskets = query.idIn(workbasketId).list(); if (workbaskets.isEmpty()) { - String currentUser = taskanaEngine.getEngine().getCurrentUserContext().getUserid(); - throw new NotAuthorizedException( - "The current user " - + currentUser - + " has no read permission for workbasket " - + workbasketId, - taskanaEngine.getEngine().getCurrentUserContext().getUserid()); + throw new MismatchedWorkbasketPermissionException( + taskanaEngine.getEngine().getCurrentUserContext().getUserid(), + workbasketId, + WorkbasketPermission.READ); } else { resultTask.setWorkbasketSummary(workbaskets.get(0)); } @@ -322,7 +320,7 @@ public class TaskServiceImpl implements TaskService { resultTask.setClassificationSummary(classification); return resultTask; } else { - throw new TaskNotFoundException(id, String.format("Task with id %s was not found.", id)); + throw new TaskNotFoundException(id); } } finally { taskanaEngine.returnConnection(); @@ -346,7 +344,7 @@ public class TaskServiceImpl implements TaskService { @Override public Task setTaskRead(String taskId, boolean isRead) throws TaskNotFoundException, NotAuthorizedException { - TaskImpl task = null; + TaskImpl task; try { taskanaEngine.openConnection(); task = (TaskImpl) getTask(taskId); @@ -692,43 +690,35 @@ public class TaskServiceImpl implements TaskService { @Override public BulkOperationResults setOwnerOfTasks( - String owner, List argTaskIds) { + String owner, List taskIds) { BulkOperationResults bulkLog = new BulkOperationResults<>(); - if (argTaskIds == null || argTaskIds.isEmpty()) { + if (taskIds == null || taskIds.isEmpty()) { return bulkLog; } - // remove duplicates - List taskIds = argTaskIds.stream().distinct().collect(Collectors.toList()); - final int requestSize = taskIds.size(); + try { taskanaEngine.openConnection(); - // use only elements we are authorized for - Pair, BulkLog> resultsPair = getMinimalTaskSummaries(taskIds); - // set the Owner of these tasks we are authorized for - List existingMinimalTaskSummaries = resultsPair.getLeft(); - taskIds = - existingMinimalTaskSummaries.stream() - .map(MinimalTaskSummary::getTaskId) - .collect(Collectors.toList()); - bulkLog.addAllErrors(resultsPair.getRight()); - if (!taskIds.isEmpty()) { - final int numberOfAffectedTasks = taskMapper.setOwnerOfTasks(owner, taskIds, Instant.now()); - if (numberOfAffectedTasks != taskIds.size()) { // all tasks were updated - // check the outcome - existingMinimalTaskSummaries = taskMapper.findExistingTasks(taskIds, null); - 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()); - } - } + Pair, BulkLog> existingAndAuthorizedTasks = + getMinimalTaskSummaries(taskIds); + bulkLog.addAllErrors(existingAndAuthorizedTasks.getRight()); + Pair, BulkLog> taskIdsToUpdate = + filterOutTasksWhichAreNotReady(existingAndAuthorizedTasks.getLeft()); + bulkLog.addAllErrors(taskIdsToUpdate.getRight()); + + if (!taskIdsToUpdate.getLeft().isEmpty()) { + taskMapper.setOwnerOfTasks(owner, taskIdsToUpdate.getLeft(), Instant.now()); } + + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "Received the Request to set owner on {} tasks, actually modified tasks = {}" + + ", could not set owner on {} tasks.", + taskIds.size(), + taskIdsToUpdate.getLeft().size(), + bulkLog.getFailedIds().size()); + } + return bulkLog; } finally { taskanaEngine.returnConnection(); @@ -874,10 +864,10 @@ public class TaskServiceImpl implements TaskService { } } - Pair, BulkLog> getMinimalTaskSummaries(List argTaskIds) { + Pair, BulkLog> getMinimalTaskSummaries(Collection argTaskIds) { BulkLog bulkLog = new BulkLog(); // remove duplicates - List taskIds = argTaskIds.stream().distinct().collect(Collectors.toList()); + Set taskIds = new HashSet<>(argTaskIds); // get existing tasks List minimalTaskSummaries = taskMapper.findExistingTasks(taskIds, null); bulkLog.addAllErrors(addExceptionsForNonExistingTasksToBulkLog(taskIds, minimalTaskSummaries)); @@ -894,39 +884,40 @@ public class TaskServiceImpl implements TaskService { if (taskanaEngine.getEngine().isUserInRole(TaskanaRole.ADMIN, TaskanaRole.TASK_ADMIN)) { return Pair.of(existingTasks, bulkLog); } else { - List taskIds = - existingTasks.stream().map(MinimalTaskSummary::getTaskId).collect(Collectors.toList()); List accessIds = taskanaEngine.getEngine().getCurrentUserContext().getAccessIds(); - List taskIdsNotAuthorizedFor = - taskMapper.filterTaskIdsNotAuthorizedFor(taskIds, accessIds); + List> taskAndWorkbasketIdsNotAuthorizedFor = + taskMapper.getTaskAndWorkbasketIdsNotAuthorizedFor(existingTasks, accessIds); String userId = taskanaEngine.getEngine().getCurrentUserContext().getUserid(); - for (String taskId : taskIdsNotAuthorizedFor) { + + for (Pair taskAndWorkbasketIds : taskAndWorkbasketIdsNotAuthorizedFor) { bulkLog.addError( - taskId, - new NotAuthorizedException( - String.format("User %s is not authorized for task %s ", userId, taskId), userId)); + taskAndWorkbasketIds.getLeft(), + new MismatchedWorkbasketPermissionException( + userId, taskAndWorkbasketIds.getRight(), WorkbasketPermission.READ)); } - taskIds.removeAll(taskIdsNotAuthorizedFor); + + Set taskIdsToRemove = + taskAndWorkbasketIdsNotAuthorizedFor.stream() + .map(Pair::getLeft) + .collect(Collectors.toSet()); List tasksAuthorizedFor = existingTasks.stream() - .filter(t -> taskIds.contains(t.getTaskId())) + .filter(t -> !taskIdsToRemove.contains(t.getTaskId())) .collect(Collectors.toList()); return Pair.of(tasksAuthorizedFor, bulkLog); } } BulkLog addExceptionsForNonExistingTasksToBulkLog( - List requestTaskIds, List existingMinimalTaskSummaries) { + Collection requestTaskIds, List existingMinimalTaskSummaries) { BulkLog bulkLog = new BulkLog(); - List nonExistingTaskIds = new ArrayList<>(requestTaskIds); - List existingTaskIds = + Set existingTaskIds = existingMinimalTaskSummaries.stream() .map(MinimalTaskSummary::getTaskId) - .collect(Collectors.toList()); - nonExistingTaskIds.removeAll(existingTaskIds); - nonExistingTaskIds.forEach( - taskId -> - bulkLog.addError(taskId, new TaskNotFoundException(taskId, "Task was not found"))); + .collect(Collectors.toSet()); + requestTaskIds.stream() + .filter(taskId -> !existingTaskIds.contains(taskId)) + .forEach(taskId -> bulkLog.addError(taskId, new TaskNotFoundException(taskId))); return bulkLog; } @@ -940,6 +931,24 @@ public class TaskServiceImpl implements TaskService { .collect(Collectors.toList()); } + private Pair, BulkLog> filterOutTasksWhichAreNotReady( + Collection minimalTaskSummaries) { + List filteredTasks = new ArrayList<>(minimalTaskSummaries.size()); + BulkLog bulkLog = new BulkLog(); + + for (MinimalTaskSummary taskSummary : minimalTaskSummaries) { + if (taskSummary.getTaskState() != TaskState.READY) { + bulkLog.addError( + taskSummary.getTaskId(), + new InvalidTaskStateException( + taskSummary.getTaskId(), taskSummary.getTaskState(), TaskState.READY)); + } else { + filteredTasks.add(taskSummary.getTaskId()); + } + } + return Pair.of(filteredTasks, bulkLog); + } + private List augmentTaskSummariesByContainedSummariesWithoutPartitioning( List taskSummaries) { List taskIds = @@ -1021,10 +1030,7 @@ public class TaskServiceImpl implements TaskService { pair -> { if (pair.getRight() == null) { String taskId = pair.getLeft(); - bulkLog.addError( - taskId, - new TaskNotFoundException( - taskId, String.format("Task with id %s was not found.", taskId))); + bulkLog.addError(taskId, new TaskNotFoundException(taskId)); return false; } return true; @@ -1056,7 +1062,7 @@ public class TaskServiceImpl implements TaskService { && !oldTaskImpl.getClaimed().equals(newTaskImpl.getClaimed()) || oldTaskImpl.getState() != null && !oldTaskImpl.getState().equals(newTaskImpl.getState())) { - throw new ConcurrencyException("The task has already been updated by another user"); + throw new ConcurrencyException(); } newTaskImpl.setModified(Instant.now()); } @@ -1064,14 +1070,12 @@ public class TaskServiceImpl implements TaskService { private TaskImpl terminateCancelCommonActions(String taskId, TaskState targetState) throws NotAuthorizedException, TaskNotFoundException, InvalidStateException { if (taskId == null || taskId.isEmpty()) { - throw new TaskNotFoundException( - taskId, String.format("Task with id %s was not found.", taskId)); + throw new TaskNotFoundException(taskId); } TaskImpl task = (TaskImpl) getTask(taskId); TaskState state = task.getState(); if (state.isEndState()) { - throw new InvalidStateException( - String.format("Task with Id %s is already in an end state.", taskId)); + throw new InvalidTaskStateException(taskId, state, TaskState.READY); } Instant now = Instant.now(); @@ -1088,30 +1092,6 @@ public class TaskServiceImpl implements TaskService { return task; } - private BulkOperationResults addExceptionsForTasksWhoseOwnerWasNotSet( - String owner, List existingMinimalTaskSummaries) { - BulkOperationResults bulkLog = new BulkOperationResults<>(); - - for (MinimalTaskSummary taskSummary : existingMinimalTaskSummaries) { - if (!owner.equals(taskSummary.getOwner())) { // owner was not set - if (!TaskState.READY.equals(taskSummary.getTaskState())) { // due to invalid state - bulkLog.addError( - taskSummary.getTaskId(), - new InvalidStateException( - String.format( - "Task with id %s is in state %s and not in state ready.", - taskSummary.getTaskId(), taskSummary.getTaskState()))); - } else { // due to unknown reason - bulkLog.addError( - taskSummary.getTaskId(), - new UpdateFailedException( - String.format("Could not set owner of Task %s .", taskSummary.getTaskId()))); - } - } - } - return bulkLog; - } - private Task claim(String taskId, boolean forceClaim) throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException { @@ -1159,16 +1139,14 @@ public class TaskServiceImpl implements TaskService { private void checkPreconditionsForClaimTask(TaskSummary task, boolean forced) throws InvalidStateException, InvalidOwnerException { TaskState state = task.getState(); - if (!state.in(TaskState.READY, TaskState.CLAIMED)) { - throw new InvalidStateException( - String.format("Task with Id %s is already in an end state.", task.getId())); + if (state.isEndState()) { + throw new InvalidTaskStateException( + task.getId(), task.getState(), EnumUtil.allValuesExceptFor(TaskState.END_STATES)); } - if (!forced - && state == TaskState.CLAIMED - && !task.getOwner().equals(taskanaEngine.getEngine().getCurrentUserContext().getUserid())) { - throw new InvalidOwnerException( - String.format( - "Task with id %s is already claimed by %s.", task.getId(), task.getOwner())); + + String userId = taskanaEngine.getEngine().getCurrentUserContext().getUserid(); + if (!forced && state == TaskState.CLAIMED && !task.getOwner().equals(userId)) { + throw new InvalidOwnerException(userId, task.getId()); } } @@ -1179,17 +1157,17 @@ public class TaskServiceImpl implements TaskService { private static void checkIfTaskIsTerminatedOrCancelled(TaskSummary task) throws InvalidStateException { if (task.getState().in(TaskState.CANCELLED, TaskState.TERMINATED)) { - throw new InvalidStateException( - String.format( - "Cannot complete task %s because it is in state %s.", task.getId(), task.getState())); + throw new InvalidTaskStateException( + task.getId(), + task.getState(), + EnumUtil.allValuesExceptFor(TaskState.CANCELLED, TaskState.TERMINATED)); } } private void checkPreconditionsForCompleteTask(TaskSummary task) throws InvalidStateException, InvalidOwnerException { if (taskIsNotClaimed(task)) { - throw new InvalidStateException( - String.format("Task with Id %s has to be claimed before.", task.getId())); + throw new InvalidTaskStateException(task.getId(), task.getState(), TaskState.CLAIMED); } else if (!taskanaEngine .getEngine() .getCurrentUserContext() @@ -1197,11 +1175,7 @@ public class TaskServiceImpl implements TaskService { .contains(task.getOwner()) && !taskanaEngine.getEngine().isUserInRole(TaskanaRole.ADMIN)) { throw new InvalidOwnerException( - String.format( - "Owner of task %s is %s, but current user is %s ", - task.getId(), - task.getOwner(), - taskanaEngine.getEngine().getCurrentUserContext().getUserid())); + taskanaEngine.getEngine().getCurrentUserContext().getUserid(), task.getId()); } } @@ -1215,12 +1189,11 @@ public class TaskServiceImpl implements TaskService { task = (TaskImpl) getTask(taskId); TaskState state = task.getState(); if (state.isEndState()) { - throw new InvalidStateException( - String.format("Task with Id %s is already in an end state.", taskId)); + throw new InvalidTaskStateException( + taskId, state, EnumUtil.allValuesExceptFor(TaskState.END_STATES)); } if (state == TaskState.CLAIMED && !forceUnclaim && !userId.equals(task.getOwner())) { - throw new InvalidOwnerException( - String.format("Task with id %s is already claimed by %s.", taskId, task.getOwner())); + throw new InvalidOwnerException(userId, taskId); } Instant now = Instant.now(); task.setOwner(null); @@ -1294,15 +1267,14 @@ public class TaskServiceImpl implements TaskService { task = (TaskImpl) getTask(taskId); if (!(task.getState().isEndState()) && !forceDelete) { - throw new InvalidStateException( - "Cannot delete Task " + taskId + " because it is not in an end state."); + throw new InvalidTaskStateException(taskId, task.getState(), TaskState.END_STATES); } if ((!task.getState().in(TaskState.TERMINATED, TaskState.CANCELLED)) && CallbackState.CALLBACK_PROCESSING_REQUIRED.equals(task.getCallbackState())) { - throw new InvalidStateException( - String.format( - "Task wit Id %s cannot be deleted because its callback is not yet processed", - taskId)); + throw new InvalidCallbackStateException( + taskId, + task.getCallbackState(), + EnumUtil.allValuesExceptFor(CallbackState.CALLBACK_PROCESSING_REQUIRED)); } attachmentMapper.deleteMultipleByTaskIds(Collections.singletonList(taskId)); @@ -1337,23 +1309,23 @@ public class TaskServiceImpl implements TaskService { .findFirst() .orElse(null); if (foundSummary == null) { - bulkLog.addError( - currentTaskId, - new TaskNotFoundException( - currentTaskId, String.format("Task with id %s was not found.", currentTaskId))); + bulkLog.addError(currentTaskId, new TaskNotFoundException(currentTaskId)); taskIdIterator.remove(); } else if (!(foundSummary.getTaskState().isEndState())) { - bulkLog.addError(currentTaskId, new InvalidStateException(currentTaskId)); + bulkLog.addError( + currentTaskId, + new InvalidTaskStateException( + currentTaskId, foundSummary.getTaskState(), TaskState.END_STATES)); taskIdIterator.remove(); } else { if ((!foundSummary.getTaskState().in(TaskState.CANCELLED, TaskState.TERMINATED)) && CallbackState.CALLBACK_PROCESSING_REQUIRED.equals(foundSummary.getCallbackState())) { bulkLog.addError( currentTaskId, - new InvalidStateException( - String.format( - "Task wit Id %s cannot be deleted because its callback is not yet processed", - currentTaskId))); + new InvalidCallbackStateException( + currentTaskId, + foundSummary.getCallbackState(), + EnumUtil.allValuesExceptFor(CallbackState.CALLBACK_PROCESSING_REQUIRED))); taskIdIterator.remove(); } } @@ -1376,23 +1348,20 @@ public class TaskServiceImpl implements TaskService { .filter(taskSummary -> currentExternalId.equals(taskSummary.getExternalId())) .findFirst(); if (foundSummary.isPresent()) { - if (!desiredCallbackStateCanBeSetForFoundSummary( - foundSummary.get(), desiredCallbackState)) { - bulkLog.addError(currentExternalId, new InvalidStateException(currentExternalId)); + Optional invalidStateException = + desiredCallbackStateCanBeSetForFoundSummary(foundSummary.get(), desiredCallbackState); + if (invalidStateException.isPresent()) { + bulkLog.addError(currentExternalId, invalidStateException.get()); externalIdIterator.remove(); } } else { - bulkLog.addError( - currentExternalId, - new TaskNotFoundException( - currentExternalId, - String.format("Task with id %s was not found.", currentExternalId))); + bulkLog.addError(currentExternalId, new TaskNotFoundException(currentExternalId)); externalIdIterator.remove(); } } } - private boolean desiredCallbackStateCanBeSetForFoundSummary( + private Optional desiredCallbackStateCanBeSetForFoundSummary( MinimalTaskSummary foundSummary, CallbackState desiredCallbackState) { CallbackState currentTaskCallbackState = foundSummary.getCallbackState(); @@ -1400,21 +1369,48 @@ public class TaskServiceImpl implements TaskService { switch (desiredCallbackState) { case CALLBACK_PROCESSING_COMPLETED: - return currentTaskState.isEndState(); - + if (!currentTaskState.isEndState()) { + return Optional.of( + new InvalidTaskStateException( + foundSummary.getTaskId(), foundSummary.getTaskState(), TaskState.END_STATES)); + } + break; case CLAIMED: if (!currentTaskState.equals(TaskState.CLAIMED)) { - return false; - } else { - return currentTaskCallbackState.equals(CallbackState.CALLBACK_PROCESSING_REQUIRED); + return Optional.of( + new InvalidTaskStateException( + foundSummary.getTaskId(), foundSummary.getTaskState(), TaskState.CLAIMED)); } - + if (!currentTaskCallbackState.equals(CallbackState.CALLBACK_PROCESSING_REQUIRED)) { + return Optional.of( + new InvalidCallbackStateException( + foundSummary.getTaskId(), + currentTaskCallbackState, + CallbackState.CALLBACK_PROCESSING_REQUIRED)); + } + break; case CALLBACK_PROCESSING_REQUIRED: - return !currentTaskCallbackState.equals(CallbackState.CALLBACK_PROCESSING_COMPLETED); - + if (currentTaskCallbackState.equals(CallbackState.CALLBACK_PROCESSING_COMPLETED)) { + return Optional.of( + new InvalidCallbackStateException( + foundSummary.getTaskId(), + currentTaskCallbackState, + EnumUtil.allValuesExceptFor(CallbackState.CALLBACK_PROCESSING_COMPLETED))); + } + break; default: - return false; + return Optional.of( + new InvalidArgumentException( + String.format( + "desired callbackState has to be in '%s'", + Arrays.toString( + new CallbackState[] { + CallbackState.CALLBACK_PROCESSING_COMPLETED, + CallbackState.CLAIMED, + CallbackState.CALLBACK_PROCESSING_REQUIRED + })))); } + return Optional.empty(); } private void standardSettingsOnTaskCreation(TaskImpl task, Classification classification) @@ -1773,10 +1769,8 @@ public class TaskServiceImpl implements TaskService { // owner can only be changed if task is in state ready boolean isOwnerChanged = !Objects.equals(newTaskImpl1.getOwner(), oldTaskImpl.getOwner()); if (isOwnerChanged && oldTaskImpl.getState() != TaskState.READY) { - throw new InvalidStateException( - String.format( - "Task with id %s is in state %s and not in state ready.", - oldTaskImpl.getId(), oldTaskImpl.getState())); + throw new InvalidTaskStateException( + oldTaskImpl.getId(), oldTaskImpl.getState(), TaskState.READY); } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskTransferrer.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskTransferrer.java index 8041acc4a..5dd5dfba3 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskTransferrer.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskTransferrer.java @@ -16,12 +16,14 @@ import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.api.exceptions.TaskanaException; import pro.taskana.common.internal.InternalTaskanaEngine; +import pro.taskana.common.internal.util.EnumUtil; import pro.taskana.common.internal.util.IdGenerator; import pro.taskana.common.internal.util.ObjectAttributeChangeDetector; import pro.taskana.spi.history.api.events.task.TaskTransferredEvent; import pro.taskana.spi.history.internal.HistoryEventManager; import pro.taskana.task.api.TaskState; import pro.taskana.task.api.exceptions.InvalidStateException; +import pro.taskana.task.api.exceptions.InvalidTaskStateException; import pro.taskana.task.api.exceptions.TaskNotFoundException; import pro.taskana.task.api.models.Task; import pro.taskana.task.api.models.TaskSummary; @@ -29,6 +31,7 @@ import pro.taskana.task.internal.models.TaskImpl; import pro.taskana.task.internal.models.TaskSummaryImpl; import pro.taskana.workbasket.api.WorkbasketPermission; import pro.taskana.workbasket.api.WorkbasketService; +import pro.taskana.workbasket.api.exceptions.MismatchedWorkbasketPermissionException; import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; import pro.taskana.workbasket.api.models.WorkbasketSummary; import pro.taskana.workbasket.internal.WorkbasketQueryImpl; @@ -154,8 +157,8 @@ final class TaskTransferrer { Task task, WorkbasketSummary destinationWorkbasket, WorkbasketSummary originWorkbasket) throws NotAuthorizedException, WorkbasketNotFoundException, InvalidStateException { if (task.getState().isEndState()) { - throw new InvalidStateException( - String.format("Task '%s' is in end state and cannot be transferred.", task.getId())); + throw new InvalidTaskStateException( + task.getId(), task.getState(), EnumUtil.allValuesExceptFor(TaskState.END_STATES)); } workbasketService.checkAuthorization(originWorkbasket.getId(), WorkbasketPermission.TRANSFER); checkDestinationWorkbasket(destinationWorkbasket); @@ -167,9 +170,7 @@ final class TaskTransferrer { destinationWorkbasket.getId(), WorkbasketPermission.APPEND); if (destinationWorkbasket.isMarkedForDeletion()) { - throw new WorkbasketNotFoundException( - destinationWorkbasket.getId(), - String.format("Workbasket '%s' was marked for deletion.", destinationWorkbasket.getId())); + throw new WorkbasketNotFoundException(destinationWorkbasket.getId()); } } @@ -204,18 +205,17 @@ final class TaskTransferrer { if (taskId == null || taskId.isEmpty()) { error = new InvalidArgumentException("TaskId should not be null or empty"); } else if (taskSummary == null) { - error = - new TaskNotFoundException( - taskId, String.format("Task with id '%s' was not found.", taskId)); + error = new TaskNotFoundException(taskId); } else if (taskSummary.getState().isEndState()) { error = - new InvalidStateException( - String.format("Task in end state with id '%s' cannot be transferred.", taskId)); + new InvalidTaskStateException( + taskId, taskSummary.getState(), EnumUtil.allValuesExceptFor(TaskState.END_STATES)); } else if (!sourceWorkbasketIds.contains(taskSummary.getWorkbasketSummary().getId())) { error = - new NotAuthorizedException( - "The workbasket of this task got not TRANSFER permissions. TaskId=" + taskId, - taskanaEngine.getEngine().getCurrentUserContext().getUserid()); + new MismatchedWorkbasketPermissionException( + taskanaEngine.getEngine().getCurrentUserContext().getUserid(), + taskSummary.getWorkbasketSummary().getId(), + WorkbasketPermission.TRANSFER); } return Optional.ofNullable(error); } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/jobs/TaskCleanupJob.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/jobs/TaskCleanupJob.java index e2d587de8..2ec73089e 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/jobs/TaskCleanupJob.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/jobs/TaskCleanupJob.java @@ -17,6 +17,7 @@ import pro.taskana.common.api.TaskanaEngine; import pro.taskana.common.api.TimeInterval; import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.NotAuthorizedException; +import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.common.api.exceptions.TaskanaException; import pro.taskana.common.internal.JobServiceImpl; import pro.taskana.common.internal.jobs.AbstractTaskanaJob; @@ -62,7 +63,7 @@ public class TaskCleanupJob extends AbstractTaskanaJob { LOGGER.info("Job ended successfully. {} tasks deleted.", totalNumberOfTasksDeleted); } catch (Exception e) { - throw new TaskanaException("Error while processing TaskCleanupJob.", e); + throw new SystemException("Error while processing TaskCleanupJob.", e); } finally { scheduleNextCleanupJob(); } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/jobs/TaskRefreshJob.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/jobs/TaskRefreshJob.java index 56bd490a2..6594f9bca 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/jobs/TaskRefreshJob.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/jobs/TaskRefreshJob.java @@ -8,6 +8,7 @@ import org.slf4j.LoggerFactory; import pro.taskana.common.api.ScheduledJob; import pro.taskana.common.api.TaskanaEngine; +import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.common.api.exceptions.TaskanaException; import pro.taskana.common.internal.jobs.AbstractTaskanaJob; import pro.taskana.common.internal.transaction.TaskanaTransactionProvider; @@ -43,7 +44,7 @@ public class TaskRefreshJob extends AbstractTaskanaJob { affectedTaskIds, serviceLevelChanged, priorityChanged); LOGGER.info("TaskRefreshJob ended successfully."); } catch (Exception e) { - throw new TaskanaException("Error while processing TaskRefreshJob.", e); + throw new SystemException("Error while processing TaskRefreshJob.", e); } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/WorkbasketService.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/WorkbasketService.java index 29aa2d257..9ddb1dc65 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/WorkbasketService.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/WorkbasketService.java @@ -8,7 +8,6 @@ import pro.taskana.common.api.exceptions.DomainNotFoundException; import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.api.exceptions.TaskanaException; -import pro.taskana.workbasket.api.exceptions.InvalidWorkbasketException; import pro.taskana.workbasket.api.exceptions.WorkbasketAccessItemAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketInUseException; @@ -56,14 +55,14 @@ public interface WorkbasketService { * * @param workbasket The Workbasket to create * @return the created and inserted Workbasket - * @throws InvalidWorkbasketException If a required property of the Workbasket is not set. + * @throws InvalidArgumentException If a required property of the Workbasket is not set. * @throws NotAuthorizedException if the current user is not member of role BUSINESS_ADMIN or * ADMIN * @throws WorkbasketAlreadyExistException if the Workbasket exists already * @throws DomainNotFoundException if the domain does not exist in the configuration. */ Workbasket createWorkbasket(Workbasket workbasket) - throws InvalidWorkbasketException, NotAuthorizedException, WorkbasketAlreadyExistException, + throws InvalidArgumentException, NotAuthorizedException, WorkbasketAlreadyExistException, DomainNotFoundException; /** @@ -71,20 +70,21 @@ public interface WorkbasketService { * * @param workbasket The Workbasket to update * @return the updated Workbasket - * @throws InvalidWorkbasketException if workbasket name or type is invalid - * @throws NotAuthorizedException if the current user is not authorized to update the work basket - * @throws WorkbasketNotFoundException if the workbasket cannot be found. - * @throws ConcurrencyException if an attempt is made to update the workbasket and another user - * updated it already + * @throws InvalidArgumentException if workbasket name or type is invalid + * @throws NotAuthorizedException if the current user is not authorized to update the {@linkplain + * Workbasket} + * @throws WorkbasketNotFoundException if the {@linkplain Workbasket} cannot be found. + * @throws ConcurrencyException if an attempt is made to update the {@linkplain Workbasket} and + * another user updated it already */ Workbasket updateWorkbasket(Workbasket workbasket) - throws InvalidWorkbasketException, NotAuthorizedException, WorkbasketNotFoundException, + throws InvalidArgumentException, NotAuthorizedException, WorkbasketNotFoundException, ConcurrencyException; /** * Returns a new WorkbasketAccessItem which is not inserted. * - * @param workbasketId the workbasket id used to identify the referenced workbasket + * @param workbasketId the workbasket id used to identify the referenced {@linkplain Workbasket} * @param accessId the group id or user id for which access is controlled * @return new WorkbasketAccessItem */ @@ -138,7 +138,8 @@ public interface WorkbasketService { * specified, the current user needs all of them. * @throws NotAuthorizedException if the current user has not the requested authorization for the * specified workbasket - * @throws WorkbasketNotFoundException if the workbasket cannot be found for the given ID. + * @throws WorkbasketNotFoundException if the {@linkplain Workbasket} cannot be found for the + * given {@linkplain Workbasket#getId() id}. */ void checkAuthorization(String workbasketId, WorkbasketPermission... permission) throws NotAuthorizedException, WorkbasketNotFoundException; @@ -159,21 +160,24 @@ public interface WorkbasketService { throws NotAuthorizedException, WorkbasketNotFoundException; /** - * Get all {@link WorkbasketAccessItem s} for a Workbasket. + * Get all {@link WorkbasketAccessItem s} for a {@linkplain Workbasket}. * - * @param workbasketId the id of the Workbasket - * @return List of WorkbasketAccessItems for the Workbasket with workbasketKey - * @throws NotAuthorizedException if the current user is not member of role BUSINESS_ADMIN or - * ADMIN + * @param workbasketId the {@linkplain Workbasket#getId() id} of the {@linkplain Workbasket} + * @return List of {@linkplain WorkbasketAccessItem}s for the {@linkplain Workbasket} + * @throws NotAuthorizedException if the current user is not member of role {@linkplain + * pro.taskana.common.api.TaskanaRole#BUSINESS_ADMIN} or {@linkplain + * pro.taskana.common.api.TaskanaRole#ADMIN} + * @throws WorkbasketNotFoundException if the {@linkplain Workbasket} cannot be found for the + * given {@linkplain Workbasket#getId() id}. */ List getWorkbasketAccessItems(String workbasketId) - throws NotAuthorizedException; + throws NotAuthorizedException, WorkbasketNotFoundException; /** * Setting up the new WorkbasketAccessItems for a Workbasket. Already stored values will be * completely replaced by the current ones. * - *

Preconditions for each {@link WorkbasketAccessItem} in {@code wbAccessItems}: + *

Preconditions for each {@link WorkbasketAccessItem} then {@code wbAccessItems}: * *

    *
  • {@link WorkbasketAccessItem#getWorkbasketId()} is not null @@ -189,10 +193,12 @@ public interface WorkbasketService { * ADMIN * @throws WorkbasketAccessItemAlreadyExistException if {@code wbAccessItems} contains multiple * accessItems with the same accessId. + * @throws WorkbasketNotFoundException if the {@linkplain Workbasket} cannot be found for the + * given {@linkplain Workbasket#getId() id}. */ void setWorkbasketAccessItems(String workbasketId, List wbAccessItems) throws InvalidArgumentException, NotAuthorizedException, - WorkbasketAccessItemAlreadyExistException; + WorkbasketAccessItemAlreadyExistException, WorkbasketNotFoundException; /** * This method provides a query builder for querying the database. diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/InvalidWorkbasketException.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/InvalidWorkbasketException.java deleted file mode 100644 index 48a1be67e..000000000 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/InvalidWorkbasketException.java +++ /dev/null @@ -1,14 +0,0 @@ -package pro.taskana.workbasket.api.exceptions; - -import pro.taskana.common.api.exceptions.TaskanaException; - -/** - * This exception is thrown when a request is made to insert or update a workbasket that is missing - * a required property. - */ -public class InvalidWorkbasketException extends TaskanaException { - - public InvalidWorkbasketException(String msg) { - super(msg); - } -} diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/MismatchedWorkbasketPermissionException.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/MismatchedWorkbasketPermissionException.java new file mode 100644 index 000000000..3d1780287 --- /dev/null +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/MismatchedWorkbasketPermissionException.java @@ -0,0 +1,96 @@ +package pro.taskana.workbasket.api.exceptions; + +import java.util.Arrays; + +import pro.taskana.common.api.exceptions.ErrorCode; +import pro.taskana.common.api.exceptions.NotAuthorizedException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.workbasket.api.WorkbasketPermission; +import pro.taskana.workbasket.api.models.Workbasket; + +/** + * This exception is thrown when the current user does not have a certain {@linkplain + * WorkbasketPermission permission} on a {@linkplain Workbasket}. + */ +public class MismatchedWorkbasketPermissionException extends NotAuthorizedException { + + public static final String ERROR_KEY_KEY_DOMAIN = "WORKBASKET_WITH_KEY_MISMATCHED_PERMISSION"; + public static final String ERROR_KEY_ID = "WORKBASKET_WITH_ID_MISMATCHED_PERMISSION"; + private final String currentUserId; + private final WorkbasketPermission[] requiredPermissions; + private final String workbasketId; + private final String workbasketKey; + private final String domain; + + public MismatchedWorkbasketPermissionException( + String currentUserId, String workbasketId, WorkbasketPermission... requiredPermissions) { + super( + String.format( + "Not authorized. The current user '%s' has no '%s' permission(s) for Workbasket '%s'.", + currentUserId, Arrays.toString(requiredPermissions), workbasketId), + ErrorCode.of( + ERROR_KEY_ID, + MapCreator.of( + "currentUserId", + currentUserId, + "workbasketId", + workbasketId, + "requiredPermissions", + requiredPermissions))); + + this.currentUserId = currentUserId; + this.requiredPermissions = requiredPermissions; + this.workbasketId = workbasketId; + workbasketKey = null; + domain = null; + } + + public MismatchedWorkbasketPermissionException( + String currentUserId, + String workbasketKey, + String domain, + WorkbasketPermission... requiredPermissions) { + super( + String.format( + "Not authorized. The current user '%s' has no '%s' permission for " + + "Workbasket with key '%s' in domain '%s'.", + currentUserId, Arrays.toString(requiredPermissions), workbasketKey, domain), + ErrorCode.of( + ERROR_KEY_KEY_DOMAIN, + MapCreator.of( + "currentUserId", + currentUserId, + "workbasketKey", + workbasketKey, + "domain", + domain, + "requiredPermissions", + requiredPermissions))); + + this.currentUserId = currentUserId; + this.requiredPermissions = requiredPermissions; + this.workbasketKey = workbasketKey; + this.domain = domain; + workbasketId = null; + } + + public String getWorkbasketKey() { + return workbasketKey; + } + + public String getDomain() { + return domain; + } + + public String getWorkbasketId() { + return workbasketId; + } + + public WorkbasketPermission[] getRequiredPermissions() { + return requiredPermissions; + } + + public String getCurrentUserId() { + return currentUserId; + } +} diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/NotAuthorizedToQueryWorkbasketException.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/NotAuthorizedToQueryWorkbasketException.java index afe31b2e7..90ae93d8a 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/NotAuthorizedToQueryWorkbasketException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/NotAuthorizedToQueryWorkbasketException.java @@ -1,15 +1,14 @@ package pro.taskana.workbasket.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaRuntimeException; +import pro.taskana.workbasket.api.models.Workbasket; -/** This exception is used to communicate that a user is not authorized to query a Workbasket. */ +/** This exception is thrown when a user is not authorized to query a {@linkplain Workbasket}. */ public class NotAuthorizedToQueryWorkbasketException extends TaskanaRuntimeException { - public NotAuthorizedToQueryWorkbasketException(String msg) { - super(msg); - } - - public NotAuthorizedToQueryWorkbasketException(String msg, Throwable cause) { - super(msg, cause); + public NotAuthorizedToQueryWorkbasketException( + String message, ErrorCode errorCode, Throwable cause) { + super(message, errorCode, cause); } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketAccessItemAlreadyExistException.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketAccessItemAlreadyExistException.java index 5d93ef5ff..61598a6c4 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketAccessItemAlreadyExistException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketAccessItemAlreadyExistException.java @@ -1,15 +1,35 @@ package pro.taskana.workbasket.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; import pro.taskana.workbasket.api.models.WorkbasketAccessItem; +/** + * This exception is thrown when an already existing {@linkplain WorkbasketAccessItem} was tried to + * be created. + */ public class WorkbasketAccessItemAlreadyExistException extends TaskanaException { - public WorkbasketAccessItemAlreadyExistException(WorkbasketAccessItem accessItem) { + public static final String ERROR_KEY = "WORKBASKET_ACCESS_ITEM_ALREADY_EXISTS"; + private final String accessId; + private final String workbasketId; + + public WorkbasketAccessItemAlreadyExistException(String accessId, String workbasketId) { super( String.format( - "WorkbasketAccessItem for accessId '%s' " - + "and WorkbasketId '%s', WorkbasketKey '%s' exists already.", - accessItem.getAccessId(), accessItem.getWorkbasketId(), accessItem.getWorkbasketKey())); + "WorkbasketAccessItem with access id '%s' and workbasket id '%s' already exists.", + accessId, workbasketId), + ErrorCode.of(ERROR_KEY, MapCreator.of("accessId", accessId, "workbasketId", workbasketId))); + this.accessId = accessId; + this.workbasketId = workbasketId; + } + + public String getAccessId() { + return accessId; + } + + public String getWorkbasketId() { + return workbasketId; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketAlreadyExistException.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketAlreadyExistException.java index 183ec8301..41cc16311 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketAlreadyExistException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketAlreadyExistException.java @@ -1,17 +1,33 @@ package pro.taskana.workbasket.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; import pro.taskana.workbasket.api.models.Workbasket; -/** Thrown, when a workbasket does already exits, but wanted to create with same ID. */ +/** + * This exception is thrown when an already existing {@linkplain Workbasket} was tried to be + * created. + */ public class WorkbasketAlreadyExistException extends TaskanaException { - public WorkbasketAlreadyExistException(Workbasket workbasket) { + public static final String ERROR_KEY = "WORKBASKET_ALREADY_EXISTS"; + private final String key; + private final String domain; + + public WorkbasketAlreadyExistException(String key, String domain) { super( - "A workbasket with key '" - + workbasket.getKey() - + "' already exists in domain '" - + workbasket.getDomain() - + "'."); + String.format("A Workbasket with key '%s' already exists in domain '%s'.", key, domain), + ErrorCode.of(ERROR_KEY, MapCreator.of("workbasketKey", key, "domain", domain))); + this.key = key; + this.domain = domain; + } + + public String getKey() { + return key; + } + + public String getDomain() { + return domain; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketInUseException.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketInUseException.java index df20cf07d..8d07ccbe6 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketInUseException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketInUseException.java @@ -1,11 +1,29 @@ package pro.taskana.workbasket.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.workbasket.api.models.Workbasket; -/** Thrown if a specific Workbasket does have content and should be deleted. */ +/** + * This exception is thrown when a specific {@linkplain Workbasket} does have content and is tried + * to be deleted. + */ public class WorkbasketInUseException extends TaskanaException { - public WorkbasketInUseException(String msg) { - super(msg); + public static final String ERROR_KEY = "WORKBASKET_IN_USE"; + private final String workbasketId; + + public WorkbasketInUseException(String workbasketId) { + super( + String.format( + "Workbasket '%s' contains non-completed Tasks and can't be marked for deletion.", + workbasketId), + ErrorCode.of(ERROR_KEY, MapCreator.of("workbasketId", workbasketId))); + this.workbasketId = workbasketId; + } + + public String getWorkbasketId() { + return workbasketId; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketMarkedForDeletionException.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketMarkedForDeletionException.java new file mode 100644 index 000000000..576a21b12 --- /dev/null +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketMarkedForDeletionException.java @@ -0,0 +1,29 @@ +package pro.taskana.workbasket.api.exceptions; + +import pro.taskana.common.api.exceptions.ErrorCode; +import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.workbasket.api.models.Workbasket; + +/** + * This exception is thrown when a {@linkplain Workbasket}, which was {@linkplain + * Workbasket#isMarkedForDeletion() marked for deletion}, could not be deleted. + */ +public class WorkbasketMarkedForDeletionException extends TaskanaException { + + public static final String ERROR_KEY = "WORKBASKET_MARKED_FOR_DELETION"; + private final String workbasketId; + + public WorkbasketMarkedForDeletionException(String workbasketId) { + super( + String.format( + "Workbasket with id '%s' could not be deleted, but was marked for deletion.", + workbasketId), + ErrorCode.of(ERROR_KEY, MapCreator.of("workbasketId", workbasketId))); + this.workbasketId = workbasketId; + } + + public String getWorkbasketId() { + return workbasketId; + } +} diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketNotFoundException.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketNotFoundException.java index 0ca048c61..8deffaf7a 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketNotFoundException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/exceptions/WorkbasketNotFoundException.java @@ -1,23 +1,41 @@ package pro.taskana.workbasket.api.exceptions; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.NotFoundException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.workbasket.api.models.Workbasket; -/** This exception will be thrown if a specific workbasket is not in the database. */ +/** This exception is thrown when a specific {@linkplain Workbasket} is not in the database. */ public class WorkbasketNotFoundException extends NotFoundException { - private String key; - private String domain; + public static final String ERROR_KEY_ID = "WORKBASKET_WITH_ID_NOT_FOUND"; + public static final String ERROR_KEY_KEY_DOMAIN = "WORKBASKET_WITH_KEY_NOT_FOUND"; + private final String id; + private final String key; + private final String domain; - public WorkbasketNotFoundException(String id, String msg) { - super(id, msg); + public WorkbasketNotFoundException(String id) { + super( + String.format("Workbasket with id '%s' was not found.", id), + ErrorCode.of(ERROR_KEY_ID, MapCreator.of("workbasketId", id))); + this.id = id; + key = null; + domain = null; } - public WorkbasketNotFoundException(String key, String domain, String msg) { - super(null, msg); + public WorkbasketNotFoundException(String key, String domain) { + super( + String.format("Workbasket with key '%s' and domain '%s' was not found.", key, domain), + ErrorCode.of(ERROR_KEY_KEY_DOMAIN, MapCreator.of("workbasketKey", key, "domain", domain))); + id = null; this.key = key; this.domain = domain; } + public String getId() { + return id; + } + public String getKey() { return key; } diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/AbstractWorkbasketAccessItemQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/AbstractWorkbasketAccessItemQueryImpl.java index 3ff1c7120..d79bd98ff 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/AbstractWorkbasketAccessItemQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/AbstractWorkbasketAccessItemQueryImpl.java @@ -6,6 +6,7 @@ import java.util.List; import org.apache.ibatis.exceptions.PersistenceException; import org.apache.ibatis.session.RowBounds; +import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.common.api.exceptions.TaskanaRuntimeException; import pro.taskana.common.internal.InternalTaskanaEngine; import pro.taskana.workbasket.api.AbstractWorkbasketAccessItemQuery; @@ -93,7 +94,7 @@ abstract class AbstractWorkbasketAccessItemQueryImpl< } catch (PersistenceException e) { if (e.getMessage().contains("ERRORCODE=-4470")) { TaskanaRuntimeException ex = - new TaskanaRuntimeException( + new SystemException( "The offset beginning was set over the amount of result-rows.", e.getCause()); ex.setStackTrace(e.getStackTrace()); throw ex; diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketAccessItemQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketAccessItemQueryImpl.java index 3e8a58d4e..f19714a07 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketAccessItemQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketAccessItemQueryImpl.java @@ -6,6 +6,7 @@ import java.util.List; import org.apache.ibatis.exceptions.PersistenceException; import org.apache.ibatis.session.RowBounds; +import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.common.api.exceptions.TaskanaRuntimeException; import pro.taskana.common.internal.InternalTaskanaEngine; import pro.taskana.workbasket.api.AccessItemQueryColumnName; @@ -116,7 +117,7 @@ public class WorkbasketAccessItemQueryImpl implements WorkbasketAccessItemQuery } catch (PersistenceException e) { if (e.getMessage().contains("ERRORCODE=-4470")) { TaskanaRuntimeException ex = - new TaskanaRuntimeException( + new SystemException( "The offset beginning was set over the amount of result-rows.", e.getCause()); ex.setStackTrace(e.getStackTrace()); throw ex; diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketQueryImpl.java index fcb0a0f26..89e3e776f 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketQueryImpl.java @@ -177,10 +177,10 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { .checkRoleMembership(TaskanaRole.ADMIN, TaskanaRole.BUSINESS_ADMIN, TaskanaRole.TASK_ADMIN); // Checking pre-conditions if (permission == null) { - throw new InvalidArgumentException("Permission can´t be null."); + throw new InvalidArgumentException("Permission can't be null."); } if (accessIds == null || accessIds.length == 0) { - throw new InvalidArgumentException("accessIds can´t be NULL or empty."); + throw new InvalidArgumentException("accessIds can't be NULL or empty."); } // set up permissions and ids @@ -376,7 +376,7 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { } catch (PersistenceException e) { if (e.getMessage().contains("ERRORCODE=-4470")) { TaskanaRuntimeException ex = - new TaskanaRuntimeException( + new SystemException( "The offset beginning was set over the amount of result-rows.", e.getCause()); ex.setStackTrace(e.getStackTrace()); throw ex; diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketServiceImpl.java index d8515f845..dbaaeeca3 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketServiceImpl.java @@ -6,6 +6,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.stream.Stream; import org.apache.ibatis.exceptions.PersistenceException; @@ -40,10 +41,11 @@ import pro.taskana.workbasket.api.WorkbasketAccessItemQuery; import pro.taskana.workbasket.api.WorkbasketPermission; import pro.taskana.workbasket.api.WorkbasketQuery; import pro.taskana.workbasket.api.WorkbasketService; -import pro.taskana.workbasket.api.exceptions.InvalidWorkbasketException; +import pro.taskana.workbasket.api.exceptions.MismatchedWorkbasketPermissionException; import pro.taskana.workbasket.api.exceptions.WorkbasketAccessItemAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketInUseException; +import pro.taskana.workbasket.api.exceptions.WorkbasketMarkedForDeletionException; import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; import pro.taskana.workbasket.api.models.Workbasket; import pro.taskana.workbasket.api.models.WorkbasketAccessItem; @@ -82,10 +84,11 @@ public class WorkbasketServiceImpl implements WorkbasketService { try { taskanaEngine.openConnection(); result = workbasketMapper.findById(workbasketId); + if (result == null) { - throw new WorkbasketNotFoundException( - workbasketId, "Workbasket with id " + workbasketId + " was not found."); + throw new WorkbasketNotFoundException(workbasketId); } + if (!taskanaEngine .getEngine() .isUserInRole(TaskanaRole.ADMIN, TaskanaRole.BUSINESS_ADMIN, TaskanaRole.TASK_ADMIN)) { @@ -100,30 +103,25 @@ public class WorkbasketServiceImpl implements WorkbasketService { @Override public Workbasket getWorkbasket(String workbasketKey, String domain) throws WorkbasketNotFoundException, NotAuthorizedException { - Workbasket result; - try { - taskanaEngine.openConnection(); - result = workbasketMapper.findByKeyAndDomain(workbasketKey, domain); - if (result == null) { - throw new WorkbasketNotFoundException( - workbasketKey, - domain, - "Workbasket with key " + workbasketKey + " and domain " + domain + " was not found."); - } - if (!taskanaEngine - .getEngine() - .isUserInRole(TaskanaRole.ADMIN, TaskanaRole.BUSINESS_ADMIN, TaskanaRole.TASK_ADMIN)) { - this.checkAuthorization(workbasketKey, domain, WorkbasketPermission.READ); - } - return result; - } finally { - taskanaEngine.returnConnection(); + if (!taskanaEngine + .getEngine() + .isUserInRole(TaskanaRole.ADMIN, TaskanaRole.BUSINESS_ADMIN, TaskanaRole.TASK_ADMIN)) { + this.checkAuthorization(workbasketKey, domain, WorkbasketPermission.READ); } + + Workbasket workbasket = + taskanaEngine.openAndReturnConnection( + () -> workbasketMapper.findByKeyAndDomain(workbasketKey, domain)); + if (workbasket == null) { + throw new WorkbasketNotFoundException(workbasketKey, domain); + } + + return workbasket; } @Override public Workbasket createWorkbasket(Workbasket newWorkbasket) - throws InvalidWorkbasketException, NotAuthorizedException, WorkbasketAlreadyExistException, + throws InvalidArgumentException, NotAuthorizedException, WorkbasketAlreadyExistException, DomainNotFoundException { taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); @@ -136,7 +134,8 @@ public class WorkbasketServiceImpl implements WorkbasketService { Workbasket existingWorkbasket = workbasketMapper.findByKeyAndDomain(newWorkbasket.getKey(), newWorkbasket.getDomain()); if (existingWorkbasket != null) { - throw new WorkbasketAlreadyExistException(existingWorkbasket); + throw new WorkbasketAlreadyExistException( + existingWorkbasket.getKey(), existingWorkbasket.getDomain()); } if (workbasket.getId() == null || workbasket.getId().isEmpty()) { @@ -169,8 +168,8 @@ public class WorkbasketServiceImpl implements WorkbasketService { @Override public Workbasket updateWorkbasket(Workbasket workbasketToUpdate) - throws NotAuthorizedException, WorkbasketNotFoundException, ConcurrencyException, - InvalidWorkbasketException { + throws InvalidArgumentException, NotAuthorizedException, WorkbasketNotFoundException, + ConcurrencyException { taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); WorkbasketImpl workbasketImplToUpdate = (WorkbasketImpl) workbasketToUpdate; @@ -179,8 +178,21 @@ public class WorkbasketServiceImpl implements WorkbasketService { try { taskanaEngine.openConnection(); - Workbasket oldWorkbasket = - this.getWorkbasket(workbasketImplToUpdate.getKey(), workbasketImplToUpdate.getDomain()); + Workbasket oldWorkbasket; + + if (workbasketImplToUpdate.getId() == null || workbasketImplToUpdate.getId().isEmpty()) { + oldWorkbasket = + getWorkbasket(workbasketImplToUpdate.getKey(), workbasketImplToUpdate.getDomain()); + } else { + oldWorkbasket = getWorkbasket(workbasketImplToUpdate.getId()); + // changing key or domain is not allowed + if (!oldWorkbasket.getKey().equals(workbasketToUpdate.getKey()) + || !oldWorkbasket.getDomain().equals(workbasketToUpdate.getDomain())) { + throw new WorkbasketNotFoundException( + workbasketToUpdate.getKey(), workbasketToUpdate.getDomain()); + } + } + checkModifiedHasNotChanged(oldWorkbasket, workbasketImplToUpdate); workbasketImplToUpdate.setModified(Instant.now()); @@ -248,11 +260,7 @@ public class WorkbasketServiceImpl implements WorkbasketService { } WorkbasketImpl wb = workbasketMapper.findById(workbasketAccessItem.getWorkbasketId()); if (wb == null) { - throw new WorkbasketNotFoundException( - workbasketAccessItem.getWorkbasketId(), - String.format( - "WorkbasketAccessItem %s refers to a not existing workbasket", - workbasketAccessItem)); + throw new WorkbasketNotFoundException(workbasketAccessItem.getWorkbasketId()); } try { workbasketAccessMapper.insert(accessItem); @@ -286,7 +294,8 @@ public class WorkbasketServiceImpl implements WorkbasketService { "UC_ACCESSID_WBID_INDEX_E" // H2 ); if (accessItemExistsIdentifier.anyMatch(e.getMessage()::contains)) { - throw new WorkbasketAccessItemAlreadyExistException(accessItem); + throw new WorkbasketAccessItemAlreadyExistException( + accessItem.getAccessId(), accessItem.getWorkbasketId()); } throw e; } @@ -386,40 +395,26 @@ public class WorkbasketServiceImpl implements WorkbasketService { taskanaEngine.openConnection(); if (workbasketMapper.findById(workbasketId) == null) { - throw new WorkbasketNotFoundException( - workbasketId, "Workbasket with id " + workbasketId + " was not found."); + throw new WorkbasketNotFoundException(workbasketId); } if (skipAuthorizationCheck(requestedPermissions)) { return; } - List accessIds = taskanaEngine.getEngine().getCurrentUserContext().getAccessIds(); - WorkbasketAccessItem wbAcc = - workbasketAccessMapper.findByWorkbasketAndAccessId(workbasketId, accessIds); - if (wbAcc == null) { - throw new NotAuthorizedException( - "Not authorized. Permission '" - + Arrays.toString(requestedPermissions) - + "' on workbasket '" - + workbasketId - + "' is needed.", - taskanaEngine.getEngine().getCurrentUserContext().getUserid()); - } + Optional> grantedPermissions = + Optional.ofNullable( + workbasketAccessMapper.findByWorkbasketAndAccessId( + workbasketId, + taskanaEngine.getEngine().getCurrentUserContext().getAccessIds())) + .map(this::getPermissionsFromWorkbasketAccessItem); - List grantedPermissions = - this.getPermissionsFromWorkbasketAccessItem(wbAcc); - - for (WorkbasketPermission perm : requestedPermissions) { - if (!grantedPermissions.contains(perm)) { - throw new NotAuthorizedException( - "Not authorized. Permission '" - + perm.name() - + "' on workbasket '" - + workbasketId - + "' is needed.", - taskanaEngine.getEngine().getCurrentUserContext().getUserid()); - } + if (!grantedPermissions.isPresent() + || !grantedPermissions.get().containsAll(Arrays.asList(requestedPermissions))) { + throw new MismatchedWorkbasketPermissionException( + taskanaEngine.getEngine().getCurrentUserContext().getUserid(), + workbasketId, + requestedPermissions); } } finally { taskanaEngine.returnConnection(); @@ -434,44 +429,27 @@ public class WorkbasketServiceImpl implements WorkbasketService { taskanaEngine.openConnection(); if (workbasketMapper.findByKeyAndDomain(workbasketKey, domain) == null) { - throw new WorkbasketNotFoundException( - workbasketKey, - domain, - "Workbasket with key " + workbasketKey + " and domain " + domain + " was not found"); + throw new WorkbasketNotFoundException(workbasketKey, domain); } if (skipAuthorizationCheck(requestedPermissions)) { return; } - List accessIds = taskanaEngine.getEngine().getCurrentUserContext().getAccessIds(); - WorkbasketAccessItem wbAcc = - workbasketAccessMapper.findByWorkbasketKeyDomainAndAccessId( - workbasketKey, domain, accessIds); - if (wbAcc == null) { - throw new NotAuthorizedException( - "Not authorized. Permission '" - + Arrays.toString(requestedPermissions) - + "' on workbasket with key '" - + workbasketKey - + "' and domain '" - + domain - + "' is needed.", - taskanaEngine.getEngine().getCurrentUserContext().getUserid()); - } - List grantedPermissions = - this.getPermissionsFromWorkbasketAccessItem(wbAcc); - for (WorkbasketPermission perm : requestedPermissions) { - if (!grantedPermissions.contains(perm)) { - throw new NotAuthorizedException( - "Not authorized. Permission '" - + perm.name() - + "' on workbasket with key '" - + workbasketKey - + "' and domain '" - + domain - + "' is needed.", - taskanaEngine.getEngine().getCurrentUserContext().getUserid()); - } + Optional> grantedPermissions = + Optional.ofNullable( + workbasketAccessMapper.findByWorkbasketKeyDomainAndAccessId( + workbasketKey, + domain, + taskanaEngine.getEngine().getCurrentUserContext().getAccessIds())) + .map(this::getPermissionsFromWorkbasketAccessItem); + + if (!grantedPermissions.isPresent() + || !grantedPermissions.get().containsAll(Arrays.asList(requestedPermissions))) { + throw new MismatchedWorkbasketPermissionException( + taskanaEngine.getEngine().getCurrentUserContext().getUserid(), + workbasketKey, + domain, + requestedPermissions); } } finally { taskanaEngine.returnConnection(); @@ -480,7 +458,7 @@ public class WorkbasketServiceImpl implements WorkbasketService { @Override public List getWorkbasketAccessItems(String workbasketId) - throws NotAuthorizedException { + throws NotAuthorizedException, WorkbasketNotFoundException { taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); List result = new ArrayList<>(); try { @@ -498,15 +476,16 @@ public class WorkbasketServiceImpl implements WorkbasketService { public void setWorkbasketAccessItems( String workbasketId, List wbAccessItems) throws NotAuthorizedException, WorkbasketAccessItemAlreadyExistException, - InvalidArgumentException { + InvalidArgumentException, WorkbasketNotFoundException { taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); - Set ids = new HashSet<>(); Set accessItems = - checkAccessItemsPreconditionsAndSetId(workbasketId, ids, wbAccessItems); + checkAccessItemsPreconditionsAndSetId(workbasketId, wbAccessItems); try { taskanaEngine.openConnection(); + // this is necessary to verify that the requested workbasket exists. + getWorkbasket(workbasketId); List originalAccessItems = new ArrayList<>(); @@ -813,11 +792,7 @@ public class WorkbasketServiceImpl implements WorkbasketService { .runAsAdmin(() -> getCountTasksNotCompletedByWorkbasketId(workbasketId)); if (countTasksNotCompletedInWorkbasket > 0) { - String errorMessage = - String.format( - "Workbasket %s contains %s non-completed tasks and can´t be marked for deletion.", - workbasketId, countTasksNotCompletedInWorkbasket); - throw new WorkbasketInUseException(errorMessage); + throw new WorkbasketInUseException(workbasketId); } long countTasksInWorkbasket = @@ -872,25 +847,10 @@ public class WorkbasketServiceImpl implements WorkbasketService { if (!deleteWorkbasket(workbasketIdForDeleting)) { bulkLog.addError( workbasketIdForDeleting, - new WorkbasketInUseException( - "Workbasket with id " - + workbasketIdForDeleting - + " contains completed tasks not deleted and will not be deleted.")); + new WorkbasketMarkedForDeletionException(workbasketIdForDeleting)); } - } catch (WorkbasketInUseException ex) { - bulkLog.addError( - workbasketIdForDeleting, - new WorkbasketInUseException( - "Workbasket with id " - + workbasketIdForDeleting - + " is in use and will not be deleted.")); } catch (TaskanaException ex) { - bulkLog.addError( - workbasketIdForDeleting, - new TaskanaException( - "Workbasket with id " - + workbasketIdForDeleting - + " Throw an exception and couldn't be deleted.")); + bulkLog.addError(workbasketIdForDeleting, ex); } } return bulkLog; @@ -990,22 +950,18 @@ public class WorkbasketServiceImpl implements WorkbasketService { Workbasket oldWorkbasket, WorkbasketImpl workbasketImplToUpdate) throws ConcurrencyException { if (!oldWorkbasket.getModified().equals(workbasketImplToUpdate.getModified())) { - - throw new ConcurrencyException( - "The current Workbasket has been modified while editing. " - + "The values can not be updated. Workbasket " - + workbasketImplToUpdate); + throw new ConcurrencyException(); } } private Set checkAccessItemsPreconditionsAndSetId( - String workbasketId, Set ids, List wbAccessItems) + String workbasketId, List wbAccessItems) throws InvalidArgumentException, WorkbasketAccessItemAlreadyExistException { + Set ids = new HashSet<>(); Set accessItems = new HashSet<>(); for (WorkbasketAccessItem workbasketAccessItem : wbAccessItems) { - if (workbasketAccessItem != null) { WorkbasketAccessItemImpl wbAccessItemImpl = (WorkbasketAccessItemImpl) workbasketAccessItem; @@ -1027,7 +983,8 @@ public class WorkbasketServiceImpl implements WorkbasketService { IdGenerator.generateWithPrefix(IdGenerator.ID_PREFIX_WORKBASKET_AUTHORIZATION)); } if (ids.contains(wbAccessItemImpl.getAccessId())) { - throw new WorkbasketAccessItemAlreadyExistException(wbAccessItemImpl); + throw new WorkbasketAccessItemAlreadyExistException( + wbAccessItemImpl.getAccessId(), wbAccessItemImpl.getWorkbasketId()); } ids.add(wbAccessItemImpl.getAccessId()); accessItems.add(wbAccessItemImpl); @@ -1084,44 +1041,42 @@ public class WorkbasketServiceImpl implements WorkbasketService { } private void validateWorkbasket(Workbasket workbasket) - throws InvalidWorkbasketException, DomainNotFoundException { + throws DomainNotFoundException, InvalidArgumentException { // check that required properties (database not null) are set validateNameAndType(workbasket); if (workbasket.getId() == null || workbasket.getId().length() == 0) { - throw new InvalidWorkbasketException("Id must not be null for " + workbasket); + throw new InvalidArgumentException("Id must not be null for " + workbasket); } if (workbasket.getKey() == null || workbasket.getKey().length() == 0) { - throw new InvalidWorkbasketException("Key must not be null for " + workbasket); + throw new InvalidArgumentException("Key must not be null for " + workbasket); } if (workbasket.getDomain() == null) { - throw new InvalidWorkbasketException("Domain must not be null for " + workbasket); + throw new InvalidArgumentException("Domain must not be null for " + workbasket); } if (!taskanaEngine.domainExists(workbasket.getDomain())) { - throw new DomainNotFoundException( - workbasket.getDomain(), - "Domain " + workbasket.getDomain() + " does not exist in the configuration."); + throw new DomainNotFoundException(workbasket.getDomain()); } } private void validateId(String workbasketId) throws InvalidArgumentException { if (workbasketId == null) { - throw new InvalidArgumentException("The WorkbasketId can´t be NULL"); + throw new InvalidArgumentException("The WorkbasketId can't be NULL"); } if (workbasketId.isEmpty()) { - throw new InvalidArgumentException("The WorkbasketId can´t be EMPTY for deleteWorkbasket()"); + throw new InvalidArgumentException("The WorkbasketId can't be EMPTY for deleteWorkbasket()"); } } - private void validateNameAndType(Workbasket workbasket) throws InvalidWorkbasketException { + private void validateNameAndType(Workbasket workbasket) throws InvalidArgumentException { if (workbasket.getName() == null) { - throw new InvalidWorkbasketException("Name must not be NULL for " + workbasket); + throw new InvalidArgumentException("Name must not be NULL for " + workbasket); } if (workbasket.getName().length() == 0) { - throw new InvalidWorkbasketException("Name must not be EMPTY for " + workbasket); + throw new InvalidArgumentException("Name must not be EMPTY for " + workbasket); } if (workbasket.getType() == null) { - throw new InvalidWorkbasketException("Type must not be NULL for " + workbasket); + throw new InvalidArgumentException("Type must not be NULL for " + workbasket); } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/jobs/WorkbasketCleanupJob.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/jobs/WorkbasketCleanupJob.java index f60b5d3b9..1938799fd 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/jobs/WorkbasketCleanupJob.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/jobs/WorkbasketCleanupJob.java @@ -11,6 +11,7 @@ import pro.taskana.common.api.ScheduledJob.Type; import pro.taskana.common.api.TaskanaEngine; import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.NotAuthorizedException; +import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.common.api.exceptions.TaskanaException; import pro.taskana.common.internal.JobServiceImpl; import pro.taskana.common.internal.jobs.AbstractTaskanaJob; @@ -49,7 +50,7 @@ public class WorkbasketCleanupJob extends AbstractTaskanaJob { LOGGER.info( "Job ended successfully. {} workbaskets deleted.", totalNumberOfWorkbasketDeleted); } catch (Exception e) { - throw new TaskanaException("Error while processing WorkbasketCleanupJob.", e); + throw new SystemException("Error while processing WorkbasketCleanupJob.", e); } finally { scheduleNextCleanupJob(); } diff --git a/lib/taskana-core/src/test/java/acceptance/ArchitectureTest.java b/lib/taskana-core/src/test/java/acceptance/ArchitectureTest.java index 805df0bf2..7993ab47e 100644 --- a/lib/taskana-core/src/test/java/acceptance/ArchitectureTest.java +++ b/lib/taskana-core/src/test/java/acceptance/ArchitectureTest.java @@ -1,6 +1,7 @@ package acceptance; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.methods; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses; import static com.tngtech.archunit.library.GeneralCodingRules.NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS; import static com.tngtech.archunit.library.GeneralCodingRules.NO_CLASSES_SHOULD_THROW_GENERIC_EXCEPTIONS; @@ -29,12 +30,17 @@ import org.apache.ibatis.annotations.Insert; import org.apache.ibatis.annotations.Select; import org.apache.ibatis.annotations.Update; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; import org.junit.jupiter.api.function.ThrowingConsumer; +import pro.taskana.common.api.exceptions.ErrorCode; +import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.api.exceptions.TaskanaRuntimeException; import pro.taskana.common.internal.logging.LoggingAspect; +import pro.taskana.common.internal.util.MapCreator; /** * Test architecture of classes in taskana. For more info and examples see @@ -80,7 +86,42 @@ class ArchitectureTest { .should() .onlyDependOnClassesThat( Predicates.resideOutsideOfPackage("..pro.taskana..internal..") - .or(Predicates.assignableTo(LoggingAspect.class))); + .or( + Predicates.assignableTo(LoggingAspect.class) + .or(Predicates.assignableTo(MapCreator.class)))); + myRule.check(importedClasses); + } + + @Test + void utilityClassesShouldNotBeInitializable() { + ArchRule myRule = + classes() + .that() + .resideInAPackage("..util..") + .and() + .areNotNestedClasses() + .should() + .haveOnlyPrivateConstructors(); + + myRule.check(importedClasses); + } + + @Test + @Disabled("until we have renamed all tests") + void testMethodNamesShouldMatchAccordingToOurGuidelines() { + ArchRule myRule = + methods() + .that() + .areAnnotatedWith(Test.class) + .or() + .areAnnotatedWith(TestFactory.class) + .and() + .areNotDeclaredIn(ArchitectureTest.class) + .should() + .bePackagePrivate() + .andShould() + .haveNameMatching("^should_[A-Z][^_]+_(For|When)_[A-Z][^_]+$"); + myRule.check(importedClasses); } @@ -99,7 +140,19 @@ class ArchitectureTest { @Test void onlyExceptionsShouldResideInExceptionPackage() { ArchRule myRule = - classes().that().resideInAPackage("..exceptions").should().beAssignableTo(Throwable.class); + classes() + .that() + .resideInAPackage("..exceptions") + .and() + .doNotBelongToAnyOf(ErrorCode.class) + .should() + .beAssignableTo( + Predicates.assignableTo(TaskanaException.class) + .or(Predicates.assignableTo(TaskanaRuntimeException.class))) + .andShould() + .bePublic() + .andShould() + .haveSimpleNameEndingWith("Exception"); myRule.check(importedClasses); } diff --git a/lib/taskana-core/src/test/java/acceptance/TaskTestMapper.java b/lib/taskana-core/src/test/java/acceptance/TaskTestMapper.java index c9a720685..40a7ba99e 100644 --- a/lib/taskana-core/src/test/java/acceptance/TaskTestMapper.java +++ b/lib/taskana-core/src/test/java/acceptance/TaskTestMapper.java @@ -12,7 +12,6 @@ import pro.taskana.common.internal.persistence.MapTypeHandler; import pro.taskana.task.internal.models.TaskImpl; /** This class contains specific mybatis mappings for task tests. */ - @SuppressWarnings({"checkstyle:LineLength"}) public interface TaskTestMapper { diff --git a/lib/taskana-core/src/test/java/acceptance/classification/CreateClassificationAccTest.java b/lib/taskana-core/src/test/java/acceptance/classification/CreateClassificationAccTest.java index 60b1c24eb..cfedca2fa 100644 --- a/lib/taskana-core/src/test/java/acceptance/classification/CreateClassificationAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/classification/CreateClassificationAccTest.java @@ -11,6 +11,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import pro.taskana.classification.api.ClassificationService; import pro.taskana.classification.api.exceptions.ClassificationAlreadyExistException; +import pro.taskana.classification.api.exceptions.MalformedServiceLevelException; import pro.taskana.classification.api.models.Classification; import pro.taskana.classification.internal.models.ClassificationImpl; import pro.taskana.common.api.exceptions.DomainNotFoundException; @@ -146,7 +147,7 @@ class CreateClassificationAccTest extends AbstractAccTest { classification.setServiceLevel("P-1D"); assertThatThrownBy(() -> CLASSIFICATION_SERVICE.createClassification(classification)) - .isInstanceOf(InvalidArgumentException.class); + .isInstanceOf(MalformedServiceLevelException.class); } @WithAccessId(user = "businessadmin") @@ -166,7 +167,7 @@ class CreateClassificationAccTest extends AbstractAccTest { CLASSIFICATION_SERVICE.newClassification("Key2", "DOMAIN_B", "TASK"); classification2.setServiceLevel("abc"); assertThatThrownBy(() -> CLASSIFICATION_SERVICE.createClassification(classification2)) - .isInstanceOf(InvalidArgumentException.class); + .isInstanceOf(MalformedServiceLevelException.class); } @WithAccessId(user = "businessadmin") 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 818c2efb5..4bbfbdb93 100644 --- a/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java @@ -141,7 +141,7 @@ class UpdateClassificationAccTest extends AbstractAccTest { Thread.sleep(20); // to avoid identity of modified timestamps between classification and base classificationService.updateClassification(base); - classification.setName("NOW IT´S MY TURN"); + classification.setName("NOW IT'S MY TURN"); classification.setDescription("IT SHOULD BE TO LATE..."); ThrowingCallable call = () -> classificationService.updateClassification(classification); assertThatThrownBy(call).isInstanceOf(ConcurrencyException.class); diff --git a/lib/taskana-core/src/test/java/acceptance/task/CallbackStateAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/CallbackStateAccTest.java index 01a6220d6..8dedc984c 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/CallbackStateAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/CallbackStateAccTest.java @@ -5,6 +5,7 @@ import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import acceptance.AbstractAccTest; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -20,6 +21,7 @@ import pro.taskana.common.test.security.WithAccessId; import pro.taskana.task.api.CallbackState; import pro.taskana.task.api.TaskService; import pro.taskana.task.api.TaskState; +import pro.taskana.task.api.exceptions.InvalidCallbackStateException; import pro.taskana.task.api.exceptions.InvalidStateException; import pro.taskana.task.api.models.Task; import pro.taskana.task.api.models.TaskSummary; @@ -64,37 +66,42 @@ class CallbackStateAccTest extends AbstractAccTest { .isEqualTo(CallbackState.CALLBACK_PROCESSING_REQUIRED); assertThat(createdTask.getState()).isEqualTo(TaskState.READY); - String endOfMessage = " cannot be deleted because its callback is not yet processed"; - ThrowingCallable call = - () -> { - taskService.forceDeleteTask(createdTask.getId()); - }; + ThrowingCallable call = () -> taskService.forceDeleteTask(createdTask.getId()); + CallbackState[] expectedCallbackStates = { + CallbackState.NONE, CallbackState.CLAIMED, CallbackState.CALLBACK_PROCESSING_COMPLETED + }; assertThatThrownBy(call) .isInstanceOf(InvalidStateException.class) - .hasMessageEndingWith(endOfMessage); + .hasMessage( + "Expected callback state of Task with id '%s' to be: '%s', but found '%s'", + createdTask.getId(), + Arrays.toString(expectedCallbackStates), + createdTask.getCallbackState()); final TaskImpl createdTask2 = (TaskImpl) taskService.claim(createdTask.getId()); assertThat(createdTask2.getState()).isEqualTo(TaskState.CLAIMED); - call = - () -> { - taskService.forceDeleteTask(createdTask2.getId()); - }; + call = () -> taskService.forceDeleteTask(createdTask2.getId()); assertThatThrownBy(call) .isInstanceOf(InvalidStateException.class) - .hasMessageEndingWith(endOfMessage); + .hasMessage( + "Expected callback state of Task with id '%s' to be: '%s', but found '%s'", + createdTask2.getId(), + Arrays.toString(expectedCallbackStates), + createdTask2.getCallbackState()); final TaskImpl createdTask3 = (TaskImpl) taskService.completeTask(createdTask.getId()); - call = - () -> { - taskService.forceDeleteTask(createdTask3.getId()); - }; + call = () -> taskService.forceDeleteTask(createdTask3.getId()); assertThatThrownBy(call) .isInstanceOf(InvalidStateException.class) - .hasMessageEndingWith(endOfMessage); + .hasMessage( + "Expected callback state of Task with id '%s' to be: '%s', but found '%s'", + createdTask3.getId(), + Arrays.toString(expectedCallbackStates), + createdTask3.getCallbackState()); } @WithAccessId(user = "admin") @@ -143,13 +150,9 @@ class CallbackStateAccTest extends AbstractAccTest { BulkOperationResults bulkResult1 = taskService.deleteTasks(taskIds); assertThat(bulkResult1.containsErrors()).isTrue(); - List failedTaskIds = bulkResult1.getFailedIds(); - assertThat(failedTaskIds).hasSize(3); - for (String taskId : failedTaskIds) { - TaskanaException excpt = bulkResult1.getErrorForId(taskId); - assertThat(excpt.getClass().getName()).isEqualTo(InvalidStateException.class.getName()); - } + assertThat(bulkResult1.getErrorMap().values()) + .hasOnlyElementsOfType(InvalidCallbackStateException.class); List externalIds = List.of( createdTask1.getExternalId(), diff --git a/lib/taskana-core/src/test/java/acceptance/task/CompleteTaskAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/CompleteTaskAccTest.java index 0862e171b..37fd3e839 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/CompleteTaskAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/CompleteTaskAccTest.java @@ -17,6 +17,7 @@ import pro.taskana.common.api.BulkOperationResults; import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.internal.util.EnumUtil; import pro.taskana.common.test.security.JaasExtension; import pro.taskana.common.test.security.WithAccessId; import pro.taskana.task.api.TaskService; @@ -362,7 +363,9 @@ class CompleteTaskAccTest extends AbstractAccTest { assertThat(results.getFailedIds()).containsExactlyInAnyOrder(id); assertThat(results.getErrorMap().values()).hasOnlyElementsOfType(InvalidStateException.class); assertThat(results.getErrorForId(id)) - .hasMessage("Task with Id %s has to be claimed before.", id); + .hasMessage( + "Task with id '%s' is in state: '%s', but must be in one of these states: '[%s]'", + id, TaskState.READY, TaskState.CLAIMED); } @WithAccessId(user = "user-1-2") @@ -371,6 +374,8 @@ class CompleteTaskAccTest extends AbstractAccTest { String id1 = "TKI:300000000000000000000000000000000000"; // task is canceled String id2 = "TKI:300000000000000000000000000000000010"; // task is terminated List taskIdList = List.of(id1, id2); + TaskState[] requiredStates = + EnumUtil.allValuesExceptFor(TaskState.TERMINATED, TaskState.CANCELLED); BulkOperationResults results = TASK_SERVICE.completeTasks(taskIdList); @@ -378,9 +383,13 @@ class CompleteTaskAccTest extends AbstractAccTest { assertThat(results.getFailedIds()).containsExactlyInAnyOrder(id1, id2); assertThat(results.getErrorMap().values()).hasOnlyElementsOfType(InvalidStateException.class); assertThat(results.getErrorForId(id1)) - .hasMessage("Cannot complete task %s because it is in state CANCELLED.", id1); + .hasMessage( + "Task with id '%s' is in state: '%s', but must be in one of these states: '%s'", + id1, TaskState.CANCELLED, Arrays.toString(requiredStates)); assertThat(results.getErrorForId(id2)) - .hasMessage("Cannot complete task %s because it is in state TERMINATED.", id2); + .hasMessage( + "Task with id '%s' is in state: '%s', but must be in one of these states: '%s'", + id2, TaskState.TERMINATED, Arrays.toString(requiredStates)); } @WithAccessId(user = "user-1-2") @@ -483,6 +492,8 @@ class CompleteTaskAccTest extends AbstractAccTest { String id1 = "TKI:300000000000000000000000000000000000"; // task is canceled String id2 = "TKI:300000000000000000000000000000000010"; // task is terminated List taskIdList = List.of(id1, id2); + TaskState[] requiredStates = + EnumUtil.allValuesExceptFor(TaskState.TERMINATED, TaskState.CANCELLED); BulkOperationResults results = TASK_SERVICE.forceCompleteTasks(taskIdList); @@ -491,9 +502,13 @@ class CompleteTaskAccTest extends AbstractAccTest { assertThat(results.getFailedIds()).containsExactlyInAnyOrder(id1, id2); assertThat(results.getErrorMap().values()).hasOnlyElementsOfType(InvalidStateException.class); assertThat(results.getErrorForId(id1)) - .hasMessage("Cannot complete task %s because it is in state CANCELLED.", id1); + .hasMessage( + "Task with id '%s' is in state: '%s', but must be in one of these states: '%s'", + id1, TaskState.CANCELLED, Arrays.toString(requiredStates)); assertThat(results.getErrorForId(id2)) - .hasMessage("Cannot complete task %s because it is in state TERMINATED.", id2); + .hasMessage( + "Task with id '%s' is in state: '%s', but must be in one of these states: '%s'", + id2, TaskState.TERMINATED, Arrays.toString(requiredStates)); } @WithAccessId(user = "user-1-2") diff --git a/lib/taskana-core/src/test/java/acceptance/task/CreateTaskAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/CreateTaskAccTest.java index 815bf3778..c47a156b7 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/CreateTaskAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/CreateTaskAccTest.java @@ -778,8 +778,7 @@ class CreateTaskAccTest extends AbstractAccTest { @WithAccessId(user = "user-1-1") @Test - void should_FetchAttachmentClassification_When_CreatingTaskWithAttachments() - throws Exception { + void should_FetchAttachmentClassification_When_CreatingTaskWithAttachments() throws Exception { Attachment attachment = taskService.newAttachment(); attachment.setObjectReference( createObjectReference("COMPANY_A", "SYSTEM_A", "INSTANCE_A", "VNR", "1234567")); diff --git a/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskAccTest.java index 44add0150..a10cfc343 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskAccTest.java @@ -5,7 +5,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import acceptance.AbstractAccTest; import acceptance.TaskanaEngineProxy; -import java.util.ArrayList; import java.util.List; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.Test; @@ -19,6 +18,7 @@ import pro.taskana.common.test.security.JaasExtension; import pro.taskana.common.test.security.WithAccessId; import pro.taskana.task.api.TaskService; import pro.taskana.task.api.exceptions.InvalidStateException; +import pro.taskana.task.api.exceptions.InvalidTaskStateException; import pro.taskana.task.api.exceptions.TaskNotFoundException; import pro.taskana.task.api.models.Task; import pro.taskana.task.internal.AttachmentMapper; @@ -27,36 +27,25 @@ import pro.taskana.task.internal.AttachmentMapper; @ExtendWith(JaasExtension.class) class DeleteTaskAccTest extends AbstractAccTest { - DeleteTaskAccTest() { - super(); - } + private final TaskService taskService = taskanaEngine.getTaskService(); @WithAccessId(user = "user-1-2") @Test void testDeleteSingleTaskNotAuthorized() { - - TaskService taskService = taskanaEngine.getTaskService(); ThrowingCallable call = - () -> { - taskService.deleteTask("TKI:000000000000000000000000000000000037"); - }; + () -> taskService.deleteTask("TKI:000000000000000000000000000000000037"); assertThatThrownBy(call).isInstanceOf(NotAuthorizedException.class); } @WithAccessId(user = "admin") @Test void should_DeleteAttachments_When_MultipleTasksAreDeleted() throws Exception { - - TaskService taskService = taskanaEngine.getTaskService(); - TaskanaEngineProxy engineProxy = new TaskanaEngineProxy(taskanaEngine); AttachmentMapper attachmentMapper = engineProxy.getEngine().getSqlSession().getMapper(AttachmentMapper.class); try { - engineProxy.openConnection(); - assertThat( attachmentMapper.findAttachmentSummariesByTaskIds( List.of( @@ -72,14 +61,13 @@ class DeleteTaskAccTest extends AbstractAccTest { "TKI:000000000000000000000000000000000067", "TKI:000000000000000000000000000000000068")); try { - + engineProxy.openConnection(); assertThat( attachmentMapper.findAttachmentSummariesByTaskIds( List.of( "TKI:000000000000000000000000000000000067", "TKI:000000000000000000000000000000000068"))) .isEmpty(); - } finally { engineProxy.returnConnection(); } @@ -88,17 +76,12 @@ class DeleteTaskAccTest extends AbstractAccTest { @WithAccessId(user = "admin") @Test void should_DeleteAttachments_When_SingleTaskIsDeleted() throws Exception { - - TaskService taskService = taskanaEngine.getTaskService(); - TaskanaEngineProxy engineProxy = new TaskanaEngineProxy(taskanaEngine); AttachmentMapper attachmentMapper = engineProxy.getSqlSession().getMapper(AttachmentMapper.class); try { - engineProxy.openConnection(); - assertThat( attachmentMapper.findAttachmentsByTaskId("TKI:000000000000000000000000000000000069")) .hasSize(1); @@ -110,7 +93,7 @@ class DeleteTaskAccTest extends AbstractAccTest { taskService.deleteTask("TKI:000000000000000000000000000000000069"); try { - + engineProxy.openConnection(); assertThat( attachmentMapper.findAttachmentsByTaskId("TKI:000000000000000000000000000000000069")) .isEmpty(); @@ -124,9 +107,6 @@ class DeleteTaskAccTest extends AbstractAccTest { @WithAccessId(user = "user-1-1") @TestTemplate void should_ThrowException_When_UserIsNotInAdminRoleButTriesToBulkDeleteTasks() { - - TaskService taskService = taskanaEngine.getTaskService(); - List taskIds = List.of( "TKI:000000000000000000000000000000000008", @@ -134,26 +114,18 @@ class DeleteTaskAccTest extends AbstractAccTest { "TKI:000000000000000000000000000000000008", "TKI:000000000000000000000000000000000010"); - ThrowingCallable call = - () -> { - taskService.deleteTasks(taskIds); - }; + ThrowingCallable call = () -> taskService.deleteTasks(taskIds); assertThatThrownBy(call).isInstanceOf(NotAuthorizedException.class); } @WithAccessId(user = "admin") @Test void testDeleteSingleTask() throws Exception { - - TaskService taskService = taskanaEngine.getTaskService(); Task task = taskService.getTask("TKI:000000000000000000000000000000000036"); taskService.deleteTask(task.getId()); - ThrowingCallable call = - () -> { - taskService.getTask("TKI:000000000000000000000000000000000036"); - }; + ThrowingCallable call = () -> taskService.getTask("TKI:000000000000000000000000000000000036"); assertThatThrownBy(call).isInstanceOf(TaskNotFoundException.class); } @@ -162,97 +134,71 @@ class DeleteTaskAccTest extends AbstractAccTest { @WithAccessId(user = "user-1-1") @TestTemplate void should_ThrowException_When_UserIsNotInAdminRole() { - - TaskService taskService = taskanaEngine.getTaskService(); - ThrowingCallable deleteTaskCall = - () -> { - taskService.deleteTask("TKI:000000000000000000000000000000000041"); - }; + () -> taskService.deleteTask("TKI:000000000000000000000000000000000041"); assertThatThrownBy(deleteTaskCall).isInstanceOf(NotAuthorizedException.class); } @WithAccessId(user = "admin") @Test void testThrowsExceptionIfTaskIsNotCompleted() throws Exception { - TaskService taskService = taskanaEngine.getTaskService(); Task task = taskService.getTask("TKI:000000000000000000000000000000000029"); - ThrowingCallable call = - () -> { - taskService.deleteTask(task.getId()); - }; + ThrowingCallable call = () -> taskService.deleteTask(task.getId()); assertThatThrownBy(call).isInstanceOf(InvalidStateException.class); } @WithAccessId(user = "admin") @Test void testForceDeleteTaskIfNotCompleted() throws Exception { - TaskService taskService = taskanaEngine.getTaskService(); Task task = taskService.getTask("TKI:000000000000000000000000000000000027"); - ThrowingCallable call = - () -> { - taskService.deleteTask(task.getId()); - }; + ThrowingCallable call = () -> taskService.deleteTask(task.getId()); assertThatThrownBy(call) .describedAs("Should not be possible to delete claimed task without force flag") .isInstanceOf(InvalidStateException.class); taskService.forceDeleteTask(task.getId()); - call = - () -> { - taskService.getTask("TKI:000000000000000000000000000000000027"); - }; + call = () -> taskService.getTask("TKI:000000000000000000000000000000000027"); assertThatThrownBy(call).isInstanceOf(TaskNotFoundException.class); } @WithAccessId(user = "admin") @Test void testBulkDeleteTask() throws Exception { - - TaskService taskService = taskanaEngine.getTaskService(); - ArrayList taskIdList = new ArrayList<>(); - taskIdList.add("TKI:000000000000000000000000000000000037"); - taskIdList.add("TKI:000000000000000000000000000000000038"); + List taskIdList = + List.of( + "TKI:000000000000000000000000000000000037", "TKI:000000000000000000000000000000000038"); BulkOperationResults results = taskService.deleteTasks(taskIdList); assertThat(results.containsErrors()).isFalse(); - ThrowingCallable call = - () -> { - taskService.getTask("TKI:000000000000000000000000000000000038"); - }; + ThrowingCallable call = () -> taskService.getTask("TKI:000000000000000000000000000000000038"); assertThatThrownBy(call).isInstanceOf(TaskNotFoundException.class); } @WithAccessId(user = "admin") @Test void testBulkDeleteTasksWithException() throws Exception { - - TaskService taskService = taskanaEngine.getTaskService(); - ArrayList taskIdList = new ArrayList<>(); - taskIdList.add("TKI:000000000000000000000000000000000039"); - taskIdList.add("TKI:000000000000000000000000000000000040"); - taskIdList.add("TKI:000000000000000000000000000000000028"); + List taskIdList = + List.of( + "TKI:000000000000000000000000000000000039", + "TKI:000000000000000000000000000000000040", + "TKI:000000000000000000000000000000000028"); BulkOperationResults results = taskService.deleteTasks(taskIdList); - String expectedFailedId = "TKI:000000000000000000000000000000000028"; assertThat(results.containsErrors()).isTrue(); - List failedTaskIds = results.getFailedIds(); - assertThat(failedTaskIds).hasSize(1); - assertThat(failedTaskIds.get(0)).isEqualTo(expectedFailedId); - assertThat(InvalidStateException.class) - .isSameAs(results.getErrorMap().get(expectedFailedId).getClass()); + + assertThat(results.getErrorMap().keySet()) + .containsExactly("TKI:000000000000000000000000000000000028"); + assertThat(results.getErrorMap().values()) + .hasOnlyElementsOfType(InvalidTaskStateException.class); Task notDeletedTask = taskService.getTask("TKI:000000000000000000000000000000000028"); assertThat(notDeletedTask).isNotNull(); - ThrowingCallable call = - () -> { - taskService.getTask("TKI:000000000000000000000000000000000040"); - }; + ThrowingCallable call = () -> taskService.getTask("TKI:000000000000000000000000000000000040"); assertThatThrownBy(call).isInstanceOf(TaskNotFoundException.class); } } diff --git a/lib/taskana-core/src/test/java/acceptance/task/QueryTasksByObjectReferenceAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/QueryTasksByObjectReferenceAccTest.java index 5efd77b74..9be92692e 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/QueryTasksByObjectReferenceAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/QueryTasksByObjectReferenceAccTest.java @@ -115,8 +115,7 @@ class QueryTasksByObjectReferenceAccTest extends AbstractAccTest { objectReference.setSystemInstance("00"); objectReference.setType("VNR"); objectReference.setValue("67890123"); - long count = - TASK_SERVICE.createTaskQuery().primaryObjectReferenceIn(objectReference).count(); + long count = TASK_SERVICE.createTaskQuery().primaryObjectReferenceIn(objectReference).count(); assertThat(count).isEqualTo(1); } diff --git a/lib/taskana-core/src/test/java/acceptance/task/SetOwnerAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/SetOwnerAccTest.java index de50a7d08..4654a2515 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/SetOwnerAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/SetOwnerAccTest.java @@ -21,9 +21,11 @@ import pro.taskana.common.test.security.WithAccessId; import pro.taskana.task.api.TaskService; import pro.taskana.task.api.TaskState; import pro.taskana.task.api.exceptions.InvalidStateException; +import pro.taskana.task.api.exceptions.InvalidTaskStateException; import pro.taskana.task.api.exceptions.TaskNotFoundException; import pro.taskana.task.api.models.Task; import pro.taskana.task.api.models.TaskSummary; +import pro.taskana.workbasket.api.exceptions.MismatchedWorkbasketPermissionException; /** Acceptance test for all "set owner" scenarios. */ @ExtendWith(JaasExtension.class) @@ -85,11 +87,12 @@ class SetOwnerAccTest extends AbstractAccTest { String anyUserName = "TestUser3"; assertThatThrownBy(() -> taskService.getTask(taskReadyId)) - .isInstanceOf(NotAuthorizedException.class); + .isInstanceOf(MismatchedWorkbasketPermissionException.class); BulkOperationResults results = taskService.setOwnerOfTasks(anyUserName, List.of(taskReadyId)); assertThat(results.containsErrors()).isTrue(); - assertThat(results.getErrorForId(taskReadyId)).isInstanceOf(NotAuthorizedException.class); + assertThat(results.getErrorForId(taskReadyId)) + .isInstanceOf(MismatchedWorkbasketPermissionException.class); } @WithAccessId(user = "user-b-2") @@ -156,7 +159,8 @@ class SetOwnerAccTest extends AbstractAccTest { resetDb(false); List allTaskSummaries = new TaskanaEngineProxy(taskanaEngine) - .getEngine().getEngine() + .getEngine() + .getEngine() .runAsAdmin(() -> taskanaEngine.getTaskService().createTaskQuery().list()); List allTaskIds = allTaskSummaries.stream().map(TaskSummary::getId).collect(Collectors.toList()); @@ -165,17 +169,19 @@ class SetOwnerAccTest extends AbstractAccTest { assertThat(allTaskSummaries).hasSize(88); assertThat(results.containsErrors()).isTrue(); - Condition invalidStateException = - new Condition<>(c -> c.getClass() == InvalidStateException.class, "InvalidStateException"); - Condition notAuthorizedException = + Condition invalidTaskStateException = new Condition<>( - c -> c.getClass() == NotAuthorizedException.class, "NotAuthorizedException"); + c -> c.getClass() == InvalidTaskStateException.class, "InvalidStateException"); + Condition mismatchedWorkbasketPermissionException = + new Condition<>( + c -> c.getClass() == MismatchedWorkbasketPermissionException.class, + "MismatchedWorkbasketPermissionException"); assertThat(results.getErrorMap()) .hasSize(86) .extractingFromEntries(Entry::getValue) - .hasOnlyElementsOfTypes(InvalidStateException.class, NotAuthorizedException.class) - .areExactly(28, invalidStateException) - .areExactly(58, notAuthorizedException); + .hasOnlyElementsOfTypes(InvalidTaskStateException.class, NotAuthorizedException.class) + .areExactly(28, invalidTaskStateException) + .areExactly(58, mismatchedWorkbasketPermissionException); } @WithAccessId(user = "admin") diff --git a/lib/taskana-core/src/test/java/acceptance/task/TaskEngineAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/TaskEngineAccTest.java index 6e3506e5f..876603239 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/TaskEngineAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/TaskEngineAccTest.java @@ -37,7 +37,8 @@ class TaskEngineAccTest extends AbstractAccTest { assertThat(taskanaEngine.isUserInRole(TaskanaRole.ADMIN)).isFalse(); new TaskanaEngineProxy(taskanaEngine) - .getEngine().getEngine() + .getEngine() + .getEngine() .runAsAdmin(() -> assertThat(taskanaEngine.isUserInRole(TaskanaRole.ADMIN)).isTrue()); assertThat(taskanaEngine.isUserInRole(TaskanaRole.ADMIN)).isFalse(); diff --git a/lib/taskana-core/src/test/java/acceptance/task/TransferTaskAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/TransferTaskAccTest.java index f7c850350..4224e5686 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/TransferTaskAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/TransferTaskAccTest.java @@ -24,9 +24,11 @@ import pro.taskana.common.test.security.WithAccessId; import pro.taskana.task.api.TaskService; import pro.taskana.task.api.TaskState; import pro.taskana.task.api.exceptions.InvalidStateException; +import pro.taskana.task.api.exceptions.InvalidTaskStateException; import pro.taskana.task.api.exceptions.TaskNotFoundException; import pro.taskana.task.api.models.Task; import pro.taskana.task.api.models.TaskSummary; +import pro.taskana.workbasket.api.exceptions.MismatchedWorkbasketPermissionException; import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; import pro.taskana.workbasket.api.models.Workbasket; @@ -140,8 +142,8 @@ class TransferTaskAccTest extends AbstractAccTest { .extracting(Throwable::getMessage) .asString() .startsWith( - "Not authorized. Permission 'TRANSFER' on workbasket " - + "'WBI:100000000000000000000000000000000005' is needed."); + "Not authorized. The current user 'teamlead-1' has no '[TRANSFER]' permission(s) " + + "for Workbasket 'WBI:100000000000000000000000000000000005'."); } @WithAccessId(user = "user-1-1", groups = GROUP_1_DN) @@ -156,8 +158,8 @@ class TransferTaskAccTest extends AbstractAccTest { .extracting(Throwable::getMessage) .asString() .startsWith( - "Not authorized. Permission 'APPEND' on workbasket " - + "'WBI:100000000000000000000000000000000008' is needed."); + "Not authorized. The current user 'user-1-1' has no '[APPEND]' permission(s) " + + "for Workbasket 'WBI:100000000000000000000000000000000008'."); } @WithAccessId(user = "teamlead-1") @@ -229,13 +231,13 @@ class TransferTaskAccTest extends AbstractAccTest { assertThat(results.containsErrors()).isTrue(); assertThat(results.getErrorMap().values()).hasSize(6); assertThat(results.getErrorForId("TKI:000000000000000000000000000000000041").getClass()) - .isEqualTo(NotAuthorizedException.class); + .isEqualTo(MismatchedWorkbasketPermissionException.class); assertThat(results.getErrorForId("TKI:200000000000000000000000000000000008").getClass()) - .isEqualTo(NotAuthorizedException.class); + .isEqualTo(MismatchedWorkbasketPermissionException.class); assertThat(results.getErrorForId("TKI:000000000000000000000000000000000099").getClass()) .isEqualTo(TaskNotFoundException.class); assertThat(results.getErrorForId("TKI:100000000000000000000000000000000006").getClass()) - .isEqualTo(InvalidStateException.class); + .isEqualTo(InvalidTaskStateException.class); assertThat(results.getErrorForId("").getClass()).isEqualTo(InvalidArgumentException.class); assertThat(results.getErrorForId(null).getClass()).isEqualTo(InvalidArgumentException.class); 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 e1394f766..e98973725 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAccTest.java @@ -136,7 +136,8 @@ class UpdateTaskAccTest extends AbstractAccTest { // TODO flaky test ... if speed is too high, assertThatThrownBy(() -> taskService.updateTask(task2)) .isInstanceOf(ConcurrencyException.class) - .hasMessage("The task has already been updated by another user"); + .hasMessage( + "The current entity cannot be updated since it has been modified while editing."); } @WithAccessId(user = "admin") diff --git a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAttachmentsAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAttachmentsAccTest.java index 07559cef7..9030ca68a 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAttachmentsAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAttachmentsAccTest.java @@ -588,8 +588,7 @@ class UpdateTaskAttachmentsAccTest extends AbstractAccTest { @WithAccessId(user = "user-1-1") @Test - void should_FetchAttachmentClassification_When_UpdatingTaskWithAttachments() - throws Exception { + void should_FetchAttachmentClassification_When_UpdatingTaskWithAttachments() throws Exception { ClassificationSummary classification = classificationService.newClassification("T2000", "DOMAIN_A", "").asSummary(); attachment.setClassificationSummary(classification); diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/CreateWorkbasketAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/CreateWorkbasketAccTest.java index e1740bd00..cb6530edc 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/CreateWorkbasketAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/CreateWorkbasketAccTest.java @@ -11,13 +11,13 @@ import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; import pro.taskana.common.api.exceptions.DomainNotFoundException; +import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.test.security.JaasExtension; import pro.taskana.common.test.security.WithAccessId; import pro.taskana.workbasket.api.WorkbasketPermission; import pro.taskana.workbasket.api.WorkbasketService; import pro.taskana.workbasket.api.WorkbasketType; -import pro.taskana.workbasket.api.exceptions.InvalidWorkbasketException; import pro.taskana.workbasket.api.exceptions.WorkbasketAccessItemAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketAlreadyExistException; import pro.taskana.workbasket.api.models.Workbasket; @@ -111,21 +111,21 @@ class CreateWorkbasketAccTest extends AbstractAccTest { workbasket.setOrgLevel1("company"); // missing key assertThatThrownBy(() -> workbasketService.createWorkbasket(workbasket)) - .isInstanceOf(InvalidWorkbasketException.class); + .isInstanceOf(InvalidArgumentException.class); Workbasket workbasket2 = workbasketService.newWorkbasket("key", "novatec"); workbasket2.setType(WorkbasketType.GROUP); workbasket2.setOrgLevel1("company"); // missing name assertThatThrownBy(() -> workbasketService.createWorkbasket(workbasket2)) - .isInstanceOf(InvalidWorkbasketException.class); + .isInstanceOf(InvalidArgumentException.class); Workbasket workbasket3 = workbasketService.newWorkbasket("key", "novatec"); workbasket3.setName("Megabasket"); workbasket3.setOrgLevel1("company"); // missing type assertThatThrownBy(() -> workbasketService.createWorkbasket(workbasket3)) - .isInstanceOf(InvalidWorkbasketException.class); + .isInstanceOf(InvalidArgumentException.class); Workbasket workbasket4 = workbasketService.newWorkbasket("key", null); workbasket4.setName("Megabasket"); @@ -133,7 +133,7 @@ class CreateWorkbasketAccTest extends AbstractAccTest { workbasket4.setOrgLevel1("company"); // missing domain assertThatThrownBy(() -> workbasketService.createWorkbasket(workbasket4)) - .isInstanceOf(InvalidWorkbasketException.class); + .isInstanceOf(InvalidArgumentException.class); Workbasket workbasket5 = workbasketService.newWorkbasket("", "novatec"); workbasket5.setName("Megabasket"); @@ -141,7 +141,7 @@ class CreateWorkbasketAccTest extends AbstractAccTest { workbasket5.setOrgLevel1("company"); // empty key assertThatThrownBy(() -> workbasketService.createWorkbasket(workbasket5)) - .isInstanceOf(InvalidWorkbasketException.class); + .isInstanceOf(InvalidArgumentException.class); Workbasket workbasket6 = workbasketService.newWorkbasket("key", "novatec"); workbasket6.setName(""); @@ -149,7 +149,7 @@ class CreateWorkbasketAccTest extends AbstractAccTest { workbasket6.setOrgLevel1("company"); // empty name assertThatThrownBy(() -> workbasketService.createWorkbasket(workbasket)) - .isInstanceOf(InvalidWorkbasketException.class); + .isInstanceOf(InvalidArgumentException.class); } @WithAccessId(user = "businessadmin") diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/DeleteWorkbasketAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/DeleteWorkbasketAccTest.java index f5a8fb7cf..66aada683 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/DeleteWorkbasketAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/DeleteWorkbasketAccTest.java @@ -40,7 +40,8 @@ class DeleteWorkbasketAccTest extends AbstractAccTest { @WithAccessId(user = "businessadmin") @Test - void testDeleteWorkbasket() throws Exception { + void should_ThrowWorkbasketNotFoundException_When_TheWorkbasketHasAlreadyBeenDeleted() + throws Exception { Workbasket wb = workbasketService.getWorkbasket("USER-2-2", "DOMAIN_A"); ThrowingCallable call = @@ -75,17 +76,19 @@ class DeleteWorkbasketAccTest extends AbstractAccTest { } @Test - void testGetWorkbasketNotAuthorized() { + void should_ThrowNotAuthorizedException_When_UnauthorizedTryingToGetWorkbaskets() { assertThatThrownBy(() -> workbasketService.getWorkbasket("TEAMLEAD-2", "DOMAIN_A")) .isInstanceOf(NotAuthorizedException.class); } @WithAccessId(user = "businessadmin") @Test - void testDeleteWorkbasketAlsoAsDistributionTarget() throws Exception { + void should_RemoveWorkbasketsFromDistributionTargets_WhenWorkbasketIsDeleted() throws Exception { Workbasket wb = workbasketService.getWorkbasket("GPK_KSC_1", "DOMAIN_A"); int distTargets = - workbasketService.getDistributionTargets("WBI:100000000000000000000000000000000001").size(); + workbasketService + .getDistributionTargets("GPK_KSC", "DOMAIN_A") + .size(); // has GPK_KSC_1 as a distribution target (+ 3 other Workbaskets) ThrowingCallable call = () -> { @@ -97,14 +100,13 @@ class DeleteWorkbasketAccTest extends AbstractAccTest { .describedAs("There should be no result for a deleted Workbasket.") .isInstanceOf(WorkbasketNotFoundException.class); - int newDistTargets = - workbasketService.getDistributionTargets("WBI:100000000000000000000000000000000001").size(); + int newDistTargets = workbasketService.getDistributionTargets("GPK_KSC", "DOMAIN_A").size(); assertThat(newDistTargets).isEqualTo(3).isLessThan(distTargets); } @WithAccessId(user = "businessadmin") @Test - void testDeleteWorkbasketWithNullOrEmptyParam() { + void should_ThrowInvalidArgumentException_When_TryingToDeleteNullOrEmptyWorkbasket() { // Test Null-Value assertThatThrownBy(() -> workbasketService.deleteWorkbasket(null)) .describedAs( @@ -122,14 +124,15 @@ class DeleteWorkbasketAccTest extends AbstractAccTest { @WithAccessId(user = "businessadmin") @Test - void testDeleteWorkbasketButNotExisting() { + void should_ThrowWorkbasketNotFoundException_When_TryingToDeleteNonExistingWorkbasket() { assertThatThrownBy(() -> workbasketService.deleteWorkbasket("SOME NOT EXISTING ID")) .isInstanceOf(WorkbasketNotFoundException.class); } @WithAccessId(user = "user-1-2", groups = "businessadmin") @Test - void testDeleteWorkbasketWhichIsUsed() throws Exception { + void should_ThrowWorkbasketInUseException_When_TryingToDeleteWorkbasketWhichIsInUse() + throws Exception { Workbasket wb = workbasketService.getWorkbasket("user-1-2", "DOMAIN_A"); // all rights, DOMAIN_A with Tasks assertThatThrownBy(() -> workbasketService.deleteWorkbasket(wb.getId())) @@ -138,7 +141,7 @@ class DeleteWorkbasketAccTest extends AbstractAccTest { @WithAccessId(user = "businessadmin") @Test - void testCascadingDeleteOfAccessItems() throws Exception { + void should_DeleteWorkbasketAccessItems_When_WorkbasketIsDeleted() throws Exception { Workbasket wb = workbasketService.getWorkbasket("WBI:100000000000000000000000000000000008"); String wbId = wb.getId(); // create 2 access Items @@ -170,7 +173,7 @@ class DeleteWorkbasketAccTest extends AbstractAccTest { @WithAccessId(user = "admin") @Test - void testMarkWorkbasketForDeletion() throws Exception { + void should_MarkWorkbasketForDeletion_When_TryingToDeleteWorkbasketWithTasks() throws Exception { final Workbasket wb = workbasketService.getWorkbasket("WBI:100000000000000000000000000000000006"); @@ -183,8 +186,8 @@ class DeleteWorkbasketAccTest extends AbstractAccTest { task = (TaskImpl) taskService.getTask("TKI:000000000000000000000000000000000066"); taskService.forceCompleteTask(task.getId()); - boolean markedForDeletion = workbasketService.deleteWorkbasket(wb.getId()); - assertThat(markedForDeletion).isFalse(); + boolean canBeDeletedNow = workbasketService.deleteWorkbasket(wb.getId()); + assertThat(canBeDeletedNow).isFalse(); Workbasket wb2 = workbasketService.getWorkbasket(wb.getId()); assertThat(wb2.isMarkedForDeletion()).isTrue(); diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/GetWorkbasketAuthorizationsAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/GetWorkbasketAuthorizationsAccTest.java index 6c7b48e9f..bcb98fddc 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/GetWorkbasketAuthorizationsAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/GetWorkbasketAuthorizationsAccTest.java @@ -19,7 +19,7 @@ class GetWorkbasketAuthorizationsAccTest extends AbstractAccTest { @WithAccessId(user = "user-1-1") @WithAccessId(user = "taskadmin") @TestTemplate - void should_ThrowException_When_UserRoleIsNotAdminOrBusinessAdmin() { + void should_ThrowNotAuthorizedException_When_UserRoleIsNotAdminOrBusinessAdmin() { final WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/UpdateWorkbasketAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/UpdateWorkbasketAccTest.java index 3ccc6bae2..041e4d291 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/UpdateWorkbasketAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/UpdateWorkbasketAccTest.java @@ -12,13 +12,13 @@ import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; import pro.taskana.common.api.exceptions.ConcurrencyException; +import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.test.security.JaasExtension; import pro.taskana.common.test.security.WithAccessId; import pro.taskana.workbasket.api.WorkbasketCustomField; import pro.taskana.workbasket.api.WorkbasketService; import pro.taskana.workbasket.api.WorkbasketType; -import pro.taskana.workbasket.api.exceptions.InvalidWorkbasketException; import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; import pro.taskana.workbasket.api.models.Workbasket; import pro.taskana.workbasket.internal.models.WorkbasketImpl; @@ -67,11 +67,11 @@ class UpdateWorkbasketAccTest extends AbstractAccTest { (WorkbasketImpl) workbasketService.getWorkbasket("GPK_KSC", "DOMAIN_A"); workbasket.setName(null); assertThatThrownBy(() -> workbasketService.updateWorkbasket(workbasket)) - .isInstanceOf(InvalidWorkbasketException.class); + .isInstanceOf(InvalidArgumentException.class); workbasket.setName(""); assertThatThrownBy(() -> workbasketService.updateWorkbasket(workbasket)) - .isInstanceOf(InvalidWorkbasketException.class); + .isInstanceOf(InvalidArgumentException.class); } @WithAccessId(user = "businessadmin") @@ -84,7 +84,7 @@ class UpdateWorkbasketAccTest extends AbstractAccTest { workbasket.setType(null); assertThatThrownBy(() -> workbasketService.updateWorkbasket(workbasket)) - .isInstanceOf(InvalidWorkbasketException.class); + .isInstanceOf(InvalidArgumentException.class); } @WithAccessId(user = "businessadmin") @@ -118,6 +118,20 @@ class UpdateWorkbasketAccTest extends AbstractAccTest { .isThrownBy(() -> workbasketService.updateWorkbasket(workbasket)); } + @WithAccessId(user = "businessadmin") + @Test + void should_ThrowException_When_TryingToUpdateUnknownWorkbasket() { + + WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + + Workbasket workbasket = workbasketService.newWorkbasket("InvalidKey", "InvalidDomain"); + workbasket.setName("bla bla"); + workbasket.setType(WorkbasketType.PERSONAL); + + assertThatThrownBy(() -> workbasketService.updateWorkbasket(workbasket)) + .isInstanceOf(WorkbasketNotFoundException.class); + } + @WithAccessId(user = "user-1-1") @WithAccessId(user = "taskadmin") @TestTemplate diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/UpdateWorkbasketAuthorizationsAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/UpdateWorkbasketAuthorizationsAccTest.java index 368d08d78..d112c670a 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/UpdateWorkbasketAuthorizationsAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/UpdateWorkbasketAuthorizationsAccTest.java @@ -25,6 +25,7 @@ import pro.taskana.task.api.models.TaskSummary; import pro.taskana.workbasket.api.WorkbasketPermission; import pro.taskana.workbasket.api.WorkbasketService; import pro.taskana.workbasket.api.exceptions.NotAuthorizedToQueryWorkbasketException; +import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; import pro.taskana.workbasket.api.models.WorkbasketAccessItem; import pro.taskana.workbasket.internal.models.WorkbasketAccessItemImpl; @@ -43,12 +44,27 @@ class UpdateWorkbasketAuthorizationsAccTest extends AbstractAccTest { workbasketService.newWorkbasketAccessItem( "WBI:100000000000000000000000000000000008", "newAccessIdForUpdate"); - workbasketAccessItem.setPermission(WorkbasketPermission.CUSTOM_1, true); - assertThatThrownBy(() -> workbasketService.updateWorkbasketAccessItem(workbasketAccessItem)) .isInstanceOf(NotAuthorizedException.class); } + @Test + @WithAccessId(user = "admin") + void + should_ThrowWorkbasketNotFoundException_When_TryingToSetAccessItemsOfNonExistingWorkbasket() { + + final WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + + WorkbasketAccessItem workbasketAccessItem = + workbasketService.newWorkbasketAccessItem("WBI:1337gibtEsNicht007", "newAccessIdForUpdate"); + + assertThatThrownBy( + () -> + workbasketService.setWorkbasketAccessItems( + "WBI:1337gibtEsNicht007", List.of(workbasketAccessItem))) + .isInstanceOf(WorkbasketNotFoundException.class); + } + @WithAccessId(user = "businessadmin") @Test void testUpdateWorkbasketAccessItemSucceeds() throws Exception { diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/WorkbasketServiceImplTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/WorkbasketServiceImplTest.java index 58109f486..508f4a142 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/WorkbasketServiceImplTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/WorkbasketServiceImplTest.java @@ -137,8 +137,7 @@ class WorkbasketServiceImplTest { verify(taskanaEngine, times(2)).checkRoleMembership(any()); verify(internalTaskanaEngineMock, times(2)).getEngine(); verify(internalTaskanaEngineMock, times(1)).domainExists(any()); - verify(distributionTargetMapperMock) - .deleteAllDistributionTargetsBySourceId(expectedWb.getId()); + verify(distributionTargetMapperMock).deleteAllDistributionTargetsBySourceId(expectedWb.getId()); verify(workbasketMapperMock).update(expectedWb); verify(internalTaskanaEngineMock, times(1)).getHistoryEventManager(); diff --git a/lib/taskana-spring-example/src/main/java/pro/taskana/example/ExampleBootstrap.java b/lib/taskana-spring-example/src/main/java/pro/taskana/example/ExampleBootstrap.java index 783be54e0..7716d6d35 100644 --- a/lib/taskana-spring-example/src/main/java/pro/taskana/example/ExampleBootstrap.java +++ b/lib/taskana-spring-example/src/main/java/pro/taskana/example/ExampleBootstrap.java @@ -7,6 +7,7 @@ import org.springframework.transaction.annotation.Transactional; import pro.taskana.classification.api.exceptions.ClassificationAlreadyExistException; import pro.taskana.classification.api.exceptions.ClassificationNotFoundException; +import pro.taskana.classification.api.exceptions.MalformedServiceLevelException; import pro.taskana.classification.api.models.Classification; import pro.taskana.common.api.TaskanaEngine; import pro.taskana.common.api.exceptions.DomainNotFoundException; @@ -21,7 +22,6 @@ import pro.taskana.task.api.exceptions.TaskNotFoundException; import pro.taskana.task.api.models.ObjectReference; import pro.taskana.task.api.models.Task; import pro.taskana.workbasket.api.WorkbasketType; -import pro.taskana.workbasket.api.exceptions.InvalidWorkbasketException; import pro.taskana.workbasket.api.exceptions.WorkbasketAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; import pro.taskana.workbasket.api.models.Workbasket; @@ -37,11 +37,11 @@ public class ExampleBootstrap { @PostConstruct public void test() - throws TaskNotFoundException, NotAuthorizedException, WorkbasketNotFoundException, - ClassificationNotFoundException, InvalidStateException, InvalidOwnerException, - TaskAlreadyExistException, InvalidArgumentException, DomainNotFoundException, - InvalidWorkbasketException, WorkbasketAlreadyExistException, - ClassificationAlreadyExistException, AttachmentPersistenceException { + throws InvalidArgumentException, WorkbasketAlreadyExistException, DomainNotFoundException, + NotAuthorizedException, ClassificationAlreadyExistException, + MalformedServiceLevelException, TaskAlreadyExistException, WorkbasketNotFoundException, + ClassificationNotFoundException, AttachmentPersistenceException, TaskNotFoundException, + InvalidOwnerException, InvalidStateException { System.out.println("---------------------------> Start App"); Workbasket wb = taskanaEngine.getWorkbasketService().newWorkbasket("workbasket", "DOMAIN_A"); diff --git a/lib/taskana-spring-example/src/main/java/pro/taskana/example/TaskanaTestController.java b/lib/taskana-spring-example/src/main/java/pro/taskana/example/TaskanaTestController.java index a11b608bc..03bcf687e 100644 --- a/lib/taskana-spring-example/src/main/java/pro/taskana/example/TaskanaTestController.java +++ b/lib/taskana-spring-example/src/main/java/pro/taskana/example/TaskanaTestController.java @@ -11,10 +11,10 @@ import org.springframework.web.bind.annotation.RestController; import pro.taskana.common.api.TaskanaEngine; import pro.taskana.common.api.exceptions.DomainNotFoundException; +import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.internal.util.IdGenerator; import pro.taskana.workbasket.api.WorkbasketType; -import pro.taskana.workbasket.api.exceptions.InvalidWorkbasketException; import pro.taskana.workbasket.api.exceptions.WorkbasketAlreadyExistException; import pro.taskana.workbasket.api.models.Workbasket; import pro.taskana.workbasket.internal.models.WorkbasketImpl; @@ -51,7 +51,7 @@ public class TaskanaTestController { @GetMapping(path = "/transaction") public @ResponseBody String transaction( @RequestParam(value = "rollback", defaultValue = "false") String rollback) - throws InvalidWorkbasketException, NotAuthorizedException, WorkbasketAlreadyExistException, + throws InvalidArgumentException, NotAuthorizedException, WorkbasketAlreadyExistException, DomainNotFoundException { taskanaEngine.getWorkbasketService().createWorkbasket(createWorkBasket("key", "workbasket")); @@ -67,7 +67,7 @@ public class TaskanaTestController { @GetMapping(path = "/transaction-many") public @ResponseBody String transactionMany( @RequestParam(value = "rollback", defaultValue = "false") String rollback) - throws InvalidWorkbasketException, NotAuthorizedException, WorkbasketAlreadyExistException, + throws InvalidArgumentException, NotAuthorizedException, WorkbasketAlreadyExistException, DomainNotFoundException { taskanaEngine.getWorkbasketService().createWorkbasket(createWorkBasket("key1", "workbasket1")); taskanaEngine.getWorkbasketService().createWorkbasket(createWorkBasket("key2", "workbasket2")); @@ -84,7 +84,7 @@ public class TaskanaTestController { @GetMapping(path = "/customdb") public @ResponseBody String transactionCustomdb( @RequestParam(value = "rollback", defaultValue = "false") String rollback) - throws InvalidWorkbasketException, NotAuthorizedException, WorkbasketAlreadyExistException, + throws InvalidArgumentException, NotAuthorizedException, WorkbasketAlreadyExistException, DomainNotFoundException { taskanaEngine.getWorkbasketService().createWorkbasket(createWorkBasket("key1", "workbasket1")); taskanaEngine.getWorkbasketService().createWorkbasket(createWorkBasket("key2", "workbasket2")); diff --git a/lib/taskana-spring-example/src/test/java/pro/taskana/example/TaskanaTransactionIntTest.java b/lib/taskana-spring-example/src/test/java/pro/taskana/example/TaskanaTransactionIntTest.java index e38fd9495..39488a6ee 100644 --- a/lib/taskana-spring-example/src/test/java/pro/taskana/example/TaskanaTransactionIntTest.java +++ b/lib/taskana-spring-example/src/test/java/pro/taskana/example/TaskanaTransactionIntTest.java @@ -209,13 +209,12 @@ class TaskanaTransactionIntTest { taskCleanupJob.run(); ThrowingCallable httpCall = - () -> { - workbasketService.deleteWorkbasket( - workbasketService.getWorkbasket("key3", "DOMAIN_A").getId()); - }; + () -> + workbasketService.deleteWorkbasket( + workbasketService.getWorkbasket("key3", "DOMAIN_A").getId()); assertThatThrownBy(httpCall) .isInstanceOf(WorkbasketInUseException.class) - .hasMessageContaining("contains 1 non-completed tasks"); + .hasMessageContaining("contains non-completed Tasks"); WorkbasketCleanupJob job = new WorkbasketCleanupJob(taskanaEngine, springTransactionProvider, null); diff --git a/rest/taskana-rest-spring-example-common/src/main/java/pro/taskana/example/jobs/JobScheduler.java b/rest/taskana-rest-spring-example-common/src/main/java/pro/taskana/example/jobs/JobScheduler.java index e2a710309..841eebb96 100644 --- a/rest/taskana-rest-spring-example-common/src/main/java/pro/taskana/example/jobs/JobScheduler.java +++ b/rest/taskana-rest-spring-example-common/src/main/java/pro/taskana/example/jobs/JobScheduler.java @@ -1,13 +1,7 @@ package pro.taskana.example.jobs; import java.lang.reflect.InvocationTargetException; -import java.security.Principal; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; -import java.util.ArrayList; -import java.util.List; import javax.annotation.PostConstruct; -import javax.security.auth.Subject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -16,8 +10,6 @@ import org.springframework.stereotype.Component; import pro.taskana.common.api.ScheduledJob.Type; import pro.taskana.common.api.TaskanaEngine; -import pro.taskana.common.api.TaskanaRole; -import pro.taskana.common.api.security.UserPrincipal; import pro.taskana.common.internal.jobs.JobRunner; import pro.taskana.common.internal.transaction.TaskanaTransactionProvider; import pro.taskana.task.internal.jobs.TaskCleanupJob; @@ -57,49 +49,18 @@ public class JobScheduler { @Scheduled(cron = "${taskana.jobscheduler.async.cron}") public void triggerJobs() { LOGGER.info("AsyncJobs started."); - try { - runAsyncJobsAsAdmin(); - LOGGER.info("AsyncJobs completed."); - } catch (PrivilegedActionException e) { - LOGGER.info("AsyncJobs failed.", e); - } + runAsyncJobsAsAdmin(); + LOGGER.info("AsyncJobs completed."); } - /* - * Creates an admin subject and runs the job using the subject. - */ - private void runAsyncJobsAsAdmin() throws PrivilegedActionException { - PrivilegedExceptionAction jobs = + private void runAsyncJobsAsAdmin() { + taskanaEngine.runAsAdmin( () -> { - try { - JobRunner runner = new JobRunner(taskanaEngine); - runner.registerTransactionProvider(springTransactionProvider); - LOGGER.info("Running Jobs"); - runner.runJobs(); - return "Successful"; - } catch (Throwable e) { - throw new Exception(e); - } - }; - Subject.doAs(getAdminSubject(), jobs); - } - - private Subject getAdminSubject() { - Subject subject = new Subject(); - List principalList = new ArrayList<>(); - try { - principalList.add( - new UserPrincipal( - taskanaEngine - .getConfiguration() - .getRoleMap() - .get(TaskanaRole.ADMIN) - .iterator() - .next())); - } catch (Throwable t) { - LOGGER.warn("Could not determine a configured admin user.", t); - } - subject.getPrincipals().addAll(principalList); - return subject; + JobRunner runner = new JobRunner(taskanaEngine); + runner.registerTransactionProvider(springTransactionProvider); + LOGGER.info("Running Jobs"); + runner.runJobs(); + return "Successful"; + }); } } diff --git a/rest/taskana-rest-spring-example-common/src/test/java/pro/taskana/example/ldap/LdapEmptySearchRootsTest.java b/rest/taskana-rest-spring-example-common/src/test/java/pro/taskana/example/ldap/LdapEmptySearchRootsTest.java index 0e81ebfac..70537af1b 100644 --- a/rest/taskana-rest-spring-example-common/src/test/java/pro/taskana/example/ldap/LdapEmptySearchRootsTest.java +++ b/rest/taskana-rest-spring-example-common/src/test/java/pro/taskana/example/ldap/LdapEmptySearchRootsTest.java @@ -27,7 +27,7 @@ class LdapEmptySearchRootsTest extends LdapTest { } @Test - void should_ReturnFullDnForUser_When_AccessIdOfUserIsGiven() { + void should_ReturnFullDnForUser_When_AccessIdOfUserIsGiven() throws Exception { String dn = ldapClient.searchDnForAccessId("otheruser"); assertThat(dn).isEqualTo("uid=otheruser,cn=other-users,ou=test,o=taskana"); } diff --git a/rest/taskana-rest-spring-example-common/src/test/java/pro/taskana/example/ldap/LdapTest.java b/rest/taskana-rest-spring-example-common/src/test/java/pro/taskana/example/ldap/LdapTest.java index 32ea78751..8e00c7b6b 100644 --- a/rest/taskana-rest-spring-example-common/src/test/java/pro/taskana/example/ldap/LdapTest.java +++ b/rest/taskana-rest-spring-example-common/src/test/java/pro/taskana/example/ldap/LdapTest.java @@ -44,7 +44,7 @@ class LdapTest { } @Test - void should_FeturnFullDnForUser_When_AccessIdOfUserIsGiven() { + void should_ReturnFullDnForUser_When_AccessIdOfUserIsGiven() throws Exception { String dn = ldapClient.searchDnForAccessId("user-2-2"); assertThat(dn).isEqualTo("uid=user-2-2,cn=users,ou=test,o=taskana"); } diff --git a/rest/taskana-rest-spring/src/docs/asciidoc/rest-api.adoc b/rest/taskana-rest-spring/src/docs/asciidoc/rest-api.adoc index f15fdcb5a..53e49c659 100644 --- a/rest/taskana-rest-spring/src/docs/asciidoc/rest-api.adoc +++ b/rest/taskana-rest-spring/src/docs/asciidoc/rest-api.adoc @@ -1,18 +1,91 @@ = TASKANA RESTful API Documentation == Overview + This is the REST documentation for http://taskana.pro)[TASKANA] - the world's first open source solution for Enterprise Task Management. *For all Query Parameters:* whenever a parameter is an array type, several values can be passed by declaring that parameter multiple times. === Hypermedia Support -NOTE: HATEOAS support is still in development. Please have a look at example responses for each resource to determine the available links. + +NOTE: HATEOAS support is still in development. +Please have a look at example responses for each resource to determine the available links. TASKANA uses the https://restfulapi.net/hateoas/)[HATEOAS] (Hypermedia as the Engine of Application State) REST constraint. Most of our resources contain a `_links` section which contains navigation links. Besides, helping to navigate through our REST API, the navigation links also encapsulate the API. Using HATEOAS allows us to change some endpoints without modifying your frontend. +=== Errors + +In order to support multilingual websites, TASKANA uses error codes to define which error occurred. +Additionally, an optional set of message variables, containing some technical information, is added, so that the website can describe the error with all details. + +[%autowidth,width="100%"] +|=== +| Status Code | Key | Message Variables +| *400 BAD_REQUEST* | CLASSIFICATION_SERVICE_LEVEL_MALFORMED | serviceLevel, classificationKey, domain +| *400 BAD_REQUEST* | CUSTOM_HOLIDAY_WRONG_FORMAT | customHoliday +| *400 BAD_REQUEST* | DOMAIN_NOT_FOUND | domain +| *400 BAD_REQUEST* | INVALID_ARGUMENT | +| *400 BAD_REQUEST* | QUERY_PARAMETER_MALFORMED | malformedQueryParameters +| *400 BAD_REQUEST* | TASK_INVALID_CALLBACK_STATE | taskId, taskCallbackState, requiredCallbackStates +| *400 BAD_REQUEST* | TASK_INVALID_OWNER | taskId, currentUserId +| *400 BAD_REQUEST* | TASK_INVALID_STATE | taskId, taskState, requiredTaskStates +| *403 FORBIDDEN* | ROLE_MISMATCHED | roles, currentUserId +| *403 FORBIDDEN* | TASK_COMMENT_CREATOR_MISMATCHED | currentUserId, taskCommentId +| *403 FORBIDDEN* | WORKBASKET_WITH_ID_MISMATCHED_PERMISSION | currentUserId, workbasketId, requiredPermissions +| *403 FORBIDDEN* | WORKBASKET_WITH_KEY_MISMATCHED_PERMISSION | currentUserId, workbasketKey, domain, requiredPermissions +| *404 NOT_FOUND* | CLASSIFICATION_WITH_ID_NOT_FOUND | classificationId +| *404 NOT_FOUND* | CLASSIFICATION_WITH_KEY_NOT_FOUND | classificationKey, domain +| *404 NOT_FOUND* | HISTORY_EVENT_NOT_FOUND | historyEventId +| *404 NOT_FOUND* | TASK_COMMENT_NOT_FOUND | taskCommentId +| *404 NOT_FOUND* | TASK_NOT_FOUND | taskId +| *404 NOT_FOUND* | WORKBASKET_WITH_ID_NOT_FOUND | workbasketId +| *404 NOT_FOUND* | WORKBASKET_WITH_KEY_NOT_FOUND | workbasketKey, domain +| *409 CONFLICT* | ATTACHMENT_ALREADY_EXISTS | attachmentId, taskId +| *409 CONFLICT* | CLASSIFICATION_ALREADY_EXISTS | classificationKey, domain +| *409 CONFLICT* | ENTITY_NOT_UP_TO_DATE | +| *409 CONFLICT* | TASK_ALREADY_EXISTS | externalTaskId +| *409 CONFLICT* | WORKBASKET_ACCESS_ITEM_ALREADY_EXISTS | accessId, workbasketId +| *409 CONFLICT* | WORKBASKET_ALREADY_EXISTS | workbasketKey, domain +| *413 PAYLOAD_TOO_LARGE* | PAYLOAD_TOO_LARGE | +| *423 LOCKED* | CLASSIFICATION_IN_USE | classificationKey, domain +| *423 LOCKED* | WORKBASKET_IN_USE | workbasketId +| *500 INTERNAL_SERVER_ERROR* | CONNECTION_AUTOCOMMIT_FAILED | +| *500 INTERNAL_SERVER_ERROR* | CONNECTION_NOT_SET | +| *500 INTERNAL_SERVER_ERROR* | CRITICAL_SYSTEM_ERROR | +| *500 INTERNAL_SERVER_ERROR* | DATABASE_UNSUPPORTED | databaseProductName +| *500 INTERNAL_SERVER_ERROR* | UNKNOWN_ERROR | +|=== + +==== Errors + +|==== +| Message Variables | Type +| accessId | String +| attachmentId | String +| classificationId | String +| classificationKey | String +| currentUserId | String +| customHoliday | String +| databaseProductName | String +| domain | String +| externalTaskId | String +| historyEventId | String +| malformedQueryParameters | MalformedQueryParameter[] +| requiredCallbackStates | CallbackState[] +| requiredPermissions | WorkbasketPermission[] +| requiredTaskStates | TaskState[] +| roles | TaskanaRole[] +| taskCallbackState | CallbackState +| taskCommentId | String +| taskId | String +| taskState | TaskState +| workbasketId | String +| workbasketKey | String +|==== + == Task Resource include::{snippets}/TaskControllerRestDocTest/createTaskDocTest/auto-section.adoc[] diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/classification/rest/ClassificationController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/classification/rest/ClassificationController.java index 3557e84b4..806e2b7e9 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/classification/rest/ClassificationController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/classification/rest/ClassificationController.java @@ -25,6 +25,7 @@ import pro.taskana.classification.api.ClassificationService; import pro.taskana.classification.api.exceptions.ClassificationAlreadyExistException; import pro.taskana.classification.api.exceptions.ClassificationInUseException; import pro.taskana.classification.api.exceptions.ClassificationNotFoundException; +import pro.taskana.classification.api.exceptions.MalformedServiceLevelException; import pro.taskana.classification.api.models.Classification; import pro.taskana.classification.api.models.ClassificationSummary; import pro.taskana.classification.rest.assembler.ClassificationRepresentationModelAssembler; @@ -123,13 +124,15 @@ public class ClassificationController { * @throws DomainNotFoundException if the domain within the new Classification does not exist. * @throws InvalidArgumentException if the new Classification does not contain all relevant * information. + * @throws MalformedServiceLevelException if the {@code serviceLevel} property does not comply * + * with the ISO 8601 specification */ @PostMapping(path = RestEndpoints.URL_CLASSIFICATIONS) @Transactional(rollbackFor = Exception.class) public ResponseEntity createClassification( @RequestBody ClassificationRepresentationModel repModel) throws NotAuthorizedException, ClassificationAlreadyExistException, DomainNotFoundException, - InvalidArgumentException { + InvalidArgumentException, MalformedServiceLevelException { Classification classification = modelAssembler.toEntityModel(repModel); classification = classificationService.createClassification(classification); @@ -148,6 +151,8 @@ public class ClassificationController { * @throws ConcurrencyException if the requested Classification Id has been modified in the * meantime by a different process. * @throws InvalidArgumentException if the Id in the path and in the request body does not match + * @throws MalformedServiceLevelException if the {@code serviceLevel} property does not comply * + * with the ISO 8601 specification */ @PutMapping(path = RestEndpoints.URL_CLASSIFICATIONS_ID) @Transactional(rollbackFor = Exception.class) @@ -155,7 +160,7 @@ public class ClassificationController { @PathVariable(value = "classificationId") String classificationId, @RequestBody ClassificationRepresentationModel resource) throws NotAuthorizedException, ClassificationNotFoundException, ConcurrencyException, - InvalidArgumentException { + InvalidArgumentException, MalformedServiceLevelException { if (!classificationId.equals(resource.getClassificationId())) { throw new InvalidArgumentException( String.format( diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/classification/rest/ClassificationDefinitionController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/classification/rest/ClassificationDefinitionController.java index 688a1e389..39c6f8adc 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/classification/rest/ClassificationDefinitionController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/classification/rest/ClassificationDefinitionController.java @@ -13,7 +13,6 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.dao.DuplicateKeyException; import org.springframework.hateoas.config.EnableHypermediaSupport; import org.springframework.http.ResponseEntity; import org.springframework.transaction.annotation.Transactional; @@ -28,6 +27,7 @@ import pro.taskana.classification.api.ClassificationQuery; import pro.taskana.classification.api.ClassificationService; import pro.taskana.classification.api.exceptions.ClassificationAlreadyExistException; import pro.taskana.classification.api.exceptions.ClassificationNotFoundException; +import pro.taskana.classification.api.exceptions.MalformedServiceLevelException; import pro.taskana.classification.api.models.Classification; import pro.taskana.classification.api.models.ClassificationSummary; import pro.taskana.classification.rest.assembler.ClassificationDefinitionCollectionRepresentationModel; @@ -62,9 +62,9 @@ public class ClassificationDefinitionController { /** * This endpoint exports all configured Classifications. * + * @title Export Classifications * @param domain Filter the export by domain * @return the configured Classifications. - * @title Export Classifications */ @GetMapping(path = RestEndpoints.URL_CLASSIFICATION_DEFINITIONS) @Transactional(readOnly = true, rollbackFor = Exception.class) @@ -100,13 +100,15 @@ public class ClassificationDefinitionController { * @throws ClassificationAlreadyExistException TODO: this makes no sense * @throws DomainNotFoundException if the domain for a specific Classification does not exist * @throws IOException if the import file could not be parsed + * @throws MalformedServiceLevelException if the {@code serviceLevel} property does not comply * + * with the ISO 8601 specification */ @PostMapping(path = RestEndpoints.URL_CLASSIFICATION_DEFINITIONS) @Transactional(rollbackFor = Exception.class) public ResponseEntity importClassifications(@RequestParam("file") MultipartFile file) throws InvalidArgumentException, NotAuthorizedException, ConcurrencyException, ClassificationNotFoundException, ClassificationAlreadyExistException, - DomainNotFoundException, IOException { + DomainNotFoundException, IOException, MalformedServiceLevelException { Map systemIds = getSystemIds(); ClassificationDefinitionCollectionRepresentationModel collection = extractClassificationResourcesFromFile(file); @@ -132,21 +134,17 @@ public class ClassificationDefinitionController { } private void checkForDuplicates( - Collection definitionList) { + Collection definitionList) + throws ClassificationAlreadyExistException { List identifiers = new ArrayList<>(); - Set duplicates = new HashSet<>(); for (ClassificationDefinitionRepresentationModel definition : definitionList) { ClassificationRepresentationModel classification = definition.getClassification(); String identifier = classification.getKey() + "|" + classification.getDomain(); if (identifiers.contains(identifier)) { - duplicates.add(identifier); - } else { - identifiers.add(identifier); + throw new ClassificationAlreadyExistException( + definition.getClassification().getKey(), definition.getClassification().getDomain()); } - } - if (!duplicates.isEmpty()) { - throw new DuplicateKeyException( - "The 'key|domain'-identifier is not unique for the value(s): " + duplicates); + identifiers.add(identifier); } } @@ -188,7 +186,8 @@ public class ClassificationDefinitionController { Collection definitionList, Map systemIds) throws ClassificationNotFoundException, NotAuthorizedException, InvalidArgumentException, - ClassificationAlreadyExistException, DomainNotFoundException, ConcurrencyException { + ClassificationAlreadyExistException, DomainNotFoundException, ConcurrencyException, + MalformedServiceLevelException { for (ClassificationDefinitionRepresentationModel definition : definitionList) { ClassificationRepresentationModel classificationRepModel = definition.getClassification(); classificationRepModel.setParentKey(null); @@ -209,7 +208,7 @@ public class ClassificationDefinitionController { private void updateParentChildrenRelations(Map childrenInFile) throws ClassificationNotFoundException, NotAuthorizedException, ConcurrencyException, - InvalidArgumentException { + InvalidArgumentException, MalformedServiceLevelException { for (Map.Entry entry : childrenInFile.entrySet()) { Classification childRes = entry.getKey(); String parentKey = entry.getValue(); @@ -232,7 +231,7 @@ public class ClassificationDefinitionController { private void updateExistingClassification(Classification newClassification, String systemId) throws ClassificationNotFoundException, NotAuthorizedException, ConcurrencyException, - InvalidArgumentException { + InvalidArgumentException, MalformedServiceLevelException { Classification currentClassification = classificationService.getClassification(systemId); if (newClassification.getType() != null && !newClassification.getType().equals(currentClassification.getType())) { diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/TaskanaRestExceptionHandler.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/TaskanaRestExceptionHandler.java index ee8443cbd..942993ee4 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/TaskanaRestExceptionHandler.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/TaskanaRestExceptionHandler.java @@ -1,12 +1,25 @@ package pro.taskana.common.rest; +import java.io.Serializable; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.springframework.beans.TypeMismatchException; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; -import org.springframework.dao.DuplicateKeyException; +import org.springframework.core.convert.ConversionFailedException; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.lang.NonNull; import org.springframework.validation.BindException; +import org.springframework.validation.FieldError; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.context.request.WebRequest; @@ -14,148 +27,211 @@ import org.springframework.web.multipart.MaxUploadSizeExceededException; import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; import pro.taskana.classification.api.exceptions.ClassificationAlreadyExistException; +import pro.taskana.classification.api.exceptions.ClassificationInUseException; +import pro.taskana.classification.api.exceptions.ClassificationNotFoundException; import pro.taskana.common.api.exceptions.ConcurrencyException; import pro.taskana.common.api.exceptions.DomainNotFoundException; +import pro.taskana.common.api.exceptions.ErrorCode; import pro.taskana.common.api.exceptions.InvalidArgumentException; -import pro.taskana.common.api.exceptions.NotAuthorizedException; +import pro.taskana.common.api.exceptions.MismatchedRoleException; import pro.taskana.common.api.exceptions.NotFoundException; -import pro.taskana.common.rest.models.TaskanaErrorData; +import pro.taskana.common.api.exceptions.TaskanaException; +import pro.taskana.common.api.exceptions.TaskanaRuntimeException; +import pro.taskana.common.internal.util.MapCreator; +import pro.taskana.common.rest.models.ExceptionRepresentationModel; +import pro.taskana.spi.history.api.exceptions.TaskanaHistoryEventNotFoundException; +import pro.taskana.task.api.exceptions.AttachmentPersistenceException; +import pro.taskana.task.api.exceptions.InvalidCallbackStateException; import pro.taskana.task.api.exceptions.InvalidOwnerException; -import pro.taskana.task.api.exceptions.InvalidStateException; +import pro.taskana.task.api.exceptions.InvalidTaskStateException; +import pro.taskana.task.api.exceptions.MismatchedTaskCommentCreatorException; import pro.taskana.task.api.exceptions.TaskAlreadyExistException; -import pro.taskana.workbasket.api.exceptions.InvalidWorkbasketException; +import pro.taskana.task.api.exceptions.TaskCommentNotFoundException; +import pro.taskana.task.api.exceptions.TaskNotFoundException; +import pro.taskana.workbasket.api.exceptions.MismatchedWorkbasketPermissionException; import pro.taskana.workbasket.api.exceptions.NotAuthorizedToQueryWorkbasketException; import pro.taskana.workbasket.api.exceptions.WorkbasketAccessItemAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketInUseException; +import pro.taskana.workbasket.api.exceptions.WorkbasketMarkedForDeletionException; +import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; -/** This class handles taskana exceptions. */ +/** This class handles TASKANA exceptions. */ @Order(Ordered.HIGHEST_PRECEDENCE) @ControllerAdvice public class TaskanaRestExceptionHandler extends ResponseEntityExceptionHandler { - @ExceptionHandler(InvalidArgumentException.class) - protected ResponseEntity handleInvalidArgument( - InvalidArgumentException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.BAD_REQUEST, false); + public static final String ERROR_KEY_QUERY_MALFORMED = "QUERY_PARAMETER_MALFORMED"; + public static final String ERROR_KEY_PAYLOAD = "PAYLOAD_TOO_LARGE"; + + @ExceptionHandler({ + MismatchedRoleException.class, + MismatchedWorkbasketPermissionException.class, + MismatchedTaskCommentCreatorException.class, + NotAuthorizedToQueryWorkbasketException.class + }) + protected ResponseEntity handleForbiddenExceptions(TaskanaException ex, WebRequest req) { + return buildResponse(ex.getErrorCode(), ex, req, HttpStatus.FORBIDDEN); } - @ExceptionHandler(NotAuthorizedException.class) - protected ResponseEntity handleNotAuthorized(NotAuthorizedException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.FORBIDDEN); + @ExceptionHandler({ + ClassificationNotFoundException.class, + TaskCommentNotFoundException.class, + TaskNotFoundException.class, + TaskanaHistoryEventNotFoundException.class, + WorkbasketNotFoundException.class + }) + protected ResponseEntity handleNotFound(NotFoundException ex, WebRequest req) { + return buildResponse(ex.getErrorCode(), ex, req, HttpStatus.NOT_FOUND); } - @ExceptionHandler(NotFoundException.class) - protected ResponseEntity handleTaskNotFound(NotFoundException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.NOT_FOUND); + @ExceptionHandler({ + TaskAlreadyExistException.class, + ClassificationAlreadyExistException.class, + ConcurrencyException.class, + WorkbasketAlreadyExistException.class, + WorkbasketAccessItemAlreadyExistException.class, + AttachmentPersistenceException.class, + WorkbasketMarkedForDeletionException.class + }) + protected ResponseEntity handleConflictExceptions(TaskanaException ex, WebRequest req) { + return buildResponse(ex.getErrorCode(), ex, req, HttpStatus.CONFLICT); } - @ExceptionHandler(TaskAlreadyExistException.class) - protected ResponseEntity handleTaskAlreadyExist( - TaskAlreadyExistException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.CONFLICT); - } - - @ExceptionHandler(NotAuthorizedToQueryWorkbasketException.class) - protected ResponseEntity handleNotAuthorizedToQueryWorkbasket( - NotAuthorizedToQueryWorkbasketException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.FORBIDDEN); - } - - @ExceptionHandler(InvalidStateException.class) - protected ResponseEntity handleInvalidState(InvalidStateException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.CONFLICT); - } - - @ExceptionHandler(InvalidOwnerException.class) - protected ResponseEntity handleInvalidOwner(InvalidOwnerException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.CONFLICT); - } - - @ExceptionHandler(ClassificationAlreadyExistException.class) - protected ResponseEntity handleClassificationAlreadyExist( - ClassificationAlreadyExistException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.CONFLICT); - } - - @ExceptionHandler(DuplicateKeyException.class) - protected ResponseEntity handleDuplicateKey(DuplicateKeyException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.CONFLICT); - } - - @ExceptionHandler(ConcurrencyException.class) - protected ResponseEntity handleConcurrencyException( - ConcurrencyException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.CONFLICT); - } - - @ExceptionHandler(WorkbasketInUseException.class) + @ExceptionHandler({WorkbasketInUseException.class, ClassificationInUseException.class}) protected ResponseEntity handleWorkbasketInUse( WorkbasketInUseException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.LOCKED); - } - - @ExceptionHandler(WorkbasketAlreadyExistException.class) - protected ResponseEntity handleWorkbasketAlreadyExist( - WorkbasketAlreadyExistException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.CONFLICT); - } - - @ExceptionHandler(WorkbasketAccessItemAlreadyExistException.class) - protected ResponseEntity handleWorkbasketAccessItemAlreadyExist( - WorkbasketAccessItemAlreadyExistException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.CONFLICT); - } - - @ExceptionHandler(InvalidWorkbasketException.class) - protected ResponseEntity handleInvalidWorkbasket( - InvalidWorkbasketException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.BAD_REQUEST); - } - - @ExceptionHandler(DomainNotFoundException.class) - protected ResponseEntity handleDomainNotFound( - DomainNotFoundException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.BAD_REQUEST); + return buildResponse(ex.getErrorCode(), ex, req, HttpStatus.LOCKED); } @ExceptionHandler(MaxUploadSizeExceededException.class) protected ResponseEntity handleMaxUploadSizeExceededException( MaxUploadSizeExceededException ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.PAYLOAD_TOO_LARGE); + return buildResponse(ErrorCode.of(ERROR_KEY_PAYLOAD), ex, req, HttpStatus.PAYLOAD_TOO_LARGE); } - @Override - protected ResponseEntity handleBindException( - BindException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { - return buildResponse(ex, request, HttpStatus.BAD_REQUEST); + @ExceptionHandler({ + InvalidTaskStateException.class, + InvalidCallbackStateException.class, + InvalidOwnerException.class, + InvalidArgumentException.class, + DomainNotFoundException.class, + TaskanaException.class + }) + protected ResponseEntity handleBadRequestExceptions(TaskanaException ex, WebRequest req) { + return buildResponse(ex.getErrorCode(), ex, req, HttpStatus.BAD_REQUEST); + } + + @ExceptionHandler(TaskanaRuntimeException.class) + protected ResponseEntity handleInternalServerExceptions( + TaskanaRuntimeException ex, WebRequest req) { + return buildResponse(ex.getErrorCode(), ex, req, HttpStatus.INTERNAL_SERVER_ERROR); } @ExceptionHandler(Exception.class) protected ResponseEntity handleGeneralException(Exception ex, WebRequest req) { - return buildResponse(ex, req, HttpStatus.BAD_REQUEST); + return buildResponse(ErrorCode.of("UNKNOWN_ERROR"), ex, req, HttpStatus.INTERNAL_SERVER_ERROR); } - private ResponseEntity buildResponse(Throwable ex, WebRequest req, HttpStatus status) { - return buildResponse(ex, req, status, true); + @Override + @NonNull + protected ResponseEntity handleBindException( + BindException ex, + @NonNull HttpHeaders headers, + @NonNull HttpStatus status, + @NonNull WebRequest request) { + + MalformedQueryParameter[] wrongQueryParameters = + ex.getBindingResult().getFieldErrors().stream() + .map(this::extractMalformedQueryParameters) + .flatMap(Collection::stream) + .toArray(MalformedQueryParameter[]::new); + + // if we have no wrong query parameter then this BindException is representing something else. + // Therefore we only create an ErrorCode when we have found a wrong query parameter. + ErrorCode errorCode = + wrongQueryParameters.length != 0 + ? ErrorCode.of( + ERROR_KEY_QUERY_MALFORMED, + MapCreator.of("malformedQueryParameters", wrongQueryParameters)) + : null; + + return buildResponse(errorCode, ex, request, HttpStatus.BAD_REQUEST); } private ResponseEntity buildResponse( - Throwable ex, WebRequest req, HttpStatus status, boolean logExceptionOnError) { - TaskanaErrorData errorData = new TaskanaErrorData(status, ex, req); - if (logExceptionOnError) { - logger.error( - String.format( - "Error occurred during processing of rest request: %s", errorData.toString()), - ex); - } else { - if (logger.isDebugEnabled()) { - logger.debug( - String.format( - "Error occurred during processing of rest request: %s", errorData.toString()), - ex); - } - } + ErrorCode errorCode, Throwable ex, WebRequest req, HttpStatus status) { + ExceptionRepresentationModel errorData = + new ExceptionRepresentationModel(errorCode, status, ex, req); + logger.error( + String.format("Error occurred during processing of rest request: %s", errorData), ex); return ResponseEntity.status(status).body(errorData); } + + private List extractMalformedQueryParameters(FieldError fieldError) { + if (fieldError.contains(TypeMismatchException.class)) { + TypeMismatchException typeMismatchException = fieldError.unwrap(TypeMismatchException.class); + if (typeMismatchException.getCause() instanceof ConversionFailedException) { + ConversionFailedException conversionFailedException = + (ConversionFailedException) typeMismatchException.getCause(); + Class targetType = conversionFailedException.getTargetType().getType(); + if (targetType.isEnum()) { + String queryParameter = fieldError.getField(); + // the redundancy below exists because we want to keep the enums sorted by their ordinal + // value for the error output and want to use the contains performance boost of a HashSet. + List enumConstants = + Arrays.stream(targetType.getEnumConstants()) + .map(Object::toString) + .collect(Collectors.toList()); + Set enumConstantSet = new HashSet<>(enumConstants); + + return getRejectedValues(typeMismatchException) + .filter(value -> !enumConstantSet.contains(value)) + .map(value -> new MalformedQueryParameter(queryParameter, value, enumConstants)) + .collect(Collectors.toList()); + } + } + } + + return Collections.emptyList(); + } + + private Stream getRejectedValues(TypeMismatchException ex) { + Object value = ex.getValue(); + if (value != null && value.getClass().isArray()) { + return Arrays.stream((Object[]) value).map(Objects::toString); + } + if (value != null && value.getClass().isAssignableFrom(Collection.class)) { + return ((Collection) value).stream().map(Objects::toString); + } + return Stream.of(value).map(Objects::toString); + } + + public static class MalformedQueryParameter implements Serializable { + private final String queryParameter; + private final String actualValue; + private final Collection expectedValues; + + MalformedQueryParameter( + String queryParameter, String actualValue, Collection expectedValues) { + this.queryParameter = queryParameter; + this.actualValue = actualValue; + this.expectedValues = expectedValues; + } + + @SuppressWarnings("unused") + public String getActualValue() { + return actualValue; + } + + @SuppressWarnings("unused") + public Collection getExpectedValues() { + return expectedValues; + } + + @SuppressWarnings("unused") + public String getQueryParameter() { + return queryParameter; + } + } } diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/ldap/LdapClient.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/ldap/LdapClient.java index 5b7df3ca4..1e55396f7 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/ldap/LdapClient.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/ldap/LdapClient.java @@ -28,7 +28,6 @@ import pro.taskana.TaskanaEngineConfiguration; import pro.taskana.common.api.TaskanaRole; import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.SystemException; -import pro.taskana.common.api.exceptions.TaskanaRuntimeException; import pro.taskana.common.rest.models.AccessIdRepresentationModel; /** Class for Ldap access. */ @@ -247,13 +246,13 @@ public class LdapClient { } /** - * Performs a lookup to retrieve correct DN for the given accessId. + * Performs a lookup to retrieve correct DN for the given access id. * - * @param accessId The AccessId to lookup - * @return the LDAP Distinguished Name for the AccessId - * @throws TaskanaRuntimeException thrown if the given AccessId is ambiguous. + * @param accessId The access id to lookup + * @return the LDAP Distinguished Name for the access id + * @throws InvalidArgumentException thrown if the given access id is ambiguous. */ - public String searchDnForAccessId(String accessId) throws TaskanaRuntimeException { + public String searchDnForAccessId(String accessId) throws InvalidArgumentException { isInitOrFail(); if (nameIsDn(accessId)) { @@ -286,7 +285,7 @@ public class LdapClient { if (distinguishedNames == null || distinguishedNames.isEmpty()) { return null; } else if (distinguishedNames.size() > 1) { - throw new TaskanaRuntimeException("Ambiguous AccessId found: " + accessId); + throw new InvalidArgumentException("Ambiguous access id found: " + accessId); } else { return distinguishedNames.get(0); } diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/models/TaskanaErrorData.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/models/ExceptionRepresentationModel.java similarity index 70% rename from rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/models/TaskanaErrorData.java rename to rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/models/ExceptionRepresentationModel.java index 0eed5c06c..5e1915e4b 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/models/TaskanaErrorData.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/models/ExceptionRepresentationModel.java @@ -1,23 +1,23 @@ package pro.taskana.common.rest.models; -import java.util.Date; import org.springframework.http.HttpStatus; import org.springframework.web.context.request.WebRequest; -/** This class holds error data. */ -public class TaskanaErrorData { +import pro.taskana.common.api.exceptions.ErrorCode; - private final Date timestamp; +/** This class holds error data. */ +public class ExceptionRepresentationModel { + + private final ErrorCode error; private final int status; - private final String error; private final String exception; private final String message; private String path; - public TaskanaErrorData(HttpStatus stat, Throwable ex, WebRequest req) { - this.timestamp = new Date(); + public ExceptionRepresentationModel( + ErrorCode errorCode, HttpStatus stat, Throwable ex, WebRequest req) { + this.error = errorCode; this.status = stat.value(); - this.error = stat.name(); this.exception = ex.getClass().getName(); this.message = ex.getMessage(); this.path = req.getDescription(false); @@ -26,18 +26,14 @@ public class TaskanaErrorData { } } - public Date getTimestamp() { - return timestamp; + public ErrorCode getError() { + return error; } public int getStatus() { return status; } - public String getError() { - return error; - } - public String getException() { return exception; } @@ -52,12 +48,10 @@ public class TaskanaErrorData { @Override public String toString() { - return "TaskanaErrorData [timestamp=" - + timestamp + return "ExceptionRepresentationModel [error=" + + error + ", status=" + status - + ", error=" - + error + ", exception=" + exception + ", message=" diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/util/QueryParamsValidator.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/util/QueryParamsValidator.java index d3d786c6e..8ede3b021 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/util/QueryParamsValidator.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/util/QueryParamsValidator.java @@ -14,8 +14,7 @@ public class QueryParamsValidator { throw new IllegalStateException("Utility class"); } - public static void validateParams(HttpServletRequest request, Class... filterOrSortingClazz) { - + public static void validateParams(HttpServletRequest request, Class... filterOrSortingClazz) { Set allowedParams = Stream.of(filterOrSortingClazz) .flatMap(clazz -> Stream.of(clazz.getDeclaredFields())) diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketController.java index f44d5d2e4..8d217d502 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketController.java @@ -35,7 +35,6 @@ import pro.taskana.common.rest.util.QueryParamsValidator; import pro.taskana.workbasket.api.WorkbasketCustomField; import pro.taskana.workbasket.api.WorkbasketQuery; import pro.taskana.workbasket.api.WorkbasketService; -import pro.taskana.workbasket.api.exceptions.InvalidWorkbasketException; import pro.taskana.workbasket.api.exceptions.WorkbasketAccessItemAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketInUseException; @@ -186,7 +185,7 @@ public class WorkbasketController { * @title Create a new Workbasket * @param workbasketRepresentationModel the Workbasket which should be created. * @return the created Workbasket - * @throws InvalidWorkbasketException if some required properties of the Workbasket are not set. + * @throws InvalidArgumentException if some required properties of the Workbasket are not set. * @throws NotAuthorizedException if the current user is not member of role BUSINESS_ADMIN or * ADMIN * @throws WorkbasketAlreadyExistException if the Workbasket exists already @@ -196,7 +195,7 @@ public class WorkbasketController { @Transactional(rollbackFor = Exception.class) public ResponseEntity createWorkbasket( @RequestBody WorkbasketRepresentationModel workbasketRepresentationModel) - throws InvalidWorkbasketException, NotAuthorizedException, WorkbasketAlreadyExistException, + throws InvalidArgumentException, NotAuthorizedException, WorkbasketAlreadyExistException, DomainNotFoundException { Workbasket workbasket = workbasketRepresentationModelAssembler.toEntityModel(workbasketRepresentationModel); @@ -213,7 +212,7 @@ public class WorkbasketController { * @param workbasketId the Id of the Workbasket which should be updated. * @param workbasketRepresentationModel the new Workbasket for the requested id. * @return the updated Workbasket - * @throws InvalidWorkbasketException if the requested Id and the Id within the new Workbasket do + * @throws InvalidArgumentException if the requested Id and the Id within the new Workbasket do * not match. * @throws WorkbasketNotFoundException if the requested workbasket does not * @throws NotAuthorizedException if the current user is not authorized to update the Workbasket @@ -225,10 +224,10 @@ public class WorkbasketController { public ResponseEntity updateWorkbasket( @PathVariable(value = "workbasketId") String workbasketId, @RequestBody WorkbasketRepresentationModel workbasketRepresentationModel) - throws InvalidWorkbasketException, WorkbasketNotFoundException, NotAuthorizedException, - ConcurrencyException { + throws WorkbasketNotFoundException, NotAuthorizedException, ConcurrencyException, + InvalidArgumentException { if (!workbasketId.equals(workbasketRepresentationModel.getWorkbasketId())) { - throw new InvalidWorkbasketException( + throw new InvalidArgumentException( "Target-WB-ID('" + workbasketId + "') is not identical with the WB-ID of to object which should be updated. ID=('" @@ -290,7 +289,7 @@ public class WorkbasketController { throws NotAuthorizedException, InvalidArgumentException, WorkbasketNotFoundException, WorkbasketAccessItemAlreadyExistException { if (workbasketAccessItemRepModels == null) { - throw new InvalidArgumentException("Can´t create something with NULL body-value."); + throw new InvalidArgumentException("Can't create something with NULL body-value."); } List wbAccessItems = diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketDefinitionController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketDefinitionController.java index 40811cdec..a71eef1de 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketDefinitionController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketDefinitionController.java @@ -15,7 +15,6 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.dao.DuplicateKeyException; import org.springframework.hateoas.config.EnableHypermediaSupport; import org.springframework.http.ResponseEntity; import org.springframework.transaction.annotation.Transactional; @@ -32,7 +31,6 @@ import pro.taskana.common.api.exceptions.NotAuthorizedException; import pro.taskana.common.rest.RestEndpoints; import pro.taskana.workbasket.api.WorkbasketQuery; import pro.taskana.workbasket.api.WorkbasketService; -import pro.taskana.workbasket.api.exceptions.InvalidWorkbasketException; import pro.taskana.workbasket.api.exceptions.WorkbasketAccessItemAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketAlreadyExistException; import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; @@ -114,24 +112,22 @@ public class WorkbasketDefinitionController { * @throws IOException if multipart file cannot be parsed. * @throws NotAuthorizedException if the user is not authorized. * @throws DomainNotFoundException if domain information is incorrect. - * @throws InvalidWorkbasketException if any Workbasket has invalid information. * @throws WorkbasketAlreadyExistException if any Workbasket already exists when trying to create * a new one. - * @throws WorkbasketNotFoundException if do not exists a workbasket in the system with the used - * id. - * @throws InvalidArgumentException if authorization information in workbaskets definitions is - * incorrect. + * @throws WorkbasketNotFoundException if do not exists a {@linkplain Workbasket} in the system + * with the used id. + * @throws InvalidArgumentException if any Workbasket has invalid information or authorization + * information in {@linkplain Workbasket}s' definitions is incorrect. * @throws WorkbasketAccessItemAlreadyExistException if a WorkbasketAccessItem for the same - * workbasket and access_id already exists. - * @throws ConcurrencyException if workbasket was updated by an other user + * Workbasket and access id already exists. + * @throws ConcurrencyException if Workbasket was updated by an other user */ @PostMapping(path = RestEndpoints.URL_WORKBASKET_DEFINITIONS) @Transactional(rollbackFor = Exception.class) public ResponseEntity importWorkbaskets(@RequestParam("file") MultipartFile file) - throws IOException, NotAuthorizedException, DomainNotFoundException, - InvalidWorkbasketException, WorkbasketAlreadyExistException, WorkbasketNotFoundException, - InvalidArgumentException, WorkbasketAccessItemAlreadyExistException, - ConcurrencyException { + throws IOException, NotAuthorizedException, DomainNotFoundException, InvalidArgumentException, + WorkbasketAlreadyExistException, WorkbasketNotFoundException, + WorkbasketAccessItemAlreadyExistException, ConcurrencyException { WorkbasketDefinitionCollectionRepresentationModel definitions = mapper.readValue( file.getInputStream(), @@ -174,7 +170,7 @@ public class WorkbasketDefinitionController { (access.getWorkbasketId().equals(importedWb.getId())) && (access.getWorkbasketKey().equals(importedWb.getKey()))); if (!authenticated && !definition.getAuthorizations().isEmpty()) { - throw new InvalidWorkbasketException( + throw new InvalidArgumentException( "The given Authentications for Workbasket " + importedWb.getId() + " don't match in WorkbasketId and WorkbasketKey. " @@ -201,7 +197,7 @@ public class WorkbasketDefinitionController { } else if (systemIds.containsValue(oldId)) { distributionTargets.add(oldId); } else { - throw new InvalidWorkbasketException( + throw new InvalidArgumentException( String.format( "invalid import state: Workbasket '%s' does not exist in the given import list", oldId)); @@ -221,20 +217,16 @@ public class WorkbasketDefinitionController { return workbasketAssembler.toEntityModel(wbRes); } - private void checkForDuplicates(Collection definitions) { - List identifiers = new ArrayList<>(); - Set duplicates = new HashSet<>(); + private void checkForDuplicates(Collection definitions) + throws WorkbasketAlreadyExistException { + Set identifiers = new HashSet<>(); for (WorkbasketDefinitionRepresentationModel definition : definitions) { String identifier = logicalId(workbasketAssembler.toEntityModel(definition.getWorkbasket())); if (identifiers.contains(identifier)) { - duplicates.add(identifier); - } else { - identifiers.add(identifier); + throw new WorkbasketAlreadyExistException( + definition.getWorkbasket().getKey(), definition.getWorkbasket().getDomain()); } - } - if (!duplicates.isEmpty()) { - throw new DuplicateKeyException( - "The 'key|domain'-identifier is not unique for the value(s): " + duplicates.toString()); + identifiers.add(identifier); } } @@ -242,10 +234,6 @@ public class WorkbasketDefinitionController { return logicalId(workbasket.getKey(), workbasket.getDomain()); } - private String logicalId(Workbasket workbasket) { - return logicalId(workbasket.getKey(), workbasket.getDomain()); - } - private String logicalId(String key, String domain) { return key + "|" + domain; } diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/classification/rest/ClassificationControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/classification/rest/ClassificationControllerIntTest.java index ae4d37ddb..82fcb278e 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/classification/rest/ClassificationControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/classification/rest/ClassificationControllerIntTest.java @@ -16,6 +16,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.test.annotation.DirtiesContext; import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.HttpServerErrorException; import pro.taskana.classification.rest.models.ClassificationRepresentationModel; import pro.taskana.classification.rest.models.ClassificationSummaryPagedRepresentationModel; @@ -344,10 +345,10 @@ class ClassificationControllerIntTest { }; assertThatThrownBy(httpCall) - .isInstanceOf(HttpClientErrorException.class) + .isInstanceOf(HttpServerErrorException.class) .hasMessageContaining( "Unkown request parameters found: [anotherIllegalParam, illegalParam]") - .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) - .isEqualTo(HttpStatus.BAD_REQUEST); + .extracting(ex -> ((HttpServerErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); } } diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/ExceptionErrorKeyTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/ExceptionErrorKeyTest.java new file mode 100644 index 000000000..a03ca097d --- /dev/null +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/ExceptionErrorKeyTest.java @@ -0,0 +1,104 @@ +package pro.taskana.common.rest; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +import pro.taskana.classification.api.exceptions.ClassificationAlreadyExistException; +import pro.taskana.classification.api.exceptions.ClassificationInUseException; +import pro.taskana.classification.api.exceptions.ClassificationNotFoundException; +import pro.taskana.classification.api.exceptions.MalformedServiceLevelException; +import pro.taskana.common.api.exceptions.AutocommitFailedException; +import pro.taskana.common.api.exceptions.ConcurrencyException; +import pro.taskana.common.api.exceptions.ConnectionNotSetException; +import pro.taskana.common.api.exceptions.DomainNotFoundException; +import pro.taskana.common.api.exceptions.MismatchedRoleException; +import pro.taskana.common.api.exceptions.SystemException; +import pro.taskana.common.api.exceptions.UnsupportedDatabaseException; +import pro.taskana.common.api.exceptions.WrongCustomHolidayFormatException; +import pro.taskana.spi.history.api.exceptions.TaskanaHistoryEventNotFoundException; +import pro.taskana.task.api.exceptions.AttachmentPersistenceException; +import pro.taskana.task.api.exceptions.InvalidCallbackStateException; +import pro.taskana.task.api.exceptions.InvalidOwnerException; +import pro.taskana.task.api.exceptions.InvalidTaskStateException; +import pro.taskana.task.api.exceptions.MismatchedTaskCommentCreatorException; +import pro.taskana.task.api.exceptions.TaskAlreadyExistException; +import pro.taskana.task.api.exceptions.TaskCommentNotFoundException; +import pro.taskana.task.api.exceptions.TaskNotFoundException; +import pro.taskana.workbasket.api.exceptions.MismatchedWorkbasketPermissionException; +import pro.taskana.workbasket.api.exceptions.WorkbasketAccessItemAlreadyExistException; +import pro.taskana.workbasket.api.exceptions.WorkbasketAlreadyExistException; +import pro.taskana.workbasket.api.exceptions.WorkbasketInUseException; +import pro.taskana.workbasket.api.exceptions.WorkbasketMarkedForDeletionException; +import pro.taskana.workbasket.api.exceptions.WorkbasketNotFoundException; + +class ExceptionErrorKeyTest { + + @Test + void should_ProvideConsistentErrorKey_For_ClassificationExceptions() { + assertThat(ClassificationAlreadyExistException.ERROR_KEY) + .isEqualTo("CLASSIFICATION_ALREADY_EXISTS"); + assertThat(ClassificationInUseException.ERROR_KEY).isEqualTo("CLASSIFICATION_IN_USE"); + assertThat(ClassificationNotFoundException.ERROR_KEY_ID) + .isEqualTo("CLASSIFICATION_WITH_ID_NOT_FOUND"); + assertThat(ClassificationNotFoundException.ERROR_KEY_KEY_DOMAIN) + .isEqualTo("CLASSIFICATION_WITH_KEY_NOT_FOUND"); + assertThat(MalformedServiceLevelException.ERROR_KEY) + .isEqualTo("CLASSIFICATION_SERVICE_LEVEL_MALFORMED"); + } + + @Test + void should_ProvideConsistentErrorKey_For_CommonExceptions() { + assertThat(AutocommitFailedException.ERROR_KEY).isEqualTo("CONNECTION_AUTOCOMMIT_FAILED"); + assertThat(ConcurrencyException.ERROR_KEY).isEqualTo("ENTITY_NOT_UP_TO_DATE"); + assertThat(ConnectionNotSetException.ERROR_KEY).isEqualTo("CONNECTION_NOT_SET"); + assertThat(DomainNotFoundException.ERROR_KEY).isEqualTo("DOMAIN_NOT_FOUND"); + assertThat(MismatchedRoleException.ERROR_KEY).isEqualTo("ROLE_MISMATCHED"); + assertThat(SystemException.ERROR_KEY).isEqualTo("CRITICAL_SYSTEM_ERROR"); + assertThat(UnsupportedDatabaseException.ERROR_KEY).isEqualTo("DATABASE_UNSUPPORTED"); + assertThat(WrongCustomHolidayFormatException.ERROR_KEY) + .isEqualTo("CUSTOM_HOLIDAY_WRONG_FORMAT"); + } + + @Test + void should_ProvideConsistentErrorKey_For_SpiExceptions() { + assertThat(TaskanaHistoryEventNotFoundException.ERROR_KEY).isEqualTo("HISTORY_EVENT_NOT_FOUND"); + } + + @Test + void should_ProvideConsistentErrorKey_For_TaskExceptions() { + assertThat(AttachmentPersistenceException.ERROR_KEY).isEqualTo("ATTACHMENT_ALREADY_EXISTS"); + assertThat(InvalidCallbackStateException.ERROR_KEY).isEqualTo("TASK_INVALID_CALLBACK_STATE"); + assertThat(InvalidOwnerException.ERROR_KEY).isEqualTo("TASK_INVALID_OWNER"); + assertThat(InvalidTaskStateException.ERROR_KEY).isEqualTo("TASK_INVALID_STATE"); + assertThat(MismatchedTaskCommentCreatorException.ERROR_KEY) + .isEqualTo("TASK_COMMENT_CREATOR_MISMATCHED"); + assertThat(TaskAlreadyExistException.ERROR_KEY).isEqualTo("TASK_ALREADY_EXISTS"); + assertThat(TaskCommentNotFoundException.ERROR_KEY).isEqualTo("TASK_COMMENT_NOT_FOUND"); + assertThat(TaskNotFoundException.ERROR_KEY).isEqualTo("TASK_NOT_FOUND"); + } + + @Test + void should_ProvideConsistentErrorKey_For_WorkbasketExceptions() { + assertThat(MismatchedWorkbasketPermissionException.ERROR_KEY_ID) + .isEqualTo("WORKBASKET_WITH_ID_MISMATCHED_PERMISSION"); + assertThat(MismatchedWorkbasketPermissionException.ERROR_KEY_KEY_DOMAIN) + .isEqualTo("WORKBASKET_WITH_KEY_MISMATCHED_PERMISSION"); + assertThat(WorkbasketAccessItemAlreadyExistException.ERROR_KEY) + .isEqualTo("WORKBASKET_ACCESS_ITEM_ALREADY_EXISTS"); + assertThat(WorkbasketAlreadyExistException.ERROR_KEY).isEqualTo("WORKBASKET_ALREADY_EXISTS"); + assertThat(WorkbasketInUseException.ERROR_KEY).isEqualTo("WORKBASKET_IN_USE"); + assertThat(WorkbasketMarkedForDeletionException.ERROR_KEY) + .isEqualTo("WORKBASKET_MARKED_FOR_DELETION"); + assertThat(WorkbasketNotFoundException.ERROR_KEY_ID).isEqualTo("WORKBASKET_WITH_ID_NOT_FOUND"); + assertThat(WorkbasketNotFoundException.ERROR_KEY_KEY_DOMAIN) + .isEqualTo("WORKBASKET_WITH_KEY_NOT_FOUND"); + } + + @Test + void should_ProvideConsistentErrorKey_For_RestExceptions() { + assertThat(TaskanaRestExceptionHandler.ERROR_KEY_PAYLOAD).isEqualTo("PAYLOAD_TOO_LARGE"); + assertThat(TaskanaRestExceptionHandler.ERROR_KEY_QUERY_MALFORMED) + .isEqualTo("QUERY_PARAMETER_MALFORMED"); + } +} diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/GeneralExceptionHandlingTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/GeneralExceptionHandlingTest.java index d51a16826..8f4d389d4 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/GeneralExceptionHandlingTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/GeneralExceptionHandlingTest.java @@ -3,6 +3,11 @@ package pro.taskana.common.rest; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static pro.taskana.common.test.rest.RestHelper.TEMPLATE; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -13,23 +18,25 @@ import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.HttpClientErrorException.BadRequest; import pro.taskana.classification.rest.models.ClassificationSummaryPagedRepresentationModel; +import pro.taskana.common.api.exceptions.ErrorCode; +import pro.taskana.common.rest.TaskanaRestExceptionHandler.MalformedQueryParameter; import pro.taskana.common.test.rest.RestHelper; import pro.taskana.common.test.rest.TaskanaSpringBootTest; +import pro.taskana.workbasket.api.WorkbasketPermission; +import pro.taskana.workbasket.api.WorkbasketType; import pro.taskana.workbasket.rest.models.WorkbasketSummaryPagedRepresentationModel; /** Test general Exception Handling. */ @TaskanaSpringBootTest class GeneralExceptionHandlingTest { - private static final ParameterizedTypeReference - WORKBASKET_SUMMARY_PAGE_MODEL_TYPE = - new ParameterizedTypeReference() {}; - private final RestHelper restHelper; + private final ObjectMapper objectMapper; @Autowired - GeneralExceptionHandlingTest(RestHelper restHelper) { + GeneralExceptionHandlingTest(RestHelper restHelper, ObjectMapper objectMapper) { this.restHelper = restHelper; + this.objectMapper = objectMapper; } @Test @@ -38,14 +45,13 @@ class GeneralExceptionHandlingTest { HttpEntity auth = new HttpEntity<>(restHelper.getHeadersTeamlead_1()); ThrowingCallable httpCall = - () -> { - TEMPLATE.exchange( - url, - HttpMethod.DELETE, - auth, - ParameterizedTypeReference.forType( - ClassificationSummaryPagedRepresentationModel.class)); - }; + () -> + TEMPLATE.exchange( + url, + HttpMethod.DELETE, + auth, + ParameterizedTypeReference.forType( + ClassificationSummaryPagedRepresentationModel.class)); assertThatThrownBy(httpCall) .isInstanceOf(HttpClientErrorException.class) @@ -53,31 +59,151 @@ class GeneralExceptionHandlingTest { } @Test - void should_ThrowExpressiveError_When_InvalidEnumValueIsProvided() { - String url = restHelper.toUrl(RestEndpoints.URL_WORKBASKET) + "?type=GROU"; + void should_ThrowExpressiveError_When_AQueryParameterIsInvalid() throws Exception { + String url = restHelper.toUrl(RestEndpoints.URL_WORKBASKET) + "?required-permission=GROU"; HttpEntity auth = new HttpEntity<>(restHelper.getHeadersAdmin()); ThrowingCallable httpCall = - () -> { - TEMPLATE.exchange(url, HttpMethod.GET, auth, WORKBASKET_SUMMARY_PAGE_MODEL_TYPE); - }; + () -> + TEMPLATE.exchange( + url, + HttpMethod.GET, + auth, + ParameterizedTypeReference.forType( + WorkbasketSummaryPagedRepresentationModel.class)); + + List expectedValues = + Arrays.stream(WorkbasketPermission.values()) + .map(Object::toString) + .collect(Collectors.toList()); + ErrorCode errorCode = + ErrorCode.of( + "QUERY_PARAMETER_MALFORMED", + Map.of( + "malformedQueryParameters", + List.of(new MalformedQueryParameter("required-permission", "GROU", expectedValues)) + .toArray(new MalformedQueryParameter[0]))); - // Inside the complete message of the exception, thrown when querying the REST controller with - // a wrong enum value (for example 'GROU' instead of 'GROUP'), there will be found following - // information about this issue: - // - // nested exception is org.springframework.core.convert.ConversionFailedException: Failed to - // convert from type [java.lang.String] to type [pro.taskana.workbasket.api.WorkbasketType] - // for value 'GROU'; nested exception is java.lang.IllegalArgumentException: No enum - // constant pro.taskana.workbasket.api.WorkbasketType.GROU - // - // Unfortunately the message of the exception thrown here is cut and thus there is no info - // about this problem. In case of querying the REST controller directly there will. assertThatThrownBy(httpCall) .isInstanceOf(BadRequest.class) - .hasMessageContaining( - "\"status\":400,\"error\":\"BAD_REQUEST\",\"exception\":\"org.springframework.web." - + "method.annotation.ModelAttributeMethodProcessor$1\",\"message\":\"org." - + "springframework"); + .extracting(BadRequest.class::cast) + .extracting(BadRequest::getResponseBodyAsString) + .asString() + .contains(objectMapper.writeValueAsString(errorCode)); + } + + @Test + void should_CombineErrors_When_SameQueryParameterDeclarationsAreInvalidMultipleTimes() + throws Exception { + String url = restHelper.toUrl(RestEndpoints.URL_WORKBASKET) + "?type=GROU&type=invalid"; + HttpEntity auth = new HttpEntity<>(restHelper.getHeadersAdmin()); + + ThrowingCallable httpCall = + () -> + TEMPLATE.exchange( + url, + HttpMethod.GET, + auth, + ParameterizedTypeReference.forType( + WorkbasketSummaryPagedRepresentationModel.class)); + + List expectedValuesForQueryParameterType = + Arrays.stream(WorkbasketType.values()).map(Object::toString).collect(Collectors.toList()); + ErrorCode errorCode = + ErrorCode.of( + "QUERY_PARAMETER_MALFORMED", + Map.of( + "malformedQueryParameters", + List.of( + new MalformedQueryParameter( + "type", "GROU", expectedValuesForQueryParameterType), + new MalformedQueryParameter( + "type", "invalid", expectedValuesForQueryParameterType)) + .toArray(new MalformedQueryParameter[0]))); + + assertThatThrownBy(httpCall) + .isInstanceOf(BadRequest.class) + .extracting(BadRequest.class::cast) + .extracting(BadRequest::getResponseBodyAsString) + .asString() + .contains(objectMapper.writeValueAsString(errorCode)); + } + + @Test + void should_FilterOutValidQueryParameters_When_OnlySomeQueryParametersDeclarationsAreInvalid() + throws Exception { + String url = restHelper.toUrl(RestEndpoints.URL_WORKBASKET) + "?type=GROUP&type=invalid"; + HttpEntity auth = new HttpEntity<>(restHelper.getHeadersAdmin()); + + ThrowingCallable httpCall = + () -> + TEMPLATE.exchange( + url, + HttpMethod.GET, + auth, + ParameterizedTypeReference.forType( + WorkbasketSummaryPagedRepresentationModel.class)); + + List expectedValuesForQueryParameterType = + Arrays.stream(WorkbasketType.values()).map(Object::toString).collect(Collectors.toList()); + ErrorCode errorCode = + ErrorCode.of( + "QUERY_PARAMETER_MALFORMED", + Map.of( + "malformedQueryParameters", + List.of( + new MalformedQueryParameter( + "type", "invalid", expectedValuesForQueryParameterType)) + .toArray(new MalformedQueryParameter[0]))); + + assertThatThrownBy(httpCall) + .isInstanceOf(BadRequest.class) + .extracting(BadRequest.class::cast) + .extracting(BadRequest::getResponseBodyAsString) + .asString() + .contains(objectMapper.writeValueAsString(errorCode)); + } + + @Test + void should_CombineErrors_When_DifferentQueryParametersAreInvalid() throws Exception { + String url = + restHelper.toUrl(RestEndpoints.URL_WORKBASKET) + "?type=GROU&required-permission=invalid"; + HttpEntity auth = new HttpEntity<>(restHelper.getHeadersAdmin()); + + ThrowingCallable httpCall = + () -> + TEMPLATE.exchange( + url, + HttpMethod.GET, + auth, + ParameterizedTypeReference.forType( + WorkbasketSummaryPagedRepresentationModel.class)); + + List expectedValuesForQueryParameterType = + Arrays.stream(WorkbasketType.values()).map(Object::toString).collect(Collectors.toList()); + List expectedValuesForQueryParameterRequiredPermission = + Arrays.stream(WorkbasketPermission.values()) + .map(Object::toString) + .collect(Collectors.toList()); + ErrorCode errorCode = + ErrorCode.of( + "QUERY_PARAMETER_MALFORMED", + Map.of( + "malformedQueryParameters", + List.of( + new MalformedQueryParameter( + "type", "GROU", expectedValuesForQueryParameterType), + new MalformedQueryParameter( + "required-permission", + "invalid", + expectedValuesForQueryParameterRequiredPermission)) + .toArray(new MalformedQueryParameter[0]))); + + assertThatThrownBy(httpCall) + .isInstanceOf(BadRequest.class) + .extracting(BadRequest.class::cast) + .extracting(BadRequest::getResponseBodyAsString) + .asString() + .contains(objectMapper.writeValueAsString(errorCode)); } } diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskCommentControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskCommentControllerIntTest.java index 984593b54..691d0252b 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskCommentControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskCommentControllerIntTest.java @@ -307,7 +307,7 @@ class TaskCommentControllerIntTest { assertThatThrownBy(httpCall) .isInstanceOf(HttpClientErrorException.class) - .hasMessageContaining("TaskComment creator and current user must match.") + .hasMessageContaining("TASK_COMMENT_CREATOR_MISMATCHED") .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) .isEqualTo(HttpStatus.FORBIDDEN); } diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskControllerIntTest.java index 123524b3c..f7624c088 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/TaskControllerIntTest.java @@ -30,6 +30,7 @@ import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.HttpServerErrorException; import pro.taskana.classification.rest.models.ClassificationSummaryRepresentationModel; import pro.taskana.common.rest.RestEndpoints; @@ -192,8 +193,8 @@ class TaskControllerIntTest { }; assertThatThrownBy(httpCall) - .isInstanceOf(HttpClientErrorException.class) - .hasMessageContaining("400"); + .isInstanceOf(HttpServerErrorException.class) + .hasMessageContaining("500"); } @Test @@ -299,10 +300,9 @@ class TaskControllerIntTest { }; assertThatThrownBy(httpCall) - .isInstanceOf(HttpClientErrorException.class) - .hasMessageContaining("400") - .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) - .isEqualTo(HttpStatus.BAD_REQUEST); + .isInstanceOf(HttpServerErrorException.class) + .extracting(ex -> ((HttpServerErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); String url2 = restHelper.toUrl(RestEndpoints.URL_TASKS) @@ -313,10 +313,9 @@ class TaskControllerIntTest { }; assertThatThrownBy(httpCall2) - .isInstanceOf(HttpClientErrorException.class) - .hasMessageContaining("400") - .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) - .isEqualTo(HttpStatus.BAD_REQUEST); + .isInstanceOf(HttpServerErrorException.class) + .extracting(ex -> ((HttpServerErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); } @Test @@ -378,8 +377,9 @@ class TaskControllerIntTest { }; assertThatThrownBy(httpCall) - .isInstanceOf(HttpClientErrorException.class) - .hasMessageContaining("400"); + .isInstanceOf(HttpServerErrorException.class) + .extracting(ex -> ((HttpServerErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); } @Test @@ -423,8 +423,8 @@ class TaskControllerIntTest { }; assertThatThrownBy(httpCall) - .isInstanceOf(HttpClientErrorException.class) - .hasMessageContaining("400"); + .isInstanceOf(HttpServerErrorException.class) + .hasMessageContaining("500"); } @Test @@ -652,7 +652,7 @@ class TaskControllerIntTest { out.write(taskToCreateJson); out.flush(); out.close(); - assertThat(con.getResponseCode()).isEqualTo(400); + assertThat(con.getResponseCode()).isEqualTo(500); con.disconnect(); final String taskToCreateJson2 = @@ -672,7 +672,7 @@ class TaskControllerIntTest { out.write(taskToCreateJson2); out.flush(); out.close(); - assertThat(con.getResponseCode()).isEqualTo(400); + assertThat(con.getResponseCode()).isEqualTo(500); con.disconnect(); } @@ -785,7 +785,7 @@ class TaskControllerIntTest { assertThatThrownBy(httpCall) .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) - .isEqualTo(HttpStatus.CONFLICT); + .isEqualTo(HttpStatus.BAD_REQUEST); } @Test @@ -842,7 +842,7 @@ class TaskControllerIntTest { assertThatThrownBy(httpCall) .isInstanceOf(HttpClientErrorException.class) - .hasMessageContaining("409"); + .hasMessageContaining("400"); } @Test @@ -881,11 +881,11 @@ class TaskControllerIntTest { }; assertThatThrownBy(httpCall) - .isInstanceOf(HttpClientErrorException.class) + .isInstanceOf(HttpServerErrorException.class) .hasMessageContaining( "Unkown request parameters found: [anotherIllegalParam, illegalParam]") - .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) - .isEqualTo(HttpStatus.BAD_REQUEST); + .extracting(ex -> ((HttpServerErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); } @TestFactory diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketAccessItemControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketAccessItemControllerIntTest.java index 269de6a53..ef7ccf9bc 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketAccessItemControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketAccessItemControllerIntTest.java @@ -21,6 +21,7 @@ import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.HttpServerErrorException; import pro.taskana.common.rest.RestEndpoints; import pro.taskana.common.test.rest.RestHelper; @@ -135,11 +136,11 @@ class WorkbasketAccessItemControllerIntTest { url, HttpMethod.GET, auth, WORKBASKET_ACCESS_ITEM_PAGED_REPRESENTATION_MODEL_TYPE); }; assertThatThrownBy(httpCall) - .isInstanceOf(HttpClientErrorException.class) + .isInstanceOf(HttpServerErrorException.class) .hasMessageContaining( "Unkown request parameters found: [anotherIllegalParam, illegalParam]") - .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) - .isEqualTo(HttpStatus.BAD_REQUEST); + .extracting(ex -> ((HttpServerErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); } @TestFactory diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketControllerIntTest.java index 106569acc..99f095459 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketControllerIntTest.java @@ -18,6 +18,7 @@ import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.HttpServerErrorException; import pro.taskana.common.rest.RestEndpoints; import pro.taskana.common.test.rest.RestHelper; @@ -301,10 +302,10 @@ class WorkbasketControllerIntTest { }; assertThatThrownBy(httpCall) - .isInstanceOf(HttpClientErrorException.class) + .isInstanceOf(HttpServerErrorException.class) .hasMessageContaining( "Unkown request parameters found: [anotherIllegalParam, illegalParam]") - .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) - .isEqualTo(HttpStatus.BAD_REQUEST); + .extracting(ex -> ((HttpServerErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); } } diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketDefinitionControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketDefinitionControllerIntTest.java index 57ccbc105..c79574ada 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketDefinitionControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketDefinitionControllerIntTest.java @@ -195,7 +195,7 @@ class WorkbasketDefinitionControllerIntTest { () -> expectStatusWhenExecutingImportRequestOfWorkbaskets(HttpStatus.BAD_REQUEST, w); assertThatThrownBy(httpCall) .isInstanceOf(HttpClientErrorException.class) - .extracting(e -> (HttpClientErrorException) e) + .extracting(HttpClientErrorException.class::cast) .extracting(HttpStatusCodeException::getStatusCode) .isEqualTo(HttpStatus.BAD_REQUEST); @@ -203,7 +203,7 @@ class WorkbasketDefinitionControllerIntTest { assertThatThrownBy(httpCall) .isInstanceOf(HttpClientErrorException.class) - .extracting(e -> (HttpClientErrorException) e) + .extracting(HttpClientErrorException.class::cast) .extracting(HttpStatusCodeException::getStatusCode) .isEqualTo(HttpStatus.BAD_REQUEST); } @@ -219,7 +219,7 @@ class WorkbasketDefinitionControllerIntTest { () -> expectStatusWhenExecutingImportRequestOfWorkbaskets(HttpStatus.CONFLICT, w, w); assertThatThrownBy(httpCall) .isInstanceOf(HttpClientErrorException.class) - .extracting(e -> (HttpClientErrorException) e) + .extracting(HttpClientErrorException.class::cast) .extracting(HttpStatusCodeException::getStatusCode) .isEqualTo(HttpStatus.CONFLICT); }