? Success

User tests: Successful: Unsuccessful:

avatar phproberto
phproberto
31 Aug 2016

Description

#__usergroups table should really extend JTableNested but it's there untouched for years. The main issue is that level of groups is not stored in database when a group is saved and that forces that any operation that requires to retrieve a list of groups with their levels uses a query like:

SELECT a.*,COUNT(DISTINCT c2.id) AS level
FROM `jml_usergroups` AS a
LEFT OUTER JOIN `jml_usergroups` AS c2 ON a.lft > c2.lft AND a.rgt < c2.rgt
GROUP BY a.id, a.lft, a.rgt, a.parent_id, a.title
ORDER BY a.lft ASC

That uses MySQL to calculate the groups level but it's a really expensive query (~0.5 seconds for 1500 groups). Most Joomla sites have < 10 user groups so the performance issues are not noticeable but as you will see in the benchmarks below this speeds things a lot.

In the near future we should modify database table to add columns, etc. but I don't have the time now to deal with such a big change and the issues it may cause.

This PR replaces the method to retrieve groups levels & paths. It really improves data reliability because it uses the group tree to populate that data which is correct even when something went wrong and lft & rgt values are wrong.

Summary of Changes

The new method to populate groups levels & paths is quite simple: the list of groups is retrieved with a simple query (~0.011 seconds for 1500 groups) like:

SELECT a.*
FROM `jml_usergroups` AS a
ORDER BY a.lft ASC

then a recursive php function traverses the group tree until it reaches parent_id = 0 which is assigned to level 0. Additionally when it reaches that level it starts to populate the group path. Data is sent back to the recursive function so all the children groups get the correct path & level.

This PR gives us other benefits:

  • There is a class with singleton mode available (JHelperUsergroups::getInstance()) that can be used to deal with user groups. Using a singleton ensures that no matter how many times you need to get the available user groups in the same page load (it happens a lot in com_users backend managements) it will only retrieve the list once.
  • We avoid multiple queries to retrieve groups in a lot of places. JHelperUsergroups becomes the central place to deal with existing groups.

Benchmarks

I did a benchmark for each function I have modified. Each benchmark value shown here is an average of 50 measurements done on a Joomla site with 1500 user groups. All the queries benchmarked use SQL_NO_CACHE to ensure that no cache affects data.

Benchmark system information:

  • OS: Ubuntu Linux v14.04.5 LTS
  • PHP version: 7.0.10-2
  • MySQL version: 5.5.50

Performance improvements for 1500 user groups:

Class::method() Before After Improvement
JHtml::_('user.groups') 0.496s 0.051s 9x faster
UsersModelGroups::getItems() 0.336s 0.012s 28x faster
JFormFieldUserGroupList::getOptions() 0.323s 0.015s 21x faster
JHtml::_('access.usergroups') 0.507s 0.197s 2.5x faster
JHtml::_('access.usergroup') 0.332s 0.021s 15x faster
UsersHelper::getGroups() 0.330s 0.014s 23x faster
JFormFieldGroupParent::getOptions() 0.309s 0.014s 22x faster
JHtmlUser:groups() 0.376s 0.061s 6x faster
JFormFieldRules::getUserGroups() 0.322s 0.011s 29x faster

Testing Instructions

  • Go to backend users view and ensure that batch works.
  • Go to backend users view and check that group searchtools filter works properly.
  • Go to backend users view and create an user. Confirm that groups selection is correctly saved.
  • Go to backend groups view and ensure that groups are shown.
  • Go to backend groups view and ensure that pagination works.
  • Go to backend groups view and try to create a new group
  • Go to backend groups view and edit a group. Ensure that the group is not shown in the parent field.
  • Go to backend groups view and edit a group to change the parent group. Confirm data is saved.
  • Go to backend articles view and edit an article to change its permissions. Confirm data is saved.

Perform any additional test you think may be applicable.

If you want to test performance and you need to create groups I have created a small shit snippet:

$userGroups  = (int) $app->input->get('userGroups', 0);
$parentGroup = (int) $app->input->get('parentGroup', 1);

if ($userGroups && $parentGroup)
{
    $table = JTable::getInstance('Usergroup', 'JTable');
    $prefix = date('H:i:s');

    if (!$table->load($parentGroup))
    {
        echo '<pre>'; print_r($table->getError()); echo '</pre>';
        exit;
    }

    $groupPrefix = $app->input->get('groupPrefix', $table->title . ' child | ' . date('H:i:s') . '-');

    for ($i = 1; $i <= $userGroups; $i++)
    {
        $table->reset();

        $tableData = array(
            'id'        => 0,
            'title'     => $groupPrefix . str_pad($i, 6, '0', STR_PAD_LEFT),
            'parent_id' => (int) $parentGroup
        );

        if (!$table->save($tableData))
        {
            echo '<pre>'; print_r($table->getError()); echo '</pre>';
            exit;
        }
    }
}

You can place that in backend isis template index.php and it will allow you to create groups with urls like:

// Create 50 child groups of group 1
http://localhost/joomla-cms/administratorindex.php?userGroups=50&parentGroup=1 

// Create 100 child groups of group 4
http://localhost/joomla-cms/administratorindex.php?userGroups=100&parentGroup=4 

Remember to remove that shit when you are finished testing this and don't be stupid to use it in other places because is not secure :P

Documentation Changes Required

Example usage of JHelperUsergroup as singleton:

// Get the singleton instance that statically caches data 
$helper = JHelperUsergroups::getInstance();

// Get all the available user groups. An array of stdClass objects with group id as key
$groups = $helper->getAll();

// Get a specific user group by its id (21)
$group = $helper->get(21);

// Check if a group exists. This requires that you have previously loaded groups through getAll()/loadAll() 
if ($helper->has(23))
{
    echo 'Group 23 exists with the title: ' . $helper->get(23)->title;
}

// Get the count total available user groups
$count = $helper->total();

Example usage of JHelperUsergroup as standard instance:

// Useful to populate level & path for a list of groups already loaded
$helper = new JHelperUsergroup($myArrayOfGroup);

// This will retrieve the list of groups with the data already populated
$groups = $helper->getAll();

// Check if a group exists. This requires that you have previously passed the group
if ($helper->has(23))
{
    echo 'Group 23 exists with the title: ' . $helper->get(23)->title;
}
avatar phproberto phproberto - open - 31 Aug 2016
avatar phproberto phproberto - change - 31 Aug 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 31 Aug 2016
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 31 Aug 2016
Category Administration Components Libraries
avatar phproberto phproberto - change - 31 Aug 2016
The description was changed
avatar phproberto phproberto - edited - 31 Aug 2016
avatar phproberto phproberto - edited - 31 Aug 2016
avatar zero-24
zero-24 - comment - 31 Aug 2016

Result.

  • Go to backend users view and ensure that batch works.
  • Go to backend users view and check that group searchtools filter works properly.
  • Go to backend users view and create an user. Confirm that groups selection is correctly saved.
  • Go to backend groups view and ensure that groups are shown.
  • Go to backend groups view and ensure that pagination works. (tip set the dropdown to 5)
  • Go to backend groups view and ensure that groups are shown. (that looks dublicated? but success ? )
  • Go to backend groups view and try to create a new group
  • Go to backend groups view and edit a group. Ensure that the group is not shown in the parent field.
  • Go to backend groups view and edit a group to change the parent group. Confirm data is saved.
  • Go to backend articles view and edit an article to change its permissions. Confirm data is saved.

note: I have not exiplite tested the performance.

avatar zero-24 zero-24 - test_item - 31 Aug 2016 - Tested successfully
avatar zero-24
zero-24 - comment - 31 Aug 2016

I have tested this item successfully on a4d42a4

works for me thanks @phproberto


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853.

avatar phproberto
phproberto - comment - 31 Aug 2016

Merged. Thanks @zero-24 :)

avatar dgt41 dgt41 - test_item - 31 Aug 2016 - Tested successfully
avatar dgt41
dgt41 - comment - 31 Aug 2016

I have tested this item successfully on 3f1c170


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853.

avatar phproberto phproberto - edited - 31 Aug 2016
avatar phproberto phproberto - change - 31 Aug 2016
The description was changed
avatar phproberto phproberto - edited - 31 Aug 2016
avatar phproberto phproberto - change - 31 Aug 2016
The description was changed
avatar phproberto phproberto - edited - 31 Aug 2016
avatar C-Lodder C-Lodder - test_item - 31 Aug 2016 - Tested successfully
avatar C-Lodder
C-Lodder - comment - 31 Aug 2016

I have tested this item successfully on

Nice one @phproberto !


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853.

avatar zero-24
zero-24 - comment - 31 Aug 2016

I have broken travis :( If that is fixed we can set it RTC. phproberto#5 Thanks ?

avatar phproberto
phproberto - comment - 31 Aug 2016

? Merged @zero-24

avatar dgt41 dgt41 - test_item - 31 Aug 2016 - Tested successfully
avatar dgt41
dgt41 - comment - 31 Aug 2016

I have tested this item successfully on 2d15209


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853.

avatar dgt41 dgt41 - alter_testresult - 31 Aug 2016 - C-Lodder: Tested successfully
avatar dgt41 dgt41 - change - 31 Aug 2016
Status Pending Ready to Commit
avatar dgt41
dgt41 - comment - 31 Aug 2016

Since the last change was only a CS I will move this to RTC

Thank you @phproberto

avatar joomla-cms-bot joomla-cms-bot - change - 31 Aug 2016
Labels Added: ?
avatar zero-24 zero-24 - test_item - 31 Aug 2016 - Tested successfully
avatar zero-24
zero-24 - comment - 31 Aug 2016

I have tested this item successfully on 2d15209

Thanks. Travis is happy now. RTC


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853.

avatar rdeutz rdeutz - change - 31 Aug 2016
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-08-31 19:57:28
Closed_By rdeutz
avatar rdeutz rdeutz - close - 31 Aug 2016
avatar rdeutz rdeutz - merge - 31 Aug 2016
avatar joomla-cms-bot joomla-cms-bot - close - 31 Aug 2016
avatar joomla-cms-bot joomla-cms-bot - change - 31 Aug 2016
Labels Removed: ?

Add a Comment

Login with GitHub to post a comment