Skip to content

Commit 1f95836

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

3 files changed

Lines changed: 19 additions & 25 deletions

File tree

build/psalm-baseline.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3873,7 +3873,7 @@
38733873
</file>
38743874
<file src="lib/private/Group/Manager.php">
38753875
<LessSpecificReturnStatement>
3876-
<code><![CDATA[$groups]]></code>
3876+
<code><![CDATA[$this->getGroupsObjects($groupIds)]]></code>
38773877
</LessSpecificReturnStatement>
38783878
<MoreSpecificReturnType>
38793879
<code><![CDATA[\OC\Group\Group[]]]></code>

lib/private/Group/Manager.php

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -295,18 +295,8 @@ public function getUserGroups(?IUser $user = null) {
295295
* @return \OC\Group\Group[]
296296
*/
297297
public function getUserIdGroups(string $uid): array {
298-
$groups = [];
299-
300-
foreach ($this->getUserIdGroupIds($uid) as $groupId) {
301-
$aGroup = $this->get($groupId);
302-
if ($aGroup instanceof IGroup) {
303-
$groups[$groupId] = $aGroup;
304-
} else {
305-
$this->logger->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', ['app' => 'core']);
306-
}
307-
}
308-
309-
return $groups;
298+
$groupIds = $this->getUserIdGroupIds($uid);
299+
return $this->getGroupsObjects($groupIds);
310300
}
311301

312302
/**

tests/lib/Group/ManagerTest.php

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -427,9 +427,9 @@ public function testGetUserGroups(): void {
427427
->with('user1')
428428
->willReturn(['group1']);
429429
$backend->expects($this->any())
430-
->method('groupExists')
430+
->method('getGroupDetails')
431431
->with('group1')
432-
->willReturn(true);
432+
->willReturn(['displayName' => 'group1']);
433433

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

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

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

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

873877
$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache, $this->remoteIpAddress);
874878
$manager->addBackend($backend);
@@ -903,9 +907,9 @@ public function testGetUserGroupsWithRemoveUser(): void {
903907
return $expectedGroups;
904908
});
905909
$backend->expects($this->any())
906-
->method('groupExists')
910+
->method('getGroupDetails')
907911
->with('group1')
908-
->willReturn(true);
912+
->willReturn(['displayName' => 'group1']);
909913
$backend->expects($this->once())
910914
->method('inGroup')
911915
->willReturn(true);

0 commit comments

Comments
 (0)