ScheduledExecution.getTrigger().getPreviousFireTime() returns the same time as ScheduledExecution.getFireTime()

[rudiger3d]

Describe the bug

The javaoc for Trigger.getPreviousFireTime() says:

Returns: the previous time at which the trigger fired, or null if it has not fired yet

However when run using Quartz and cron-expressions it returns the same time as ScheduledExecution.getFireTime() (modulo milliseconds)

Expected behavior

Given

    @Scheduled(cron = "0 0/1 * ? * *", identity = "task-job")
    void schedule(ScheduledExecution execution) {
        execution.getTrigger().getPreviousFireTime();
    }

When reading the Javadoc for io.quarkus.scheduler.Scheduled, io.quarkus.scheduler.ScheduledExecution and io.quarkus.scheduler.Trigger I would assume that the first time my @Scheduled annotated method is executed at time T, that the value of SceduledExecution.getTriggger().getPreviousFireTime() is null. And the second time it is executed at time T+1 that the value is T.

Actual behavior

That the value of execution.getTrigger().getPreviousFireTime(); at time T is T and at T+1 the value is T+1

If I'm misinterpreting the javadoc it is unexpected that the ScheduledExecution and Trigger interfaces have different methods that returns the same value.

How to Reproduce?

1. clone https://github.com/quarkusio/quarkus-quickstarts and cd into quartz-quickstart
2. Modify the TaskBean.schedule() method to:

    @Scheduled(cron = "0/30 * * ? * *", identity = "task-job")
    void schedule(ScheduledExecution execution) {
        System.out.println("firetime         " + execution.getFireTime());
        System.out.println("previousfiretime " + execution.getTrigger().getPreviousFireTime());
    }

3. run the example

Output of uname -a or ver

Darwin DK4N9LVCF.local 21.6.0 Darwin Kernel Version 21.6.0: Wed Oct 4 23:55:28 PDT 2023; root:xnu-8020.240.18.704.15~1/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "17.0.5" 2022-10-18 OpenJDK Runtime Environment Temurin-17.0.5+8 (build 17.0.5+8) OpenJDK 64-Bit Server VM Temurin-17.0.5+8 (build 17.0.5+8, mixed mode, sharing)

Quarkus version or git rev

3.6.1

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

Also tested using MSSQL as database. I get the same result when using quarkus-scheduler, i.e. not quarkus-quartz.

Am I just mis-interpreting the javadoc? If so, it would make a lot of sense to have a getPreviousFireTime() method on ScheduledExecution that returned T-1 at time T.

[quarkus-bot]

/cc @mkouba (scheduler)

[rudiger3d]

As the behaviour is uniform across two different databases (quartz) and the simple scheduler as well this must be working as intended.
With no prior knowledge to scheduling and the API behind, this is not very intuitive :)

[mkouba]

Hm, I think that the ScheduledExecution.getFireTime() was intended to reflect the exact time a scheduled method/task was executed. While the Trigger#getPreviousFireTime() should reflect the time the trigger was fired which is not necessarily the same thing. A trigger is fired by the scheduler when the conditions are met (e.g. one second passed) and then a relevant scheduled method/task is scheduled for execution (based on its attributes, e.g. blocking/reactive etc.).

I agree that the javadoc is not clear at all.

Furthermore, the Quartz implementation does not correspond to the original intent.

We should clarify the docs/javadoc and fix the Quartz impl.

[mkouba]

I think that I was actually wrong. The reason why the Trigger#getPreviousFireTime() is sometimes the same as ScheduledExecution.getFireTime() is that trigger is a mutable object that reflects the current state whereas the ScheduledExecution is immutable. In other words, ScheduledExecution.getFireTime() always returns the same value whereas Trigger#getPreviousFireTime() returns the actual value.

A javadoc clarification would not hurt though.