Skip to content

Commit 700517a

Browse files
committed
feat: Fetch groups in batch in getUserGroups
Signed-off-by: Carl Schwan <carlschwan@kde.org>
1 parent 60f4e84 commit 700517a

2 files changed

Lines changed: 18 additions & 22 deletions

File tree

lib/private/Group/Manager.php

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -312,16 +312,8 @@ public function getUserGroups(?IUser $user = null): array {
312312
public function getUserIdGroups(string $uid): array {
313313
$groups = [];
314314

315-
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-
}
322-
}
323-
324-
return $groups;
315+
$groupIds = $this->getUserIdGroupIds($uid);
316+
return $this->getGroupsObjects($groupIds);
325317
}
326318

327319
/**

tests/lib/Group/ManagerTest.php

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -428,9 +428,9 @@ public function testGetUserGroups(): void {
428428
->with('user1')
429429
->willReturn(['group1']);
430430
$backend->expects($this->any())
431-
->method('groupExists')
431+
->method('getGroupDetails')
432432
->with('group1')
433-
->willReturn(true);
433+
->willReturn(['displayName' => 'group1']);
434434

435435
$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
436436
$manager->addBackend($backend);
@@ -476,9 +476,9 @@ public function testGetUserGroupsWithDeletedGroup(): void {
476476
->with('user1')
477477
->willReturn(['group1']);
478478
$backend->expects($this->any())
479-
->method('groupExists')
480-
->with('group1')
481-
->willReturn(false);
479+
->method('getGroupsDetails')
480+
->with(['group1'])
481+
->willReturn(['group1' => []]);
482482

483483
$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
484484
$manager->addBackend($backend);
@@ -560,8 +560,8 @@ public function testGetUserGroupsMultipleBackends(): void {
560560
->with('user1')
561561
->willReturn(['group1']);
562562
$backend1->expects($this->any())
563-
->method('groupExists')
564-
->willReturn(true);
563+
->method('getGroupDetails')
564+
->willReturnCallback(fn ($gid) => $gid === 'group1' ? ['displayName' => 'group1'] : []);
565565

566566
/**
567567
* @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend2
@@ -571,9 +571,9 @@ public function testGetUserGroupsMultipleBackends(): void {
571571
->method('getUserGroups')
572572
->with('user1')
573573
->willReturn(['group1', 'group2']);
574-
$backend1->expects($this->any())
575-
->method('groupExists')
576-
->willReturn(true);
574+
$backend2->expects($this->any())
575+
->method('getGroupDetails')
576+
->willReturnCallback(fn ($gid) => ['displayName' => $gid]);
577577

578578
$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
579579
$manager->addBackend($backend1);
@@ -870,6 +870,10 @@ public function testGetUserGroupsWithAddUser(): void {
870870
->method('groupExists')
871871
->with('group1')
872872
->willReturn(true);
873+
$backend->expects($this->any())
874+
->method('getGroupDetails')
875+
->with('group1')
876+
->willReturn(['displayName' => 'group1']);
873877

874878
$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
875879
$manager->addBackend($backend);
@@ -904,9 +908,9 @@ public function testGetUserGroupsWithRemoveUser(): void {
904908
return $expectedGroups;
905909
});
906910
$backend->expects($this->any())
907-
->method('groupExists')
911+
->method('getGroupDetails')
908912
->with('group1')
909-
->willReturn(true);
913+
->willReturn(['displayName' => 'group1']);
910914
$backend->expects($this->once())
911915
->method('inGroup')
912916
->willReturn(true);

0 commit comments

Comments
 (0)