Merge pull request #7745 from owncloud/fix_6946_stable6

Group Database backend must not gather user details itself but ask user backends, fixes #6946 in stable6 

11 files changed, 95 insertions, 125 deletions

diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php
index 32e2cec5960..30217412611 100644
--- a/apps/user_ldap/group_ldap.php
+++ b/apps/user_ldap/group_ldap.php
@@ -219,25 +219,6 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 	}
 
 	/**
-	 * @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
 	 *
@@ -318,6 +299,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	& $actions);
+		return false;
 	}
 }
diff --git a/apps/user_ldap/group_proxy.php b/apps/user_ldap/group_proxy.php
index 4404bd7fe3a..97b342c1183 100644
--- a/apps/user_ldap/group_proxy.php
+++ b/apps/user_ldap/group_proxy.php
@@ -145,22 +145,6 @@ class Group_Proxy extends lib\Proxy implements \OCP\GroupInterface {
 	}
 
 	/**
-	 * @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 444788c97f1..b45855c9215 100644
--- a/lib/private/group.php
+++ b/lib/private/group.php
@@ -263,17 +263,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 2e17b5d0b7f..11ec6c2920d 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
 
 /**
  * Abstract base class for user management
@@ -44,7 +44,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',
 	);
 
 	/**
@@ -135,23 +134,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 d0974685ff6..fa43d4e12fa 100644
--- a/lib/private/group/database.php
+++ b/lib/private/group/database.php
@@ -210,30 +210,4 @@ class OC_Group_Database extends OC_Group_Backend {
 		}
 		return $users;
 	}
-
-	/**
-	 * @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 d5a17f6d215..ae8eeacb23a 100644
--- a/lib/private/group/group.php
+++ b/lib/private/group/group.php
@@ -180,11 +180,7 @@ class Group {
 	 */
 	public function searchDisplayName($search, $limit = null, $offset = null) {
 		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 454e2b63b47..e5d3076b7eb 100644
--- a/lib/private/group/manager.php
+++ b/lib/private/group/manager.php
@@ -160,4 +160,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 61abb00f995..d23ef381bec 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 8de8d033e19..26232187c36 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 830eb1b48bf..18be3eb96c1 100644
--- a/tests/lib/group/manager.php
+++ b/tests/lib/group/manager.php
@@ -310,4 +310,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']));
+	}
 }
diff --git a/tests/lib/user/manager.php b/tests/lib/user/manager.php
index 8ca0f94c6fa..fd0931af7e4 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() {