Skip to content

Commit 14823d6

Browse files
CarlSchwanCarlSchwan
committed
perf: Introduce LazyGroup
Similar to LazyUser, this allow to get a IGroup object where properties are lazyly evaluated only when needed. This also reuse the DisplayNameCache from the IGroupManager. Signed-off-by: Carl Schwan <carlschwan@kde.org>
1 parent fe19ec8 commit 14823d6

7 files changed

Lines changed: 157 additions & 39 deletions

File tree

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1880,6 +1880,7 @@
18801880
'OC\\Group\\Database' => $baseDir . '/lib/private/Group/Database.php',
18811881
'OC\\Group\\DisplayNameCache' => $baseDir . '/lib/private/Group/DisplayNameCache.php',
18821882
'OC\\Group\\Group' => $baseDir . '/lib/private/Group/Group.php',
1883+
'OC\\Group\\LazyGroup' => $baseDir . '/lib/private/Group/LazyGroup.php',
18831884
'OC\\Group\\Manager' => $baseDir . '/lib/private/Group/Manager.php',
18841885
'OC\\Group\\MetaData' => $baseDir . '/lib/private/Group/MetaData.php',
18851886
'OC\\HintException' => $baseDir . '/lib/private/HintException.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1921,6 +1921,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
19211921
'OC\\Group\\Database' => __DIR__ . '/../../..' . '/lib/private/Group/Database.php',
19221922
'OC\\Group\\DisplayNameCache' => __DIR__ . '/../../..' . '/lib/private/Group/DisplayNameCache.php',
19231923
'OC\\Group\\Group' => __DIR__ . '/../../..' . '/lib/private/Group/Group.php',
1924+
'OC\\Group\\LazyGroup' => __DIR__ . '/../../..' . '/lib/private/Group/LazyGroup.php',
19241925
'OC\\Group\\Manager' => __DIR__ . '/../../..' . '/lib/private/Group/Manager.php',
19251926
'OC\\Group\\MetaData' => __DIR__ . '/../../..' . '/lib/private/Group/MetaData.php',
19261927
'OC\\HintException' => __DIR__ . '/../../..' . '/lib/private/HintException.php',

‎lib/private/Group/Group.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class Group implements IGroup {
4040
/** @var User[] */
4141
private array $users = [];
4242
private bool $usersLoaded = false;
43+
private bool $isDeleted = false;
4344

4445
public function __construct(
4546
private string $gid,
@@ -336,6 +337,7 @@ public function delete(): bool {
336337
$this->emitter->emit('\OC\Group', 'postDelete', [$this]);
337338
}
338339
}
340+
$this->isDeleted = $result;
339341
return $result;
340342
}
341343

@@ -393,4 +395,8 @@ public function hideFromCollaboration(): bool {
393395
return $hide || ($backend instanceof IHideFromCollaborationBackend && $backend->hideGroup($this->gid));
394396
}, false);
395397
}
398+
399+
public function isDeleted(): bool {
400+
return $this->isDeleted;
401+
}
396402
}

‎lib/private/Group/LazyGroup.php‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
6+
* SPDX-License-Identifier: AGPL-3.0-only
7+
*/
8+
9+
namespace OC\Group;
10+
11+
use OCP\EventDispatcher\IEventDispatcher;
12+
use OCP\IGroup;
13+
use OCP\IGroupManager;
14+
use OCP\IUser;
15+
use OCP\IUserManager;
16+
use OCP\Server;
17+
use Psr\Log\LoggerInterface;
18+
19+
class LazyGroup implements IGroup {
20+
private ?IGroup $group = null;
21+
private ?bool $isDeleted = null;
22+
23+
public function __construct(
24+
private string $gid,
25+
private IGroupManager $groupManager,
26+
) {
27+
}
28+
29+
#[\Override]
30+
public function getGID(): string {
31+
return $this->gid;
32+
}
33+
34+
private function getGroup(): IGroup {
35+
if ($this->group === null) {
36+
$this->group = $this->groupManager->get($this->gid);
37+
;
38+
}
39+
if ($this->group === null) {
40+
Server::get(LoggerInterface::class)->debug('Trying to use the deleted group: "' . $this->gid . '"', ['app' => 'core']);
41+
$this->isDeleted = true;
42+
$this->group = new Group($this->gid, [], Server::get(IEventDispatcher::class), Server::get(IUserManager::class));
43+
} else {
44+
$this->isDeleted = false;
45+
}
46+
return $this->group;
47+
}
48+
49+
#[\Override]
50+
public function getDisplayName(): string {
51+
// Use display name cache from IGroupManager
52+
return $this->groupManager->getDisplayName($this->gid);
53+
}
54+
55+
#[\Override]
56+
public function setDisplayName(string $displayName): bool {
57+
return $this->group->setDisplayName($displayName);
58+
}
59+
60+
#[\Override]
61+
public function getUsers(): array {
62+
return $this->getGroup()->getUsers();
63+
}
64+
65+
#[\Override]
66+
public function inGroup(IUser $user): bool {
67+
return $this->getGroup()->inGroup($user);
68+
}
69+
70+
#[\Override]
71+
public function addUser(IUser $user): void {
72+
$this->getGroup()->addUser($user);
73+
}
74+
75+
#[\Override]
76+
public function removeUser(IUser $user): void {
77+
$this->getGroup()->removeUser($user);
78+
}
79+
80+
#[\Override]
81+
public function searchUsers(string $search, ?int $limit = null, ?int $offset = null): array {
82+
return $this->getGroup()->searchUsers($search, $limit, $offset);
83+
}
84+
85+
#[\Override]
86+
public function count($search = ''): int|bool {
87+
return $this->getGroup()->count($search);
88+
}
89+
90+
#[\Override]
91+
public function countDisabled(): int|bool {
92+
return $this->getGroup()->countDisabled();
93+
}
94+
95+
#[\Override]
96+
public function searchDisplayName(string $search, ?int $limit = null, ?int $offset = null): array {
97+
return $this->getGroup()->searchDisplayName($search, $limit, $offset);
98+
}
99+
100+
#[\Override]
101+
public function getBackendNames(): array {
102+
return $this->getGroup()->getBackendNames();
103+
}
104+
105+
#[\Override]
106+
public function delete(): bool {
107+
return $this->getGroup()->delete();
108+
}
109+
110+
#[\Override]
111+
public function canRemoveUser(): bool {
112+
return $this->getGroup()->canRemoveUser();
113+
}
114+
115+
#[\Override]
116+
public function canAddUser(): bool {
117+
return $this->getGroup()->canAddUser();
118+
}
119+
120+
#[\Override]
121+
public function hideFromCollaboration(): bool {
122+
return $this->getGroup()->hideFromCollaboration();
123+
}
124+
125+
#[\Override]
126+
public function isDeleted(): bool {
127+
if ($this->isDeleted === null) {
128+
$this->getGroup();
129+
}
130+
return $this->isDeleted === true ? true : $this->getGroup()->isDeleted();
131+
}
132+
}

‎lib/private/Group/Manager.php‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -313,12 +313,7 @@ public function getUserIdGroups(string $uid): array {
313313
$groups = [];
314314

315315
foreach ($this->getUserIdGroupIds($uid) as $groupId) {
316-
$aGroup = $this->get($groupId);
317-
if ($aGroup instanceof IGroup) {
318-
$groups[$groupId] = $aGroup;
319-
} else {
320-
$this->logger->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', ['app' => 'core']);
321-
}
316+
$groups[$groupId] = new LazyGroup($groupId, $this);
322317
}
323318

324319
return $groups;
@@ -401,18 +396,6 @@ public function getDisplayName(string $groupId): ?string {
401396
return $this->displayNameCache->getDisplayName($groupId);
402397
}
403398

404-
/**
405-
* get an array of groupid and displayName for a user
406-
*
407-
* @param IUser $user
408-
* @return array ['displayName' => displayname]
409-
*/
410-
public function getUserGroupNames(IUser $user) {
411-
return array_map(function ($group) {
412-
return ['displayName' => $this->displayNameCache->getDisplayName($group->getGID())];
413-
}, $this->getUserGroups($user));
414-
}
415-
416399
#[\Override]
417400
public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) {
418401
$group = $this->get($gid);

‎lib/public/IGroup.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,11 @@ public function canAddUser(): bool;
148148
* @since 16.0.0
149149
*/
150150
public function hideFromCollaboration(): bool;
151+
152+
/**
153+
* Return whether the group is deleted.
154+
*
155+
* @since 35.0.0
156+
*/
157+
public function isDeleted(): bool;
151158
}

‎tests/lib/Group/ManagerTest.php‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,11 @@ abstract class TestBackend extends ABackend implements ISearchableGroupBackend,
3131
}
3232

3333
class ManagerTest extends TestCase {
34-
/** @var Manager|MockObject */
35-
protected $userManager;
36-
/** @var IEventDispatcher|MockObject */
37-
protected $dispatcher;
38-
/** @var LoggerInterface|MockObject */
39-
protected $logger;
40-
/** @var ICacheFactory|MockObject */
41-
private $cache;
42-
/** @var IRemoteAddress|MockObject */
43-
private $remoteIpAddress;
34+
protected Manager&MockObject $userManager;
35+
protected IEventDispatcher&MockObject $dispatcher;
36+
protected LoggerInterface&MockObject $logger;
37+
private ICacheFactory&MockObject $cache;
38+
private IRemoteAddress&MockObject $remoteIpAddress;
4439

4540
#[\Override]
4641
protected function setUp(): void {
@@ -55,7 +50,7 @@ protected function setUp(): void {
5550
$this->remoteIpAddress->method('allowsAdminActions')->willReturn(true);
5651
}
5752

58-
private function getTestUser($userId) {
53+
private function getTestUser(string $userId): IUser&MockObject {
5954
$mockUser = $this->createMock(IUser::class);
6055
$mockUser->expects($this->any())
6156
->method('getUID')
@@ -68,9 +63,8 @@ private function getTestUser($userId) {
6863

6964
/**
7065
* @param null|int $implementedActions
71-
* @return \PHPUnit\Framework\MockObject\MockObject
7266
*/
73-
private function getTestBackend($implementedActions = null) {
67+
private function getTestBackend(?int $implementedActions = null): MockObject&TestBackend {
7468
if ($implementedActions === null) {
7569
$implementedActions
7670
= GroupInterface::ADD_TO_GROUP
@@ -467,9 +461,6 @@ public function testGetUserGroupIds(): void {
467461
}
468462

469463
public function testGetUserGroupsWithDeletedGroup(): void {
470-
/**
471-
* @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend
472-
*/
473464
$backend = $this->createMock(Database::class);
474465
$backend->expects($this->once())
475466
->method('getUserGroups')
@@ -483,20 +474,17 @@ public function testGetUserGroupsWithDeletedGroup(): void {
483474
$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
484475
$manager->addBackend($backend);
485476

486-
/** @var User|\PHPUnit\Framework\MockObject\MockObject $user */
487477
$user = $this->createMock(IUser::class);
488478
$user->expects($this->atLeastOnce())
489479
->method('getUID')
490480
->willReturn('user1');
491481

492482
$groups = $manager->getUserGroups($user);
493-
$this->assertEmpty($groups);
483+
$this->assertCount(1, $groups);
484+
$this->assertTrue($groups['group1']->isDeleted());
494485
}
495486

496487
public function testInGroup(): void {
497-
/**
498-
* @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend
499-
*/
500488
$backend = $this->getTestBackend();
501489
$backend->expects($this->once())
502490
->method('getUserGroups')

0 commit comments

Comments
 (0)