Fixed various errors concerning cronjobs
authorMatthias Schmidt <gravatronics@live.com>
Fri, 12 Aug 2011 14:30:08 +0000 (16:30 +0200)
committerMatthias Schmidt <gravatronics@live.com>
Fri, 12 Aug 2011 14:30:08 +0000 (16:30 +0200)
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.

wcfsetup/install/files/lib/data/cronjob/CronjobEditor.class.php
wcfsetup/install/files/lib/system/cache/builder/CacheBuilderCronjob.class.php
wcfsetup/install/files/lib/system/cronjob/CronjobScheduler.class.php
wcfsetup/install/files/lib/system/package/plugin/CronjobsPackageInstallationPlugin.class.php

index b976c213c4648d818ec77eb3d4002a25cf3dee87..f68d8310e40a509012d05c6c65ef749c7b404d9f 100644 (file)
@@ -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()
         */
index 335fdf153d187918b338bf0019258227d770bfbf..fd0903eb3e10e5550143d3f83063f5d6bcdc28fe 100644 (file)
@@ -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
index c8c308efe4669754f7b09a3a0389e21a0f112316..c858370b3b04b68491e3349edec3da14bf845a78 100644 (file)
@@ -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<CronjobEditor>
+        * cached times of the next and after next cronjob execution
+        * @var array<integer>
+        */
+       protected $cache = array();
+       
+       /**
+        * list of editors for outstanding cronjobs
+        * @var array<wcf\data\cronjob\CronjobEditor>
         */
-       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');
        }
 }
index 92c7f58942bee8b0f085ee605e2980f405367984..8b29bf32214ae534760250d2c092d39c0373dc50 100644 (file)
@@ -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;
-       }
 }