From 24c4ef833b87e22ae5de92c9d973dc7730a3d0ca Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 8 Jul 2026 10:24:37 +0200 Subject: [PATCH 1/3] perf: Replace more calls from getUserGroups to getUserGroupIds Faster as we only need the gid and not the full group objects Signed-off-by: Carl Schwan --- .../lib/Controller/AUserDataOCSController.php | 6 +----- .../lib/Controller/UsersController.php | 3 +-- .../tests/Controller/GroupsControllerTest.php | 8 ++++---- .../tests/Controller/UsersControllerTest.php | 15 +++------------ 4 files changed, 9 insertions(+), 23 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php index 465188fd9b9e4..4ee02276e32bd 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 { 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..b3cf54b2034a2 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()) @@ -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()) 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], From c4024c01f12793c5b460142f5a9444ac850d8468 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Fri, 10 Jul 2026 11:44:51 +0200 Subject: [PATCH 2/3] perf: Batch getting the display names of groups Signed-off-by: Carl Schwan --- .../lib/Controller/AUserDataOCSController.php | 9 +++-- .../tests/Controller/GroupsControllerTest.php | 12 +++--- lib/private/Group/DisplayNameCache.php | 39 +++++++++++++++++++ lib/private/Group/Manager.php | 2 +- 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php index 4ee02276e32bd..6bae439688d65 100644 --- a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php +++ b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php @@ -274,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/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php index b3cf54b2034a2..d4c1e796262c8 100644 --- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php +++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php @@ -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); @@ -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/lib/private/Group/DisplayNameCache.php b/lib/private/Group/DisplayNameCache.php index cfd3989701520..06df58410b52f 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() ?? $group->getGID(); + $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) { From 8d004fb86907cb10c924a04f34c4e0032024e030 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 16 Jul 2026 06:10:14 +0200 Subject: [PATCH 3/3] fix(psaml): correct type Signed-off-by: Carl Schwan --- lib/private/Group/DisplayNameCache.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Group/DisplayNameCache.php b/lib/private/Group/DisplayNameCache.php index 06df58410b52f..b85244c8fc116 100644 --- a/lib/private/Group/DisplayNameCache.php +++ b/lib/private/Group/DisplayNameCache.php @@ -82,7 +82,7 @@ public function getDisplayNames(array $groupIds): array { $groups = $groupManager->getGroupsObjects($missing); $stillMissingGroups = array_diff($missing, array_keys($groups)); foreach ($groups as $groupId => $group) { - $displayName = $group->getDisplayName() ?? $group->getGID(); + $displayName = $group->getDisplayName(); $this->cache[$groupId] = $displayName; $this->memCache->set($groupId, $displayName, 60 * 10); // 10 minutes $result[$groupId] = $displayName;