Skip to content

Commit e9ae315

Browse files
CarlSchwanCarlSchwan
committed
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>
1 parent 596ead7 commit e9ae315

8 files changed

Lines changed: 242 additions & 80 deletions

File tree

‎apps/user_ldap/lib/Group_LDAP.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,13 @@
4747
use OC;
4848
use OC\Cache\CappedMemoryCache;
4949
use OC\ServerNotAvailableException;
50+
use OCP\Group\Backend\ABackend;
5051
use OCP\Group\Backend\IGetDisplayNameBackend;
5152
use OCP\Group\Backend\IDeleteGroupBackend;
5253
use OCP\GroupInterface;
5354
use Psr\Log\LoggerInterface;
5455

55-
class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
56+
class Group_LDAP extends ABackend implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
5657
protected $enabled = false;
5758

5859
/** @var CappedMemoryCache<string[]> $cachedGroupMembers array of users with gid as key */
@@ -61,18 +62,17 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
6162
protected CappedMemoryCache $cachedGroupsByMember;
6263
/** @var CappedMemoryCache<string[]> $cachedNestedGroups array of groups with gid (DN) as key */
6364
protected CappedMemoryCache $cachedNestedGroups;
64-
/** @var GroupPluginManager */
65-
protected $groupPluginManager;
66-
/** @var LoggerInterface */
67-
protected $logger;
65+
protected GroupPluginManager $groupPluginManager;
66+
protected LoggerInterface $logger;
67+
protected Access $access;
6868

6969
/**
7070
* @var string $ldapGroupMemberAssocAttr contains the LDAP setting (in lower case) with the same name
7171
*/
7272
protected $ldapGroupMemberAssocAttr;
7373

7474
public function __construct(Access $access, GroupPluginManager $groupPluginManager) {
75-
parent::__construct($access);
75+
$this->access = $access;
7676
$filter = $this->access->connection->ldapGroupFilter;
7777
$gAssoc = $this->access->connection->ldapGroupMemberAssocAttr;
7878
if (!empty($filter) && !empty($gAssoc)) {
@@ -1203,7 +1203,7 @@ protected function filterValidGroups(array $listOfGroups): array {
12031203
* Returns the supported actions as int to be
12041204
* compared with GroupInterface::CREATE_GROUP etc.
12051205
*/
1206-
public function implementsActions($actions) {
1206+
public function implementsActions($actions): bool {
12071207
return (bool)((GroupInterface::COUNT_USERS |
12081208
GroupInterface::DELETE_GROUP |
12091209
$this->groupPluginManager->getImplementedActions()) & $actions);

‎apps/user_ldap/lib/Group_Proxy.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030

3131
use OCP\Group\Backend\IDeleteGroupBackend;
3232
use OCP\Group\Backend\IGetDisplayNameBackend;
33+
use OCP\Group\Backend\IGroupDetailsBackend;
3334
use OCP\Group\Backend\INamedBackend;
35+
use OCP\GroupInterface;
3436

3537
class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend {
3638
private $backends = [];
@@ -229,6 +231,21 @@ public function getGroupDetails($gid) {
229231
$gid, 'getGroupDetails', [$gid]);
230232
}
231233

234+
/**
235+
* {@inheritdoc}
236+
*/
237+
public function getGroupsDetails(array $gids): array {
238+
if (!($this instanceof IGroupDetailsBackend || $this->implementsActions(GroupInterface::GROUP_DETAILS))) {
239+
throw new \Exception("Should not have been called");
240+
}
241+
242+
$groupData = [];
243+
foreach ($gids as $gid) {
244+
$groupData[$gid] = $this->handleRequest($gid, 'getGroupDetails', [$gid]);
245+
}
246+
return $groupData;
247+
}
248+
232249
/**
233250
* get a list of all groups
234251
*
@@ -259,6 +276,20 @@ public function groupExists($gid) {
259276
return $this->handleRequest($gid, 'groupExists', [$gid]);
260277
}
261278

279+
/**
280+
* {@inheritdoc}
281+
*/
282+
public function groupsExists(array $gids): array {
283+
$existingGroups = [];
284+
foreach ($gids as $gid) {
285+
$exits = $this->handleRequest($gid, 'groupExists', [$gid]);
286+
if ($exits) {
287+
$existingGroups[] = $gid;
288+
}
289+
}
290+
return $existingGroups;
291+
}
292+
262293
/**
263294
* Check if backend implements actions
264295
*

‎build/psalm-baseline.xml‎

Lines changed: 2 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -136,16 +136,6 @@
136136
</ParamNameMismatch>
137137
</file>
138138
<file src="apps/dav/lib/CalDAV/CalDavBackend.php">
139-
<InvalidArgument occurrences="8">
140-
<code>'\OCA\DAV\CalDAV\CalDavBackend::createCachedCalendarObject'</code>
141-
<code>'\OCA\DAV\CalDAV\CalDavBackend::createSubscription'</code>
142-
<code>'\OCA\DAV\CalDAV\CalDavBackend::deleteCachedCalendarObject'</code>
143-
<code>'\OCA\DAV\CalDAV\CalDavBackend::deleteSubscription'</code>
144-
<code>'\OCA\DAV\CalDAV\CalDavBackend::publishCalendar'</code>
145-
<code>'\OCA\DAV\CalDAV\CalDavBackend::updateCachedCalendarObject'</code>
146-
<code>'\OCA\DAV\CalDAV\CalDavBackend::updateShares'</code>
147-
<code>'\OCA\DAV\CalDAV\CalDavBackend::updateSubscription'</code>
148-
</InvalidArgument>
149139
<InvalidNullableReturnType occurrences="2">
150140
<code>array</code>
151141
<code>array</code>
@@ -161,16 +151,6 @@
161151
<RedundantCast occurrences="1">
162152
<code>(int)$calendarId</code>
163153
</RedundantCast>
164-
<TooManyArguments occurrences="8">
165-
<code>dispatch</code>
166-
<code>dispatch</code>
167-
<code>dispatch</code>
168-
<code>dispatch</code>
169-
<code>dispatch</code>
170-
<code>dispatch</code>
171-
<code>dispatch</code>
172-
<code>dispatch</code>
173-
</TooManyArguments>
174154
<UndefinedFunction occurrences="4">
175155
<code>Uri\split($principalUri)</code>
176156
<code>Uri\split($row['principaluri'])</code>
@@ -341,9 +321,6 @@
341321
</InvalidArgument>
342322
</file>
343323
<file src="apps/dav/lib/CardDAV/AddressBookImpl.php">
344-
<InvalidArgument occurrences="1">
345-
<code>$id</code>
346-
</InvalidArgument>
347324
<InvalidScalarArgument occurrences="2">
348325
<code>$this-&gt;getKey()</code>
349326
<code>$this-&gt;getKey()</code>
@@ -358,16 +335,6 @@
358335
<FalsableReturnStatement occurrences="1">
359336
<code>false</code>
360337
</FalsableReturnStatement>
361-
<InvalidArgument occurrences="3">
362-
<code>'\OCA\DAV\CardDAV\CardDavBackend::createCard'</code>
363-
<code>'\OCA\DAV\CardDAV\CardDavBackend::deleteCard'</code>
364-
<code>'\OCA\DAV\CardDAV\CardDavBackend::updateCard'</code>
365-
</InvalidArgument>
366-
<TooManyArguments occurrences="3">
367-
<code>dispatch</code>
368-
<code>dispatch</code>
369-
<code>dispatch</code>
370-
</TooManyArguments>
371338
<TypeDoesNotContainType occurrences="1">
372339
<code>$addressBooks[$row['id']][$readOnlyPropertyName] === 0</code>
373340
</TypeDoesNotContainType>
@@ -889,7 +856,6 @@
889856
</RedundantCondition>
890857
<TypeDoesNotContainType occurrences="2">
891858
<code>get_class($res) === 'OpenSSLAsymmetricKey'</code>
892-
<code>is_object($res)</code>
893859
</TypeDoesNotContainType>
894860
</file>
895861
<file src="apps/encryption/lib/Crypto/EncryptAll.php">
@@ -1732,14 +1698,6 @@
17321698
<code>0</code>
17331699
</InvalidScalarArgument>
17341700
</file>
1735-
<file src="apps/updatenotification/lib/ResetTokenBackgroundJob.php">
1736-
<InvalidOperand occurrences="1">
1737-
<code>$this-&gt;config-&gt;getAppValue('core', 'updater.secret.created', $this-&gt;timeFactory-&gt;getTime())</code>
1738-
</InvalidOperand>
1739-
<InvalidScalarArgument occurrences="1">
1740-
<code>$this-&gt;timeFactory-&gt;getTime()</code>
1741-
</InvalidScalarArgument>
1742-
</file>
17431701
<file src="apps/updatenotification/lib/Settings/Admin.php">
17441702
<InvalidScalarArgument occurrences="1">
17451703
<code>$lastUpdateCheckTimestamp</code>
@@ -3366,16 +3324,13 @@
33663324
<code>$result</code>
33673325
<code>$this-&gt;copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file, false, $isRename)</code>
33683326
</InvalidOperand>
3369-
<InvalidReturnStatement occurrences="6">
3327+
<InvalidReturnStatement occurrences="4">
33703328
<code>$newUnencryptedSize</code>
33713329
<code>$result</code>
3372-
<code>$stat</code>
33733330
<code>$this-&gt;storage-&gt;file_get_contents($path)</code>
33743331
<code>$this-&gt;storage-&gt;filesize($path)</code>
3375-
<code>$this-&gt;storage-&gt;getLocalFile($path)</code>
33763332
</InvalidReturnStatement>
3377-
<InvalidReturnType occurrences="5">
3378-
<code>array</code>
3333+
<InvalidReturnType occurrences="3">
33793334
<code>bool</code>
33803335
<code>int</code>
33813336
<code>string</code>
@@ -3476,16 +3431,6 @@
34763431
<code>is_null($this-&gt;getContent())</code>
34773432
</TypeDoesNotContainNull>
34783433
</file>
3479-
<file src="lib/private/Group/Database.php">
3480-
<InvalidArrayOffset occurrences="1">
3481-
<code>$this-&gt;groupCache[$gid]['displayname']</code>
3482-
</InvalidArrayOffset>
3483-
<InvalidPropertyAssignmentValue occurrences="3">
3484-
<code>$this-&gt;groupCache</code>
3485-
<code>$this-&gt;groupCache</code>
3486-
<code>$this-&gt;groupCache</code>
3487-
</InvalidPropertyAssignmentValue>
3488-
</file>
34893434
<file src="lib/private/Group/Group.php">
34903435
<InvalidArgument occurrences="7">
34913436
<code>IGroup::class . '::postAddUser'</code>

‎lib/private/Group/Database.php‎

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,9 @@ class Database extends ABackend implements
5959
ISetDisplayNameBackend,
6060
INamedBackend {
6161

62-
/** @var string[] */
62+
/** @var array<string, array{gid: string, displayname: string}> */
6363
private $groupCache = [];
64-
65-
/** @var IDBConnection */
66-
private $dbConn;
64+
private ?IDBConnection $dbConn;
6765

6866
/**
6967
* \OC\Group\Database constructor.
@@ -267,7 +265,7 @@ public function getGroups($search = '', $limit = null, $offset = null) {
267265
$this->fixDI();
268266

269267
$query = $this->dbConn->getQueryBuilder();
270-
$query->select('gid')
268+
$query->select('gid', 'displayname')
271269
->from('groups')
272270
->orderBy('gid', 'ASC');
273271

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

287285
$groups = [];
288286
while ($row = $result->fetch()) {
287+
$this->groupCache[$row['gid']] = [
288+
'displayname' => $row['displayname'],
289+
'gid' => $row['gid'],
290+
];
289291
$groups[] = $row['gid'];
290292
}
291293
$result->closeCursor();
@@ -324,6 +326,43 @@ public function groupExists($gid) {
324326
return false;
325327
}
326328

329+
/**
330+
* {@inheritdoc}
331+
*/
332+
public function groupsExists(array $gids): array {
333+
$notFounsGids = [];
334+
$existingGroups = [];
335+
336+
// In case the data is already locally accessible, not need to do SQL query
337+
// or do a SQL query but with a smaller in clause
338+
foreach ($gids as $gid) {
339+
if (isset($this->groupCache[$gid])) {
340+
$existingGroups[] = $gid;
341+
} else {
342+
$notFounsGids[] = $gid;
343+
}
344+
}
345+
346+
foreach (array_chunk($gids, 1000) as $chunk) {
347+
$qb = $this->dbConn->getQueryBuilder();
348+
$result = $qb->select('gid', 'displayname')
349+
->from('groups')
350+
->where($qb->expr()->in('gid', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_STR_ARRAY)))
351+
->executeQuery();
352+
while ($row = $result->fetch()) {
353+
$details[$row['gid']] = $row['displayname'];
354+
$this->groupCache[$row['gid']] = [
355+
'displayname' => $row['displayname'],
356+
'gid' => $row['gid'],
357+
];
358+
$existingGroups[] = $gid;
359+
}
360+
$result->closeCursor();
361+
}
362+
363+
return $existingGroups;
364+
}
365+
327366
/**
328367
* get a list of all users in a group
329368
* @param string $gid
@@ -474,6 +513,43 @@ public function getGroupDetails(string $gid): array {
474513
return [];
475514
}
476515

516+
/**
517+
* {@inheritdoc}
518+
*/
519+
public function getGroupsDetails(array $gids): array {
520+
$notFounsGids = [];
521+
$details = [];
522+
523+
// In case the data is already locally accessible, not need to do SQL query
524+
// or do a SQL query but with a smaller in clause
525+
foreach ($gids as $gid) {
526+
if (isset($this->groupCache[$gid])) {
527+
$details[$gid] = $this->groupCache[$gid]['displayname'];
528+
} else {
529+
$notFounsGids[] = $gid;
530+
}
531+
}
532+
533+
foreach (array_chunk($gids, 1000) as $chunk) {
534+
$query = $this->dbConn->getQueryBuilder();
535+
$query->select('gid', 'displayname')
536+
->from('groups')
537+
->where($query->expr()->in('gid', $query->createNamedParameter($chunk, IQueryBuilder::PARAM_STR_ARRAY)));
538+
539+
$result = $query->executeQuery();
540+
while ($row = $result->fetch()) {
541+
$details[$row['gid']] = $row['displayname'];
542+
$this->groupCache[$row['gid']] = [
543+
'displayname' => $row['displayname'],
544+
'gid' => $row['gid'],
545+
];
546+
}
547+
$result->closeCursor();
548+
}
549+
550+
return $details;
551+
}
552+
477553
public function setDisplayName(string $gid, string $displayName): bool {
478554
if (!$this->groupExists($gid)) {
479555
return false;

0 commit comments

Comments
 (0)