Add batch methods in user backends

This allows for faster group search with significantly less DB traffic

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
pull/32912/head
Carl Schwan 2 years ago committed by Côme Chilliet
parent e547247281
commit 3270b7f12e
No known key found for this signature in database
GPG Key ID: A3E2F658B28C760A

@ -48,11 +48,12 @@ use Exception;
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,6 +64,7 @@ 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
@ -70,7 +72,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
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)) {

@ -31,7 +31,9 @@ namespace OCA\User_LDAP;
use OC\ServerNotAvailableException;
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 {
private $backends = [];
@ -256,6 +258,21 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet
$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 +321,20 @@ class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGet
return $this->handleRequest($id, 'dn2GroupName', [$dn]);
}
/**
* {@inheritdoc}
*/
public function groupsExists(array $gids): array {
$existingGroups = [];
foreach ($gids as $gid) {
$exists = $this->handleRequest($gid, 'groupExists', [$gid]);
if ($exists) {
$existingGroups[$gid] = $gid;
}
}
return $existingGroups;
}
/**
* Check if backend implements actions
*

@ -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>

@ -62,11 +62,9 @@ class Database extends ABackend implements
ISetDisplayNameBackend,
ISearchableGroupBackend,
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 +268,7 @@ class Database extends ABackend implements
$this->fixDI();
$query = $this->dbConn->getQueryBuilder();
$query->select('gid')
$query->select('gid', 'displayname')
->from('groups')
->orderBy('gid', 'ASC');
@ -293,6 +291,10 @@ class Database extends ABackend implements
$groups = [];
while ($row = $result->fetch()) {
$this->groupCache[$row['gid']] = [
'displayname' => $row['displayname'],
'gid' => $row['gid'],
];
$groups[] = $row['gid'];
}
$result->closeCursor();
@ -331,6 +333,42 @@ class Database extends ABackend implements
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;
}
}
foreach (array_chunk($notFoundGids, 1000) as $chunk) {
$qb = $this->dbConn->getQueryBuilder();
$result = $qb->select('gid', 'displayname')
->from('groups')
->where($qb->expr()->in('gid', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_STR_ARRAY)))
->executeQuery();
while ($row = $result->fetch()) {
$this->groupCache[$row['gid']] = [
'displayname' => $row['displayname'],
'gid' => $row['gid'],
];
$existingGroups[] = $gid;
}
$result->closeCursor();
}
return $existingGroups;
}
/**
* Get a list of all users in a group
* @param string $gid
@ -488,6 +526,43 @@ class Database extends ABackend implements
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[$row['gid']] = ['displayName' => $row['displayname']];
$this->groupCache[$row['gid']] = [
'displayname' => $row['displayname'],
'gid' => $row['gid'],
];
}
$result->closeCursor();
}
return $details;
}
public function setDisplayName(string $gid, string $displayName): bool {
if (!$this->groupExists($gid)) {
return false;

@ -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,7 @@ namespace OC\Group;
use OC\Hooks\PublicEmitter;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Group\Backend\IGroupDetailsBackend;
use OCP\Group\Events\BeforeGroupCreatedEvent;
use OCP\Group\Events\GroupCreatedEvent;
use OCP\GroupInterface;
@ -74,10 +76,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 +187,7 @@ class Manager extends PublicEmitter implements IGroupManager {
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 +200,57 @@ class Manager extends PublicEmitter implements IGroupManager {
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 getGroupsObject(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 */
$groupDatas = $backend->getGroupsDetails($gids);
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 {
$existingGroups = $backend->groupsExists($gids);
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 +295,9 @@ class Manager extends PublicEmitter implements IGroupManager {
$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->getGroupsObject($groupIds);
foreach ($newGroups as $groupId => $group) {
$groups[$groupId] = $group;
}
if (!is_null($limit) and $limit <= 0) {
return array_values($groups);

@ -6,6 +6,7 @@ declare(strict_types=1);
* @copyright Copyright (c) 2018 Roeland Jago Douma <roeland@famdouma.nl>
*
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Carl Schwan <carl@carlschwan.eu>
*
* @license GNU AGPL version 3 or any later version
*
@ -65,4 +66,33 @@ abstract class ABackend implements GroupInterface {
return (bool)($actions & $implements);
}
/**
* @since 26.0.0
*/
public function groupsExists(array $gids): array {
$existingGroups = [];
foreach ($gids as $gid) {
$exists = $this->groupExists($gid);
if ($exists) {
$existingGroups[$gid] = $gid;
}
}
return $existingGroups;
}
/**
* @since 26.0.0
*/
public function getGroupsDetails(array $gids): array {
if (!($this instanceof IGroupDetailsBackend || $this->implementsActions(GroupInterface::GROUP_DETAILS))) {
throw new \Exception("Should not have been called");
}
/** @var IGroupDetailsBackend $this */
$groupData = [];
foreach ($gids as $gid) {
$groupData[$gid] = $this->getGroupDetails($gid);
}
return $groupData;
}
}

@ -6,6 +6,7 @@ declare(strict_types=1);
* @copyright Copyright (c) 2018 Roeland Jago Douma <roeland@famdouma.nl>
*
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Carl Schwan <carl@carlschwan.eu>
*
* @license GNU AGPL version 3 or any later version
*
@ -26,11 +27,33 @@ declare(strict_types=1);
namespace OCP\Group\Backend;
/**
* @brief Optional interface for group backends
* @since 14.0.0
*/
interface IGroupDetailsBackend {
/**
* @brief Get additional details for a group, for example the display name.
*
* The array returned can be empty when no additional information is available
* for the group.
*
* @return array{displayName?: string}
* @since 14.0.0
*/
public function getGroupDetails(string $gid): array;
/**
* @brief Batch method to get the group details of a list of groups
*
* The default implementation in ABackend will just call getGroupDetail in
* a loop. But a GroupBackend implementation should provides a more optimized
* override this method to provide a more optimized way to execute this operation.
*
* @throw \RuntimeException if called on a backend that doesn't implements IGroupDetailsBackend
*
* @return array<string, array{displayName: string}>
* @since 26.0.0
*/
public function getGroupsDetails(array $gids): array;
}

@ -86,7 +86,8 @@ interface GroupInterface {
public function getUserGroups($uid);
/**
* get a list of all groups
* @brief Get a list of all groups
*
* @param string $search
* @param int $limit
* @param int $offset
@ -98,13 +99,27 @@ interface GroupInterface {
public function getGroups(string $search = '', int $limit = -1, int $offset = 0);
/**
* check if a group exists
* @brief Check if a group exists
*
* @param string $gid
* @return bool
* @since 4.5.0
*/
public function groupExists($gid);
/**
* @brief Batch method to check if a list of groups exists
*
* The default implementation in ABackend will just call groupExists in
* a loop. But a GroupBackend implementation should provides a more optimized
* override this method to provide a more optimized way to execute this operation.
*
* @param list<string> $gids
* @return list<string> the list of group that exists
* @since 25.0.0
*/
public function groupsExists(array $gids): array;
/**
* @brief Get a list of user ids in a group matching the given search parameters.
*

@ -92,6 +92,7 @@ class ManagerTest extends TestCase {
'inGroup',
'getGroups',
'groupExists',
'groupsExists',
'usersInGroup',
'createGroup',
'addToGroup',
@ -361,10 +362,12 @@ class ManagerTest extends TestCase {
->method('getGroups')
->with('1')
->willReturn(['group1']);
$backend->expects($this->never())
->method('groupExists');
$backend->expects($this->once())
->method('groupExists')
->with('group1')
->willReturn(false);
->method('getGroupsDetails')
->with(['group1'])
->willReturn([]);
/** @var \OC\User\Manager $userManager */
$userManager = $this->createMock(Manager::class);

Loading…
Cancel
Save