From ee1ceb92814eb76d2154d693deb7ca0e8c102ab5 Mon Sep 17 00:00:00 2001 From: Duncan Sommerville Date: Tue, 22 Sep 2015 20:21:19 -0400 Subject: [PATCH] Update TaskManager to be a bit easier to extend --- airtime_mvc/application/Bootstrap.php | 2 +- .../application/common/TaskManager.php | 91 ++++++++++--------- 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/airtime_mvc/application/Bootstrap.php b/airtime_mvc/application/Bootstrap.php index 0b3bba17f..b5f481e95 100644 --- a/airtime_mvc/application/Bootstrap.php +++ b/airtime_mvc/application/Bootstrap.php @@ -149,7 +149,7 @@ class Bootstrap extends Zend_Application_Bootstrap_Bootstrap */ if (getenv("AIRTIME_UNIT_TEST") != 1) { $taskManager = TaskManager::getInstance(); - $taskManager->runTask(AirtimeTask::UPGRADE); // Run the upgrade on each request (if it needs to be run) + $taskManager->runTask(TaskFactory::UPGRADE); // Run the upgrade on each request (if it needs to be run) //This will do the upgrade too if it's needed... $taskManager->runTasks(); } diff --git a/airtime_mvc/application/common/TaskManager.php b/airtime_mvc/application/common/TaskManager.php index 0275d08f3..61e559e04 100644 --- a/airtime_mvc/application/common/TaskManager.php +++ b/airtime_mvc/application/common/TaskManager.php @@ -3,8 +3,8 @@ /** * Class TaskManager * - * When adding a new task, the new AirtimeTask class will need to be added to the internal task list, - * as an ENUM value to the AirtimeTask interface, and as a case in the TaskFactory. + * When adding a new task, the new AirtimeTask class will also need to be added + * as a class constant and to the array in TaskFactory */ final class TaskManager { @@ -12,10 +12,7 @@ final class TaskManager { * @var array tasks to be run. Maps task names to a boolean value denoting * whether the task has been checked/run */ - protected $_taskList = [ - AirtimeTask::UPGRADE => false, - AirtimeTask::CELERY => false, - ]; + protected $_taskList; /** * @var TaskManager singleton instance object @@ -25,7 +22,7 @@ final class TaskManager { /** * @var int TASK_INTERVAL_SECONDS how often, in seconds, to run the TaskManager tasks */ - const TASK_INTERVAL_SECONDS = 30; + const TASK_INTERVAL_SECONDS = 300; // 5 minutes - will be run on every pypo request /** * @var $con PDO Propel connection object @@ -36,6 +33,9 @@ final class TaskManager { * Private constructor so class is uninstantiable */ private function __construct() { + foreach (array_keys(TaskFactory::$tasks) as $v) { + $this->_taskList[$v] = false; + } } /** @@ -77,7 +77,7 @@ final class TaskManager { */ public function runTasks() { // If there is data in auth storage, this could be a user request - // so we should lock the TaskManager to avoid blocking + // so we should just return to avoid blocking if ($this->_isUserSessionRequest()) { return; } @@ -85,7 +85,7 @@ final class TaskManager { $this->_con->beginTransaction(); try { $lock = $this->_getLock(); - if ($lock && microtime(true) < $lock['valstr'] + self::TASK_INTERVAL_SECONDS) { + if ($lock && (microtime(true) < $lock['valstr'] + self::TASK_INTERVAL_SECONDS)) { // Propel caches the database connection and uses it persistently, so if we don't // use commit() here, we end up blocking other queries made within this request $this->_con->commit(); @@ -96,8 +96,7 @@ final class TaskManager { } catch (Exception $e) { // We get here if there are simultaneous requests trying to fetch the lock row $this->_con->rollBack(); - // Logging::info($e->getMessage()); // We actually get here a lot, so it's - // better to be silent here to avoid log bloat + Logging::warn($e->getMessage()); return; } foreach ($this->_taskList as $task => $hasTaskRun) { @@ -154,14 +153,6 @@ final class TaskManager { */ interface AirtimeTask { - /** - * PHP doesn't have ENUMs so declare them as interface constants - * Task types - values don't really matter as long as they're unique - */ - - const UPGRADE = "upgrade"; - const CELERY = "celery"; - /** * Check whether the task should be run * @@ -178,31 +169,6 @@ interface AirtimeTask { } -/** - * Class TaskFactory Factory class to abstract task instantiation - */ -class TaskFactory { - - /** - * Get an AirtimeTask based on a task type - * - * @param $task string the task type; uses AirtimeTask constants as an ENUM - * - * @return AirtimeTask|null return a task of the given type or null if no corresponding - * task exists or is implemented - */ - public static function getTask($task) { - switch($task) { - case AirtimeTask::UPGRADE: - return new UpgradeTask(); - case AirtimeTask::CELERY: - return new CeleryTask(); - } - return null; - } - -} - /** * Class UpgradeTask */ @@ -247,4 +213,39 @@ class CeleryTask implements AirtimeTask { CeleryManager::pollBrokerTaskQueue(); } -} \ No newline at end of file +} + +/** + * Class TaskFactory Factory class to abstract task instantiation + */ +class TaskFactory { + + /** + * PHP doesn't have ENUMs so declare them as interface constants + * Task types - values don't really matter as long as they're unique + */ + + const UPGRADE = "upgrade"; + const CELERY = "celery"; + + /** + * @var array map of arbitrary identifiers to class names to be instantiated reflectively + */ + public static $tasks = array( + "upgrade" => "UpgradeTask", + "celery" => "CeleryTask", + ); + + /** + * Get an AirtimeTask based on a task type + * + * @param $task string the task type; uses AirtimeTask constants as an ENUM + * + * @return AirtimeTask|null return a task of the given type or null if no corresponding + * task exists or is implemented + */ + public static function getTask($task) { + return new self::$tasks[$task](); + } + +}