Fix performance problem when searching for users in the groupfolder app

Before we were fetching 1000 users and then filter them by name

Signed-off-by: Carl Schwan <[email protected]>

apps/user_ldap/lib/Group_LDAP.php

@@ -47,9 +47,13 @@
 use OC;
 use OC\Cache\CappedMemoryCache;
 use OC\ServerNotAvailableException;
+use OC\User\DisplayNameCache;
+use OC\User\LazyUser;
 use OCP\Group\Backend\IGetDisplayNameBackend;
 use OCP\Group\Backend\IDeleteGroupBackend;
 use OCP\GroupInterface;
+use OCP\IUser;
+use OCP\IUserManager;
 use Psr\Log\LoggerInterface;
 
 class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
@@ -1370,4 +1374,16 @@ public function getDisplayName(string $gid): string {
 
 		return '';
 	}
+
+	public function searchDisplayName(string $gid, string $search = '', int $limit = -1, int $offset = 0): array {
+		if (!$this->enabled) {
+			return [];
+		}
+		$users = $this->usersInGroup($gid, $search, $limit, $offset);
+		$userManager = \OC::$server->get(IUserManager::class);
+		$displayNameCache = \OC::$server->get(DisplayNameCache::class);
+		return array_map(function (string $userId) use ($userManager, $displayNameCache): IUser {
+			return new LazyUser($userId, $displayNameCache, $userManager);
+		}, $users);
+	}
 }

apps/user_ldap/lib/Group_Proxy.php

@@ -306,4 +306,17 @@ public function getDisplayName(string $gid): string {
 	public function getBackendName(): string {
 		return 'LDAP';
 	}
+
+	public function searchDisplayName(string $gid, string $search = '', int $limit = -1, int $offset = 0): array {
+		$users = [];
+
+		foreach ($this->backends as $backend) {
+			$backendUsers = $backend->searchDisplayName($gid, $search, $limit, $offset);
+			if (is_array($backendUsers)) {
+				$users = array_merge($users, $backendUsers);
+			}
+		}
+
+		return $users;
+	}
 }

lib/private/Group/Backend.php

@@ -131,4 +131,8 @@ public function groupExists($gid) {
 	public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
 		return [];
 	}
+
+	public function searchDisplayName(string $gid, string $search = '', int $limit = -1, int $offset = 0): array {
+		return $this->usersInGroup($gid, $search, $limit, $offset);
+	}
 }

lib/private/Group/Database.php

@@ -30,6 +30,8 @@
 namespace OC\Group;
 
 use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
+use OC\User\DisplayNameCache;
+use OC\User\LazyUser;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Group\Backend\ABackend;
 use OCP\Group\Backend\IAddToGroupBackend;
@@ -43,6 +45,8 @@
 use OCP\Group\Backend\ISetDisplayNameBackend;
 use OCP\Group\Backend\INamedBackend;
 use OCP\IDBConnection;
+use OCP\IUser;
+use OCP\IUserManager;
 
 /**
  * Class for group management in a SQL Database (e.g. MySQL, SQLite)
@@ -377,6 +381,67 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
 		return $users;
 	}
 
+	/**
+	 * Get a list of all users in a group by display name
+	 * @return IUser[]
+	 */
+	public function searchDisplayName(string $gid, string $search = '', int $limit = -1, int $offset = 0): array {
+		$this->fixDI();
+
+		$query = $this->dbConn->getQueryBuilder();
+		if ($search !== '') {
+			$query->select('g.uid', 'u.displayname');
+		} else {
+			$query->select('g.uid');
+		}
+
+		$query->from('group_user', 'g')
+			->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
+			->orderBy('g.uid', 'ASC');
+
+		if ($search !== '') {
+			$query->leftJoin('g', 'users', 'u', $query->expr()->eq('g.uid', 'u.uid'))
+				->leftJoin('u', 'preferences', 'p', $query->expr()->andX(
+					$query->expr()->eq('p.userid', 'u.uid'),
+					$query->expr()->eq('p.appid', $query->expr()->literal('settings')),
+					$query->expr()->eq('p.configkey', $query->expr()->literal('email')))
+				)
+				// sqlite doesn't like re-using a single named parameter here
+				->andWhere(
+					$query->expr()->orX(
+						$query->expr()->ilike('g.uid', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')),
+						$query->expr()->ilike('u.displayname', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')),
+						$query->expr()->ilike('p.configvalue', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%'))
+					)
+				)
+				->orderBy('u.uid_lower', 'ASC');
+		}
+
+		if ($limit !== -1) {
+			$query->setMaxResults($limit);
+		}
+		if ($offset !== 0) {
+			$query->setFirstResult($offset);
+		}
+
+		$result = $query->execute();
+
+		$users = [];
+		$userManager = \OC::$server->get(IUserManager::class);
+		$displayNameCache = \OC::$server->get(DisplayNameCache::class);
+		while ($row = $result->fetch()) {
+			if (isset($row['displayname'])) {
+				$users[] = new LazyUser($row['uid'], $displayNameCache, $userManager, $row['displayname']);
+			} else {
+				// TODO maybe also fetch the displayname directly here
+				$users[] = new LazyUser($row['uid'], $displayNameCache, $userManager);
+			}
+		}
+		$result->closeCursor();
+
+		return $users;
+	}
+
 	/**
 	 * get the number of all users matching the search string in a group
 	 * @param string $gid

lib/private/Group/Group.php

@@ -299,18 +299,17 @@ public function countDisabled() {
 	}
 
 	/**
-	 * search for users in the group by displayname
+	 * Search for users in the group by display name
 	 *
 	 * @param string $search
 	 * @param int $limit
 	 * @param int $offset
-	 * @return \OC\User\User[]
+	 * @return \OCP\IUser[]
 	 */
 	public function searchDisplayName($search, $limit = null, $offset = null) {
 		$users = [];
 		foreach ($this->backends as $backend) {
-			$userIds = $backend->usersInGroup($this->gid, $search, $limit, $offset);
-			$users = $this->getVerifiedUsers($userIds);
+			$users = $backend->searchDisplayName($this->gid, $search, $limit, $offset);
 			if (!is_null($limit) and $limit <= 0) {
 				return array_values($users);
 			}
@@ -369,7 +368,7 @@ public function delete() {
 	/**
 	 * returns all the Users from an array that really exists
 	 * @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
+	 * @return \OCP\IUser[] an Array with the userId as Key and \OC\User\User as value
 	 */
 	private function getVerifiedUsers($userIds) {
 		if (!is_array($userIds)) {

lib/private/Group/Manager.php

@@ -351,7 +351,7 @@ public function getUserGroupNames(IUser $user) {
 	}
 
 	/**
-	 * get a list of all display names in a group
+	 * Get a list of all display names in a group
 	 *
 	 * @param string $gid
 	 * @param string $search
@@ -360,40 +360,12 @@ public function getUserGroupNames(IUser $user) {
 	 * @return array an array of display names (value) and user ids (key)
 	 */
 	public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) {
+		$search = trim($search);
 		$group = $this->get($gid);
 		if (is_null($group)) {
 			return [];
 		}
-
-		$search = trim($search);
-		$groupUsers = [];
-
-		if (!empty($search)) {
-			// only user backends have the capability to do a complex search for users
-			$searchOffset = 0;
-			$searchLimit = $limit * 100;
-			if ($limit === -1) {
-				$searchLimit = 500;
-			}
-
-			do {
-				$filteredUsers = $this->userManager->searchDisplayName($search, $searchLimit, $searchOffset);
-				foreach ($filteredUsers as $filteredUser) {
-					if ($group->inGroup($filteredUser)) {
-						$groupUsers[] = $filteredUser;
-					}
-				}
-				$searchOffset += $searchLimit;
-			} while (count($groupUsers) < $searchLimit + $offset && count($filteredUsers) >= $searchLimit);
-
-			if ($limit === -1) {
-				$groupUsers = array_slice($groupUsers, $offset);
-			} else {
-				$groupUsers = array_slice($groupUsers, $offset, $limit);
-			}
-		} else {
-			$groupUsers = $group->searchUsers('', $limit, $offset);
-		}
+		$groupUsers = $group->searchDisplayName($search, $limit, $offset);
 
 		$matchingUsers = [];
 		foreach ($groupUsers as $groupUser) {

lib/private/User/LazyUser.php

@@ -31,12 +31,14 @@ class LazyUser implements IUser {
 	private ?IUser $user = null;
 	private DisplayNameCache $displayNameCache;
 	private string $uid;
+	private ?string $displayName;
 	private IUserManager $userManager;
 
-	public function __construct(string $uid, DisplayNameCache $displayNameCache, IUserManager $userManager) {
+	public function __construct(string $uid, DisplayNameCache $displayNameCache, IUserManager $userManager, ?string $displayName = null) {
 		$this->displayNameCache = $displayNameCache;
 		$this->uid = $uid;
 		$this->userManager = $userManager;
+		$this->displayName = $displayName;
 	}
 
 	private function getUser(): IUser {
@@ -53,6 +55,10 @@ public function getUID() {
 	}
 
 	public function getDisplayName() {
+		if ($this->displayName) {
+			return $this->displayName;
+		}
+
 		return $this->displayNameCache->getDisplayName($this->uid);
 	}
 

lib/private/User/Manager.php

@@ -323,12 +323,7 @@ public function search($pattern, $limit = null, $offset = null) {
 	public function searchDisplayName($pattern, $limit = null, $offset = null) {
 		$users = [];
 		foreach ($this->backends as $backend) {
-			$backendUsers = $backend->getDisplayNames($pattern, $limit, $offset);
-			if (is_array($backendUsers)) {
-				foreach ($backendUsers as $uid => $displayName) {
-					$users[] = $this->getUserObject($uid, $backend);
-				}
-			}
+			$users = array_merge($users, $backend->getDisplayNames($pattern, $limit, $offset));
 		}
 
 		usort($users, function ($a, $b) {

lib/public/GroupInterface.php

@@ -116,4 +116,9 @@ public function groupExists($gid);
 	 * @since 4.5.0
 	 */
 	public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0);
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function searchDisplayName(string $gid, string $search = '', int $limit = -1, int $offset = 0): array;
 }