From f828d24f8ab8fbedb9e301ba7b5e8e376dea92e0 Mon Sep 17 00:00:00 2001 From: Paul Sterl Date: Sat, 4 Jan 2025 10:00:04 +0100 Subject: [PATCH 1/8] simpler class hierarchie and added test to find the Transactional annotation --- README.md | 4 +- .../api/AddTriggerRequest.java | 2 +- .../persistent_tasks/api/PersistentTask.java | 31 ++++ .../persistent_tasks/api/SpringBeanTask.java | 27 +--- .../spring/persistent_tasks/api/Task.java | 13 -- .../spring/persistent_tasks/api/TaskId.java | 2 +- .../persistent_tasks/api/TriggerKey.java | 4 +- .../api/event/TriggerTaskCommand.java | 2 +- .../scheduler/SchedulerService.java | 4 +- .../scheduler/SchedulerTimer.java | 2 +- .../persistent_tasks/task/TaskService.java | 61 ++++---- .../task/config/TaskConfig.java | 27 ++-- .../task/model/RegisteredTask.java | 45 ------ .../task/repository/TaskRepository.java | 47 +++---- .../trigger/TriggerService.java | 2 +- .../component/RunTriggerComponent.java | 10 +- .../trigger/model/TriggerEntity.java | 2 +- .../persistent_tasks/AbstractSpringTest.java | 12 +- .../TaskSchedulerServiceTest.java | 4 +- .../SchedulerServiceTransactionTest.java | 10 +- .../task/TaskServiceTest.java | 11 +- .../task/TransactionAnnotationTest.java | 133 ++++++++++++++++++ .../trigger/TriggerServiceTest.java | 2 +- .../spring/sample_app/person/PersonBE.java | 4 +- .../vehicle/task/FailingBuildVehicleTask.java | 2 +- .../SpringPersistentTasksUIConfig.java | 8 +- 26 files changed, 268 insertions(+), 203 deletions(-) create mode 100644 core/src/main/java/org/sterl/spring/persistent_tasks/api/PersistentTask.java delete mode 100644 core/src/main/java/org/sterl/spring/persistent_tasks/api/Task.java delete mode 100644 core/src/main/java/org/sterl/spring/persistent_tasks/task/model/RegisteredTask.java create mode 100644 core/src/test/java/org/sterl/spring/persistent_tasks/task/TransactionAnnotationTest.java diff --git a/README.md b/README.md index 61fd3012b..d68038060 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ public class ExampleApplication { @Component(BuildVehicleTask.NAME) @RequiredArgsConstructor @Slf4j -public class BuildVehicleTask implements SpringBeanTask { +public class BuildVehicleTask implements PersistentTask { private static final String NAME = "buildVehicleTask"; public static final TaskId ID = new TaskId<>(NAME); @@ -109,7 +109,7 @@ Simple task will use defaults: ```java @Bean -SpringBeanTask task1(VehicleHttpConnector vehicleHttpConnector) { +PersistentTask task1(VehicleHttpConnector vehicleHttpConnector) { return v -> vehicleHttpConnector.send(v); } ``` diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/api/AddTriggerRequest.java b/core/src/main/java/org/sterl/spring/persistent_tasks/api/AddTriggerRequest.java index e51e0d8b8..143b8c8a2 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/api/AddTriggerRequest.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/api/AddTriggerRequest.java @@ -5,7 +5,7 @@ /** /** - * For any registered task a task trigger represent one unit of work, executing this task once. + * For any registered persistentTask a persistentTask trigger represent one unit of work, executing this persistentTask once. * @param state type which has to be of {@link Serializable} */ public record AddTriggerRequest( diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/api/PersistentTask.java b/core/src/main/java/org/sterl/spring/persistent_tasks/api/PersistentTask.java new file mode 100644 index 000000000..9e595f497 --- /dev/null +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/api/PersistentTask.java @@ -0,0 +1,31 @@ +package org.sterl.spring.persistent_tasks.api; + +import java.io.Serializable; + +/** + * A spring persistentTask which state is saved in a {@link Trigger}. + * + * @param the state type + */ +@FunctionalInterface +public interface PersistentTask { + void accept(T state); + + default RetryStrategy retryStrategy() { + return RetryStrategy.THREE_RETRIES; + } + + /** + * Whether the persistentTask is transaction or not. If true the execution + * is wrapped into the default transaction template together with the state update + * and the following events: + *
    + *
  1. org.sterl.spring.persistent_tasks.trigger.event.TriggerRunningEvent
    2. + *
  2. org.sterl.spring.persistent_tasks.trigger.event.TriggerSuccessEvent
    3. + *
+ * @return {@code true} if the persistentTask is transactional; {@code false} otherwise. + */ + default boolean isTransactional() { + return false; + } +} diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/api/SpringBeanTask.java b/core/src/main/java/org/sterl/spring/persistent_tasks/api/SpringBeanTask.java index 83f302e4a..3953812fe 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/api/SpringBeanTask.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/api/SpringBeanTask.java @@ -1,29 +1,12 @@ package org.sterl.spring.persistent_tasks.api; import java.io.Serializable; -import java.util.function.Consumer; +/** + * Same as {@link PersistentTask} + */ +@Deprecated @FunctionalInterface -public interface SpringBeanTask extends Consumer { - @Override - void accept(T state); - - default RetryStrategy retryStrategy() { - return RetryStrategy.THREE_RETRIES; - } +public interface SpringBeanTask extends PersistentTask { - /** - * Whether the task is transaction or not. If true the execution - * is wrapped into the default transaction template together with the state update - * and the following events: - *
    - *
  1. org.sterl.spring.persistent_tasks.trigger.event.TriggerRunningEvent
    2. - *
  2. org.sterl.spring.persistent_tasks.trigger.event.TriggerSuccessEvent
    3. - *
  3. org.sterl.spring.persistent_tasks.trigger.event.TriggerFailedEvent
    4. - *
- * @return {@code true} if the task is transactional; {@code false} otherwise. - */ - default boolean isTransactional() { - return false; - } } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/api/Task.java b/core/src/main/java/org/sterl/spring/persistent_tasks/api/Task.java deleted file mode 100644 index 7cccd16a9..000000000 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/api/Task.java +++ /dev/null @@ -1,13 +0,0 @@ -package org.sterl.spring.persistent_tasks.api; - -import java.io.Serializable; - -import org.sterl.spring.persistent_tasks.api.TaskId.TaskTriggerBuilder; - -public interface Task extends SpringBeanTask { - TaskId getId(); - - default TaskTriggerBuilder newTrigger() { - return getId().newTrigger(); - } -} diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/api/TaskId.java b/core/src/main/java/org/sterl/spring/persistent_tasks/api/TaskId.java index 0daaf9601..1c631af8f 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/api/TaskId.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/api/TaskId.java @@ -8,7 +8,7 @@ import lombok.RequiredArgsConstructor; /** - * Represents the ID of a task, which is currently not running. + * Represents the ID of a persistentTask, which is currently not running. */ public record TaskId(String name) implements Serializable { diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/api/TriggerKey.java b/core/src/main/java/org/sterl/spring/persistent_tasks/api/TriggerKey.java index ce8c5cec8..881448cb1 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/api/TriggerKey.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/api/TriggerKey.java @@ -32,7 +32,7 @@ public TaskId toTaskId() { return new TaskId<>(taskName); } /** - * Builds a trigger for the given task name + * Builds a trigger for the given persistentTask name */ public TriggerKey(String taskName) { id = UUID.randomUUID().toString(); @@ -40,7 +40,7 @@ public TriggerKey(String taskName) { } /** - * Just triggers the given task to be executed using null as state. + * Just triggers the given persistentTask to be executed using null as state. */ public AddTriggerRequest newTrigger(TaskId taskId) { return newTrigger(taskId, null); diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/api/event/TriggerTaskCommand.java b/core/src/main/java/org/sterl/spring/persistent_tasks/api/event/TriggerTaskCommand.java index 5ce158a09..05485691e 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/api/event/TriggerTaskCommand.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/api/event/TriggerTaskCommand.java @@ -9,7 +9,7 @@ import org.sterl.spring.persistent_tasks.api.TaskId.TaskTriggerBuilder; /** - * An event to trigger one or multiple task executions + * An event to trigger one or multiple persistentTask executions */ public record TriggerTaskCommand(Collection> triggers) { diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerService.java b/core/src/main/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerService.java index 195153cd1..93070040c 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerService.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerService.java @@ -90,7 +90,7 @@ public Optional findStatus(String name) { } /** - * Simply triggers the next task which is now due to be executed + * Simply triggers the next persistentTask which is now due to be executed */ @NonNull public List> triggerNextTasks() { @@ -134,7 +134,7 @@ public Future runOrQueue( trigger = triggerService.markTriggersAsRunning(trigger, name); pingRegistry().addRunning(1); } else { - log.debug("Currently not enough free thread available {} of {} in use. Task {} queued.", + log.debug("Currently not enough free thread available {} of {} in use. PersistentTask {} queued.", taskExecutor.getFreeThreads(), taskExecutor.getMaxThreads(), trigger.getKey()); } return trigger; diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerTimer.java b/core/src/main/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerTimer.java index e2717cfd5..51d6d77f2 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerTimer.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerTimer.java @@ -35,7 +35,7 @@ void triggerNextTasks() { } } - @Scheduled(fixedDelayString = "${spring.persistent-tasks.poll-task-timeout:300}", timeUnit = TimeUnit.SECONDS) + @Scheduled(fixedDelayString = "${spring.persistent-tasks.poll-persistentTask-timeout:300}", timeUnit = TimeUnit.SECONDS) void rescheduleAbandonedTasks() { var timeout = OffsetDateTime.now().minus(taskTimeout); for (SchedulerService s : schedulerServices) { diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java index c96673af4..247aff93c 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java @@ -9,10 +9,8 @@ import org.springframework.lang.NonNull; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import org.sterl.spring.persistent_tasks.api.SpringBeanTask; -import org.sterl.spring.persistent_tasks.api.Task; +import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.api.TaskId; -import org.sterl.spring.persistent_tasks.task.model.RegisteredTask; import org.sterl.spring.persistent_tasks.task.repository.TaskRepository; import lombok.RequiredArgsConstructor; @@ -30,61 +28,54 @@ public Set> findAllTaskIds() { } - public Optional> get(TaskId id) { + public Optional> get(TaskId id) { return taskRepository.get(id); } /** - * Check if the {@link Task} is known or not. + * Check if the {@link PersistentTask} is known or not. * * @param the state type - * @param id the {@link TaskId} of the {@link Task} + * @param id the {@link TaskId} of the {@link PersistentTask} * @throws IllegalStateException if the id is unknown - * @return the {@link Task} registered to the given id + * @return the {@link PersistentTask} registered to the given id */ @NonNull - public Task assertIsKnown(@NonNull TaskId id) { + public PersistentTask assertIsKnown(@NonNull TaskId id) { final var task = taskRepository.get(id); if (task.isEmpty()) { - throw new IllegalStateException("Task with ID " + id + throw new IllegalStateException("PersistentTask with ID " + id + " is unknown. Known tasks: " + taskRepository.all()); } return task.get(); } /** - * A way to manually register a task, usually better to use {@link SpringBeanTask}. + * A way to manually register a persistentTask, usually better to use {@link PersistentTask}. */ - public TaskId register(String name, Consumer task) { - RegisteredTask t = new RegisteredTask<>(name, task); - return register(t); + public TaskId register(String name, Consumer task) { + return register(name, new PersistentTask() { + @Override + public void accept(Serializable state) { + task.accept(state); + } + }); } /** - * A way to manually register a task, usually not needed as spring beans will be added automatically. + * A way to manually register a persistentTask, usually not needed as spring beans will be added automatically. */ - public TaskId register(String name, SpringBeanTask task) { - RegisteredTask t = new RegisteredTask<>(name, task); - return register(t); + @SuppressWarnings("unchecked") + public TaskId register(String name, PersistentTask task) { + var id = (TaskId)TaskId.of(name); + return taskRepository.addTask(id, task); } /** - * A way to manually register a task, usually not needed as spring beans will be added anyway. + * A way to manually register a persistentTask, usually not needed as spring beans will be added automatically. */ - public TaskId register(RegisteredTask task) { - return taskRepository.addTask(task); - } - - /** - * A way to manually register a task, usually not needed as spring beans will be added anyway. - */ - public TaskId replace(RegisteredTask task) { - taskRepository.remove(task); - return register(task); - } - /** - * A way to manually register a task, usually not needed as spring beans will be added automatically. - */ - public TaskId replace(String name, SpringBeanTask task) { - RegisteredTask t = new RegisteredTask<>(name, task); - return replace(t); + @SuppressWarnings("unchecked") + public TaskId replace(String name, PersistentTask task) { + var id = (TaskId)TaskId.of(name); + taskRepository.remove(id); + return taskRepository.addTask(id, task); } } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java index c78389342..c496fbb10 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java @@ -1,14 +1,13 @@ package org.sterl.spring.persistent_tasks.task.config; +import java.io.Serializable; import java.util.Map.Entry; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.support.GenericBeanDefinition; import org.springframework.context.annotation.Configuration; import org.springframework.context.support.GenericApplicationContext; -import org.sterl.spring.persistent_tasks.api.SpringBeanTask; +import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.api.TaskId; -import org.sterl.spring.persistent_tasks.task.model.RegisteredTask; import org.sterl.spring.persistent_tasks.task.repository.TaskRepository; import lombok.extern.slf4j.Slf4j; @@ -20,22 +19,20 @@ public class TaskConfig { @Autowired void configureSimpleTasks(GenericApplicationContext context, TaskRepository taskRepository) { - final var simpleTasks = context.getBeansOfType(SpringBeanTask.class); - for(Entry t : simpleTasks.entrySet()) { - final var registeredTask = new RegisteredTask<>(t.getKey(), t.getValue()); - taskRepository.addTask(registeredTask); + final var simpleTasks = context.getBeansOfType(PersistentTask.class); + for(Entry t : simpleTasks.entrySet()) { + var id = taskRepository.addTask( + (TaskId)TaskId.of(t.getKey()), t.getValue()); - addTaskIdIfMissing(context, registeredTask); + addTaskIdIfMissing(context, id, t.getValue()); } } - private void addTaskIdIfMissing(GenericApplicationContext context, final RegisteredTask registeredTask) { - final var taskIdContextName = registeredTask.getId().name() + "Id"; + private void addTaskIdIfMissing(GenericApplicationContext context, + TaskId id, PersistentTask task) { + final var taskIdContextName = id.name() + "Id"; if (!context.containsBean(taskIdContextName)) { - log.info("Adding TaskId={} with name={} to spring context", registeredTask.getId(), taskIdContextName); - var beanDefinition = new GenericBeanDefinition(); - beanDefinition.setBeanClass(registeredTask.getId().getClass()); - context.registerBean(taskIdContextName, - TaskId.class, () -> registeredTask.getId()); + log.info("Adding {} with name={} to spring context", id, taskIdContextName); + context.registerBean(taskIdContextName, TaskId.class, () -> id); } } } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/model/RegisteredTask.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/model/RegisteredTask.java deleted file mode 100644 index e7245a629..000000000 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/task/model/RegisteredTask.java +++ /dev/null @@ -1,45 +0,0 @@ -package org.sterl.spring.persistent_tasks.task.model; - -import java.io.Serializable; -import java.util.function.Consumer; - -import org.sterl.spring.persistent_tasks.api.RetryStrategy; -import org.sterl.spring.persistent_tasks.api.SpringBeanTask; -import org.sterl.spring.persistent_tasks.api.Task; -import org.sterl.spring.persistent_tasks.api.TaskId; - -import lombok.Getter; - -public class RegisteredTask implements Task { - - @Getter - private final TaskId id; - private final SpringBeanTask fun; - @Getter - private RetryStrategy retryStrategy = RetryStrategy.THREE_RETRIES; - - public RegisteredTask(String name, SpringBeanTask fun) { - super(); - this.id = new TaskId<>(name); - this.fun = fun; - } - - public RegisteredTask(String name, Consumer consumer) { - this(name, s -> { - consumer.accept(s); - }); - } - - @Override - public void accept(T state) { - this.fun.accept(state); - } - @Override - public RetryStrategy retryStrategy() { - return this.fun.retryStrategy(); - } - @Override - public boolean isTransactional() { - return this.fun.isTransactional(); - } -} diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/repository/TaskRepository.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/repository/TaskRepository.java index 5e3d46e6b..fc8d3ee57 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/task/repository/TaskRepository.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/task/repository/TaskRepository.java @@ -2,7 +2,6 @@ import java.io.Serializable; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -10,7 +9,7 @@ import org.springframework.lang.NonNull; import org.springframework.stereotype.Component; -import org.sterl.spring.persistent_tasks.api.Task; +import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.api.TaskId; import lombok.extern.slf4j.Slf4j; @@ -18,56 +17,48 @@ @Slf4j @Component public class TaskRepository { - private final Map, Task> tasks = new ConcurrentHashMap<>(); + private final Map, PersistentTask> persistentTasks = new ConcurrentHashMap<>(); - public TaskRepository(List> tasks) { - super(); - for (Task task : tasks) { - addTask(task); - } - } - - @SuppressWarnings("unchecked") - public Task remove(Task task) { - if (task == null) { + public PersistentTask remove(TaskId taskId) { + if (taskId == null) { return null; } - return (Task)tasks.remove(task.getId()); + return persistentTasks.remove(taskId); } - public TaskId addTask(Task task) { - if (contains(task.getId())) { - throw new IllegalStateException("The task id " + task.getId() + " is already used!"); + public TaskId addTask(@NonNull TaskId taskId, PersistentTask task) { + if (contains(taskId)) { + throw new IllegalStateException("The " + taskId + " is already used!"); } - log.info("Adding task={} to={}", task.getId(), task.getClass()); - this.tasks.put(task.getId(), task); - return task.getId(); + log.info("Adding {} to={}", taskId, task.getClass()); + this.persistentTasks.put(taskId, task); + return taskId; } @NonNull @SuppressWarnings("unchecked") - public Optional> get(@NonNull TaskId taskId) { + public Optional> get(@NonNull TaskId taskId) { assert taskId != null; - return Optional.ofNullable((Task)tasks.get(taskId)); + return Optional.ofNullable((PersistentTask)persistentTasks.get(taskId)); } /** - * Removes all tasks, should only be used for testing! + * Removes all persistentTasks, should only be used for testing! */ public void clear() { - log.warn("*** All tasks {} will be removed now! ***", tasks.size()); - tasks.clear(); + log.warn("*** All persistentTasks {} will be removed now! ***", persistentTasks.size()); + persistentTasks.clear(); } public boolean contains(String name) { return contains(new TaskId<>(name)); } - public boolean contains(TaskId id) { - return tasks.containsKey(id); + public boolean contains(TaskId id) { + return persistentTasks.containsKey(id); } public Set> all() { - return new HashSet<>(tasks.keySet()); + return new HashSet<>(persistentTasks.keySet()); } } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/TriggerService.java b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/TriggerService.java index 782b2a372..9e1f66fa9 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/TriggerService.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/TriggerService.java @@ -124,7 +124,7 @@ public TriggerEntity queue(AddTriggerRequest tigger) } /** - * If you changed your mind, cancel the task + * If you changed your mind, cancel the persistentTask */ public Optional cancel(TriggerKey key) { return editTrigger.cancelTask(key); diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/RunTriggerComponent.java b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/RunTriggerComponent.java index de15e5308..cae05cbd3 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/RunTriggerComponent.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/RunTriggerComponent.java @@ -9,7 +9,7 @@ import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; import org.springframework.transaction.support.TransactionTemplate; -import org.sterl.spring.persistent_tasks.api.Task; +import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.task.TaskService; import org.sterl.spring.persistent_tasks.trigger.event.TriggerRunningEvent; import org.sterl.spring.persistent_tasks.trigger.model.TriggerEntity; @@ -66,17 +66,17 @@ private TaskAndState getTastAndState(TriggerEntity trigger) { } @RequiredArgsConstructor private class TaskAndState implements Callable> { - final Task task; + final PersistentTask persistentTask; final Serializable state; final TriggerEntity trigger; boolean isTransactional() { - return task.isTransactional(); + return persistentTask.isTransactional(); } public Optional call() { eventPublisher.publishEvent(new TriggerRunningEvent(trigger)); - task.accept(state); + persistentTask.accept(state); var result = editTrigger.completeTaskWithSuccess(trigger.getKey()); editTrigger.deleteTrigger(trigger); @@ -90,7 +90,7 @@ private Optional handleTaskException(TaskAndState taskAndState, @Nullable Exception e) { var trigger = taskAndState.trigger; - var task = taskAndState.task; + var task = taskAndState.persistentTask; var result = editTrigger.completeTaskWithStatus(trigger.getKey(), e); if (task != null && diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/model/TriggerEntity.java b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/model/TriggerEntity.java index a9879cce6..fe4a4e26c 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/model/TriggerEntity.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/model/TriggerEntity.java @@ -69,7 +69,7 @@ public TriggerKey getKey() { public TriggerEntity cancel() { this.data.setEnd(OffsetDateTime.now()); this.data.setStatus(TriggerStatus.CANCELED); - this.data.setExceptionName("Task canceled"); + this.data.setExceptionName("PersistentTask canceled"); this.data.setRunningDurationInMs(null); return this; } diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java index 1606ed4ea..0547f2018 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java @@ -17,7 +17,7 @@ import org.springframework.stereotype.Component; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.support.TransactionTemplate; -import org.sterl.spring.persistent_tasks.api.SpringBeanTask; +import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.api.TaskId; import org.sterl.spring.persistent_tasks.api.event.TriggerTaskCommand; import org.sterl.spring.persistent_tasks.history.HistoryService; @@ -111,10 +111,10 @@ SchedulerService schedulerB(TriggerService triggerService, EditSchedulerStatusCo } /** - * This task will trigger task2 + * This persistentTask will trigger task2 */ @Bean - SpringBeanTask task1(ApplicationEventPublisher publisher, AsyncAsserts asserts) { + PersistentTask task1(ApplicationEventPublisher publisher, AsyncAsserts asserts) { return (String state) -> { asserts.info("task1::" + state); publisher.publishEvent(TriggerTaskCommand.of("task2", "task1::" + state)); @@ -122,13 +122,13 @@ SpringBeanTask task1(ApplicationEventPublisher publisher, AsyncAsserts a } @Bean - SpringBeanTask task2(AsyncAsserts asserts) { + PersistentTask task2(AsyncAsserts asserts) { return state -> asserts.info("task2::" + state); } @Component(Task3.NAME) @RequiredArgsConstructor - public static class Task3 implements SpringBeanTask { + public static class Task3 implements PersistentTask { public static final String NAME = "task3"; public static final TaskId ID = new TaskId<>(NAME); @@ -141,7 +141,7 @@ public void accept(String state) { } @Bean - SpringBeanTask slowTask(AsyncAsserts asserts) { + PersistentTask slowTask(AsyncAsserts asserts) { return sleepTime -> { try { if (sleepTime == null) { diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/TaskSchedulerServiceTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/TaskSchedulerServiceTest.java index fb4fd2332..8a42eb481 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/TaskSchedulerServiceTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/TaskSchedulerServiceTest.java @@ -7,7 +7,7 @@ import org.junit.jupiter.api.Test; import org.sterl.spring.persistent_tasks.api.RetryStrategy; -import org.sterl.spring.persistent_tasks.api.SpringBeanTask; +import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.api.TaskId; import org.sterl.spring.persistent_tasks.shared.model.TriggerStatus; @@ -17,7 +17,7 @@ class TaskSchedulerServiceTest extends AbstractSpringTest { void testFailedTasksAreRetried() throws Exception { // GIVEN TaskId task = taskService.replace("foo", - new SpringBeanTask() { + new PersistentTask() { @Override public void accept(String state) { asserts.info(state); diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java index fcf90639e..55b000f87 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java @@ -2,7 +2,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.time.temporal.ChronoUnit; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicBoolean; @@ -14,8 +13,7 @@ import org.springframework.transaction.annotation.Transactional; import org.sterl.spring.persistent_tasks.AbstractSpringTest; import org.sterl.spring.persistent_tasks.api.RetryStrategy; -import org.sterl.spring.persistent_tasks.api.RetryStrategy.MultiplicativeRetryStrategy; -import org.sterl.spring.persistent_tasks.api.SpringBeanTask; +import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.api.TaskId.TaskTriggerBuilder; import org.sterl.spring.persistent_tasks.api.TriggerKey; import org.sterl.spring.persistent_tasks.shared.model.TriggerStatus; @@ -32,8 +30,8 @@ class SchedulerServiceTransactionTest extends AbstractSpringTest { @Configuration static class Config { @Bean - SpringBeanTask savePerson(PersonRepository personRepository) { - return new SpringBeanTask<>() { + PersistentTask savePerson(PersonRepository personRepository) { + return new PersistentTask<>() { @Transactional @Override public void accept(String name) { @@ -116,6 +114,8 @@ void testRollbackAndRetry() throws Exception { // GIVEN final var triggerRequest = TaskTriggerBuilder.newTrigger("savePerson").state("Paul").build(); sendError.set(true); + inTrx.set(true); + // WHEN var key = subject.runOrQueue(triggerRequest); // THEN diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskServiceTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskServiceTest.java index 10f9fc620..73043f13b 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskServiceTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskServiceTest.java @@ -2,27 +2,22 @@ import static org.junit.jupiter.api.Assertions.assertThrows; -import java.util.ArrayList; - import org.junit.jupiter.api.Test; import org.sterl.spring.persistent_tasks.api.TaskId; -import org.sterl.spring.persistent_tasks.task.model.RegisteredTask; import org.sterl.spring.persistent_tasks.task.repository.TaskRepository; class TaskServiceTest { - private final TaskService subject = new TaskService(new TaskRepository(new ArrayList<>())); + private final TaskService subject = new TaskService(new TaskRepository()); @Test void testAssertIsKnown() { // GIVEN - RegisteredTask t = new RegisteredTask<>("foo", (s) -> {}); - // WHEN - subject.replace(t); + var id = subject.replace("foo", (s) -> {}); // THEN - subject.assertIsKnown(t.getId()); + subject.assertIsKnown(id); subject.assertIsKnown(new TaskId("foo")); assertThrows(IllegalStateException.class, () -> subject.assertIsKnown(new TaskId("1"))); } diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TransactionAnnotationTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TransactionAnnotationTest.java new file mode 100644 index 000000000..ff26c89cd --- /dev/null +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TransactionAnnotationTest.java @@ -0,0 +1,133 @@ +package org.sterl.spring.persistent_tasks.task; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.Serializable; +import java.lang.annotation.Annotation; + +import org.junit.jupiter.api.Test; +import org.springframework.aop.framework.AopProxyUtils; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.DefaultTransactionDefinition; +import org.springframework.util.ReflectionUtils; +import org.sterl.spring.persistent_tasks.AbstractSpringTest; +import org.sterl.spring.persistent_tasks.api.PersistentTask; +import org.sterl.spring.sample_app.person.PersonBE; +import org.sterl.spring.sample_app.person.PersonRepository; + +import lombok.RequiredArgsConstructor; + +class TransactionAnnotationTest extends AbstractSpringTest { + @Component("transactionalClass") + @Transactional(timeout = 5, propagation = Propagation.MANDATORY) + @RequiredArgsConstructor + static class TransactionalClass implements PersistentTask { + private final PersonRepository personRepository; + @Override + public void accept(PersonBE state) { + personRepository.save(state); + } + } + @Component("transactionalMethod") + @RequiredArgsConstructor + static class TransactionalMethod implements PersistentTask { + private final PersonRepository personRepository; + @Transactional(timeout = 6, propagation = Propagation.MANDATORY) + @Override + public void accept(PersonBE state) { + personRepository.save(state); + } + } + + /** + * A closure cannot be annotated, so we use a anonymous class + */ + @Configuration + static class Config { + @Bean("transactionalClosure") + PersistentTask transactionalClosure(PersonRepository personRepository) { + return new PersistentTask() { + @Transactional(timeout = 7, propagation = Propagation.MANDATORY) + @Override + public void accept(PersonBE state) { + personRepository.save(state); + } + }; + } + } + + @Autowired @Qualifier("transactionalClass") + PersistentTask transactionalClass; + @Autowired @Qualifier("transactionalMethod") + PersistentTask transactionalMethod; + @Autowired @Qualifier("transactionalMethod") + PersistentTask transactionalClosure; + + @Test + void testFindTransactionAnnotation() throws Exception { + var a = getTrxAnnotation(transactionalClass.getClass(), Transactional.class); + assertThat(a).isNotNull(); + assertThat(a.timeout()).isEqualTo(5); + + a = getTrxAnnotation(transactionalMethod.getClass(), Transactional.class); + assertThat(a).isNotNull(); + assertThat(a.timeout()).isEqualTo(6); + + a = getTrxAnnotation(transactionalClosure.getClass(), Transactional.class); + assertThat(a).isNotNull(); + assertThat(a.timeout()).isEqualTo(6); + } + + @Test + void testA() { + // Resolve the actual target class + var targetClass = AopProxyUtils.ultimateTargetClass(transactionalMethod); // ; + + // Retrieve the method and its @Transactional annotation + //var targetMethod = targetClass.getMethod("accept", Serializable.class); + var targetMethod = ReflectionUtils.findMethod(targetClass, "accept", Serializable.class); + var transactionalMethodAnnotation = AnnotationUtils.findAnnotation(targetMethod, Transactional.class); + System.out.println("Method-level Transactional Annotation: " + transactionalMethodAnnotation); + + // Retrieve the class-level @Transactional annotation + var transactionalClassAnnotation = AnnotationUtils.findAnnotation(targetClass, Transactional.class); + System.out.println("Class-level Transactional Annotation: " + transactionalClassAnnotation); + + DefaultTransactionDefinition def = new DefaultTransactionDefinition(); + } + + public A getTrxAnnotation(Class inTask, Class searchFor) { + var task = AopProxyUtils.ultimateTargetClass(inTask); + A result = AnnotationUtils.findAnnotation(task, searchFor); + if (result != null) return result; + + var targetMethod = ReflectionUtils.findMethod(task, "accept", Serializable.class); + if (targetMethod == null) return null; + + result = AnnotationUtils.findAnnotation(targetMethod, searchFor); + return result; + } + + public static DefaultTransactionDefinition convertTransactionalToDefinition(Transactional transactional) { + DefaultTransactionDefinition def = new DefaultTransactionDefinition(); + + // Map Transactional attributes to DefaultTransactionDefinition + def.setIsolationLevel(transactional.isolation().value()); + def.setPropagationBehavior(transactional.propagation().value()); + def.setTimeout(transactional.timeout()); + def.setReadOnly(transactional.readOnly()); + // No direct mapping for 'rollbackFor' or 'noRollbackFor' + // Set a name if desired (e.g., based on transactional class/method) + def.setName("TransactionalDefinition"); + + return def; + } + +} diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/trigger/TriggerServiceTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/trigger/TriggerServiceTest.java index b8f756996..1d7b0401e 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/trigger/TriggerServiceTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/trigger/TriggerServiceTest.java @@ -32,7 +32,7 @@ class TriggerServiceTest extends AbstractSpringTest { @Autowired private TaskRepository taskRepository; - // ensure task in the spring context + // ensure persistentTask in the spring context @Autowired private TaskId task1Id; @Autowired diff --git a/core/src/test/java/org/sterl/spring/sample_app/person/PersonBE.java b/core/src/test/java/org/sterl/spring/sample_app/person/PersonBE.java index e9a5eee39..4cd12c398 100644 --- a/core/src/test/java/org/sterl/spring/sample_app/person/PersonBE.java +++ b/core/src/test/java/org/sterl/spring/sample_app/person/PersonBE.java @@ -1,5 +1,7 @@ package org.sterl.spring.sample_app.person; +import java.io.Serializable; + import jakarta.persistence.Entity; import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; @@ -10,7 +12,7 @@ @Table @Entity @Data @NoArgsConstructor -public class PersonBE { +public class PersonBE implements Serializable { @Id @GeneratedValue diff --git a/example/src/main/java/org/sterl/spring/example_app/vehicle/task/FailingBuildVehicleTask.java b/example/src/main/java/org/sterl/spring/example_app/vehicle/task/FailingBuildVehicleTask.java index 22f3cdb37..45c89aba3 100644 --- a/example/src/main/java/org/sterl/spring/example_app/vehicle/task/FailingBuildVehicleTask.java +++ b/example/src/main/java/org/sterl/spring/example_app/vehicle/task/FailingBuildVehicleTask.java @@ -28,6 +28,6 @@ public void accept(Vehicle vehicle) { } catch (InterruptedException e) { Thread.interrupted(); } - throw new RuntimeException("This task will always fail!"); + throw new RuntimeException("This persistentTask will always fail!"); } } diff --git a/ui/src/main/java/org/sterl/spring/persistent_tasks_ui/SpringPersistentTasksUIConfig.java b/ui/src/main/java/org/sterl/spring/persistent_tasks_ui/SpringPersistentTasksUIConfig.java index cb40c7f1b..f60801e99 100644 --- a/ui/src/main/java/org/sterl/spring/persistent_tasks_ui/SpringPersistentTasksUIConfig.java +++ b/ui/src/main/java/org/sterl/spring/persistent_tasks_ui/SpringPersistentTasksUIConfig.java @@ -12,14 +12,14 @@ public class SpringPersistentTasksUIConfig implements WebMvcConfigurer { @Override public void addViewControllers(ViewControllerRegistry registry) { - registry.addViewController("/task-ui").setViewName("/task-ui/index.html"); - //registry.addRedirectViewController("/task-ui/", "/task-ui"); + registry.addViewController("/persistentTask-ui").setViewName("/persistentTask-ui/index.html"); + //registry.addRedirectViewController("/persistentTask-ui/", "/persistentTask-ui"); } @Override public void addResourceHandlers(ResourceHandlerRegistry registry) { - registry.addResourceHandler("/task-ui/assets/**") - .addResourceLocations("classpath:/static/task-ui/assets/") + registry.addResourceHandler("/persistentTask-ui/assets/**") + .addResourceLocations("classpath:/static/persistentTask-ui/assets/") .setCacheControl(CacheControl.maxAge(90, TimeUnit.DAYS)); } } From 8cf01aa2b6a9e45a5ddc143e926b904d22ad717e Mon Sep 17 00:00:00 2001 From: Paul Sterl Date: Sat, 4 Jan 2025 22:30:12 +0100 Subject: [PATCH 2/8] added transaction behavior test --- .../persistent_tasks/api/PersistentTask.java | 9 +- .../api/TransactionalTask.java | 30 ++++ .../persistent_tasks/task/TaskService.java | 1 - .../task/util/ReflectionUtil.java | 24 ++++ .../persistent_tasks/AbstractSpringTest.java | 2 +- .../task/TaskTransactionTest.java | 129 +++++++++++++++++ .../task/TransactionAnnotationTest.java | 133 ------------------ .../trigger/TriggerServiceTest.java | 4 + ui/src/server-api.d.ts | 20 +-- 9 files changed, 205 insertions(+), 147 deletions(-) create mode 100644 core/src/main/java/org/sterl/spring/persistent_tasks/api/TransactionalTask.java create mode 100644 core/src/main/java/org/sterl/spring/persistent_tasks/task/util/ReflectionUtil.java create mode 100644 core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java delete mode 100644 core/src/test/java/org/sterl/spring/persistent_tasks/task/TransactionAnnotationTest.java diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/api/PersistentTask.java b/core/src/main/java/org/sterl/spring/persistent_tasks/api/PersistentTask.java index 9e595f497..a14d2bdbf 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/api/PersistentTask.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/api/PersistentTask.java @@ -3,9 +3,12 @@ import java.io.Serializable; /** - * A spring persistentTask which state is saved in a {@link Trigger}. - * - * @param the state type + * A Spring persistent task whose state is saved in a {@link Trigger}. + * + *

This interface defines a task that accepts a state of type T and + * provides default implementations for retry strategies. + * + * @param the type of the state, which must be {@link Serializable} */ @FunctionalInterface public interface PersistentTask { diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/api/TransactionalTask.java b/core/src/main/java/org/sterl/spring/persistent_tasks/api/TransactionalTask.java new file mode 100644 index 000000000..f8abd2948 --- /dev/null +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/api/TransactionalTask.java @@ -0,0 +1,30 @@ +package org.sterl.spring.persistent_tasks.api; + +import java.io.Serializable; + +/** + * Similar to {@link PersistentTask} but specifically for transactional workloads. + * Use this interface when the task execution should be wrapped in a transaction. + * + *

This interface ensures that the task's execution is transactional, meaning that it will + * be executed within a transaction context, along with the state update and the dispatching of + * relevant events. + * + * @param the type of the state, which must be {@link Serializable} + */ +@FunctionalInterface +public interface TransactionalTask extends PersistentTask { + /** + * Whether the persistentTask is transaction or not. If true the execution + * is wrapped into the default transaction template together with the state update + * and the following events: + *

    + *
  1. org.sterl.spring.persistent_tasks.trigger.event.TriggerRunningEvent
    2. + *
  2. org.sterl.spring.persistent_tasks.trigger.event.TriggerSuccessEvent
    3. + *
+ * @return {@code true} if the persistentTask is transactional; {@code false} otherwise. + */ + default boolean isTransactional() { + return true; + } +} diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java index 247aff93c..ffa758694 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java @@ -27,7 +27,6 @@ public Set> findAllTaskIds() { return this.taskRepository.all(); } - public Optional> get(TaskId id) { return taskRepository.get(id); } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/util/ReflectionUtil.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/util/ReflectionUtil.java new file mode 100644 index 000000000..47726215d --- /dev/null +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/task/util/ReflectionUtil.java @@ -0,0 +1,24 @@ +package org.sterl.spring.persistent_tasks.task.util; + +import java.io.Serializable; +import java.lang.annotation.Annotation; + +import org.springframework.aop.framework.AopProxyUtils; +import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.util.ReflectionUtils; +import org.sterl.spring.persistent_tasks.api.PersistentTask; + +public abstract class ReflectionUtil { + + public static A getAnnotation(PersistentTask inTask, Class searchFor) { + var task = AopProxyUtils.ultimateTargetClass(inTask); + A result = AnnotationUtils.findAnnotation(task, searchFor); + if (result != null) return result; + + var targetMethod = ReflectionUtils.findMethod(task, "accept", Serializable.class); + if (targetMethod == null) return null; + + result = AnnotationUtils.findAnnotation(targetMethod, searchFor); + return result; + } +} diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java index 0547f2018..6e943192b 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java @@ -163,7 +163,6 @@ protected Optional runNextTrigger() { @BeforeEach public void beforeEach() throws Exception { - hibernateAsserts.reset(); triggerService.deleteAll(); historyService.deleteAll(); asserts.clear(); @@ -171,6 +170,7 @@ public void beforeEach() throws Exception { schedulerB.setMaxThreads(20); schedulerA.start(); schedulerB.start(); + hibernateAsserts.reset(); } @AfterEach diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java new file mode 100644 index 000000000..f3e404695 --- /dev/null +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java @@ -0,0 +1,129 @@ +package org.sterl.spring.persistent_tasks.task; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.DefaultTransactionDefinition; +import org.sterl.spring.persistent_tasks.AbstractSpringTest; +import org.sterl.spring.persistent_tasks.api.PersistentTask; +import org.sterl.spring.persistent_tasks.api.TaskId.TaskTriggerBuilder; +import org.sterl.spring.persistent_tasks.api.TransactionalTask; +import org.sterl.spring.persistent_tasks.task.util.ReflectionUtil; +import org.sterl.spring.sample_app.person.PersonBE; +import org.sterl.spring.sample_app.person.PersonRepository; + +import lombok.RequiredArgsConstructor; + +class TaskTransactionTest extends AbstractSpringTest { + + @Component("transactionalClass") + @Transactional(timeout = 5, propagation = Propagation.MANDATORY) + @RequiredArgsConstructor + static class TransactionalClass implements PersistentTask { + private final PersonRepository personRepository; + @Override + public void accept(String name) { + personRepository.save(new PersonBE(name)); + } + } + @Component("transactionalMethod") + @RequiredArgsConstructor + static class TransactionalMethod implements PersistentTask { + private final PersonRepository personRepository; + @Transactional(timeout = 6, propagation = Propagation.MANDATORY) + @Override + public void accept(String name) { + personRepository.save(new PersonBE(name)); + } + } + + /** + * A closure cannot be annotated, so we use a anonymous class + */ + @Configuration + static class Config { + @Bean("transactionalAnonymous") + PersistentTask transactionalAnonymous(PersonRepository personRepository) { + return new PersistentTask() { + @Transactional(timeout = 7, propagation = Propagation.MANDATORY) + @Override + public void accept(String name) { + personRepository.save(new PersonBE(name)); + } + }; + } + @Bean("transactionalClosure") + TransactionalTask transactionalClosure(PersonRepository personRepository) { + return name -> { + personRepository.save(new PersonBE(name)); + personRepository.save(new PersonBE(name)); + }; + } + } + + @Autowired PersonRepository personRepository; + + @Autowired @Qualifier("transactionalClass") + PersistentTask transactionalClass; + @Autowired @Qualifier("transactionalMethod") + PersistentTask transactionalMethod; + @Autowired @Qualifier("transactionalAnonymous") + PersistentTask transactionalAnonymous; + + @Test + void testFindTransactionAnnotation() { + var a = ReflectionUtil.getAnnotation(transactionalClass, Transactional.class); + assertThat(a).isNotNull(); + assertThat(a.timeout()).isEqualTo(5); + + a = ReflectionUtil.getAnnotation(transactionalMethod, Transactional.class); + assertThat(a).isNotNull(); + assertThat(a.timeout()).isEqualTo(6); + + a = ReflectionUtil.getAnnotation(transactionalAnonymous, Transactional.class); + assertThat(a).isNotNull(); + assertThat(a.timeout()).isEqualTo(7); + } + + @ParameterizedTest + @ValueSource(strings = {"transactionalClass", "transactionalMethod", "transactionalClosure"}) + void testTransactionalTask(String task) { + // GIVEN + var t = triggerService.queue(TaskTriggerBuilder + .newTrigger(task, "test").build()); + + // WHEN + personRepository.deleteAllInBatch(); + hibernateAsserts.reset(); + triggerService.run(t).get(); + + // THEN + hibernateAsserts.assertTrxCount(1); + assertThat(personRepository.count()).isEqualTo(2); + } + + public static DefaultTransactionDefinition convertTransactionalToDefinition(Transactional transactional) { + DefaultTransactionDefinition def = new DefaultTransactionDefinition(); + + // Map Transactional attributes to DefaultTransactionDefinition + def.setIsolationLevel(transactional.isolation().value()); + def.setPropagationBehavior(transactional.propagation().value()); + def.setTimeout(transactional.timeout()); + def.setReadOnly(transactional.readOnly()); + // No direct mapping for 'rollbackFor' or 'noRollbackFor' + // Set a name if desired (e.g., based on transactional class/method) + def.setName("TransactionalDefinition"); + + return def; + } + +} diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TransactionAnnotationTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TransactionAnnotationTest.java deleted file mode 100644 index ff26c89cd..000000000 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TransactionAnnotationTest.java +++ /dev/null @@ -1,133 +0,0 @@ -package org.sterl.spring.persistent_tasks.task; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.io.Serializable; -import java.lang.annotation.Annotation; - -import org.junit.jupiter.api.Test; -import org.springframework.aop.framework.AopProxyUtils; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.core.annotation.AnnotationUtils; -import org.springframework.stereotype.Component; -import org.springframework.transaction.annotation.Propagation; -import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.support.DefaultTransactionDefinition; -import org.springframework.util.ReflectionUtils; -import org.sterl.spring.persistent_tasks.AbstractSpringTest; -import org.sterl.spring.persistent_tasks.api.PersistentTask; -import org.sterl.spring.sample_app.person.PersonBE; -import org.sterl.spring.sample_app.person.PersonRepository; - -import lombok.RequiredArgsConstructor; - -class TransactionAnnotationTest extends AbstractSpringTest { - @Component("transactionalClass") - @Transactional(timeout = 5, propagation = Propagation.MANDATORY) - @RequiredArgsConstructor - static class TransactionalClass implements PersistentTask { - private final PersonRepository personRepository; - @Override - public void accept(PersonBE state) { - personRepository.save(state); - } - } - @Component("transactionalMethod") - @RequiredArgsConstructor - static class TransactionalMethod implements PersistentTask { - private final PersonRepository personRepository; - @Transactional(timeout = 6, propagation = Propagation.MANDATORY) - @Override - public void accept(PersonBE state) { - personRepository.save(state); - } - } - - /** - * A closure cannot be annotated, so we use a anonymous class - */ - @Configuration - static class Config { - @Bean("transactionalClosure") - PersistentTask transactionalClosure(PersonRepository personRepository) { - return new PersistentTask() { - @Transactional(timeout = 7, propagation = Propagation.MANDATORY) - @Override - public void accept(PersonBE state) { - personRepository.save(state); - } - }; - } - } - - @Autowired @Qualifier("transactionalClass") - PersistentTask transactionalClass; - @Autowired @Qualifier("transactionalMethod") - PersistentTask transactionalMethod; - @Autowired @Qualifier("transactionalMethod") - PersistentTask transactionalClosure; - - @Test - void testFindTransactionAnnotation() throws Exception { - var a = getTrxAnnotation(transactionalClass.getClass(), Transactional.class); - assertThat(a).isNotNull(); - assertThat(a.timeout()).isEqualTo(5); - - a = getTrxAnnotation(transactionalMethod.getClass(), Transactional.class); - assertThat(a).isNotNull(); - assertThat(a.timeout()).isEqualTo(6); - - a = getTrxAnnotation(transactionalClosure.getClass(), Transactional.class); - assertThat(a).isNotNull(); - assertThat(a.timeout()).isEqualTo(6); - } - - @Test - void testA() { - // Resolve the actual target class - var targetClass = AopProxyUtils.ultimateTargetClass(transactionalMethod); // ; - - // Retrieve the method and its @Transactional annotation - //var targetMethod = targetClass.getMethod("accept", Serializable.class); - var targetMethod = ReflectionUtils.findMethod(targetClass, "accept", Serializable.class); - var transactionalMethodAnnotation = AnnotationUtils.findAnnotation(targetMethod, Transactional.class); - System.out.println("Method-level Transactional Annotation: " + transactionalMethodAnnotation); - - // Retrieve the class-level @Transactional annotation - var transactionalClassAnnotation = AnnotationUtils.findAnnotation(targetClass, Transactional.class); - System.out.println("Class-level Transactional Annotation: " + transactionalClassAnnotation); - - DefaultTransactionDefinition def = new DefaultTransactionDefinition(); - } - - public A getTrxAnnotation(Class inTask, Class searchFor) { - var task = AopProxyUtils.ultimateTargetClass(inTask); - A result = AnnotationUtils.findAnnotation(task, searchFor); - if (result != null) return result; - - var targetMethod = ReflectionUtils.findMethod(task, "accept", Serializable.class); - if (targetMethod == null) return null; - - result = AnnotationUtils.findAnnotation(targetMethod, searchFor); - return result; - } - - public static DefaultTransactionDefinition convertTransactionalToDefinition(Transactional transactional) { - DefaultTransactionDefinition def = new DefaultTransactionDefinition(); - - // Map Transactional attributes to DefaultTransactionDefinition - def.setIsolationLevel(transactional.isolation().value()); - def.setPropagationBehavior(transactional.propagation().value()); - def.setTimeout(transactional.timeout()); - def.setReadOnly(transactional.readOnly()); - // No direct mapping for 'rollbackFor' or 'noRollbackFor' - // Set a name if desired (e.g., based on transactional class/method) - def.setName("TransactionalDefinition"); - - return def; - } - -} diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/trigger/TriggerServiceTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/trigger/TriggerServiceTest.java index 1d7b0401e..3656ee70a 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/trigger/TriggerServiceTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/trigger/TriggerServiceTest.java @@ -57,6 +57,10 @@ void testAddTrigger() throws Exception { final var triggerId = subject.queue(trigger).getKey(); // THEN + hibernateAsserts.assertTrxCount(1); + // one for the trigger and two for the history + hibernateAsserts.assertInsertCount(3); + // AND final var e = subject.get(triggerId); assertThat(e).isPresent(); assertThat(e.get().getData().getRunAt().toEpochSecond()).isEqualTo(triggerTime.toEpochSecond()); diff --git a/ui/src/server-api.d.ts b/ui/src/server-api.d.ts index 499c781e1..e04a567b7 100644 --- a/ui/src/server-api.d.ts +++ b/ui/src/server-api.d.ts @@ -34,6 +34,10 @@ export interface HistoryOverview { runningDurationInMs: number; } +export interface PersistentTask { + transactional: boolean; +} + export interface RetryStrategy { } @@ -43,12 +47,10 @@ export interface LinearRetryStrategy extends RetryStrategy { export interface MultiplicativeRetryStrategy extends RetryStrategy { } -export interface SpringBeanTask extends Consumer { - transactional: boolean; -} - -export interface Task extends SpringBeanTask { - id: TaskId; +/** + * @deprecated + */ +export interface SpringBeanTask extends PersistentTask { } export interface TaskId extends Serializable { @@ -58,6 +60,9 @@ export interface TaskId extends Serializable { export interface TaskTriggerBuilder { } +export interface TransactionalTask extends PersistentTask { +} + export interface Trigger { id: number; instanceId: number; @@ -98,7 +103,4 @@ export interface PageMetadata { export interface Serializable { } -export interface Consumer { -} - export type TriggerStatus = "WAITING" | "RUNNING" | "SUCCESS" | "FAILED" | "CANCELED"; From e5f79c24b91857d44107cb34852f058a63046c6d Mon Sep 17 00:00:00 2001 From: Paul Sterl Date: Sun, 5 Jan 2025 15:38:27 +0100 Subject: [PATCH 3/8] added detection for Transactional methods --- core/pom.xml | 2 +- .../persistent_tasks/task/TaskService.java | 18 ++++- .../component/TaskTransactionComponent.java | 81 +++++++++++++++++++ .../task/config/TaskConfig.java | 7 +- .../component/RunTriggerComponent.java | 42 +++++----- .../task/TaskServiceTest.java | 5 +- .../task/TaskTransactionTest.java | 42 +++++----- db/pom.xml | 2 +- example/pom.xml | 2 +- pom.xml | 2 +- ui/pom.xml | 2 +- 11 files changed, 154 insertions(+), 51 deletions(-) create mode 100644 core/src/main/java/org/sterl/spring/persistent_tasks/task/component/TaskTransactionComponent.java diff --git a/core/pom.xml b/core/pom.xml index d45f6ead2..bc3ab7a68 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.3.2-SNAPSHOT + 1.4.0-SNAPSHOT ../pom.xml diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java index ffa758694..9591d6cca 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/task/TaskService.java @@ -9,8 +9,10 @@ import org.springframework.lang.NonNull; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionTemplate; import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.api.TaskId; +import org.sterl.spring.persistent_tasks.task.component.TaskTransactionComponent; import org.sterl.spring.persistent_tasks.task.repository.TaskRepository; import lombok.RequiredArgsConstructor; @@ -20,6 +22,7 @@ @RequiredArgsConstructor public class TaskService { + private final TaskTransactionComponent taskTransactionComponent; private final TaskRepository taskRepository; @Transactional(readOnly = true) @@ -30,6 +33,11 @@ public Set> findAllTaskIds() { public Optional> get(TaskId id) { return taskRepository.get(id); } + + public Optional getTransactionTemplate( + PersistentTask task) { + return taskTransactionComponent.getTransactionTemplate(task); + } /** * Check if the {@link PersistentTask} is known or not. @@ -66,6 +74,14 @@ public void accept(Serializable state) { @SuppressWarnings("unchecked") public TaskId register(String name, PersistentTask task) { var id = (TaskId)TaskId.of(name); + return register(id, task); + } + /** + * A way to manually register a persistentTask, usually not needed as spring beans will be added automatically. + */ + public TaskId register(TaskId id, PersistentTask task) { + // init any transaction as needed + taskTransactionComponent.getTransactionTemplate(task); return taskRepository.addTask(id, task); } /** @@ -75,6 +91,6 @@ public TaskId register(String name, PersistentTask TaskId replace(String name, PersistentTask task) { var id = (TaskId)TaskId.of(name); taskRepository.remove(id); - return taskRepository.addTask(id, task); + return register(id, task); } } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/component/TaskTransactionComponent.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/component/TaskTransactionComponent.java new file mode 100644 index 000000000..bbe03f180 --- /dev/null +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/task/component/TaskTransactionComponent.java @@ -0,0 +1,81 @@ +package org.sterl.spring.persistent_tasks.task.component; + +import java.io.Serializable; +import java.util.EnumSet; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import org.springframework.stereotype.Component; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.DefaultTransactionDefinition; +import org.springframework.transaction.support.TransactionTemplate; +import org.sterl.spring.persistent_tasks.api.PersistentTask; +import org.sterl.spring.persistent_tasks.task.util.ReflectionUtil; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +@Component +@Slf4j +@RequiredArgsConstructor +public class TaskTransactionComponent { + + private final PlatformTransactionManager transactionManager; + private final TransactionTemplate template; + private final Set joinTransaction = EnumSet.of( + Propagation.MANDATORY, Propagation.REQUIRED, Propagation.SUPPORTS); + private final Map, Optional> cache = new ConcurrentHashMap<>(); + + public Optional getTransactionTemplate(PersistentTask task) { + if (cache.containsKey(task)) return cache.get(task); + + Optional result; + // first we apply a default + if (task.isTransactional()) result = Optional.of(template); + else result = Optional.empty(); + + var annotation = ReflectionUtil.getAnnotation(task, Transactional.class); + if (annotation != null) { + log.debug("found {} on task={}, creating custom ", annotation, task.getClass().getName()); + result = Optional.ofNullable(builTransactionTemplate(task, annotation)); + } + cache.put(task, result); + return result; + } + + private TransactionTemplate builTransactionTemplate(PersistentTask task, Transactional annotation) { + TransactionTemplate result; + if (joinTransaction.contains(annotation.propagation())) { + // No direct mapping for 'rollbackFor' or 'noRollbackFor' + if (annotation.noRollbackFor().length > 0 || annotation.rollbackFor().length > 0) { + throw new IllegalArgumentException("noRollbackFor or rollbackFor not supported. Please remove the settings on " + + task.getClass()); + } else { + var dev = convertTransactionalToDefinition(annotation); + dev.setName(task.getClass().getSimpleName()); + result = new TransactionTemplate(transactionManager, dev); + } + } else { + log.info("Propagation={} disables join of transaction for {}", + annotation.propagation(), task.getClass().getName()); + result = null; + } + return result; + } + + static DefaultTransactionDefinition convertTransactionalToDefinition(Transactional transactional) { + DefaultTransactionDefinition def = new DefaultTransactionDefinition(); + + // Map Transactional attributes to DefaultTransactionDefinition + def.setIsolationLevel(transactional.isolation().value()); + def.setPropagationBehavior(Propagation.REQUIRED.value()); + def.setTimeout(transactional.timeout()); + def.setReadOnly(false); + + return def; + } +} diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java index c496fbb10..b2a991abc 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java @@ -8,7 +8,7 @@ import org.springframework.context.support.GenericApplicationContext; import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.api.TaskId; -import org.sterl.spring.persistent_tasks.task.repository.TaskRepository; +import org.sterl.spring.persistent_tasks.task.TaskService; import lombok.extern.slf4j.Slf4j; @@ -18,11 +18,10 @@ public class TaskConfig { @SuppressWarnings({ "rawtypes", "unchecked" }) @Autowired void configureSimpleTasks(GenericApplicationContext context, - TaskRepository taskRepository) { + TaskService taskService) { final var simpleTasks = context.getBeansOfType(PersistentTask.class); for(Entry t : simpleTasks.entrySet()) { - var id = taskRepository.addTask( - (TaskId)TaskId.of(t.getKey()), t.getValue()); + var id = taskService.register(t.getKey(), t.getValue()); addTaskIdIfMissing(context, id, t.getValue()); } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/RunTriggerComponent.java b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/RunTriggerComponent.java index cae05cbd3..afac1a966 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/RunTriggerComponent.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/RunTriggerComponent.java @@ -3,7 +3,6 @@ import java.io.Serializable; import java.time.OffsetDateTime; import java.util.Optional; -import java.util.concurrent.Callable; import org.springframework.context.ApplicationEventPublisher; import org.springframework.lang.Nullable; @@ -25,7 +24,6 @@ public class RunTriggerComponent { private final TaskService taskService; private final EditTriggerComponent editTrigger; private final ApplicationEventPublisher eventPublisher; - private final TransactionTemplate trx; private final StateSerializer serializer = new StateSerializer(); /** @@ -40,40 +38,41 @@ public Optional execute(TriggerEntity trigger) { if (taskAndState == null) return Optional.of(trigger); try { - Optional result; - if (taskAndState.isTransactional()) { - result = trx.execute(t -> taskAndState.call()); - } else { - result = taskAndState.call(); - } - - return result; + return taskAndState.call(); } catch (Exception e) { return handleTaskException(taskAndState, e); } } + @Nullable private TaskAndState getTastAndState(TriggerEntity trigger) { try { var task = taskService.assertIsKnown(trigger.newTaskId()); + var trx = taskService.getTransactionTemplate(task); var state = serializer.deserialize(trigger.getData().getState()); - return new TaskAndState(task, state, trigger); + return new TaskAndState(task, trx, state, trigger); } catch (Exception e) { // this trigger is somehow crap, no retry and done. - handleTaskException(new TaskAndState(null, null, trigger), e); + handleTaskException(new TaskAndState(null, Optional.empty(), null, trigger), e); return null; } } @RequiredArgsConstructor - private class TaskAndState implements Callable> { + private class TaskAndState { final PersistentTask persistentTask; + final Optional trx; final Serializable state; final TriggerEntity trigger; - boolean isTransactional() { - return persistentTask.isTransactional(); + Optional call() { + if (trx.isPresent()) { + return trx.get().execute(t -> runTask()); + } else { + return runTask(); + } } - public Optional call() { + + private Optional runTask() { eventPublisher.publishEvent(new TriggerRunningEvent(trigger)); persistentTask.accept(state); @@ -82,7 +81,6 @@ public Optional call() { editTrigger.deleteTrigger(trigger); return result; - } } @@ -93,17 +91,18 @@ private Optional handleTaskException(TaskAndState taskAndState, var task = taskAndState.persistentTask; var result = editTrigger.completeTaskWithStatus(trigger.getKey(), e); - if (task != null && - task.retryStrategy().shouldRetry(trigger.getData().getExecutionCount(), e)) { + if (task != null + && task.retryStrategy().shouldRetry(trigger.getData().getExecutionCount(), e)) { final OffsetDateTime retryAt = task.retryStrategy().retryAt(trigger.getData().getExecutionCount(), e); result = editTrigger.retryTrigger(trigger.getKey(), retryAt); if (result.isPresent()) { + var data = result.get().getData(); log.warn("{} failed, retry will be done at={} status={}!", trigger.getKey(), - result.get().getData().getRunAt(), - result.get().getData().getStatus(), + data.getRunAt(), + data.getStatus(), e); } else { log.error("Trigger with key={} not found and may be at a wrong state!", @@ -117,5 +116,4 @@ private Optional handleTaskException(TaskAndState taskAndState, } return result; } - } diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskServiceTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskServiceTest.java index 73043f13b..246249147 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskServiceTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskServiceTest.java @@ -4,11 +4,14 @@ import org.junit.jupiter.api.Test; import org.sterl.spring.persistent_tasks.api.TaskId; +import org.sterl.spring.persistent_tasks.task.component.TaskTransactionComponent; import org.sterl.spring.persistent_tasks.task.repository.TaskRepository; class TaskServiceTest { - private final TaskService subject = new TaskService(new TaskRepository()); + private final TaskService subject = new TaskService( + new TaskTransactionComponent(null, null), + new TaskRepository()); @Test void testAssertIsKnown() { diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java index f3e404695..d1e48ba7b 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java @@ -10,9 +10,9 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Isolation; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.support.DefaultTransactionDefinition; import org.sterl.spring.persistent_tasks.AbstractSpringTest; import org.sterl.spring.persistent_tasks.api.PersistentTask; import org.sterl.spring.persistent_tasks.api.TaskId.TaskTriggerBuilder; @@ -33,16 +33,18 @@ static class TransactionalClass implements PersistentTask { @Override public void accept(String name) { personRepository.save(new PersonBE(name)); + personRepository.save(new PersonBE(name)); } } @Component("transactionalMethod") @RequiredArgsConstructor static class TransactionalMethod implements PersistentTask { private final PersonRepository personRepository; - @Transactional(timeout = 6, propagation = Propagation.MANDATORY) + @Transactional(timeout = 6, propagation = Propagation.MANDATORY, isolation = Isolation.REPEATABLE_READ) @Override public void accept(String name) { personRepository.save(new PersonBE(name)); + personRepository.save(new PersonBE(name)); } } @@ -70,6 +72,7 @@ TransactionalTask transactionalClosure(PersonRepository personRepository } } + @Autowired TaskService subject; @Autowired PersonRepository personRepository; @Autowired @Qualifier("transactionalClass") @@ -93,6 +96,25 @@ void testFindTransactionAnnotation() { assertThat(a).isNotNull(); assertThat(a.timeout()).isEqualTo(7); } + + @Test + void testGetTransactionTemplate() { + var a = subject.getTransactionTemplate(transactionalClass); + assertThat(a).isPresent(); + assertThat(a.get().getTimeout()).isEqualTo(5); + assertThat(a.get().getPropagationBehavior()).isEqualTo(Propagation.REQUIRED.value()); + + a = subject.getTransactionTemplate(transactionalMethod); + assertThat(a).isPresent(); + assertThat(a.get().getTimeout()).isEqualTo(6); + assertThat(a.get().getPropagationBehavior()).isEqualTo(Propagation.REQUIRED.value()); + assertThat(a.get().getIsolationLevel()).isEqualTo(Isolation.REPEATABLE_READ.value()); + + a = subject.getTransactionTemplate(transactionalAnonymous); + assertThat(a).isPresent(); + assertThat(a.get().getTimeout()).isEqualTo(7); + assertThat(a.get().getPropagationBehavior()).isEqualTo(Propagation.REQUIRED.value()); + } @ParameterizedTest @ValueSource(strings = {"transactionalClass", "transactionalMethod", "transactionalClosure"}) @@ -110,20 +132,4 @@ void testTransactionalTask(String task) { hibernateAsserts.assertTrxCount(1); assertThat(personRepository.count()).isEqualTo(2); } - - public static DefaultTransactionDefinition convertTransactionalToDefinition(Transactional transactional) { - DefaultTransactionDefinition def = new DefaultTransactionDefinition(); - - // Map Transactional attributes to DefaultTransactionDefinition - def.setIsolationLevel(transactional.isolation().value()); - def.setPropagationBehavior(transactional.propagation().value()); - def.setTimeout(transactional.timeout()); - def.setReadOnly(transactional.readOnly()); - // No direct mapping for 'rollbackFor' or 'noRollbackFor' - // Set a name if desired (e.g., based on transactional class/method) - def.setName("TransactionalDefinition"); - - return def; - } - } diff --git a/db/pom.xml b/db/pom.xml index c8fff6174..3dee9ed43 100644 --- a/db/pom.xml +++ b/db/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.3.2-SNAPSHOT + 1.4.0-SNAPSHOT ../pom.xml diff --git a/example/pom.xml b/example/pom.xml index 1b2293204..6ee9baf71 100644 --- a/example/pom.xml +++ b/example/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.3.2-SNAPSHOT + 1.4.0-SNAPSHOT ../pom.xml diff --git a/pom.xml b/pom.xml index d19e32525..14797f16e 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.3.2-SNAPSHOT + 1.4.0-SNAPSHOT pom 2024 diff --git a/ui/pom.xml b/ui/pom.xml index f541831b8..468a1620f 100644 --- a/ui/pom.xml +++ b/ui/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.3.2-SNAPSHOT + 1.4.0-SNAPSHOT ../pom.xml From ad790677dfc8dcf7c06e9ebfdf3ea8d078824538 Mon Sep 17 00:00:00 2001 From: Paul Sterl Date: Sun, 5 Jan 2025 16:07:32 +0100 Subject: [PATCH 4/8] added test for REQUIRES_NEW --- RUN_AND_BUILD.md | 2 +- .../SchedulerServiceTransactionTest.java | 65 +++++++++++-------- .../task/TaskTransactionTest.java | 22 +++++-- 3 files changed, 56 insertions(+), 33 deletions(-) diff --git a/RUN_AND_BUILD.md b/RUN_AND_BUILD.md index 0e998e949..cf28f4f12 100644 --- a/RUN_AND_BUILD.md +++ b/RUN_AND_BUILD.md @@ -1,5 +1,5 @@ mvn versions:display-dependency-updates -mvn versions:set -DnewVersion=1.3.1 -DgenerateBackupPoms=false +mvn versions:set -DnewVersion=1.4.0-SNAPSHOT -DgenerateBackupPoms=false mvn versions:set -DnewVersion=1.3.2-SNAPSHOT -DgenerateBackupPoms=false ## postgres diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java index 55b000f87..15d37d247 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java @@ -10,10 +10,10 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.transaction.annotation.Transactional; import org.sterl.spring.persistent_tasks.AbstractSpringTest; -import org.sterl.spring.persistent_tasks.api.RetryStrategy; import org.sterl.spring.persistent_tasks.api.PersistentTask; +import org.sterl.spring.persistent_tasks.api.RetryStrategy; +import org.sterl.spring.persistent_tasks.api.TransactionalTask; import org.sterl.spring.persistent_tasks.api.TaskId.TaskTriggerBuilder; import org.sterl.spring.persistent_tasks.api.TriggerKey; import org.sterl.spring.persistent_tasks.shared.model.TriggerStatus; @@ -24,15 +24,13 @@ class SchedulerServiceTransactionTest extends AbstractSpringTest { private SchedulerService subject; private static AtomicBoolean sendError = new AtomicBoolean(false); - private static AtomicBoolean inTrx = new AtomicBoolean(false); @Autowired private PersonRepository personRepository; @Configuration static class Config { @Bean - PersistentTask savePerson(PersonRepository personRepository) { - return new PersistentTask<>() { - @Transactional + PersistentTask savePersonInTrx(PersonRepository personRepository) { + return new TransactionalTask() { @Override public void accept(String name) { personRepository.save(new PersonBE(name)); @@ -43,9 +41,21 @@ public void accept(String name) { public RetryStrategy retryStrategy() { return RetryStrategy.THREE_RETRIES_IMMEDIATELY; } + }; + } + + @Bean + PersistentTask savePersonNoTrx(PersonRepository personRepository) { + return new PersistentTask<>() { @Override - public boolean isTransactional() { - return inTrx.get(); + public void accept(String name) { + personRepository.save(new PersonBE(name)); + if (sendError.get()) { + throw new RuntimeException("Error requested for " + name); + } + } + public RetryStrategy retryStrategy() { + return RetryStrategy.THREE_RETRIES_IMMEDIATELY; } }; } @@ -57,29 +67,29 @@ public void beforeEach() throws Exception { subject = schedulerService; personRepository.deleteAllInBatch(); sendError.set(false); - inTrx.set(false); } - + @Test - void testSaveEntity() throws Exception { + void testSaveTransactions() throws Exception { // GIVEN - final var trigger = TaskTriggerBuilder.newTrigger("savePerson").state("Paul").build(); + final var request = TaskTriggerBuilder.newTrigger("savePersonNoTrx").state("Paul").build(); + var trigger = triggerService.queue(request); // WHEN hibernateAsserts.reset(); - subject.runOrQueue(trigger).get(); + triggerService.run(trigger); // THEN - // AND one the service, one the event and one more status update, - // one more to save the trigger - hibernateAsserts.assertTrxCount(4); + // AND one the service, one the event and one more status update + hibernateAsserts.assertTrxCount(3); assertThat(personRepository.count()).isOne(); } + @Test - void testSaveTransactions() throws Exception { + void testTrxCountTriggerService() throws Exception { // GIVEN - final var request = TaskTriggerBuilder.newTrigger("savePerson").state("Paul").build(); + final var request = TaskTriggerBuilder.newTrigger("savePersonInTrx").state("Paul").build(); var trigger = triggerService.queue(request); // WHEN @@ -87,34 +97,33 @@ void testSaveTransactions() throws Exception { triggerService.run(trigger); // THEN - // AND one the service, one the event and one more status update - hibernateAsserts.assertTrxCount(3); + hibernateAsserts.assertTrxCount(1); assertThat(personRepository.count()).isOne(); } - @Test - void testTrxCountTriggerService() throws Exception { + void testFailTrxCount() throws Exception { // GIVEN - final var request = TaskTriggerBuilder.newTrigger("savePerson").state("Paul").build(); + final var request = TaskTriggerBuilder.newTrigger("savePersonInTrx").state("Paul").build(); var trigger = triggerService.queue(request); - inTrx.set(true); + sendError.set(true); // WHEN hibernateAsserts.reset(); triggerService.run(trigger); // THEN - hibernateAsserts.assertTrxCount(1); - assertThat(personRepository.count()).isOne(); + // first the work which runs on error + // second the update to the trigger + // third to write the history + hibernateAsserts.assertTrxCount(3); } @Test void testRollbackAndRetry() throws Exception { // GIVEN - final var triggerRequest = TaskTriggerBuilder.newTrigger("savePerson").state("Paul").build(); + final var triggerRequest = TaskTriggerBuilder.newTrigger("savePersonInTrx").state("Paul").build(); sendError.set(true); - inTrx.set(true); // WHEN var key = subject.runOrQueue(triggerRequest); diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java index d1e48ba7b..678fb8a47 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java @@ -56,7 +56,7 @@ static class Config { @Bean("transactionalAnonymous") PersistentTask transactionalAnonymous(PersonRepository personRepository) { return new PersistentTask() { - @Transactional(timeout = 7, propagation = Propagation.MANDATORY) + @Transactional(timeout = 7, propagation = Propagation.REQUIRES_NEW) @Override public void accept(String name) { personRepository.save(new PersonBE(name)); @@ -111,9 +111,23 @@ void testGetTransactionTemplate() { assertThat(a.get().getIsolationLevel()).isEqualTo(Isolation.REPEATABLE_READ.value()); a = subject.getTransactionTemplate(transactionalAnonymous); - assertThat(a).isPresent(); - assertThat(a.get().getTimeout()).isEqualTo(7); - assertThat(a.get().getPropagationBehavior()).isEqualTo(Propagation.REQUIRED.value()); + assertThat(a).isEmpty(); + } + + @Test + void testRequiresNewHasOwnTransaction() { + // GIVEN + var t = triggerService.queue(TaskTriggerBuilder + .newTrigger("transactionalAnonymous", "test").build()); + + // WHEN + personRepository.deleteAllInBatch(); + hibernateAsserts.reset(); + triggerService.run(t).get(); + + // THEN + hibernateAsserts.assertTrxCount(3); + assertThat(personRepository.count()).isEqualTo(1); } @ParameterizedTest From 62b7515b0a0b15dc27ddc370b7a106a4a12832e4 Mon Sep 17 00:00:00 2001 From: Paul Sterl Date: Sun, 5 Jan 2025 17:34:02 +0100 Subject: [PATCH 5/8] fixed PMD issues --- .../spring/persistent_tasks/task/config/TaskConfig.java | 4 ++-- .../scheduler/SchedulerServiceTransactionTest.java | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java b/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java index b2a991abc..f27d89762 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/task/config/TaskConfig.java @@ -23,11 +23,11 @@ void configureSimpleTasks(GenericApplicationContext context, for(Entry t : simpleTasks.entrySet()) { var id = taskService.register(t.getKey(), t.getValue()); - addTaskIdIfMissing(context, id, t.getValue()); + addTaskIdIfMissing(context, id); } } private void addTaskIdIfMissing(GenericApplicationContext context, - TaskId id, PersistentTask task) { + TaskId id) { final var taskIdContextName = id.name() + "Id"; if (!context.containsBean(taskIdContextName)) { log.info("Adding {} with name={} to spring context", id, taskIdContextName); diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java index 15d37d247..2f069aa71 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java @@ -29,7 +29,7 @@ class SchedulerServiceTransactionTest extends AbstractSpringTest { @Configuration static class Config { @Bean - PersistentTask savePersonInTrx(PersonRepository personRepository) { + TransactionalTask savePersonInTrx(PersonRepository personRepository) { return new TransactionalTask() { @Override public void accept(String name) { @@ -57,6 +57,10 @@ public void accept(String name) { public RetryStrategy retryStrategy() { return RetryStrategy.THREE_RETRIES_IMMEDIATELY; } + @Override + public boolean isTransactional() { + return false; + } }; } } From d08a1e53420fed00d479516d719fd631f22c803b Mon Sep 17 00:00:00 2001 From: Paul Sterl Date: Sun, 5 Jan 2025 17:48:52 +0100 Subject: [PATCH 6/8] update to the readme --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index d68038060..7817dac9a 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,10 @@ PersistentTask task1(VehicleHttpConnector vehicleHttpConnector) { } ``` +### Task Transaction Management + +[Transaction-Management Task](https://github.com/sterlp/spring-persistent-tasks/wiki/Transaction-Management) + ## Queue a task execution ### Direct usage of the `TriggerService` or `PersistentTaskService`. From 7ae1b3b8d93f57ce08fd3c11264b1402d06efdad Mon Sep 17 00:00:00 2001 From: Paul Sterl Date: Sun, 5 Jan 2025 17:53:37 +0100 Subject: [PATCH 7/8] v1.4.0 release --- CHANGELOG.md | 11 +++++++++++ RUN_AND_BUILD.md | 2 +- core/pom.xml | 2 +- db/pom.xml | 2 +- example/pom.xml | 2 +- pom.xml | 2 +- ui/pom.xml | 2 +- 7 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf1398d8d..65159df61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## v1.4.0 - (2025-01-02) + +- @Transactional Annotation support +- PersistentTask instead of Task or SpringBeanTask + + +## v1.3.1 - (2025-01-02) + +- Bugfixes +- Sprign Transaction Template support + ## v1.3.0 - (2025-01-01) - MariaDB support diff --git a/RUN_AND_BUILD.md b/RUN_AND_BUILD.md index cf28f4f12..9ac78e6c3 100644 --- a/RUN_AND_BUILD.md +++ b/RUN_AND_BUILD.md @@ -1,6 +1,6 @@ mvn versions:display-dependency-updates mvn versions:set -DnewVersion=1.4.0-SNAPSHOT -DgenerateBackupPoms=false -mvn versions:set -DnewVersion=1.3.2-SNAPSHOT -DgenerateBackupPoms=false +mvn versions:set -DnewVersion=1.4.0 -DgenerateBackupPoms=false ## postgres docker run --name pg-container -e POSTGRES_USER=sa -e POSTGRES_PASSWORD=veryStrong123 -p 5432:5432 -d postgres diff --git a/core/pom.xml b/core/pom.xml index bc3ab7a68..63d26261b 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0-SNAPSHOT + 1.4.0 ../pom.xml diff --git a/db/pom.xml b/db/pom.xml index 3dee9ed43..e9499684f 100644 --- a/db/pom.xml +++ b/db/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0-SNAPSHOT + 1.4.0 ../pom.xml diff --git a/example/pom.xml b/example/pom.xml index 6ee9baf71..f7dc07e23 100644 --- a/example/pom.xml +++ b/example/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0-SNAPSHOT + 1.4.0 ../pom.xml diff --git a/pom.xml b/pom.xml index 14797f16e..21996efe8 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0-SNAPSHOT + 1.4.0 pom 2024 diff --git a/ui/pom.xml b/ui/pom.xml index 468a1620f..0c41ef493 100644 --- a/ui/pom.xml +++ b/ui/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0-SNAPSHOT + 1.4.0 ../pom.xml From 83fb31790b3b4a02bd9c7325ab99ddee19317657 Mon Sep 17 00:00:00 2001 From: Paul Sterl Date: Sun, 5 Jan 2025 17:54:54 +0100 Subject: [PATCH 8/8] v1.4.0-SNAPSHOT --- core/pom.xml | 2 +- db/pom.xml | 2 +- example/pom.xml | 2 +- pom.xml | 2 +- ui/pom.xml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 63d26261b..bc3ab7a68 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0 + 1.4.0-SNAPSHOT ../pom.xml diff --git a/db/pom.xml b/db/pom.xml index e9499684f..3dee9ed43 100644 --- a/db/pom.xml +++ b/db/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0 + 1.4.0-SNAPSHOT ../pom.xml diff --git a/example/pom.xml b/example/pom.xml index f7dc07e23..6ee9baf71 100644 --- a/example/pom.xml +++ b/example/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0 + 1.4.0-SNAPSHOT ../pom.xml diff --git a/pom.xml b/pom.xml index 21996efe8..14797f16e 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0 + 1.4.0-SNAPSHOT pom 2024 diff --git a/ui/pom.xml b/ui/pom.xml index 0c41ef493..468a1620f 100644 --- a/ui/pom.xml +++ b/ui/pom.xml @@ -6,7 +6,7 @@ org.sterl.spring spring-persistent-tasks-root - 1.4.0 + 1.4.0-SNAPSHOT ../pom.xml