Repository: kagkarlsson/db-scheduler (java)

Issue:
Ability to decorate task execution
Thank you for this wonderful library.

We need to decorate all of our task executions for miscellaneous things:
* setup some ThreadLocal values
* trigger specific processes on failure
* define transactions

`SchedulerBuilder` only accepts the abstract class `Task` as the task type. Since it is an abstract class, I had to rely on `byte-buddy` to create a proxy of this type to perform delegation.

Would you accept a pull request turning `Task` into an interface?
In the PR we would:
* rename `Task` into `AbstractTask`
* create an interface named `Task`
* make `AbstractTask` implement `Task`

If we want to avoid breaking change, we can let `Task` being an abstract class and maybe name the new interface `TaskDefinition`?


Discussion:
Hi! It probably would be a good direction to evolve the `Task` into an interface. Not sure how easy/hard it is to do though. On first glance it looks rather straight-forward.

Question though: is it not possible to decorate the underlying `ExecutionHandler`? https://github.com/kagkarlsson/db-scheduler/blob/master/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/ExecutionHandler.java

Patch:
diff --git a/db-scheduler-boot-starter/src/main/java/com/github/kagkarlsson/scheduler/boot/autoconfigure/DbSchedulerAutoConfiguration.java b/db-scheduler-boot-starter/src/main/java/com/github/kagkarlsson/scheduler/boot/autoconfigure/DbSchedulerAutoConfiguration.java
--- a/db-scheduler-boot-starter/src/main/java/com/github/kagkarlsson/scheduler/boot/autoconfigure/DbSchedulerAutoConfiguration.java
+++ b/db-scheduler-boot-starter/src/main/java/com/github/kagkarlsson/scheduler/boot/autoconfigure/DbSchedulerAutoConfiguration.java
@@ -31,7 +31,6 @@
 import com.github.kagkarlsson.scheduler.task.OnStartup;
 import com.github.kagkarlsson.scheduler.task.Task;
 import io.micrometer.core.instrument.MeterRegistry;
-
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.ObjectInput;
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/AbstractTask.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/AbstractTask.java
new file mode 100644
--- /dev/null
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/AbstractTask.java
@@ -0,0 +1,66 @@
+/**
+ * Copyright (C) Gustav Karlsson
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.github.kagkarlsson.scheduler.task;
+
+public abstract class AbstractTask<T> implements Task<T> {
+    protected final String name;
+    private final FailureHandler<T> failureHandler;
+    private final DeadExecutionHandler<T> deadExecutionHandler;
+    private final Class<T> dataClass;
+
+    public AbstractTask(String name, Class<T> dataClass, FailureHandler<T> failureHandler, DeadExecutionHandler<T> deadExecutionHandler) {
+        this.name = name;
+        this.dataClass = dataClass;
+        this.failureHandler = failureHandler;
+        this.deadExecutionHandler = deadExecutionHandler;
+    }
+
+    @Override
+    public String getName() {
+        return name;
+    }
+
+    @Override
+    public Class<T> getDataClass() {
+        return dataClass;
+    }
+
+    @Override
+    public TaskInstance<T> instance(String id) {
+        return new TaskInstance<>(this.name, id);
+    }
+
+    @Override
+    public TaskInstance<T> instance(String id, T data) {
+        return new TaskInstance<>(this.name, id, data);
+    }
+
+    @Override
+    public FailureHandler<T> getFailureHandler() {
+        return failureHandler;
+    }
+
+    @Override
+    public DeadExecutionHandler<T> getDeadExecutionHandler() {
+        return deadExecutionHandler;
+    }
+
+    @Override
+    public String toString() {
+        return "Task " + "name=" + getName();
+    }
+
+}
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/Task.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/Task.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/Task.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/Task.java
@@ -15,48 +15,16 @@
  */
 package com.github.kagkarlsson.scheduler.task;
 
-public abstract class Task<T> implements ExecutionHandler<T> {
-    protected final String name;
-    private final FailureHandler<T> failureHandler;
-    private final DeadExecutionHandler<T> deadExecutionHandler;
-    private final Class<T> dataClass;
+public interface Task<T> extends ExecutionHandler<T> {
+    String getName();
 
-    public Task(String name, Class<T> dataClass, FailureHandler<T> failureHandler, DeadExecutionHandler<T> deadExecutionHandler) {
-        this.name = name;
-        this.dataClass = dataClass;
-        this.failureHandler = failureHandler;
-        this.deadExecutionHandler = deadExecutionHandler;
-    }
+    Class<T> getDataClass();
 
-    public String getName() {
-        return name;
-    }
+    TaskInstance<T> instance(String id);
 
-    public Class<T> getDataClass() {
-        return dataClass;
-    }
+    TaskInstance<T> instance(String id, T data);
 
-    public TaskInstance<T> instance(String id) {
-        return new TaskInstance<>(this.name, id);
-    }
-
-    public TaskInstance<T> instance(String id, T data) {
-        return new TaskInstance<>(this.name, id, data);
-    }
-
-    public abstract CompletionHandler<T> execute(TaskInstance<T> taskInstance, ExecutionContext executionContext);
-
-    public FailureHandler<T> getFailureHandler() {
-        return failureHandler;
-    }
-
-    public DeadExecutionHandler<T> getDeadExecutionHandler() {
-        return deadExecutionHandler;
-    }
-
-    @Override
-    public String toString() {
-        return "Task " + "name=" + getName();
-    }
+    FailureHandler<T> getFailureHandler();
 
+    DeadExecutionHandler<T> getDeadExecutionHandler();
 }
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/ComposableTask.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/ComposableTask.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/ComposableTask.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/ComposableTask.java
@@ -42,7 +42,7 @@ public void executeOnce(TaskInstance<T> taskInstance, ExecutionContext execution
     }
 
     public static <T> Task<T> customTask(String name, Class<T> dataClass, CompletionHandler<T> completionHandler, VoidExecutionHandler<T> executionHandler) {
-        return new Task<T>(name, dataClass, new FailureHandler.OnFailureRetryLater<>(Duration.ofMinutes(5)), new DeadExecutionHandler.ReviveDeadExecution<>()) {
+        return new AbstractTask<T>(name, dataClass, new FailureHandler.OnFailureRetryLater<>(Duration.ofMinutes(5)), new DeadExecutionHandler.ReviveDeadExecution<>()) {
             @Override
             public CompletionHandler<T> execute(TaskInstance<T> taskInstance, ExecutionContext executionContext) {
                 executionHandler.execute(taskInstance, executionContext);
@@ -52,7 +52,7 @@ public CompletionHandler<T> execute(TaskInstance<T> taskInstance, ExecutionConte
     }
 
     public static <T> Task<T> customTask(String name, Class<T> dataClass, CompletionHandler<T> completionHandler, FailureHandler<T> failureHandler, VoidExecutionHandler<T> executionHandler) {
-        return new Task<T>(name, dataClass, failureHandler, new DeadExecutionHandler.ReviveDeadExecution<>()) {
+        return new AbstractTask<T>(name, dataClass, failureHandler, new DeadExecutionHandler.ReviveDeadExecution<>()) {
             @Override
             public CompletionHandler<T> execute(TaskInstance<T> taskInstance, ExecutionContext executionContext) {
                 executionHandler.execute(taskInstance, executionContext);
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/CustomTask.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/CustomTask.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/CustomTask.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/CustomTask.java
@@ -17,12 +17,12 @@
 
 import com.github.kagkarlsson.scheduler.Clock;
 import com.github.kagkarlsson.scheduler.Scheduler;
+import com.github.kagkarlsson.scheduler.task.AbstractTask;
 import com.github.kagkarlsson.scheduler.task.DeadExecutionHandler;
 import com.github.kagkarlsson.scheduler.task.FailureHandler;
 import com.github.kagkarlsson.scheduler.task.OnStartup;
-import com.github.kagkarlsson.scheduler.task.Task;
 
-public abstract class CustomTask<T> extends Task<T> implements OnStartup {
+public abstract class CustomTask<T> extends AbstractTask<T> implements OnStartup {
     private ScheduleOnStartup<T> scheduleOnStartup;
 
     public CustomTask(String name, Class<T> dataClass, ScheduleOnStartup<T> scheduleOnStartup, FailureHandler<T> failureHandler, DeadExecutionHandler<T> deadExecutionHandler) {
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/OneTimeTask.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/OneTimeTask.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/OneTimeTask.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/OneTimeTask.java
@@ -22,7 +22,7 @@
 
 import java.time.Duration;
 
-public abstract class OneTimeTask<T> extends Task<T> {
+public abstract class OneTimeTask<T> extends AbstractTask<T> {
 
     public OneTimeTask(String name, Class<T> dataClass) {
         this(name, dataClass, new OnFailureRetryLater<>(Duration.ofMinutes(5)), new ReviveDeadExecution<>());
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/RecurringTask.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/RecurringTask.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/RecurringTask.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/RecurringTask.java
@@ -22,7 +22,7 @@
 import com.github.kagkarlsson.scheduler.task.DeadExecutionHandler.ReviveDeadExecution;
 import com.github.kagkarlsson.scheduler.task.schedule.Schedule;
 
-public abstract class RecurringTask<T> extends Task<T> implements OnStartup {
+public abstract class RecurringTask<T> extends AbstractTask<T> implements OnStartup {
 
     public static final String INSTANCE = "recurring";
     private final OnCompleteReschedule<T> onComplete;


Test patch:
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/testhelper/TestHelper.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/testhelper/TestHelper.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/testhelper/TestHelper.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/testhelper/TestHelper.java
@@ -16,23 +16,22 @@
 package com.github.kagkarlsson.scheduler.testhelper;
 
 import com.github.kagkarlsson.scheduler.PollingStrategyConfig;
+import com.github.kagkarlsson.scheduler.SchedulerBuilder;
 import com.github.kagkarlsson.scheduler.SchedulerName;
+import com.github.kagkarlsson.scheduler.TaskResolver;
 import com.github.kagkarlsson.scheduler.jdbc.DefaultJdbcCustomization;
-import com.github.kagkarlsson.scheduler.SchedulerBuilder;
 import com.github.kagkarlsson.scheduler.jdbc.JdbcTaskRepository;
-import com.github.kagkarlsson.scheduler.TaskResolver;
 import com.github.kagkarlsson.scheduler.logging.LogLevel;
 import com.github.kagkarlsson.scheduler.stats.StatsRegistry;
 import com.github.kagkarlsson.scheduler.task.OnStartup;
 import com.github.kagkarlsson.scheduler.task.Task;
-
-import javax.sql.DataSource;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.AbstractExecutorService;
 import java.util.concurrent.TimeUnit;
+import javax.sql.DataSource;
 
 public class TestHelper {
 
diff --git a/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/DeadExecutionsTest.java b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/DeadExecutionsTest.java
--- a/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/DeadExecutionsTest.java
+++ b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/DeadExecutionsTest.java
@@ -1,36 +1,35 @@
 package com.github.kagkarlsson.scheduler;
 
+import static com.github.kagkarlsson.scheduler.jdbc.JdbcTaskRepository.DEFAULT_TABLE_NAME;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 import com.github.kagkarlsson.scheduler.jdbc.JdbcTaskRepository;
 import com.github.kagkarlsson.scheduler.logging.LogLevel;
 import com.github.kagkarlsson.scheduler.stats.StatsRegistry;
+import com.github.kagkarlsson.scheduler.task.AbstractTask;
 import com.github.kagkarlsson.scheduler.task.CompletionHandler;
 import com.github.kagkarlsson.scheduler.task.DeadExecutionHandler;
 import com.github.kagkarlsson.scheduler.task.Execution;
 import com.github.kagkarlsson.scheduler.task.ExecutionContext;
 import com.github.kagkarlsson.scheduler.task.ExecutionOperations;
-import com.github.kagkarlsson.scheduler.task.Task;
 import com.github.kagkarlsson.scheduler.task.TaskInstance;
 import com.github.kagkarlsson.scheduler.task.VoidExecutionHandler;
 import com.github.kagkarlsson.scheduler.task.helper.OneTimeTask;
 import com.github.kagkarlsson.scheduler.testhelper.SettableClock;
 import com.google.common.util.concurrent.MoreExecutors;
-import org.hamcrest.Matchers;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
-
-import static com.github.kagkarlsson.scheduler.jdbc.JdbcTaskRepository.DEFAULT_TABLE_NAME;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.hasSize;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.nullValue;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 
 public class DeadExecutionsTest {
@@ -131,7 +130,7 @@ public void scheduler_should_detect_dead_execution_that_never_updated_heartbeat(
         assertThat(nonCompletingExecutionHandler.timesExecuted.get(), is(2));
     }
 
-    public static class NonCompletingTask<T> extends Task<T> {
+    public static class NonCompletingTask<T> extends AbstractTask<T> {
         private final VoidExecutionHandler<T> handler;
 
         public NonCompletingTask(String name, Class<T> dataClass, VoidExecutionHandler<T> handler, DeadExecutionHandler<T> deadExecutionHandler) {
diff --git a/examples/spring-boot-example/src/test/java/com/github/kagkarlsson/examples/boot/SmokeTest.java b/examples/spring-boot-example/src/test/java/com/github/kagkarlsson/examples/boot/SmokeTest.java
--- a/examples/spring-boot-example/src/test/java/com/github/kagkarlsson/examples/boot/SmokeTest.java
+++ b/examples/spring-boot-example/src/test/java/com/github/kagkarlsson/examples/boot/SmokeTest.java
@@ -11,6 +11,7 @@
 import com.github.kagkarlsson.scheduler.task.TaskInstance;
 import com.github.kagkarlsson.scheduler.task.helper.OneTimeTask;
 import com.github.kagkarlsson.scheduler.task.helper.RecurringTask;
+import java.time.Instant;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -20,15 +21,10 @@
 import org.springframework.boot.actuate.health.Status;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.context.assertj.AssertableWebApplicationContext;
-import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
-import org.springframework.transaction.TransactionException;
 import org.springframework.transaction.support.TransactionTemplate;
 import org.springframework.web.context.ConfigurableWebApplicationContext;
 
-import java.time.Duration;
-import java.time.Instant;
-
 @ExtendWith(SpringExtension.class)
 @SpringBootTest
 public class SmokeTest {