xoops forums

irmtfan

Module Developer
Posted on: 2013/5/7 10:35
irmtfan
irmtfan (Show more)
Module Developer
Posts: 3419
Since: 2003/12/7
#1

Xoops 2.5.6 and Xoops 2.6 bug (bad hard-coded query): $member_handler-> getUserList($criteria)

Title: Xoops 2.5.6 and Xoops 2.6 bug (bad hard-coded query): $member_handler->getUserList($criteria) Text: Today In my running website with +20000 registered users, I recognized one query which has these very bad issues: 1- take many times for executation. 2- run more than once in a page.
[b]0.822133[/b] - SELECT FROM `usersORDER BY uname ASC LIMIT 0200
...

[
b]0.312342[/b] - SELECT FROM `usersORDER BY uname ASC LIMIT 0200
3- "ORDER BY uname ASC LIMIT 0, 200" is a useless query for a website because in a large websites we have many users with !!name, 111, 222 unames. Then i recognized that it comes from the popular "XoopsFormSelectUser" element. If you have this form in a page, It will get a query for the first 200 "uname" fields for every instance of the element in that page.(so 10 elements means 10 bad query ) Of course it is not acceptable for me and i think it is not acceptable for anybody. you can find yourself the "ASC LIMIT 0, 200" which is hard-coded in XOOPS256|Xoops26/class/xoopsform/formselectuser.php:
function XoopsFormSelectUser($caption$name$include_anon false$value null$size 1$multiple false)
    {
        
$limit 200;
        
$select_element = new XoopsFormSelect(''$name$value$size$multiple);
        if (
$include_anon) {
            
$select_element->addOption(0$GLOBALS['xoopsConfig']['anonymous']);
        }
        
$member_handler =& xoops_gethandler('member');
        
$user_count $member_handler->getUserCount();
        
$value is_array($value) ? $value : (empty($value) ? array() : array($value));
        if (
$user_count $limit && count($value) > 0) {
            
$criteria = new CriteriaCompo(new Criteria('uid''(' implode(','$value) . ')''IN'));
        } else {
            
$criteria = new CriteriaCompo();
            
$criteria->setLimit($limit);
        }
        
$criteria->setSort('uname');
        
$criteria->setOrder('ASC');
        
$users $member_handler->getUserList($criteria);
        
$select_element->addOptionArray($users);
        if (
$user_count <= $limit) {
            
$this->XoopsFormElementTray($caption""$name);
            
$this->addElement($select_element);
            return;
        }
At first i decided to change this function But then I found the root of this issue comes from $member_handler->getUserList($criteria) So i went to XOOPS256|Xoops26/kernel/member.php and replace getUserList function:
/**
     * get a list of usernames and their IDs
     *
     * @param CriteriaElement|null $criteria {@link CriteriaElement} object
     * @return array associative array of user-IDs and names
     */
    
function getUserList(CriteriaElement $criteria null)
    {
        
$users $this->_uHandler->getObjects($criteriatrue);
        
$ret = array();
        foreach (
array_keys($users) as $i) {
            
$ret[$i] = $users[$i]->getVar('uname');
        }
        return 
$ret;
    }
with getUserList and setUserList functions:
// START irmtfan to enhance the getUserList function
    /**
     * get a list of usernames and their IDs
     *
     * @param object $criteria {@link CriteriaElement} object
     * @param bool $force true: get from cache file or database
     * @return array associative array of user-IDs and names
     */
    
public function getUserList($criteria null$force true)
    {
        static 
$user_list;
        if (
is_array($user_list) && !$force) {
            return 
$user_list;
        }
        if (!
class_exists('XoopsLoad')) {
            require_once 
XOOPS_ROOT_PATH '/class/xoopsload.php';
        }
        
XoopsLoad::load('XoopsCache');
        if (!
$user_list XoopsCache::read('user_list')) {
            
$user_list $this->setUserList($criteria);
        } else if(
strpos($criteria->renderWhere(),"uid")) {
            
$user_list $this->setUserList($criteria$user_list);
        }
        return 
$user_list;
    }
    
/**
     * set and get a list of usernames and their IDs
     *
     * @param object $criteria {@link CriteriaElement} object
     * @param array $user_list array($uid1=>$uname1, $uid2=>$uname2);
     * @return array associative array of user-IDs and names
     */
    
public function setUserList($criteria null$user_list = array())
    {
        if (!
class_exists('XoopsLoad')) {
            require_once 
XOOPS_ROOT_PATH '/class/xoopsload.php';
        }
        
XoopsLoad::load('XoopsCache');
        
$users $this->_uHandler->getObjects($criteriatrue);
        
$ret = array();
        foreach (
array_keys($users) as $i) {
            
$ret[$i] = $users[$i]->getVar('uname');
        }
        if (!empty(
$user_list)) {
            
$ret array_unique($ret $user_list); // new users will be added to the top!
        
}
        if(!empty(
$ret)) {
            
XoopsCache::write('user_list'$ret);
        }
        return 
$ret;
    }
    
// END irmtfan to enhance the getUserList function
Now I am very happy with this code because: 1- While It has back-ward compatibility, it is compatible with Xoops256 and xoops26 ALPHA versions too. 2- When the list is empty the bad query "ASC LIMIT 0, 200" just run once until webmaster clear xoops caches. (Of course we can change "XoopsFormSelectUser" element function too in the future in core or any developer can extend the class and change that function) 3- IF you add a new member in one "XoopsFormSelectUser" element in a page, it will be added to the list for future usage and it will be added in top of the list 4- It solved all above three issues. ps: please somebody send it to sf.net tracker. I dont know whether bug or feature request is a proper place for it but i prefer the bug tracker.

Mamba

Moderator
Posted on: 2013/5/7 12:30
Mamba
Mamba (Show more)
Moderator
Posts: 10960
Since: 2004/4/23
#2

Re: Xoops 2.5.6 and Xoops 2.6 bug (bad hard-coded query): $member_handler->getUserList($criteria)

Quote:
please somebody send it to sf.net tracker.

https://sourceforge.net/p/xoops/bugs/1268/
Support XOOPS => DONATE
Use 2.5.10 | Docs | Modules | Bugs

irmtfan

Module Developer
Posted on: 2013/5/8 5:18
irmtfan
irmtfan (Show more)
Module Developer
Posts: 3419
Since: 2003/12/7
#3

Re: Xoops 2.5.6 and Xoops 2.6 bug (bad hard-coded query): $member_handler-> getUserList($criteria)

Thank you, Mamba
I enhance the list to show name and uid like this
$uid1- $uname1 ($name1)
$uid2- $uname2 ($name2)

also i add a max = 100 so only the top 100 users will be remained in the list and others will be removed with the first come first go(remove) rule.
// START irmtfan to enhance the getUserList function
    /**
     * get a list of usernames and their IDs
     *
     * @param object $criteria {@link CriteriaElement} object
     * @param bool $force true: get from cache file or database
     * @return array associative array of user-IDs and names
     */
    
public function getUserList($criteria null$force true)
    {
        static 
$user_list;
        if (
is_array($user_list) && !$force) {
            return 
$user_list;
        }
        if (!
class_exists('XoopsLoad')) {
            require_once 
XOOPS_ROOT_PATH '/class/xoopsload.php';
        }
        
XoopsLoad::load('XoopsCache');
        if (!
$user_list XoopsCache::read('user_list')) {
            
$user_list $this->setUserList($criteria);
        } else if(
strpos($criteria->renderWhere(),"uid")) {
            
$user_list $this->setUserList($criteria$user_list);
        }
        return 
$user_list;
    }
    
/**
     * set and get a list of usernames and their IDs
     *
     * @param object $criteria {@link CriteriaElement} object
     * @param array $user_list array($uid1=>"$uid1- $uname1 ($name1)", "$uid2- $uname2 ($name2)");
     * @return array associative array of user-IDs and names
     */
    
public function setUserList($criteria null$user_list = array(), $max 100)
    {
        if (!
class_exists('XoopsLoad')) {
            require_once 
XOOPS_ROOT_PATH '/class/xoopsload.php';
        }
        
XoopsLoad::load('XoopsCache');
        
$users $this->_uHandler->getObjects($criteriatrue);
        
$ret = array();
        foreach (
array_keys($users) as $i) {
            
$ret[$i] = $i "- " $users[$i]->getVar('uname') . " (" $users[$i]->getVar('name') . ")";
        }
        if (!empty(
$user_list)) {
            
$ret array_unique($ret array_slice($user_list,0,$max,true)); // new users will be added to the top! size of user list = new added + $max
        
}
        if(!empty(
$ret)) {
            
XoopsCache::write('user_list'$ret);
        }
        return 
$ret;
    }
    
// END irmtfan to enhance the getUserList function