Repository: kagkarlsson/db-scheduler (java)

Issue:
Add support for disabled cron expression "-"
According to the docs, db-scheduler supports

> Spring-style cron-expression.

However, a Spring cron expression of "-" means [the task will never run](https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/scheduling/annotation/Scheduled.html#cron--), which is useful for disabling a task in a local/test environment.

I tried this expression with db-scheduler, but it caused this exception:

```
Caused by: java.lang.IllegalArgumentException: Cron expression contains 1 parts but we expect one of [6]
	at com.github.kagkarlsson.shaded.cronutils.parser.CronParser.parse(CronParser.java:120) ~[db-scheduler-10.3.jar:na]
	at com.github.kagkarlsson.scheduler.task.schedule.CronSchedule.<init>(CronSchedule.java:46) ~[db-scheduler-10.3.jar:na]
	at com.github.kagkarlsson.scheduler.task.schedule.CronSchedule.<init>(CronSchedule.java:56) ~[db-scheduler-10.3.jar:na]
	at com.github.kagkarlsson.scheduler.task.schedule.Schedules.cron(Schedules.java:39) ~[db-scheduler-10.3.jar:na]
```


Discussion:
Interesting. Yeah, that is not supported. I think I primarily meant that seconds are supported, so I probably should update the doc.

Not sure if it is possible to support that scenario just yet. When we add support for pausing/stopping an execution in db-scheduler, this would be a nice addition
We currently use this workaround, which is okay but not perfect:
```
    @Bean
    Task<Void> triggerCleanUp(final CleanUpService service,
            @Value("${cleanup.cron.expression}") final String cronPattern) {
        if (cronPattern == null || cronPattern.equals("-")) {
            return Tasks.custom("cleanup", Void.class).execute((i, c) -> new OnCompleteRemove<>());
        }
        return Tasks.recurring("cleanup", Schedules.cron(cronPattern)).execute((instance, ctx) -> {
            service.cleanUp();
        });
    }
```
This is the workaround that I use. The `taskInvitationsSender` and `autocompleteEventsUpdater` beans are jobs that are run by the scheduler.

```java
@Configuration
public class ClusteredSchedulerConfiguration {

    private static final String DISABLED_CRON_EXPRESSION = "-";

    private static final Logger logger = LoggerFactory.getLogger(ClusteredSchedulerConfiguration.class);

    @Bean
    Task<Void> scheduleTaskInvitationsSender(ScheduledTask taskInvitationsSender,
                                             @Value("${cron.task-invitations-sender}") String schedule) {
        return scheduleTask(schedule, taskInvitationsSender);
    }

    @Bean
    Task<Void> scheduleAutocompleteEvents(ScheduledTask autocompleteEventsUpdater,
                                          @Value("${cron.autocomplete-events}") String schedule) {
        return scheduleTask(schedule, autocompleteEventsUpdater);
    }

    private Task<Void> scheduleTask(String cronExpression, ScheduledTask task) {
        boolean isTaskDisabled = DISABLED_CRON_EXPRESSION.equals(cronExpression);

        if (isTaskDisabled) {
            var taskName = task.getTaskName();
            logger.warn("The {} scheduled task is disabled in the current environment", taskName);

            /*
            A workaround for the fact that db-scheduler doesn't support Spring's disabled cron expression
            https://github.com/kagkarlsson/db-scheduler/issues/216

            Return a once-off task that is never scheduled and does nothing when executed
             */
            return Tasks.oneTime(taskName, Void.class).execute((inst, ctx) -> {
            });

        } else {
            Schedule schedule = cron(cronExpression);
            return Tasks.recurring(task.getTaskName(), schedule).execute((instance, ctx) -> task.run());
        }
    }
}
```

Patch:
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/ScheduleRecurringOnStartup.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/ScheduleRecurringOnStartup.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/ScheduleRecurringOnStartup.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/helper/ScheduleRecurringOnStartup.java
@@ -46,6 +46,17 @@ public void apply(SchedulerClient scheduler, Clock clock, Task<T> task) {
         final TaskInstance<T> instanceWithoutData = task.instance(this.instance);
         final Optional<ScheduledExecution<Object>> preexistingExecution = scheduler.getScheduledExecution(instanceWithoutData);
 
+        if (schedule.isDisabled()) {
+            if (preexistingExecution.isPresent()) {
+                LOG.info("Task-instance '{}' has a Schedule that has been marked as disabled. Removing existing execution with execution-time '{}'.",
+                    instanceWithoutData, preexistingExecution.get().getExecutionTime());
+                tryCancel(scheduler, instanceWithoutData);
+            } else {
+                LOG.info("Task-instance '{}' has a Schedule that has been marked as disabled. Will not schedule a new execution", instanceWithoutData);
+            }
+            return;
+        }
+
         if (preexistingExecution.isPresent()) {
             Optional<Instant> newNextExecutionTime = checkForNewExecutionTime(clock, instanceWithoutData, preexistingExecution.get());
 
@@ -62,6 +73,15 @@ public void apply(SchedulerClient scheduler, Clock clock, Task<T> task) {
         }
     }
 
+    private void tryCancel(SchedulerClient scheduler, TaskInstance<T> instanceWithoutData) {
+        try {
+            scheduler.cancel(instanceWithoutData);
+        } catch (RuntimeException e) {
+            LOG.warn("Failed to cancel existing execution for a Task with a Scheduled marked as disabled. " +
+                "May happen if another instance already did it, or if it is currently executing.", e);
+        }
+    }
+
     Optional<Instant> checkForNewExecutionTime(Clock clock, TaskInstance<T> instanceWithoutData, ScheduledExecution<Object> preexistingExecution) {
         final Instant preexistingExecutionTime = preexistingExecution.getExecutionTime();
         final Instant nextExecutionTimeRelativeToNow = schedule.getNextExecutionTime(ExecutionComplete.simulatedSuccess(clock.now()));
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/CronSchedule.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/CronSchedule.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/CronSchedule.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/CronSchedule.java
@@ -24,6 +24,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.time.Duration;
 import java.time.Instant;
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
@@ -35,25 +36,30 @@
  */
 public class CronSchedule implements Schedule {
 
+    private static final String DISABLED = "-";
     private static final Logger LOG = LoggerFactory.getLogger(CronSchedule.class);
     private final String pattern;
     private final ZoneId zoneId;
     private final ExecutionTime cronExecutionTime;
 
+    public CronSchedule(String pattern) {
+        this(pattern, ZoneId.systemDefault());
+    }
+
     public CronSchedule(String pattern, ZoneId zoneId) {
         this.pattern = pattern;
-        CronParser parser = new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.SPRING));
-        Cron cron = parser.parse(pattern);
-        this.cronExecutionTime = ExecutionTime.forCron(cron);
-
         if (zoneId == null) {
             throw new IllegalArgumentException("zoneId may not be null");
         }
         this.zoneId = zoneId;
-    }
 
-    public CronSchedule(String pattern) {
-        this(pattern, ZoneId.systemDefault());
+        if (isDisabled()) {
+            this.cronExecutionTime = new DisabledScheduleExecutionTime();
+        } else {
+            CronParser parser = new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.SPRING));
+            Cron cron = parser.parse(pattern);
+            this.cronExecutionTime = ExecutionTime.forCron(cron);
+        }
     }
 
     @Override
@@ -78,4 +84,40 @@ public boolean isDeterministic() {
     public String toString() {
         return "CronSchedule pattern=" + pattern + ", zone=" + zoneId;
     }
+
+    private static class DisabledScheduleExecutionTime implements ExecutionTime {
+        @Override
+        public Optional<ZonedDateTime> nextExecution(ZonedDateTime date) {
+            throw unsupportedException();
+        }
+
+        @Override
+        public Optional<Duration> timeToNextExecution(ZonedDateTime date) {
+            throw unsupportedException();
+        }
+
+        @Override
+        public Optional<ZonedDateTime> lastExecution(ZonedDateTime date) {
+            throw unsupportedException();
+        }
+
+        @Override
+        public Optional<Duration> timeFromLastExecution(ZonedDateTime date) {
+            throw unsupportedException();
+        }
+
+        @Override
+        public boolean isMatch(ZonedDateTime date) {
+            throw unsupportedException();
+        }
+
+        private UnsupportedOperationException unsupportedException() {
+            return new UnsupportedOperationException("Schedule is marked as disabled. Method should never be called");
+        }
+    }
+
+    @Override
+    public boolean isDisabled() {
+        return DISABLED.equals(pattern);
+    }
 }
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/DisabledSchedule.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/DisabledSchedule.java
new file mode 100644
--- /dev/null
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/DisabledSchedule.java
@@ -0,0 +1,47 @@
+/**
+ * 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.schedule;
+
+import com.github.kagkarlsson.scheduler.task.ExecutionComplete;
+
+import java.time.Instant;
+
+public class DisabledSchedule implements Schedule{
+
+    @Override
+    public Instant getNextExecutionTime(ExecutionComplete executionComplete) {
+        throw unsupportedException();
+    }
+
+    @Override
+    public boolean isDeterministic() {
+        throw unsupportedException();
+    }
+
+    @Override
+    public Instant getInitialExecutionTime(Instant now) {
+        throw unsupportedException();
+    }
+
+    @Override
+    public boolean isDisabled() {
+        return true;
+    }
+
+    private UnsupportedOperationException unsupportedException() {
+        return new UnsupportedOperationException("DisabledSchedule does not support any other operations than 'isDisabled()'. This appears to be a bug.");
+    }
+}
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/DisabledScheduleParser.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/DisabledScheduleParser.java
new file mode 100644
--- /dev/null
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/DisabledScheduleParser.java
@@ -0,0 +1,35 @@
+/**
+ * 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.schedule;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.regex.MatchResult;
+import java.util.regex.Pattern;
+
+final class DisabledScheduleParser extends RegexBasedParser {
+    private static final Pattern DISABLED_PATTERN = Pattern.compile("^-$");
+    private static final List<String> EXAMPLES = Collections.singletonList("-");
+
+    DisabledScheduleParser() {
+        super(DISABLED_PATTERN, EXAMPLES);
+    }
+
+    @Override
+    protected Schedule matchedSchedule(MatchResult matchResult) {
+        return new DisabledSchedule();
+    }
+}
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/Schedule.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/Schedule.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/Schedule.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/Schedule.java
@@ -32,4 +32,8 @@ default Instant getInitialExecutionTime(Instant now) {
 
     boolean isDeterministic();
 
+    default boolean isDisabled() {
+        return false;
+    }
+
 }
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/Schedules.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/Schedules.java
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/Schedules.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/task/schedule/Schedules.java
@@ -21,7 +21,7 @@
 import java.util.List;
 
 public class Schedules {
-    private static final Parser SCHEDULE_PARSER = CompositeParser.of(new FixedDelayParser(), new DailyParser());
+    private static final Parser SCHEDULE_PARSER = CompositeParser.of(new FixedDelayParser(), new DailyParser(), new DisabledScheduleParser());
 
     public static Daily daily(LocalTime... times) {
         return new Daily(times);
diff --git a/examples/features/src/main/java/com/github/kagkarlsson/examples/JobChainingPocMain.java b/examples/features/src/main/java/com/github/kagkarlsson/examples/JobChainingPocMain.java
--- a/examples/features/src/main/java/com/github/kagkarlsson/examples/JobChainingPocMain.java
+++ b/examples/features/src/main/java/com/github/kagkarlsson/examples/JobChainingPocMain.java
@@ -1,3 +1,18 @@
+/**
+ * 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.examples;
 
 import com.github.kagkarlsson.examples.helpers.Example;


Test patch:
diff --git a/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/functional/DisabledCronTaskTest.java b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/functional/DisabledCronTaskTest.java
new file mode 100644
--- /dev/null
+++ b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/functional/DisabledCronTaskTest.java
@@ -0,0 +1,74 @@
+package com.github.kagkarlsson.scheduler.functional;
+
+import com.github.kagkarlsson.scheduler.EmbeddedPostgresqlExtension;
+import com.github.kagkarlsson.scheduler.TestTasks;
+import com.github.kagkarlsson.scheduler.task.TaskInstanceId;
+import com.github.kagkarlsson.scheduler.task.helper.RecurringTask;
+import com.github.kagkarlsson.scheduler.task.helper.Tasks;
+import com.github.kagkarlsson.scheduler.task.schedule.Schedules;
+import com.github.kagkarlsson.scheduler.testhelper.ManualScheduler;
+import com.github.kagkarlsson.scheduler.testhelper.SettableClock;
+import com.github.kagkarlsson.scheduler.testhelper.TestHelper;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import java.time.LocalDate;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+
+import static java.util.Collections.singletonList;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class DisabledCronTaskTest {
+
+    public static final String RECURRING_A = "recurring-a";
+    private SettableClock clock;
+
+    @RegisterExtension
+    public EmbeddedPostgresqlExtension postgres = new EmbeddedPostgresqlExtension();
+
+    @BeforeEach
+    public void setUp() {
+        clock = new SettableClock();
+        clock.set(
+            ZonedDateTime.of(
+                LocalDate.of(2018, 3, 1),
+                LocalTime.of(8, 0),
+                ZoneId.systemDefault()).toInstant());
+    }
+
+    @Test
+    public void should_remove_existing_executions_for_tasks_with_disabled_schedule() {
+        RecurringTask<Void> recurringTask = Tasks.recurring(RECURRING_A, Schedules.cron("0 0 12 * * ?"))
+            .execute(TestTasks.DO_NOTHING);
+
+        ManualScheduler scheduler = manualSchedulerFor(recurringTask);
+        scheduler.start();
+        scheduler.stop();
+
+        assertTrue(scheduler.getScheduledExecution(TaskInstanceId.of(RECURRING_A, RecurringTask.INSTANCE)).isPresent());
+
+        RecurringTask<Void> disabledRecurringTask = Tasks.recurring(RECURRING_A, Schedules.cron("-"))
+            .execute(TestTasks.DO_NOTHING);
+
+        ManualScheduler restartedScheduler = manualSchedulerFor(disabledRecurringTask);
+        restartedScheduler.start();
+        restartedScheduler.stop();
+
+        assertFalse(scheduler.getScheduledExecution(TaskInstanceId.of(RECURRING_A, RecurringTask.INSTANCE)).isPresent());
+
+        restartedScheduler.start();
+
+        assertFalse(scheduler.getScheduledExecution(TaskInstanceId.of(RECURRING_A, RecurringTask.INSTANCE)).isPresent());
+    }
+
+    private ManualScheduler manualSchedulerFor(RecurringTask<?> recurringTasks) {
+        return TestHelper.createManualScheduler(postgres.getDataSource())
+            .clock(clock)
+            .startTasks(singletonList(recurringTasks))
+            .build();
+    }
+}
diff --git a/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/task/CronTest.java b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/task/CronTest.java
--- a/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/task/CronTest.java
+++ b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/task/CronTest.java
@@ -12,6 +12,8 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 
 public class CronTest {
@@ -70,6 +72,12 @@ public void should_always_use_time_zone() {
         assertNextExecutionTime(firstJanuaryMiddayUTC, "0 05 13,20 * * ?", newYork, ZonedDateTime.of(2000, 1, 1, 13, 5, 0, 0, newYork));  //next fire time should be 13:05 New York time
     }
 
+    @Test
+    public void should_mark_schedule_as_disabled() {
+        assertTrue(Schedules.cron("-").isDisabled());
+        assertFalse(Schedules.cron("0 * * * * ?").isDisabled());
+    }
+
     private void assertNextExecutionTime(ZonedDateTime timeDone, String cronPattern, ZonedDateTime expectedTime) {
         assertNextExecutionTime(timeDone, expectedTime, new CronSchedule(cronPattern));
     }
diff --git a/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/task/SchedulesTest.java b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/task/SchedulesTest.java
--- a/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/task/SchedulesTest.java
+++ b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/task/SchedulesTest.java
@@ -1,6 +1,7 @@
 package com.github.kagkarlsson.scheduler.task;
 
 import com.github.kagkarlsson.scheduler.task.schedule.Daily;
+import com.github.kagkarlsson.scheduler.task.schedule.DisabledSchedule;
 import com.github.kagkarlsson.scheduler.task.schedule.FixedDelay;
 import com.github.kagkarlsson.scheduler.task.schedule.Schedule;
 import com.github.kagkarlsson.scheduler.task.schedule.Schedules;
@@ -50,6 +51,8 @@ public void should_validate_pattern() {
 
         Schedule fixedDelaySchedule = assertParsable("FIXED_DELAY|10s", FixedDelay.class);
         assertThat(fixedDelaySchedule.getNextExecutionTime(complete(NOON_TODAY)), is(NOON_TODAY.plusSeconds(10)));
+
+        assertParsable("-", DisabledSchedule.class);
     }
 
     private ExecutionComplete complete(Instant timeDone) {