diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php index 3b5f377e10..e5bec602f6 100644 --- a/apps/user_ldap/group_ldap.php +++ b/apps/user_ldap/group_ldap.php @@ -362,25 +362,6 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { return count($groupUsers); } - /** - * @brief get a list of all display names in a group - * @returns array with display names (value) and user ids(key) - */ - public function displayNamesInGroup($gid, $search, $limit, $offset) { - if(!$this->enabled) { - return array(); - } - if(!$this->groupExists($gid)) { - return array(); - } - $users = $this->usersInGroup($gid, $search, $limit, $offset); - $displayNames = array(); - foreach($users as $user) { - $displayNames[$user] = \OC_User::getDisplayName($user); - } - return $displayNames; - } - /** * @brief get a list of all groups * @returns array with group names @@ -507,9 +488,6 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface { * compared with OC_USER_BACKEND_CREATE_USER etc. */ public function implementsActions($actions) { - return (bool)(( - OC_GROUP_BACKEND_GET_DISPLAYNAME - | OC_GROUP_BACKEND_COUNT_USERS - ) & $actions); + return (bool)(OC_GROUP_BACKEND_COUNT_USERS & $actions); } } diff --git a/apps/user_ldap/group_proxy.php b/apps/user_ldap/group_proxy.php index c000973623..ea94990ffe 100644 --- a/apps/user_ldap/group_proxy.php +++ b/apps/user_ldap/group_proxy.php @@ -155,22 +155,6 @@ class Group_Proxy extends lib\Proxy implements \OCP\GroupInterface { $gid, 'countUsersInGroup', array($gid, $search)); } - /** - * @brief get a list of all display names in a group - * @returns array with display names (value) and user ids(key) - */ - public function displayNamesInGroup($gid, $search, $limit, $offset) { - $displayNames = array(); - - foreach($this->backends as $backend) { - $backendUsers = $backend->displayNamesInGroup($gid, $search, $limit, $offset); - if (is_array($backendUsers)) { - $displayNames = array_merge($displayNames, $backendUsers); - } - } - return $displayNames; - } - /** * @brief get a list of all groups * @returns array with group names diff --git a/lib/private/group.php b/lib/private/group.php index d9f430f833..ea6384bae3 100644 --- a/lib/private/group.php +++ b/lib/private/group.php @@ -274,17 +274,7 @@ class OC_Group { * @returns array with display names (value) and user ids(key) */ public static function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) { - $group = self::getManager()->get($gid); - if ($group) { - $users = $group->searchDisplayName($search, $limit, $offset); - $displayNames = array(); - foreach ($users as $user) { - $displayNames[$user->getUID()] = $user->getDisplayName(); - } - return $displayNames; - } else { - return array(); - } + return self::getManager()->displayNamesInGroup($gid, $search, $limit, $offset); } /** diff --git a/lib/private/group/backend.php b/lib/private/group/backend.php index b0ed0d90d7..cc61fce161 100644 --- a/lib/private/group/backend.php +++ b/lib/private/group/backend.php @@ -33,7 +33,7 @@ define('OC_GROUP_BACKEND_CREATE_GROUP', 0x00000001); define('OC_GROUP_BACKEND_DELETE_GROUP', 0x00000010); define('OC_GROUP_BACKEND_ADD_TO_GROUP', 0x00000100); define('OC_GROUP_BACKEND_REMOVE_FROM_GOUP', 0x00001000); -define('OC_GROUP_BACKEND_GET_DISPLAYNAME', 0x00010000); +define('OC_GROUP_BACKEND_GET_DISPLAYNAME', 0x00010000); //OBSOLETE define('OC_GROUP_BACKEND_COUNT_USERS', 0x00100000); /** @@ -45,7 +45,6 @@ abstract class OC_Group_Backend implements OC_Group_Interface { OC_GROUP_BACKEND_DELETE_GROUP => 'deleteGroup', OC_GROUP_BACKEND_ADD_TO_GROUP => 'addToGroup', OC_GROUP_BACKEND_REMOVE_FROM_GOUP => 'removeFromGroup', - OC_GROUP_BACKEND_GET_DISPLAYNAME => 'displayNamesInGroup', OC_GROUP_BACKEND_COUNT_USERS => 'countUsersInGroup', ); @@ -137,23 +136,4 @@ abstract class OC_Group_Backend implements OC_Group_Interface { public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) { return array(); } - - /** - * @brief get a list of all display names in a group - * @param string $gid - * @param string $search - * @param int $limit - * @param int $offset - * @return array with display names (value) and user ids (key) - */ - public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) { - $displayNames = array(); - $users = $this->usersInGroup($gid, $search, $limit, $offset); - foreach ($users as $user) { - $displayNames[$user] = $user; - } - - return $displayNames; - } - } diff --git a/lib/private/group/database.php b/lib/private/group/database.php index 3815dcff2e..df0d84d0d2 100644 --- a/lib/private/group/database.php +++ b/lib/private/group/database.php @@ -225,29 +225,4 @@ class OC_Group_Database extends OC_Group_Backend { return $result->fetchOne(); } - /** - * @brief get a list of all display names in a group - * @param string $gid - * @param string $search - * @param int $limit - * @param int $offset - * @return array with display names (value) and user ids (key) - */ - public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) { - $displayNames = array(); - - $stmt = OC_DB::prepare('SELECT `*PREFIX*users`.`uid`, `*PREFIX*users`.`displayname`' - .' FROM `*PREFIX*users`' - .' INNER JOIN `*PREFIX*group_user` ON `*PREFIX*group_user`.`uid` = `*PREFIX*users`.`uid`' - .' WHERE `gid` = ? AND `*PREFIX*group_user`.`uid` LIKE ?', - $limit, - $offset); - $result = $stmt->execute(array($gid, $search.'%')); - $users = array(); - while ($row = $result->fetchRow()) { - $displayName = trim($row['displayname'], ' '); - $displayNames[$row['uid']] = empty($displayName) ? $row['uid'] : $displayName; - } - return $displayNames; - } } diff --git a/lib/private/group/group.php b/lib/private/group/group.php index 3efbb6e702..7593bb68d1 100644 --- a/lib/private/group/group.php +++ b/lib/private/group/group.php @@ -212,11 +212,7 @@ class Group { public function searchDisplayName($search, $limit = null, $offset = null) { $users = array(); foreach ($this->backends as $backend) { - if ($backend->implementsActions(OC_GROUP_BACKEND_GET_DISPLAYNAME)) { - $userIds = array_keys($backend->displayNamesInGroup($this->gid, $search, $limit, $offset)); - } else { - $userIds = $backend->usersInGroup($this->gid, $search, $limit, $offset); - } + $userIds = $backend->usersInGroup($this->gid, $search, $limit, $offset); $users = $this->getVerifiedUsers($userIds); if (!is_null($limit) and $limit <= 0) { return array_values($users); diff --git a/lib/private/group/manager.php b/lib/private/group/manager.php index f591bd32ab..d31225e3c2 100644 --- a/lib/private/group/manager.php +++ b/lib/private/group/manager.php @@ -158,4 +158,38 @@ class Manager extends PublicEmitter { } return array_values($groups); } + + /** + * @brief get a list of all display names in a group + * @param string $gid + * @param string $search + * @param int $limit + * @param int $offset + * @return array with display names (value) and user ids (key) + */ + public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) { + $group = $this->get($gid); + if(is_null($group)) { + return array(); + } + // only user backends have the capability to do a complex search for users + $groupUsers = $group->searchUsers('', $limit, $offset); + $search = trim($search); + if(!empty($search)) { + //TODO: for OC 7 earliest: user backend should get a method to check selected users against a pattern + $filteredUsers = $this->userManager->search($search); + $testUsers = true; + } else { + $filteredUsers = array(); + $testUsers = false; + } + + $matchingUsers = array(); + foreach($groupUsers as $user) { + if(!$testUsers || isset($filteredUsers[$user->getUID()])) { + $matchingUsers[$user->getUID()] = $user->getDisplayName(); + } + } + return $matchingUsers; + } } diff --git a/lib/private/user/manager.php b/lib/private/user/manager.php index 14698452e8..6f6fd80a8e 100644 --- a/lib/private/user/manager.php +++ b/lib/private/user/manager.php @@ -174,12 +174,12 @@ class Manager extends PublicEmitter { $backendUsers = $backend->getUsers($pattern, $limit, $offset); if (is_array($backendUsers)) { foreach ($backendUsers as $uid) { - $users[] = $this->getUserObject($uid, $backend); + $users[$uid] = $this->getUserObject($uid, $backend); } } } - usort($users, function ($a, $b) { + uasort($users, function ($a, $b) { /** * @var \OC\User\User $a * @var \OC\User\User $b diff --git a/tests/lib/group.php b/tests/lib/group.php index 8de8d033e1..26232187c3 100644 --- a/tests/lib/group.php +++ b/tests/lib/group.php @@ -109,25 +109,6 @@ class Test_Group extends PHPUnit_Framework_TestCase { $this->assertEquals(array(), OC_Group::getGroups()); } - public function testDisplayNamesInGroup() { - OC_Group::useBackend(new OC_Group_Dummy()); - $userBackend = new \OC_User_Dummy(); - \OC_User::getManager()->registerBackend($userBackend); - - $group1 = uniqid(); - $user1 = 'uid1'; - $user2 = 'uid2'; - OC_Group::createGroup($group1); - $userBackend->createUser($user1, ''); - $userBackend->createUser($user2, ''); - OC_Group::addToGroup($user1, $group1); - OC_Group::addToGroup($user2, $group1); - //Dummy backend does not support setting displaynames, uid will always - //be returned. This checks primarily, that the return format is okay. - $expected = array($user1 => $user1, $user2 => $user2); - $this->assertEquals($expected, OC_Group::displayNamesInGroup($group1)); - } - public function testUsersInGroup() { OC_Group::useBackend(new OC_Group_Dummy()); $userBackend = new \OC_User_Dummy(); diff --git a/tests/lib/group/manager.php b/tests/lib/group/manager.php index a03997c58e..9d1f175c10 100644 --- a/tests/lib/group/manager.php +++ b/tests/lib/group/manager.php @@ -343,4 +343,54 @@ class Manager extends \PHPUnit_Framework_TestCase { $this->assertEquals('group1', $group1->getGID()); $this->assertEquals('group2', $group2->getGID()); } + + public function testDisplayNamesInGroupMultipleUserBackends() { + $user1 = new User('user1', null); + $user2 = new User('user2', null); + $user3 = new User('user3', null); + $user4 = new User('user33', null); + + /** + * @var \PHPUnit_Framework_MockObject_MockObject | \OC_Group_Backend $backend1 + */ + $backend = $this->getMock('\OC_Group_Database'); + $backend->expects($this->exactly(1)) + ->method('groupExists') + ->with('testgroup') + ->will($this->returnValue(true)); + + $backend->expects($this->once()) + ->method('usersInGroup') + ->with('testgroup', '', -1, 0) + ->will($this->returnValue(array('user2', 'user33'))); + + /** + * @var \OC\User\Manager $userManager + */ + $userManager = $this->getMock('\OC\User\Manager'); + $userManager->expects($this->once()) + ->method('search') + ->with('user3') + ->will($this->returnValue(array('user3' => $user3, 'user33' => $user4))); + + $userManager->expects($this->any()) + ->method('get') + ->will($this->returnCallback(function($uid) { + switch($uid) { + case 'user1' : return new User('user1', null); + case 'user2' : return new User('user2', null); + case 'user3' : return new User('user3', null); + case 'user33': return new User('user33', null); + default: + return null; + } + })); + + $manager = new \OC\Group\Manager($userManager); + $manager->addBackend($backend); + + $users = $manager->displayNamesInGroup('testgroup', 'user3'); + $this->assertEquals(1, count($users)); + $this->assertTrue(isset($users['user33'])); + } } diff --git a/tests/lib/user/manager.php b/tests/lib/user/manager.php index 8ca0f94c6f..fd0931af7e 100644 --- a/tests/lib/user/manager.php +++ b/tests/lib/user/manager.php @@ -190,8 +190,8 @@ class Manager extends \PHPUnit_Framework_TestCase { $result = $manager->search('fo'); $this->assertEquals(2, count($result)); - $this->assertEquals('afoo', $result[0]->getUID()); - $this->assertEquals('foo', $result[1]->getUID()); + $this->assertEquals('afoo', array_shift($result)->getUID()); + $this->assertEquals('foo', array_shift($result)->getUID()); } public function testSearchTwoBackendLimitOffset() { @@ -219,9 +219,9 @@ class Manager extends \PHPUnit_Framework_TestCase { $result = $manager->search('fo', 3, 1); $this->assertEquals(3, count($result)); - $this->assertEquals('foo1', $result[0]->getUID()); - $this->assertEquals('foo2', $result[1]->getUID()); - $this->assertEquals('foo3', $result[2]->getUID()); + $this->assertEquals('foo1', array_shift($result)->getUID()); + $this->assertEquals('foo2', array_shift($result)->getUID()); + $this->assertEquals('foo3', array_shift($result)->getUID()); } public function testCreateUserSingleBackendNotExists() {