fix: Fix getExecutionsFailingLongerThan to not return executions that have previously failed but are now running ok (#685)

getExecutionsFailingLongerThan is not working as expected. Returns
execution forever if it has failed once..

## Fixes
#563


## Reminders
- [ ] Added/ran automated tests
- [ ] Update README and/or examples
- [ ] Ran `mvn spotless:apply`

Author: kagkarlsson

db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/JdbcTaskRepository.java

@@ -673,12 +673,12 @@ public List<Execution> getExecutionsFailingLongerThan(Duration interval) {
         "select * from "
             + tableName
             + " where "
-            + "    ((last_success is null and last_failure is not null)"
-            + "    or (last_failure is not null and last_success < ?)) "
+            + "    consecutive_failures > 0 "
+            + "    and (last_success is null or last_success < ?) "
             + unresolvedFilter.andCondition(),
         (PreparedStatement p) -> {
           int index = 1;
-          jdbcCustomization.setInstant(p, index++, Instant.now().minus(interval));
+          jdbcCustomization.setInstant(p, index++, clock.now().minus(interval));
           unresolvedFilter.setParameters(p, index);
         },
         new ExecutionResultSetMapper(false, false));

db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/JdbcTaskRepositoryTest.java

@@ -3,6 +3,8 @@
 import static com.github.kagkarlsson.scheduler.ScheduledExecutionsFilter.all;
 import static com.github.kagkarlsson.scheduler.ScheduledExecutionsFilter.onlyResolved;
 import static com.github.kagkarlsson.scheduler.jdbc.JdbcTaskRepository.DEFAULT_TABLE_NAME;
+import static java.time.Duration.ofDays;
+import static java.time.Duration.ofMinutes;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.empty;
@@ -175,13 +177,13 @@ public void get_due_should_be_sorted_by_priority_when_enabled() {
     Instant now = TimeHelper.truncatedInstantNow();
     SchedulableTaskInstance<Void> id1 =
         new SchedulableTaskInstance<>(
-            oneTimeTask.instanceBuilder("id1").priority(1).build(), now.minus(Duration.ofDays(1)));
+            oneTimeTask.instanceBuilder("id1").priority(1).build(), now.minus(ofDays(1)));
     SchedulableTaskInstance<Void> id2 =
         new SchedulableTaskInstance<>(
-            oneTimeTask.instanceBuilder("id2").priority(10).build(), now.minus(Duration.ofDays(2)));
+            oneTimeTask.instanceBuilder("id2").priority(10).build(), now.minus(ofDays(2)));
     SchedulableTaskInstance<Void> id3 =
         new SchedulableTaskInstance<>(
-            oneTimeTask.instanceBuilder("id3").priority(5).build(), now.minus(Duration.ofDays(3)));
+            oneTimeTask.instanceBuilder("id3").priority(5).build(), now.minus(ofDays(3)));
 
     Stream.of(id1, id2, id3).forEach(taskRespositoryWithPriority::createIfNotExists);
 
@@ -295,7 +297,7 @@ public void reschedule_should_move_execution_in_time() {
 
     Execution execution = due.get(0);
     final Optional<Execution> pickedExecution = taskRepository.pick(execution, now);
-    final Instant nextExecutionTime = now.plus(Duration.ofMinutes(1));
+    final Instant nextExecutionTime = now.plus(ofMinutes(1));
     taskRepository.reschedule(pickedExecution.get(), nextExecutionTime, now, null, 0);
 
     assertThat(taskRepository.getDue(now, POLLING_LIMIT), hasSize(0));
@@ -318,7 +320,7 @@ public void reschedule_should_persist_consecutive_failures() {
 
     Execution execution = due.get(0);
     final Optional<Execution> pickedExecution = taskRepository.pick(execution, now);
-    final Instant nextExecutionTime = now.plus(Duration.ofMinutes(1));
+    final Instant nextExecutionTime = now.plus(ofMinutes(1));
     taskRepository.reschedule(
         pickedExecution.get(), nextExecutionTime, now.minusSeconds(1), now, 1);
 
@@ -336,7 +338,7 @@ public void reschedule_should_update_data_if_specified() {
     Execution created = taskRepository.getExecution(instance).get();
     assertEquals(created.taskInstance.getData(), 1);
 
-    final Instant nextExecutionTime = now.plus(Duration.ofMinutes(1));
+    final Instant nextExecutionTime = now.plus(ofMinutes(1));
     taskRepository.reschedule(created, nextExecutionTime, 2, now, null, 0);
 
     final Execution rescheduled = taskRepository.getExecution(instance).get();
@@ -359,16 +361,43 @@ public void test_get_failing_executions() {
 
     taskRepository.reschedule(getSingleDueExecution(), now, null, now, 1);
     assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ZERO), hasSize(1));
-    assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ofMinutes(1)), hasSize(1));
-    assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ofDays(1)), hasSize(1));
+    assertThat(taskRepository.getExecutionsFailingLongerThan(ofMinutes(1)), hasSize(1));
+    assertThat(taskRepository.getExecutionsFailingLongerThan(ofDays(1)), hasSize(1));
 
-    taskRepository.reschedule(
-        getSingleDueExecution(), now, now.minus(Duration.ofMinutes(1)), now, 1);
+    taskRepository.reschedule(getSingleDueExecution(), now, now.minus(ofMinutes(1)), now, 1);
     assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ZERO), hasSize(1));
     assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ofSeconds(1)), hasSize(1));
     assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ofHours(1)), hasSize(0));
   }
 
+  @Test
+  public void
+      get_failing_executions_should_not_return_previously_failed_but_currently_successful() {
+    Instant third = TimeHelper.truncatedInstantNow();
+    Instant second = third.minus(ofMinutes(1));
+    Instant first = second.minus(ofMinutes(1));
+
+    Instant timeToRun = first;
+
+    final TaskInstance<Void> instance = oneTimeTask.instance("id1");
+    taskRepository.createIfNotExists(new SchedulableTaskInstance<>(instance, timeToRun));
+
+    assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ZERO), hasSize(0));
+
+    // simulate success
+    taskRepository.reschedule(getSingleDueExecution(), timeToRun, first, null, 0);
+    assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ZERO), hasSize(0));
+
+    // simulate failure
+    taskRepository.reschedule(getSingleDueExecution(), timeToRun, first, second, 1);
+    assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ZERO), hasSize(1));
+    assertThat(taskRepository.getExecutionsFailingLongerThan(ofMinutes(5)), hasSize(0));
+
+    // simulate success
+    taskRepository.reschedule(getSingleDueExecution(), timeToRun, third, second, 0);
+    assertThat(taskRepository.getExecutionsFailingLongerThan(Duration.ZERO), hasSize(0));
+  }
+
   @Test
   public void get_scheduled_executions() {
     Instant now = TimeHelper.truncatedInstantNow();
@@ -426,7 +455,7 @@ public void get_dead_executions_should_not_include_previously_unresolved() {
     Instant now = TimeHelper.truncatedInstantNow();
 
     // 1
-    final Instant timeDied = now.minus(Duration.ofDays(5));
+    final Instant timeDied = now.minus(ofDays(5));
     createDeadExecution(oneTimeTask.instance("id1"), timeDied);
 
     TaskResolver taskResolverMissingTask =

db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/helper/TimeHelper.java

@@ -13,6 +13,10 @@ private TimeHelper() {}
    * @see <a href="https://bugs.openjdk.java.net/browse/JDK-8068730">JDK-8068730</a>
    */
   public static Instant truncatedInstantNow() {
-    return Instant.now().truncatedTo(MILLIS);
+    return truncated(Instant.now());
+  }
+
+  private static Instant truncated(Instant instant) {
+    return instant.truncatedTo(MILLIS);
   }
 }