From d550e02d1e0c6f77e9f678c95c61f6ea0d33a559 Mon Sep 17 00:00:00 2001 From: John van Breda Date: Tue, 15 Oct 2024 11:29:59 +0100 Subject: [PATCH 1/4] Fix the comment quick reply page Fixes https://github.com/Indicia-Team/warehouse/issues/526 --- occurrence_comment_quick_reply_page.php | 48 +++++++++++++------------ 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/occurrence_comment_quick_reply_page.php b/occurrence_comment_quick_reply_page.php index 34ad42bbab..766868c3a2 100644 --- a/occurrence_comment_quick_reply_page.php +++ b/occurrence_comment_quick_reply_page.php @@ -271,7 +271,9 @@ public static function getAuth($password) { * Need the warehouse url for various functions. */ private static function getWarehouseUrl() { - return $_SERVER['HTTP_HOST'] . '/' . trim(str_replace('\\', '/', dirname($_SERVER['PHP_SELF'])), '/') . '/'; + $protocol = ((!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off') || $_SERVER['SERVER_PORT'] == 443) ? "https://" : "http://"; + $host = rtrim($_SERVER['HTTP_HOST'], '/'); + return $protocol . $host . '/' . trim(str_replace('\\', '/', dirname($_SERVER['PHP_SELF'])), '/') . '/'; } /** @@ -306,7 +308,7 @@ private static function httpPost($url, $postargs = NULL) { /** * Convert a timestamp into readable format for use on a comment list. * - * @type timestamp $timestamp + * @param int $timestamp * The date time to convert. * * @return string @@ -314,27 +316,27 @@ private static function httpPost($url, $postargs = NULL) { */ public static function ago($timestamp) { $difference = time() - $timestamp; - $periods = array( - lang::get("{1} second ago"), - lang::get("{1} minute ago"), - lang::get("{1} hour ago"), - lang::get("Yesterday"), - lang::get("{1} week ago"), - lang::get("{1} month ago"), - lang::get("{1} year ago"), - lang::get("{1} decade ago"), - ); - $periodsPlural = array( - lang::get("{1} seconds ago"), - lang::get("{1} minutes ago"), - lang::get("{1} hours ago"), - lang::get("{1} days ago"), - lang::get("{1} weeks ago"), - lang::get("{1} months ago"), - lang::get("{1} years ago"), - lang::get("{1} decades ago"), - ); - $lengths = array("60", "60", "24", "7", "4.35", "12", "10"); + $periods = [ + lang::get('{1} second ago'), + lang::get('{1} minute ago'), + lang::get('{1} hour ago'), + lang::get('Yesterday'), + lang::get('{1} week ago'), + lang::get('{1} month ago'), + lang::get('{1} year ago'), + lang::get('{1} decade ago'), + ]; + $periodsPlural = [ + lang::get('{1} seconds ago'), + lang::get('{1} minutes ago'), + lang::get('{1} hours ago'), + lang::get('{1} days ago'), + lang::get('{1} weeks ago'), + lang::get('{1} months ago'), + lang::get('{1} years ago'), + lang::get('{1} decades ago'), + ]; + $lengths = ['60', '60', '24', '7', '4.35', '12', '10']; for ($j = 0; $difference >= $lengths[$j]; $j++) { $difference /= $lengths[$j]; From 85953c5846d3d1d54d04a10b0d82465a4dfad89b Mon Sep 17 00:00:00 2001 From: John van Breda Date: Tue, 15 Oct 2024 15:03:22 +0100 Subject: [PATCH 2/4] Implements a lock file for scheduled tasks Prevents multiple instances of the same configuration from running. Fixes https://github.com/Indicia-Team/warehouse/issues/527. --- application/controllers/scheduled_tasks.php | 181 +++++++++++++------- 1 file changed, 117 insertions(+), 64 deletions(-) diff --git a/application/controllers/scheduled_tasks.php b/application/controllers/scheduled_tasks.php index a4b4961ae4..25899ba517 100644 --- a/application/controllers/scheduled_tasks.php +++ b/application/controllers/scheduled_tasks.php @@ -45,6 +45,8 @@ class Scheduled_Tasks_Controller extends Controller { */ private Database $db; + private $lock; + /** * Main entry point for scheduled tasks. * @@ -57,78 +59,129 @@ class Scheduled_Tasks_Controller extends Controller { * then everything is run. */ public function index() { - $tm = microtime(TRUE); - $this->db = new Database(); - $system = new System_Model(); - $allNonPluginTasks = ['notifications', 'work_queue']; - // Allow tasks to be specified on command line or URL parameter. - global $argv; - $args = []; - if ($argv) { - parse_str(implode('&', array_slice($argv, 1)), $args); - } - $tasks = $_GET['tasks'] ?? $args['tasks'] ?? NULL; - if ($tasks !== NULL) { - $requestedTasks = explode(',', $tasks); - $scheduledPlugins = array_diff($requestedTasks, $allNonPluginTasks); - $nonPluginTasks = array_intersect($allNonPluginTasks, $requestedTasks); - } - else { - $nonPluginTasks = $allNonPluginTasks; - $scheduledPlugins = ['all_modules']; - } - // Grab the time before we start, so there is no chance of a record coming - // in while we run that is missed. - $currentTime = time(); - if (in_array('notifications', $nonPluginTasks)) { - $this->lastRunDate = $system->getLastScheduledTaskCheck(); - $this->checkTriggers(); - $tmtask = microtime(TRUE) - $tm; - if ($tmtask > 5) { - self::msg("Triggers & notifications scheduled task took $tmtask seconds.", 'alert'); - } - if (class_exists('request_logging')) { - request_logging::log('a', 'scheduled_tasks', NULL, 'triggers_notifications', 0, 0, $tm, $this->db); + $this->lock(); + try { + $tm = microtime(TRUE); + $this->db = new Database(); + $system = new System_Model(); + $allNonPluginTasks = ['notifications', 'work_queue']; + // Allow tasks to be specified on command line or URL parameter. + global $argv; + $args = []; + if ($argv) { + parse_str(implode('&', array_slice($argv, 1)), $args); } - } - if ($scheduledPlugins) { - $this->runScheduledPlugins($system, $scheduledPlugins); - } - if (in_array('notifications', $nonPluginTasks)) { - $email_config = Kohana::config('email'); - if (array_key_exists('do_not_send', $email_config) and $email_config['do_not_send']) { - kohana::log('info', "Email configured for do_not_send: ignoring notifications from scheduled tasks"); + $tasks = $_GET['tasks'] ?? $args['tasks'] ?? NULL; + if ($tasks !== NULL) { + $requestedTasks = explode(',', $tasks); + $scheduledPlugins = array_diff($requestedTasks, $allNonPluginTasks); + $nonPluginTasks = array_intersect($allNonPluginTasks, $requestedTasks); } else { - $swift = email::connect(); - $this->doRecordOwnerNotifications($swift); - $this->doNotificationDigestEmailsForTriggers($swift); + $nonPluginTasks = $allNonPluginTasks; + $scheduledPlugins = ['all_modules']; } - // The value of the last_scheduled_task_check on the Indicia system entry - // is used to mark the last time notifications were handled, so we can - // process new notification info next time notifications are handled. - $this->db->update('system', ['last_scheduled_task_check' => "'" . date('c', $currentTime) . "'"], ['id' => 1]); - } - if (in_array('work_queue', $nonPluginTasks)) { - $timeAtStart = microtime(TRUE); - $queue = new WorkQueue(); - $queue->process($this->db); - $timeTaken = microtime(TRUE) - $timeAtStart; - if ($timeTaken > 10) { - self::msg("Work queue processing took $timeTaken seconds.", 'alert'); + // Grab the time before we start, so there is no chance of a record coming + // in while we run that is missed. + $currentTime = time(); + if (in_array('notifications', $nonPluginTasks)) { + $this->lastRunDate = $system->getLastScheduledTaskCheck(); + $this->checkTriggers(); + $tmtask = microtime(TRUE) - $tm; + if ($tmtask > 5) { + self::msg("Triggers & notifications scheduled task took $tmtask seconds.", 'alert'); + } + if (class_exists('request_logging')) { + request_logging::log('a', 'scheduled_tasks', NULL, 'triggers_notifications', 0, 0, $tm, $this->db); + } + } + if ($scheduledPlugins) { + $this->runScheduledPlugins($system, $scheduledPlugins); + } + if (in_array('notifications', $nonPluginTasks)) { + $email_config = Kohana::config('email'); + if (array_key_exists('do_not_send', $email_config) and $email_config['do_not_send']) { + kohana::log('info', "Email configured for do_not_send: ignoring notifications from scheduled tasks"); + } + else { + $swift = email::connect(); + $this->doRecordOwnerNotifications($swift); + $this->doNotificationDigestEmailsForTriggers($swift); + } + // The value of the last_scheduled_task_check on the Indicia system entry + // is used to mark the last time notifications were handled, so we can + // process new notification info next time notifications are handled. + $this->db->update('system', ['last_scheduled_task_check' => "'" . date('c', $currentTime) . "'"], ['id' => 1]); + } + if (in_array('work_queue', $nonPluginTasks)) { + $timeAtStart = microtime(TRUE); + $queue = new WorkQueue(); + $queue->process($this->db); + $timeTaken = microtime(TRUE) - $timeAtStart; + if ($timeTaken > 10) { + self::msg("Work queue processing took $timeTaken seconds.", 'alert'); + } + if (class_exists('request_logging')) { + request_logging::log('a', 'scheduled_tasks', NULL, 'work_queue', 0, 0, $timeAtStart, $this->db); + } } - if (class_exists('request_logging')) { - request_logging::log('a', 'scheduled_tasks', NULL, 'work_queue', 0, 0, $timeAtStart, $this->db); + self::msg("Ok!"); + $tm = microtime(TRUE) - $tm; + if ($tm > 30) { + self::msg( + "Scheduled tasks for " . implode(', ', array_merge($nonPluginTasks, $scheduledPlugins)) . " took $tm seconds.", + 'alert' + ); } } - self::msg("Ok!"); - $tm = microtime(TRUE) - $tm; - if ($tm > 30) { - self::msg( - "Scheduled tasks for " . implode(', ', array_merge($nonPluginTasks, $scheduledPlugins)) . " took $tm seconds.", - 'alert' - ); + finally { + $this->unlock(); + } + } + + /** + * Build a suitable filename for the lock file. + * + * Different configurations of the scheduled tasks call (specified by query + * string or command line parameters) have their own separate locking, so + * you can run work_queue alongside other tasks for example. + * + * @return string + * A filename which includes a hash of the parameters, so that it is + * unique to this configuration of the scheduled tasks. + */ + private function getLockFilename() { + global $argv; + if (isset($argv)) { + parse_str(implode('&', array_slice($argv, 1)), $params); } + else { + $params = $_GET; + } + return DOCROOT . 'application/cache/scheduled-tasks.lock-' . md5(http_build_query($params)) . '.lock'; + } + + /** + * Grab a file lock if possible. + * + * Will fail if another process is already running the same configuration of + * scheduled tasks. + */ + private function lock() { + $this->lock = fopen($this->getLockFilename(), 'w+'); + if (!flock($this->lock, LOCK_EX | LOCK_NB)) { + kohana::log('alert', 'Scheduled tasks attempt aborted as already running.'); + die("\nScheduled tasks attempt aborted as already running.\n"); + } + fwrite($this->lock, 'Got a lock: ' . var_export($_GET, TRUE)); + } + + /** + * Release and clean up the lock file. + */ + private function unlock() { + fclose($this->lock); + unlink($this->getLockFilename()); } /** From 0371ba52f10adf7a4ff707d623123ff85a7711d8 Mon Sep 17 00:00:00 2001 From: John van Breda Date: Tue, 15 Oct 2024 15:03:35 +0100 Subject: [PATCH 3/4] Update changelog --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eabad30973..f6654ce6d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ Notable changes to the Indicia warehouse are documented here. This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Version 9.5.0 +*2024-10-08* + +### Changes + +* The scheduled tasks process will now skip any attempts to run if there is a previous attempt + which is still running that has the same configuration. This prevents multiple processes running + simultaneously and competing for resources. See https://github.com/Indicia-Team/warehouse/issues/527. + +### Bugfixes + +* The page for quick replies to record comments that is linked to by verification query emails is + now fixed - previously there was an error causing a blank page. See + https://github.com/Indicia-Team/warehouse/issues/526. + ## Version 9.4.2 *2024-10-08* From 412374197ab2845330a7ef8b590ada37824a9249 Mon Sep 17 00:00:00 2001 From: John van Breda Date: Tue, 15 Oct 2024 15:03:45 +0100 Subject: [PATCH 4/4] Version bump --- application/config/version.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/config/version.php b/application/config/version.php index c7468c8166..1ca5e952c0 100644 --- a/application/config/version.php +++ b/application/config/version.php @@ -29,14 +29,14 @@ * * @var string */ -$config['version'] = '9.4.2'; +$config['version'] = '9.5.0'; /** * Version release date. * * @var string */ -$config['release_date'] = '2024-10-08'; +$config['release_date'] = '2024-10-15'; /** * Link to the code repository downloads page.