From 10296ba7e50ede8eee0ed18f477eb3e67bfd5878 Mon Sep 17 00:00:00 2001
From: =?utf8?q?C=C3=B4me=20Chilliet?= <come.chilliet@nextcloud.com>
Date: Tue, 2 May 2023 11:35:41 +0200
Subject: [PATCH] Fix tests, and fix Group::searchUsers to avoid duplicates
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

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>
---
 lib/private/Group/Group.php     | 17 +++++++----------
 tests/lib/Group/GroupTest.php   | 10 +++++-----
 tests/lib/Group/ManagerTest.php |  6 +++++-
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/lib/private/Group/Group.php b/lib/private/Group/Group.php
index 7f04e45de80..efc21ad7c0d 100644
--- 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 99389880383..ac648fd7b4b 100644
--- 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 186f9d619a2..710d3888d55 100644
--- 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',
-- 
2.39.5