Skip to content

Commit

Permalink
Add batch methods in user backends
Browse files Browse the repository at this point in the history
This allows for faster group search with significantly less DB traffic

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
  • Loading branch information
CarlSchwan committed Jun 17, 2022
1 parent 596ead7 commit 905f9f8
Show file tree
Hide file tree
Showing 7 changed files with 232 additions and 14 deletions.
5 changes: 3 additions & 2 deletions apps/user_ldap/lib/Group_LDAP.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@
use OC;
use OC\Cache\CappedMemoryCache;
use OC\ServerNotAvailableException;
use OCP\Group\Backend\ABackend;
use OCP\Group\Backend\IGetDisplayNameBackend;
use OCP\Group\Backend\IDeleteGroupBackend;
use OCP\GroupInterface;
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 $enabled = false;

/** @var CappedMemoryCache<string[]> $cachedGroupMembers array of users with gid as key */
Expand All @@ -72,7 +73,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
protected $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)) {
Expand Down
29 changes: 29 additions & 0 deletions apps/user_ldap/lib/Group_Proxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,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
*
Expand Down Expand Up @@ -259,6 +274,20 @@ public function groupExists($gid) {
return $this->handleRequest($gid, 'groupExists', [$gid]);
}

/**
* {@inheritdoc}
*/
public function groupsExists(array $gids): array {
$existingGroups = [];
foreach ($gids as $gid) {
$exits = $this->handleRequest($gid, 'groupExists', [$gid]);
if ($exits) {
$existingGroups[] = $gid;
}
}
return $existingGroups;
}

/**
* Check if backend implements actions
*
Expand Down
81 changes: 80 additions & 1 deletion lib/private/Group/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ public function getGroups($search = '', $limit = null, $offset = null) {
$this->fixDI();

$query = $this->dbConn->getQueryBuilder();
$query->select('gid')
$query->select('gid', 'displayname')
->from('groups')
->orderBy('gid', 'ASC');

Expand All @@ -286,6 +286,10 @@ public function getGroups($search = '', $limit = null, $offset = null) {

$groups = [];
while ($row = $result->fetch()) {
$this->groupCache[$row['gid']] = [
'displayname' => $row['displayname'],
'gid' => $row['gid'],
];
$groups[] = $row['gid'];
}
$result->closeCursor();
Expand Down Expand Up @@ -324,6 +328,44 @@ public function groupExists($gid) {
return false;
}

/**
* {@inheritdoc}
*/
public function groupsExists(array $gids): array {
$notFounsGids = []
$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 {
$notFounsGids[] = $gid;
}
}

foreach (array_chunk($gids, 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()) {
while ($row = $result->fetch()) {
$details[$row['gid']] = $row['displayname'];
$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
Expand Down Expand Up @@ -474,6 +516,43 @@ public function getGroupDetails(string $gid): array {
return [];
}

/**
* {@inheritdoc}
*/
public function getGroupsDetails(array $gids): array {
$notFounsGids = []
$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] = $this->groupCache[$gid]['displayname'];
} else {
$notFounsGids[] = $gid;
}
}

foreach (array_chunk($gids, 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']] = $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;
Expand Down
61 changes: 52 additions & 9 deletions lib/private/Group/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -73,10 +74,10 @@ class Manager extends PublicEmitter implements IGroupManager {
private $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 */
Expand Down Expand Up @@ -193,10 +194,56 @@ 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 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)) {
$groupDatas = $backend->getGroupsDetails($gids);
foreach ($groupDatas as $gid => $groupData) {
if (!empty($groupData)) {
// take the display name from the first 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
Expand Down Expand Up @@ -239,13 +286,9 @@ public function search($search, $limit = null, $offset = null) {
$groups = [];
foreach ($this->backends as $backend) {
$groupIds = $backend->getGroups($search, $limit, $offset);
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);
Expand Down
30 changes: 30 additions & 0 deletions lib/public/Group/Backend/ABackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* @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
*
Expand Down Expand Up @@ -65,4 +66,33 @@ public function implementsActions($actions): bool {

return (bool)($actions & $implements);
}

/**
* @since 25.0.0
*/
public function groupsExists(array $gids): array {
$existingGroups = [];
foreach ($gids as $gid) {
$exists = $this->groupExists($gid);
if ($exists) {
$existringGroups[] = $gid;
}
}
return $existingGroups;
}

/**
* @since 25.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;
}
}
8 changes: 8 additions & 0 deletions lib/public/Group/Backend/IGroupDetailsBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* @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
*
Expand All @@ -26,11 +27,18 @@
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;
Expand Down
32 changes: 30 additions & 2 deletions lib/public/GroupInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ public function inGroup($uid, $gid);
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
Expand All @@ -99,13 +100,40 @@ public function getUserGroups($uid);
public function getGroups($search = '', $limit = -1, $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 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
*
* @since 25.0.0
*/
public function getGroupsDetails(array $gids): array;

/**
* get a list of all users in a group
* @param string $gid
Expand Down

0 comments on commit 905f9f8

Please sign in to comment.