From eb2ab640d90a7a07cefec00a8c768b5835bbfa3d Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Sun, 1 Nov 2026 00:05:24 +0100 Subject: [PATCH 1/9] feat(api): add task handler interceptors with outcome reporting Refs #278 --- .../task/AbstractTaskSubscriptionApiImpl.kt | 13 +- .../impl/task/InterceptingTaskHandler.kt | 67 ++++++ ...nterceptorAwareServiceTaskCompletionApi.kt | 37 ++++ .../impl/task/TaskHandlerOutcomeRecorder.kt | 36 ++++ .../AbstractTaskSubscriptionApiImplTest.kt | 71 ++++++- .../impl/task/InterceptingTaskHandlerTest.kt | 151 ++++++++++++++ ...ceptorAwareServiceTaskCompletionApiTest.kt | 192 ++++++++++++++++++ .../task/TaskHandlerInterceptor.kt | 27 +++ .../task/TaskHandlerInterceptorChain.kt | 15 ++ .../task/TaskHandlerInterceptorContext.kt | 24 +++ .../task/TaskHandlerOutcome.kt | 50 +++++ docs/task-subscription-api.md | 63 ++++++ 12 files changed, 742 insertions(+), 4 deletions(-) create mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandler.kt create mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApi.kt create mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt create mode 100644 api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandlerTest.kt create mode 100644 api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApiTest.kt create mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptor.kt create mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorChain.kt create mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorContext.kt create mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt index f23bc6d5..ba807e76 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt @@ -2,6 +2,7 @@ package dev.bpmcrafters.processengineapi.impl.task import dev.bpmcrafters.processengineapi.Empty import dev.bpmcrafters.processengineapi.task.SubscribeForTaskCmd +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor import dev.bpmcrafters.processengineapi.task.TaskSubscription import dev.bpmcrafters.processengineapi.task.TaskSubscriptionApi import dev.bpmcrafters.processengineapi.task.UnsubscribeFromTaskCmd @@ -14,8 +15,9 @@ private val logger = KotlinLogging.logger {} * Abstract task subscription api implementation, using subscription repository and a list of completion strategies. * @since 0.0.2 */ -abstract class AbstractTaskSubscriptionApiImpl( - private val subscriptionRepository: SubscriptionRepository +abstract class AbstractTaskSubscriptionApiImpl @JvmOverloads constructor( + private val subscriptionRepository: SubscriptionRepository, + private val interceptors: List = emptyList() ) : TaskSubscriptionApi { override fun subscribeForTask(cmd: SubscribeForTaskCmd): CompletableFuture { @@ -23,7 +25,12 @@ abstract class AbstractTaskSubscriptionApiImpl( taskDescriptionKey = cmd.taskDescriptionKey, payloadDescription = cmd.payloadDescription, restrictions = cmd.restrictions, - action = cmd.action, + action = InterceptingTaskHandler( + delegate = cmd.action, + interceptors = interceptors, + taskDescriptionKey = cmd.taskDescriptionKey, + taskType = cmd.taskType + ), taskType = cmd.taskType, termination = cmd.termination ).let { diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandler.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandler.kt new file mode 100644 index 00000000..037a9ffd --- /dev/null +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandler.kt @@ -0,0 +1,67 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.task.TaskHandler +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptorContext +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome +import dev.bpmcrafters.processengineapi.task.TaskInformation +import dev.bpmcrafters.processengineapi.task.TaskType +import io.github.oshai.kotlinlogging.KotlinLogging +import java.util.Collections + +private val logger = KotlinLogging.logger {} + +/** + * Runs [TaskHandlerInterceptor]s in list order around the [delegate]. The thread context class + * loader is the delegate's, because adapters would otherwise derive it from this wrapper. + * + * An interceptor exception after the handler returned is deliberately only logged: the task may be + * finished, so the delivery must not report a failure for it. + * @since 1.8 + */ +internal class InterceptingTaskHandler( + private val delegate: TaskHandler, + private val interceptors: List, + private val taskDescriptionKey: String?, + private val taskType: TaskType +) : TaskHandler { + + override fun accept(taskInformation: TaskInformation, payload: Map) { + val context = TaskHandlerInterceptorContext(taskInformation, Collections.unmodifiableMap(payload), taskDescriptionKey, taskType) + var handlerReturned = false + + fun proceed(index: Int): TaskHandlerOutcome { + TaskHandlerOutcomeRecorder.clearRecordedOutcome() + if (index < interceptors.size) { + interceptors[index].intercept(context) { proceed(index + 1) } + } else { + handlerReturned = false + delegate.accept(taskInformation, payload) + handlerReturned = true + } + return TaskHandlerOutcomeRecorder.recordedOutcome() + } + + withDelegateContextClassLoader { + TaskHandlerOutcomeRecorder.recordingFor(taskInformation.taskId) { + try { + proceed(0) + } catch (e: Exception) { + if (!handlerReturned) throw e + logger.error(e) { "PROCESS-ENGINE-API-016: Interceptor failed after handler of task ${taskInformation.taskId} returned." } + } + } + } + } + + private fun withDelegateContextClassLoader(execution: () -> Unit) { + val thread = Thread.currentThread() + val originalClassLoader = thread.contextClassLoader + thread.contextClassLoader = delegate.javaClass.classLoader + try { + execution() + } finally { + thread.contextClassLoader = originalClassLoader + } + } +} diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApi.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApi.kt new file mode 100644 index 00000000..99fd15e4 --- /dev/null +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApi.kt @@ -0,0 +1,37 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.Empty +import dev.bpmcrafters.processengineapi.task.CompleteTaskByErrorCmd +import dev.bpmcrafters.processengineapi.task.CompleteTaskCmd +import dev.bpmcrafters.processengineapi.task.FailTaskCmd +import dev.bpmcrafters.processengineapi.task.ServiceTaskCompletionApi +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome +import java.util.concurrent.CompletableFuture + +/** + * Decorates a [ServiceTaskCompletionApi] and reports each accepted request of a task handler as + * [TaskHandlerOutcome] to the running [TaskHandlerInterceptor]s, regardless of the returned future. + * + * Reports only calls made in the handler's thread, and nothing if the call throws synchronously. + * @since 1.8 + */ +class InterceptorAwareServiceTaskCompletionApi( + private val delegate: ServiceTaskCompletionApi +) : ServiceTaskCompletionApi { + + override fun completeTask(cmd: CompleteTaskCmd): CompletableFuture = + delegate.completeTask(cmd).also { + TaskHandlerOutcomeRecorder.record(cmd.taskId, TaskHandlerOutcome.Completed) + } + + override fun completeTaskByError(cmd: CompleteTaskByErrorCmd): CompletableFuture = + delegate.completeTaskByError(cmd).also { + TaskHandlerOutcomeRecorder.record(cmd.taskId, TaskHandlerOutcome.CompletedByError(cmd.errorCode, cmd.errorMessage)) + } + + override fun failTask(cmd: FailTaskCmd): CompletableFuture = + delegate.failTask(cmd).also { + TaskHandlerOutcomeRecorder.record(cmd.taskId, TaskHandlerOutcome.Failed(cmd.reason, cmd.errorDetails)) + } +} diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt new file mode 100644 index 00000000..d4c0ffe7 --- /dev/null +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt @@ -0,0 +1,36 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome + +/** + * Keeps the [TaskHandlerOutcome] reported for the task currently handled by the executing thread. + * @since 1.8 + */ +internal object TaskHandlerOutcomeRecorder { + + private class Recording(val taskId: String) { + var outcome: TaskHandlerOutcome = TaskHandlerOutcome.Undetermined + } + + private val currentRecording = ThreadLocal() + + fun recordingFor(taskId: String, execution: () -> T): T { + val enclosingRecording = currentRecording.get() + currentRecording.set(Recording(taskId)) + try { + return execution() + } finally { + if (enclosingRecording == null) currentRecording.remove() else currentRecording.set(enclosingRecording) + } + } + + fun clearRecordedOutcome() { + currentRecording.get()?.outcome = TaskHandlerOutcome.Undetermined + } + + fun record(taskId: String, outcome: TaskHandlerOutcome) { + currentRecording.get()?.takeIf { it.taskId == taskId }?.outcome = outcome + } + + fun recordedOutcome(): TaskHandlerOutcome = currentRecording.get()?.outcome ?: TaskHandlerOutcome.Undetermined +} diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt index 8a9f48e0..15b382f1 100644 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt @@ -4,6 +4,11 @@ import dev.bpmcrafters.processengineapi.CommonRestrictions import dev.bpmcrafters.processengineapi.MetaInfo import dev.bpmcrafters.processengineapi.MetaInfoAware import dev.bpmcrafters.processengineapi.task.SubscribeForTaskCmd +import dev.bpmcrafters.processengineapi.task.TaskHandler +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptorChain +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptorContext +import dev.bpmcrafters.processengineapi.task.TaskInformation import dev.bpmcrafters.processengineapi.task.TaskType import dev.bpmcrafters.processengineapi.task.UnsubscribeFromTaskCmd import org.assertj.core.api.Assertions @@ -13,7 +18,10 @@ internal class AbstractTaskSubscriptionApiImplTest { private val subscriptionRepository: SubscriptionRepository = InMemSubscriptionRepository() private val taskSubscriptionApi = MyTaskSubscriptionApiImpl(subscriptionRepository) - class MyTaskSubscriptionApiImpl(subscriptionRepository: SubscriptionRepository) : AbstractTaskSubscriptionApiImpl(subscriptionRepository) { + class MyTaskSubscriptionApiImpl( + subscriptionRepository: SubscriptionRepository, + interceptors: List = emptyList() + ) : AbstractTaskSubscriptionApiImpl(subscriptionRepository, interceptors) { override fun meta(instance: MetaInfoAware): MetaInfo { TODO("Not yet implemented") @@ -46,4 +54,65 @@ internal class AbstractTaskSubscriptionApiImplTest { Assertions.assertThat(this.subscriptionRepository.getTaskSubscriptions()).isEmpty() } + + @Test + fun `keeps constructor without interceptors for existing adapters`() { + Assertions.assertThat(AbstractTaskSubscriptionApiImpl::class.java.getDeclaredConstructor(SubscriptionRepository::class.java)).isNotNull + } + + @Test + fun `invokes action when no interceptors are configured`() { + var invocations = 0 + val action = TaskHandler { _, _ -> invocations++ } + + val handle = taskSubscriptionApi.subscribeForTask( + SubscribeForTaskCmd( + CommonRestrictions.builder().withProcessDefinitionKey("process-key").build(), + TaskType.EXTERNAL, + null, + setOf("var1"), + action, + { _ -> } + ) + ).get() as TaskSubscriptionHandle + + handle.action.accept(TaskInformation("task-id", emptyMap()), emptyMap()) + + Assertions.assertThat(invocations).isEqualTo(1) + } + + @Test + fun `wraps action so interceptor runs on delivery and unsubscribe still works`() { + val interceptor = TracingInterceptor() + val api = MyTaskSubscriptionApiImpl(subscriptionRepository, listOf(interceptor)) + val action = TaskHandler { _, _ -> interceptor.trace += "action" } + + val handle = api.subscribeForTask( + SubscribeForTaskCmd( + CommonRestrictions.builder().withProcessDefinitionKey("process-key").build(), + TaskType.EXTERNAL, + null, + setOf("var1"), + action, + { _ -> } + ) + ).get() as TaskSubscriptionHandle + + Assertions.assertThat(handle.action).isNotSameAs(action) + handle.action.accept(TaskInformation("task-id", emptyMap()), emptyMap()) + Assertions.assertThat(interceptor.trace).containsExactly("before", "action", "after") + + api.unsubscribe(UnsubscribeFromTaskCmd(handle)).get() + Assertions.assertThat(subscriptionRepository.getTaskSubscriptions()).isEmpty() + } + + private class TracingInterceptor : TaskHandlerInterceptor { + val trace = mutableListOf() + + override fun intercept(context: TaskHandlerInterceptorContext, chain: TaskHandlerInterceptorChain) { + trace += "before" + chain.proceed() + trace += "after" + } + } } diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandlerTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandlerTest.kt new file mode 100644 index 00000000..43fdddd8 --- /dev/null +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandlerTest.kt @@ -0,0 +1,151 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.task.TaskHandler +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptorContext +import dev.bpmcrafters.processengineapi.task.TaskInformation +import dev.bpmcrafters.processengineapi.task.TaskType +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test +import java.net.URLClassLoader + +internal class InterceptingTaskHandlerTest { + + private val taskInformation = TaskInformation("task-id", mapOf("meta-key" to "meta-value")) + private val payload = mapOf("var1" to "value1") + + @Test + fun `propagates exception thrown by interceptor without invoking handler`() { + var handlerInvoked = false + val interceptor = TaskHandlerInterceptor { _, _ -> throw IllegalStateException("boom") } + val handler = InterceptingTaskHandler({ _, _ -> handlerInvoked = true }, listOf(interceptor), "key", TaskType.EXTERNAL) + + assertThatThrownBy { handler.accept(TaskInformation("task-id", emptyMap()), emptyMap()) } + .isInstanceOf(IllegalStateException::class.java) + + assertThat(handlerInvoked).isFalse() + } + + @Test + fun `does not propagate exception thrown by interceptor after handler returned`() { + val interceptor = TaskHandlerInterceptor { _, chain -> + chain.proceed() + throw IllegalStateException("boom") + } + + InterceptingTaskHandler({ _, _ -> }, listOf(interceptor), "key", TaskType.EXTERNAL) + .accept(TaskInformation("task-id", emptyMap()), emptyMap()) + } + + @Test + fun `propagates handler exception of a second attempt`() { + var attempts = 0 + val interceptor = TaskHandlerInterceptor { _, chain -> + chain.proceed() + chain.proceed() + } + val delegate = TaskHandler { _, _ -> if (attempts++ == 1) throw IllegalStateException("boom") } + + assertThatThrownBy { + InterceptingTaskHandler(delegate, listOf(interceptor), "task-key", TaskType.EXTERNAL) + .accept(taskInformation, payload) + }.isInstanceOf(IllegalStateException::class.java).hasMessage("boom") + } + + @Test + fun `exposes payload as unmodifiable view`() { + val interceptor = TaskHandlerInterceptor { context, chain -> + assertThatThrownBy { (context.payload as MutableMap)["key"] = "value" } + .isInstanceOf(UnsupportedOperationException::class.java) + chain.proceed() + } + + InterceptingTaskHandler({ _, _ -> }, listOf(interceptor), "key", TaskType.EXTERNAL) + .accept(TaskInformation("task-id", emptyMap()), mutableMapOf()) + } + + @Test + fun `runs with class loader of handler and restores the original one`() { + val thread = Thread.currentThread() + val originalClassLoader = thread.contextClassLoader + val foreignClassLoader = URLClassLoader(emptyArray()) + thread.contextClassLoader = foreignClassLoader + try { + val action = TaskHandler { _, _ -> } + var seenClassLoader: ClassLoader? = null + val interceptor = TaskHandlerInterceptor { _, chain -> + seenClassLoader = Thread.currentThread().contextClassLoader + chain.proceed() + } + + InterceptingTaskHandler(action, listOf(interceptor), "key", TaskType.EXTERNAL) + .accept(TaskInformation("task-id", emptyMap()), emptyMap()) + + assertThat(seenClassLoader).isSameAs(action.javaClass.classLoader) + assertThat(thread.contextClassLoader).isSameAs(foreignClassLoader) + } finally { + thread.contextClassLoader = originalClassLoader + } + } + + @Test + fun `runs interceptors around delegate in list order`() { + val trace = mutableListOf() + val first = TaskHandlerInterceptor { _, chain -> + trace += "first-before" + chain.proceed().also { trace += "first-after" } + } + val second = TaskHandlerInterceptor { _, chain -> + trace += "second-before" + chain.proceed().also { trace += "second-after" } + } + val delegate = TaskHandler { _, _ -> trace += "delegate" } + + InterceptingTaskHandler(delegate, listOf(first, second), "task-key", TaskType.EXTERNAL) + .accept(taskInformation, payload) + + assertThat(trace).containsExactly( + "first-before", "second-before", "delegate", "second-after", "first-after" + ) + } + + @Test + fun `provides context to interceptors`() { + var captured: TaskHandlerInterceptorContext? = null + val interceptor = TaskHandlerInterceptor { context, chain -> + captured = context + chain.proceed() + } + + InterceptingTaskHandler(TaskHandler { _, _ -> }, listOf(interceptor), "task-key", TaskType.USER) + .accept(taskInformation, payload) + + assertThat(captured).isEqualTo( + TaskHandlerInterceptorContext(taskInformation, payload, "task-key", TaskType.USER) + ) + } + + @Test + fun `interceptor without proceed prevents delegate invocation`() { + var delegateCalled = false + val interceptor = TaskHandlerInterceptor { _, _ -> } + val delegate = TaskHandler { _, _ -> delegateCalled = true } + + InterceptingTaskHandler(delegate, listOf(interceptor), "task-key", TaskType.EXTERNAL) + .accept(taskInformation, payload) + + assertThat(delegateCalled).isFalse() + } + + @Test + fun `delegate exception propagates through the chain`() { + val interceptor = TaskHandlerInterceptor { _, chain -> chain.proceed() } + val delegate = TaskHandler { _, _ -> throw IllegalStateException("boom") } + + assertThatThrownBy { + InterceptingTaskHandler(delegate, listOf(interceptor), "task-key", TaskType.EXTERNAL) + .accept(taskInformation, payload) + }.isInstanceOf(IllegalStateException::class.java).hasMessage("boom") + } +} diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApiTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApiTest.kt new file mode 100644 index 00000000..b199ece3 --- /dev/null +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApiTest.kt @@ -0,0 +1,192 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.Empty +import dev.bpmcrafters.processengineapi.task.CompleteTaskByErrorCmd +import dev.bpmcrafters.processengineapi.task.CompleteTaskCmd +import dev.bpmcrafters.processengineapi.task.FailTaskCmd +import dev.bpmcrafters.processengineapi.task.ServiceTaskCompletionApi +import dev.bpmcrafters.processengineapi.task.TaskHandler +import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome +import dev.bpmcrafters.processengineapi.task.TaskInformation +import dev.bpmcrafters.processengineapi.task.TaskType +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test +import java.util.concurrent.CompletableFuture + +internal class InterceptorAwareServiceTaskCompletionApiTest { + + private val taskInformation = TaskInformation("task-id", emptyMap()) + private val delegatedCommands = mutableListOf() + private val completionApi = InterceptorAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { + override fun completeTask(cmd: CompleteTaskCmd) = delegated(cmd) + override fun completeTaskByError(cmd: CompleteTaskByErrorCmd) = delegated(cmd) + override fun failTask(cmd: FailTaskCmd) = delegated(cmd) + }) + + private fun delegated(cmd: Any): CompletableFuture { + delegatedCommands += cmd + return CompletableFuture.completedFuture(Empty) + } + + private fun outcomeSeenByInterceptorOf(handler: TaskHandler): TaskHandlerOutcome { + var seenOutcome: TaskHandlerOutcome? = null + val interceptor = TaskHandlerInterceptor { _, chain -> chain.proceed().also { seenOutcome = it } } + InterceptingTaskHandler(handler, listOf(interceptor), "task-key", TaskType.EXTERNAL) + .accept(taskInformation, emptyMap()) + return seenOutcome!! + } + + @Test + fun `reports failed task`() { + val cmd = FailTaskCmd("task-id", "reason", "details") + + val outcome = outcomeSeenByInterceptorOf { _, _ -> completionApi.failTask(cmd).get() } + + assertThat(outcome).isEqualTo(TaskHandlerOutcome.Failed("reason", "details")) + assertThat(delegatedCommands).containsExactly(cmd) + } + + @Test + fun `reports task completed by error`() { + val cmd = CompleteTaskByErrorCmd("task-id", "error-code", "message") + + val outcome = outcomeSeenByInterceptorOf { _, _ -> completionApi.completeTaskByError(cmd).get() } + + assertThat(outcome).isEqualTo(TaskHandlerOutcome.CompletedByError("error-code", "message")) + assertThat(delegatedCommands).containsExactly(cmd) + } + + @Test + fun `reports completed task`() { + val cmd = CompleteTaskCmd("task-id") { emptyMap() } + + val outcome = outcomeSeenByInterceptorOf { _, _ -> completionApi.completeTask(cmd).get() } + + assertThat(outcome).isEqualTo(TaskHandlerOutcome.Completed) + assertThat(delegatedCommands).containsExactly(cmd) + } + + @Test + fun `does not report outcome if the engine call throws`() { + val failingApi = InterceptorAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { + override fun completeTask(cmd: CompleteTaskCmd): CompletableFuture = throw IllegalStateException("boom") + override fun completeTaskByError(cmd: CompleteTaskByErrorCmd) = delegated(cmd) + override fun failTask(cmd: FailTaskCmd) = delegated(cmd) + }) + + val outcome = outcomeSeenByInterceptorOf { _, _ -> + runCatching { failingApi.completeTask(CompleteTaskCmd("task-id") { emptyMap() }) } + } + + assertThat(outcome).isEqualTo(TaskHandlerOutcome.Undetermined) + } + + @Test + fun `reports outcome regardless of the result of the returned future`() { + val pendingApi = InterceptorAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { + override fun completeTask(cmd: CompleteTaskCmd) = CompletableFuture() + override fun completeTaskByError(cmd: CompleteTaskByErrorCmd) = delegated(cmd) + override fun failTask(cmd: FailTaskCmd) = delegated(cmd) + }) + + val outcome = outcomeSeenByInterceptorOf { _, _ -> + pendingApi.completeTask(CompleteTaskCmd("task-id") { emptyMap() }) + } + + assertThat(outcome).isEqualTo(TaskHandlerOutcome.Completed) + } + + @Test + fun `keeps outcome of enclosing execution after a nested execution`() { + val outcome = outcomeSeenByInterceptorOf { _, _ -> + completionApi.failTask(FailTaskCmd("task-id", "reason", null)) + outcomeSeenByInterceptorOf { _, _ -> } + } + + assertThat(outcome).isEqualTo(TaskHandlerOutcome.Failed("reason", null)) + } + + @Test + fun `does not report outcome of a previous attempt if interceptor proceeds twice`() { + var attempts = 0 + val retryingInterceptor = TaskHandlerInterceptor { _, chain -> + chain.proceed() + chain.proceed() + } + var seenOutcome: TaskHandlerOutcome? = null + val observingInterceptor = TaskHandlerInterceptor { _, chain -> chain.proceed().also { seenOutcome = it } } + val handler = TaskHandler { _, _ -> + if (attempts++ == 0) completionApi.failTask(FailTaskCmd("task-id", "reason", null)) + } + + InterceptingTaskHandler(handler, listOf(retryingInterceptor, observingInterceptor), "task-key", TaskType.EXTERNAL) + .accept(taskInformation, emptyMap()) + + assertThat(seenOutcome).isEqualTo(TaskHandlerOutcome.Undetermined) + } + + @Test + fun `does not report outcome of a previous attempt if inner interceptor skips the handler`() { + var attempts = 0 + var secondOutcome: TaskHandlerOutcome? = null + val retryingInterceptor = TaskHandlerInterceptor { _, chain -> + chain.proceed() + secondOutcome = chain.proceed() + } + val skippingInterceptor = TaskHandlerInterceptor { _, chain -> if (attempts++ == 0) chain.proceed() } + val handler = TaskHandler { _, _ -> completionApi.failTask(FailTaskCmd("task-id", "reason", null)) } + + InterceptingTaskHandler(handler, listOf(retryingInterceptor, skippingInterceptor), "task-key", TaskType.EXTERNAL) + .accept(taskInformation, emptyMap()) + + assertThat(secondOutcome).isEqualTo(TaskHandlerOutcome.Undetermined) + } + + @Test + fun `reports undetermined outcome if handler does not finish the task`() { + val outcome = outcomeSeenByInterceptorOf { _, _ -> } + + assertThat(outcome).isEqualTo(TaskHandlerOutcome.Undetermined) + } + + @Test + fun `ignores outcome of another task`() { + val outcome = outcomeSeenByInterceptorOf { _, _ -> + completionApi.failTask(FailTaskCmd("other-task-id", "reason", null)).get() + } + + assertThat(outcome).isEqualTo(TaskHandlerOutcome.Undetermined) + } + + @Test + fun `ignores outcome reported by another thread`() { + val outcome = outcomeSeenByInterceptorOf { _, _ -> + CompletableFuture.runAsync { completionApi.failTask(FailTaskCmd("task-id", "reason", null)).get() }.get() + } + + assertThat(outcome).isEqualTo(TaskHandlerOutcome.Undetermined) + } + + @Test + fun `does not leak outcome of a throwing handler into the next execution`() { + assertThatThrownBy { + outcomeSeenByInterceptorOf { _, _ -> + completionApi.failTask(FailTaskCmd("task-id", "reason", null)).get() + throw IllegalStateException("boom") + } + }.isInstanceOf(IllegalStateException::class.java) + + assertThat(TaskHandlerOutcomeRecorder.recordedOutcome()).isEqualTo(TaskHandlerOutcome.Undetermined) + } + + @Test + fun `executes command without a running handler execution`() { + val cmd = FailTaskCmd("task-id", "reason", null) + + completionApi.failTask(cmd).get() + + assertThat(delegatedCommands).containsExactly(cmd) + } +} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptor.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptor.kt new file mode 100644 index 00000000..ca37c9ba --- /dev/null +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptor.kt @@ -0,0 +1,27 @@ +package dev.bpmcrafters.processengineapi.task + +/** + * Adds cross-cutting concerns (logging, tracing, auditing) around every [TaskHandler] execution of + * a subscription, without replacing the delivery layer or using AOP. + * + * Runs in the delivery thread, inside the delivery's try/catch, for every [TaskType]. Filter on + * [TaskHandlerInterceptorContext.taskType] if needed. + * + * @since 1.8 + */ +fun interface TaskHandlerInterceptor { + /** + * Call [TaskHandlerInterceptorChain.proceed] to run the next interceptor and finally the handler. + * Skipping the call skips the handler. + * + * An exception thrown before the handler returned acts like a handler exception and consumes a + * retry, so throw it only to veto the execution. An exception thrown afterwards is deliberately + * only logged and does not affect the task. + * + * A handler may report a failure or BPMN error instead of throwing. `proceed` then returns + * normally, and its [TaskHandlerOutcome] tells what happened. + * @param context context of the intercepted execution. + * @param chain chain to proceed with. + */ + fun intercept(context: TaskHandlerInterceptorContext, chain: TaskHandlerInterceptorChain) +} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorChain.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorChain.kt new file mode 100644 index 00000000..4916d45d --- /dev/null +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorChain.kt @@ -0,0 +1,15 @@ +package dev.bpmcrafters.processengineapi.task + +/** + * Chain to continue an intercepted [TaskHandler] execution. + * @since 1.8 + */ +fun interface TaskHandlerInterceptorChain { + /** + * Proceeds with the next interceptor and finally the wrapped handler. + * Must be called in the interceptor's thread. Each call invokes the handler again, which may + * finish the task twice. + * @return outcome reported by the wrapped handler. + */ + fun proceed(): TaskHandlerOutcome +} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorContext.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorContext.kt new file mode 100644 index 00000000..e219559b --- /dev/null +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorContext.kt @@ -0,0 +1,24 @@ +package dev.bpmcrafters.processengineapi.task + +/** + * Read-only context of an intercepted [TaskHandler] execution. + * @since 1.8 + */ +data class TaskHandlerInterceptorContext( + /** + * Information about the task being delivered. + */ + val taskInformation: TaskInformation, + /** + * Unmodifiable view of the payload variables delivered to the handler. + */ + val payload: Map, + /** + * Task description key of the subscription, if any. + */ + val taskDescriptionKey: String?, + /** + * Type of the intercepted task. + */ + val taskType: TaskType +) diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt new file mode 100644 index 00000000..0943b363 --- /dev/null +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt @@ -0,0 +1,50 @@ +package dev.bpmcrafters.processengineapi.task + +/** + * What a [TaskHandler] requested via the [ServiceTaskCompletionApi] in its executing thread, not + * whether the engine processed it. Requires a completion API wrapped in + * `InterceptorAwareServiceTaskCompletionApi`. + * @since 1.8 + */ +sealed interface TaskHandlerOutcome { + + /** + * The handler completed the task. + */ + data object Completed : TaskHandlerOutcome + + /** + * The handler completed the task by throwing a BPMN error. + */ + data class CompletedByError( + /** + * Error code. + */ + val errorCode: String, + /** + * Optional error details. + */ + val errorMessage: String? + ) : TaskHandlerOutcome + + /** + * The handler failed the task because of a technical failure. + */ + data class Failed( + /** + * Failure reason. + */ + val reason: String, + /** + * Optional failure details. + */ + val errorDetails: String? + ) : TaskHandlerOutcome + + /** + * No outcome was reported in the executing thread: the task is finished later or by another + * thread, is a user task, the completion call threw synchronously, the adapter does not report + * outcomes, or the handler was not invoked. + */ + data object Undetermined : TaskHandlerOutcome +} diff --git a/docs/task-subscription-api.md b/docs/task-subscription-api.md index c333f402..6653d1cf 100644 --- a/docs/task-subscription-api.md +++ b/docs/task-subscription-api.md @@ -52,4 +52,67 @@ public class TaskSubscriber { } +``` + +## Intercepting task handlers + +Cross-cutting concerns such as logging, tracing, or auditing can wrap every task delivery via a +`TaskHandlerInterceptor`. An interceptor wraps the call of the subscription action for both +registration paths (`@ProcessEngineWorker` and manual `subscribeForTask`) and applies to every +`TaskType`. Filter on `context.getTaskType()` if an interceptor should only act on a specific type. + +Interceptors run in the delivery thread, inside the delivery's try/catch. An exception thrown by an +interceptor before the handler returned is treated like a handler exception and leads to failure +handling or a retry by the adapter. An exception thrown after the handler returned is deliberately +only logged, because the task may already be finished. Enclosing interceptors still see such an +exception from `chain.proceed()`. An interceptor that swallows a handler exception makes the delivery +see a success. The context is read-only; interceptors observe the delivery but do not mutate payload +or task information. Calling `chain.proceed()` again invokes the handler again. + +An exception thrown before the handler ran fails the delivery and consumes a retry, although the +handler was not executed. Throw only to veto the execution deliberately. Secure fallible work such +as a synchronous audit call inside the interceptor. + +A handler may report a failure or a BPMN error itself instead of throwing an exception, as the +`process-engine-worker` does. `chain.proceed()` then returns normally, and the returned +`TaskHandlerOutcome` tells what happened: `Completed`, `CompletedByError`, `Failed`, or +`Undetermined`. The outcome is only known if the handler finishes the task in the delivery thread +through a `ServiceTaskCompletionApi` wrapped in `InterceptorAwareServiceTaskCompletionApi`. A task +finished later or by another thread results in `Undetermined`, as does every `USER` task. +`Undetermined` means neither success nor failure, so do not treat it as either. The +outcome reflects the accepted request, not the result of the returned future. A completion call +that throws synchronously reports nothing. + +An adapter passes interceptors to the constructor of `AbstractTaskSubscriptionApiImpl`, in execution +order. Interceptors therefore only take effect with an adapter that supports them. The following +example assumes an adapter starter that collects them as Spring beans and orders them by +`@Order`/`Ordered`. + +```java + +@Component +@Slf4j +@Order(0) +public class LoggingTaskHandlerInterceptor implements TaskHandlerInterceptor { + + @Override + public void intercept(TaskHandlerInterceptorContext context, TaskHandlerInterceptorChain chain) { + log.info("[TASK]: Handling {} of type {}", context.getTaskInformation().getTaskId(), context.getTaskType()); + try { + TaskHandlerOutcome outcome = chain.proceed(); + if (outcome instanceof TaskHandlerOutcome.Failed failed) { + log.error("[TASK]: Failed handling {}: {}", context.getTaskInformation().getTaskId(), failed.getReason()); + } else if (outcome instanceof TaskHandlerOutcome.Undetermined) { + log.info("[TASK]: Handler returned for {}, outcome unknown", context.getTaskInformation().getTaskId()); + } else { + log.info("[TASK]: Handled {}", context.getTaskInformation().getTaskId()); + } + } catch (RuntimeException e) { + log.error("[TASK]: Failed handling {}", context.getTaskInformation().getTaskId(), e); + throw e; + } + } +} + + ``` From 1c86d66335dae5e93a403057020cf50187dd20bb Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 3 Nov 2026 10:32:05 +0100 Subject: [PATCH 2/9] refactor(api): replace task handler interceptors with task handler observers Observers watch the handler via before/after instead of wrapping it, so they cannot veto, repeat or swallow a handler execution. Observer exceptions are only logged; state flows from before to after of the same observer without ThreadLocals. The name avoids confusion with engine listeners such as Camunda ExecutionListener and TaskListener. --- .../task/AbstractTaskSubscriptionApiImpl.kt | 8 +- .../impl/task/InterceptingTaskHandler.kt | 67 ------ ... ObserverAwareServiceTaskCompletionApi.kt} | 8 +- .../impl/task/ObserverAwareTaskHandler.kt | 88 ++++++++ .../impl/task/TaskHandlerOutcomeRecorder.kt | 4 - .../AbstractTaskSubscriptionApiImplTest.kt | 32 +-- .../impl/task/InterceptingTaskHandlerTest.kt | 151 ------------- ...erverAwareServiceTaskCompletionApiTest.kt} | 106 +++++----- .../impl/task/ObserverAwareTaskHandlerTest.kt | 199 ++++++++++++++++++ .../task/TaskHandlerContext.kt | 30 +++ .../task/TaskHandlerInterceptor.kt | 27 --- .../task/TaskHandlerInterceptorChain.kt | 15 -- .../task/TaskHandlerInterceptorContext.kt | 24 --- .../task/TaskHandlerObserver.kt | 38 ++++ .../task/TaskHandlerOutcome.kt | 11 +- docs/task-subscription-api.md | 149 +++++++++---- 16 files changed, 541 insertions(+), 416 deletions(-) delete mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandler.kt rename api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/{InterceptorAwareServiceTaskCompletionApi.kt => ObserverAwareServiceTaskCompletionApi.kt} (79%) create mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandler.kt delete mode 100644 api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandlerTest.kt rename api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/{InterceptorAwareServiceTaskCompletionApiTest.kt => ObserverAwareServiceTaskCompletionApiTest.kt} (60%) create mode 100644 api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandlerTest.kt create mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerContext.kt delete mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptor.kt delete mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorChain.kt delete mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorContext.kt create mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt index ba807e76..4591f436 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt @@ -2,7 +2,7 @@ package dev.bpmcrafters.processengineapi.impl.task import dev.bpmcrafters.processengineapi.Empty import dev.bpmcrafters.processengineapi.task.SubscribeForTaskCmd -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor +import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver import dev.bpmcrafters.processengineapi.task.TaskSubscription import dev.bpmcrafters.processengineapi.task.TaskSubscriptionApi import dev.bpmcrafters.processengineapi.task.UnsubscribeFromTaskCmd @@ -17,7 +17,7 @@ private val logger = KotlinLogging.logger {} */ abstract class AbstractTaskSubscriptionApiImpl @JvmOverloads constructor( private val subscriptionRepository: SubscriptionRepository, - private val interceptors: List = emptyList() + private val observers: List> = emptyList() ) : TaskSubscriptionApi { override fun subscribeForTask(cmd: SubscribeForTaskCmd): CompletableFuture { @@ -25,9 +25,9 @@ abstract class AbstractTaskSubscriptionApiImpl @JvmOverloads constructor( taskDescriptionKey = cmd.taskDescriptionKey, payloadDescription = cmd.payloadDescription, restrictions = cmd.restrictions, - action = InterceptingTaskHandler( + action = ObserverAwareTaskHandler( delegate = cmd.action, - interceptors = interceptors, + observers = observers, taskDescriptionKey = cmd.taskDescriptionKey, taskType = cmd.taskType ), diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandler.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandler.kt deleted file mode 100644 index 037a9ffd..00000000 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandler.kt +++ /dev/null @@ -1,67 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -import dev.bpmcrafters.processengineapi.task.TaskHandler -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptorContext -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome -import dev.bpmcrafters.processengineapi.task.TaskInformation -import dev.bpmcrafters.processengineapi.task.TaskType -import io.github.oshai.kotlinlogging.KotlinLogging -import java.util.Collections - -private val logger = KotlinLogging.logger {} - -/** - * Runs [TaskHandlerInterceptor]s in list order around the [delegate]. The thread context class - * loader is the delegate's, because adapters would otherwise derive it from this wrapper. - * - * An interceptor exception after the handler returned is deliberately only logged: the task may be - * finished, so the delivery must not report a failure for it. - * @since 1.8 - */ -internal class InterceptingTaskHandler( - private val delegate: TaskHandler, - private val interceptors: List, - private val taskDescriptionKey: String?, - private val taskType: TaskType -) : TaskHandler { - - override fun accept(taskInformation: TaskInformation, payload: Map) { - val context = TaskHandlerInterceptorContext(taskInformation, Collections.unmodifiableMap(payload), taskDescriptionKey, taskType) - var handlerReturned = false - - fun proceed(index: Int): TaskHandlerOutcome { - TaskHandlerOutcomeRecorder.clearRecordedOutcome() - if (index < interceptors.size) { - interceptors[index].intercept(context) { proceed(index + 1) } - } else { - handlerReturned = false - delegate.accept(taskInformation, payload) - handlerReturned = true - } - return TaskHandlerOutcomeRecorder.recordedOutcome() - } - - withDelegateContextClassLoader { - TaskHandlerOutcomeRecorder.recordingFor(taskInformation.taskId) { - try { - proceed(0) - } catch (e: Exception) { - if (!handlerReturned) throw e - logger.error(e) { "PROCESS-ENGINE-API-016: Interceptor failed after handler of task ${taskInformation.taskId} returned." } - } - } - } - } - - private fun withDelegateContextClassLoader(execution: () -> Unit) { - val thread = Thread.currentThread() - val originalClassLoader = thread.contextClassLoader - thread.contextClassLoader = delegate.javaClass.classLoader - try { - execution() - } finally { - thread.contextClassLoader = originalClassLoader - } - } -} diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApi.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt similarity index 79% rename from api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApi.kt rename to api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt index 99fd15e4..ad6fb1d6 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApi.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt @@ -5,18 +5,20 @@ import dev.bpmcrafters.processengineapi.task.CompleteTaskByErrorCmd import dev.bpmcrafters.processengineapi.task.CompleteTaskCmd import dev.bpmcrafters.processengineapi.task.FailTaskCmd import dev.bpmcrafters.processengineapi.task.ServiceTaskCompletionApi -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor +import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome import java.util.concurrent.CompletableFuture /** * Decorates a [ServiceTaskCompletionApi] and reports each accepted request of a task handler as - * [TaskHandlerOutcome] to the running [TaskHandlerInterceptor]s, regardless of the returned future. + * [TaskHandlerOutcome] to the running [TaskHandlerObserver]s, regardless of the returned future. * * Reports only calls made in the handler's thread, and nothing if the call throws synchronously. + * Use it together with the observers passed to [AbstractTaskSubscriptionApiImpl]; either one alone + * makes every outcome [TaskHandlerOutcome.Undetermined]. * @since 1.8 */ -class InterceptorAwareServiceTaskCompletionApi( +class ObserverAwareServiceTaskCompletionApi( private val delegate: ServiceTaskCompletionApi ) : ServiceTaskCompletionApi { diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandler.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandler.kt new file mode 100644 index 00000000..20c7ffdd --- /dev/null +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandler.kt @@ -0,0 +1,88 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.task.TaskHandlerContext +import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver +import dev.bpmcrafters.processengineapi.task.TaskHandler +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome +import dev.bpmcrafters.processengineapi.task.TaskInformation +import dev.bpmcrafters.processengineapi.task.TaskType +import io.github.oshai.kotlinlogging.KotlinLogging +import java.util.Collections + +private val logger = KotlinLogging.logger {} + +/** + * Runs the [delegate] exactly once, calling [TaskHandlerObserver.before] in list order before it + * and [TaskHandlerObserver.after] in reverse order after it. Everything an observer throws is only + * logged, so the delivery sees the same result as without observers. Only a [VirtualMachineError] + * propagates, because the JVM cannot be trusted afterwards. The thread context class loader is the + * delegate's, because adapters would otherwise derive it from this wrapper. + * @since 1.8 + */ +internal class ObserverAwareTaskHandler( + private val delegate: TaskHandler, + private val observers: List>, + private val taskDescriptionKey: String?, + private val taskType: TaskType +) : TaskHandler { + + override fun accept(taskInformation: TaskInformation, payload: Map) { + val context = TaskHandlerContext( + taskInformation.copy(meta = Collections.unmodifiableMap(taskInformation.meta)), + Collections.unmodifiableMap(payload), + taskDescriptionKey, + taskType + ) + withDelegateContextClassLoader { + TaskHandlerOutcomeRecorder.recordingFor(taskInformation.taskId) { + val startedObservers = mutableListOf>() + var handlerError: Throwable? = null + try { + observers.forEach { observer -> start(observer, context)?.let(startedObservers::add) } + delegate.accept(taskInformation, payload) + } catch (e: Throwable) { + handlerError = e + throw e + } finally { + val outcome = TaskHandlerOutcomeRecorder.recordedOutcome() + startedObservers.asReversed().forEach { it.finish(context, outcome, handlerError) } + } + } + } + } + + private fun start(observer: TaskHandlerObserver, context: TaskHandlerContext): StartedObserver? = + logFailureOf({ "PROCESS-ENGINE-API-016: Observer failed before handler of task ${context.taskInformation.taskId}." }) { + StartedObserver(observer, observer.before(context)) + } + + private class StartedObserver(private val observer: TaskHandlerObserver, private val state: S) { + fun finish(context: TaskHandlerContext, outcome: TaskHandlerOutcome, handlerError: Throwable?) { + logFailureOf({ "PROCESS-ENGINE-API-017: Observer failed after handler of task ${context.taskInformation.taskId}." }) { + observer.after(context, state, outcome, handlerError) + } + } + } + + private fun withDelegateContextClassLoader(execution: () -> Unit) { + val thread = Thread.currentThread() + val originalClassLoader = thread.contextClassLoader + val delegateClassLoader = delegate.javaClass.classLoader ?: return execution() + thread.contextClassLoader = delegateClassLoader + try { + execution() + } finally { + thread.contextClassLoader = originalClassLoader + } + } +} + +private inline fun logFailureOf(noinline message: () -> String, execution: () -> T): T? = + try { + execution() + } catch (e: VirtualMachineError) { + throw e + } catch (e: Throwable) { + logger.error(e, message) + null + } diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt index d4c0ffe7..d5c4bbdc 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt @@ -24,10 +24,6 @@ internal object TaskHandlerOutcomeRecorder { } } - fun clearRecordedOutcome() { - currentRecording.get()?.outcome = TaskHandlerOutcome.Undetermined - } - fun record(taskId: String, outcome: TaskHandlerOutcome) { currentRecording.get()?.takeIf { it.taskId == taskId }?.outcome = outcome } diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt index 15b382f1..b7b072a2 100644 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt @@ -4,10 +4,10 @@ import dev.bpmcrafters.processengineapi.CommonRestrictions import dev.bpmcrafters.processengineapi.MetaInfo import dev.bpmcrafters.processengineapi.MetaInfoAware import dev.bpmcrafters.processengineapi.task.SubscribeForTaskCmd +import dev.bpmcrafters.processengineapi.task.TaskHandlerContext +import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver import dev.bpmcrafters.processengineapi.task.TaskHandler -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptorChain -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptorContext +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome import dev.bpmcrafters.processengineapi.task.TaskInformation import dev.bpmcrafters.processengineapi.task.TaskType import dev.bpmcrafters.processengineapi.task.UnsubscribeFromTaskCmd @@ -20,8 +20,8 @@ internal class AbstractTaskSubscriptionApiImplTest { private val taskSubscriptionApi = MyTaskSubscriptionApiImpl(subscriptionRepository) class MyTaskSubscriptionApiImpl( subscriptionRepository: SubscriptionRepository, - interceptors: List = emptyList() - ) : AbstractTaskSubscriptionApiImpl(subscriptionRepository, interceptors) { + observers: List> = emptyList() + ) : AbstractTaskSubscriptionApiImpl(subscriptionRepository, observers) { override fun meta(instance: MetaInfoAware): MetaInfo { TODO("Not yet implemented") @@ -56,12 +56,12 @@ internal class AbstractTaskSubscriptionApiImplTest { } @Test - fun `keeps constructor without interceptors for existing adapters`() { + fun `keeps constructor without observers for existing adapters`() { Assertions.assertThat(AbstractTaskSubscriptionApiImpl::class.java.getDeclaredConstructor(SubscriptionRepository::class.java)).isNotNull } @Test - fun `invokes action when no interceptors are configured`() { + fun `invokes action when no observers are configured`() { var invocations = 0 val action = TaskHandler { _, _ -> invocations++ } @@ -82,10 +82,10 @@ internal class AbstractTaskSubscriptionApiImplTest { } @Test - fun `wraps action so interceptor runs on delivery and unsubscribe still works`() { - val interceptor = TracingInterceptor() - val api = MyTaskSubscriptionApiImpl(subscriptionRepository, listOf(interceptor)) - val action = TaskHandler { _, _ -> interceptor.trace += "action" } + fun `wraps action so observer runs on delivery and unsubscribe still works`() { + val observer = TracingObserver() + val api = MyTaskSubscriptionApiImpl(subscriptionRepository, listOf(observer)) + val action = TaskHandler { _, _ -> observer.trace += "action" } val handle = api.subscribeForTask( SubscribeForTaskCmd( @@ -100,18 +100,20 @@ internal class AbstractTaskSubscriptionApiImplTest { Assertions.assertThat(handle.action).isNotSameAs(action) handle.action.accept(TaskInformation("task-id", emptyMap()), emptyMap()) - Assertions.assertThat(interceptor.trace).containsExactly("before", "action", "after") + Assertions.assertThat(observer.trace).containsExactly("before", "action", "after") api.unsubscribe(UnsubscribeFromTaskCmd(handle)).get() Assertions.assertThat(subscriptionRepository.getTaskSubscriptions()).isEmpty() } - private class TracingInterceptor : TaskHandlerInterceptor { + private class TracingObserver : TaskHandlerObserver { val trace = mutableListOf() - override fun intercept(context: TaskHandlerInterceptorContext, chain: TaskHandlerInterceptorChain) { + override fun before(context: TaskHandlerContext) { trace += "before" - chain.proceed() + } + + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { trace += "after" } } diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandlerTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandlerTest.kt deleted file mode 100644 index 43fdddd8..00000000 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptingTaskHandlerTest.kt +++ /dev/null @@ -1,151 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -import dev.bpmcrafters.processengineapi.task.TaskHandler -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptorContext -import dev.bpmcrafters.processengineapi.task.TaskInformation -import dev.bpmcrafters.processengineapi.task.TaskType -import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.Assertions.assertThatThrownBy -import org.junit.jupiter.api.Test -import java.net.URLClassLoader - -internal class InterceptingTaskHandlerTest { - - private val taskInformation = TaskInformation("task-id", mapOf("meta-key" to "meta-value")) - private val payload = mapOf("var1" to "value1") - - @Test - fun `propagates exception thrown by interceptor without invoking handler`() { - var handlerInvoked = false - val interceptor = TaskHandlerInterceptor { _, _ -> throw IllegalStateException("boom") } - val handler = InterceptingTaskHandler({ _, _ -> handlerInvoked = true }, listOf(interceptor), "key", TaskType.EXTERNAL) - - assertThatThrownBy { handler.accept(TaskInformation("task-id", emptyMap()), emptyMap()) } - .isInstanceOf(IllegalStateException::class.java) - - assertThat(handlerInvoked).isFalse() - } - - @Test - fun `does not propagate exception thrown by interceptor after handler returned`() { - val interceptor = TaskHandlerInterceptor { _, chain -> - chain.proceed() - throw IllegalStateException("boom") - } - - InterceptingTaskHandler({ _, _ -> }, listOf(interceptor), "key", TaskType.EXTERNAL) - .accept(TaskInformation("task-id", emptyMap()), emptyMap()) - } - - @Test - fun `propagates handler exception of a second attempt`() { - var attempts = 0 - val interceptor = TaskHandlerInterceptor { _, chain -> - chain.proceed() - chain.proceed() - } - val delegate = TaskHandler { _, _ -> if (attempts++ == 1) throw IllegalStateException("boom") } - - assertThatThrownBy { - InterceptingTaskHandler(delegate, listOf(interceptor), "task-key", TaskType.EXTERNAL) - .accept(taskInformation, payload) - }.isInstanceOf(IllegalStateException::class.java).hasMessage("boom") - } - - @Test - fun `exposes payload as unmodifiable view`() { - val interceptor = TaskHandlerInterceptor { context, chain -> - assertThatThrownBy { (context.payload as MutableMap)["key"] = "value" } - .isInstanceOf(UnsupportedOperationException::class.java) - chain.proceed() - } - - InterceptingTaskHandler({ _, _ -> }, listOf(interceptor), "key", TaskType.EXTERNAL) - .accept(TaskInformation("task-id", emptyMap()), mutableMapOf()) - } - - @Test - fun `runs with class loader of handler and restores the original one`() { - val thread = Thread.currentThread() - val originalClassLoader = thread.contextClassLoader - val foreignClassLoader = URLClassLoader(emptyArray()) - thread.contextClassLoader = foreignClassLoader - try { - val action = TaskHandler { _, _ -> } - var seenClassLoader: ClassLoader? = null - val interceptor = TaskHandlerInterceptor { _, chain -> - seenClassLoader = Thread.currentThread().contextClassLoader - chain.proceed() - } - - InterceptingTaskHandler(action, listOf(interceptor), "key", TaskType.EXTERNAL) - .accept(TaskInformation("task-id", emptyMap()), emptyMap()) - - assertThat(seenClassLoader).isSameAs(action.javaClass.classLoader) - assertThat(thread.contextClassLoader).isSameAs(foreignClassLoader) - } finally { - thread.contextClassLoader = originalClassLoader - } - } - - @Test - fun `runs interceptors around delegate in list order`() { - val trace = mutableListOf() - val first = TaskHandlerInterceptor { _, chain -> - trace += "first-before" - chain.proceed().also { trace += "first-after" } - } - val second = TaskHandlerInterceptor { _, chain -> - trace += "second-before" - chain.proceed().also { trace += "second-after" } - } - val delegate = TaskHandler { _, _ -> trace += "delegate" } - - InterceptingTaskHandler(delegate, listOf(first, second), "task-key", TaskType.EXTERNAL) - .accept(taskInformation, payload) - - assertThat(trace).containsExactly( - "first-before", "second-before", "delegate", "second-after", "first-after" - ) - } - - @Test - fun `provides context to interceptors`() { - var captured: TaskHandlerInterceptorContext? = null - val interceptor = TaskHandlerInterceptor { context, chain -> - captured = context - chain.proceed() - } - - InterceptingTaskHandler(TaskHandler { _, _ -> }, listOf(interceptor), "task-key", TaskType.USER) - .accept(taskInformation, payload) - - assertThat(captured).isEqualTo( - TaskHandlerInterceptorContext(taskInformation, payload, "task-key", TaskType.USER) - ) - } - - @Test - fun `interceptor without proceed prevents delegate invocation`() { - var delegateCalled = false - val interceptor = TaskHandlerInterceptor { _, _ -> } - val delegate = TaskHandler { _, _ -> delegateCalled = true } - - InterceptingTaskHandler(delegate, listOf(interceptor), "task-key", TaskType.EXTERNAL) - .accept(taskInformation, payload) - - assertThat(delegateCalled).isFalse() - } - - @Test - fun `delegate exception propagates through the chain`() { - val interceptor = TaskHandlerInterceptor { _, chain -> chain.proceed() } - val delegate = TaskHandler { _, _ -> throw IllegalStateException("boom") } - - assertThatThrownBy { - InterceptingTaskHandler(delegate, listOf(interceptor), "task-key", TaskType.EXTERNAL) - .accept(taskInformation, payload) - }.isInstanceOf(IllegalStateException::class.java).hasMessage("boom") - } -} diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApiTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt similarity index 60% rename from api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApiTest.kt rename to api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt index b199ece3..303ff8dd 100644 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/InterceptorAwareServiceTaskCompletionApiTest.kt +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt @@ -5,8 +5,9 @@ import dev.bpmcrafters.processengineapi.task.CompleteTaskByErrorCmd import dev.bpmcrafters.processengineapi.task.CompleteTaskCmd import dev.bpmcrafters.processengineapi.task.FailTaskCmd import dev.bpmcrafters.processengineapi.task.ServiceTaskCompletionApi +import dev.bpmcrafters.processengineapi.task.TaskHandlerContext +import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver import dev.bpmcrafters.processengineapi.task.TaskHandler -import dev.bpmcrafters.processengineapi.task.TaskHandlerInterceptor import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome import dev.bpmcrafters.processengineapi.task.TaskInformation import dev.bpmcrafters.processengineapi.task.TaskType @@ -15,11 +16,11 @@ import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.jupiter.api.Test import java.util.concurrent.CompletableFuture -internal class InterceptorAwareServiceTaskCompletionApiTest { +internal class ObserverAwareServiceTaskCompletionApiTest { private val taskInformation = TaskInformation("task-id", emptyMap()) private val delegatedCommands = mutableListOf() - private val completionApi = InterceptorAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { + private val completionApi = ObserverAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { override fun completeTask(cmd: CompleteTaskCmd) = delegated(cmd) override fun completeTaskByError(cmd: CompleteTaskByErrorCmd) = delegated(cmd) override fun failTask(cmd: FailTaskCmd) = delegated(cmd) @@ -30,10 +31,15 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { return CompletableFuture.completedFuture(Empty) } - private fun outcomeSeenByInterceptorOf(handler: TaskHandler): TaskHandlerOutcome { + private fun outcomeSeenByObserverOf(handler: TaskHandler): TaskHandlerOutcome { var seenOutcome: TaskHandlerOutcome? = null - val interceptor = TaskHandlerInterceptor { _, chain -> chain.proceed().also { seenOutcome = it } } - InterceptingTaskHandler(handler, listOf(interceptor), "task-key", TaskType.EXTERNAL) + val observer = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = Unit + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { + seenOutcome = outcome + } + } + ObserverAwareTaskHandler(handler, listOf(observer), "task-key", TaskType.EXTERNAL) .accept(taskInformation, emptyMap()) return seenOutcome!! } @@ -42,7 +48,7 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { fun `reports failed task`() { val cmd = FailTaskCmd("task-id", "reason", "details") - val outcome = outcomeSeenByInterceptorOf { _, _ -> completionApi.failTask(cmd).get() } + val outcome = outcomeSeenByObserverOf { _, _ -> completionApi.failTask(cmd).get() } assertThat(outcome).isEqualTo(TaskHandlerOutcome.Failed("reason", "details")) assertThat(delegatedCommands).containsExactly(cmd) @@ -52,7 +58,7 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { fun `reports task completed by error`() { val cmd = CompleteTaskByErrorCmd("task-id", "error-code", "message") - val outcome = outcomeSeenByInterceptorOf { _, _ -> completionApi.completeTaskByError(cmd).get() } + val outcome = outcomeSeenByObserverOf { _, _ -> completionApi.completeTaskByError(cmd).get() } assertThat(outcome).isEqualTo(TaskHandlerOutcome.CompletedByError("error-code", "message")) assertThat(delegatedCommands).containsExactly(cmd) @@ -62,7 +68,7 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { fun `reports completed task`() { val cmd = CompleteTaskCmd("task-id") { emptyMap() } - val outcome = outcomeSeenByInterceptorOf { _, _ -> completionApi.completeTask(cmd).get() } + val outcome = outcomeSeenByObserverOf { _, _ -> completionApi.completeTask(cmd).get() } assertThat(outcome).isEqualTo(TaskHandlerOutcome.Completed) assertThat(delegatedCommands).containsExactly(cmd) @@ -70,13 +76,13 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { @Test fun `does not report outcome if the engine call throws`() { - val failingApi = InterceptorAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { + val failingApi = ObserverAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { override fun completeTask(cmd: CompleteTaskCmd): CompletableFuture = throw IllegalStateException("boom") override fun completeTaskByError(cmd: CompleteTaskByErrorCmd) = delegated(cmd) override fun failTask(cmd: FailTaskCmd) = delegated(cmd) }) - val outcome = outcomeSeenByInterceptorOf { _, _ -> + val outcome = outcomeSeenByObserverOf { _, _ -> runCatching { failingApi.completeTask(CompleteTaskCmd("task-id") { emptyMap() }) } } @@ -85,13 +91,13 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { @Test fun `reports outcome regardless of the result of the returned future`() { - val pendingApi = InterceptorAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { + val pendingApi = ObserverAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { override fun completeTask(cmd: CompleteTaskCmd) = CompletableFuture() override fun completeTaskByError(cmd: CompleteTaskByErrorCmd) = delegated(cmd) override fun failTask(cmd: FailTaskCmd) = delegated(cmd) }) - val outcome = outcomeSeenByInterceptorOf { _, _ -> + val outcome = outcomeSeenByObserverOf { _, _ -> pendingApi.completeTask(CompleteTaskCmd("task-id") { emptyMap() }) } @@ -99,61 +105,27 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { } @Test - fun `keeps outcome of enclosing execution after a nested execution`() { - val outcome = outcomeSeenByInterceptorOf { _, _ -> + fun `keeps outcomes of nested and enclosing execution apart`() { + var innerOutcome: TaskHandlerOutcome? = null + val outerOutcome = outcomeSeenByObserverOf { _, _ -> completionApi.failTask(FailTaskCmd("task-id", "reason", null)) - outcomeSeenByInterceptorOf { _, _ -> } - } - - assertThat(outcome).isEqualTo(TaskHandlerOutcome.Failed("reason", null)) - } - - @Test - fun `does not report outcome of a previous attempt if interceptor proceeds twice`() { - var attempts = 0 - val retryingInterceptor = TaskHandlerInterceptor { _, chain -> - chain.proceed() - chain.proceed() - } - var seenOutcome: TaskHandlerOutcome? = null - val observingInterceptor = TaskHandlerInterceptor { _, chain -> chain.proceed().also { seenOutcome = it } } - val handler = TaskHandler { _, _ -> - if (attempts++ == 0) completionApi.failTask(FailTaskCmd("task-id", "reason", null)) - } - - InterceptingTaskHandler(handler, listOf(retryingInterceptor, observingInterceptor), "task-key", TaskType.EXTERNAL) - .accept(taskInformation, emptyMap()) - - assertThat(seenOutcome).isEqualTo(TaskHandlerOutcome.Undetermined) - } - - @Test - fun `does not report outcome of a previous attempt if inner interceptor skips the handler`() { - var attempts = 0 - var secondOutcome: TaskHandlerOutcome? = null - val retryingInterceptor = TaskHandlerInterceptor { _, chain -> - chain.proceed() - secondOutcome = chain.proceed() + innerOutcome = outcomeSeenByObserverOf { _, _ -> completionApi.completeTask(CompleteTaskCmd("task-id") { emptyMap() }) } } - val skippingInterceptor = TaskHandlerInterceptor { _, chain -> if (attempts++ == 0) chain.proceed() } - val handler = TaskHandler { _, _ -> completionApi.failTask(FailTaskCmd("task-id", "reason", null)) } - - InterceptingTaskHandler(handler, listOf(retryingInterceptor, skippingInterceptor), "task-key", TaskType.EXTERNAL) - .accept(taskInformation, emptyMap()) - assertThat(secondOutcome).isEqualTo(TaskHandlerOutcome.Undetermined) + assertThat(innerOutcome).isEqualTo(TaskHandlerOutcome.Completed) + assertThat(outerOutcome).isEqualTo(TaskHandlerOutcome.Failed("reason", null)) } @Test fun `reports undetermined outcome if handler does not finish the task`() { - val outcome = outcomeSeenByInterceptorOf { _, _ -> } + val outcome = outcomeSeenByObserverOf { _, _ -> } assertThat(outcome).isEqualTo(TaskHandlerOutcome.Undetermined) } @Test fun `ignores outcome of another task`() { - val outcome = outcomeSeenByInterceptorOf { _, _ -> + val outcome = outcomeSeenByObserverOf { _, _ -> completionApi.failTask(FailTaskCmd("other-task-id", "reason", null)).get() } @@ -162,7 +134,7 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { @Test fun `ignores outcome reported by another thread`() { - val outcome = outcomeSeenByInterceptorOf { _, _ -> + val outcome = outcomeSeenByObserverOf { _, _ -> CompletableFuture.runAsync { completionApi.failTask(FailTaskCmd("task-id", "reason", null)).get() }.get() } @@ -172,7 +144,7 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { @Test fun `does not leak outcome of a throwing handler into the next execution`() { assertThatThrownBy { - outcomeSeenByInterceptorOf { _, _ -> + outcomeSeenByObserverOf { _, _ -> completionApi.failTask(FailTaskCmd("task-id", "reason", null)).get() throw IllegalStateException("boom") } @@ -181,6 +153,26 @@ internal class InterceptorAwareServiceTaskCompletionApiTest { assertThat(TaskHandlerOutcomeRecorder.recordedOutcome()).isEqualTo(TaskHandlerOutcome.Undetermined) } + @Test + fun `reports outcome to observers if handler throws after finishing the task`() { + var seenOutcome: TaskHandlerOutcome? = null + val observer = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = Unit + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { + seenOutcome = outcome + } + } + + assertThatThrownBy { + ObserverAwareTaskHandler({ _, _ -> + completionApi.failTask(FailTaskCmd("task-id", "reason", null)).get() + throw IllegalStateException("boom") + }, listOf(observer), "task-key", TaskType.EXTERNAL).accept(taskInformation, emptyMap()) + }.isInstanceOf(IllegalStateException::class.java) + + assertThat(seenOutcome).isEqualTo(TaskHandlerOutcome.Failed("reason", null)) + } + @Test fun `executes command without a running handler execution`() { val cmd = FailTaskCmd("task-id", "reason", null) diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandlerTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandlerTest.kt new file mode 100644 index 00000000..794eb994 --- /dev/null +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandlerTest.kt @@ -0,0 +1,199 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.task.TaskHandlerContext +import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver +import dev.bpmcrafters.processengineapi.task.TaskHandler +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome +import dev.bpmcrafters.processengineapi.task.TaskInformation +import dev.bpmcrafters.processengineapi.task.TaskType +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test +import java.net.URLClassLoader + +internal class ObserverAwareTaskHandlerTest { + + private val taskInformation = TaskInformation("task-id", mutableMapOf("meta-key" to "meta-value")) + private val payload = mutableMapOf("var1" to "value1") + private val trace = mutableListOf() + + private inner class TracingObserver(private val name: String) : TaskHandlerObserver { + override fun before(context: TaskHandlerContext): String { + trace += "$name-before" + return "$name-state" + } + + override fun after(context: TaskHandlerContext, state: String, outcome: TaskHandlerOutcome, error: Throwable?) { + trace += "$name-after:$state" + } + } + + private fun observed(delegate: TaskHandler, vararg observers: TaskHandlerObserver<*>) = + ObserverAwareTaskHandler(delegate, observers.toList(), "task-key", TaskType.EXTERNAL) + + @Test + fun `runs observers around handler and hands each its own state`() { + observed({ _, _ -> trace += "handler" }, TracingObserver("first"), TracingObserver("second")) + .accept(taskInformation, payload) + + assertThat(trace).containsExactly( + "first-before", "second-before", "handler", "second-after:second-state", "first-after:first-state" + ) + } + + @Test + fun `runs handler and other observers if before throws and skips after of the failed observer`() { + val failing = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = throw IllegalStateException("boom") + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { + trace += "failing-after" + } + } + + observed({ _, _ -> trace += "handler" }, TracingObserver("first"), failing) + .accept(taskInformation, payload) + + assertThat(trace).containsExactly("first-before", "handler", "first-after:first-state") + } + + @Test + fun `does not propagate exception thrown by after and still runs other observers`() { + val failing = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = Unit + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = + throw IllegalStateException("boom") + } + + observed({ _, _ -> trace += "handler" }, TracingObserver("first"), failing) + .accept(taskInformation, payload) + + assertThat(trace).containsExactly("first-before", "handler", "first-after:first-state") + } + + @Test + fun `propagates handler exception unchanged and passes it to observers`() { + var seenError: Throwable? = null + val observing = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = Unit + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { + seenError = error + throw IllegalArgumentException("observer") + } + } + val handlerError = IllegalStateException("handler") + + assertThatThrownBy { observed({ _, _ -> throw handlerError }, observing).accept(taskInformation, payload) } + .isSameAs(handlerError) + assertThat(seenError).isSameAs(handlerError) + } + + @Test + fun `only logs errors thrown by observers and still runs handler and other observers`() { + val failingBefore = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = throw AssertionError("before") + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit + } + val failingAfter = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = Unit + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = + throw NoClassDefFoundError("after") + } + var seenError: Throwable? = Throwable("not called") + val observing = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = Unit + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { + seenError = error + } + } + + observed({ _, _ -> trace += "handler" }, observing, failingBefore, failingAfter).accept(taskInformation, payload) + + assertThat(trace).containsExactly("handler") + assertThat(seenError).isNull() + } + + @Test + fun `propagates virtual machine errors thrown by observers`() { + val failing = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = throw StackOverflowError("before") + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit + } + + assertThatThrownBy { observed({ _, _ -> }, failing).accept(taskInformation, payload) } + .isInstanceOf(StackOverflowError::class.java) + } + + @Test + fun `propagates error thrown by handler unchanged`() { + val handlerError = AssertionError("handler") + + assertThatThrownBy { observed({ _, _ -> throw handlerError }, TracingObserver("first")).accept(taskInformation, payload) } + .isSameAs(handlerError) + assertThat(trace).containsExactly("first-before", "first-after:first-state") + } + + @Test + fun `describes context without payload`() { + val context = TaskHandlerContext(taskInformation, mapOf("secret" to "value"), "task-key", TaskType.EXTERNAL) + + assertThat(context.toString()) + .isEqualTo("TaskHandlerContext(taskId=task-id, taskDescriptionKey=task-key, taskType=EXTERNAL)") + } + + @Test + fun `invokes handler exactly once`() { + var invocations = 0 + + observed({ _, _ -> invocations++ }, TracingObserver("first")).accept(taskInformation, payload) + + assertThat(invocations).isEqualTo(1) + } + + @Test + fun `provides read-only context`() { + var captured: TaskHandlerContext? = null + val capturing = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) { + captured = context + } + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit + } + + ObserverAwareTaskHandler({ _, _ -> }, listOf(capturing), "task-key", TaskType.USER).accept(taskInformation, payload) + + val context = captured!! + assertThat(context.taskInformation).isEqualTo(taskInformation) + assertThat(context.payload).isEqualTo(payload) + assertThat(context.taskDescriptionKey).isEqualTo("task-key") + assertThat(context.taskType).isEqualTo(TaskType.USER) + assertThatThrownBy { (context.payload as MutableMap)["key"] = "value" } + .isInstanceOf(UnsupportedOperationException::class.java) + assertThatThrownBy { (context.taskInformation.meta as MutableMap)["key"] = "value" } + .isInstanceOf(UnsupportedOperationException::class.java) + } + + @Test + fun `runs with class loader of handler and restores the original one`() { + val thread = Thread.currentThread() + val originalClassLoader = thread.contextClassLoader + val foreignClassLoader = URLClassLoader(emptyArray()) + thread.contextClassLoader = foreignClassLoader + try { + val action = TaskHandler { _, _ -> } + var seenClassLoader: ClassLoader? = null + val observing = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) { + seenClassLoader = Thread.currentThread().contextClassLoader + } + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit + } + + observed(action, observing).accept(taskInformation, payload) + + assertThat(seenClassLoader).isSameAs(action.javaClass.classLoader) + assertThat(thread.contextClassLoader).isSameAs(foreignClassLoader) + } finally { + thread.contextClassLoader = originalClassLoader + } + } +} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerContext.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerContext.kt new file mode 100644 index 00000000..8ff61a89 --- /dev/null +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerContext.kt @@ -0,0 +1,30 @@ +package dev.bpmcrafters.processengineapi.task + +/** + * Read-only context of an observed [TaskHandler] execution. The handler receives the original + * arguments, so changing this context never reaches it. The payload is a view, not a copy: values + * inside it are shared with the handler and must not be mutated, and changes the handler makes to a + * mutable payload map are visible in [TaskHandlerObserver.after]. + * @since 1.8 + */ +class TaskHandlerContext( + /** + * Information about the task being delivered, with an unmodifiable view of its meta. + */ + val taskInformation: TaskInformation, + /** + * Unmodifiable view of the payload variables delivered to the handler. + */ + val payload: Map, + /** + * Task description key of the subscription, if any. + */ + val taskDescriptionKey: String?, + /** + * Type of the observed task. + */ + val taskType: TaskType +) { + override fun toString(): String = + "TaskHandlerContext(taskId=${taskInformation.taskId}, taskDescriptionKey=$taskDescriptionKey, taskType=$taskType)" +} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptor.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptor.kt deleted file mode 100644 index ca37c9ba..00000000 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptor.kt +++ /dev/null @@ -1,27 +0,0 @@ -package dev.bpmcrafters.processengineapi.task - -/** - * Adds cross-cutting concerns (logging, tracing, auditing) around every [TaskHandler] execution of - * a subscription, without replacing the delivery layer or using AOP. - * - * Runs in the delivery thread, inside the delivery's try/catch, for every [TaskType]. Filter on - * [TaskHandlerInterceptorContext.taskType] if needed. - * - * @since 1.8 - */ -fun interface TaskHandlerInterceptor { - /** - * Call [TaskHandlerInterceptorChain.proceed] to run the next interceptor and finally the handler. - * Skipping the call skips the handler. - * - * An exception thrown before the handler returned acts like a handler exception and consumes a - * retry, so throw it only to veto the execution. An exception thrown afterwards is deliberately - * only logged and does not affect the task. - * - * A handler may report a failure or BPMN error instead of throwing. `proceed` then returns - * normally, and its [TaskHandlerOutcome] tells what happened. - * @param context context of the intercepted execution. - * @param chain chain to proceed with. - */ - fun intercept(context: TaskHandlerInterceptorContext, chain: TaskHandlerInterceptorChain) -} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorChain.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorChain.kt deleted file mode 100644 index 4916d45d..00000000 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorChain.kt +++ /dev/null @@ -1,15 +0,0 @@ -package dev.bpmcrafters.processengineapi.task - -/** - * Chain to continue an intercepted [TaskHandler] execution. - * @since 1.8 - */ -fun interface TaskHandlerInterceptorChain { - /** - * Proceeds with the next interceptor and finally the wrapped handler. - * Must be called in the interceptor's thread. Each call invokes the handler again, which may - * finish the task twice. - * @return outcome reported by the wrapped handler. - */ - fun proceed(): TaskHandlerOutcome -} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorContext.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorContext.kt deleted file mode 100644 index e219559b..00000000 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerInterceptorContext.kt +++ /dev/null @@ -1,24 +0,0 @@ -package dev.bpmcrafters.processengineapi.task - -/** - * Read-only context of an intercepted [TaskHandler] execution. - * @since 1.8 - */ -data class TaskHandlerInterceptorContext( - /** - * Information about the task being delivered. - */ - val taskInformation: TaskInformation, - /** - * Unmodifiable view of the payload variables delivered to the handler. - */ - val payload: Map, - /** - * Task description key of the subscription, if any. - */ - val taskDescriptionKey: String?, - /** - * Type of the intercepted task. - */ - val taskType: TaskType -) diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt new file mode 100644 index 00000000..5b47029d --- /dev/null +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt @@ -0,0 +1,38 @@ +package dev.bpmcrafters.processengineapi.task + +/** + * Observes every [TaskHandler] execution of a subscription, for cross-cutting concerns such as + * logging, tracing or auditing, without replacing the delivery layer or using AOP. It is not an + * engine listener such as a Camunda `ExecutionListener` or `TaskListener`. + * + * An observer cannot affect the task: the handler always runs exactly once, and everything an + * observer throws is only logged, except a [VirtualMachineError]. Observers run synchronously in the + * delivery thread, for every [TaskType], so keep them cheap and non-blocking. + * + * The outcome is only reported if the adapter also wraps its completion API in + * `ObserverAwareServiceTaskCompletionApi` (module `process-engine-api-impl`); otherwise it is always + * [TaskHandlerOutcome.Undetermined]. + * + * @param S state passed from [before] to [after] of the same execution: [Unit] in Kotlin or `Void` + * (returning `null`) in Java if none is needed. + * @since 1.8 + */ +interface TaskHandlerObserver { + /** + * Called before the handler. Acquire resources all or nothing: if this method throws, [after] + * is not called for this observer. + * @param context context of the execution. + * @return state handed to [after]. + */ + fun before(context: TaskHandlerContext): S + + /** + * Called after the handler, even if it threw. Release the resources acquired in [before] in a + * `finally` block, otherwise they leak into the next execution on this thread. + * @param context context of the execution. + * @param state state returned by [before]. + * @param outcome outcome reported by the handler. + * @param error exception thrown by the handler, if any. It is passed on to the delivery afterwards. + */ + fun after(context: TaskHandlerContext, state: S, outcome: TaskHandlerOutcome, error: Throwable?) +} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt index 0943b363..9c11a512 100644 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt @@ -3,7 +3,8 @@ package dev.bpmcrafters.processengineapi.task /** * What a [TaskHandler] requested via the [ServiceTaskCompletionApi] in its executing thread, not * whether the engine processed it. Requires a completion API wrapped in - * `InterceptorAwareServiceTaskCompletionApi`. + * `ObserverAwareServiceTaskCompletionApi` (module `process-engine-api-impl`). From Java, the + * singletons are `TaskHandlerOutcome.Completed.INSTANCE` and `TaskHandlerOutcome.Undetermined.INSTANCE`. * @since 1.8 */ sealed interface TaskHandlerOutcome { @@ -14,7 +15,7 @@ sealed interface TaskHandlerOutcome { data object Completed : TaskHandlerOutcome /** - * The handler completed the task by throwing a BPMN error. + * The handler completed the task by throwing a BPMN error. Mirrors [CompleteTaskByErrorCmd]. */ data class CompletedByError( /** @@ -28,7 +29,7 @@ sealed interface TaskHandlerOutcome { ) : TaskHandlerOutcome /** - * The handler failed the task because of a technical failure. + * The handler failed the task because of a technical failure. Mirrors [FailTaskCmd]. */ data class Failed( /** @@ -43,8 +44,8 @@ sealed interface TaskHandlerOutcome { /** * No outcome was reported in the executing thread: the task is finished later or by another - * thread, is a user task, the completion call threw synchronously, the adapter does not report - * outcomes, or the handler was not invoked. + * thread, is not finished through a [ServiceTaskCompletionApi] (as user tasks), the completion + * call threw synchronously, or the adapter does not report outcomes. */ data object Undetermined : TaskHandlerOutcome } diff --git a/docs/task-subscription-api.md b/docs/task-subscription-api.md index 6653d1cf..496a257b 100644 --- a/docs/task-subscription-api.md +++ b/docs/task-subscription-api.md @@ -54,65 +54,126 @@ public class TaskSubscriber { ``` -## Intercepting task handlers - -Cross-cutting concerns such as logging, tracing, or auditing can wrap every task delivery via a -`TaskHandlerInterceptor`. An interceptor wraps the call of the subscription action for both -registration paths (`@ProcessEngineWorker` and manual `subscribeForTask`) and applies to every -`TaskType`. Filter on `context.getTaskType()` if an interceptor should only act on a specific type. - -Interceptors run in the delivery thread, inside the delivery's try/catch. An exception thrown by an -interceptor before the handler returned is treated like a handler exception and leads to failure -handling or a retry by the adapter. An exception thrown after the handler returned is deliberately -only logged, because the task may already be finished. Enclosing interceptors still see such an -exception from `chain.proceed()`. An interceptor that swallows a handler exception makes the delivery -see a success. The context is read-only; interceptors observe the delivery but do not mutate payload -or task information. Calling `chain.proceed()` again invokes the handler again. - -An exception thrown before the handler ran fails the delivery and consumes a retry, although the -handler was not executed. Throw only to veto the execution deliberately. Secure fallible work such -as a synchronous audit call inside the interceptor. +## Task handler observers + +Cross-cutting concerns such as logging, tracing, or auditing can observe every task delivery via a +`TaskHandlerObserver`. An observer applies to both registration paths (`@ProcessEngineWorker` and +manual `subscribeForTask`) and to every `TaskType`. Filter on `context.getTaskType()` if an observer +should only act on a specific type. + +An observer watches the handler call of a subscription in the application. It is not an engine +listener such as a Camunda `ExecutionListener` or `TaskListener`, and it is not declared in the BPMN +model. + +The handler always runs exactly once, and `before` is called before it in list order, `after` after +it in reverse order. An observer cannot affect the task: + +- Everything thrown by `before` or `after` is only logged, except a `VirtualMachineError` such as + `OutOfMemoryError`. If `before` throws, `after` is not called for this observer; the handler and + all other observers still run. +- An exception thrown by the handler reaches `after` as `error` and is then passed on to the + delivery unchanged. +- The context is read-only. The handler receives the original arguments, so changing the context + never reaches it. The payload is a view, not a copy: do not mutate values inside it, as they are + shared with the handler, and expect changes the handler makes to a mutable payload map to be + visible in `after`. + +`before` returns a state that the same observer receives in `after`, so no `ThreadLocal` is needed +and no observer can see the state of another. If no state is needed, use `Unit` in Kotlin, or `Void` +in Java and return `null` from `before`. + +Observers run synchronously in the delivery thread. Work in `before` delays the handler and uses up +the lock time of the task, and work in `after` blocks the worker thread. Keep them cheap and +non-blocking, and do not call external systems synchronously. Acquire resources in `before` all or +nothing, and release them in `after` in a `finally` block: a scope or MDC entry left open leaks into +the next task on this thread. A handler may report a failure or a BPMN error itself instead of throwing an exception, as the -`process-engine-worker` does. `chain.proceed()` then returns normally, and the returned -`TaskHandlerOutcome` tells what happened: `Completed`, `CompletedByError`, `Failed`, or -`Undetermined`. The outcome is only known if the handler finishes the task in the delivery thread -through a `ServiceTaskCompletionApi` wrapped in `InterceptorAwareServiceTaskCompletionApi`. A task -finished later or by another thread results in `Undetermined`, as does every `USER` task. -`Undetermined` means neither success nor failure, so do not treat it as either. The -outcome reflects the accepted request, not the result of the returned future. A completion call -that throws synchronously reports nothing. - -An adapter passes interceptors to the constructor of `AbstractTaskSubscriptionApiImpl`, in execution -order. Interceptors therefore only take effect with an adapter that supports them. The following +`process-engine-worker` does. The `TaskHandlerOutcome` passed to `after` tells what happened: +`Completed`, `CompletedByError`, `Failed`, or `Undetermined`. From Java, the singletons are +`TaskHandlerOutcome.Completed.INSTANCE` and `TaskHandlerOutcome.Undetermined.INSTANCE`; match the +others with `instanceof`. The outcome is only known if the handler finishes the task in the delivery +thread through a `ServiceTaskCompletionApi` wrapped in `ObserverAwareServiceTaskCompletionApi` +(module `process-engine-api-impl`). The adapter has to wrap its completion API and pass the +observers; either one alone makes every outcome `Undetermined`. A task finished later, by another +thread, or not through a `ServiceTaskCompletionApi` (as user tasks are) results in `Undetermined`. +`Undetermined` means neither success nor failure, so do not treat it as either. The outcome reflects +the accepted request, not the result of the returned future. A completion call that throws +synchronously results in `Undetermined`. + +An adapter passes observers to the constructor of `AbstractTaskSubscriptionApiImpl`, in execution +order. Observers therefore only take effect with an adapter that supports them. The following example assumes an adapter starter that collects them as Spring beans and orders them by -`@Order`/`Ordered`. +`@Order`/`Ordered`. It runs every task in its own span and puts the activity id into the MDC. ```java +import dev.bpmcrafters.processengineapi.CommonRestrictions; +import dev.bpmcrafters.processengineapi.task.TaskHandlerContext; +import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver; +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Scope; +import lombok.RequiredArgsConstructor; +import org.slf4j.MDC; +import org.springframework.core.annotation.Order; +import org.springframework.stereotype.Component; @Component -@Slf4j @Order(0) -public class LoggingTaskHandlerInterceptor implements TaskHandlerInterceptor { +@RequiredArgsConstructor +public class TracingTaskHandlerObserver implements TaskHandlerObserver { + + public record OpenSpan(Span span, Scope scope, MDC.MDCCloseable mdc) {} + + private final Tracer tracer; @Override - public void intercept(TaskHandlerInterceptorContext context, TaskHandlerInterceptorChain chain) { - log.info("[TASK]: Handling {} of type {}", context.getTaskInformation().getTaskId(), context.getTaskType()); + public OpenSpan before(TaskHandlerContext context) { + String activityId = context.getTaskInformation().getMeta().getOrDefault(CommonRestrictions.ACTIVITY_ID, ""); + Span span = tracer.spanBuilder(context.getTaskDescriptionKey() + ".handle") + .setNoParent() + .setSpanKind(SpanKind.CONSUMER) + .startSpan(); + Scope scope = span.makeCurrent(); + MDC.MDCCloseable mdc = MDC.putCloseable("activityId", activityId); + return new OpenSpan(span, scope, mdc); + } + + @Override + public void after(TaskHandlerContext context, OpenSpan state, TaskHandlerOutcome outcome, Throwable error) { try { - TaskHandlerOutcome outcome = chain.proceed(); - if (outcome instanceof TaskHandlerOutcome.Failed failed) { - log.error("[TASK]: Failed handling {}: {}", context.getTaskInformation().getTaskId(), failed.getReason()); - } else if (outcome instanceof TaskHandlerOutcome.Undetermined) { - log.info("[TASK]: Handler returned for {}, outcome unknown", context.getTaskInformation().getTaskId()); - } else { - log.info("[TASK]: Handled {}", context.getTaskInformation().getTaskId()); + if (error != null) { + state.span().recordException(error).setStatus(StatusCode.ERROR); + } else if (outcome instanceof TaskHandlerOutcome.Failed failed) { + state.span().setStatus(StatusCode.ERROR, failed.getReason()); } - } catch (RuntimeException e) { - log.error("[TASK]: Failed handling {}", context.getTaskInformation().getTaskId(), e); - throw e; + } finally { + state.mdc().close(); + state.scope().close(); + state.span().end(); } } } +``` +An observer without state, for example for logging: +```java +@Component +@Slf4j +public class LoggingTaskHandlerObserver implements TaskHandlerObserver { + + @Override + public Void before(TaskHandlerContext context) { + return null; + } + + @Override + public void after(TaskHandlerContext context, Void state, TaskHandlerOutcome outcome, Throwable error) { + log.info("[TASK]: {} finished with {}", context.getTaskInformation().getTaskId(), outcome); + } +} ``` From c83ad4fe87d85f955a6a9b27ac225a8899344e36 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 3 Nov 2026 11:35:21 +0100 Subject: [PATCH 3/9] refactor(api): let deliveries run task handler observers The subscription decorator only spanned the handler call, so the delivery's failure handling ran after the observers and its logs lost their trace and MDC context. Deliveries now call TaskHandlerObservation around their whole per-task work and report failed or vanished tasks through a TaskDeliveryReport. Only external tasks are observed. --- .../task/AbstractTaskSubscriptionApiImpl.kt | 13 +- .../ObserverAwareServiceTaskCompletionApi.kt | 4 +- .../impl/task/ObserverAwareTaskHandler.kt | 88 ------- .../impl/task/TaskHandlerObservation.kt | 139 +++++++++++ .../AbstractTaskSubscriptionApiImplTest.kt | 73 +----- ...serverAwareServiceTaskCompletionApiTest.kt | 14 +- .../impl/task/ObserverAwareTaskHandlerTest.kt | 199 ---------------- .../impl/task/TaskHandlerObservationTest.kt | 221 ++++++++++++++++++ .../task/TaskHandlerObserver.kt | 26 ++- .../task/TaskHandlerOutcome.kt | 13 +- docs/task-subscription-api.md | 59 +++-- 11 files changed, 445 insertions(+), 404 deletions(-) delete mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandler.kt create mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservation.kt delete mode 100644 api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandlerTest.kt create mode 100644 api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservationTest.kt diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt index 4591f436..f23bc6d5 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImpl.kt @@ -2,7 +2,6 @@ package dev.bpmcrafters.processengineapi.impl.task import dev.bpmcrafters.processengineapi.Empty import dev.bpmcrafters.processengineapi.task.SubscribeForTaskCmd -import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver import dev.bpmcrafters.processengineapi.task.TaskSubscription import dev.bpmcrafters.processengineapi.task.TaskSubscriptionApi import dev.bpmcrafters.processengineapi.task.UnsubscribeFromTaskCmd @@ -15,9 +14,8 @@ private val logger = KotlinLogging.logger {} * Abstract task subscription api implementation, using subscription repository and a list of completion strategies. * @since 0.0.2 */ -abstract class AbstractTaskSubscriptionApiImpl @JvmOverloads constructor( - private val subscriptionRepository: SubscriptionRepository, - private val observers: List> = emptyList() +abstract class AbstractTaskSubscriptionApiImpl( + private val subscriptionRepository: SubscriptionRepository ) : TaskSubscriptionApi { override fun subscribeForTask(cmd: SubscribeForTaskCmd): CompletableFuture { @@ -25,12 +23,7 @@ abstract class AbstractTaskSubscriptionApiImpl @JvmOverloads constructor( taskDescriptionKey = cmd.taskDescriptionKey, payloadDescription = cmd.payloadDescription, restrictions = cmd.restrictions, - action = ObserverAwareTaskHandler( - delegate = cmd.action, - observers = observers, - taskDescriptionKey = cmd.taskDescriptionKey, - taskType = cmd.taskType - ), + action = cmd.action, taskType = cmd.taskType, termination = cmd.termination ).let { diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt index ad6fb1d6..b48c0552 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt @@ -14,8 +14,8 @@ import java.util.concurrent.CompletableFuture * [TaskHandlerOutcome] to the running [TaskHandlerObserver]s, regardless of the returned future. * * Reports only calls made in the handler's thread, and nothing if the call throws synchronously. - * Use it together with the observers passed to [AbstractTaskSubscriptionApiImpl]; either one alone - * makes every outcome [TaskHandlerOutcome.Undetermined]. + * Use it together with a [TaskHandlerObservation] in the deliveries; without it, observers only see + * outcomes the delivery reports. * @since 1.8 */ class ObserverAwareServiceTaskCompletionApi( diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandler.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandler.kt deleted file mode 100644 index 20c7ffdd..00000000 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandler.kt +++ /dev/null @@ -1,88 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -import dev.bpmcrafters.processengineapi.task.TaskHandlerContext -import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver -import dev.bpmcrafters.processengineapi.task.TaskHandler -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome -import dev.bpmcrafters.processengineapi.task.TaskInformation -import dev.bpmcrafters.processengineapi.task.TaskType -import io.github.oshai.kotlinlogging.KotlinLogging -import java.util.Collections - -private val logger = KotlinLogging.logger {} - -/** - * Runs the [delegate] exactly once, calling [TaskHandlerObserver.before] in list order before it - * and [TaskHandlerObserver.after] in reverse order after it. Everything an observer throws is only - * logged, so the delivery sees the same result as without observers. Only a [VirtualMachineError] - * propagates, because the JVM cannot be trusted afterwards. The thread context class loader is the - * delegate's, because adapters would otherwise derive it from this wrapper. - * @since 1.8 - */ -internal class ObserverAwareTaskHandler( - private val delegate: TaskHandler, - private val observers: List>, - private val taskDescriptionKey: String?, - private val taskType: TaskType -) : TaskHandler { - - override fun accept(taskInformation: TaskInformation, payload: Map) { - val context = TaskHandlerContext( - taskInformation.copy(meta = Collections.unmodifiableMap(taskInformation.meta)), - Collections.unmodifiableMap(payload), - taskDescriptionKey, - taskType - ) - withDelegateContextClassLoader { - TaskHandlerOutcomeRecorder.recordingFor(taskInformation.taskId) { - val startedObservers = mutableListOf>() - var handlerError: Throwable? = null - try { - observers.forEach { observer -> start(observer, context)?.let(startedObservers::add) } - delegate.accept(taskInformation, payload) - } catch (e: Throwable) { - handlerError = e - throw e - } finally { - val outcome = TaskHandlerOutcomeRecorder.recordedOutcome() - startedObservers.asReversed().forEach { it.finish(context, outcome, handlerError) } - } - } - } - } - - private fun start(observer: TaskHandlerObserver, context: TaskHandlerContext): StartedObserver? = - logFailureOf({ "PROCESS-ENGINE-API-016: Observer failed before handler of task ${context.taskInformation.taskId}." }) { - StartedObserver(observer, observer.before(context)) - } - - private class StartedObserver(private val observer: TaskHandlerObserver, private val state: S) { - fun finish(context: TaskHandlerContext, outcome: TaskHandlerOutcome, handlerError: Throwable?) { - logFailureOf({ "PROCESS-ENGINE-API-017: Observer failed after handler of task ${context.taskInformation.taskId}." }) { - observer.after(context, state, outcome, handlerError) - } - } - } - - private fun withDelegateContextClassLoader(execution: () -> Unit) { - val thread = Thread.currentThread() - val originalClassLoader = thread.contextClassLoader - val delegateClassLoader = delegate.javaClass.classLoader ?: return execution() - thread.contextClassLoader = delegateClassLoader - try { - execution() - } finally { - thread.contextClassLoader = originalClassLoader - } - } -} - -private inline fun logFailureOf(noinline message: () -> String, execution: () -> T): T? = - try { - execution() - } catch (e: VirtualMachineError) { - throw e - } catch (e: Throwable) { - logger.error(e, message) - null - } diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservation.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservation.kt new file mode 100644 index 00000000..7f654873 --- /dev/null +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservation.kt @@ -0,0 +1,139 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.task.TaskHandlerContext +import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome +import dev.bpmcrafters.processengineapi.task.TaskInformation +import dev.bpmcrafters.processengineapi.task.TaskType +import io.github.oshai.kotlinlogging.KotlinLogging +import java.util.Collections + +private val logger = KotlinLogging.logger {} + +/** + * Runs [TaskHandlerObserver]s around the delivery of one external task. A delivery calls [observe] + * around its whole per-task work, so the handler call and the delivery's own failure handling run + * between [TaskHandlerObserver.before] and [TaskHandlerObserver.after]: logs written while reporting + * a failure still see the context an observer opened. + * + * Observers run in list order before and in reverse order after the delivery. Everything an + * observer throws is only logged, except a [VirtualMachineError], so the delivery behaves the same + * as without observers. Only `EXTERNAL` tasks are observed; for other task types and without + * observers, the delivery just runs. + * @since 1.8 + */ +class TaskHandlerObservation( + private val observers: List> +) { + + /** + * Observes the delivery of a task to [subscription]. + * @param subscription subscription the task is delivered to. + * @param taskInformation task information passed to the handler. + * @param payload payload passed to the handler. + * @param delivery per-task work of the delivery, including its failure handling. It reports what + * the handler could not report itself through the given [TaskDeliveryReport]. + */ + fun observe( + subscription: TaskSubscriptionHandle, + taskInformation: TaskInformation, + payload: Map, + delivery: (TaskDeliveryReport) -> Unit + ) { + if (observers.isEmpty() || subscription.taskType != TaskType.EXTERNAL) { + delivery(TaskDeliveryReport.IGNORED) + return + } + val context = TaskHandlerContext( + taskInformation.copy(meta = Collections.unmodifiableMap(taskInformation.meta)), + Collections.unmodifiableMap(payload), + subscription.taskDescriptionKey, + subscription.taskType + ) + val report = RecordedDeliveryReport() + TaskHandlerOutcomeRecorder.recordingFor(taskInformation.taskId) { + val startedObservers = mutableListOf>() + var deliveryError: Throwable? = null + try { + observers.forEach { observer -> start(observer, context)?.let(startedObservers::add) } + delivery(report) + } catch (e: Throwable) { + deliveryError = e + throw e + } finally { + val outcome = report.outcome ?: TaskHandlerOutcomeRecorder.recordedOutcome() + val error = deliveryError ?: report.error + startedObservers.asReversed().forEach { it.finish(context, outcome, error) } + } + } + } + + private fun start(observer: TaskHandlerObserver, context: TaskHandlerContext): StartedObserver? = + logFailureOf({ "PROCESS-ENGINE-API-016: Observer failed before handler of task ${context.taskInformation.taskId}." }) { + StartedObserver(observer, observer.before(context)) + } + + private class StartedObserver(private val observer: TaskHandlerObserver, private val state: S) { + fun finish(context: TaskHandlerContext, outcome: TaskHandlerOutcome, error: Throwable?) { + logFailureOf({ "PROCESS-ENGINE-API-017: Observer failed after handler of task ${context.taskInformation.taskId}." }) { + observer.after(context, state, outcome, error) + } + } + } + + private class RecordedDeliveryReport : TaskDeliveryReport { + var outcome: TaskHandlerOutcome? = null + var error: Throwable? = null + + override fun failed(error: Throwable) { + this.error = error + if (outcome != TaskHandlerOutcome.Vanished) { + outcome = TaskHandlerOutcome.Failed(error.message ?: error.javaClass.name, null) + } + } + + override fun vanished(error: Throwable) { + this.error = error + outcome = TaskHandlerOutcome.Vanished + } + } +} + +/** + * Lets a delivery report to the [TaskHandlerObserver]s what happened to a task it handles itself, + * without rethrowing. A reported outcome takes precedence over the one the handler reported through + * the completion API, and [vanished] over [failed]. + * @since 1.8 + */ +interface TaskDeliveryReport { + /** + * The delivery caught [error] and reported a failure of the task to the engine. + */ + fun failed(error: Throwable) + + /** + * The task was gone before the delivery could finish it, e.g. because its lock expired. + */ + fun vanished(error: Throwable) + + companion object { + /** + * Report that discards everything, used when no observer runs. + */ + @JvmField + val IGNORED: TaskDeliveryReport = object : TaskDeliveryReport { + override fun failed(error: Throwable) = Unit + override fun vanished(error: Throwable) = Unit + } + } +} + +private inline fun logFailureOf(noinline message: () -> String, execution: () -> T): T? = + try { + execution() + } catch (e: VirtualMachineError) { + throw e + } catch (e: Throwable) { + logger.error(e, message) + null + } diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt index b7b072a2..8a9f48e0 100644 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/AbstractTaskSubscriptionApiImplTest.kt @@ -4,11 +4,6 @@ import dev.bpmcrafters.processengineapi.CommonRestrictions import dev.bpmcrafters.processengineapi.MetaInfo import dev.bpmcrafters.processengineapi.MetaInfoAware import dev.bpmcrafters.processengineapi.task.SubscribeForTaskCmd -import dev.bpmcrafters.processengineapi.task.TaskHandlerContext -import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver -import dev.bpmcrafters.processengineapi.task.TaskHandler -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome -import dev.bpmcrafters.processengineapi.task.TaskInformation import dev.bpmcrafters.processengineapi.task.TaskType import dev.bpmcrafters.processengineapi.task.UnsubscribeFromTaskCmd import org.assertj.core.api.Assertions @@ -18,10 +13,7 @@ internal class AbstractTaskSubscriptionApiImplTest { private val subscriptionRepository: SubscriptionRepository = InMemSubscriptionRepository() private val taskSubscriptionApi = MyTaskSubscriptionApiImpl(subscriptionRepository) - class MyTaskSubscriptionApiImpl( - subscriptionRepository: SubscriptionRepository, - observers: List> = emptyList() - ) : AbstractTaskSubscriptionApiImpl(subscriptionRepository, observers) { + class MyTaskSubscriptionApiImpl(subscriptionRepository: SubscriptionRepository) : AbstractTaskSubscriptionApiImpl(subscriptionRepository) { override fun meta(instance: MetaInfoAware): MetaInfo { TODO("Not yet implemented") @@ -54,67 +46,4 @@ internal class AbstractTaskSubscriptionApiImplTest { Assertions.assertThat(this.subscriptionRepository.getTaskSubscriptions()).isEmpty() } - - @Test - fun `keeps constructor without observers for existing adapters`() { - Assertions.assertThat(AbstractTaskSubscriptionApiImpl::class.java.getDeclaredConstructor(SubscriptionRepository::class.java)).isNotNull - } - - @Test - fun `invokes action when no observers are configured`() { - var invocations = 0 - val action = TaskHandler { _, _ -> invocations++ } - - val handle = taskSubscriptionApi.subscribeForTask( - SubscribeForTaskCmd( - CommonRestrictions.builder().withProcessDefinitionKey("process-key").build(), - TaskType.EXTERNAL, - null, - setOf("var1"), - action, - { _ -> } - ) - ).get() as TaskSubscriptionHandle - - handle.action.accept(TaskInformation("task-id", emptyMap()), emptyMap()) - - Assertions.assertThat(invocations).isEqualTo(1) - } - - @Test - fun `wraps action so observer runs on delivery and unsubscribe still works`() { - val observer = TracingObserver() - val api = MyTaskSubscriptionApiImpl(subscriptionRepository, listOf(observer)) - val action = TaskHandler { _, _ -> observer.trace += "action" } - - val handle = api.subscribeForTask( - SubscribeForTaskCmd( - CommonRestrictions.builder().withProcessDefinitionKey("process-key").build(), - TaskType.EXTERNAL, - null, - setOf("var1"), - action, - { _ -> } - ) - ).get() as TaskSubscriptionHandle - - Assertions.assertThat(handle.action).isNotSameAs(action) - handle.action.accept(TaskInformation("task-id", emptyMap()), emptyMap()) - Assertions.assertThat(observer.trace).containsExactly("before", "action", "after") - - api.unsubscribe(UnsubscribeFromTaskCmd(handle)).get() - Assertions.assertThat(subscriptionRepository.getTaskSubscriptions()).isEmpty() - } - - private class TracingObserver : TaskHandlerObserver { - val trace = mutableListOf() - - override fun before(context: TaskHandlerContext) { - trace += "before" - } - - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { - trace += "after" - } - } } diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt index 303ff8dd..b63f95ab 100644 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt @@ -31,6 +31,13 @@ internal class ObserverAwareServiceTaskCompletionApiTest { return CompletableFuture.completedFuture(Empty) } + private fun observe(handler: TaskHandler, observer: TaskHandlerObserver<*>) { + val subscription = TaskSubscriptionHandle(TaskType.EXTERNAL, null, emptyMap(), "task-key", handler) { } + TaskHandlerObservation(listOf(observer)).observe(subscription, taskInformation, emptyMap()) { + subscription.action.accept(taskInformation, emptyMap()) + } + } + private fun outcomeSeenByObserverOf(handler: TaskHandler): TaskHandlerOutcome { var seenOutcome: TaskHandlerOutcome? = null val observer = object : TaskHandlerObserver { @@ -39,8 +46,7 @@ internal class ObserverAwareServiceTaskCompletionApiTest { seenOutcome = outcome } } - ObserverAwareTaskHandler(handler, listOf(observer), "task-key", TaskType.EXTERNAL) - .accept(taskInformation, emptyMap()) + observe(handler, observer) return seenOutcome!! } @@ -164,10 +170,10 @@ internal class ObserverAwareServiceTaskCompletionApiTest { } assertThatThrownBy { - ObserverAwareTaskHandler({ _, _ -> + observe({ _, _ -> completionApi.failTask(FailTaskCmd("task-id", "reason", null)).get() throw IllegalStateException("boom") - }, listOf(observer), "task-key", TaskType.EXTERNAL).accept(taskInformation, emptyMap()) + }, observer) }.isInstanceOf(IllegalStateException::class.java) assertThat(seenOutcome).isEqualTo(TaskHandlerOutcome.Failed("reason", null)) diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandlerTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandlerTest.kt deleted file mode 100644 index 794eb994..00000000 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareTaskHandlerTest.kt +++ /dev/null @@ -1,199 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -import dev.bpmcrafters.processengineapi.task.TaskHandlerContext -import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver -import dev.bpmcrafters.processengineapi.task.TaskHandler -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome -import dev.bpmcrafters.processengineapi.task.TaskInformation -import dev.bpmcrafters.processengineapi.task.TaskType -import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.Assertions.assertThatThrownBy -import org.junit.jupiter.api.Test -import java.net.URLClassLoader - -internal class ObserverAwareTaskHandlerTest { - - private val taskInformation = TaskInformation("task-id", mutableMapOf("meta-key" to "meta-value")) - private val payload = mutableMapOf("var1" to "value1") - private val trace = mutableListOf() - - private inner class TracingObserver(private val name: String) : TaskHandlerObserver { - override fun before(context: TaskHandlerContext): String { - trace += "$name-before" - return "$name-state" - } - - override fun after(context: TaskHandlerContext, state: String, outcome: TaskHandlerOutcome, error: Throwable?) { - trace += "$name-after:$state" - } - } - - private fun observed(delegate: TaskHandler, vararg observers: TaskHandlerObserver<*>) = - ObserverAwareTaskHandler(delegate, observers.toList(), "task-key", TaskType.EXTERNAL) - - @Test - fun `runs observers around handler and hands each its own state`() { - observed({ _, _ -> trace += "handler" }, TracingObserver("first"), TracingObserver("second")) - .accept(taskInformation, payload) - - assertThat(trace).containsExactly( - "first-before", "second-before", "handler", "second-after:second-state", "first-after:first-state" - ) - } - - @Test - fun `runs handler and other observers if before throws and skips after of the failed observer`() { - val failing = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = throw IllegalStateException("boom") - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { - trace += "failing-after" - } - } - - observed({ _, _ -> trace += "handler" }, TracingObserver("first"), failing) - .accept(taskInformation, payload) - - assertThat(trace).containsExactly("first-before", "handler", "first-after:first-state") - } - - @Test - fun `does not propagate exception thrown by after and still runs other observers`() { - val failing = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = Unit - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = - throw IllegalStateException("boom") - } - - observed({ _, _ -> trace += "handler" }, TracingObserver("first"), failing) - .accept(taskInformation, payload) - - assertThat(trace).containsExactly("first-before", "handler", "first-after:first-state") - } - - @Test - fun `propagates handler exception unchanged and passes it to observers`() { - var seenError: Throwable? = null - val observing = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = Unit - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { - seenError = error - throw IllegalArgumentException("observer") - } - } - val handlerError = IllegalStateException("handler") - - assertThatThrownBy { observed({ _, _ -> throw handlerError }, observing).accept(taskInformation, payload) } - .isSameAs(handlerError) - assertThat(seenError).isSameAs(handlerError) - } - - @Test - fun `only logs errors thrown by observers and still runs handler and other observers`() { - val failingBefore = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = throw AssertionError("before") - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit - } - val failingAfter = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = Unit - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = - throw NoClassDefFoundError("after") - } - var seenError: Throwable? = Throwable("not called") - val observing = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = Unit - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { - seenError = error - } - } - - observed({ _, _ -> trace += "handler" }, observing, failingBefore, failingAfter).accept(taskInformation, payload) - - assertThat(trace).containsExactly("handler") - assertThat(seenError).isNull() - } - - @Test - fun `propagates virtual machine errors thrown by observers`() { - val failing = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = throw StackOverflowError("before") - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit - } - - assertThatThrownBy { observed({ _, _ -> }, failing).accept(taskInformation, payload) } - .isInstanceOf(StackOverflowError::class.java) - } - - @Test - fun `propagates error thrown by handler unchanged`() { - val handlerError = AssertionError("handler") - - assertThatThrownBy { observed({ _, _ -> throw handlerError }, TracingObserver("first")).accept(taskInformation, payload) } - .isSameAs(handlerError) - assertThat(trace).containsExactly("first-before", "first-after:first-state") - } - - @Test - fun `describes context without payload`() { - val context = TaskHandlerContext(taskInformation, mapOf("secret" to "value"), "task-key", TaskType.EXTERNAL) - - assertThat(context.toString()) - .isEqualTo("TaskHandlerContext(taskId=task-id, taskDescriptionKey=task-key, taskType=EXTERNAL)") - } - - @Test - fun `invokes handler exactly once`() { - var invocations = 0 - - observed({ _, _ -> invocations++ }, TracingObserver("first")).accept(taskInformation, payload) - - assertThat(invocations).isEqualTo(1) - } - - @Test - fun `provides read-only context`() { - var captured: TaskHandlerContext? = null - val capturing = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) { - captured = context - } - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit - } - - ObserverAwareTaskHandler({ _, _ -> }, listOf(capturing), "task-key", TaskType.USER).accept(taskInformation, payload) - - val context = captured!! - assertThat(context.taskInformation).isEqualTo(taskInformation) - assertThat(context.payload).isEqualTo(payload) - assertThat(context.taskDescriptionKey).isEqualTo("task-key") - assertThat(context.taskType).isEqualTo(TaskType.USER) - assertThatThrownBy { (context.payload as MutableMap)["key"] = "value" } - .isInstanceOf(UnsupportedOperationException::class.java) - assertThatThrownBy { (context.taskInformation.meta as MutableMap)["key"] = "value" } - .isInstanceOf(UnsupportedOperationException::class.java) - } - - @Test - fun `runs with class loader of handler and restores the original one`() { - val thread = Thread.currentThread() - val originalClassLoader = thread.contextClassLoader - val foreignClassLoader = URLClassLoader(emptyArray()) - thread.contextClassLoader = foreignClassLoader - try { - val action = TaskHandler { _, _ -> } - var seenClassLoader: ClassLoader? = null - val observing = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) { - seenClassLoader = Thread.currentThread().contextClassLoader - } - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit - } - - observed(action, observing).accept(taskInformation, payload) - - assertThat(seenClassLoader).isSameAs(action.javaClass.classLoader) - assertThat(thread.contextClassLoader).isSameAs(foreignClassLoader) - } finally { - thread.contextClassLoader = originalClassLoader - } - } -} diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservationTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservationTest.kt new file mode 100644 index 00000000..0e6b91cd --- /dev/null +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservationTest.kt @@ -0,0 +1,221 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.task.TaskHandlerContext +import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver +import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome +import dev.bpmcrafters.processengineapi.task.TaskInformation +import dev.bpmcrafters.processengineapi.task.TaskType +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test + +internal class TaskHandlerObservationTest { + + private val taskInformation = TaskInformation("task-id", mutableMapOf("meta-key" to "meta-value")) + private val payload = mutableMapOf("var1" to "value1") + private val trace = mutableListOf() + + private inner class TracingObserver(private val name: String) : TaskHandlerObserver { + var seenOutcome: TaskHandlerOutcome? = null + var seenError: Throwable? = null + + override fun before(context: TaskHandlerContext): String { + trace += "$name-before" + return "$name-state" + } + + override fun after(context: TaskHandlerContext, state: String, outcome: TaskHandlerOutcome, error: Throwable?) { + trace += "$name-after:$state" + seenOutcome = outcome + seenError = error + } + } + + private fun throwingBefore(failure: Throwable) = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = throw failure + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { + trace += "failing-after" + } + } + + private fun throwingAfter(failure: Throwable) = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) = Unit + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = throw failure + } + + private fun subscription(taskType: TaskType = TaskType.EXTERNAL) = + TaskSubscriptionHandle(taskType, null, emptyMap(), "task-key", { _, _ -> }, { }) + + private fun observe( + vararg observers: TaskHandlerObserver<*>, + taskType: TaskType = TaskType.EXTERNAL, + delivery: (TaskDeliveryReport) -> Unit = { trace += "delivery" } + ) = TaskHandlerObservation(observers.toList()).observe(subscription(taskType), taskInformation, payload, delivery) + + @Test + fun `runs observers around delivery and hands each its own state`() { + observe(TracingObserver("first"), TracingObserver("second")) + + assertThat(trace).containsExactly( + "first-before", "second-before", "delivery", "second-after:second-state", "first-after:first-state" + ) + } + + @Test + fun `runs delivery failure handling between before and after`() { + val observer = TracingObserver("first") + val handlerError = IllegalStateException("boom") + + observe(observer) { report -> + try { + throw handlerError + } catch (e: Exception) { + trace += "failure-handling" + report.failed(e) + } + } + + assertThat(trace).containsExactly("first-before", "failure-handling", "first-after:first-state") + assertThat(observer.seenOutcome).isEqualTo(TaskHandlerOutcome.Failed("boom", null)) + assertThat(observer.seenError).isSameAs(handlerError) + } + + @Test + fun `reports vanished task and lets it take precedence over failed`() { + val observer = TracingObserver("first") + val notFound = IllegalStateException("not found") + + observe(observer) { report -> + report.vanished(notFound) + report.failed(notFound) + } + + assertThat(observer.seenOutcome).isEqualTo(TaskHandlerOutcome.Vanished) + assertThat(observer.seenError).isSameAs(notFound) + } + + @Test + fun `lets reported failure take precedence over outcome recorded by the handler`() { + val observer = TracingObserver("first") + + observe(observer) { report -> + TaskHandlerOutcomeRecorder.record("task-id", TaskHandlerOutcome.Completed) + report.failed(IllegalStateException("not found")) + } + + assertThat(observer.seenOutcome).isEqualTo(TaskHandlerOutcome.Failed("not found", null)) + } + + @Test + fun `passes outcome recorded by the handler if delivery reports nothing`() { + val observer = TracingObserver("first") + + observe(observer) { TaskHandlerOutcomeRecorder.record("task-id", TaskHandlerOutcome.Completed) } + + assertThat(observer.seenOutcome).isEqualTo(TaskHandlerOutcome.Completed) + assertThat(observer.seenError).isNull() + } + + @Test + fun `propagates exception thrown by delivery unchanged and passes it to observers`() { + val observer = TracingObserver("first") + val deliveryError = IllegalStateException("delivery") + + assertThatThrownBy { observe(observer, throwingAfter(IllegalArgumentException("observer"))) { throw deliveryError } } + .isSameAs(deliveryError) + assertThat(observer.seenError).isSameAs(deliveryError) + } + + @Test + fun `propagates error thrown by delivery unchanged`() { + val deliveryError = AssertionError("delivery") + + assertThatThrownBy { observe(TracingObserver("first")) { throw deliveryError } }.isSameAs(deliveryError) + assertThat(trace).containsExactly("first-before", "first-after:first-state") + } + + @Test + fun `runs delivery and other observers if before throws and skips after of the failed observer`() { + observe(TracingObserver("first"), throwingBefore(IllegalStateException("boom"))) + + assertThat(trace).containsExactly("first-before", "delivery", "first-after:first-state") + } + + @Test + fun `does not propagate exception thrown by after and still runs other observers`() { + observe(TracingObserver("first"), throwingAfter(IllegalStateException("boom"))) + + assertThat(trace).containsExactly("first-before", "delivery", "first-after:first-state") + } + + @Test + fun `only logs errors thrown by observers`() { + val observer = TracingObserver("first") + + observe(observer, throwingBefore(AssertionError("before")), throwingAfter(NoClassDefFoundError("after"))) + + assertThat(trace).containsExactly("first-before", "delivery", "first-after:first-state") + assertThat(observer.seenError).isNull() + } + + @Test + fun `propagates virtual machine errors thrown by observers`() { + assertThatThrownBy { observe(throwingBefore(StackOverflowError("before"))) } + .isInstanceOf(StackOverflowError::class.java) + } + + @Test + fun `runs delivery without observers for other task types`() { + observe(TracingObserver("first"), taskType = TaskType.USER) + + assertThat(trace).containsExactly("delivery") + } + + @Test + fun `runs delivery without observers`() { + observe() + + assertThat(trace).containsExactly("delivery") + } + + @Test + fun `invokes delivery exactly once`() { + var invocations = 0 + + observe(TracingObserver("first")) { invocations++ } + + assertThat(invocations).isEqualTo(1) + } + + @Test + fun `provides read-only context`() { + var captured: TaskHandlerContext? = null + val capturing = object : TaskHandlerObserver { + override fun before(context: TaskHandlerContext) { + captured = context + } + override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit + } + + observe(capturing) + + val context = captured!! + assertThat(context.taskInformation).isEqualTo(taskInformation) + assertThat(context.payload).isEqualTo(payload) + assertThat(context.taskDescriptionKey).isEqualTo("task-key") + assertThat(context.taskType).isEqualTo(TaskType.EXTERNAL) + assertThatThrownBy { (context.payload as MutableMap)["key"] = "value" } + .isInstanceOf(UnsupportedOperationException::class.java) + assertThatThrownBy { (context.taskInformation.meta as MutableMap)["key"] = "value" } + .isInstanceOf(UnsupportedOperationException::class.java) + } + + @Test + fun `describes context without payload`() { + val context = TaskHandlerContext(taskInformation, mapOf("secret" to "value"), "task-key", TaskType.EXTERNAL) + + assertThat(context.toString()) + .isEqualTo("TaskHandlerContext(taskId=task-id, taskDescriptionKey=task-key, taskType=EXTERNAL)") + } + +} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt index 5b47029d..de28d5fd 100644 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt @@ -1,17 +1,22 @@ package dev.bpmcrafters.processengineapi.task /** - * Observes every [TaskHandler] execution of a subscription, for cross-cutting concerns such as - * logging, tracing or auditing, without replacing the delivery layer or using AOP. It is not an - * engine listener such as a Camunda `ExecutionListener` or `TaskListener`. + * Observes the delivery of external service tasks to a [TaskHandler], for cross-cutting concerns + * such as logging, tracing or auditing, without replacing the delivery layer or using AOP. It is not + * an engine listener such as a Camunda `ExecutionListener` or `TaskListener`. + * + * The delivery runs observers around its whole per-task work: [before] runs before the handler, + * [after] once the delivery has finished, including its failure handling. Only `EXTERNAL` tasks are + * observed. * * An observer cannot affect the task: the handler always runs exactly once, and everything an * observer throws is only logged, except a [VirtualMachineError]. Observers run synchronously in the - * delivery thread, for every [TaskType], so keep them cheap and non-blocking. + * delivery thread, so keep them cheap and non-blocking. * - * The outcome is only reported if the adapter also wraps its completion API in - * `ObserverAwareServiceTaskCompletionApi` (module `process-engine-api-impl`); otherwise it is always - * [TaskHandlerOutcome.Undetermined]. + * Observers only run with an adapter whose deliveries use `TaskHandlerObservation` (module + * `process-engine-api-impl`). The outcome of the handler is only known if the adapter also wraps + * its completion API in `ObserverAwareServiceTaskCompletionApi`; otherwise it is + * [TaskHandlerOutcome.Undetermined] unless the delivery reports one. * * @param S state passed from [before] to [after] of the same execution: [Unit] in Kotlin or `Void` * (returning `null`) in Java if none is needed. @@ -27,12 +32,13 @@ interface TaskHandlerObserver { fun before(context: TaskHandlerContext): S /** - * Called after the handler, even if it threw. Release the resources acquired in [before] in a + * Called after the delivery has finished, even if the handler threw. Release the resources acquired in [before] in a * `finally` block, otherwise they leak into the next execution on this thread. * @param context context of the execution. * @param state state returned by [before]. - * @param outcome outcome reported by the handler. - * @param error exception thrown by the handler, if any. It is passed on to the delivery afterwards. + * @param outcome outcome reported by the handler or the delivery. + * @param error exception thrown by the handler or the delivery, if any, also if the delivery + * handled it. */ fun after(context: TaskHandlerContext, state: S, outcome: TaskHandlerOutcome, error: Throwable?) } diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt index 9c11a512..b7cbc9d3 100644 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt @@ -1,10 +1,11 @@ package dev.bpmcrafters.processengineapi.task /** - * What a [TaskHandler] requested via the [ServiceTaskCompletionApi] in its executing thread, not - * whether the engine processed it. Requires a completion API wrapped in + * What a [TaskHandler] requested via the [ServiceTaskCompletionApi] in its executing thread, or + * what the delivery reported for it, not whether the engine processed it. Requires a completion API wrapped in * `ObserverAwareServiceTaskCompletionApi` (module `process-engine-api-impl`). From Java, the - * singletons are `TaskHandlerOutcome.Completed.INSTANCE` and `TaskHandlerOutcome.Undetermined.INSTANCE`. + * singletons are `TaskHandlerOutcome.Completed.INSTANCE`, `TaskHandlerOutcome.Vanished.INSTANCE` and + * `TaskHandlerOutcome.Undetermined.INSTANCE`. * @since 1.8 */ sealed interface TaskHandlerOutcome { @@ -42,6 +43,12 @@ sealed interface TaskHandlerOutcome { val errorDetails: String? ) : TaskHandlerOutcome + /** + * The task was gone before the delivery could finish it, e.g. because its lock expired and another + * worker took it over. Reported by the delivery; not a failure of the handler. + */ + data object Vanished : TaskHandlerOutcome + /** * No outcome was reported in the executing thread: the task is finished later or by another * thread, is not finished through a [ServiceTaskCompletionApi] (as user tasks), the completion diff --git a/docs/task-subscription-api.md b/docs/task-subscription-api.md index 496a257b..3644a924 100644 --- a/docs/task-subscription-api.md +++ b/docs/task-subscription-api.md @@ -56,23 +56,24 @@ public class TaskSubscriber { ## Task handler observers -Cross-cutting concerns such as logging, tracing, or auditing can observe every task delivery via a -`TaskHandlerObserver`. An observer applies to both registration paths (`@ProcessEngineWorker` and -manual `subscribeForTask`) and to every `TaskType`. Filter on `context.getTaskType()` if an observer -should only act on a specific type. +Cross-cutting concerns such as logging, tracing, or auditing can observe the delivery of external +service tasks via a `TaskHandlerObserver`. An observer applies to both registration paths +(`@ProcessEngineWorker` and manual `subscribeForTask`). Only `EXTERNAL` tasks are observed. An observer watches the handler call of a subscription in the application. It is not an engine listener such as a Camunda `ExecutionListener` or `TaskListener`, and it is not declared in the BPMN model. -The handler always runs exactly once, and `before` is called before it in list order, `after` after -it in reverse order. An observer cannot affect the task: +The delivery runs the observers around its whole per-task work: `before` in list order before the +handler, `after` in reverse order once the delivery has finished, including its failure handling. +Logs the delivery writes while reporting a failure therefore still see the trace or MDC context an +observer opened. The handler always runs exactly once. An observer cannot affect the task: - Everything thrown by `before` or `after` is only logged, except a `VirtualMachineError` such as `OutOfMemoryError`. If `before` throws, `after` is not called for this observer; the handler and all other observers still run. -- An exception thrown by the handler reaches `after` as `error` and is then passed on to the - delivery unchanged. +- An exception thrown by the handler reaches `after` as `error`, also if the delivery catches it and + reports a failure to the engine. The delivery handles it as without observers. - The context is read-only. The handler receives the original arguments, so changing the context never reaches it. The payload is a view, not a copy: do not mutate values inside it, as they are shared with the handler, and expect changes the handler makes to a mutable payload map to be @@ -90,21 +91,22 @@ the next task on this thread. A handler may report a failure or a BPMN error itself instead of throwing an exception, as the `process-engine-worker` does. The `TaskHandlerOutcome` passed to `after` tells what happened: -`Completed`, `CompletedByError`, `Failed`, or `Undetermined`. From Java, the singletons are -`TaskHandlerOutcome.Completed.INSTANCE` and `TaskHandlerOutcome.Undetermined.INSTANCE`; match the +`Completed`, `CompletedByError`, `Failed`, `Vanished`, or `Undetermined`. `Failed` is also reported +by the delivery if it catches a handler exception. `Vanished` means the task was gone before the +delivery could finish it, for example because its lock expired and another worker took it over; it +is not a failure. From Java, the singletons are `TaskHandlerOutcome.Completed.INSTANCE`, +`TaskHandlerOutcome.Vanished.INSTANCE` and `TaskHandlerOutcome.Undetermined.INSTANCE`; match the others with `instanceof`. The outcome is only known if the handler finishes the task in the delivery thread through a `ServiceTaskCompletionApi` wrapped in `ObserverAwareServiceTaskCompletionApi` -(module `process-engine-api-impl`). The adapter has to wrap its completion API and pass the -observers; either one alone makes every outcome `Undetermined`. A task finished later, by another +(module `process-engine-api-impl`). A task finished later, by another thread, or not through a `ServiceTaskCompletionApi` (as user tasks are) results in `Undetermined`. `Undetermined` means neither success nor failure, so do not treat it as either. The outcome reflects the accepted request, not the result of the returned future. A completion call that throws synchronously results in `Undetermined`. -An adapter passes observers to the constructor of `AbstractTaskSubscriptionApiImpl`, in execution -order. Observers therefore only take effect with an adapter that supports them. The following -example assumes an adapter starter that collects them as Spring beans and orders them by -`@Order`/`Ordered`. It runs every task in its own span and puts the activity id into the MDC. +Observers only take effect with an adapter whose deliveries support them. The following example +assumes an adapter starter that collects them as Spring beans and orders them by `@Order`/`Ordered`. +It runs every task in its own span and puts the activity id into the MDC. ```java import dev.bpmcrafters.processengineapi.CommonRestrictions; @@ -177,3 +179,28 @@ public class LoggingTaskHandlerObserver implements TaskHandlerObserver { } } ``` + +### Supporting observers in an adapter + +An adapter creates one `TaskHandlerObservation` from the observers in execution order and calls it +in each delivery of external tasks, around the whole per-task work including the failure handling. +It reports what the handler could not report itself through the `TaskDeliveryReport`, and wraps its +`ServiceTaskCompletionApi` in `ObserverAwareServiceTaskCompletionApi`, so observers also see +outcomes reported by the handler. A reported outcome takes precedence over the one of the handler, +and `vanished` over `failed`. Without observers, or for other task types, `observe` just runs the +delivery. + +```kotlin +observation.observe(subscription, taskInformation, payload) { report -> + try { + subscription.action.accept(taskInformation, payload) + } catch (e: NotFoundException) { + logger.debug(e) { "Task ${taskInformation.taskId} is gone." } + report.vanished(e) + } catch (e: Exception) { + logger.warn(e) { "Failed delivering task ${taskInformation.taskId}." } + externalTaskService.handleFailure(taskInformation.taskId, workerId, e.message, retries - 1, retryTimeout) + report.failed(e) + } +} +``` From 8a53a4d7972a8d80e63ef47b9a237d68f99f8c05 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 3 Nov 2026 12:58:47 +0100 Subject: [PATCH 4/9] refactor(api): replace task handler observers with a task execution decorator Adapters wrap their whole per-task work, including their failure handling, with TaskExecutionDecorators, in the spirit of Spring's TaskDecorator. A span or MDC entry opened by a decorator thus covers the handler and the delivery's failure logs. The outcome model, the outcome recorder, the decorated completion API and the delivery report are removed, as the tracing use case only needs the scope. --- .../ObserverAwareServiceTaskCompletionApi.kt | 39 ---- .../impl/task/TaskExecutionDecoration.kt | 100 ++++++++ .../impl/task/TaskHandlerObservation.kt | 139 ----------- .../impl/task/TaskHandlerOutcomeRecorder.kt | 32 --- ...serverAwareServiceTaskCompletionApiTest.kt | 190 --------------- .../impl/task/TaskExecutionDecorationTest.kt | 141 +++++++++++ .../impl/task/TaskHandlerObservationTest.kt | 221 ------------------ .../task/TaskExecutionDecorator.kt | 30 +++ .../task/TaskHandlerContext.kt | 30 --- .../task/TaskHandlerObserver.kt | 44 ---- .../task/TaskHandlerOutcome.kt | 58 ----- docs/task-subscription-api.md | 182 +++++---------- 12 files changed, 335 insertions(+), 871 deletions(-) delete mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt create mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt delete mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservation.kt delete mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt delete mode 100644 api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt create mode 100644 api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecorationTest.kt delete mode 100644 api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservationTest.kt create mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskExecutionDecorator.kt delete mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerContext.kt delete mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt delete mode 100644 api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt deleted file mode 100644 index b48c0552..00000000 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApi.kt +++ /dev/null @@ -1,39 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -import dev.bpmcrafters.processengineapi.Empty -import dev.bpmcrafters.processengineapi.task.CompleteTaskByErrorCmd -import dev.bpmcrafters.processengineapi.task.CompleteTaskCmd -import dev.bpmcrafters.processengineapi.task.FailTaskCmd -import dev.bpmcrafters.processengineapi.task.ServiceTaskCompletionApi -import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome -import java.util.concurrent.CompletableFuture - -/** - * Decorates a [ServiceTaskCompletionApi] and reports each accepted request of a task handler as - * [TaskHandlerOutcome] to the running [TaskHandlerObserver]s, regardless of the returned future. - * - * Reports only calls made in the handler's thread, and nothing if the call throws synchronously. - * Use it together with a [TaskHandlerObservation] in the deliveries; without it, observers only see - * outcomes the delivery reports. - * @since 1.8 - */ -class ObserverAwareServiceTaskCompletionApi( - private val delegate: ServiceTaskCompletionApi -) : ServiceTaskCompletionApi { - - override fun completeTask(cmd: CompleteTaskCmd): CompletableFuture = - delegate.completeTask(cmd).also { - TaskHandlerOutcomeRecorder.record(cmd.taskId, TaskHandlerOutcome.Completed) - } - - override fun completeTaskByError(cmd: CompleteTaskByErrorCmd): CompletableFuture = - delegate.completeTaskByError(cmd).also { - TaskHandlerOutcomeRecorder.record(cmd.taskId, TaskHandlerOutcome.CompletedByError(cmd.errorCode, cmd.errorMessage)) - } - - override fun failTask(cmd: FailTaskCmd): CompletableFuture = - delegate.failTask(cmd).also { - TaskHandlerOutcomeRecorder.record(cmd.taskId, TaskHandlerOutcome.Failed(cmd.reason, cmd.errorDetails)) - } -} diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt new file mode 100644 index 00000000..c1caee97 --- /dev/null +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt @@ -0,0 +1,100 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.task.TaskExecutionDecorator +import dev.bpmcrafters.processengineapi.task.TaskInformation +import io.github.oshai.kotlinlogging.KotlinLogging +import java.util.Collections +import java.util.concurrent.atomic.AtomicBoolean + +private val logger = KotlinLogging.logger {} + +/** + * Runs the per-task work of a delivery with the [TaskExecutionDecorator]s around it. Adapters create + * one instance from the decorators in execution order and call [run] in every delivery of external + * tasks, around the whole per-task work including its failure handling. + * + * The first decorator is the outermost one. The work runs exactly once: a decorator that fails, skips + * or repeats the work is only logged. An exception thrown by the work itself passes through unchanged; + * only a [VirtualMachineError] thrown by a decorator propagates as well. + * @param decorators decorators in execution order. + * @since 1.8 + */ +class TaskExecutionDecoration(decorators: List) { + + private val decorators = decorators.toList() + + /** + * Runs the per-task work of a delivery with the decorators around it. + * @param task information about the delivered task. + * @param payload payload delivered to the handler. + * @param execution per-task work of the delivery, including its failure handling. + */ + fun run(task: TaskInformation, payload: Map, execution: Runnable) { + if (decorators.isEmpty()) { + execution.run() + return + } + val readOnlyTask = task.copy(meta = Collections.unmodifiableMap(task.meta)) + val readOnlyPayload = Collections.unmodifiableMap(payload) + val work = SingleRun(task.taskId, execution) + val decorated = decorators.foldRight(work) { decorator, inner -> + decorateSafely(decorator, readOnlyTask, readOnlyPayload, inner) + } + try { + decorated.run() + } catch (e: VirtualMachineError) { + throw e + } catch (e: Throwable) { + if (e === work.error) throw e + logger.error(e) { "PROCESS-ENGINE-API-016: Task execution decorator failed for task ${task.taskId}." } + } + if (!work.started) { + logger.warn { "PROCESS-ENGINE-API-017: Decorators did not run task ${task.taskId}, running it without them." } + work.run() + } + } + + companion object { + /** + * Decoration without decorators, which just runs the work. + */ + @JvmField + val NONE = TaskExecutionDecoration(emptyList()) + } +} + +private fun decorateSafely( + decorator: TaskExecutionDecorator, + task: TaskInformation, + payload: Map, + inner: Runnable +): Runnable = + try { + decorator.decorate(task, payload, inner) + } catch (e: VirtualMachineError) { + throw e + } catch (e: Throwable) { + logger.error(e) { "PROCESS-ENGINE-API-016: Task execution decorator failed for task ${task.taskId}." } + inner + } + +private class SingleRun(private val taskId: String, private val execution: Runnable) : Runnable { + private val running = AtomicBoolean(false) + var error: Throwable? = null + private set + val started: Boolean + get() = running.get() + + override fun run() { + if (!running.compareAndSet(false, true)) { + logger.warn { "PROCESS-ENGINE-API-018: Decorator ran task $taskId again, ignoring the repeated run." } + return + } + try { + execution.run() + } catch (e: Throwable) { + error = e + throw e + } + } +} diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservation.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservation.kt deleted file mode 100644 index 7f654873..00000000 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservation.kt +++ /dev/null @@ -1,139 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -import dev.bpmcrafters.processengineapi.task.TaskHandlerContext -import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome -import dev.bpmcrafters.processengineapi.task.TaskInformation -import dev.bpmcrafters.processengineapi.task.TaskType -import io.github.oshai.kotlinlogging.KotlinLogging -import java.util.Collections - -private val logger = KotlinLogging.logger {} - -/** - * Runs [TaskHandlerObserver]s around the delivery of one external task. A delivery calls [observe] - * around its whole per-task work, so the handler call and the delivery's own failure handling run - * between [TaskHandlerObserver.before] and [TaskHandlerObserver.after]: logs written while reporting - * a failure still see the context an observer opened. - * - * Observers run in list order before and in reverse order after the delivery. Everything an - * observer throws is only logged, except a [VirtualMachineError], so the delivery behaves the same - * as without observers. Only `EXTERNAL` tasks are observed; for other task types and without - * observers, the delivery just runs. - * @since 1.8 - */ -class TaskHandlerObservation( - private val observers: List> -) { - - /** - * Observes the delivery of a task to [subscription]. - * @param subscription subscription the task is delivered to. - * @param taskInformation task information passed to the handler. - * @param payload payload passed to the handler. - * @param delivery per-task work of the delivery, including its failure handling. It reports what - * the handler could not report itself through the given [TaskDeliveryReport]. - */ - fun observe( - subscription: TaskSubscriptionHandle, - taskInformation: TaskInformation, - payload: Map, - delivery: (TaskDeliveryReport) -> Unit - ) { - if (observers.isEmpty() || subscription.taskType != TaskType.EXTERNAL) { - delivery(TaskDeliveryReport.IGNORED) - return - } - val context = TaskHandlerContext( - taskInformation.copy(meta = Collections.unmodifiableMap(taskInformation.meta)), - Collections.unmodifiableMap(payload), - subscription.taskDescriptionKey, - subscription.taskType - ) - val report = RecordedDeliveryReport() - TaskHandlerOutcomeRecorder.recordingFor(taskInformation.taskId) { - val startedObservers = mutableListOf>() - var deliveryError: Throwable? = null - try { - observers.forEach { observer -> start(observer, context)?.let(startedObservers::add) } - delivery(report) - } catch (e: Throwable) { - deliveryError = e - throw e - } finally { - val outcome = report.outcome ?: TaskHandlerOutcomeRecorder.recordedOutcome() - val error = deliveryError ?: report.error - startedObservers.asReversed().forEach { it.finish(context, outcome, error) } - } - } - } - - private fun start(observer: TaskHandlerObserver, context: TaskHandlerContext): StartedObserver? = - logFailureOf({ "PROCESS-ENGINE-API-016: Observer failed before handler of task ${context.taskInformation.taskId}." }) { - StartedObserver(observer, observer.before(context)) - } - - private class StartedObserver(private val observer: TaskHandlerObserver, private val state: S) { - fun finish(context: TaskHandlerContext, outcome: TaskHandlerOutcome, error: Throwable?) { - logFailureOf({ "PROCESS-ENGINE-API-017: Observer failed after handler of task ${context.taskInformation.taskId}." }) { - observer.after(context, state, outcome, error) - } - } - } - - private class RecordedDeliveryReport : TaskDeliveryReport { - var outcome: TaskHandlerOutcome? = null - var error: Throwable? = null - - override fun failed(error: Throwable) { - this.error = error - if (outcome != TaskHandlerOutcome.Vanished) { - outcome = TaskHandlerOutcome.Failed(error.message ?: error.javaClass.name, null) - } - } - - override fun vanished(error: Throwable) { - this.error = error - outcome = TaskHandlerOutcome.Vanished - } - } -} - -/** - * Lets a delivery report to the [TaskHandlerObserver]s what happened to a task it handles itself, - * without rethrowing. A reported outcome takes precedence over the one the handler reported through - * the completion API, and [vanished] over [failed]. - * @since 1.8 - */ -interface TaskDeliveryReport { - /** - * The delivery caught [error] and reported a failure of the task to the engine. - */ - fun failed(error: Throwable) - - /** - * The task was gone before the delivery could finish it, e.g. because its lock expired. - */ - fun vanished(error: Throwable) - - companion object { - /** - * Report that discards everything, used when no observer runs. - */ - @JvmField - val IGNORED: TaskDeliveryReport = object : TaskDeliveryReport { - override fun failed(error: Throwable) = Unit - override fun vanished(error: Throwable) = Unit - } - } -} - -private inline fun logFailureOf(noinline message: () -> String, execution: () -> T): T? = - try { - execution() - } catch (e: VirtualMachineError) { - throw e - } catch (e: Throwable) { - logger.error(e, message) - null - } diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt deleted file mode 100644 index d5c4bbdc..00000000 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerOutcomeRecorder.kt +++ /dev/null @@ -1,32 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome - -/** - * Keeps the [TaskHandlerOutcome] reported for the task currently handled by the executing thread. - * @since 1.8 - */ -internal object TaskHandlerOutcomeRecorder { - - private class Recording(val taskId: String) { - var outcome: TaskHandlerOutcome = TaskHandlerOutcome.Undetermined - } - - private val currentRecording = ThreadLocal() - - fun recordingFor(taskId: String, execution: () -> T): T { - val enclosingRecording = currentRecording.get() - currentRecording.set(Recording(taskId)) - try { - return execution() - } finally { - if (enclosingRecording == null) currentRecording.remove() else currentRecording.set(enclosingRecording) - } - } - - fun record(taskId: String, outcome: TaskHandlerOutcome) { - currentRecording.get()?.takeIf { it.taskId == taskId }?.outcome = outcome - } - - fun recordedOutcome(): TaskHandlerOutcome = currentRecording.get()?.outcome ?: TaskHandlerOutcome.Undetermined -} diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt deleted file mode 100644 index b63f95ab..00000000 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/ObserverAwareServiceTaskCompletionApiTest.kt +++ /dev/null @@ -1,190 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -import dev.bpmcrafters.processengineapi.Empty -import dev.bpmcrafters.processengineapi.task.CompleteTaskByErrorCmd -import dev.bpmcrafters.processengineapi.task.CompleteTaskCmd -import dev.bpmcrafters.processengineapi.task.FailTaskCmd -import dev.bpmcrafters.processengineapi.task.ServiceTaskCompletionApi -import dev.bpmcrafters.processengineapi.task.TaskHandlerContext -import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver -import dev.bpmcrafters.processengineapi.task.TaskHandler -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome -import dev.bpmcrafters.processengineapi.task.TaskInformation -import dev.bpmcrafters.processengineapi.task.TaskType -import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.Assertions.assertThatThrownBy -import org.junit.jupiter.api.Test -import java.util.concurrent.CompletableFuture - -internal class ObserverAwareServiceTaskCompletionApiTest { - - private val taskInformation = TaskInformation("task-id", emptyMap()) - private val delegatedCommands = mutableListOf() - private val completionApi = ObserverAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { - override fun completeTask(cmd: CompleteTaskCmd) = delegated(cmd) - override fun completeTaskByError(cmd: CompleteTaskByErrorCmd) = delegated(cmd) - override fun failTask(cmd: FailTaskCmd) = delegated(cmd) - }) - - private fun delegated(cmd: Any): CompletableFuture { - delegatedCommands += cmd - return CompletableFuture.completedFuture(Empty) - } - - private fun observe(handler: TaskHandler, observer: TaskHandlerObserver<*>) { - val subscription = TaskSubscriptionHandle(TaskType.EXTERNAL, null, emptyMap(), "task-key", handler) { } - TaskHandlerObservation(listOf(observer)).observe(subscription, taskInformation, emptyMap()) { - subscription.action.accept(taskInformation, emptyMap()) - } - } - - private fun outcomeSeenByObserverOf(handler: TaskHandler): TaskHandlerOutcome { - var seenOutcome: TaskHandlerOutcome? = null - val observer = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = Unit - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { - seenOutcome = outcome - } - } - observe(handler, observer) - return seenOutcome!! - } - - @Test - fun `reports failed task`() { - val cmd = FailTaskCmd("task-id", "reason", "details") - - val outcome = outcomeSeenByObserverOf { _, _ -> completionApi.failTask(cmd).get() } - - assertThat(outcome).isEqualTo(TaskHandlerOutcome.Failed("reason", "details")) - assertThat(delegatedCommands).containsExactly(cmd) - } - - @Test - fun `reports task completed by error`() { - val cmd = CompleteTaskByErrorCmd("task-id", "error-code", "message") - - val outcome = outcomeSeenByObserverOf { _, _ -> completionApi.completeTaskByError(cmd).get() } - - assertThat(outcome).isEqualTo(TaskHandlerOutcome.CompletedByError("error-code", "message")) - assertThat(delegatedCommands).containsExactly(cmd) - } - - @Test - fun `reports completed task`() { - val cmd = CompleteTaskCmd("task-id") { emptyMap() } - - val outcome = outcomeSeenByObserverOf { _, _ -> completionApi.completeTask(cmd).get() } - - assertThat(outcome).isEqualTo(TaskHandlerOutcome.Completed) - assertThat(delegatedCommands).containsExactly(cmd) - } - - @Test - fun `does not report outcome if the engine call throws`() { - val failingApi = ObserverAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { - override fun completeTask(cmd: CompleteTaskCmd): CompletableFuture = throw IllegalStateException("boom") - override fun completeTaskByError(cmd: CompleteTaskByErrorCmd) = delegated(cmd) - override fun failTask(cmd: FailTaskCmd) = delegated(cmd) - }) - - val outcome = outcomeSeenByObserverOf { _, _ -> - runCatching { failingApi.completeTask(CompleteTaskCmd("task-id") { emptyMap() }) } - } - - assertThat(outcome).isEqualTo(TaskHandlerOutcome.Undetermined) - } - - @Test - fun `reports outcome regardless of the result of the returned future`() { - val pendingApi = ObserverAwareServiceTaskCompletionApi(object : ServiceTaskCompletionApi { - override fun completeTask(cmd: CompleteTaskCmd) = CompletableFuture() - override fun completeTaskByError(cmd: CompleteTaskByErrorCmd) = delegated(cmd) - override fun failTask(cmd: FailTaskCmd) = delegated(cmd) - }) - - val outcome = outcomeSeenByObserverOf { _, _ -> - pendingApi.completeTask(CompleteTaskCmd("task-id") { emptyMap() }) - } - - assertThat(outcome).isEqualTo(TaskHandlerOutcome.Completed) - } - - @Test - fun `keeps outcomes of nested and enclosing execution apart`() { - var innerOutcome: TaskHandlerOutcome? = null - val outerOutcome = outcomeSeenByObserverOf { _, _ -> - completionApi.failTask(FailTaskCmd("task-id", "reason", null)) - innerOutcome = outcomeSeenByObserverOf { _, _ -> completionApi.completeTask(CompleteTaskCmd("task-id") { emptyMap() }) } - } - - assertThat(innerOutcome).isEqualTo(TaskHandlerOutcome.Completed) - assertThat(outerOutcome).isEqualTo(TaskHandlerOutcome.Failed("reason", null)) - } - - @Test - fun `reports undetermined outcome if handler does not finish the task`() { - val outcome = outcomeSeenByObserverOf { _, _ -> } - - assertThat(outcome).isEqualTo(TaskHandlerOutcome.Undetermined) - } - - @Test - fun `ignores outcome of another task`() { - val outcome = outcomeSeenByObserverOf { _, _ -> - completionApi.failTask(FailTaskCmd("other-task-id", "reason", null)).get() - } - - assertThat(outcome).isEqualTo(TaskHandlerOutcome.Undetermined) - } - - @Test - fun `ignores outcome reported by another thread`() { - val outcome = outcomeSeenByObserverOf { _, _ -> - CompletableFuture.runAsync { completionApi.failTask(FailTaskCmd("task-id", "reason", null)).get() }.get() - } - - assertThat(outcome).isEqualTo(TaskHandlerOutcome.Undetermined) - } - - @Test - fun `does not leak outcome of a throwing handler into the next execution`() { - assertThatThrownBy { - outcomeSeenByObserverOf { _, _ -> - completionApi.failTask(FailTaskCmd("task-id", "reason", null)).get() - throw IllegalStateException("boom") - } - }.isInstanceOf(IllegalStateException::class.java) - - assertThat(TaskHandlerOutcomeRecorder.recordedOutcome()).isEqualTo(TaskHandlerOutcome.Undetermined) - } - - @Test - fun `reports outcome to observers if handler throws after finishing the task`() { - var seenOutcome: TaskHandlerOutcome? = null - val observer = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = Unit - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { - seenOutcome = outcome - } - } - - assertThatThrownBy { - observe({ _, _ -> - completionApi.failTask(FailTaskCmd("task-id", "reason", null)).get() - throw IllegalStateException("boom") - }, observer) - }.isInstanceOf(IllegalStateException::class.java) - - assertThat(seenOutcome).isEqualTo(TaskHandlerOutcome.Failed("reason", null)) - } - - @Test - fun `executes command without a running handler execution`() { - val cmd = FailTaskCmd("task-id", "reason", null) - - completionApi.failTask(cmd).get() - - assertThat(delegatedCommands).containsExactly(cmd) - } -} diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecorationTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecorationTest.kt new file mode 100644 index 00000000..746304b2 --- /dev/null +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecorationTest.kt @@ -0,0 +1,141 @@ +package dev.bpmcrafters.processengineapi.impl.task + +import dev.bpmcrafters.processengineapi.task.TaskExecutionDecorator +import dev.bpmcrafters.processengineapi.task.TaskInformation +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test + +internal class TaskExecutionDecorationTest { + + private val task = TaskInformation("task-id", mutableMapOf("topicName" to "my-topic")) + private val payload = mutableMapOf("var1" to "value1") + private val trace = mutableListOf() + private val execution = Runnable { trace += "execution" } + + private fun tracing(name: String) = TaskExecutionDecorator { _, _, inner -> + Runnable { + trace += "$name-before" + try { + inner.run() + } finally { + trace += "$name-after" + } + } + } + + @Test + fun `runs execution directly without decorators`() { + TaskExecutionDecoration.NONE.run(task, payload, execution) + + assertThat(trace).containsExactly("execution") + } + + @Test + fun `is not affected by later changes of the decorator list`() { + val decorators = mutableListOf(tracing("first")) + val decoration = TaskExecutionDecoration(decorators) + decorators += tracing("second") + + decoration.run(task, payload, execution) + + assertThat(trace).containsExactly("first-before", "execution", "first-after") + } + + @Test + fun `runs decorators around execution with the first one outermost`() { + TaskExecutionDecoration(listOf(tracing("first"), tracing("second"))).run(task, payload, execution) + + assertThat(trace).containsExactly("first-before", "second-before", "execution", "second-after", "first-after") + } + + @Test + fun `passes read-only task and payload to decorators`() { + var seenTask: TaskInformation? = null + var seenPayload: Map? = null + val capturing = TaskExecutionDecorator { decoratedTask, decoratedPayload, inner -> + seenTask = decoratedTask + seenPayload = decoratedPayload + inner + } + + TaskExecutionDecoration(listOf(capturing)).run(task, payload, execution) + + assertThat(seenTask).isEqualTo(task) + assertThat(seenPayload).isEqualTo(payload) + assertThatThrownBy { (seenTask!!.meta as MutableMap)["key"] = "value" } + .isInstanceOf(UnsupportedOperationException::class.java) + assertThatThrownBy { (seenPayload as MutableMap)["key"] = "value" } + .isInstanceOf(UnsupportedOperationException::class.java) + } + + @Test + fun `runs execution once if a decorator skips it`() { + val skipping = TaskExecutionDecorator { _, _, _ -> Runnable { trace += "skipping" } } + + TaskExecutionDecoration(listOf(tracing("first"), skipping)).run(task, payload, execution) + + assertThat(trace).containsExactly("first-before", "skipping", "first-after", "execution") + } + + @Test + fun `runs execution once if a decorator repeats it`() { + val repeating = TaskExecutionDecorator { _, _, inner -> Runnable { inner.run(); inner.run() } } + + TaskExecutionDecoration(listOf(repeating)).run(task, payload, execution) + + assertThat(trace).containsExactly("execution") + } + + @Test + fun `runs execution with remaining decorators if decorate throws`() { + val failing = TaskExecutionDecorator { _, _, _ -> throw IllegalStateException("boom") } + + TaskExecutionDecoration(listOf(tracing("first"), failing)).run(task, payload, execution) + + assertThat(trace).containsExactly("first-before", "execution", "first-after") + } + + @Test + fun `runs execution if a decorator throws before running it`() { + val failing = TaskExecutionDecorator { _, _, _ -> Runnable { throw AssertionError("boom") } } + + TaskExecutionDecoration(listOf(failing)).run(task, payload, execution) + + assertThat(trace).containsExactly("execution") + } + + @Test + fun `only logs exception thrown by a decorator after the execution ran`() { + val failing = TaskExecutionDecorator { _, _, inner -> Runnable { inner.run(); throw IllegalStateException("boom") } } + + TaskExecutionDecoration(listOf(failing)).run(task, payload, execution) + + assertThat(trace).containsExactly("execution") + } + + @Test + fun `propagates exception thrown by the execution unchanged through decorators`() { + val executionError = IllegalStateException("execution") + + assertThatThrownBy { TaskExecutionDecoration(listOf(tracing("first"))).run(task, payload) { throw executionError } } + .isSameAs(executionError) + assertThat(trace).containsExactly("first-before", "first-after") + } + + @Test + fun `propagates error thrown by the execution unchanged through decorators`() { + val executionError = AssertionError("execution") + + assertThatThrownBy { TaskExecutionDecoration(listOf(tracing("first"))).run(task, payload) { throw executionError } } + .isSameAs(executionError) + } + + @Test + fun `propagates virtual machine errors thrown by decorators`() { + val failing = TaskExecutionDecorator { _, _, _ -> throw StackOverflowError("boom") } + + assertThatThrownBy { TaskExecutionDecoration(listOf(failing)).run(task, payload, execution) } + .isInstanceOf(StackOverflowError::class.java) + } +} diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservationTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservationTest.kt deleted file mode 100644 index 0e6b91cd..00000000 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskHandlerObservationTest.kt +++ /dev/null @@ -1,221 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -import dev.bpmcrafters.processengineapi.task.TaskHandlerContext -import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome -import dev.bpmcrafters.processengineapi.task.TaskInformation -import dev.bpmcrafters.processengineapi.task.TaskType -import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.Assertions.assertThatThrownBy -import org.junit.jupiter.api.Test - -internal class TaskHandlerObservationTest { - - private val taskInformation = TaskInformation("task-id", mutableMapOf("meta-key" to "meta-value")) - private val payload = mutableMapOf("var1" to "value1") - private val trace = mutableListOf() - - private inner class TracingObserver(private val name: String) : TaskHandlerObserver { - var seenOutcome: TaskHandlerOutcome? = null - var seenError: Throwable? = null - - override fun before(context: TaskHandlerContext): String { - trace += "$name-before" - return "$name-state" - } - - override fun after(context: TaskHandlerContext, state: String, outcome: TaskHandlerOutcome, error: Throwable?) { - trace += "$name-after:$state" - seenOutcome = outcome - seenError = error - } - } - - private fun throwingBefore(failure: Throwable) = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = throw failure - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) { - trace += "failing-after" - } - } - - private fun throwingAfter(failure: Throwable) = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) = Unit - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = throw failure - } - - private fun subscription(taskType: TaskType = TaskType.EXTERNAL) = - TaskSubscriptionHandle(taskType, null, emptyMap(), "task-key", { _, _ -> }, { }) - - private fun observe( - vararg observers: TaskHandlerObserver<*>, - taskType: TaskType = TaskType.EXTERNAL, - delivery: (TaskDeliveryReport) -> Unit = { trace += "delivery" } - ) = TaskHandlerObservation(observers.toList()).observe(subscription(taskType), taskInformation, payload, delivery) - - @Test - fun `runs observers around delivery and hands each its own state`() { - observe(TracingObserver("first"), TracingObserver("second")) - - assertThat(trace).containsExactly( - "first-before", "second-before", "delivery", "second-after:second-state", "first-after:first-state" - ) - } - - @Test - fun `runs delivery failure handling between before and after`() { - val observer = TracingObserver("first") - val handlerError = IllegalStateException("boom") - - observe(observer) { report -> - try { - throw handlerError - } catch (e: Exception) { - trace += "failure-handling" - report.failed(e) - } - } - - assertThat(trace).containsExactly("first-before", "failure-handling", "first-after:first-state") - assertThat(observer.seenOutcome).isEqualTo(TaskHandlerOutcome.Failed("boom", null)) - assertThat(observer.seenError).isSameAs(handlerError) - } - - @Test - fun `reports vanished task and lets it take precedence over failed`() { - val observer = TracingObserver("first") - val notFound = IllegalStateException("not found") - - observe(observer) { report -> - report.vanished(notFound) - report.failed(notFound) - } - - assertThat(observer.seenOutcome).isEqualTo(TaskHandlerOutcome.Vanished) - assertThat(observer.seenError).isSameAs(notFound) - } - - @Test - fun `lets reported failure take precedence over outcome recorded by the handler`() { - val observer = TracingObserver("first") - - observe(observer) { report -> - TaskHandlerOutcomeRecorder.record("task-id", TaskHandlerOutcome.Completed) - report.failed(IllegalStateException("not found")) - } - - assertThat(observer.seenOutcome).isEqualTo(TaskHandlerOutcome.Failed("not found", null)) - } - - @Test - fun `passes outcome recorded by the handler if delivery reports nothing`() { - val observer = TracingObserver("first") - - observe(observer) { TaskHandlerOutcomeRecorder.record("task-id", TaskHandlerOutcome.Completed) } - - assertThat(observer.seenOutcome).isEqualTo(TaskHandlerOutcome.Completed) - assertThat(observer.seenError).isNull() - } - - @Test - fun `propagates exception thrown by delivery unchanged and passes it to observers`() { - val observer = TracingObserver("first") - val deliveryError = IllegalStateException("delivery") - - assertThatThrownBy { observe(observer, throwingAfter(IllegalArgumentException("observer"))) { throw deliveryError } } - .isSameAs(deliveryError) - assertThat(observer.seenError).isSameAs(deliveryError) - } - - @Test - fun `propagates error thrown by delivery unchanged`() { - val deliveryError = AssertionError("delivery") - - assertThatThrownBy { observe(TracingObserver("first")) { throw deliveryError } }.isSameAs(deliveryError) - assertThat(trace).containsExactly("first-before", "first-after:first-state") - } - - @Test - fun `runs delivery and other observers if before throws and skips after of the failed observer`() { - observe(TracingObserver("first"), throwingBefore(IllegalStateException("boom"))) - - assertThat(trace).containsExactly("first-before", "delivery", "first-after:first-state") - } - - @Test - fun `does not propagate exception thrown by after and still runs other observers`() { - observe(TracingObserver("first"), throwingAfter(IllegalStateException("boom"))) - - assertThat(trace).containsExactly("first-before", "delivery", "first-after:first-state") - } - - @Test - fun `only logs errors thrown by observers`() { - val observer = TracingObserver("first") - - observe(observer, throwingBefore(AssertionError("before")), throwingAfter(NoClassDefFoundError("after"))) - - assertThat(trace).containsExactly("first-before", "delivery", "first-after:first-state") - assertThat(observer.seenError).isNull() - } - - @Test - fun `propagates virtual machine errors thrown by observers`() { - assertThatThrownBy { observe(throwingBefore(StackOverflowError("before"))) } - .isInstanceOf(StackOverflowError::class.java) - } - - @Test - fun `runs delivery without observers for other task types`() { - observe(TracingObserver("first"), taskType = TaskType.USER) - - assertThat(trace).containsExactly("delivery") - } - - @Test - fun `runs delivery without observers`() { - observe() - - assertThat(trace).containsExactly("delivery") - } - - @Test - fun `invokes delivery exactly once`() { - var invocations = 0 - - observe(TracingObserver("first")) { invocations++ } - - assertThat(invocations).isEqualTo(1) - } - - @Test - fun `provides read-only context`() { - var captured: TaskHandlerContext? = null - val capturing = object : TaskHandlerObserver { - override fun before(context: TaskHandlerContext) { - captured = context - } - override fun after(context: TaskHandlerContext, state: Unit, outcome: TaskHandlerOutcome, error: Throwable?) = Unit - } - - observe(capturing) - - val context = captured!! - assertThat(context.taskInformation).isEqualTo(taskInformation) - assertThat(context.payload).isEqualTo(payload) - assertThat(context.taskDescriptionKey).isEqualTo("task-key") - assertThat(context.taskType).isEqualTo(TaskType.EXTERNAL) - assertThatThrownBy { (context.payload as MutableMap)["key"] = "value" } - .isInstanceOf(UnsupportedOperationException::class.java) - assertThatThrownBy { (context.taskInformation.meta as MutableMap)["key"] = "value" } - .isInstanceOf(UnsupportedOperationException::class.java) - } - - @Test - fun `describes context without payload`() { - val context = TaskHandlerContext(taskInformation, mapOf("secret" to "value"), "task-key", TaskType.EXTERNAL) - - assertThat(context.toString()) - .isEqualTo("TaskHandlerContext(taskId=task-id, taskDescriptionKey=task-key, taskType=EXTERNAL)") - } - -} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskExecutionDecorator.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskExecutionDecorator.kt new file mode 100644 index 00000000..6342f911 --- /dev/null +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskExecutionDecorator.kt @@ -0,0 +1,30 @@ +package dev.bpmcrafters.processengineapi.task + +/** + * Decorates the execution of an external service task, for cross-cutting concerns such as tracing, + * MDC, logging or metrics, in the spirit of Spring's `TaskDecorator`. + * + * The adapter delivery passes its whole per-task work as [execution]: the call of the [TaskHandler] + * and the delivery's own failure handling and logging. Code placed around `execution.run()` therefore + * runs before and after both, and a span or MDC entry opened there also covers the delivery's + * failure logs. + * + * A decorator cannot change the task: it receives read-only views of the task information and the + * payload. Call `execution.run()` exactly once, in the thread the decorated runnable runs in. The + * adapter runs the execution once even if a decorator skips, repeats or fails around it, and only + * logs such misbehaviour. Decorators run synchronously in the delivery thread, so keep them cheap and + * non-blocking. + * + * This is not an engine listener such as a Camunda `ExecutionListener` or `TaskListener`. + * @since 1.8 + */ +fun interface TaskExecutionDecorator { + /** + * Decorates the execution of one task. + * @param task information about the delivered task, as an unmodifiable view. + * @param payload payload delivered to the handler, as an unmodifiable view. + * @param execution per-task work of the delivery. Run it exactly once. + * @return runnable that runs [execution] with the decoration around it. + */ + fun decorate(task: TaskInformation, payload: Map, execution: Runnable): Runnable +} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerContext.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerContext.kt deleted file mode 100644 index 8ff61a89..00000000 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerContext.kt +++ /dev/null @@ -1,30 +0,0 @@ -package dev.bpmcrafters.processengineapi.task - -/** - * Read-only context of an observed [TaskHandler] execution. The handler receives the original - * arguments, so changing this context never reaches it. The payload is a view, not a copy: values - * inside it are shared with the handler and must not be mutated, and changes the handler makes to a - * mutable payload map are visible in [TaskHandlerObserver.after]. - * @since 1.8 - */ -class TaskHandlerContext( - /** - * Information about the task being delivered, with an unmodifiable view of its meta. - */ - val taskInformation: TaskInformation, - /** - * Unmodifiable view of the payload variables delivered to the handler. - */ - val payload: Map, - /** - * Task description key of the subscription, if any. - */ - val taskDescriptionKey: String?, - /** - * Type of the observed task. - */ - val taskType: TaskType -) { - override fun toString(): String = - "TaskHandlerContext(taskId=${taskInformation.taskId}, taskDescriptionKey=$taskDescriptionKey, taskType=$taskType)" -} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt deleted file mode 100644 index de28d5fd..00000000 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerObserver.kt +++ /dev/null @@ -1,44 +0,0 @@ -package dev.bpmcrafters.processengineapi.task - -/** - * Observes the delivery of external service tasks to a [TaskHandler], for cross-cutting concerns - * such as logging, tracing or auditing, without replacing the delivery layer or using AOP. It is not - * an engine listener such as a Camunda `ExecutionListener` or `TaskListener`. - * - * The delivery runs observers around its whole per-task work: [before] runs before the handler, - * [after] once the delivery has finished, including its failure handling. Only `EXTERNAL` tasks are - * observed. - * - * An observer cannot affect the task: the handler always runs exactly once, and everything an - * observer throws is only logged, except a [VirtualMachineError]. Observers run synchronously in the - * delivery thread, so keep them cheap and non-blocking. - * - * Observers only run with an adapter whose deliveries use `TaskHandlerObservation` (module - * `process-engine-api-impl`). The outcome of the handler is only known if the adapter also wraps - * its completion API in `ObserverAwareServiceTaskCompletionApi`; otherwise it is - * [TaskHandlerOutcome.Undetermined] unless the delivery reports one. - * - * @param S state passed from [before] to [after] of the same execution: [Unit] in Kotlin or `Void` - * (returning `null`) in Java if none is needed. - * @since 1.8 - */ -interface TaskHandlerObserver { - /** - * Called before the handler. Acquire resources all or nothing: if this method throws, [after] - * is not called for this observer. - * @param context context of the execution. - * @return state handed to [after]. - */ - fun before(context: TaskHandlerContext): S - - /** - * Called after the delivery has finished, even if the handler threw. Release the resources acquired in [before] in a - * `finally` block, otherwise they leak into the next execution on this thread. - * @param context context of the execution. - * @param state state returned by [before]. - * @param outcome outcome reported by the handler or the delivery. - * @param error exception thrown by the handler or the delivery, if any, also if the delivery - * handled it. - */ - fun after(context: TaskHandlerContext, state: S, outcome: TaskHandlerOutcome, error: Throwable?) -} diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt deleted file mode 100644 index b7cbc9d3..00000000 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskHandlerOutcome.kt +++ /dev/null @@ -1,58 +0,0 @@ -package dev.bpmcrafters.processengineapi.task - -/** - * What a [TaskHandler] requested via the [ServiceTaskCompletionApi] in its executing thread, or - * what the delivery reported for it, not whether the engine processed it. Requires a completion API wrapped in - * `ObserverAwareServiceTaskCompletionApi` (module `process-engine-api-impl`). From Java, the - * singletons are `TaskHandlerOutcome.Completed.INSTANCE`, `TaskHandlerOutcome.Vanished.INSTANCE` and - * `TaskHandlerOutcome.Undetermined.INSTANCE`. - * @since 1.8 - */ -sealed interface TaskHandlerOutcome { - - /** - * The handler completed the task. - */ - data object Completed : TaskHandlerOutcome - - /** - * The handler completed the task by throwing a BPMN error. Mirrors [CompleteTaskByErrorCmd]. - */ - data class CompletedByError( - /** - * Error code. - */ - val errorCode: String, - /** - * Optional error details. - */ - val errorMessage: String? - ) : TaskHandlerOutcome - - /** - * The handler failed the task because of a technical failure. Mirrors [FailTaskCmd]. - */ - data class Failed( - /** - * Failure reason. - */ - val reason: String, - /** - * Optional failure details. - */ - val errorDetails: String? - ) : TaskHandlerOutcome - - /** - * The task was gone before the delivery could finish it, e.g. because its lock expired and another - * worker took it over. Reported by the delivery; not a failure of the handler. - */ - data object Vanished : TaskHandlerOutcome - - /** - * No outcome was reported in the executing thread: the task is finished later or by another - * thread, is not finished through a [ServiceTaskCompletionApi] (as user tasks), the completion - * call threw synchronously, or the adapter does not report outcomes. - */ - data object Undetermined : TaskHandlerOutcome -} diff --git a/docs/task-subscription-api.md b/docs/task-subscription-api.md index 3644a924..1f955c4d 100644 --- a/docs/task-subscription-api.md +++ b/docs/task-subscription-api.md @@ -54,153 +54,99 @@ public class TaskSubscriber { ``` -## Task handler observers - -Cross-cutting concerns such as logging, tracing, or auditing can observe the delivery of external -service tasks via a `TaskHandlerObserver`. An observer applies to both registration paths -(`@ProcessEngineWorker` and manual `subscribeForTask`). Only `EXTERNAL` tasks are observed. - -An observer watches the handler call of a subscription in the application. It is not an engine -listener such as a Camunda `ExecutionListener` or `TaskListener`, and it is not declared in the BPMN -model. - -The delivery runs the observers around its whole per-task work: `before` in list order before the -handler, `after` in reverse order once the delivery has finished, including its failure handling. -Logs the delivery writes while reporting a failure therefore still see the trace or MDC context an -observer opened. The handler always runs exactly once. An observer cannot affect the task: - -- Everything thrown by `before` or `after` is only logged, except a `VirtualMachineError` such as - `OutOfMemoryError`. If `before` throws, `after` is not called for this observer; the handler and - all other observers still run. -- An exception thrown by the handler reaches `after` as `error`, also if the delivery catches it and - reports a failure to the engine. The delivery handles it as without observers. -- The context is read-only. The handler receives the original arguments, so changing the context - never reaches it. The payload is a view, not a copy: do not mutate values inside it, as they are - shared with the handler, and expect changes the handler makes to a mutable payload map to be - visible in `after`. - -`before` returns a state that the same observer receives in `after`, so no `ThreadLocal` is needed -and no observer can see the state of another. If no state is needed, use `Unit` in Kotlin, or `Void` -in Java and return `null` from `before`. - -Observers run synchronously in the delivery thread. Work in `before` delays the handler and uses up -the lock time of the task, and work in `after` blocks the worker thread. Keep them cheap and -non-blocking, and do not call external systems synchronously. Acquire resources in `before` all or -nothing, and release them in `after` in a `finally` block: a scope or MDC entry left open leaks into -the next task on this thread. - -A handler may report a failure or a BPMN error itself instead of throwing an exception, as the -`process-engine-worker` does. The `TaskHandlerOutcome` passed to `after` tells what happened: -`Completed`, `CompletedByError`, `Failed`, `Vanished`, or `Undetermined`. `Failed` is also reported -by the delivery if it catches a handler exception. `Vanished` means the task was gone before the -delivery could finish it, for example because its lock expired and another worker took it over; it -is not a failure. From Java, the singletons are `TaskHandlerOutcome.Completed.INSTANCE`, -`TaskHandlerOutcome.Vanished.INSTANCE` and `TaskHandlerOutcome.Undetermined.INSTANCE`; match the -others with `instanceof`. The outcome is only known if the handler finishes the task in the delivery -thread through a `ServiceTaskCompletionApi` wrapped in `ObserverAwareServiceTaskCompletionApi` -(module `process-engine-api-impl`). A task finished later, by another -thread, or not through a `ServiceTaskCompletionApi` (as user tasks are) results in `Undetermined`. -`Undetermined` means neither success nor failure, so do not treat it as either. The outcome reflects -the accepted request, not the result of the returned future. A completion call that throws -synchronously results in `Undetermined`. - -Observers only take effect with an adapter whose deliveries support them. The following example -assumes an adapter starter that collects them as Spring beans and orders them by `@Order`/`Ordered`. -It runs every task in its own span and puts the activity id into the MDC. - -```java -import dev.bpmcrafters.processengineapi.CommonRestrictions; -import dev.bpmcrafters.processengineapi.task.TaskHandlerContext; -import dev.bpmcrafters.processengineapi.task.TaskHandlerObserver; -import dev.bpmcrafters.processengineapi.task.TaskHandlerOutcome; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.api.trace.StatusCode; -import io.opentelemetry.api.trace.Tracer; -import io.opentelemetry.context.Scope; -import lombok.RequiredArgsConstructor; -import org.slf4j.MDC; -import org.springframework.core.annotation.Order; -import org.springframework.stereotype.Component; +## Task execution decorators + +Cross-cutting concerns such as tracing, MDC, logging, or metrics can wrap the execution of external +service tasks with a `TaskExecutionDecorator`, in the spirit of Spring's `TaskDecorator`. A decorator +applies to both registration paths (`@ProcessEngineWorker` and manual `subscribeForTask`). It is not an +engine listener such as a Camunda `ExecutionListener` or `TaskListener`, and it is not declared in the +BPMN model. + +The adapter delivery passes its whole per-task work to the decorators: the call of the handler and the +delivery's own failure handling and logging. Code placed before and after `execution.run()` therefore +runs before and after both. A span or MDC entry opened around `execution.run()` also covers the +handler's logs, the logs of the `process-engine-worker`, and the logs the delivery writes when it +reports a failure. + +A decorator cannot change the task: + +- It receives read-only views of the task information and the payload. Do not mutate values inside + the payload, as they are shared with the handler. +- Call `execution.run()` exactly once, in the thread of the decorated runnable. The adapter runs the + execution exactly once anyway: if a decorator skips it, throws before running it, or runs it twice, + this is only logged. +- An exception a decorator throws is only logged, except a `VirtualMachineError`. An exception thrown + by the execution itself passes through the decorators unchanged. +- The delivery handles a failing handler itself, so `execution.run()` returns normally even if the + task failed. A decorator does not learn the outcome; the failure is visible in the logs written + inside its scope. + +Several decorators can be registered. They are nested in execution order, the first one outermost, +so put tracing first to give the inner decorators' logs the trace id. Decorators run synchronously in +the delivery thread. Keep them cheap and non-blocking, and release what you open in a `finally` block, +otherwise a scope or MDC entry leaks into the next task on this thread. + +Decorators only take effect with an adapter that supports them. The following example assumes an +adapter starter that collects them as Spring beans and orders them by `@Order`/`Ordered`. It runs +every task in its own span and puts the activity id into the MDC. +```kotlin @Component @Order(0) -@RequiredArgsConstructor -public class TracingTaskHandlerObserver implements TaskHandlerObserver { - - public record OpenSpan(Span span, Scope scope, MDC.MDCCloseable mdc) {} +class TracingTaskExecutionDecorator(private val tracer: Tracer) : TaskExecutionDecorator { - private final Tracer tracer; - - @Override - public OpenSpan before(TaskHandlerContext context) { - String activityId = context.getTaskInformation().getMeta().getOrDefault(CommonRestrictions.ACTIVITY_ID, ""); - Span span = tracer.spanBuilder(context.getTaskDescriptionKey() + ".handle") + override fun decorate(task: TaskInformation, payload: Map, execution: Runnable) = Runnable { + val span = tracer.spanBuilder("${task.meta["topicName"]}.handle") .setNoParent() .setSpanKind(SpanKind.CONSUMER) - .startSpan(); - Scope scope = span.makeCurrent(); - MDC.MDCCloseable mdc = MDC.putCloseable("activityId", activityId); - return new OpenSpan(span, scope, mdc); - } - - @Override - public void after(TaskHandlerContext context, OpenSpan state, TaskHandlerOutcome outcome, Throwable error) { + .setAttribute("engine.task.id", task.taskId) + .startSpan() try { - if (error != null) { - state.span().recordException(error).setStatus(StatusCode.ERROR); - } else if (outcome instanceof TaskHandlerOutcome.Failed failed) { - state.span().setStatus(StatusCode.ERROR, failed.getReason()); + span.makeCurrent().use { + MDC.putCloseable("activityId", task.meta[CommonRestrictions.ACTIVITY_ID]).use { + execution.run() + } } } finally { - state.mdc().close(); - state.scope().close(); - state.span().end(); + span.end() } } } ``` -An observer without state, for example for logging: +The same in Java, as a lambda that logs before and after each task: ```java -@Component -@Slf4j -public class LoggingTaskHandlerObserver implements TaskHandlerObserver { - - @Override - public Void before(TaskHandlerContext context) { - return null; - } - - @Override - public void after(TaskHandlerContext context, Void state, TaskHandlerOutcome outcome, Throwable error) { - log.info("[TASK]: {} finished with {}", context.getTaskInformation().getTaskId(), outcome); - } +@Bean +@Order(1) +TaskExecutionDecorator loggingTaskExecutionDecorator() { + return (task, payload, execution) -> () -> { + log.info("Processing task {} with variables {}", task.getTaskId(), payload); + try { + execution.run(); + } finally { + log.info("Finished task {}", task.getTaskId()); + } + }; } ``` -### Supporting observers in an adapter +### Supporting decorators in an adapter -An adapter creates one `TaskHandlerObservation` from the observers in execution order and calls it -in each delivery of external tasks, around the whole per-task work including the failure handling. -It reports what the handler could not report itself through the `TaskDeliveryReport`, and wraps its -`ServiceTaskCompletionApi` in `ObserverAwareServiceTaskCompletionApi`, so observers also see -outcomes reported by the handler. A reported outcome takes precedence over the one of the handler, -and `vanished` over `failed`. Without observers, or for other task types, `observe` just runs the -delivery. +An adapter creates one `TaskExecutionDecoration` (module `process-engine-api-impl`) from the +decorators in execution order, passes it to its deliveries, and calls `run` in each delivery of +external tasks, around the whole per-task work including its failure handling. +`TaskExecutionDecoration.NONE` just runs the work. ```kotlin -observation.observe(subscription, taskInformation, payload) { report -> +decoration.run(taskInformation, payload) { try { subscription.action.accept(taskInformation, payload) } catch (e: NotFoundException) { logger.debug(e) { "Task ${taskInformation.taskId} is gone." } - report.vanished(e) } catch (e: Exception) { logger.warn(e) { "Failed delivering task ${taskInformation.taskId}." } externalTaskService.handleFailure(taskInformation.taskId, workerId, e.message, retries - 1, retryTimeout) - report.failed(e) } } ``` From 2534ac21ecc33642845b5da787d9560b40180460 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 3 Nov 2026 13:15:44 +0100 Subject: [PATCH 5/9] docs(api): compact task execution decorator kdoc --- .../task/TaskExecutionDecorator.kt | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskExecutionDecorator.kt b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskExecutionDecorator.kt index 6342f911..55bd1c3f 100644 --- a/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskExecutionDecorator.kt +++ b/api/src/main/kotlin/dev/bpmcrafters/processengineapi/task/TaskExecutionDecorator.kt @@ -1,21 +1,16 @@ package dev.bpmcrafters.processengineapi.task /** - * Decorates the execution of an external service task, for cross-cutting concerns such as tracing, - * MDC, logging or metrics, in the spirit of Spring's `TaskDecorator`. + * Decorates the execution of an external service task, e.g. for tracing, MDC, logging or metrics. + * Similar to Spring's `TaskDecorator`, not an engine listener such as Camunda's `ExecutionListener`. * - * The adapter delivery passes its whole per-task work as [execution]: the call of the [TaskHandler] - * and the delivery's own failure handling and logging. Code placed around `execution.run()` therefore - * runs before and after both, and a span or MDC entry opened there also covers the delivery's - * failure logs. + * `execution` is the whole per-task work of the delivery: + * the [TaskHandler] call and the delivery's failure handling. + * Code around `execution.run()` therefore also covers the delivery's failure logs. * - * A decorator cannot change the task: it receives read-only views of the task information and the - * payload. Call `execution.run()` exactly once, in the thread the decorated runnable runs in. The - * adapter runs the execution once even if a decorator skips, repeats or fails around it, and only - * logs such misbehaviour. Decorators run synchronously in the delivery thread, so keep them cheap and - * non-blocking. - * - * This is not an engine listener such as a Camunda `ExecutionListener` or `TaskListener`. + * A decorator cannot change the task, as task information and payload are read-only. + * Call `execution.run()` exactly once; the adapter ensures this and only logs misbehaviour. + * Decorators run synchronously in the delivery thread, so keep them cheap. * @since 1.8 */ fun interface TaskExecutionDecorator { From 57155f4bd6e988a59e186432eb979d2983f250c6 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 3 Nov 2026 13:26:03 +0100 Subject: [PATCH 6/9] refactor(api): simplify task execution decoration --- .../impl/task/TaskExecutionDecoration.kt | 110 ++++++++---------- .../impl/task/TaskExecutionDecorationTest.kt | 63 ++++------ docs/task-subscription-api.md | 61 ++++------ 3 files changed, 94 insertions(+), 140 deletions(-) diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt index c1caee97..c94273b2 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt @@ -9,51 +9,71 @@ import java.util.concurrent.atomic.AtomicBoolean private val logger = KotlinLogging.logger {} /** - * Runs the per-task work of a delivery with the [TaskExecutionDecorator]s around it. Adapters create - * one instance from the decorators in execution order and call [run] in every delivery of external - * tasks, around the whole per-task work including its failure handling. - * - * The first decorator is the outermost one. The work runs exactly once: a decorator that fails, skips - * or repeats the work is only logged. An exception thrown by the work itself passes through unchanged; - * only a [VirtualMachineError] thrown by a decorator propagates as well. + * Runs the per-task work of a delivery with the [TaskExecutionDecorator]s around it, the first one outermost. + * Adapters create one instance and call [run] around the whole per-task work, including its failure handling. + * The work always runs exactly once; a decorator that fails, skips or repeats it is only logged. * @param decorators decorators in execution order. * @since 1.8 */ -class TaskExecutionDecoration(decorators: List) { - - private val decorators = decorators.toList() +class TaskExecutionDecoration(private val decorators: List) { /** * Runs the per-task work of a delivery with the decorators around it. + * Exceptions thrown by the work pass through unchanged. * @param task information about the delivered task. * @param payload payload delivered to the handler. * @param execution per-task work of the delivery, including its failure handling. */ fun run(task: TaskInformation, payload: Map, execution: Runnable) { - if (decorators.isEmpty()) { - execution.run() - return - } - val readOnlyTask = task.copy(meta = Collections.unmodifiableMap(task.meta)) - val readOnlyPayload = Collections.unmodifiableMap(payload) - val work = SingleRun(task.taskId, execution) - val decorated = decorators.foldRight(work) { decorator, inner -> - decorateSafely(decorator, readOnlyTask, readOnlyPayload, inner) + val executionStarted = AtomicBoolean(false) + val executionFailed = AtomicBoolean(false) + val trackedExecution = Runnable { + if (executionStarted.getAndSet(true)) { + logger.warn { "PROCESS-ENGINE-API-018: Decorator ran task ${task.taskId} again, ignoring the repeated run." } + return@Runnable + } + try { + execution.run() + } catch (e: Exception) { + executionFailed.set(true) + throw e + } } + try { - decorated.run() - } catch (e: VirtualMachineError) { - throw e - } catch (e: Throwable) { - if (e === work.error) throw e - logger.error(e) { "PROCESS-ENGINE-API-016: Task execution decorator failed for task ${task.taskId}." } + wrapInDecorators(readOnly(task), Collections.unmodifiableMap(payload), trackedExecution).run() + } catch (e: Exception) { + if (executionFailed.get()) throw e + logDecoratorFailure(task, e) } - if (!work.started) { + + if (!executionStarted.get()) { logger.warn { "PROCESS-ENGINE-API-017: Decorators did not run task ${task.taskId}, running it without them." } - work.run() + execution.run() } } + private fun wrapInDecorators(task: TaskInformation, payload: Map, execution: Runnable): Runnable = + decorators.reversed().fold(execution) { inner, decorator -> decorateOrSkip(decorator, task, payload, inner) } + + private fun decorateOrSkip( + decorator: TaskExecutionDecorator, + task: TaskInformation, + payload: Map, + inner: Runnable + ): Runnable = + try { + decorator.decorate(task, payload, inner) + } catch (e: Exception) { + logDecoratorFailure(task, e) + inner + } + + private fun readOnly(task: TaskInformation) = task.copy(meta = Collections.unmodifiableMap(task.meta)) + + private fun logDecoratorFailure(task: TaskInformation, e: Exception) = + logger.error(e) { "PROCESS-ENGINE-API-016: Task execution decorator failed for task ${task.taskId}." } + companion object { /** * Decoration without decorators, which just runs the work. @@ -62,39 +82,3 @@ class TaskExecutionDecoration(decorators: List) { val NONE = TaskExecutionDecoration(emptyList()) } } - -private fun decorateSafely( - decorator: TaskExecutionDecorator, - task: TaskInformation, - payload: Map, - inner: Runnable -): Runnable = - try { - decorator.decorate(task, payload, inner) - } catch (e: VirtualMachineError) { - throw e - } catch (e: Throwable) { - logger.error(e) { "PROCESS-ENGINE-API-016: Task execution decorator failed for task ${task.taskId}." } - inner - } - -private class SingleRun(private val taskId: String, private val execution: Runnable) : Runnable { - private val running = AtomicBoolean(false) - var error: Throwable? = null - private set - val started: Boolean - get() = running.get() - - override fun run() { - if (!running.compareAndSet(false, true)) { - logger.warn { "PROCESS-ENGINE-API-018: Decorator ran task $taskId again, ignoring the repeated run." } - return - } - try { - execution.run() - } catch (e: Throwable) { - error = e - throw e - } - } -} diff --git a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecorationTest.kt b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecorationTest.kt index 746304b2..d9c3ac49 100644 --- a/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecorationTest.kt +++ b/api-impl/src/test/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecorationTest.kt @@ -10,16 +10,16 @@ internal class TaskExecutionDecorationTest { private val task = TaskInformation("task-id", mutableMapOf("topicName" to "my-topic")) private val payload = mutableMapOf("var1" to "value1") - private val trace = mutableListOf() - private val execution = Runnable { trace += "execution" } + private val recordedSteps = mutableListOf() + private val execution = Runnable { recordedSteps += "execution" } - private fun tracing(name: String) = TaskExecutionDecorator { _, _, inner -> + private fun recording(name: String) = TaskExecutionDecorator { _, _, inner -> Runnable { - trace += "$name-before" + recordedSteps += "$name-before" try { inner.run() } finally { - trace += "$name-after" + recordedSteps += "$name-after" } } } @@ -28,25 +28,14 @@ internal class TaskExecutionDecorationTest { fun `runs execution directly without decorators`() { TaskExecutionDecoration.NONE.run(task, payload, execution) - assertThat(trace).containsExactly("execution") - } - - @Test - fun `is not affected by later changes of the decorator list`() { - val decorators = mutableListOf(tracing("first")) - val decoration = TaskExecutionDecoration(decorators) - decorators += tracing("second") - - decoration.run(task, payload, execution) - - assertThat(trace).containsExactly("first-before", "execution", "first-after") + assertThat(recordedSteps).containsExactly("execution") } @Test fun `runs decorators around execution with the first one outermost`() { - TaskExecutionDecoration(listOf(tracing("first"), tracing("second"))).run(task, payload, execution) + TaskExecutionDecoration(listOf(recording("first"), recording("second"))).run(task, payload, execution) - assertThat(trace).containsExactly("first-before", "second-before", "execution", "second-after", "first-after") + assertThat(recordedSteps).containsExactly("first-before", "second-before", "execution", "second-after", "first-after") } @Test @@ -71,11 +60,11 @@ internal class TaskExecutionDecorationTest { @Test fun `runs execution once if a decorator skips it`() { - val skipping = TaskExecutionDecorator { _, _, _ -> Runnable { trace += "skipping" } } + val skipping = TaskExecutionDecorator { _, _, _ -> Runnable { recordedSteps += "skipping" } } - TaskExecutionDecoration(listOf(tracing("first"), skipping)).run(task, payload, execution) + TaskExecutionDecoration(listOf(recording("first"), skipping)).run(task, payload, execution) - assertThat(trace).containsExactly("first-before", "skipping", "first-after", "execution") + assertThat(recordedSteps).containsExactly("first-before", "skipping", "first-after", "execution") } @Test @@ -84,25 +73,25 @@ internal class TaskExecutionDecorationTest { TaskExecutionDecoration(listOf(repeating)).run(task, payload, execution) - assertThat(trace).containsExactly("execution") + assertThat(recordedSteps).containsExactly("execution") } @Test - fun `runs execution with remaining decorators if decorate throws`() { + fun `skips a decorator whose decorate throws`() { val failing = TaskExecutionDecorator { _, _, _ -> throw IllegalStateException("boom") } - TaskExecutionDecoration(listOf(tracing("first"), failing)).run(task, payload, execution) + TaskExecutionDecoration(listOf(recording("first"), failing)).run(task, payload, execution) - assertThat(trace).containsExactly("first-before", "execution", "first-after") + assertThat(recordedSteps).containsExactly("first-before", "execution", "first-after") } @Test - fun `runs execution if a decorator throws before running it`() { - val failing = TaskExecutionDecorator { _, _, _ -> Runnable { throw AssertionError("boom") } } + fun `runs execution when the decorated runnable throws first`() { + val failing = TaskExecutionDecorator { _, _, _ -> Runnable { throw IllegalStateException("boom") } } TaskExecutionDecoration(listOf(failing)).run(task, payload, execution) - assertThat(trace).containsExactly("execution") + assertThat(recordedSteps).containsExactly("execution") } @Test @@ -111,31 +100,23 @@ internal class TaskExecutionDecorationTest { TaskExecutionDecoration(listOf(failing)).run(task, payload, execution) - assertThat(trace).containsExactly("execution") + assertThat(recordedSteps).containsExactly("execution") } @Test fun `propagates exception thrown by the execution unchanged through decorators`() { val executionError = IllegalStateException("execution") - assertThatThrownBy { TaskExecutionDecoration(listOf(tracing("first"))).run(task, payload) { throw executionError } } + assertThatThrownBy { TaskExecutionDecoration(listOf(recording("first"))).run(task, payload) { throw executionError } } .isSameAs(executionError) - assertThat(trace).containsExactly("first-before", "first-after") + assertThat(recordedSteps).containsExactly("first-before", "first-after") } @Test fun `propagates error thrown by the execution unchanged through decorators`() { val executionError = AssertionError("execution") - assertThatThrownBy { TaskExecutionDecoration(listOf(tracing("first"))).run(task, payload) { throw executionError } } + assertThatThrownBy { TaskExecutionDecoration(listOf(recording("first"))).run(task, payload) { throw executionError } } .isSameAs(executionError) } - - @Test - fun `propagates virtual machine errors thrown by decorators`() { - val failing = TaskExecutionDecorator { _, _, _ -> throw StackOverflowError("boom") } - - assertThatThrownBy { TaskExecutionDecoration(listOf(failing)).run(task, payload, execution) } - .isInstanceOf(StackOverflowError::class.java) - } } diff --git a/docs/task-subscription-api.md b/docs/task-subscription-api.md index 1f955c4d..b7fcae06 100644 --- a/docs/task-subscription-api.md +++ b/docs/task-subscription-api.md @@ -56,39 +56,29 @@ public class TaskSubscriber { ## Task execution decorators -Cross-cutting concerns such as tracing, MDC, logging, or metrics can wrap the execution of external -service tasks with a `TaskExecutionDecorator`, in the spirit of Spring's `TaskDecorator`. A decorator -applies to both registration paths (`@ProcessEngineWorker` and manual `subscribeForTask`). It is not an -engine listener such as a Camunda `ExecutionListener` or `TaskListener`, and it is not declared in the -BPMN model. - -The adapter delivery passes its whole per-task work to the decorators: the call of the handler and the -delivery's own failure handling and logging. Code placed before and after `execution.run()` therefore -runs before and after both. A span or MDC entry opened around `execution.run()` also covers the -handler's logs, the logs of the `process-engine-worker`, and the logs the delivery writes when it -reports a failure. - -A decorator cannot change the task: - -- It receives read-only views of the task information and the payload. Do not mutate values inside - the payload, as they are shared with the handler. -- Call `execution.run()` exactly once, in the thread of the decorated runnable. The adapter runs the - execution exactly once anyway: if a decorator skips it, throws before running it, or runs it twice, - this is only logged. -- An exception a decorator throws is only logged, except a `VirtualMachineError`. An exception thrown - by the execution itself passes through the decorators unchanged. -- The delivery handles a failing handler itself, so `execution.run()` returns normally even if the - task failed. A decorator does not learn the outcome; the failure is visible in the logs written - inside its scope. - -Several decorators can be registered. They are nested in execution order, the first one outermost, -so put tracing first to give the inner decorators' logs the trace id. Decorators run synchronously in -the delivery thread. Keep them cheap and non-blocking, and release what you open in a `finally` block, -otherwise a scope or MDC entry leaks into the next task on this thread. - -Decorators only take effect with an adapter that supports them. The following example assumes an -adapter starter that collects them as Spring beans and orders them by `@Order`/`Ordered`. It runs -every task in its own span and puts the activity id into the MDC. +Cross-cutting concerns such as tracing, MDC, logging, or metrics can wrap the execution of external service tasks with a `TaskExecutionDecorator`. +It works like Spring's `TaskDecorator` and applies to `@ProcessEngineWorker` as well as to manual `subscribeForTask`. +It is not an engine listener such as a Camunda `ExecutionListener`, and it is not declared in the BPMN model. + +The adapter delivery passes its whole per-task work to the decorators: the handler call and its own failure handling. +Code before and after `execution.run()` therefore runs before and after both. +A span or MDC entry opened around `execution.run()` covers the logs of the handler, of the `process-engine-worker` and of the delivery's failure handling. + +Rules for decorators: + +- Task information and payload are read-only. Do not mutate values inside the payload, as they are shared with the handler. +- Call `execution.run()` exactly once, in the thread of the decorated runnable. If a decorator skips or repeats it, the adapter still runs it exactly once and logs a warning. +- An exception thrown by a decorator is only logged. An exception thrown by the execution passes through the decorators unchanged. +- The delivery handles a failing handler itself, so `execution.run()` returns normally even if the task failed. The failure is visible in the logs written inside the decorator's scope. + +Several decorators can be registered. They are nested in execution order, the first one outermost. +Put tracing first, so the logs of the inner decorators carry the trace id. +Decorators run synchronously in the delivery thread, so keep them cheap and non-blocking. +Release what you open in a `finally` block, otherwise a scope or MDC entry leaks into the next task on this thread. + +Decorators only take effect with an adapter that supports them. +The following example assumes an adapter starter that collects them as Spring beans ordered by `@Order`/`Ordered`. +It runs every task in its own span and puts the activity id into the MDC. ```kotlin @Component @@ -133,9 +123,8 @@ TaskExecutionDecorator loggingTaskExecutionDecorator() { ### Supporting decorators in an adapter -An adapter creates one `TaskExecutionDecoration` (module `process-engine-api-impl`) from the -decorators in execution order, passes it to its deliveries, and calls `run` in each delivery of -external tasks, around the whole per-task work including its failure handling. +An adapter creates one `TaskExecutionDecoration` (module `process-engine-api-impl`) from the decorators in execution order and passes it to its deliveries. +Each delivery of external tasks calls `run` around its whole per-task work, including the failure handling. `TaskExecutionDecoration.NONE` just runs the work. ```kotlin From bd9590f362b82db4398b9eefa267a44b0379d9dc Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 3 Nov 2026 13:40:52 +0100 Subject: [PATCH 7/9] refactor(api): describe task execution decoration step by step --- .../impl/task/TaskExecutionDecoration.kt | 70 +++++++++---------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt index c94273b2..3eb7886b 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt @@ -4,7 +4,6 @@ import dev.bpmcrafters.processengineapi.task.TaskExecutionDecorator import dev.bpmcrafters.processengineapi.task.TaskInformation import io.github.oshai.kotlinlogging.KotlinLogging import java.util.Collections -import java.util.concurrent.atomic.AtomicBoolean private val logger = KotlinLogging.logger {} @@ -18,59 +17,60 @@ private val logger = KotlinLogging.logger {} class TaskExecutionDecoration(private val decorators: List) { /** - * Runs the per-task work of a delivery with the decorators around it. * Exceptions thrown by the work pass through unchanged. * @param task information about the delivered task. * @param payload payload delivered to the handler. - * @param execution per-task work of the delivery, including its failure handling. + * @param execution per-task work of the delivery. */ fun run(task: TaskInformation, payload: Map, execution: Runnable) { - val executionStarted = AtomicBoolean(false) - val executionFailed = AtomicBoolean(false) - val trackedExecution = Runnable { - if (executionStarted.getAndSet(true)) { - logger.warn { "PROCESS-ENGINE-API-018: Decorator ran task ${task.taskId} again, ignoring the repeated run." } - return@Runnable - } - try { - execution.run() - } catch (e: Exception) { - executionFailed.set(true) - throw e - } + val executionResult = resultOfSingleRun(execution) + runDecorated(task, payload, executionResult) + if (!executionResult.isInitialized()) { + runUndecorated(task, execution) } + } + private fun resultOfSingleRun(execution: Runnable) = lazy { + runCatching { execution.run() } + } + + private fun runDecorated(task: TaskInformation, payload: Map, executionResult: Lazy>) { try { - wrapInDecorators(readOnly(task), Collections.unmodifiableMap(payload), trackedExecution).run() + val executionOnce = runOnce(task, executionResult) + val decoratedExecution = decorate(readOnly(task), Collections.unmodifiableMap(payload), executionOnce) + decoratedExecution.run() } catch (e: Exception) { - if (executionFailed.get()) throw e + if (executionResult.isFailure()) throw e logDecoratorFailure(task, e) } + } - if (!executionStarted.get()) { - logger.warn { "PROCESS-ENGINE-API-017: Decorators did not run task ${task.taskId}, running it without them." } - execution.run() - } + private fun runOnce(task: TaskInformation, executionResult: Lazy>) = Runnable { + if (executionResult.isInitialized()) warnRepeatedRun(task) else executionResult.value.getOrThrow() } - private fun wrapInDecorators(task: TaskInformation, payload: Map, execution: Runnable): Runnable = - decorators.reversed().fold(execution) { inner, decorator -> decorateOrSkip(decorator, task, payload, inner) } + private fun runUndecorated(task: TaskInformation, execution: Runnable) { + logger.warn { "PROCESS-ENGINE-API-017: Decorators did not run task ${task.taskId}, running it without them." } + execution.run() + } - private fun decorateOrSkip( - decorator: TaskExecutionDecorator, - task: TaskInformation, - payload: Map, - inner: Runnable - ): Runnable = - try { - decorator.decorate(task, payload, inner) - } catch (e: Exception) { - logDecoratorFailure(task, e) - inner + private fun decorate(task: TaskInformation, payload: Map, execution: Runnable): Runnable = + decorators.foldRight(execution) { decorator, inner -> + try { + decorator.decorate(task, payload, inner) + } catch (e: Exception) { + logDecoratorFailure(task, e) + inner + } } + private fun Lazy>.isFailure() = isInitialized() && value.isFailure + private fun readOnly(task: TaskInformation) = task.copy(meta = Collections.unmodifiableMap(task.meta)) + private fun warnRepeatedRun(task: TaskInformation) = + logger.warn { "PROCESS-ENGINE-API-018: Decorator ran task ${task.taskId} again, ignoring the repeated run." } + private fun logDecoratorFailure(task: TaskInformation, e: Exception) = logger.error(e) { "PROCESS-ENGINE-API-016: Task execution decorator failed for task ${task.taskId}." } From 3a894eaf2a7d3bbdf3f986ea5a1a9dbdb44ad584 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 3 Nov 2026 13:41:05 +0100 Subject: [PATCH 8/9] refactor(api): extract single execution from task execution decoration --- .../impl/task/SingleExecution.kt | 27 +++++++++++++++ .../impl/task/TaskExecutionDecoration.kt | 34 ++++++------------- 2 files changed, 37 insertions(+), 24 deletions(-) create mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleExecution.kt diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleExecution.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleExecution.kt new file mode 100644 index 00000000..314c4b95 --- /dev/null +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleExecution.kt @@ -0,0 +1,27 @@ +package dev.bpmcrafters.processengineapi.impl.task + +/** + * Runs the [execution] at most once and remembers whether it ran and whether it failed. + * A failure of the execution is rethrown unchanged. + */ +internal class SingleExecution( + private val execution: Runnable, + private val onRepeatedRun: () -> Unit +) : Runnable { + + private val result = lazy { runCatching { execution.run() } } + + val hasRun: Boolean + get() = result.isInitialized() + + val hasFailed: Boolean + get() = hasRun && result.value.isFailure + + override fun run() { + if (hasRun) { + onRepeatedRun() + } else { + result.value.getOrThrow() + } + } +} diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt index 3eb7886b..91c36442 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt @@ -23,35 +23,20 @@ class TaskExecutionDecoration(private val decorators: List, execution: Runnable) { - val executionResult = resultOfSingleRun(execution) - runDecorated(task, payload, executionResult) - if (!executionResult.isInitialized()) { - runUndecorated(task, execution) - } - } - - private fun resultOfSingleRun(execution: Runnable) = lazy { - runCatching { execution.run() } - } + val singleExecution = SingleExecution(execution) { warnRepeatedRun(task) } - private fun runDecorated(task: TaskInformation, payload: Map, executionResult: Lazy>) { try { - val executionOnce = runOnce(task, executionResult) - val decoratedExecution = decorate(readOnly(task), Collections.unmodifiableMap(payload), executionOnce) + val decoratedExecution = decorate(readOnly(task), Collections.unmodifiableMap(payload), singleExecution) decoratedExecution.run() } catch (e: Exception) { - if (executionResult.isFailure()) throw e + if (singleExecution.hasFailed) throw e logDecoratorFailure(task, e) } - } - - private fun runOnce(task: TaskInformation, executionResult: Lazy>) = Runnable { - if (executionResult.isInitialized()) warnRepeatedRun(task) else executionResult.value.getOrThrow() - } - private fun runUndecorated(task: TaskInformation, execution: Runnable) { - logger.warn { "PROCESS-ENGINE-API-017: Decorators did not run task ${task.taskId}, running it without them." } - execution.run() + if (!singleExecution.hasRun) { + warnSkippedRun(task) + singleExecution.run() + } } private fun decorate(task: TaskInformation, payload: Map, execution: Runnable): Runnable = @@ -64,13 +49,14 @@ class TaskExecutionDecoration(private val decorators: List>.isFailure() = isInitialized() && value.isFailure - private fun readOnly(task: TaskInformation) = task.copy(meta = Collections.unmodifiableMap(task.meta)) private fun warnRepeatedRun(task: TaskInformation) = logger.warn { "PROCESS-ENGINE-API-018: Decorator ran task ${task.taskId} again, ignoring the repeated run." } + private fun warnSkippedRun(task: TaskInformation) = + logger.warn { "PROCESS-ENGINE-API-017: Decorators did not run task ${task.taskId}, running it without them." } + private fun logDecoratorFailure(task: TaskInformation, e: Exception) = logger.error(e) { "PROCESS-ENGINE-API-016: Task execution decorator failed for task ${task.taskId}." } From 92e53095dbff5bb93b03aacde6416866eb859363 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 3 Nov 2026 13:43:40 +0100 Subject: [PATCH 9/9] refactor(api): name the single task execution after what it runs --- .../impl/task/SingleExecution.kt | 27 ------------------ .../impl/task/SingleTaskExecution.kt | 28 +++++++++++++++++++ .../impl/task/TaskExecutionDecoration.kt | 10 +++---- 3 files changed, 33 insertions(+), 32 deletions(-) delete mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleExecution.kt create mode 100644 api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleTaskExecution.kt diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleExecution.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleExecution.kt deleted file mode 100644 index 314c4b95..00000000 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleExecution.kt +++ /dev/null @@ -1,27 +0,0 @@ -package dev.bpmcrafters.processengineapi.impl.task - -/** - * Runs the [execution] at most once and remembers whether it ran and whether it failed. - * A failure of the execution is rethrown unchanged. - */ -internal class SingleExecution( - private val execution: Runnable, - private val onRepeatedRun: () -> Unit -) : Runnable { - - private val result = lazy { runCatching { execution.run() } } - - val hasRun: Boolean - get() = result.isInitialized() - - val hasFailed: Boolean - get() = hasRun && result.value.isFailure - - override fun run() { - if (hasRun) { - onRepeatedRun() - } else { - result.value.getOrThrow() - } - } -} diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleTaskExecution.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleTaskExecution.kt new file mode 100644 index 00000000..9debc1e4 --- /dev/null +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/SingleTaskExecution.kt @@ -0,0 +1,28 @@ +package dev.bpmcrafters.processengineapi.impl.task + +/** + * Runs the execution of one delivered task at most once. + * The task execution is what the adapter delivery does for this task: calling the task handler and handling its failure. + * Remembers whether the task execution ran and whether it failed; a failure is rethrown unchanged. + */ +internal class SingleTaskExecution( + private val taskExecution: Runnable, + private val onRepeatedRun: () -> Unit +) : Runnable { + + private val result = lazy { runCatching { taskExecution.run() } } + + val hasRun: Boolean + get() = result.isInitialized() + + val hasFailed: Boolean + get() = hasRun && result.value.isFailure + + override fun run() { + if (hasRun) { + onRepeatedRun() + } else { + result.value.getOrThrow() + } + } +} diff --git a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt index 91c36442..1dea4575 100644 --- a/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt +++ b/api-impl/src/main/kotlin/dev/bpmcrafters/processengineapi/impl/task/TaskExecutionDecoration.kt @@ -23,19 +23,19 @@ class TaskExecutionDecoration(private val decorators: List, execution: Runnable) { - val singleExecution = SingleExecution(execution) { warnRepeatedRun(task) } + val singleTaskExecution = SingleTaskExecution(execution) { warnRepeatedRun(task) } try { - val decoratedExecution = decorate(readOnly(task), Collections.unmodifiableMap(payload), singleExecution) + val decoratedExecution = decorate(readOnly(task), Collections.unmodifiableMap(payload), singleTaskExecution) decoratedExecution.run() } catch (e: Exception) { - if (singleExecution.hasFailed) throw e + if (singleTaskExecution.hasFailed) throw e logDecoratorFailure(task, e) } - if (!singleExecution.hasRun) { + if (!singleTaskExecution.hasRun) { warnSkippedRun(task) - singleExecution.run() + singleTaskExecution.run() } }