diff --git a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php index 465188fd9b9e4..6bae439688d65 100644 --- a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php +++ b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php @@ -105,11 +105,7 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar // Get groups data $userAccount = $this->accountManager->getAccount($targetUserObject); - $groups = $this->groupManager->getUserGroups($targetUserObject); - $gids = []; - foreach ($groups as $group) { - $gids[] = $group->getGID(); - } + $gids = $this->groupManager->getUserGroupIds($targetUserObject); if ($isAdmin || $isDelegatedAdmin) { try { @@ -278,10 +274,11 @@ protected function findGroupsWithDisplayname(array $userDetails): array { $groupIds = array_unique($groupIds); sort($groupIds); - return array_map(function ($groupId) { - $displayname = $this->groupDisplayNameCache->getDisplayName($groupId) ?? $groupId; - return ['id' => $groupId, 'displayname' => $displayname]; - }, $groupIds); + $info = []; + foreach ($this->groupDisplayNameCache->getDisplayNames($groupIds) as $groupId => $displayName) { + $info[] = ['id' => $groupId, 'displayname' => $displayName ?? $groupId]; + } + return $info; } /** diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index f86109bc4dc08..3f3b60af19460 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -1082,8 +1082,7 @@ public function editUserMultiField( } if ($groups !== null) { - $currentGroups = $this->groupManager->getUserGroups($targetUser); - $currentGroupIds = array_map(fn (IGroup $g) => $g->getGID(), $currentGroups); + $currentGroupIds = $this->groupManager->getUserGroupIds($targetUser); foreach (array_diff($currentGroupIds, $groups) as $gid) { $this->groupManager->get($gid)?->removeUser($targetUser); } diff --git a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php index 0b60ddab7c0db..d4c1e796262c8 100644 --- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php +++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php @@ -483,8 +483,8 @@ public function testGetGroupUsersDetails(): void { ->with($gid) ->willReturn($group); $this->groupManager->expects($this->any()) - ->method('getUserGroups') - ->willReturn([$group]); + ->method('getUserGroupIds') + ->willReturn(['ncg1']); /** @var MockObject */ $this->subAdminManager->expects($this->any()) @@ -495,9 +495,9 @@ public function testGetGroupUsersDetails(): void { ->willReturn([]); $this->groupDisplayNameCache - ->method('getDisplayName') - ->with('ncg1') - ->willReturn('Group One'); + ->method('getDisplayNames') + ->with(['ncg1']) + ->willReturn(['ncg1' => 'Group One']); $result = $this->api->getGroupUsersDetails($gid); @@ -538,8 +538,8 @@ public function testGetGroupUsersDetailsEncoded(): void { ->with($gid) ->willReturn($group); $this->groupManager->expects($this->any()) - ->method('getUserGroups') - ->willReturn([$group]); + ->method('getUserGroupIds') + ->willReturn(['Department A/B C/D']); /** @var MockObject */ $this->subAdminManager->expects($this->any()) @@ -550,9 +550,9 @@ public function testGetGroupUsersDetailsEncoded(): void { ->willReturn([]); $this->groupDisplayNameCache - ->method('getDisplayName') - ->with('Department A/B C/D') - ->willReturn('Department A/B C/D-name'); + ->method('getDisplayNames') + ->with(['Department A/B C/D']) + ->willReturn(['Department A/B C/D' => 'Department A/B C/D-name']); $result = $this->api->getGroupUsersDetails(urlencode($gid)); diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 1078f348f95af..cd0f7033896a4 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -1206,8 +1206,8 @@ public function testGetUserDataAsAdmin(): void { ->willReturn(true); $this->groupManager ->expects($this->any()) - ->method('getUserGroups') - ->willReturn([$group0, $group1, $group2]); + ->method('getUserGroupIds') + ->willReturn(['group0', 'group1', 'group2']); $this->groupManager ->expects($this->once()) ->method('getSubAdmin') @@ -1216,15 +1216,6 @@ public function testGetUserDataAsAdmin(): void { ->expects($this->once()) ->method('getSubAdminsGroups') ->willReturn([$group3]); - $group0->expects($this->exactly(1)) - ->method('getGID') - ->willReturn('group0'); - $group1->expects($this->exactly(1)) - ->method('getGID') - ->willReturn('group1'); - $group2->expects($this->exactly(1)) - ->method('getGID') - ->willReturn('group2'); $group3->expects($this->once()) ->method('getGID') ->willReturn('group3'); @@ -2778,7 +2769,7 @@ public function testUpdateUserGroupDiff(): void { $newGroup = $this->createMock(IGroup::class); $newGroup->method('getGID')->willReturn('newgroup'); - $this->groupManager->method('getUserGroups')->willReturn([$oldGroup]); + $this->groupManager->method('getUserGroupIds')->willReturn(['oldgroup']); $this->groupManager->method('groupExists')->willReturn(true); $this->groupManager->method('get')->willReturnMap([ ['newgroup', $newGroup], diff --git a/lib/private/Group/DisplayNameCache.php b/lib/private/Group/DisplayNameCache.php index cfd3989701520..b85244c8fc116 100644 --- a/lib/private/Group/DisplayNameCache.php +++ b/lib/private/Group/DisplayNameCache.php @@ -56,6 +56,45 @@ public function getDisplayName(string $groupId): ?string { return $displayName; } + /** + * @param list $groupIds + * @return array + */ + public function getDisplayNames(array $groupIds): array { + $result = []; + $missing = []; + foreach ($groupIds as $groupId) { + if (isset($this->cache[$groupId])) { + $result[$groupId] = $this->cache[$groupId]; + } else { + $displayName = $this->memCache->get($groupId); + if ($displayName) { + $this->cache[$groupId] = $displayName; + $result[$groupId] = $displayName; + } else { + $missing[] = $groupId; + } + } + } + + /** @var Manager $groupManager */ + $groupManager = $this->groupManager; + $groups = $groupManager->getGroupsObjects($missing); + $stillMissingGroups = array_diff($missing, array_keys($groups)); + foreach ($groups as $groupId => $group) { + $displayName = $group->getDisplayName(); + $this->cache[$groupId] = $displayName; + $this->memCache->set($groupId, $displayName, 60 * 10); // 10 minutes + $result[$groupId] = $displayName; + } + + foreach ($stillMissingGroups as $groupId) { + $result[$groupId] = null; + } + + return $result; + } + public function clear(): void { $this->cache = new CappedMemoryCache(); $this->memCache->clear(); diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php index c7bbf147fc05b..adf41940aec79 100644 --- a/lib/private/Group/Manager.php +++ b/lib/private/Group/Manager.php @@ -173,7 +173,7 @@ protected function getGroupObject($gid, $displayName = null) { * @param array $displayNames Array containing already know display name for a groupId * @return array */ - protected function getGroupsObjects(array $gids, array $displayNames = []): array { + public function getGroupsObjects(array $gids, array $displayNames = []): array { $backends = []; $groups = []; foreach ($gids as $gid) {