Make sure task is not executed after is has cancelled himself

scm-webapp/src/main/java/sonia/scm/schedule/CronExpression.java

@@ -30,7 +30,7 @@ final class CronExpression {
 
   boolean shouldRun(ZonedDateTime time) {
     ZonedDateTime now = ZonedDateTime.now(clock);
-    return time.isBefore(now);
+    return time.isBefore(now) || time.isEqual(now);
   }
 
   Optional<ZonedDateTime> calculateNextRun() {

scm-webapp/src/main/java/sonia/scm/schedule/CronTask.java

@@ -32,7 +32,7 @@ class CronTask implements Task, Runnable {
 
   @Override
   public synchronized void run() {
-    if (expression.shouldRun(nextRun)) {
+    if (hasNextRun() && expression.shouldRun(nextRun)) {
       LOG.debug("execute task {}, because of matching expression {}", name, expression);
       runnable.run();
       Optional<ZonedDateTime> next = expression.calculateNextRun();
@@ -40,6 +40,7 @@ class CronTask implements Task, Runnable {
         nextRun = next.get();
       } else {
         LOG.debug("cancel task {}, because expression {} has no next execution", name, expression);
+        nextRun = null;
         cancel();
       }
     } else {

scm-webapp/src/test/java/sonia/scm/schedule/CronTaskTest.java

@@ -53,7 +53,7 @@ class CronTaskTest {
   @Test
   void shouldCancelWithoutNextRun() {
     ZonedDateTime time = ZonedDateTime.now();
-    when(expression.calculateNextRun()).thenAnswer(new FirstTimeAnswer(Optional.of(time), Optional.empty()));
+    when(expression.calculateNextRun()).thenReturn(Optional.of(time), Optional.empty());
     when(expression.shouldRun(time)).thenReturn(true);
 
     CronTask task = task();
@@ -64,32 +64,26 @@ class CronTaskTest {
     verify(future).cancel(false);
   }
 
+  @Test
+  void shouldNotRunAfterCancelHasBeenCalledIfRunIsCalledAgain() {
+    ZonedDateTime time = ZonedDateTime.now();
+    when(expression.calculateNextRun()).thenReturn(Optional.of(time), Optional.empty());
+    when(expression.shouldRun(time)).thenReturn(true);
+
+    CronTask task = task();
+    task.setFuture(future);
+
+    task.run();
+    task.run();
+
+    verify(future).cancel(false);
+    verify(runnable).run();
+  }
+
   @Test
   void shouldNotRun() {
     task().run();
 
     verify(runnable, never()).run();
   }
-
-  private static class FirstTimeAnswer implements Answer<Object> {
-
-    private boolean first = true;
-    private final Object answer;
-    private final Object secondAnswer;
-
-    FirstTimeAnswer(Object answer, Object secondAnswer) {
-      this.answer = answer;
-      this.secondAnswer = secondAnswer;
-    }
-
-    @Override
-    public Object answer(InvocationOnMock invocation) {
-      if (first) {
-        first = false;
-        return answer;
-      }
-      return secondAnswer;
-    }
-  }
-
 }