consolidate validity check for users in group class

diff --git a/lib/group/group.php b/lib/group/group.php
index bb1537b5c66785b894bc75ae9093b96c3a87f04b..bcd2419b309b5964e43ca5353e6da4bb059d2b3b 100644 (file)
--- 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 75e975d9e6505bb60e3bb45248b545faac0afc94..f1fda3b928836d1c0bac790cc404f2de1746f97d 100644 (file)
--- 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());