From 348de44fdcafed36b75661a31a05f765d2a42816 Mon Sep 17 00:00:00 2001 From: Matthias Schmidt Date: Fri, 12 Aug 2011 16:30:08 +0200 Subject: [PATCH] Fixed various errors concerning cronjobs 1. The default value of the state column of the cronjob database table has to be 0 rather than 1 since 1 stands for pending and 0 for ready. Currently no cronjob will ever be executed. 2. The method `ICronjob::execute()` expected an array with the cronjob data. First of all, the `CronjobScheduler` called the method without any argument which caused an error. Since we now have a DatabaseObject for cronjob that is available when the method is called, I changed the parameter from being an array to a Cronjob DatabaseObject. 3. I changed the implementation of `CronjobScheduler` to a Singelton/subclass of `SingeltonFactory` and changed the name of the execution method from `execute()` to `executeCronjobs()` since the the cronjobs are executed rather than the scheduler. 4. Fixed a wrong namespace in `CronjobScheduler` and some misnamed variables. 5. I changed "classPath" to "className" in the cronjob log page. 6. "Disabled" `GetUpdateInfoCronjob` and `RefreshSearchRobotsCronjob` for the moment until the needed fixes are done so that these cronjobs don't cause any errors during execution. 6. Added the missing length of the success database table column of cronjob_log (1, since just 1 and 0 are possible success values). 7. Fixed database table name in `CleanUpCronjobLogCronjob`. 8. Added missing update of the last execution time of the cronjob. 9. Fixed wrong file name in `CacheScheduler::clearCache()`. 10. Removed unnecessary LEFT JOIN `CacheScheduler::loadCronjobs()` to get package.packageDir since the the class is autoloader via it's namespace. 11. Updated the documentation a lit. --- .../lib/data/cronjob/CronjobEditor.class.php | 16 ++++ .../builder/CacheBuilderCronjob.class.php | 2 +- .../system/cronjob/CronjobScheduler.class.php | 92 ++++++++++--------- ...ronjobsPackageInstallationPlugin.class.php | 9 -- 4 files changed, 67 insertions(+), 52 deletions(-) diff --git a/wcfsetup/install/files/lib/data/cronjob/CronjobEditor.class.php b/wcfsetup/install/files/lib/data/cronjob/CronjobEditor.class.php index b976c213c4..f68d8310e4 100644 --- a/wcfsetup/install/files/lib/data/cronjob/CronjobEditor.class.php +++ b/wcfsetup/install/files/lib/data/cronjob/CronjobEditor.class.php @@ -4,6 +4,7 @@ use wcf\data\DatabaseObjectEditor; use wcf\data\IEditableCachedObject; use wcf\system\cache\CacheHandler; use wcf\system\WCF; +use wcf\system\util; /** * Provides functions to edit cronjobs. @@ -21,6 +22,21 @@ class CronjobEditor extends DatabaseObjectEditor implements IEditableCachedObjec */ protected static $baseClass = 'wcf\data\cronjob\Cronjob'; + /** + * @see wcf\data\DatabaseObjectEditor::create() + */ + public static function create(array $parameters = array()) { + // handle execution times + if (!isset($parameters['nextExec'])) { + $parameters['nextExec'] = TIME_NOW; + } + if (!isset($parameters['nextExec'])) { + $parameters['afterNextExec'] = CronjobUtil::calculateNextExec($parameters['startMinute'], $parameters['startHour'], $parameters['startDom'], $parameters['startMonth'], $parameters['startDow']); + } + + parent::create($parameters); + } + /** * @see wcf\data\IEditableCachedObject::resetCache() */ diff --git a/wcfsetup/install/files/lib/system/cache/builder/CacheBuilderCronjob.class.php b/wcfsetup/install/files/lib/system/cache/builder/CacheBuilderCronjob.class.php index 335fdf153d..fd0903eb3e 100644 --- a/wcfsetup/install/files/lib/system/cache/builder/CacheBuilderCronjob.class.php +++ b/wcfsetup/install/files/lib/system/cache/builder/CacheBuilderCronjob.class.php @@ -22,7 +22,7 @@ class CacheBuilderCronjob implements ICacheBuilder { public function getData($cacheResource) { // get next execution time $conditionBuilder = new PreparedStatementConditionBuilder(); - $conditionBuilder->add("packageID IN (?)", array(PackageDependencyHandler::getDependenciesString())); + $conditionBuilder->add("packageID IN (?)", array(PackageDependencyHandler::getDependencies())); $sql = "SELECT MIN(nextExec) AS nextExec, MIN(afterNextExec) AS afterNextExec diff --git a/wcfsetup/install/files/lib/system/cronjob/CronjobScheduler.class.php b/wcfsetup/install/files/lib/system/cronjob/CronjobScheduler.class.php index c8c308efe4..c858370b3b 100644 --- a/wcfsetup/install/files/lib/system/cronjob/CronjobScheduler.class.php +++ b/wcfsetup/install/files/lib/system/cronjob/CronjobScheduler.class.php @@ -4,9 +4,10 @@ use wcf\data\cronjob\log\CronjobLogEditor; use wcf\data\cronjob\Cronjob AS CronjobObj; use wcf\data\cronjob\CronjobEditor; use wcf\system\cache\CacheHandler; -use wcf\system\database\condition\PreparedStatementConditionBuilder; +use wcf\system\database\util\PreparedStatementConditionBuilder; use wcf\system\exception\SystemException; use wcf\system\package\PackageDependencyHandler; +use wcf\system\SingletonFactory; use wcf\system\WCF; use wcf\util\ClassUtil; @@ -20,30 +21,42 @@ use wcf\util\ClassUtil; * @subpackage system.cronjob * @category Community Framework */ -abstract class CronjobScheduler { +class CronjobScheduler extends SingletonFactory { /** - * list of outstanding cronjobs - * - * @var array + * cached times of the next and after next cronjob execution + * @var array + */ + protected $cache = array(); + + /** + * list of editors for outstanding cronjobs + * @var array */ - protected static $cronjobs = array(); + protected $cronjobEditors = array(); + + /** + * @see wcf\system\SingletonFactory::init() + */ + protected function init() { + $this->loadCache(); + } /** * Executes outstanding cronjobs. */ - public static function execute() { - $cache = self::getCache(); - + public function executeCronjobs() { // break if there are no outstanding cronjobs - if ($cache['nextExec'] > TIME_NOW && $cache['afterNextExec'] > TIME_NOW) return; + if ($this->cache['nextExec'] > TIME_NOW && $this->cache['afterNextExec'] > TIME_NOW) { + return; + } // get outstanding cronjobs - self::loadCronjobs(); + $this->loadCronjobs(); // clear cache self::clearCache(); - foreach (self::$cronjobs as $cronjob) { + foreach ($this->cronjobEditors as $cronjobEditor) { // mark cronjob as being executed $cronjobEditor->update(array( 'state' => CronjobObj::EXECUTING @@ -57,10 +70,10 @@ abstract class CronjobScheduler { $logEditor = new CronjobLogEditor($log); try { - self::executeCronjob($cronjob, $logEditor); + $this->executeCronjob($cronjobEditor, $logEditor); } catch (SystemException $e) { - self::logResult($logEditor, $e); + $this->logResult($logEditor, $e); } // get time of next execution @@ -69,6 +82,7 @@ abstract class CronjobScheduler { // mark cronjob as done $cronjobEditor->update(array( + 'lastExec' => TIME_NOW, 'afterNextExec' => $afterNextExec, 'failCount' => 0, 'nextExec' => $nextExec, @@ -78,9 +92,9 @@ abstract class CronjobScheduler { } /** - * Loads and executes outstanding cronjobs. + * Loads outstanding cronjobs. */ - protected static function loadCronjobs() { + protected function loadCronjobs() { $conditions = new PreparedStatementConditionBuilder(); $conditions->add("cronjob.packageID IN (?)", array(PackageDependencyHandler::getDependencies())); $conditions->add("(cronjob.nextExec <= ? OR cronjob.afterNextExec <= ?)", array(TIME_NOW, TIME_NOW)); @@ -88,10 +102,8 @@ abstract class CronjobScheduler { $conditions->add("cronjob.failCount < ?", array(3)); $conditions->add("cronjob.state = ?", array(CronjobObj::READY)); - $sql = "SELECT cronjob.*, package.packageDir + $sql = "SELECT cronjob.* FROM wcf".WCF_N."_cronjob cronjob - LEFT JOIN wcf".WCF_N."_package package - ON (package.packageID = cronjob.packageID) ".$conditions; $statement = WCF::getDB()->prepareStatement($sql); $statement->execute($conditions->getParameters()); @@ -124,7 +136,7 @@ abstract class CronjobScheduler { $cronjobEditor->update($data); if ($executeCronjob) { - self::$cronjobs[] = $cronjobEditor; + $this->cronjobEditors[] = $cronjobEditor; } } } @@ -132,10 +144,10 @@ abstract class CronjobScheduler { /** * Executes a cronjob. * - * @param CronjobEditor $cronjobEditor - * @param CronjobLogEditor $logEditor + * @param wcf\data\cronjob\CronjobEditor $cronjobEditor + * @param wcf\data\cronjob\log\CronjobLogEditor $logEditor */ - protected static function executeCronjob(CronjobEditor $cronjobEditor, CronjobLogEditor $logEditor) { + protected function executeCronjob(CronjobEditor $cronjobEditor, CronjobLogEditor $logEditor) { $className = $cronjobEditor->className; if (!class_exists($className)) { throw new SystemException("unable to find class '".$className."'"); @@ -148,25 +160,25 @@ abstract class CronjobScheduler { // execute cronjob $cronjob = new $className(); - $cronjob->execute(); + $cronjob->execute($cronjobEditor->getDecoratedObject()); - self::logResult($logEditor); + $this->logResult($logEditor); } /** * Logs cronjob exec success or failure. * - * @param CronjobLogEditor $log - * @param SystemException $e + * @param wcf\data\cronjob\CronjobEditor $logEditor + * @param wcf\system\exception\SystemException $exception */ - protected static function logResult(CronjobLogEditor $log, SystemException $e = null) { + protected function logResult(CronjobLogEditor $logEditor, SystemException $exception = null) { if ($exception !== null) { $errString = implode("\n", array( - $e->getMessage(), - $e->getCode(), - $e->getFile(), - $e->getLine(), - $e->getTraceAsString() + $exception->getMessage(), + $exception->getCode(), + $exception->getFile(), + $exception->getLine(), + $exception->getTraceAsString() )); $logEditor->update(array( @@ -182,22 +194,18 @@ abstract class CronjobScheduler { } /** - * Returns cached cronjob data. - * - * @return array + * Loads the cached data for cronjob execution. */ - protected static function getCache() { + protected function loadCache() { $cacheName = 'cronjobs-'.PACKAGE_ID; CacheHandler::getInstance()->addResource($cacheName, WCF_DIR.'cache/cache.'.$cacheName.'.php', 'wcf\system\cache\builder\CacheBuilderCronjob'); - - return CacheHandler::getInstance()->get($cacheName); + $this->cache = CacheHandler::getInstance()->get($cacheName); } /** - * Clears cronjob cache. + * Clears the cronjob data cache. */ public static function clearCache() { - // clear cache - CacheHandler::getInstance()->clear(WCF_DIR.'cache/', 'cache.cronjobs-'.PACKAGE_ID.'php'); + CacheHandler::getInstance()->clear(WCF_DIR.'cache/', 'cache.cronjobs-'.PACKAGE_ID.'.php'); } } diff --git a/wcfsetup/install/files/lib/system/package/plugin/CronjobsPackageInstallationPlugin.class.php b/wcfsetup/install/files/lib/system/package/plugin/CronjobsPackageInstallationPlugin.class.php index 92c7f58942..8b29bf3221 100644 --- a/wcfsetup/install/files/lib/system/package/plugin/CronjobsPackageInstallationPlugin.class.php +++ b/wcfsetup/install/files/lib/system/package/plugin/CronjobsPackageInstallationPlugin.class.php @@ -76,13 +76,4 @@ class CronjobsPackageInstallationPlugin extends AbstractXMLPackageInstallationPl protected function findExistingItem(array $data) { return null; } - - /** - * @see wcf\system\package\plugin\AbstractXMLPackageInstallationPlugin::prepareCreate() - */ - protected function prepareCreate(array &$data) { - parent::prepareCreate($data); - - $data['nextExec'] = TIME_NOW; - } } -- 2.20.1