diff --git a/src/Illuminate/Console/Scheduling/CronExpressionTimezoneConverter.php b/src/Illuminate/Console/Scheduling/CronExpressionTimezoneConverter.php index d15b8185ad79..58b47ff1cf98 100644 --- a/src/Illuminate/Console/Scheduling/CronExpressionTimezoneConverter.php +++ b/src/Illuminate/Console/Scheduling/CronExpressionTimezoneConverter.php @@ -5,6 +5,7 @@ use DateTimeZone; use Illuminate\Support\Carbon; use Illuminate\Support\Collection; +use Throwable; class CronExpressionTimezoneConverter { @@ -22,33 +23,20 @@ public static function forEvent(Event $event, DateTimeZone $timezone) $eventTimezone = static::resolveEventTimezone($event, $timezone); [$totalOffsetMinutes, $hourOffset, $minuteOffset] = static::offsetComponents( - $eventTimezone, $timezone + $event, $eventTimezone, $timezone ); if ($totalOffsetMinutes === 0) { return [$event->expression]; } - $segments = preg_split("/\s+/", $event->expression); - $minuteGroups = static::shiftAndGroup($segments[0], $minuteOffset, 60); + $segments = preg_split("/\s+/", trim($event->expression)); - $expressions = []; - - foreach ($minuteGroups as $minuteCarry => $minuteValues) { - $hourGroups = static::shiftAndGroup($segments[1], $hourOffset + $minuteCarry, 24); - - foreach ($hourGroups as $hourCarry => $hourValues) { - $parts = $segments; - $parts[0] = $minuteValues; - $parts[1] = $hourValues; - - foreach (static::expressionsForHourCarry($segments, $parts, $hourCarry) as $expression) { - $expressions[] = $expression; - } - } + if (count($segments) !== 5) { + return [$event->expression]; } - return $expressions; + return static::convert($segments, $hourOffset, $minuteOffset) ?? [$event->expression]; } /** @@ -70,24 +58,76 @@ protected static function resolveEventTimezone(Event $event, DateTimeZone $defau * * @return array{int, int, int} */ - protected static function offsetComponents(DateTimeZone $eventTimezone, DateTimeZone $displayTimezone) + protected static function offsetComponents(Event $event, DateTimeZone $eventTimezone, DateTimeZone $displayTimezone) { - $now = Carbon::now(); + try { + $at = $event->nextRunDate(Carbon::now()); + } catch (Throwable) { + $at = Carbon::now(); + } $totalOffsetMinutes = intdiv( - $displayTimezone->getOffset($now) - $eventTimezone->getOffset($now), + $displayTimezone->getOffset($at) - $eventTimezone->getOffset($at), 60 ); return [$totalOffsetMinutes, intdiv($totalOffsetMinutes, 60), $totalOffsetMinutes % 60]; } + /** + * Convert the expression segments by the given offsets. + * + * @param array $segments + * @param int $hourOffset + * @param int $minuteOffset + * @return array|null + */ + protected static function convert(array $segments, int $hourOffset, int $minuteOffset) + { + $daysAreWildcard = $segments[2] === '*' && $segments[3] === '*' && $segments[4] === '*'; + + $minuteGroups = static::shiftedGroups( + $segments[0], $minuteOffset, 0, 59, + mergeCarries: $daysAreWildcard && static::expand($segments[1], 0, 23) === range(0, 23), + ); + + if (is_null($minuteGroups)) { + return null; + } + + $expressions = []; + + foreach ($minuteGroups as $minuteCarry => $minuteValues) { + $hourGroups = static::shiftedGroups( + $segments[1], $hourOffset + $minuteCarry, 0, 23, mergeCarries: $daysAreWildcard, + ); + + if (is_null($hourGroups)) { + return null; + } + + foreach ($hourGroups as $hourCarry => $hourValues) { + $parts = $segments; + $parts[0] = $minuteValues; + $parts[1] = $hourValues; + + if (is_null($carried = static::expressionsForHourCarry($segments, $parts, $hourCarry))) { + return null; + } + + $expressions = array_merge($expressions, $carried); + } + } + + return $expressions; + } + /** * Build expressions for the given hour carry direction. * * @param array $segments * @param array $parts - * @return array + * @return array|null */ protected static function expressionsForHourCarry(array $segments, array $parts, int $hourCarry) { @@ -95,83 +135,310 @@ protected static function expressionsForHourCarry(array $segments, array $parts, return [implode(' ', $parts)]; } - $parts[4] = static::shiftField($segments[4], $hourCarry, 7); + if ($segments[4] !== '*') { + // Cron treats restricted day-of-month and day-of-week fields as an "or", so they cannot shift together... + if ($segments[2] !== '*' || $segments[3] !== '*') { + return null; + } - $dayGroups = static::shiftAndGroup($segments[2], $hourCarry, 31, min: 1); + if (is_null($days = static::expand($segments[4], 0, 7))) { + return null; + } - $expressions = []; + $parts[4] = static::collapse(static::shiftWrapped($days, $hourCarry, 7), 0, 6); - foreach ($dayGroups as $dayCarry => $dayValues) { - $dayParts = $parts; - $dayParts[2] = $dayValues; + return [implode(' ', $parts)]; + } - if ($dayCarry !== 0) { - $dayParts[3] = static::shiftField($segments[3], $dayCarry, 12, min: 1); + if ($segments[2] === '*' && $segments[3] === '*') { + return [implode(' ', $parts)]; + } + + if (is_null($dayGroups = static::shiftDaysOfMonth($segments[2], $segments[3], $hourCarry))) { + return null; + } + + return array_map(function ($group) use ($parts) { + [$parts[2], $parts[3]] = $group; + + return implode(' ', $parts); + }, $dayGroups); + } + + /** + * Shift the day-of-month field by the given carry, respecting month lengths. + * + * @param string $domField + * @param string $monthField + * @param int $carry + * @return array|null + */ + protected static function shiftDaysOfMonth(string $domField, string $monthField, int $carry) + { + $months = $monthField === '*' ? range(1, 12) : static::expand($monthField, 1, 12); + $days = $domField === '*' ? range(1, 31) : static::expand($domField, 1, 31); + + if (is_null($months) || is_null($days)) { + return null; + } + + $shifted = []; + + foreach ($months as $month) { + foreach ($days as $day) { + if ($month === 2 && $day === 29) { + return null; + } + + if ($day > static::daysInMonth($month)) { + continue; + } + + [$targetMonth, $targetDay] = [$month, $day + $carry]; + + if ($targetDay < 1) { + $targetMonth = $month === 1 ? 12 : $month - 1; + + if ($targetMonth === 2) { + return null; + } + + $targetDay = static::daysInMonth($targetMonth); + } elseif ($month === 2 && $targetDay > 28) { + return null; + } elseif ($targetDay > static::daysInMonth($month)) { + [$targetMonth, $targetDay] = [$month === 12 ? 1 : $month + 1, 1]; + } + + $shifted[$targetMonth][$targetDay] = true; } + } - $expressions[] = implode(' ', $dayParts); + if ($shifted === []) { + return null; } - return $expressions; + $groups = []; + + foreach ($shifted as $month => $days) { + $days = array_keys($days); + + sort($days); + + $groups[implode(',', $days)][] = $month; + } + + return (new Collection($groups))->map(function ($months, $days) { + sort($months); + + return [ + static::collapse(array_map(intval(...), explode(',', $days)), 1, 31), + static::collapse($months, 1, 12), + ]; + })->values()->all(); } /** - * Shift values in a cron field and group them by carry direction. + * Get the number of days in the given month of a non-leap year. + * + * @param int $month + * @return int + */ + protected static function daysInMonth(int $month) + { + return Carbon::create(2023, $month)->daysInMonth; + } + + /** + * Shift a cron field by the given offset and group it by carry direction. * * @param string $field * @param int $offset - * @param int $mod - * @return array + * @param int $min + * @param int $max + * @param bool $mergeCarries + * @return array|null */ - protected static function shiftAndGroup($field, $offset, $mod, $min = 0) + protected static function shiftedGroups(string $field, int $offset, int $min, int $max, bool $mergeCarries) { - if ($offset === 0 || ! preg_match('/^[\d,]+$/', $field)) { + if ($offset === 0) { return [0 => $field]; } - $groups = []; + if (is_null($values = static::expand($field, $min, $max))) { + return null; + } - foreach (explode(',', $field) as $value) { - $new = (int) $value + $offset; - $carry = 0; + $groups = static::shiftAndGroupValues($values, $offset, $max - $min + 1, $min); - if ($new >= $mod + $min) { - $carry = 1; - $new -= $mod; - } elseif ($new < $min) { - $carry = -1; - $new += $mod; + if ($mergeCarries && count($groups) > 1) { + $groups = [0 => static::mergeGroups($groups)]; + } + + return array_map(fn ($group) => static::collapse($group, $min, $max), $groups); + } + + /** + * Expand a cron field into the sorted list of values it matches. + * + * @param string $field + * @param int $min + * @param int $max + * @return array|null + */ + protected static function expand(string $field, int $min, int $max) + { + if ($field === '*') { + return range($min, $max); + } + + $values = []; + + foreach (explode(',', $field) as $part) { + if (! preg_match('/^(\*|\d+(?:-\d+)?)(?:\/(\d+))?$/', $part, $matches)) { + return null; + } + + $step = ($matches[2] ?? '') !== '' ? (int) $matches[2] : null; + + if ($step === 0) { + return null; + } + + if ($matches[1] === '*') { + [$start, $end] = [$min, $max]; + } elseif (str_contains($matches[1], '-')) { + [$start, $end] = array_map(intval(...), explode('-', $matches[1])); + } else { + [$start, $end] = [(int) $matches[1], is_null($step) ? (int) $matches[1] : $max]; + } + + if ($start < $min || $end > $max || $start > $end) { + return null; } - $groups[$carry][] = $new; + for ($value = $start; $value <= $end; $value += $step ?? 1) { + $values[] = $value; + } } - return (new Collection($groups))->map(function ($values) { - sort($values); + $values = array_values(array_unique($values)); + + sort($values); - return implode(',', $values); - })->all(); + return $values; } /** - * Shift a cron field by the given offset. + * Shift values in a cron field and group them by carry direction. * - * @param string $field + * @param array $values * @param int $offset * @param int $mod * @param int $min + * @return array> + */ + protected static function shiftAndGroupValues(array $values, int $offset, int $mod, int $min = 0) + { + $groups = []; + + foreach ($values as $value) { + $carry = (int) floor(($value + $offset - $min) / $mod); + + $groups[$carry][] = $value + $offset - $carry * $mod; + } + + return array_map(function ($group) { + sort($group); + + return $group; + }, $groups); + } + + /** + * Shift values within a cron field, wrapping around its domain. + * + * @param array $values + * @param int $offset + * @param int $mod + * @param int $min + * @return array + */ + protected static function shiftWrapped(array $values, int $offset, int $mod, int $min = 0) + { + $shifted = array_values(array_unique(array_map( + fn ($value) => (($value + $offset - $min) % $mod + $mod) % $mod + $min, $values + ))); + + sort($shifted); + + return $shifted; + } + + /** + * Merge grouped values into a single sorted group. + * + * @param array> $groups + * @return array + */ + protected static function mergeGroups(array $groups) + { + $merged = array_merge(...array_values($groups)); + + sort($merged); + + return $merged; + } + + /** + * Collapse a sorted list of values into the most compact cron field syntax. + * + * @param array $values + * @param int $min + * @param int $max * @return string */ - protected static function shiftField($field, $offset, $mod, $min = 0) + protected static function collapse(array $values, int $min, int $max) { - if ($offset === 0 || ! preg_match('/^[\d,]+$/', $field)) { - return $field; + if ($values === range($min, $max)) { + return '*'; + } + + if (count($values) === 1) { + return (string) $values[0]; + } + + $steps = (new Collection($values)) + ->sliding(2) + ->map(fn ($pair) => $pair->last() - $pair->first()) + ->unique(); + + if (count($values) < 3 || $steps->count() > 1) { + return static::collapseRuns($values); } - $shifted = (new Collection(explode(',', $field))) - ->map(fn ($v) => (((int) $v + $offset - $min) % $mod + $mod) % $mod + $min) - ->sort(); + [$first, $last, $step] = [$values[0], end($values), $steps->first()]; + + if ($step === 1) { + return "{$first}-{$last}"; + } + + return $first === $min && $last + $step > $max + ? "*/{$step}" + : "{$first}-{$last}/{$step}"; + } - return $shifted->implode(','); + /** + * Collapse consecutive runs of three or more values into ranges. + * + * @param array $values + * @return string + */ + protected static function collapseRuns(array $values) + { + return (new Collection($values)) + ->chunkWhile(fn ($value, $key, $chunk) => $value === $chunk->last() + 1) + ->map(fn ($run) => $run->count() >= 3 ? $run->first().'-'.$run->last() : $run->implode(',')) + ->implode(','); } } diff --git a/tests/Integration/Console/Scheduling/ScheduleListCommandTest.php b/tests/Integration/Console/Scheduling/ScheduleListCommandTest.php index 884c948c1ea3..8c1e080e70ca 100644 --- a/tests/Integration/Console/Scheduling/ScheduleListCommandTest.php +++ b/tests/Integration/Console/Scheduling/ScheduleListCommandTest.php @@ -256,13 +256,15 @@ public static function expressionTimezoneConversionProvider() // No conversion needed — same timezone 'same timezone' => ['0 8 * * *', 'UTC', null, ['0 8 * * *']], - // Wildcards/steps — pass through unchanged + // Wildcards — shift-invariant when the day fields are wildcards 'every minute' => ['* * * * *', 'America/New_York', null, ['* * * * *']], 'every five minutes' => ['*/5 * * * *', 'Asia/Tokyo', null, ['*/5 * * * *']], - 'every two hours' => ['0 */2 * * *', 'Asia/Tokyo', null, ['0 */2 * * *']], - 'every odd hour' => ['0 1-23/2 * * *', 'Asia/Tokyo', null, ['0 1-23/2 * * *']], - 'quarterly step month' => ['0 0 1 1-12/3 *', 'Asia/Tokyo', null, ['0 15 31 1-12/3 *']], - 'weekdays range' => ['0 8 * * 1-5', 'Asia/Tokyo', null, ['0 23 * * 1-5']], + + // Steps/ranges — expanded, shifted, and re-collapsed + 'every two hours' => ['0 */2 * * *', 'Asia/Tokyo', null, ['0 1-23/2 * * *']], + 'every odd hour' => ['0 1-23/2 * * *', 'Asia/Tokyo', null, ['0 */2 * * *']], + 'quarterly step month' => ['0 0 1 1-12/3 *', 'Asia/Tokyo', null, ['0 15 31 3,12 *', '0 15 30 6,9 *']], + 'weekdays range' => ['0 8 * * 1-5', 'Asia/Tokyo', null, ['0 23 * * 0-4']], // Simple hour shift (no day boundary) 'daily LA to UTC' => ['0 8 * * *', 'America/Los_Angeles', null, ['0 16 * * *']], @@ -281,9 +283,9 @@ public static function expressionTimezoneConversionProvider() 'twice daily Tokyo' => ['0 9,17 * * *', 'Asia/Tokyo', null, ['0 0,8 * * *']], 'twice daily Tokyo wrapping' => ['0 13,22 * * *', 'Asia/Tokyo', null, ['0 4,13 * * *']], - // Comma-separated hours — mixed carries (splits into two entries) - 'twice daily LA mixed carry' => ['0 13,17 * * *', 'America/Los_Angeles', null, ['0 21 * * *', '0 1 * * *']], - 'twice daily Tokyo mixed carry' => ['0 3,20 * * *', 'Asia/Tokyo', null, ['0 18 * * *', '0 11 * * *']], + // Comma-separated hours — mixed carries merge when the day fields are wildcards + 'twice daily LA mixed carry' => ['0 13,17 * * *', 'America/Los_Angeles', null, ['0 1,21 * * *']], + 'twice daily Tokyo mixed carry' => ['0 3,20 * * *', 'Asia/Tokyo', null, ['0 11,18 * * *']], // Day-of-week shifts 'weekly Monday night LA' => ['0 22 * * 1', 'America/Los_Angeles', null, ['0 6 * * 2']], @@ -294,18 +296,20 @@ public static function expressionTimezoneConversionProvider() // Day-of-month shifts 'monthly 15th Tokyo (no day wrap)' => ['0 12 15 * *', 'Asia/Tokyo', null, ['0 3 15 * *']], 'monthly 15th Tokyo (day wraps back)' => ['0 8 15 * *', 'Asia/Tokyo', null, ['0 23 14 * *']], - 'monthly 1st Tokyo (day wraps to 31)' => ['0 1 1 * *', 'Asia/Tokyo', null, ['0 16 31 * *']], + // day 1 wraps into the previous month, whose last day varies (incl. February) — unchanged + 'monthly 1st Tokyo (day wraps back)' => ['0 1 1 * *', 'Asia/Tokyo', null, ['0 1 1 * *']], 'monthly 1st LA (day wraps forward)' => ['0 22 1 * *', 'America/Los_Angeles', null, ['0 6 2 * *']], - // Month shifts (day-of-month carry propagates to month) + // Month shifts (day-of-month carry respects month lengths) 'yearly Jan 1 Tokyo' => ['0 1 1 1 *', 'Asia/Tokyo', null, ['0 16 31 12 *']], - 'yearly Jul 1 Tokyo' => ['0 1 1 7 *', 'Asia/Tokyo', null, ['0 16 31 6 *']], + 'yearly Jul 1 Tokyo' => ['0 1 1 7 *', 'Asia/Tokyo', null, ['0 16 30 6 *']], 'yearly Dec 31 LA' => ['0 22 31 12 *', 'America/Los_Angeles', null, ['0 6 1 1 *']], + 'dom 31 across months' => ['0 1 31 * *', 'Asia/Tokyo', null, ['0 16 30 1,3,5,7,8,10,12 *']], // Comma day-of-month 'twice monthly Tokyo (no wrap)' => ['0 12 1,16 * *', 'Asia/Tokyo', null, ['0 3 1,16 * *']], - // day 1→31 (carry -1 to month) and 16→15 (no carry) — splits - 'twice monthly Tokyo (wraps)' => ['0 1 1,16 * *', 'Asia/Tokyo', null, ['0 16 31 * *', '0 16 15 * *']], + // day 1 wraps into a month whose last day varies — unchanged + 'twice monthly Tokyo (wraps)' => ['0 1 1,16 * *', 'Asia/Tokyo', null, ['0 1 1,16 * *']], // Comma day-of-week 'weekends LA night' => ['0 22 * * 0,6', 'America/Los_Angeles', null, ['0 6 * * 0,1']], @@ -317,6 +321,50 @@ public static function expressionTimezoneConversionProvider() // Comma minutes with half-hour timezone — mixed minute carries (splits) // 15+(-30)=-15→45 (carry -1) and 45+(-30)=15 (no carry) 'comma minutes Kolkata mixed carry' => ['15,45 8 * * *', 'Asia/Kolkata', null, ['45 2 * * *', '15 3 * * *']], + + // Hour ranges + 'hour range NY' => ['0 0-4 * * *', 'America/New_York', null, ['0 5-9 * * *']], + 'hour range with minute step Amsterdam' => ['*/10 8-23 * * *', 'Europe/Amsterdam', null, ['*/10 7-22 * * *']], + 'hour range with minute step Johannesburg' => ['*/10 8-23 * * *', 'Africa/Johannesburg', null, ['*/10 6-21 * * *']], + 'hour range LA' => ['0 8-11 * * *', 'America/Los_Angeles', null, ['0 16-19 * * *']], + + // Ranges/steps with half-hour timezone offsets + 'minute step Kolkata' => ['*/10 * * * *', 'Asia/Kolkata', null, ['*/10 * * * *']], + 'hour range Kolkata' => ['0 9-17 * * *', 'Asia/Kolkata', null, ['30 3-11 * * *']], + 'hour range Kathmandu' => ['0 9-11 * * *', 'Asia/Kathmandu', null, ['15 3-5 * * *']], + + // Day-of-week ranges shift with the day carry + 'weeknights range LA' => ['0 22 * * 1-5', 'America/Los_Angeles', null, ['0 6 * * 2-6']], + 'dow range wraps through Sunday' => ['0 3 * * 0-2', 'Asia/Tokyo', null, ['0 18 * * 0,1,6']], + + // Steps/wildcards split when a day field is fixed (no merge) + 'hour step with fixed day' => ['0 */2 15 * *', 'Asia/Tokyo', null, ['0 15-23/2 14 * *', '0 1-13/2 15 * *']], + 'wildcard hour with fixed day' => ['30 * 15 * *', 'Asia/Tokyo', null, ['30 15-23 14 * *', '30 0-14 15 * *']], + 'wildcard minute with half-hour offset' => ['* 8 * * *', 'Asia/Kolkata', null, ['30-59 2 * * *', '0-29 3 * * *']], + + // Unsupported syntax in a field that needs shifting — unchanged + 'named dow range needing shift' => ['0 8 * * MON-FRI', 'Asia/Tokyo', null, ['0 8 * * MON-FRI']], + 'last day of month needing shift' => ['0 8 L * *', 'Asia/Tokyo', null, ['0 8 L * *']], + + // Unsupported syntax in a field that does not need shifting — converts + 'named dow without day carry' => ['0 14 * * MON', 'Asia/Tokyo', null, ['0 5 * * MON']], + + // Wildcard dom with a restricted month splits at the month boundary + 'wildcard dom restricted month' => ['0 1 * 1 *', 'Asia/Tokyo', null, ['0 16 31 12 *', '0 16 1-30 1 *']], + + // Restricted dom and dow combine with "or" semantics — unchanged + 'dom and dow both restricted' => ['0 1 1 3 1', 'Asia/Tokyo', null, ['0 1 1 3 1']], + 'dow with restricted month' => ['0 1 * 1 1', 'Asia/Tokyo', null, ['0 1 * 1 1']], + + // February's length depends on the year — unchanged + 'into February' => ['0 1 1 3 *', 'Asia/Tokyo', null, ['0 1 1 3 *']], + 'out of February' => ['0 23 28 2 *', 'America/Los_Angeles', null, ['0 23 28 2 *']], + 'February 29th' => ['0 1 29 2 *', 'Asia/Tokyo', null, ['0 1 29 2 *']], + 'wildcard dom in February' => ['0 1 * 2 *', 'Asia/Tokyo', null, ['0 1 * 2 *']], + + // February shifts that do not depend on the year — convert + 'within February' => ['0 1 15 2 *', 'Asia/Tokyo', null, ['0 16 14 2 *']], + 'into February 1st' => ['0 22 31 1 *', 'America/Los_Angeles', null, ['0 6 1 2 *']], ]; } @@ -343,15 +391,44 @@ public function testExpressionTimezoneConversion($expression, $eventTimezone, $d } } + public function testExpressionTimezoneConversionUsesOffsetAtNextRunDate() + { + // Listed during BST (UTC+1), but both events next run after DST has + // ended, so they convert with the UTC+0 offset in effect when they run. + Carbon::setTestNow('2023-10-29'); + + $this->schedule->command('inspire')->cron('0 0 30 10 *')->timezone('Europe/London'); + $this->schedule->command('inspire')->cron('0 0 30 6 *')->timezone('Europe/London'); + + $this->withoutMockingConsoleOutput()->artisan(ScheduleListCommand::class, [ + '--timezone' => 'UTC', + '--json' => true, + ]); + + $data = json_decode(Artisan::output(), true); + + $this->assertSame(['0 0 30 10 *', '0 23 29 6 *'], array_column($data, 'expression')); + } + public function testDisplayScheduleCliSplitsExpressionWhenMixedCarry() { - // 13+8=21 (no carry), 17+8=1 (carry +1) — splits into two CLI rows + // 13+8=21 (no carry), 17+8=1 (carry +1) — the fixed day forces two CLI rows + $this->schedule->command('inspire')->cron('0 13,17 1 * *')->timezone('America/Los_Angeles'); + + $this->artisan(ScheduleListCommand::class) + ->assertSuccessful() + ->expectsOutputToContain('0 21 1 * *') + ->expectsOutputToContain('0 1 2 * *'); + } + + public function testDisplayScheduleCliMergesMixedCarryWhenDaysAreWildcards() + { + // 13+8=21 (no carry), 17+8=1 (carry +1) — wildcard days merge into one CLI row $this->schedule->command('inspire')->twiceDaily(13, 17)->timezone('America/Los_Angeles'); $this->artisan(ScheduleListCommand::class) ->assertSuccessful() - ->expectsOutputToContain('0 21 * * *') - ->expectsOutputToContain('0 1 * * *'); + ->expectsOutputToContain('0 1,21 * * *'); } public function testDisplayScheduleCliConvertsExpression()