Review due time calculation for new cron events
Problem/Motivation
Over in #3284704: Test that events are being properly triggered there is a proposed solution to properly calculate the first due time for a cron event that had never been executed ever before. In a code review comment we have this:
Choosing "yesterday" as a fallback seems a bit arbitrary. Why not using zero instead?
If the comment was about "weekly" or even "annually" instead of "daily", then the fallback timestamp should be at that time initially.
But the example case could also be "hourly", then the fallback needed to be current time - 3600.
None of those would ever match all possible use cases, therefore the fallback 0 indicates that this job had not triggered before. That way, the cronjob is always due if it wasn't executed ever before.
One could argue that this may execute an e.g. weekly task which should execute on Mondays but the job was created on a Tuesday. Then it should not execute before next Monday.
Proposed resolution
This could be mitigated by calculating the time between the next two execution timestamps and if that's larger than the difference between the next and the current time, then it wouldn't be due yet. Maybe something like this:
<?php
if (!$lastRun) {
$dtNext1 = new \DateTime();
$dtNext1
->setTimezone(new \DateTimeZone('UTC'))
->setTimestamp($currentTime);
$next1Time = $cron->getNextRunDate($dtNext1)->getTimestamp();
$dtNext2 = new \DateTime();
$dtNext2
->setTimezone(new \DateTimeZone('UTC'))
->setTimestamp($next1Time);
$next2Time = $cron->getNextRunDate($dtNext2)->getTimestamp();
if (($next2Time - $next1Time) > ($next1Time - $currentTime)) {
$lastRun = $currentTime;
}
}
?>