consolidate validity check for users in group class

2 files changed, 29 insertions, 28 deletions

diff --git a/lib/group/group.php b/lib/group/group.php
index bb1537b5c66..bcd2419b309 100644
--- a/lib/group/group.php
+++ b/lib/group/group.php
@@ -62,7 +62,6 @@ class Group {
 			return $this->users;
 		}
 
-		$users = array();
 		$userIds = array();
 		foreach ($this->backends as $backend) {
 			$diff = array_diff(
@@ -74,14 +73,8 @@ class Group {
 			}
 		}
 
-		foreach ($userIds as $userId) {
-			$user = $this->userManager->get($userId);
-			if(!is_null($user)) {
-				$users[] = $user;
-			}
-		}
-		$this->users = $users;
-		return $users;
+		$this->users = $this->getVerifiedUsers($userIds);
+		return $this->users;
 	}
 
 	/**
@@ -116,7 +109,7 @@ class Group {
 			if ($backend->implementsActions(OC_GROUP_BACKEND_ADD_TO_GROUP)) {
 				$backend->addToGroup($user->getUID(), $this->gid);
 				if ($this->users) {
-					$this->users[] = $user;
+					$this->users[$user->getUID()] = $user;
 				}
 				if ($this->emitter) {
 					$this->emitter->emit('\OC\Group', 'postAddUser', array($this, $user));
@@ -175,12 +168,7 @@ class Group {
 			if (!is_null($offset)) {
 				$offset -= count($userIds);
 			}
-			foreach ($userIds as $userId) {
-				$user = $this->userManager->get($userId);
-				if(!is_null($user)) {
-					$users[$userId] = $user;
-				}
-			}
+			$users += $this->getVerifiedUsers($userIds);
 			if (!is_null($limit) and $limit <= 0) {
 				return array_values($users);
 			}
@@ -197,7 +185,6 @@ class Group {
 	 * @return \OC\User\User[]
 	 */
 	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));
@@ -210,12 +197,7 @@ class Group {
 			if (!is_null($offset)) {
 				$offset -= count($userIds);
 			}
-			foreach ($userIds as $userId) {
-				$user = $this->userManager->get($userId);
-				if(!is_null($user)) {
-					$users[$userId] = $user;
-				}
-			}
+			$users = $this->getVerifiedUsers($userIds);
 			if (!is_null($limit) and $limit <= 0) {
 				return array_values($users);
 			}
@@ -244,4 +226,23 @@ class Group {
 		}
 		return $result;
 	}
+
+	/**
+	 * @brief returns all the Users from an array that really exists
+	 * @param $userIds an array containing user IDs
+	 * @return an Array with the userId as Key and \OC\User\User as value
+	 */
+	private function getVerifiedUsers($userIds) {
+		if(!is_array($userIds)) {
+			return array();
+		}
+		$users = array();
+		foreach ($userIds as $userId) {
+			$user = $this->userManager->get($userId);
+			if(!is_null($user)) {
+				$users[$userId] = $user;
+			}
+		}
+		return $users;
+	}
 }
diff --git a/tests/lib/group/group.php b/tests/lib/group/group.php
index 75e975d9e65..f1fda3b9288 100644
--- a/tests/lib/group/group.php
+++ b/tests/lib/group/group.php
@@ -43,8 +43,8 @@ class Group extends \PHPUnit_Framework_TestCase {
 		$users = $group->getUsers();
 
 		$this->assertEquals(2, count($users));
-		$user1 = $users[0];
-		$user2 = $users[1];
+		$user1 = $users['user1'];
+		$user2 = $users['user2'];
 		$this->assertEquals('user1', $user1->getUID());
 		$this->assertEquals('user2', $user2->getUID());
 	}
@@ -68,9 +68,9 @@ class Group extends \PHPUnit_Framework_TestCase {
 		$users = $group->getUsers();
 
 		$this->assertEquals(3, count($users));
-		$user1 = $users[0];
-		$user2 = $users[1];
-		$user3 = $users[2];
+		$user1 = $users['user1'];
+		$user2 = $users['user2'];
+		$user3 = $users['user3'];
 		$this->assertEquals('user1', $user1->getUID());
 		$this->assertEquals('user2', $user2->getUID());
 		$this->assertEquals('user3', $user3->getUID());