Skip to content

Commit 0e9fa24

Browse files
AllenJBdragonmantank
AllenJB
and
dragonmantank
authored
DST handling fixes (fixes #111, #112) (#115)
* DST fix (attempt 1) * More tests and further fixes WIP as I believe the change to HoursField::isSatisfiedBy is going to break expressions with multiple parts - but one thing at a time! * WIP More tests and further fixes; API change: isSatisfiedBy now requires knowledge of which direction we're travelling in to handle checking initial values correctly * WIP Save Point - Trying to fix one thing breaks another, but I have an idea... * WIP Abstracted NextRunDateTime which keeps track of offset changes, regardless of whether they occurred when changing minute or hour (and to persist them until a next change is made) * WIP Fix easy fixes * WIP More test fixes * All current tests pass! * Fix for issue #112; Use a cache of timezone transitions to avoid having to modify date/time objects every single time we want to check if hour is satisfied All tests pass * Cleanup All tests pass * Cleanup All tests pass * Cleanup - backing out the NextRunDateTime abstraction; All tests pass * Cleanup; All tests pass * Cleanup (diff tidy, restoring deleted tests); All tests pass * Cleanup (diff tidy); All tests pass * Fix CI issues * Fix CI issues Co-authored-by: Chris Tankersley <[email protected]>
1 parent 2c74e61 commit 0e9fa24

15 files changed

+724
-82
lines changed

‎src/Cron/AbstractField.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace Cron;
66

7+
use DateTimeInterface;
8+
79
/**
810
* Abstract CRON expression field.
911
*/
@@ -300,4 +302,28 @@ public function validate(string $value): bool
300302

301303
return \in_array($value, $this->fullRange, true);
302304
}
305+
306+
protected function timezoneSafeModify(DateTimeInterface $dt, string $modification): DateTimeInterface
307+
{
308+
$timezone = $dt->getTimezone();
309+
$dt = $dt->setTimezone(new \DateTimeZone("UTC"));
310+
$dt = $dt->modify($modification);
311+
$dt = $dt->setTimezone($timezone);
312+
return $dt;
313+
}
314+
315+
protected function setTimeHour(DateTimeInterface $date, bool $invert, int $originalTimestamp): DateTimeInterface
316+
{
317+
$date = $date->setTime((int)$date->format('H'), ($invert ? 59 : 0));
318+
319+
// setTime caused the offset to change, moving time in the wrong direction
320+
$actualTimestamp = $date->format('U');
321+
if ((! $invert) && ($actualTimestamp <= $originalTimestamp)) {
322+
$date = $this->timezoneSafeModify($date, "+1 hour");
323+
} elseif ($invert && ($actualTimestamp >= $originalTimestamp)) {
324+
$date = $this->timezoneSafeModify($date, "-1 hour");
325+
}
326+
327+
return $date;
328+
}
303329
}

‎src/Cron/CronExpression.php

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -240,14 +240,32 @@ public function getPreviousRunDate($currentTime = 'now', int $nth = 0, bool $all
240240
*/
241241
public function getMultipleRunDates(int $total, $currentTime = 'now', bool $invert = false, bool $allowCurrentDate = false, $timeZone = null): array
242242
{
243+
$timeZone = $this->determineTimeZone($currentTime, $timeZone);
244+
245+
if ('now' === $currentTime) {
246+
$currentTime = new DateTime();
247+
} elseif ($currentTime instanceof DateTime) {
248+
$currentTime = clone $currentTime;
249+
} elseif ($currentTime instanceof DateTimeImmutable) {
250+
$currentTime = DateTime::createFromFormat('U', $currentTime->format('U'));
251+
} elseif (\is_string($currentTime)) {
252+
$currentTime = new DateTime($currentTime);
253+
}
254+
255+
Assert::isInstanceOf($currentTime, DateTime::class);
256+
$currentTime->setTimezone(new DateTimeZone($timeZone));
257+
243258
$matches = [];
244259
for ($i = 0; $i < $total; ++$i) {
245260
try {
246-
$currentTime = $this->getRunDate($currentTime, 0, $invert, $i === 0 ? $allowCurrentDate : false, $timeZone);
247-
$matches[] = $currentTime;
261+
$result = $this->getRunDate($currentTime, 0, $invert, $allowCurrentDate, $timeZone);
248262
} catch (RuntimeException $e) {
249263
break;
250264
}
265+
266+
$allowCurrentDate = false;
267+
$currentTime = clone $result;
268+
$matches[] = $result;
251269
}
252270

253271
return $matches;
@@ -364,7 +382,9 @@ protected function getRunDate($currentTime = null, int $nth = 0, bool $invert =
364382

365383
Assert::isInstanceOf($currentDate, DateTime::class);
366384
$currentDate->setTimezone(new DateTimeZone($timeZone));
367-
$currentDate->setTime((int) $currentDate->format('H'), (int) $currentDate->format('i'), 0);
385+
// Workaround for setTime causing an offset change: https://bugs.php.net/bug.php?id=81074
386+
$currentDate = DateTime::createFromFormat("!Y-m-d H:iO", $currentDate->format("Y-m-d H:iP"), $currentDate->getTimezone());
387+
$currentDate->setTimezone(new DateTimeZone($timeZone));
368388

369389
$nextRun = clone $currentDate;
370390

@@ -380,7 +400,7 @@ protected function getRunDate($currentTime = null, int $nth = 0, bool $invert =
380400
$fields[$position] = $this->fieldFactory->getField($position);
381401
}
382402

383-
if (isset($parts[2]) && isset($parts[4])) {
403+
if (isset($parts[self::DAY]) && isset($parts[self::WEEKDAY])) {
384404
$domExpression = sprintf('%s %s %s %s *', $this->getExpression(0), $this->getExpression(1), $this->getExpression(2), $this->getExpression(3));
385405
$dowExpression = sprintf('%s %s * %s %s', $this->getExpression(0), $this->getExpression(1), $this->getExpression(3), $this->getExpression(4));
386406

@@ -409,10 +429,10 @@ protected function getRunDate($currentTime = null, int $nth = 0, bool $invert =
409429
$field = $fields[$position];
410430
// Check if this is singular or a list
411431
if (false === strpos($part, ',')) {
412-
$satisfied = $field->isSatisfiedBy($nextRun, $part);
432+
$satisfied = $field->isSatisfiedBy($nextRun, $part, $invert);
413433
} else {
414434
foreach (array_map('trim', explode(',', $part)) as $listPart) {
415-
if ($field->isSatisfiedBy($nextRun, $listPart)) {
435+
if ($field->isSatisfiedBy($nextRun, $listPart, $invert)) {
416436
$satisfied = true;
417437

418438
break;
@@ -430,8 +450,7 @@ protected function getRunDate($currentTime = null, int $nth = 0, bool $invert =
430450

431451
// Skip this match if needed
432452
if ((!$allowCurrentDate && $nextRun == $currentDate) || --$nth > -1) {
433-
$this->fieldFactory->getField(0)->increment($nextRun, $invert, $parts[0] ?? null);
434-
453+
$this->fieldFactory->getField(self::MINUTE)->increment($nextRun, $invert, $parts[self::MINUTE] ?? null);
435454
continue;
436455
}
437456

@@ -458,7 +477,7 @@ protected function determineTimeZone($currentTime, ?string $timeZone): string
458477
}
459478

460479
if ($currentTime instanceof DateTimeInterface) {
461-
return $currentTime->getTimeZone()->getName();
480+
return $currentTime->getTimezone()->getName();
462481
}
463482

464483
return date_default_timezone_get();

‎src/Cron/DayOfMonthField.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ private static function getNearestWeekday(int $currentYear, int $currentMonth, i
7979
/**
8080
* {@inheritdoc}
8181
*/
82-
public function isSatisfiedBy(DateTimeInterface $date, $value): bool
82+
public function isSatisfiedBy(DateTimeInterface $date, $value, bool $invert): bool
8383
{
8484
// ? states that the field value is to be skipped
8585
if ('?' === $value) {
@@ -117,10 +117,12 @@ public function isSatisfiedBy(DateTimeInterface $date, $value): bool
117117
*/
118118
public function increment(DateTimeInterface &$date, $invert = false, $parts = null): FieldInterface
119119
{
120-
if ($invert) {
121-
$date = $date->modify('previous day')->setTime(23, 59);
120+
if (! $invert) {
121+
$date = $this->timezoneSafeModify($date, '+1 day');
122+
$date = $date->setTime(0, 0);
122123
} else {
123-
$date = $date->modify('next day')->setTime(0, 0);
124+
$date = $this->timezoneSafeModify($date, '-1 day');
125+
$date = $date->setTime(23, 59);
124126
}
125127

126128
return $this;

‎src/Cron/DayOfWeekField.php

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
namespace Cron;
66

7-
use DateTime;
87
use DateTimeInterface;
98
use InvalidArgumentException;
109

@@ -54,10 +53,8 @@ public function __construct()
5453

5554
/**
5655
* @inheritDoc
57-
*
58-
* @param \DateTime|\DateTimeImmutable $date
5956
*/
60-
public function isSatisfiedBy(DateTimeInterface $date, $value): bool
57+
public function isSatisfiedBy(DateTimeInterface $date, $value, bool $invert): bool
6158
{
6259
if ('?' === $value) {
6360
return true;
@@ -76,15 +73,9 @@ public function isSatisfiedBy(DateTimeInterface $date, $value): bool
7673
$weekday = $this->convertLiterals(substr($value, 0, strpos($value, 'L')));
7774
$weekday %= 7;
7875

79-
$tdate = clone $date;
80-
$tdate = $tdate->setDate($currentYear, $currentMonth, $lastDayOfMonth);
81-
while ($tdate->format('w') != $weekday) {
82-
$tdateClone = new DateTime();
83-
$tdate = $tdateClone->setTimezone($tdate->getTimezone())
84-
->setDate($currentYear, $currentMonth, --$lastDayOfMonth);
85-
}
86-
87-
return (int) $date->format('j') === $lastDayOfMonth;
76+
$daysInMonth = (int) $date->format('t');
77+
$remainingDaysInMonth = $daysInMonth - (int) $date->format('d');
78+
return (($weekday === (int) $date->format('w')) && ($remainingDaysInMonth < 7));
8879
}
8980

9081
// Handle # hash tokens
@@ -156,15 +147,15 @@ public function isSatisfiedBy(DateTimeInterface $date, $value): bool
156147

157148
/**
158149
* @inheritDoc
159-
*
160-
* @param \DateTime|\DateTimeImmutable $date
161150
*/
162151
public function increment(DateTimeInterface &$date, $invert = false, $parts = null): FieldInterface
163152
{
164-
if ($invert) {
165-
$date = $date->modify('-1 day')->setTime(23, 59, 0);
153+
if (! $invert) {
154+
$date = $this->timezoneSafeModify($date, '+1 day');
155+
$date = $date->setTime(0, 0);
166156
} else {
167-
$date = $date->modify('+1 day')->setTime(0, 0, 0);
157+
$date = $this->timezoneSafeModify($date, '-1 day');
158+
$date = $date->setTime(23, 59);
168159
}
169160

170161
return $this;

‎src/Cron/FieldInterface.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ interface FieldInterface
1919
*
2020
* @return bool Returns TRUE if satisfied, FALSE otherwise
2121
*/
22-
public function isSatisfiedBy(DateTimeInterface $date, $value): bool;
22+
public function isSatisfiedBy(DateTimeInterface $date, $value, bool $invert): bool;
2323

2424
/**
2525
* When a CRON expression is not satisfied, this method is used to increment

0 commit comments

Comments
 (0)