Merge pull request #32912 from nextcloud/group-backend-batch-method

Add batch methods in user backends

apps/user_ldap/lib/Group_LDAP.php

@@ -48,11 +48,12 @@
 use OC\ServerNotAvailableException;
 use OCP\Cache\CappedMemoryCache;
 use OCP\GroupInterface;
+use OCP\Group\Backend\ABackend;
 use OCP\Group\Backend\IDeleteGroupBackend;
 use OCP\Group\Backend\IGetDisplayNameBackend;
 use Psr\Log\LoggerInterface;
 
-class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
+class Group_LDAP extends ABackend implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
 	protected bool $enabled = false;
 
 	/** @var CappedMemoryCache<string[]> $cachedGroupMembers array of users with gid as key */
@@ -63,14 +64,15 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
 	protected CappedMemoryCache $cachedNestedGroups;
 	protected GroupPluginManager $groupPluginManager;
 	protected LoggerInterface $logger;
+	protected Access $access;
 
 	/**
 	 * @var string $ldapGroupMemberAssocAttr contains the LDAP setting (in lower case) with the same name
 	 */
 	protected string $ldapGroupMemberAssocAttr;
 
 	public function __construct(Access $access, GroupPluginManager $groupPluginManager) {
-		parent::__construct($access);
+		$this->access = $access;
 		$filter = $this->access->connection->ldapGroupFilter;
 		$gAssoc = $this->access->connection->ldapGroupMemberAssocAttr;
 		if (!empty($filter) && !empty($gAssoc)) {

apps/user_ldap/lib/Group_Proxy.php

@@ -29,11 +29,14 @@
 namespace OCA\User_LDAP;
 
 use OC\ServerNotAvailableException;
+use OCP\Group\Backend\IBatchMethodsBackend;
 use OCP\Group\Backend\IDeleteGroupBackend;
 use OCP\Group\Backend\IGetDisplayNameBackend;
+use OCP\Group\Backend\IGroupDetailsBackend;
 use OCP\Group\Backend\INamedBackend;
+use OCP\GroupInterface;
 
-class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend {
+class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend, IBatchMethodsBackend {
 	private $backends = [];
 	private ?Group_LDAP $refBackend = null;
 	private Helper $helper;
@@ -256,6 +259,21 @@ public function getGroupDetails($gid) {
 			$gid, 'getGroupDetails', [$gid]);
 	}
 
+	/**
+	 * {@inheritdoc}
+	 */
+	public function getGroupsDetails(array $gids): array {
+		if (!($this instanceof IGroupDetailsBackend || $this->implementsActions(GroupInterface::GROUP_DETAILS))) {
+			throw new \Exception("Should not have been called");
+		}
+
+		$groupData = [];
+		foreach ($gids as $gid) {
+			$groupData[$gid] = $this->handleRequest($gid, 'getGroupDetails', [$gid]);
+		}
+		return $groupData;
+	}
+
 	/**
 	 * get a list of all groups
 	 *
@@ -304,6 +322,16 @@ public function dn2GroupName(string $dn): string|false {
 		return $this->handleRequest($id, 'dn2GroupName', [$dn]);
 	}
 
+	/**
+	 * {@inheritdoc}
+	 */
+	public function groupsExists(array $gids): array {
+		return array_values(array_filter(
+			$gids,
+			fn (string $gid): bool => $this->handleRequest($gid, 'groupExists', [$gid]),
+		));
+	}
+
 	/**
 	 * Check if backend implements actions
 	 *

apps/user_ldap/lib/Proxy.php

@@ -130,7 +130,7 @@ protected function isSingleBackend(): bool {
 	 * @param string $method string, the method of the user backend that shall be called
 	 * @param array $parameters an array of parameters to be passed
 	 * @param bool $passOnWhen
-	 * @return mixed, the result of the specified method
+	 * @return mixed the result of the specified method
 	 */
 	protected function handleRequest($id, $method, $parameters, $passOnWhen = false) {
 		if (!$this->isSingleBackend()) {

build/psalm-baseline.xml

@@ -2905,16 +2905,6 @@
       <code><![CDATA[is_null($this->getContent())]]></code>
     </TypeDoesNotContainNull>
   </file>
-  <file src="lib/private/Group/Database.php">
-    <InvalidArrayOffset>
-      <code><![CDATA[$this->groupCache[$gid]['displayname']]]></code>
-    </InvalidArrayOffset>
-    <InvalidPropertyAssignmentValue>
-      <code><![CDATA[$this->groupCache]]></code>
-      <code><![CDATA[$this->groupCache]]></code>
-      <code><![CDATA[$this->groupCache]]></code>
-    </InvalidPropertyAssignmentValue>
-  </file>
   <file src="lib/private/Group/DisplayNameCache.php">
     <MissingTemplateParam>
       <code>IEventListener</code>

lib/composer/composer/autoload_classmap.php

@@ -422,6 +422,7 @@
     'OCP\\GroupInterface' => $baseDir . '/lib/public/GroupInterface.php',
     'OCP\\Group\\Backend\\ABackend' => $baseDir . '/lib/public/Group/Backend/ABackend.php',
     'OCP\\Group\\Backend\\IAddToGroupBackend' => $baseDir . '/lib/public/Group/Backend/IAddToGroupBackend.php',
+    'OCP\\Group\\Backend\\IBatchMethodsBackend' => $baseDir . '/lib/public/Group/Backend/IBatchMethodsBackend.php',
     'OCP\\Group\\Backend\\ICountDisabledInGroup' => $baseDir . '/lib/public/Group/Backend/ICountDisabledInGroup.php',
     'OCP\\Group\\Backend\\ICountUsersBackend' => $baseDir . '/lib/public/Group/Backend/ICountUsersBackend.php',
     'OCP\\Group\\Backend\\ICreateGroupBackend' => $baseDir . '/lib/public/Group/Backend/ICreateGroupBackend.php',

lib/composer/composer/autoload_static.php

@@ -455,6 +455,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OCP\\GroupInterface' => __DIR__ . '/../../..' . '/lib/public/GroupInterface.php',
         'OCP\\Group\\Backend\\ABackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/ABackend.php',
         'OCP\\Group\\Backend\\IAddToGroupBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/IAddToGroupBackend.php',
+        'OCP\\Group\\Backend\\IBatchMethodsBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/IBatchMethodsBackend.php',
         'OCP\\Group\\Backend\\ICountDisabledInGroup' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/ICountDisabledInGroup.php',
         'OCP\\Group\\Backend\\ICountUsersBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/ICountUsersBackend.php',
         'OCP\\Group\\Backend\\ICreateGroupBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/ICreateGroupBackend.php',

lib/private/Group/Database.php

@@ -33,6 +33,7 @@
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Group\Backend\ABackend;
 use OCP\Group\Backend\IAddToGroupBackend;
+use OCP\Group\Backend\IBatchMethodsBackend;
 use OCP\Group\Backend\ICountDisabledInGroup;
 use OCP\Group\Backend\ICountUsersBackend;
 use OCP\Group\Backend\ICreateGroupBackend;
@@ -61,12 +62,11 @@ class Database extends ABackend implements
 	IRemoveFromGroupBackend,
 	ISetDisplayNameBackend,
 	ISearchableGroupBackend,
+	IBatchMethodsBackend,
 	INamedBackend {
-	/** @var string[] */
+	/** @var array<string, array{gid: string, displayname: string}> */
 	private $groupCache = [];
-
-	/** @var IDBConnection */
-	private $dbConn;
+	private ?IDBConnection $dbConn;
 
 	/**
 	 * \OC\Group\Database constructor.
@@ -270,7 +270,7 @@ public function getGroups(string $search = '', int $limit = -1, int $offset = 0)
 		$this->fixDI();
 
 		$query = $this->dbConn->getQueryBuilder();
-		$query->select('gid')
+		$query->select('gid', 'displayname')
 			->from('groups')
 			->orderBy('gid', 'ASC');
 
@@ -293,6 +293,10 @@ public function getGroups(string $search = '', int $limit = -1, int $offset = 0)
 
 		$groups = [];
 		while ($row = $result->fetch()) {
+			$this->groupCache[$row['gid']] = [
+				'displayname' => $row['displayname'],
+				'gid' => $row['gid'],
+			];
 			$groups[] = $row['gid'];
 		}
 		$result->closeCursor();
@@ -331,6 +335,43 @@ public function groupExists($gid) {
 		return false;
 	}
 
+	/**
+	 * {@inheritdoc}
+	 */
+	public function groupsExists(array $gids): array {
+		$notFoundGids = [];
+		$existingGroups = [];
+
+		// In case the data is already locally accessible, not need to do SQL query
+		// or do a SQL query but with a smaller in clause
+		foreach ($gids as $gid) {
+			if (isset($this->groupCache[$gid])) {
+				$existingGroups[] = $gid;
+			} else {
+				$notFoundGids[] = $gid;
+			}
+		}
+
+		$qb = $this->dbConn->getQueryBuilder();
+		$qb->select('gid', 'displayname')
+				->from('groups')
+				->where($qb->expr()->in('gid', $qb->createParameter('ids')));
+		foreach (array_chunk($notFoundGids, 1000) as $chunk) {
+			$qb->setParameter('ids', $chunk, IQueryBuilder::PARAM_STR_ARRAY);
+			$result = $qb->executeQuery();
+			while ($row = $result->fetch()) {
+				$this->groupCache[(string)$row['gid']] = [
+					'displayname' => (string)$row['displayname'],
+					'gid' => (string)$row['gid'],
+				];
+				$existingGroups[] = (string)$row['gid'];
+			}
+			$result->closeCursor();
+		}
+
+		return $existingGroups;
+	}
+
 	/**
 	 * Get a list of all users in a group
 	 * @param string $gid
@@ -488,6 +529,43 @@ public function getGroupDetails(string $gid): array {
 		return [];
 	}
 
+	/**
+	 * {@inheritdoc}
+	 */
+	public function getGroupsDetails(array $gids): array {
+		$notFoundGids = [];
+		$details = [];
+
+		// In case the data is already locally accessible, not need to do SQL query
+		// or do a SQL query but with a smaller in clause
+		foreach ($gids as $gid) {
+			if (isset($this->groupCache[$gid])) {
+				$details[$gid] = ['displayName' => $this->groupCache[$gid]['displayname']];
+			} else {
+				$notFoundGids[] = $gid;
+			}
+		}
+
+		foreach (array_chunk($notFoundGids, 1000) as $chunk) {
+			$query = $this->dbConn->getQueryBuilder();
+			$query->select('gid', 'displayname')
+				->from('groups')
+				->where($query->expr()->in('gid', $query->createNamedParameter($chunk, IQueryBuilder::PARAM_STR_ARRAY)));
+
+			$result = $query->executeQuery();
+			while ($row = $result->fetch()) {
+				$details[(string)$row['gid']] = ['displayName' => (string)$row['displayname']];
+				$this->groupCache[(string)$row['gid']] = [
+					'displayname' => (string)$row['displayname'],
+					'gid' => (string)$row['gid'],
+				];
+			}
+			$result->closeCursor();
+		}
+
+		return $details;
+	}
+
 	public function setDisplayName(string $gid, string $displayName): bool {
 		if (!$this->groupExists($gid)) {
 			return false;

lib/private/Group/Manager.php

@@ -21,6 +21,7 @@
  * @author Vincent Petry <vincent@nextcloud.com>
  * @author Vinicius Cubas Brand <vinicius@eita.org.br>
  * @author voxsim "Simon Vocella"
+ * @author Carl Schwan <carl@carlschwan.eu>
  *
  * @license AGPL-3.0
  *
@@ -41,6 +42,8 @@
 
 use OC\Hooks\PublicEmitter;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Group\Backend\IBatchMethodsBackend;
+use OCP\Group\Backend\IGroupDetailsBackend;
 use OCP\Group\Events\BeforeGroupCreatedEvent;
 use OCP\Group\Events\GroupCreatedEvent;
 use OCP\GroupInterface;
@@ -74,10 +77,10 @@ class Manager extends PublicEmitter implements IGroupManager {
 	private IEventDispatcher $dispatcher;
 	private LoggerInterface $logger;
 
-	/** @var \OC\Group\Group[] */
+	/** @var array<string, IGroup> */
 	private $cachedGroups = [];
 
-	/** @var (string[])[] */
+	/** @var array<string, list<string>> */
 	private $cachedUserGroups = [];
 
 	/** @var \OC\SubAdmin */
@@ -185,7 +188,7 @@ protected function getGroupObject($gid, $displayName = null) {
 			if ($backend->implementsActions(Backend::GROUP_DETAILS)) {
 				$groupData = $backend->getGroupDetails($gid);
 				if (is_array($groupData) && !empty($groupData)) {
-					// take the display name from the first backend that has a non-null one
+					// take the display name from the last backend that has a non-null one
 					if (is_null($displayName) && isset($groupData['displayName'])) {
 						$displayName = $groupData['displayName'];
 					}
@@ -198,10 +201,68 @@ protected function getGroupObject($gid, $displayName = null) {
 		if (count($backends) === 0) {
 			return null;
 		}
+		/** @var GroupInterface[] $backends */
 		$this->cachedGroups[$gid] = new Group($gid, $backends, $this->dispatcher, $this->userManager, $this, $displayName);
 		return $this->cachedGroups[$gid];
 	}
 
+	/**
+	 * @brief Batch method to create group objects
+	 *
+	 * @param list<string> $gids List of groupIds for which we want to create a IGroup object
+	 * @param array<string, string> $displayNames Array containing already know display name for a groupId
+	 * @return array<string, IGroup>
+	 */
+	protected function getGroupsObjects(array $gids, array $displayNames = []): array {
+		$backends = [];
+		$groups = [];
+		foreach ($gids as $gid) {
+			$backends[$gid] = [];
+			if (!isset($displayNames[$gid])) {
+				$displayNames[$gid] = null;
+			}
+		}
+		foreach ($this->backends as $backend) {
+			if ($backend instanceof IGroupDetailsBackend || $backend->implementsActions(GroupInterface::GROUP_DETAILS)) {
+				/** @var IGroupDetailsBackend $backend */
+				if ($backend instanceof IBatchMethodsBackend) {
+					$groupDatas = $backend->getGroupsDetails($gids);
+				} else {
+					$groupDatas = [];
+					foreach ($gids as $gid) {
+						$groupDatas[$gid] = $backend->getGroupDetails($gid);
+					}
+				}
+				foreach ($groupDatas as $gid => $groupData) {
+					if (!empty($groupData)) {
+						// take the display name from the last backend that has a non-null one
+						if (isset($groupData['displayName'])) {
+							$displayNames[$gid] = $groupData['displayName'];
+						}
+						$backends[$gid][] = $backend;
+					}
+				}
+			} else {
+				if ($backend instanceof IBatchMethodsBackend) {
+					$existingGroups = $backend->groupsExists($gids);
+				} else {
+					$existingGroups = array_filter($gids, fn (string $gid): bool => $backend->groupExists($gid));
+				}
+				foreach ($existingGroups as $group) {
+					$backends[$group][] = $backend;
+				}
+			}
+		}
+		foreach ($gids as $gid) {
+			if (count($backends[$gid]) === 0) {
+				continue;
+			}
+			$this->cachedGroups[$gid] = new Group($gid, $backends[$gid], $this->dispatcher, $this->userManager, $this, $displayNames[$gid]);
+			$groups[$gid] = $this->cachedGroups[$gid];
+		}
+		return $groups;
+	}
+
 	/**
 	 * @param string $gid
 	 * @return bool
@@ -246,13 +307,9 @@ public function search(string $search, ?int $limit = null, ?int $offset = 0) {
 		$groups = [];
 		foreach ($this->backends as $backend) {
 			$groupIds = $backend->getGroups($search, $limit ?? -1, $offset ?? 0);
-			foreach ($groupIds as $groupId) {
-				$aGroup = $this->get($groupId);
-				if ($aGroup instanceof IGroup) {
-					$groups[$groupId] = $aGroup;
-				} else {
-					$this->logger->debug('Group "' . $groupId . '" was returned by search but not found through direct access', ['app' => 'core']);
-				}
+			$newGroups = $this->getGroupsObjects($groupIds);
+			foreach ($newGroups as $groupId => $group) {
+				$groups[$groupId] = $group;
 			}
 			if (!is_null($limit) and $limit <= 0) {
 				return array_values($groups);