From b5f245ce09b8bb15faa3bcba9247ca11df3e9600 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 18 Mar 2026 10:49:57 +0100 Subject: [PATCH 1/3] fix(LDAP): use displayname from DB, before reaching out to LDAP As we do it with other information of the user, we now use the known value of a users displayname, and leave the updating to the background job. This improves performance of user facing actions where the display name is required and reduces queries to the LDAP server that are typically more expensive. Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Access.php | 20 ++++------ apps/user_ldap/lib/User/User.php | 12 +++--- apps/user_ldap/lib/User_LDAP.php | 64 +++++++++++++++++++++----------- 3 files changed, 56 insertions(+), 40 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 9fe0aa6426856..b79bebc29b416 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -706,7 +706,7 @@ private function ldap2NextcloudNames(array $ldapObjects, bool $isUsers): array { continue; } $sndName = $ldapObject[$sndAttribute][0] ?? ''; - $this->cacheUserDisplayName($ncName, $nameByLDAP, $sndName); + $this->applyUserDisplayName($ncName, $nameByLDAP, $sndName); } elseif ($nameByLDAP !== null) { $this->cacheGroupDisplayName($ncName, $nameByLDAP); } @@ -754,20 +754,16 @@ public function cacheGroupExists(string $gid): void { $this->connection->writeToCache('groupExists' . $gid, true); } - /** - * caches the user display name - * - * @param string $ocName the internal Nextcloud username - * @param string $displayName the display name - * @param string $displayName2 the second display name - * @throws \Exception - */ - public function cacheUserDisplayName(string $ocName, string $displayName, string $displayName2 = ''): void { - $user = $this->userManager->get($ocName); + public function applyUserDisplayName(string $uid, string $displayName, string $displayName2 = ''): void { + $user = $this->userManager->get($uid); if ($user === null) { return; } - $displayName = $user->composeAndStoreDisplayName($displayName, $displayName2); + $composedDisplayName = $user->composeAndStoreDisplayName($displayName, $displayName2); + $this->cacheUserDisplayName($uid, $composedDisplayName); + } + + public function cacheUserDisplayName(string $ocName, string $displayName): void { $cacheKeyTrunk = 'getDisplayName'; $this->connection->writeToCache($cacheKeyTrunk . $ocName, $displayName); } diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 8f97ec1701f24..b54b17fba9fd5 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -114,12 +114,8 @@ public function processAttributes(array $ldapEntry): void { $displayName2 = (string)$ldapEntry[$attr][0]; } if ($displayName !== '') { - $this->composeAndStoreDisplayName($displayName, $displayName2); - $this->access->cacheUserDisplayName( - $this->getUsername(), - $displayName, - $displayName2 - ); + $composedDisplayName = $this->composeAndStoreDisplayName($displayName, $displayName2); + $this->access->cacheUserDisplayName($this->getUsername(), $composedDisplayName); } unset($attr); @@ -452,6 +448,10 @@ public function composeAndStoreDisplayName(string $displayName, string $displayN return $displayName; } + public function fetchStoredDisplayName(): string { + return $this->config->getUserValue($this->uid, 'user_ldap', 'displayName', ''); + } + /** * Stores the LDAP Username in the Database */ diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php index 708aeb037fd2b..e058c2b2ad1b6 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -420,26 +420,21 @@ public function getHome($uid) { return $path; } - /** - * get display name of the user - * @param string $uid user ID of the user - * @return string|false display name - */ - public function getDisplayName($uid) { - if ($this->userPluginManager->implementsActions(Backend::GET_DISPLAYNAME)) { - return $this->userPluginManager->getDisplayName($uid); - } - - if (!$this->userExists($uid)) { - return false; + private function getDisplayNameFromDatabase(string $uid): ?string { + $user = $this->access->userManager->get($uid); + if ($user instanceof User) { + $displayName = $user->fetchStoredDisplayName(); + if ($displayName !== '') { + return $displayName; + } } - - $cacheKey = 'getDisplayName' . $uid; - if (!is_null($displayName = $this->access->connection->getFromCache($cacheKey))) { - return $displayName; + if ($user instanceof OfflineUser) { + return $user->getDisplayName(); } + return null; + } - //Check whether the display name is configured to have a 2nd feature + private function getDisplayNameFromLdap(string $uid): string { $additionalAttribute = $this->access->connection->ldapUserDisplayName2; $displayName2 = ''; if ($additionalAttribute !== '') { @@ -461,16 +456,40 @@ public function getDisplayName($uid) { $user = $this->access->userManager->get($uid); if ($user instanceof User) { - $displayName = $user->composeAndStoreDisplayName($displayName, (string)$displayName2); - $this->access->connection->writeToCache($cacheKey, $displayName); + return $user->composeAndStoreDisplayName($displayName, (string)$displayName2); } if ($user instanceof OfflineUser) { - $displayName = $user->getDisplayName(); + return $user->getDisplayName(); } + } + + return ''; + } + + public function getDisplayName($uid): string { + if ($this->userPluginManager->implementsActions(Backend::GET_DISPLAYNAME)) { + return $this->userPluginManager->getDisplayName($uid); + } + + if (!$this->userExists($uid)) { + return ''; + } + + $cacheKey = 'getDisplayName' . $uid; + if (!is_null($displayName = $this->access->connection->getFromCache($cacheKey))) { return $displayName; } - return null; + if ($displayName = $this->getDisplayNameFromDatabase($uid)) { + $this->access->connection->writeToCache($cacheKey, $displayName); + return $displayName; + } + + if ($displayName = $this->getDisplayNameFromLdap($uid)) { + $this->access->connection->writeToCache($cacheKey, $displayName); + } + + return $displayName; } /** @@ -494,7 +513,8 @@ public function setDisplayName($uid, $displayName) { * @param string $search * @param int|null $limit * @param int|null $offset - * @return array an array of all displayNames (value) and the corresponding uids (key) + * @return array an array of all displayNames (value) and the corresponding + * uids (key) */ public function getDisplayNames($search = '', $limit = null, $offset = null) { $cacheKey = 'getDisplayNames-' . $search . '-' . $limit . '-' . $offset; From 31f35897622633076f4ee524915034aba05a047c Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 18 Mar 2026 10:55:22 +0100 Subject: [PATCH 2/3] fix(LDAP): do not use count() inside a loop Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/User/User.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index b54b17fba9fd5..5b3bb2e1e35aa 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -125,7 +125,8 @@ public function processAttributes(array $ldapEntry): void { $attr = strtolower($this->connection->ldapEmailAttribute); if (isset($ldapEntry[$attr])) { $mailValue = 0; - for ($x = 0; $x < count($ldapEntry[$attr]); $x++) { + $emailValues = count($ldapEntry[$attr]); + for ($x = 0; $x < $emailValues; $x++) { if (filter_var($ldapEntry[$attr][$x], FILTER_VALIDATE_EMAIL)) { $mailValue = $x; break; From adc98fac7aedefa772b69e30e8e042d785964c47 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 18 Mar 2026 11:09:31 +0100 Subject: [PATCH 3/3] ci: update psalm baseline Signed-off-by: Arthur Schiwon --- build/psalm-baseline.xml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 940b950fbecdd..b8805ae77cf25 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2650,16 +2650,10 @@ - - - - - - 0)]]>