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

diff --git a/lib/private/group/metadata.php b/lib/private/group/metadata.php
index 1883ba727e0b2dcf697830576eaca8326e08ed60..303543d48c65de2148db6b69231f5fb89de51db6 100644 (file)
--- 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 91700adc35997f9602b56fd3bbf1b3b5e6724dd7..52df98c4e5e3ed45855be9c2ad6f9b106db63798 100644 (file)
--- 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 1f7a29de0c99775dba03ad1a1649b74badf87375..c5944e9b4b108d2039d6914d0bdc223545bc2db4 100644 (file)
--- 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 22f5c9d2a72f9025cc6565f15ed5d37d7f30943d..b4cf73c59f28f55a726f4140ba3352eef27e5805 100644 (file)
--- 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 (file)
index 0000000..7ef2d6b
--- /dev/null
+++ b/tests/lib/group/metadata.php
@@ -0,0 +1,101 @@
+<?php
+
+/**
+ * Copyright (c) 2014 Arthur Schiwon <blizzz@owncloud.com>
+ * 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.
+
+}