CMS MADE SIMPLE FORGE

CMS Made Simple Core

 

[#11664] CmsJobManager.module.php code relies on count(BOOLEAN)

avatar
Created By: Ruud van der Velden (ruudvdvelden)
Date Submitted: Sun Dec 10 18:12:57 -0500 2017

Assigned To:
Version: 2.2.4
CMSMS Version: 2.2.4
Severity: Minor
Resolution: None
State: Open
Summary:
CmsJobManager.module.php code relies on count(BOOLEAN)
Detailed Description:
During debugging I noticed the use of count() of boolean values on multiple
places in the CmsJobManager module. I noticed too that both count(true) and
count(false) evaluate to 1.

Examples below. Possibly it's used in more modules/code too.

    \modules\CmsJobManager\CmsJobManager.module.php
     
    protected function check_for_jobs_or_tasks()
        {
            // this is cheaper.
            $out = \CmsJobManager\JobQueue::get_jobs(1);
            if( count($out) ) return TRUE;
     
            // gotta check for tasks, which is more expensive
            $now = time();
            $lastcheck = (int) $this->GetPreference('tasks_lastcheck');
            if( $lastcheck < $now - 900 ) {
                $this->SetPreference('tasks_lastcheck',$now);
                $tasks = $this->create_jobs_from_eligible_tasks();
                if( count($tasks) ) return TRUE;
            }
            return FALSE;
        }
     
        protected function create_jobs_from_eligible_tasks()
        {
// this creates jobs out of CmsRegularTask objects that we find,and
that need to be executed.
            $now = time();
            $res = false;
     
                    // 1.  Get task objects from files.
                    $dir = CMS_ROOT_PATH.'/lib/tasks';
     
// fairly expensive as we have to iterate a directory and load files
and create objects.
                    $tmp = new DirectoryIterator($dir);
$iterator = new RegexIterator($tmp,'/class\..+task\.php$/');
                    foreach( $iterator as $match ) {
                            $tmp = explode('.',basename($match->current()));
                            if( is_array($tmp) && count($tmp) == 4 ) {
                                    $classname = $tmp[1].'Task';
                                    require_once($dir.'/'.$match->current());
                                    $obj = new $classname;
if( !$obj instanceof CmsRegularTask )
continue;
                    if( !$obj->test($now) ) continue;
                    $job = new \CMSMS\Async\RegularTask($obj);
                    $job->save();
                    $res = true;
                            }
                    }
     
                    // 2.  Get task objects from modules.
                    $opts = ModuleOperations::get_instance();
                    $modules = $opts->get_modules_with_capability('tasks');
                    if (!$modules) return;
                    foreach( $modules as $one ) {
if( !is_object($one) ) $one =
\cms_utils::get_module($one);
                            if( !method_exists($one,'get_tasks') ) continue;
     
                            $tasks = $one->get_tasks();
                            if( !$tasks ) continue;
                if( !is_array($tasks) ) $tasks = array($tasks);
     
                foreach( $tasks as $onetask ) {
                    if( ! is_object($onetask) ) continue;
                    if( ! $onetask instanceof CmsRegularTask ) continue;
                    if( ! $onetask->test() ) continue;
                    $job = new \CMSMS\Async\RegularTask($onetask);
                    $job->module = $one->GetName();
                    $job->save();
                    $res = true;
                }
                    }
     
            return $res;
        }
     
     
    Then read this (example 1): http://php.net/manual/en/function.count.php
     
    $result = count(false);
    // $result == 1
     
    Same here:
     
     protected function check_for_jobs_or_tasks()
        {
            // this is cheaper.
            $out = \CmsJobManager\JobQueue::get_jobs(1);
            if( count($out) ) return TRUE;
     
            // gotta check for tasks, which is more expensive
            $now = time();
            $lastcheck = (int) $this->GetPreference('tasks_lastcheck');
            if( $lastcheck < $now - 900 ) {
                $this->SetPreference('tasks_lastcheck',$now);
                $tasks = $this->create_jobs_from_eligible_tasks();
                if( count($tasks) ) return TRUE;
            }
            return FALSE;
        }
     
     
     $jobs = $this->check_for_jobs_or_tasks();
if( !count($jobs) ) return; // nothing to do.  --> will never return


History