From ab2c7e06a4ea3c751058b6b72dc9b8832836669a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 9 Jul 2014 12:19:50 +0200 Subject: [PATCH] remove dead code do not filter groups. but update the user count according to the filter improve phpdoc improve metadata runtime cache add metadata tests --- lib/private/group/metadata.php | 54 +++++++----------- settings/ajax/grouplist.php | 14 ++--- settings/js/users/filter.js | 6 +- settings/js/users/groups.js | 5 +- tests/lib/group/metadata.php | 101 +++++++++++++++++++++++++++++++++ 5 files changed, 137 insertions(+), 43 deletions(-) create mode 100644 tests/lib/group/metadata.php diff --git a/lib/private/group/metadata.php b/lib/private/group/metadata.php index 1883ba727e..303543d48c 100644 --- a/lib/private/group/metadata.php +++ b/lib/private/group/metadata.php @@ -24,9 +24,9 @@ class MetaData { protected $isAdmin; /** - * @var string[] $groups + * @var array $metaData */ - protected $groups = array(); + protected $metaData = array(); /** * @var \OC\Group\Manager $groupManager @@ -38,11 +38,6 @@ class MetaData { */ protected $sorting = false; - /** - * @var string $lastSearch - */ - protected $lastSearch; - /** * @param string the uid of the current user * @param bool whether the current users is an admin @@ -63,14 +58,15 @@ class MetaData { * the array is structured as follows: * [0] array containing meta data about admin groups * [1] array containing meta data about unprivileged groups - * @param string only effective when instance was created with isAdmin being - * true + * @param string $groupSearch only effective when instance was created with + * isAdmin being true + * @param string $userSearch the pattern users are search for * @return array */ - public function get($search = '') { - if($this->lastSearch !== $search) { - $this->lastSearch = $search; - $this->groups = array(); + public function get($groupSearch = '', $userSearch = '') { + $key = $groupSearch . '::' . $userSearch; + if(isset($this->metaData[$key])) { + return $this->metaData[$key]; } $adminGroups = array(); @@ -80,8 +76,8 @@ class MetaData { $sortAdminGroupsIndex = 0; $sortAdminGroupsKeys = array(); - foreach($this->getGroups($search) as $group) { - $groupMetaData = $this->generateGroupMetaData($group); + foreach($this->getGroups($groupSearch) as $group) { + $groupMetaData = $this->generateGroupMetaData($group, $userSearch); if (strtolower($group->getGID()) !== 'admin') { $this->addEntry( $groups, @@ -104,7 +100,8 @@ class MetaData { $this->sort($groups, $sortGroupsKeys); $this->sort($adminGroups, $sortAdminGroupsKeys); - return array($adminGroups, $groups); + $this->metaData[$key] = array($adminGroups, $groups); + return $this->metaData[$key]; } /** @@ -138,14 +135,15 @@ class MetaData { /** * @brief creates an array containing the group meta data - * @param \OC\Group\Group + * @param \OC\Group\Group $group + * @param string $userSearch * @return array with the keys 'id', 'name' and 'usercount' */ - private function generateGroupMetaData(\OC\Group\Group $group) { + private function generateGroupMetaData(\OC\Group\Group $group, $userSearch) { return array( - 'id' => str_replace(' ','', $group->getGID()), + 'id' => $group->getGID(), 'name' => $group->getGID(), - 'usercount' => $group->count() + 'usercount' => $group->count($userSearch) ); } @@ -167,22 +165,10 @@ class MetaData { * @return \OC\Group\Group[] */ private function getGroups($search = '') { - if(count($this->groups) === 0) { - $this->fetchGroups($search); - } - return $this->groups; - } - - /** - * @brief fetches the group using the group manager or the subAdmin API - * @param string a search string - * @return null - */ - private function fetchGroups($search = '') { if($this->isAdmin) { - $this->groups = $this->groupManager->search($search); + return $this->groupManager->search($search); } else { - $this->groups = \OC_SubAdmin::getSubAdminsGroups($this->user); + return \OC_SubAdmin::getSubAdminsGroups($this->user); } } } diff --git a/settings/ajax/grouplist.php b/settings/ajax/grouplist.php index 91700adc35..52df98c4e5 100644 --- a/settings/ajax/grouplist.php +++ b/settings/ajax/grouplist.php @@ -27,6 +27,12 @@ if (isset($_GET['pattern']) && !empty($_GET['pattern'])) { } else { $pattern = ''; } +if (isset($_GET['filterGroups']) && !empty($_GET['filterGroups'])) { + $filterGroups = intval($_GET['filterGroups']) === 1; +} else { + $filterGroups = false; +} +$groupPattern = $filterGroups ? $pattern : ''; $groups = array(); $adminGroups = array(); $groupManager = \OC_Group::getManager(); @@ -36,13 +42,7 @@ $isAdmin = OC_User::isAdminUser(OC_User::getUser()); //groups will be filtered out later $groupsInfo = new \OC\Group\MetaData(OC_User::getUser(), true, $groupManager); $groupsInfo->setSorting($groupsInfo::SORT_USERCOUNT); -list($adminGroups, $groups) = $groupsInfo->get($pattern); - -$accessibleGroups = $groupManager->search($pattern); -if(!$isAdmin) { - $subadminGroups = OC_SubAdmin::getSubAdminsGroups(OC_User::getUser()); - $accessibleGroups = array_intersect($groups, $subadminGroups); -} +list($adminGroups, $groups) = $groupsInfo->get($groupPattern, $pattern); OC_JSON::success( array('data' => array('adminGroups' => $adminGroups, 'groups' => $groups))); diff --git a/settings/js/users/filter.js b/settings/js/users/filter.js index 1f7a29de0c..c5944e9b4b 100644 --- a/settings/js/users/filter.js +++ b/settings/js/users/filter.js @@ -14,6 +14,7 @@ function UserManagementFilter(filterInput, userList, groupList) { this.filterInput = filterInput; this.userList = userList; this.groupList = groupList; + this.filterGroups = false; this.thread = undefined; this.oldval = this.filterInput.val(); @@ -55,7 +56,10 @@ UserManagementFilter.prototype.init = function() { UserManagementFilter.prototype.run = _.debounce(function() { this.userList.empty(); this.userList.update(GroupList.getCurrentGID()); - this.groupList.empty(); + if(this.filterGroups) { + // user counts are being updated nevertheless + this.groupList.empty(); + } this.groupList.update(); }, 300 diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js index 22f5c9d2a7..b4cf73c59f 100644 --- a/settings/js/users/groups.js +++ b/settings/js/users/groups.js @@ -114,7 +114,10 @@ GroupList = { GroupList.updating = true; $.get( OC.generateUrl('/settings/ajax/grouplist'), - {pattern: filter.getPattern()}, + { + pattern: filter.getPattern(), + filterGroups: filter.filterGroups ? 1 : 0 + }, function (result) { var lis = []; diff --git a/tests/lib/group/metadata.php b/tests/lib/group/metadata.php new file mode 100644 index 0000000000..7ef2d6b35f --- /dev/null +++ b/tests/lib/group/metadata.php @@ -0,0 +1,101 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test\Group; + +class Test_MetaData extends \PHPUnit_Framework_TestCase { + private function getGroupManagerMock() { + return $this->getMockBuilder('\OC\Group\Manager') + ->disableOriginalConstructor() + ->getMock(); + } + + private function getGroupMock() { + $group = $this->getMockBuilder('\OC\Group\Group') + ->disableOriginalConstructor() + ->getMock(); + + $group->expects($this->exactly(9)) + ->method('getGID') + ->will($this->onConsecutiveCalls( + 'admin', 'admin', 'admin', + 'g2', 'g2', 'g2', + 'g3', 'g3', 'g3')); + + $group->expects($this->exactly(3)) + ->method('count') + ->with('') + ->will($this->onConsecutiveCalls(2, 3, 5)); + + return $group; + } + + + public function testGet() { + $groupManager = $this->getGroupManagerMock(); + $groupMetaData = new \OC\Group\MetaData('foo', true, $groupManager); + $group = $this->getGroupMock(); + $groups = array_fill(0, 3, $group); + + $groupManager->expects($this->once()) + ->method('search') + ->with('') + ->will($this->returnValue($groups)); + + list($adminGroups, $ordinaryGroups) = $groupMetaData->get(); + + $this->assertSame(1, count($adminGroups)); + $this->assertSame(2, count($ordinaryGroups)); + + $this->assertSame('g2', $ordinaryGroups[0]['name']); + $this->assertSame(3, $ordinaryGroups[0]['usercount']); + } + + public function testGetWithSorting() { + $groupManager = $this->getGroupManagerMock(); + $groupMetaData = new \OC\Group\MetaData('foo', true, $groupManager); + $groupMetaData->setSorting($groupMetaData::SORT_USERCOUNT); + $group = $this->getGroupMock(); + $groups = array_fill(0, 3, $group); + + $groupManager->expects($this->once()) + ->method('search') + ->with('') + ->will($this->returnValue($groups)); + + list($adminGroups, $ordinaryGroups) = $groupMetaData->get(); + + $this->assertSame(1, count($adminGroups)); + $this->assertSame(2, count($ordinaryGroups)); + + $this->assertSame('g3', $ordinaryGroups[0]['name']); + $this->assertSame(5, $ordinaryGroups[0]['usercount']); + } + + public function testGetWithCache() { + $groupManager = $this->getGroupManagerMock(); + $groupMetaData = new \OC\Group\MetaData('foo', true, $groupManager); + $group = $this->getGroupMock(); + $groups = array_fill(0, 3, $group); + + $groupManager->expects($this->once()) + ->method('search') + ->with('') + ->will($this->returnValue($groups)); + + //two calls, if caching fails call counts for group and groupmanager + //are exceeded + $groupMetaData->get(); + $groupMetaData->get(); + } + + //get() does not need to be tested with search parameters, because they are + //solely and only passed to GroupManager and Group. + +}