From 1c01895796bef464a4af6543336a64306ebd827d Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 18 Mar 2026 10:49:57 +0100 Subject: [PATCH] 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 | 67 +++++++++++++++++++++----------- 3 files changed, 58 insertions(+), 41 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 3e4efc5b8db..9dbabe4e3f7 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -704,7 +704,7 @@ class Access extends LDAPUtility { continue; } $sndName = $ldapObject[$sndAttribute][0] ?? ''; - $this->cacheUserDisplayName($ncName, $nameByLDAP, $sndName); + $this->applyUserDisplayName($ncName, $nameByLDAP, $sndName); } elseif ($nameByLDAP !== null) { $this->cacheGroupDisplayName($ncName, $nameByLDAP); } @@ -752,20 +752,16 @@ class Access extends LDAPUtility { $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 2f928d1b67e..cb7ab06bc09 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -119,12 +119,8 @@ class User { $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); @@ -457,6 +453,10 @@ class User { return $displayName; } + public function fetchStoredDisplayName(): string { + return $this->userConfig->getValueString($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 c515002a2de..22475ceccab 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -262,7 +262,8 @@ class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, I /** * checks whether a user is still available on LDAP * - * @param string|User $user either the Nextcloud user id or an instance of that user + * @param string|User $user either the Nextcloud user id or an instance of + * that user * @throws \Exception * @throws ServerNotAvailableException */ @@ -421,26 +422,21 @@ class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, I 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); + private function getDisplayNameFromDatabase(string $uid): ?string { + $user = $this->access->userManager->get($uid); + if ($user instanceof User) { + $displayName = $user->fetchStoredDisplayName(); + if ($displayName !== '') { + return $displayName; + } } - - if (!$this->userExists($uid)) { - return false; + if ($user instanceof OfflineUser) { + return $user->getDisplayName(); } + return null; + } - $cacheKey = 'getDisplayName' . $uid; - if (!is_null($displayName = $this->access->connection->getFromCache($cacheKey))) { - return $displayName; - } - - //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 !== '') { @@ -462,16 +458,40 @@ class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, I $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; } /** @@ -495,7 +515,8 @@ class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, I * @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;