move scheduled tasks to a separate class, add some try-catches, improve/shorten logging and descriptions

3 files changed, 136 insertions, 106 deletions

diff --git a/classes/PluginHost.php b/classes/PluginHost.php
index 5cff4afcb..f61a5a9a4 100644
--- a/classes/PluginHost.php
+++ b/classes/PluginHost.php
@@ -32,9 +32,6 @@ class PluginHost {
 	/** @var array<string, array<int, array{'action': string, 'description': string, 'sender': Plugin}>> */
 	private array $plugin_actions = [];
 
-	/** @var array<string, mixed> */
-	private array $scheduled_tasks = [];
-
 	private ?int $owner_uid = null;
 
 	private bool $data_loaded = false;
@@ -910,98 +907,4 @@ class PluginHost {
 		return basename(dirname(dirname($ref->getFileName()))) == "plugins.local";
 	}
 
-	/**
-	 * Adds a backend scheduled task which will be executed by updater (if due) when idle during
-	 * RSSUtils::housekeeping_common().
-	 *
-	 * The granularity is not strictly guaranteed, housekeeping is invoked several times per hour
-	 * depending on how fast feed batch was processed, but no more than once per minute.
-	 *
-	 * Tasks are not run in user context and are available to system plugins only. Task names may not
-	 * overlap.
-	 *
-	 * Tasks should return an integer value (return code) which is stored in the database, a value of
-	 * 0 is considered successful.
-	 *
-	 * @param string $task_name unique name for this task, plugins should prefix this with plugin name
-	 * @param string $cron_expression schedule for this task in cron format
-	 * @param Closure $callback task code that gets executed
-	*/
-	function add_scheduled_task(string $task_name, string $cron_expression, Closure $callback) : bool {
-		$task_name = strtolower($task_name);
-
-		if (isset($this->scheduled_tasks[$task_name])) {
-			user_error("Attempted to override already registered scheduled task $task_name", E_USER_WARNING);
-			return false;
-		} else {
-			$cron = new Cron\CronExpression($cron_expression);
-
-			$this->scheduled_tasks[$task_name] = [
-				"cron" => $cron,
-				"callback" => $callback,
-			];
-			return true;
-		}
-	}
-
-	/**
-	 * Execute scheduled tasks which are due to run and record last run timestamps.
-	 * @return void
-	 */
-	function run_due_tasks() {
-		Debug::log('Processing all scheduled tasks...');
-
-		$tasks_run = 0;
-
-		foreach ($this->scheduled_tasks as $task_name => $task) {
-			$last_run = '1970-01-01 00:00';
-
-			$task_record = ORM::for_table('ttrss_scheduled_tasks')
-				->where('task_name', $task_name)
-				->find_one();
-
-			if ($task_record)
-				$last_run = $task_record->last_run;
-
-			Debug::log("Checking scheduled task: $task_name, last run: $last_run");
-
-			// because we don't schedule tasks every minute, we assume that task is due if its
-			// next estimated run based on previous timestamp is in the past
-			if ($task['cron']->getNextRunDate($last_run)->getTimestamp() - time() < 0) {
-				Debug::log("Task $task_name is due, executing...");
-
-				$task_started = time();
-				$rc = (int) $task['callback']();
-				$task_duration = time() - $task_started;
-
-				++$tasks_run;
-
-				Debug::log("Task $task_name has finished in $task_duration seconds with RC=$rc, recording timestamp...");
-
-				if ($task_record) {
-					$task_record->last_run = Db::NOW();
-					$task_record->last_duration = $task_duration;
-					$task_record->last_rc = $rc;
-
-					$task_record->save();
-				} else {
-					$task_record = ORM::for_table('ttrss_scheduled_tasks')->create();
-
-					$task_record->set([
-						'task_name' => $task_name,
-						'last_duration' => $task_duration,
-						'last_rc' => $rc,
-						'last_run' => Db::NOW(),
-					]);
-
-					$task_record->save();
-				}
-			}
-		}
-
-		Debug::log("Finished with $tasks_run tasks executed.");
-	}
-
-	// TODO implement some sort of automatic cleanup for orphan task execution records
-
 }
diff --git a/classes/RSSUtils.php b/classes/RSSUtils.php
index 575a1eda1..bf05e0b98 100644
--- a/classes/RSSUtils.php
+++ b/classes/RSSUtils.php
@@ -1715,7 +1715,9 @@ class RSSUtils {
 	static function init_housekeeping_tasks() : void {
 		Debug::log('Registering scheduled tasks for housekeeping...');
 
-		PluginHost::getInstance()->add_scheduled_task('purge_orphans', '@daily',
+		$scheduler = Scheduler::getInstance();
+
+		$scheduler->add_scheduled_task('purge_orphans', '@daily',
 			function() {
 				Article::_purge_orphans();
 
@@ -1723,7 +1725,7 @@ class RSSUtils {
 			}
 		);
 
-		PluginHost::getInstance()->add_scheduled_task('disk_cache_expire_all', '@daily',
+		$scheduler->add_scheduled_task('disk_cache_expire_all', '@daily',
 			function() {
 				$cache = DiskCache::instance("");
 				$cache->expire_all();
@@ -1732,7 +1734,7 @@ class RSSUtils {
 			}
 		);
 
-		PluginHost::getInstance()->add_scheduled_task('expire_error_log', '@hourly',
+		$scheduler->add_scheduled_task('expire_error_log', '@hourly',
 			function() {
 				self::expire_error_log();
 
@@ -1740,7 +1742,7 @@ class RSSUtils {
 			}
 		);
 
-		PluginHost::getInstance()->add_scheduled_task('expire_lock_files', '@hourly',
+		$scheduler->add_scheduled_task('expire_lock_files', '@hourly',
 			function() {
 				self::expire_lock_files();
 
@@ -1748,7 +1750,7 @@ class RSSUtils {
 			}
 		);
 
-		PluginHost::getInstance()->add_scheduled_task('disable_failed_feeds', '@daily',
+		$scheduler->add_scheduled_task('disable_failed_feeds', '@daily',
 			function() {
 				self::disable_failed_feeds();
 
@@ -1756,7 +1758,7 @@ class RSSUtils {
 			}
 		);
 
-		PluginHost::getInstance()->add_scheduled_task('migrate_feed_icons', '@daily',
+		$scheduler->add_scheduled_task('migrate_feed_icons', '@daily',
 			function() {
 				self::migrate_feed_icons();
 
@@ -1764,7 +1766,7 @@ class RSSUtils {
 			}
 		);
 
-		PluginHost::getInstance()->add_scheduled_task('cleanup_feed_icons', '@daily',
+		$scheduler->add_scheduled_task('cleanup_feed_icons', '@daily',
 			function() {
 				self::cleanup_feed_icons();
 
@@ -1772,7 +1774,7 @@ class RSSUtils {
 			}
 		);
 
-		PluginHost::getInstance()->add_scheduled_task('send_headlines_digests', '@hourly',
+		$scheduler->add_scheduled_task('send_headlines_digests', '@hourly',
 			function() {
 				Digest::send_headlines_digests();
 
@@ -1782,7 +1784,8 @@ class RSSUtils {
 	}
 
 	static function housekeeping_common(): void {
-		PluginHost::getInstance()->run_due_tasks();
+		Scheduler::getInstance()->run_due_tasks();
+
 		PluginHost::getInstance()->run_hooks(PluginHost::HOOK_HOUSE_KEEPING);
 	}
 
diff --git a/classes/Scheduler.php b/classes/Scheduler.php
new file mode 100644
index 000000000..f0ca38303
--- /dev/null
+++ b/classes/Scheduler.php
@@ -0,0 +1,124 @@
+<?php
+class Scheduler {
+	private static ?Scheduler $instance = null;
+
+	const TASK_RC_EXCEPTION = -100;
+
+	/** @var array<string, mixed> */
+	private array $scheduled_tasks = [];
+
+	public static function getInstance(): Scheduler {
+		if (self::$instance == null)
+			self::$instance = new self();
+
+		return self::$instance;
+	}
+
+	/**
+	 * Adds a backend scheduled task which will be executed by updater (if due) during housekeeping.
+	 *
+	 * The granularity is not strictly guaranteed, housekeeping is invoked several times per hour
+	 * depending on how fast feed batch was processed, but no more than once per minute.
+	 *
+	 * Tasks do not run in user context. Task names may not overlap. Plugins should register tasks
+	 * via PluginHost methods (to be implemented later).
+	 *
+	 * Tasks should return an integer value (return code) which is stored in the database, a value of
+	 * 0 is considered successful.
+	 *
+	 * @param string $task_name unique name for this task, plugins should prefix this with plugin name
+	 * @param string $cron_expression schedule for this task in cron format
+	 * @param Closure $callback task code that gets executed
+	*/
+	function add_scheduled_task(string $task_name, string $cron_expression, Closure $callback) : bool {
+		$task_name = strtolower($task_name);
+
+		if (isset($this->scheduled_tasks[$task_name])) {
+			user_error("Attempted to override already registered scheduled task $task_name", E_USER_WARNING);
+			return false;
+		} else {
+			try {
+				$cron = new Cron\CronExpression($cron_expression);
+			} catch (InvalidArgumentException $e) {
+				user_error("Attempt to register scheduled task $task_name failed: " . $e->getMessage(), E_USER_WARNING);
+				return false;
+			}
+
+			$this->scheduled_tasks[$task_name] = [
+				"cron" => $cron,
+				"callback" => $callback,
+			];
+			return true;
+		}
+	}
+
+	/**
+	 * Execute scheduled tasks which are due to run and record last run timestamps.
+	 */
+	function run_due_tasks() : void {
+		Debug::log('Processing all scheduled tasks...');
+
+		$tasks_succeeded = 0;
+		$tasks_failed = 0;
+
+		foreach ($this->scheduled_tasks as $task_name => $task) {
+			$task_record = ORM::for_table('ttrss_scheduled_tasks')
+				->where('task_name', $task_name)
+				->find_one();
+
+			if ($task_record)
+				$last_run = $task_record->last_run;
+			else
+				$last_run = '1970-01-01 00:00';
+
+			// because we don't schedule tasks every minute, we assume that task is due if its
+			// next estimated run based on previous timestamp is in the past
+			if ($task['cron']->getNextRunDate($last_run)->getTimestamp() - time() < 0) {
+				Debug::log("Task $task_name is due, executing...");
+
+				$task_started = time();
+
+				try {
+					$rc = (int) $task['callback']();
+				} catch (Exception $e) {
+					user_error("Scheduled task $task_name failed with exception: " . $e->getMessage(), E_USER_WARNING);
+
+					$rc = self::TASK_RC_EXCEPTION;
+				}
+
+				$task_duration = time() - $task_started;
+
+				if ($rc === 0) {
+					++$tasks_succeeded;
+					Debug::log("Task $task_name has finished in $task_duration seconds.");
+				} else {
+					$tasks_failed++;
+					Debug::log("Task $task_name has failed with RC: $rc after $task_duration seconds.");
+				}
+
+				if ($task_record) {
+					$task_record->last_run = Db::NOW();
+					$task_record->last_duration = $task_duration;
+					$task_record->last_rc = $rc;
+
+					$task_record->save();
+				} else {
+					$task_record = ORM::for_table('ttrss_scheduled_tasks')->create();
+
+					$task_record->set([
+						'task_name' => $task_name,
+						'last_duration' => $task_duration,
+						'last_rc' => $rc,
+						'last_run' => Db::NOW(),
+					]);
+
+					$task_record->save();
+				}
+			}
+		}
+
+		Debug::log("Finished with $tasks_succeeded tasks succeeded and $tasks_failed tasks failed.");
+	}
+
+	// TODO implement some sort of automatic cleanup for orphan task execution records
+}
\ No newline at end of file