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 `users` ORDER BY uname ASC LIMIT 0, 200
...
[b]0.312342[/b] - SELECT * FROM `users` ORDER BY uname ASC LIMIT 0, 200
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($criteria, true);
$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($criteria, true);
$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.
