Group Database backend must not gather user details itself but ask user

backends. This is a port to master from PR #7745

remove OC_GROUP_BACKEND_GET_DISPLAYNAME option for group backends

Conflicts:
	lib/private/group/backend.php

LDAP: getDisplayNamesInGroup is not an option for group backends anymore

Conflicts:
	apps/user_ldap/group_ldap.php
	apps/user_ldap/group_proxy.php

clean up group backends

Conflicts:
	lib/private/group/database.php

remove now unnecessary test

implement getDisplayNames in group manager

adjust user manager tests

test for group manager's displayNamesInGroup

trim must not be used in empty in PHP < 5.5

keep the constant to not provoke PHP warnings

Conflicts:
	lib/private/group/backend.php
This commit is contained in:
Arthur Schiwon 2014-03-14 13:51:17 +01:00
parent e27dc5b6f9
commit 45e42c25de
11 changed files with 95 additions and 127 deletions

View File

@ -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);
}
}

View File

@ -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

View File

@ -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);
}
/**

View File

@ -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;
}
}

View File

@ -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;
}
}

View File

@ -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);
}
$users = $this->getVerifiedUsers($userIds);
if (!is_null($limit) and $limit <= 0) {
return array_values($users);

View File

@ -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;
}
}

View File

@ -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

View File

@ -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();

View File

@ -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(2))
->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']));
}
}

View File

@ -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() {