From d7e1b5cea90965dad6b117710987dfbe305530ca Mon Sep 17 00:00:00 2001 From: Joerg Heffner <56156750+gitgoodjhe@users.noreply.github.com> Date: Mon, 19 Oct 2020 14:26:31 +0200 Subject: [PATCH] TSK-1403: Add a log message to each SPI operation in case of a thrown Exception --- .../pro/taskana/common/api/TaskanaEngine.java | 1 - .../internal/InternalTaskanaEngine.java | 2 +- .../common/internal/TaskanaEngineImpl.java | 2 +- .../history/internal/HistoryEventManager.java | 64 +++++++++++++++---- .../routing}/internal/TaskRoutingManager.java | 21 ++++-- .../CreateTaskPreprocessorManager.java | 17 ++++- .../java/pro/taskana/ArchitectureTest.java | 1 + 7 files changed, 86 insertions(+), 22 deletions(-) rename lib/taskana-core/src/main/java/pro/taskana/{task => spi/routing}/internal/TaskRoutingManager.java (80%) 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 6d2744e40..44de7ee0a 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 @@ -122,7 +122,6 @@ public interface TaskanaEngine { */ CurrentUserContext getCurrentUserContext(); - /** * Connection management mode. Controls the connection handling of taskana * 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 e97a2be72..5f65344bf 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 @@ -5,8 +5,8 @@ import org.apache.ibatis.session.SqlSession; import pro.taskana.common.api.TaskanaEngine; import pro.taskana.spi.history.internal.HistoryEventManager; +import pro.taskana.spi.routing.internal.TaskRoutingManager; import pro.taskana.spi.task.internal.CreateTaskPreprocessorManager; -import pro.taskana.task.internal.TaskRoutingManager; /** * FOR INTERNAL USE ONLY. 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 56e76a975..fe34fbb87 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 @@ -51,6 +51,7 @@ import pro.taskana.monitor.api.MonitorService; import pro.taskana.monitor.internal.MonitorMapper; import pro.taskana.monitor.internal.MonitorServiceImpl; import pro.taskana.spi.history.internal.HistoryEventManager; +import pro.taskana.spi.routing.internal.TaskRoutingManager; import pro.taskana.spi.task.internal.CreateTaskPreprocessorManager; import pro.taskana.task.api.TaskService; import pro.taskana.task.internal.AttachmentMapper; @@ -58,7 +59,6 @@ import pro.taskana.task.internal.ObjectReferenceMapper; import pro.taskana.task.internal.TaskCommentMapper; import pro.taskana.task.internal.TaskMapper; import pro.taskana.task.internal.TaskQueryMapper; -import pro.taskana.task.internal.TaskRoutingManager; import pro.taskana.task.internal.TaskServiceImpl; import pro.taskana.workbasket.api.WorkbasketService; import pro.taskana.workbasket.internal.DistributionTargetMapper; diff --git a/lib/taskana-core/src/main/java/pro/taskana/spi/history/internal/HistoryEventManager.java b/lib/taskana-core/src/main/java/pro/taskana/spi/history/internal/HistoryEventManager.java index 6c347a906..4f568fc5b 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/spi/history/internal/HistoryEventManager.java +++ b/lib/taskana-core/src/main/java/pro/taskana/spi/history/internal/HistoryEventManager.java @@ -7,8 +7,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; 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.spi.history.api.TaskanaHistory; import pro.taskana.spi.history.api.events.classification.ClassificationHistoryEvent; import pro.taskana.spi.history.api.events.task.TaskHistoryEvent; @@ -18,10 +17,9 @@ import pro.taskana.spi.history.api.events.workbasket.WorkbasketHistoryEvent; public final class HistoryEventManager { private static final Logger LOGGER = LoggerFactory.getLogger(HistoryEventManager.class); - private static final String SENDING_EVENT = "Sending event to history service providers: {}"; private static HistoryEventManager singleton; + private final ServiceLoader serviceLoader; private boolean enabled = false; - private ServiceLoader serviceLoader; private HistoryEventManager(TaskanaEngine taskanaEngine) { serviceLoader = ServiceLoader.load(TaskanaHistory.class); @@ -47,18 +45,55 @@ public final class HistoryEventManager { } public void createEvent(TaskHistoryEvent event) { - LOGGER.debug(SENDING_EVENT, event); - serviceLoader.forEach(historyProvider -> historyProvider.create(event)); + LOGGER.debug("Sending event to history service providers: {}", event); + serviceLoader.forEach( + historyProvider -> { + try { + historyProvider.create(event); + } catch (Exception e) { + LOGGER.error( + String.format( + "Caught an exception while trying to create TaskHistoryEvent in class %s", + historyProvider.getClass().getName()), + e); + throw new SystemException(e.getMessage(), e.getCause()); + } + }); } public void createEvent(WorkbasketHistoryEvent event) { - LOGGER.debug(SENDING_EVENT, event); - serviceLoader.forEach(historyProvider -> historyProvider.create(event)); + LOGGER.debug("Sending event to history service providers: {}", event); + serviceLoader.forEach( + historyProvider -> { + try { + historyProvider.create(event); + } catch (Exception e) { + LOGGER.error( + String.format( + "Caught an exception while trying to create WorkbasketHistoryEvent in class %s", + historyProvider.getClass().getName()), + e); + throw new SystemException(e.getMessage(), e.getCause()); + } + }); } public void createEvent(ClassificationHistoryEvent event) { - LOGGER.debug(SENDING_EVENT, event); - serviceLoader.forEach(historyProvider -> historyProvider.create(event)); + LOGGER.debug("Sending event to history service providers: {}", event); + serviceLoader.forEach( + historyProvider -> { + try { + historyProvider.create(event); + } catch (Exception e) { + LOGGER.error( + String.format( + "Caught an exception while trying to create " + + "ClassificationHistoryEvent in class %s", + historyProvider.getClass().getName()), + e); + throw new SystemException(e.getMessage(), e.getCause()); + } + }); } public void deleteEvents(List taskIds) { @@ -67,8 +102,13 @@ public final class HistoryEventManager { historyProvider -> { try { historyProvider.deleteHistoryEventsByTaskIds(taskIds); - } catch (InvalidArgumentException | NotAuthorizedException e) { - LOGGER.warn("Caught an exception while trying to delete HistoryEvents", e); + } catch (Exception e) { + LOGGER.error( + String.format( + "Caught an exception while trying to delete HistoryEvents in class %s", + historyProvider.getClass().getName()), + e); + throw new SystemException(e.getMessage(), e.getCause()); } }); } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskRoutingManager.java b/lib/taskana-core/src/main/java/pro/taskana/spi/routing/internal/TaskRoutingManager.java similarity index 80% rename from lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskRoutingManager.java rename to lib/taskana-core/src/main/java/pro/taskana/spi/routing/internal/TaskRoutingManager.java index d526965d7..9af6badef 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskRoutingManager.java +++ b/lib/taskana-core/src/main/java/pro/taskana/spi/routing/internal/TaskRoutingManager.java @@ -1,4 +1,4 @@ -package pro.taskana.task.internal; +package pro.taskana.spi.routing.internal; import java.util.ArrayList; import java.util.List; @@ -10,6 +10,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import pro.taskana.common.api.TaskanaEngine; +import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.common.internal.util.LogSanitizer; import pro.taskana.spi.routing.api.TaskRoutingProvider; import pro.taskana.task.api.models.Task; @@ -23,8 +24,8 @@ public final class TaskRoutingManager { private static final Logger LOGGER = LoggerFactory.getLogger(TaskRoutingManager.class); private static TaskRoutingManager singleton; private boolean enabled = false; - private List theTaskRoutingProviders = new ArrayList<>(); - private ServiceLoader serviceLoader; + private final List theTaskRoutingProviders = new ArrayList<>(); + private final ServiceLoader serviceLoader; private TaskRoutingManager(TaskanaEngine taskanaEngine) { serviceLoader = ServiceLoader.load(TaskRoutingProvider.class); @@ -72,7 +73,19 @@ public final class TaskRoutingManager { // collect in a set to see whether different workbasket ids are returned Set workbasketIds = theTaskRoutingProviders.stream() - .map(rtr -> rtr.determineWorkbasketId(task)) + .map( + rtr -> { + try { + return rtr.determineWorkbasketId(task); + } catch (Exception e) { + LOGGER.error( + String.format( + "Caught Exception while trying to determine workbasket in class %s", + rtr.getClass().getName()), + e); + throw new SystemException(e.getMessage(), e.getCause()); + } + }) .filter(Objects::nonNull) .collect(Collectors.toSet()); if (workbasketIds.isEmpty()) { diff --git a/lib/taskana-core/src/main/java/pro/taskana/spi/task/internal/CreateTaskPreprocessorManager.java b/lib/taskana-core/src/main/java/pro/taskana/spi/task/internal/CreateTaskPreprocessorManager.java index f0acbec01..7415ab328 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/spi/task/internal/CreateTaskPreprocessorManager.java +++ b/lib/taskana-core/src/main/java/pro/taskana/spi/task/internal/CreateTaskPreprocessorManager.java @@ -5,6 +5,7 @@ import java.util.ServiceLoader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import pro.taskana.common.api.exceptions.SystemException; import pro.taskana.spi.task.api.CreateTaskPreprocessor; import pro.taskana.task.api.models.Task; @@ -13,7 +14,7 @@ public class CreateTaskPreprocessorManager { private static final Logger LOGGER = LoggerFactory.getLogger(CreateTaskPreprocessorManager.class); private static CreateTaskPreprocessorManager singleton; private boolean enabled = false; - private ServiceLoader serviceLoader; + private final ServiceLoader serviceLoader; private CreateTaskPreprocessorManager() { serviceLoader = ServiceLoader.load(CreateTaskPreprocessor.class); @@ -41,8 +42,18 @@ public class CreateTaskPreprocessorManager { public Task processTaskBeforeCreation(Task taskToProcess) { LOGGER.debug("Sending task to CreateTaskPreprocessor providers: {}", taskToProcess); serviceLoader.forEach( - createTaskPreprocessorProvider -> - createTaskPreprocessorProvider.processTaskBeforeCreation(taskToProcess)); + createTaskPreprocessorProvider -> { + try { + createTaskPreprocessorProvider.processTaskBeforeCreation(taskToProcess); + } catch (Exception e) { + LOGGER.error( + String.format( + "Caught exception while processing task before creation in class %s", + createTaskPreprocessorProvider.getClass().getName()), + e); + throw new SystemException(e.getMessage(), e.getCause()); + } + }); return taskToProcess; } } diff --git a/lib/taskana-core/src/test/java/pro/taskana/ArchitectureTest.java b/lib/taskana-core/src/test/java/pro/taskana/ArchitectureTest.java index ea63633fa..586864848 100644 --- a/lib/taskana-core/src/test/java/pro/taskana/ArchitectureTest.java +++ b/lib/taskana-core/src/test/java/pro/taskana/ArchitectureTest.java @@ -47,6 +47,7 @@ class ArchitectureTest { "pro.taskana.workbasket.api", "pro.taskana.workbasket.internal", "pro.taskana.spi.routing.api", + "pro.taskana.spi.routing.internal", "pro.taskana.spi.task.api", "pro.taskana.spi.task.internal" );