Scheduler: fix Trigger#getNextFireTime() for cron-based jobs

[mkouba]

• fixes quarkus-scheduler trigger getNextFireTime does not consider cron timezone #41717

[mkouba]

@alanmscott it would be great if you could test this PR!

[alanmscott]

Hi @mkouba

I've had a test with the new SimpleScheduler. Whilst it is casting the Instant into the value correctly, it still has the problem where it can return a "nextFireTime" in the past.

E.g.

USING: cron = "0 35 12 ? * * *";

2024-07-09 11:58:25,719 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) NextFireTimes:
2024-07-09 11:58:25,721 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob1(UTC     )=2024-07-09T12:35:00Z / 2024-07-09T12:35:00Z[UTC]
2024-07-09 11:58:25,722 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob2(London  )=2024-07-09T11:35:00Z / 2024-07-09T12:35:00+01:00[Europe/London]
2024-07-09 11:58:25,722 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob3(Berlin  )=2024-07-09T10:35:00Z / 2024-07-09T12:35:00+02:00[Europe/Berlin]
2024-07-09 11:58:25,722 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob4(Istanbul)=2024-07-09T09:35:00Z / 2024-07-09T12:35:00+03:00[Europe/Istanbul]
2024-07-09 11:58:25,722 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) compareTo result1-2=1     <-- should be greater than 0
2024-07-09 11:58:25,723 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) compareTo result2-3=1     <-- should be greater than 0
2024-07-09 11:58:25,723 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) compareTo result3-4=1     <-- should be greater than 0

As you can see, the local time (London time) is 09/07/2024 11:58.
The UTC and London nextFireTime is perfectly correct, but for the others, it shows an almost sensible time, but it's actually in the past.
i.e. at 11:58 in London, the time in Berlin is 12:58, so the nextFireTime should be 10/07/2024 12:35
Similarly, for Istanbul, the time is 13:58, so the nextFireTime should 10/07/2024 12:35.

Is perhaps a problem with the value being returned by the underlying cronutils library?

[mkouba]

Is perhaps a problem with the value being returned by the underlying cronutils library?

Hi @alanmscott, thanks a lot for testing this PR!

You're right and I don't think it's the problem of cronutils. It's the way we adapt the next fire time because we retain the local date-time and change the timezone. Which is not correct. Let me try to fix that. Oh, timezones 🤦.

[mkouba]

@alanmscott Ok, another attempt. Note that I've removed the comparison of instants because it really depends on the current time.

[alanmscott]

I think the latest change looks good. It's returning what I'd expect for the test harness:

USING Cron = "0 06 17 ? * * *";

2024-07-09 16:05:36,103 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) NextFireTimes:
2024-07-09 16:05:36,106 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob1(UTC     )=2024-07-09T17:06:00Z / 2024-07-09T17:06:00Z[UTC]
2024-07-09 16:05:36,107 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob2(London  )=2024-07-09T16:06:00Z / 2024-07-09T17:06:00+01:00[Europe/London]
2024-07-09 16:05:36,107 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob3(Berlin  )=2024-07-09T15:06:00Z / 2024-07-09T17:06:00+02:00[Europe/Berlin]
2024-07-09 16:05:36,107 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob4(Istanbul)=2024-07-10T14:06:00Z / 2024-07-10T17:06:00+03:00[Europe/Istanbul]

Which is right. 17:06 for London = 18:06 for Berlin, so that is still "today", but has passed for Istanbul which is now "tomorrow".
I noted that the trigger still executed for "job3" also, demonstrating the triggering is unchanged and correct.

Fantastic, thank you!!!

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 30af1a6.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.