jmrozanec / cron-utils

Cron utils for parsing, validations and human readable descriptions as well as date/time interoperability.
http://cron-utils.com
Apache License 2.0
1.15k stars 262 forks source link

LastExecution comes up with incorrect day when reference month is shorter than month of last execution #611

Open paul-feuer opened 1 year ago

paul-feuer commented 1 year ago

I have a use case where, when i start up, i want to see if i missed anything while down.

M general logic is to check for the nextExecution from my last recorded execution and see if i missed anything.

In the event i don't have a last recorded run, i use the default of lastExecution.

This approach fails in this case because, when I asked in November for the last execution, it told me a time that was the penultimate execution; therefore when I ask for the next execution from that, I get something, instead of seeing the cron's end.

  @Test
  void shouldFindCorrectLastExecTimeWhenDaysOfMonthsAreDifferent() {
    final var parser = new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.QUARTZ));
    // last run is in a month with 31 days
    final var execTime = ExecutionTime.forCron(parser.parse("* 30 23 * * ? 2022"));

    // this calculates 2023-12-30T23:30:00Z which is wrong
    // the candidate months when iterating back do not include day=31 bc the list was derived from november 
    // (note that if you change the input to october, it works)
    final var lastExec = execTime.lastExecution(Instant.parse("2023-11-24T12:00:00Z").atZone(ZoneId.of("UTC"))).get();

    // here, i would expect next exec to be empty bc it ended in 2022, but instead next exec is 2022-12-31 23:30 bc
    // it incorrectly calculates the lastExecution using my current month's number of days to generate the
    // possible candidate times
    final var nextExec = execTime.nextExecution(lastExec);

    System.out.println("Last Exec: "+ lastExec);  
    System.out.println("Next Exec: "+ nextExec); 
    assertThat(lastExec, equalTo("2022-12-31T23:30:00Z"));
  }
jmrozanec commented 12 months ago

@paul-feuer thank you for reporting this. May we ask you to create a PR with the test? Thanks! 😄