Skip to content

Commit

Permalink
chore(cron): improved stability of cron interval handling
Browse files Browse the repository at this point in the history
  • Loading branch information
jeabakker committed Jul 27, 2023
1 parent 2ac3d7c commit b6f2565
Showing 1 changed file with 62 additions and 44 deletions.
106 changes: 62 additions & 44 deletions engine/classes/Elgg/Cron.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace Elgg;

use Elgg\Exceptions\CronException;
use Elgg\Exceptions\Exception;
use Elgg\I18n\Translator;
use Elgg\Traits\Loggable;
use Elgg\Traits\TimeUsing;
use GO\Job;
Expand All @@ -18,10 +20,7 @@ class Cron {
use Loggable;
use TimeUsing;

/**
* @var array
*/
protected $default_intervals = [
protected array $default_intervals = [
'minute' => '* * * * *',
'fiveminute' => '*/5 * * * *',
'fifteenmin' => '*/15 * * * *',
Expand All @@ -33,30 +32,31 @@ class Cron {
'yearly' => '0 0 1 1 *',
];

/**
* @var EventsService
*/
protected $events;
protected EventsService $events;

protected Translator $translator;

/**
* Constructor
*
* @param EventsService $events Events service
* @param EventsService $events Events service
* @param Translator $translator Translator service
*/
public function __construct(EventsService $events) {
public function __construct(EventsService $events, Translator $translator) {
$this->events = $events;
$this->translator = $translator;
}

/**
* Executes handlers for periods that have elapsed since last cron
*
* @param array $intervals Interval names to run
* @param bool $force Force cron jobs to run even they are not yet due
* @param null|array $intervals Interval names to run (default: all cron intervals)
* @param bool $force Force cron jobs to run even they are not yet due
*
* @return Job[]
* @throws CronException
*/
public function run(array $intervals = null, $force = false) {
public function run(array $intervals = null, bool $force = false): array {

if (!isset($intervals)) {
$intervals = array_keys($this->default_intervals);
Expand All @@ -69,7 +69,7 @@ public function run(array $intervals = null, $force = false) {

foreach ($intervals as $interval) {
if (!array_key_exists($interval, $allowed_intervals)) {
throw new CronException("$interval is not a recognized cron interval");
throw new CronException("{$interval} is not a recognized cron interval");
}

$cron_interval = $force ? $allowed_intervals['minute'] : $allowed_intervals[$interval];
Expand All @@ -93,39 +93,43 @@ public function run(array $intervals = null, $force = false) {
/**
* Execute commands before cron interval is run
*
* @param string $interval Interval name
* @param \DateTime $time Time of the cron initialization
* @param string $interval Interval name
* @param null|\DateTime $time Time of the cron initialization (default: current service time)
*
* @return void
*/
protected function before($interval, \DateTime $time = null) {
protected function before(string $interval, \DateTime $time = null): void {

if (!isset($time)) {
$time = $this->getCurrentTime();
}

$this->events->triggerBefore('cron', $interval, $time);
try {
$this->events->triggerBefore('cron', $interval, $time);
} catch (\Throwable $t) {
$this->getLogger()->error($t);
}

// give every period at least 'max_execution_time' (PHP ini setting)
set_time_limit((int) ini_get('max_execution_time'));

$now = new \DateTime();

$msg = elgg_echo('admin:cron:started', [$interval, $time->format(DATE_RFC2822)]) . PHP_EOL;
$msg .= elgg_echo('admin:cron:started:actual', [$interval, $now->format(DATE_RFC2822)]) . PHP_EOL;
$msg = $this->translator->translate('admin:cron:started', [$interval, $time->format(DATE_RFC2822)]) . PHP_EOL;
$msg .= $this->translator->translate('admin:cron:started:actual', [$interval, $now->format(DATE_RFC2822)]) . PHP_EOL;

$this->cronLog('output', $interval, $msg);
}

/**
* Execute handlers attached to a specific cron interval
*
* @param string $interval Cron interval to execute
* @param \DateTime $time Time of cron initialization
* @param string $interval Cron interval to execute
* @param null|\DateTime $time Time of cron initialization (default: current service time)
*
* @return string
*/
protected function execute($interval, \DateTime $time = null) {
protected function execute(string $interval, \DateTime $time = null): string {

if (!isset($time)) {
$time = $this->getCurrentTime();
Expand All @@ -135,22 +139,28 @@ protected function execute($interval, \DateTime $time = null) {

$output = [];

$output[] = elgg_echo('admin:cron:started', [$interval, $time->format(DATE_RFC2822)]);
$output[] = elgg_echo('admin:cron:started:actual', [$interval, $now->format(DATE_RFC2822)]);

ob_start();
$output[] = $this->translator->translate('admin:cron:started', [$interval, $time->format(DATE_RFC2822)]);
$output[] = $this->translator->translate('admin:cron:started:actual', [$interval, $now->format(DATE_RFC2822)]);

$old_stdout = $this->events->triggerResults('cron', $interval, [
'time' => $time->getTimestamp(),
'dt' => $time,
], '');

$output[] = ob_get_clean();
$output[] = $old_stdout;
try {
ob_start();

$old_stdout = $this->events->triggerResults('cron', $interval, [
'time' => $time->getTimestamp(),
'dt' => $time,
], '');

$output[] = ob_get_clean();
$output[] = $old_stdout;
} catch (\Throwable $t) {
$output[] = ob_get_clean();

$this->getLogger()->error($t);
}

$now = new \DateTime();

$output[] = elgg_echo('admin:cron:complete', [$interval, $now->format(DATE_RFC2822)]);
$output[] = $this->translator->translate('admin:cron:complete', [$interval, $now->format(DATE_RFC2822)]);

return implode(PHP_EOL, array_filter($output));
}
Expand All @@ -163,16 +173,19 @@ protected function execute($interval, \DateTime $time = null) {
*
* @return void
*/
protected function after($output, $interval) {

protected function after(string $output, string $interval): void {
$time = new \DateTime();

$this->cronLog('output', $interval, $output);
$this->cronLog('completion', $interval, $time->getTimestamp());

$this->getLogger()->info($output);

$this->events->triggerAfter('cron', $interval, $time);

try {
$this->events->triggerAfter('cron', $interval, $time);
} catch (\Throwable $t) {
$this->getLogger()->error($t);
}
}

/**
Expand All @@ -184,15 +197,20 @@ protected function after($output, $interval) {
*
* @return void
*/
protected function cronLog($setting, $interval, $msg = '') {
protected function cronLog(string $setting, string $interval, string $msg = ''): void {
$suffix = $setting ?: 'output';

$fh = new \ElggFile();
$fh->owner_guid = elgg_get_site_entity()->guid;
$fh->setFilename("{$interval}-{$suffix}.log");

$fh->open('write');
$fh->write($msg);
try {
$fh->open('write');
$fh->write($msg);
} catch (Exception $e) {
// don't do anything
}

$fh->close();
}

Expand All @@ -204,7 +222,7 @@ protected function cronLog($setting, $interval, $msg = '') {
*
* @return string
*/
public function getLog($setting, $interval) {
public function getLog(string $setting, string $interval): string {
$suffix = $setting ?: 'output';

$fh = new \ElggFile();
Expand All @@ -231,7 +249,7 @@ public function getLog($setting, $interval) {
* @return array
* @since 3.2
*/
public function getConfiguredIntervals(bool $only_names = false) {
public function getConfiguredIntervals(bool $only_names = false): array {
$result = $this->events->triggerResults('cron:intervals', 'system', [], $this->default_intervals);
if (!is_array($result)) {
$this->getLogger()->warning("The event 'cron:intervals', 'system' should return an array, " . gettype($result) . ' given');
Expand Down

0 comments on commit b6f2565

Please sign in to comment.