Skip to content

Commit

Permalink
fix(core): properly check next scheduled date for backfill execution (#…
Browse files Browse the repository at this point in the history
…6413)

Changes:
When a trigger is evaluated for in a back-fill context, we have to make sure
that current-date is strictly after the next execution date for an execution to be eligible.

fix: #6413
  • Loading branch information
fhussonnois committed Dec 17, 2024
1 parent 122b409 commit ca00769
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 2 deletions.
12 changes: 10 additions & 2 deletions core/src/main/java/io/kestra/plugin/core/trigger/Schedule.java
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,8 @@ public Optional<Execution> evaluate(ConditionContext conditionContext, TriggerCo
RunContext runContext = conditionContext.getRunContext();
ExecutionTime executionTime = this.executionTime();
ZonedDateTime currentDateTimeExecution = convertDateTime(triggerContext.getDate());
Backfill backfill = triggerContext.getBackfill();

final Backfill backfill = triggerContext.getBackfill();

if (backfill != null) {
if (backfill.getPaused()) {
Expand All @@ -352,7 +353,14 @@ public Optional<Execution> evaluate(ConditionContext conditionContext, TriggerCo
return Optional.empty();
}

ZonedDateTime next = scheduleDates.getDate();
final ZonedDateTime next = scheduleDates.getDate();

// If the trigger is evaluated for 'back-fill', we have to make sure
// that 'current-date' is strictly after the next execution date for an execution to be eligible.
if (backfill != null && currentDateTimeExecution.isBefore(next)) {
// Otherwise, skip the execution.
return Optional.empty();
}

// we are in the future don't allow
// No use case, just here for prevention but it should never happen
Expand Down
44 changes: 44 additions & 0 deletions core/src/test/java/io/kestra/plugin/core/trigger/ScheduleTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io.kestra.core.models.Label;
import io.kestra.core.models.conditions.ConditionContext;
import io.kestra.core.models.triggers.Backfill;
import io.kestra.core.runners.DefaultRunContext;
import io.kestra.core.runners.RunContextInitializer;
import io.kestra.plugin.core.condition.DateTimeBetween;
Expand All @@ -21,6 +22,8 @@

import java.time.DayOfWeek;
import java.time.Duration;
import java.time.Instant;
import java.time.LocalTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
Expand All @@ -36,6 +39,8 @@
@KestraTest
class ScheduleTest {

private static final String TEST_CRON_EVERYDAY_AT_8 = "0 8 * * *";

@Inject
RunContextFactory runContextFactory;

Expand Down Expand Up @@ -214,6 +219,45 @@ void everySecond() throws Exception {
assertThat(dateFromVars(vars.get("previous"), date), is(date.minus(Duration.ofSeconds(1))));
}

@Test
void shouldNotReturnExecutionForBackFillWhenCurrentDateIsBeforeScheduleDate() throws Exception {
// Given
Schedule trigger = Schedule.builder().id("schedule").cron(TEST_CRON_EVERYDAY_AT_8).build();
ZonedDateTime now = ZonedDateTime.now();
TriggerContext triggerContext = triggerContext(now, trigger).toBuilder()
.backfill(Backfill
.builder()
.currentDate(ZonedDateTime.now().with(LocalTime.MIN))
.end(ZonedDateTime.now().with(LocalTime.MAX))
.build()
).build();
// When
Optional<Execution> result = trigger.evaluate(conditionContext(trigger), triggerContext);

// Then
assertThat(result.isEmpty(), is(true));
}

@Test
void shouldReturnExecutionForBackFillWhenCurrentDateIsAfterScheduleDate() throws Exception {
// Given
Schedule trigger = Schedule.builder().id("schedule").cron(TEST_CRON_EVERYDAY_AT_8).build();
ZonedDateTime now = ZonedDateTime.now();
TriggerContext triggerContext = triggerContext(now, trigger).toBuilder()
.backfill(Backfill
.builder()
.currentDate(ZonedDateTime.now().with(LocalTime.MIN).plus(Duration.ofHours(8)))
.end(ZonedDateTime.now().with(LocalTime.MAX))
.build()
)
.build();
// When
Optional<Execution> result = trigger.evaluate(conditionContext(trigger), triggerContext);

// Then
assertThat(result.isPresent(), is(true));
}

@Test
void noBackfillNextDate() throws Exception {
Schedule trigger = Schedule.builder().id("schedule").cron("0 0 * * *").build();
Expand Down

0 comments on commit ca00769

Please sign in to comment.