Fix tests, and fix Group::searchUsers to avoid duplicates

Also went back to searchUsers indexing the array by uid as it was the
 previous behavior and the IGroup phpdoc does not say anything about the
 keys.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

diff --git a/lib/private/Group/Group.php b/lib/private/Group/Group.php
index 7f04e45de80b3bf972b725c5e7d39c9a13e249eb..efc21ad7c0dc01f5209b88624f627c430498e10a 100644 (file)
--- a/lib/private/Group/Group.php
+++ b/lib/private/Group/Group.php
@@ -251,15 +251,15 @@ class Group implements IGroup {
                $users = [];
                foreach ($this->backends as $backend) {
                        if ($backend instanceof ISearchableGroupBackend) {
-                               $users = array_merge($users, array_values($backend->searchInGroup($this->gid, $search, $limit ?? -1, $offset ?? 0)));
+                               $users += $backend->searchInGroup($this->gid, $search, $limit ?? -1, $offset ?? 0);
                        } else {
                                $userIds = $backend->usersInGroup($this->gid, $search, $limit ?? -1, $offset ?? 0);
                                $userManager = \OCP\Server::get(IUserManager::class);
-                               $userObjects = array_map(
-                                       fn (string $userId): IUser => new LazyUser($userId, $userManager),
-                                       $userIds
-                               );
-                               $users = array_merge($users, $userObjects);
+                               foreach ($userIds as $userId) {
+                                       if (!isset($users[$userId])) {
+                                               $users[$userId] = new LazyUser($userId, $userManager);
+                                       }
+                               }
                        }
                        if (!is_null($limit) and $limit <= 0) {
                                return $users;
@@ -375,10 +375,7 @@ class Group implements IGroup {
         * @param string[] $userIds an array containing user IDs
         * @return \OC\User\User[] an Array with the userId as Key and \OC\User\User as value
         */
-       private function getVerifiedUsers($userIds) {
-               if (!is_array($userIds)) {
-                       return [];
-               }
+       private function getVerifiedUsers(array $userIds): array {
                $users = [];
                foreach ($userIds as $userId) {
                        $user = $this->userManager->get($userId);

diff --git a/tests/lib/Group/GroupTest.php b/tests/lib/Group/GroupTest.php
index 993898803831b5509ba2174b9037244489389c69..ac648fd7b4bbf06e6da2d4c97e2028164949f1d1 100644 (file)
--- a/tests/lib/Group/GroupTest.php
+++ b/tests/lib/Group/GroupTest.php
@@ -310,7 +310,7 @@ class GroupTest extends \Test\TestCase {
                $users = $group->searchUsers('2');
 
                $this->assertEquals(1, count($users));
-               $user2 = $users['user2'];
+               $user2 = reset($users);
                $this->assertEquals('user2', $user2->getUID());
        }
 
@@ -336,7 +336,7 @@ class GroupTest extends \Test\TestCase {
                $users = $group->searchUsers('2');
 
                $this->assertEquals(1, count($users));
-               $user2 = $users['user2'];
+               $user2 = reset($users);
                $this->assertEquals('user2', $user2->getUID());
        }
 
@@ -355,7 +355,7 @@ class GroupTest extends \Test\TestCase {
                $users = $group->searchUsers('user', 1, 1);
 
                $this->assertEquals(1, count($users));
-               $user2 = $users['user2'];
+               $user2 = reset($users);
                $this->assertEquals('user2', $user2->getUID());
        }
 
@@ -381,8 +381,8 @@ class GroupTest extends \Test\TestCase {
                $users = $group->searchUsers('user', 2, 1);
 
                $this->assertEquals(2, count($users));
-               $user2 = $users['user2'];
-               $user1 = $users['user1'];
+               $user2 = reset($users);
+               $user1 = next($users);
                $this->assertEquals('user2', $user2->getUID());
                $this->assertEquals('user1', $user1->getUID());
        }

diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php
index 186f9d619a29afca87d2a788a333b7478a6f9fb2..710d3888d55ab80667e847b5d1c2c115417b93be 100644 (file)
--- a/tests/lib/Group/ManagerTest.php
+++ b/tests/lib/Group/ManagerTest.php
@@ -27,6 +27,7 @@ use OC\Group\Database;
 use OC\User\User;
 use OC\User\Manager;
 use OCP\GroupInterface;
+use OCP\Group\Backend\ISearchableGroupBackend;
 use OCP\ICacheFactory;
 use OCP\IUser;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -34,6 +35,9 @@ use Psr\Log\LoggerInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Test\TestCase;
 
+interface ISearchableGroupInterface extends ISearchableGroupBackend, GroupInterface {
+}
+
 class ManagerTest extends TestCase {
        /** @var Manager|MockObject */
        protected $userManager;
@@ -79,7 +83,7 @@ class ManagerTest extends TestCase {
                }
                // need to declare it this way due to optional methods
                // thanks to the implementsActions logic
-               $backend = $this->getMockBuilder(GroupInterface::class)
+               $backend = $this->getMockBuilder(ISearchableGroupInterface::class)
                        ->disableOriginalConstructor()
                        ->setMethods([
                                'getGroupDetails',