feat(updatenotification): Add a limit to user count from LDAP so save performances

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

apps/updatenotification/lib/Settings/Admin.php

@@ -8,7 +8,6 @@ declare(strict_types=1);
  */
 namespace OCA\UpdateNotification\Settings;
 
-use OC\User\Backend;
 use OCA\UpdateNotification\UpdateChecker;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
@@ -20,7 +19,6 @@ use OCP\IUserManager;
 use OCP\L10N\IFactory;
 use OCP\Settings\ISettings;
 use OCP\Support\Subscription\IRegistry;
-use OCP\User\Backend\ICountUsersBackend;
 use OCP\Util;
 use Psr\Log\LoggerInterface;
 
@@ -141,26 +139,6 @@ class Admin implements ISettings {
 	}
 
 	private function isWebUpdaterRecommended(): bool {
-		return $this->getUserCount() < 100;
-	}
-
-	/**
-	 * @see https://github.com/nextcloud/server/blob/39494fbf794d982f6f6551c984e6ca4c4e947d01/lib/private/Support/Subscription/Registry.php#L188-L216 implementation reference
-	 */
-	private function getUserCount(): int {
-		$userCount = 0;
-		$backends = $this->userManager->getBackends();
-		foreach ($backends as $backend) {
-			// TODO: change below to 'if ($backend instanceof ICountUsersBackend) {'
-			if ($backend->implementsActions(Backend::COUNT_USERS)) {
-				/** @var ICountUsersBackend $backend */
-				$backendUsers = $backend->countUsers();
-				if ($backendUsers !== false) {
-					$userCount += $backendUsers;
-				}
-			}
-		}
-
-		return $userCount;
+		return (int)$this->userManager->countUsersTotal(100) < 100;
 	}
 }

apps/user_ldap/lib/User_LDAP.php

@@ -17,12 +17,12 @@ use OCA\User_LDAP\User\User;
 use OCP\IUserBackend;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\User\Backend\ICountMappedUsersBackend;
-use OCP\User\Backend\ICountUsersBackend;
+use OCP\User\Backend\ILimitAwareCountUsersBackend;
 use OCP\User\Backend\IProvideEnabledStateBackend;
 use OCP\UserInterface;
 use Psr\Log\LoggerInterface;
 
-class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, IUserLDAP, ICountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend {
+class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, IUserLDAP, ILimitAwareCountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend {
 	public function __construct(
 		Access $access,
 		protected INotificationManager $notificationManager,
@@ -528,20 +528,18 @@ class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, I
 
 	/**
 	 * counts the users in LDAP
-	 *
-	 * @return int|false
 	 */
-	public function countUsers() {
+	public function countUsers(int $limit = 0): int|false {
 		if ($this->userPluginManager->implementsActions(Backend::COUNT_USERS)) {
 			return $this->userPluginManager->countUsers();
 		}
 
 		$filter = $this->access->getFilterForUserCount();
-		$cacheKey = 'countUsers-' . $filter;
+		$cacheKey = 'countUsers-' . $filter . '-' . $limit;
 		if (!is_null($entries = $this->access->connection->getFromCache($cacheKey))) {
 			return $entries;
 		}
-		$entries = $this->access->countUsers($filter);
+		$entries = $this->access->countUsers($filter, limit:$limit);
 		$this->access->connection->writeToCache($cacheKey, $entries);
 		return $entries;
 	}

apps/user_ldap/lib/User_Proxy.php

@@ -13,12 +13,12 @@ use OCA\User_LDAP\User\User;
 use OCP\IUserBackend;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\User\Backend\ICountMappedUsersBackend;
-use OCP\User\Backend\ICountUsersBackend;
+use OCP\User\Backend\ILimitAwareCountUsersBackend;
 use OCP\User\Backend\IProvideEnabledStateBackend;
 use OCP\UserInterface;
 use Psr\Log\LoggerInterface;
 
-class User_Proxy extends Proxy implements IUserBackend, UserInterface, IUserLDAP, ICountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend {
+class User_Proxy extends Proxy implements IUserBackend, UserInterface, IUserLDAP, ILimitAwareCountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend {
 	/** @var User_LDAP[] */
 	private array $backends = [];
 	private ?User_LDAP $refBackend = null;
@@ -350,17 +350,21 @@ class User_Proxy extends Proxy implements IUserBackend, UserInterface, IUserLDAP
 
 	/**
 	 * Count the number of users
-	 *
-	 * @return int|false
 	 */
-	public function countUsers() {
+	public function countUsers(int $limit = 0): int|false {
 		$this->setup();
 
 		$users = false;
 		foreach ($this->backends as $backend) {
-			$backendUsers = $backend->countUsers();
+			$backendUsers = $backend->countUsers($limit);
 			if ($backendUsers !== false) {
 				$users = (int)$users + $backendUsers;
+				if ($limit > 0) {
+					if ($users >= $limit) {
+						break;
+					}
+					$limit -= $users;
+				}
 			}
 		}
 		return $users;

lib/private/Support/Subscription/Registry.php

@@ -8,7 +8,6 @@ declare(strict_types=1);
  */
 namespace OC\Support\Subscription;
 
-use OC\User\Backend;
 use OCP\AppFramework\QueryException;
 use OCP\IConfig;
 use OCP\IGroupManager;
@@ -19,8 +18,6 @@ use OCP\Support\Subscription\Exception\AlreadyRegisteredException;
 use OCP\Support\Subscription\IRegistry;
 use OCP\Support\Subscription\ISubscription;
 use OCP\Support\Subscription\ISupportedApps;
-use OCP\User\Backend\ICountMappedUsersBackend;
-use OCP\User\Backend\ICountUsersBackend;
 use Psr\Log\LoggerInterface;
 
 class Registry implements IRegistry {
@@ -167,22 +164,8 @@ class Registry implements IRegistry {
 	}
 
 	private function getUserCount(): int {
-		$userCount = 0;
-		$backends = $this->userManager->getBackends();
-		foreach ($backends as $backend) {
-			if ($backend instanceof ICountMappedUsersBackend) {
-				$userCount += $backend->countMappedUsers();
-			} elseif ($backend->implementsActions(Backend::COUNT_USERS)) {
-				/** @var ICountUsersBackend $backend */
-				$backendUsers = $backend->countUsers();
-				if ($backendUsers !== false) {
-					$userCount += $backendUsers;
-				} else {
-					// TODO what if the user count can't be determined?
-					$this->logger->warning('Can not determine user count for ' . get_class($backend), ['app' => 'lib']);
-				}
-			}
-		}
+		/* We cannot limit because we substract disabled users afterward. But we limit to mapped users so should be not too expensive. */
+		$userCount = (int)$this->userManager->countUsersTotal(0, true);
 
 		$disabledUsers = $this->config->getUsersForUserValue('core', 'enabled', 'false');
 		$disabledUsersCount = count($disabledUsers);

lib/private/User/Manager.php

@@ -24,8 +24,10 @@ use OCP\L10N\IFactory;
 use OCP\Server;
 use OCP\Support\Subscription\IAssertion;
 use OCP\User\Backend\ICheckPasswordBackend;
+use OCP\User\Backend\ICountMappedUsersBackend;
 use OCP\User\Backend\ICountUsersBackend;
 use OCP\User\Backend\IGetRealUIDBackend;
+use OCP\User\Backend\ILimitAwareCountUsersBackend;
 use OCP\User\Backend\IProvideEnabledStateBackend;
 use OCP\User\Backend\ISearchKnownUsersBackend;
 use OCP\User\Events\BeforeUserCreatedEvent;
@@ -488,6 +490,35 @@ class Manager extends PublicEmitter implements IUserManager {
 		return $userCountStatistics;
 	}
 
+	public function countUsersTotal(int $limit = 0, bool $onlyMappedUsers = false): int|false {
+		$userCount = false;
+		foreach ($this->backends as $backend) {
+			if ($onlyMappedUsers && $backend instanceof ICountMappedUsersBackend) {
+				$backendUsers = $backend->countMappedUsers();
+			} elseif ($backend instanceof ILimitAwareCountUsersBackend) {
+				$backendUsers = $backend->countUsers($limit);
+			} elseif ($backend instanceof ICountUsersBackend || $backend->implementsActions(Backend::COUNT_USERS)) {
+				/** @var ICountUsersBackend $backend */
+				$backendUsers = $backend->countUsers();
+			} else {
+				$this->logger->debug('Skip backend for user count: ' . get_class($backend));
+				continue;
+			}
+			if ($backendUsers !== false) {
+				$userCount += $backendUsers;
+				if ($limit > 0) {
+					if ($userCount >= $limit) {
+						break;
+					}
+					$limit -= $userCount;
+				}
+			} else {
+				$this->logger->warning('Can not determine user count for ' . get_class($backend));
+			}
+		}
+		return $userCount;
+	}
+
 	/**
 	 * returns how many users per backend exist in the requested groups (if supported by backend)
 	 *

lib/public/IUserManager.php

@@ -163,6 +163,16 @@ interface IUserManager {
 	 */
 	public function countUsers();
 
+	/**
+	 * Get how many users exists in total, whithin limit
+	 *
+	 * @param int $limit Limit the count to avoid resource waste. 0 to disable
+	 * @param bool $onlyMappedUsers Count mapped users instead of all users for compatible backends
+	 *
+	 * @since 31.0.0
+	 */
+	public function countUsersTotal(int $limit = 0, bool $onlyMappedUsers = false): int|false;
+
 	/**
 	 * @param \Closure $callback
 	 * @psalm-param \Closure(\OCP\IUser):void $callback

lib/public/User/Backend/ICountUsersBackend.php

@@ -10,6 +10,7 @@ namespace OCP\User\Backend;
 
 /**
  * @since 14.0.0
+ * @deprecated 31.0.0 use and implement ILimitAwareCountUsersBackend instead.
  */
 interface ICountUsersBackend {
 	/**