Skip to content

Commit 3be8a65

Browse files
blizzzblizzz
authored
Merge pull request #59025 from nextcloud/perf/noid/ldap-displayname-from-db
fix(LDAP): use displayname from DB, before reaching out to LDAP
2 parents 44175e3 + badd759 commit 3be8a65

4 files changed

Lines changed: 60 additions & 48 deletions

File tree

‎apps/user_ldap/lib/Access.php‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,7 @@ private function ldap2NextcloudNames(array $ldapObjects, bool $isUsers): array {
704704
continue;
705705
}
706706
$sndName = $ldapObject[$sndAttribute][0] ?? '';
707-
$this->cacheUserDisplayName($ncName, $nameByLDAP, $sndName);
707+
$this->applyUserDisplayName($ncName, $nameByLDAP, $sndName);
708708
} elseif ($nameByLDAP !== null) {
709709
$this->cacheGroupDisplayName($ncName, $nameByLDAP);
710710
}
@@ -752,20 +752,16 @@ public function cacheGroupExists(string $gid): void {
752752
$this->connection->writeToCache('groupExists' . $gid, true);
753753
}
754754

755-
/**
756-
* caches the user display name
757-
*
758-
* @param string $ocName the internal Nextcloud username
759-
* @param string $displayName the display name
760-
* @param string $displayName2 the second display name
761-
* @throws \Exception
762-
*/
763-
public function cacheUserDisplayName(string $ocName, string $displayName, string $displayName2 = ''): void {
764-
$user = $this->userManager->get($ocName);
755+
public function applyUserDisplayName(string $uid, string $displayName, string $displayName2 = ''): void {
756+
$user = $this->userManager->get($uid);
765757
if ($user === null) {
766758
return;
767759
}
768-
$displayName = $user->composeAndStoreDisplayName($displayName, $displayName2);
760+
$composedDisplayName = $user->composeAndStoreDisplayName($displayName, $displayName2);
761+
$this->cacheUserDisplayName($uid, $composedDisplayName);
762+
}
763+
764+
public function cacheUserDisplayName(string $ocName, string $displayName): void {
769765
$cacheKeyTrunk = 'getDisplayName';
770766
$this->connection->writeToCache($cacheKeyTrunk . $ocName, $displayName);
771767
}

‎apps/user_ldap/lib/User/User.php‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,8 @@ public function processAttributes(array $ldapEntry): void {
119119
$displayName2 = (string)$ldapEntry[$attr][0];
120120
}
121121
if ($displayName !== '') {
122-
$this->composeAndStoreDisplayName($displayName, $displayName2);
123-
$this->access->cacheUserDisplayName(
124-
$this->getUsername(),
125-
$displayName,
126-
$displayName2
127-
);
122+
$composedDisplayName = $this->composeAndStoreDisplayName($displayName, $displayName2);
123+
$this->access->cacheUserDisplayName($this->getUsername(), $composedDisplayName);
128124
}
129125
unset($attr);
130126

@@ -134,7 +130,8 @@ public function processAttributes(array $ldapEntry): void {
134130
$attr = strtolower($this->connection->ldapEmailAttribute);
135131
if (isset($ldapEntry[$attr])) {
136132
$mailValue = 0;
137-
for ($x = 0; $x < count($ldapEntry[$attr]); $x++) {
133+
$emailValues = count($ldapEntry[$attr]);
134+
for ($x = 0; $x < $emailValues; $x++) {
138135
if (filter_var($ldapEntry[$attr][$x], FILTER_VALIDATE_EMAIL)) {
139136
$mailValue = $x;
140137
break;
@@ -457,6 +454,10 @@ public function composeAndStoreDisplayName(string $displayName, string $displayN
457454
return $displayName;
458455
}
459456

457+
public function fetchStoredDisplayName(): string {
458+
return $this->userConfig->getValueString($this->uid, 'user_ldap', 'displayName', '');
459+
}
460+
460461
/**
461462
* Stores the LDAP Username in the Database
462463
*/

‎apps/user_ldap/lib/User_LDAP.php‎

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ public function getUsers($search = '', $limit = 10, $offset = 0) {
262262
/**
263263
* checks whether a user is still available on LDAP
264264
*
265-
* @param string|User $user either the Nextcloud user id or an instance of that user
265+
* @param string|User $user either the Nextcloud user id or an instance of
266+
* that user
266267
* @throws \Exception
267268
* @throws ServerNotAvailableException
268269
*/
@@ -421,26 +422,21 @@ public function getHome($uid) {
421422
return $path;
422423
}
423424

424-
/**
425-
* get display name of the user
426-
* @param string $uid user ID of the user
427-
* @return string|false display name
428-
*/
429-
public function getDisplayName($uid) {
430-
if ($this->userPluginManager->implementsActions(Backend::GET_DISPLAYNAME)) {
431-
return $this->userPluginManager->getDisplayName($uid);
432-
}
433-
434-
if (!$this->userExists($uid)) {
435-
return false;
425+
private function getDisplayNameFromDatabase(string $uid): ?string {
426+
$user = $this->access->userManager->get($uid);
427+
if ($user instanceof User) {
428+
$displayName = $user->fetchStoredDisplayName();
429+
if ($displayName !== '') {
430+
return $displayName;
431+
}
436432
}
437-
438-
$cacheKey = 'getDisplayName' . $uid;
439-
if (!is_null($displayName = $this->access->connection->getFromCache($cacheKey))) {
440-
return $displayName;
433+
if ($user instanceof OfflineUser) {
434+
return $user->getDisplayName();
441435
}
436+
return null;
437+
}
442438

443-
//Check whether the display name is configured to have a 2nd feature
439+
private function getDisplayNameFromLdap(string $uid): string {
444440
$additionalAttribute = $this->access->connection->ldapUserDisplayName2;
445441
$displayName2 = '';
446442
if ($additionalAttribute !== '') {
@@ -462,16 +458,40 @@ public function getDisplayName($uid) {
462458

463459
$user = $this->access->userManager->get($uid);
464460
if ($user instanceof User) {
465-
$displayName = $user->composeAndStoreDisplayName($displayName, (string)$displayName2);
466-
$this->access->connection->writeToCache($cacheKey, $displayName);
461+
return $user->composeAndStoreDisplayName($displayName, (string)$displayName2);
467462
}
468463
if ($user instanceof OfflineUser) {
469-
$displayName = $user->getDisplayName();
464+
return $user->getDisplayName();
470465
}
466+
}
467+
468+
return '';
469+
}
470+
471+
public function getDisplayName($uid): string {
472+
if ($this->userPluginManager->implementsActions(Backend::GET_DISPLAYNAME)) {
473+
return $this->userPluginManager->getDisplayName($uid);
474+
}
475+
476+
if (!$this->userExists($uid)) {
477+
return '';
478+
}
479+
480+
$cacheKey = 'getDisplayName' . $uid;
481+
if (!is_null($displayName = $this->access->connection->getFromCache($cacheKey))) {
471482
return $displayName;
472483
}
473484

474-
return null;
485+
if ($displayName = $this->getDisplayNameFromDatabase($uid)) {
486+
$this->access->connection->writeToCache($cacheKey, $displayName);
487+
return $displayName;
488+
}
489+
490+
if ($displayName = $this->getDisplayNameFromLdap($uid)) {
491+
$this->access->connection->writeToCache($cacheKey, $displayName);
492+
}
493+
494+
return $displayName;
475495
}
476496

477497
/**
@@ -495,7 +515,8 @@ public function setDisplayName($uid, $displayName) {
495515
* @param string $search
496516
* @param int|null $limit
497517
* @param int|null $offset
498-
* @return array an array of all displayNames (value) and the corresponding uids (key)
518+
* @return array an array of all displayNames (value) and the corresponding
519+
* uids (key)
499520
*/
500521
public function getDisplayNames($search = '', $limit = null, $offset = null) {
501522
$cacheKey = 'getDisplayNames-' . $search . '-' . $limit . '-' . $offset;

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2605,16 +2605,10 @@
26052605
<DeprecatedInterface>
26062606
<code><![CDATA[User_LDAP]]></code>
26072607
</DeprecatedInterface>
2608-
<ImplementedReturnTypeMismatch>
2609-
<code><![CDATA[string|false]]></code>
2610-
</ImplementedReturnTypeMismatch>
26112608
<MoreSpecificImplementedParamType>
26122609
<code><![CDATA[$limit]]></code>
26132610
<code><![CDATA[$offset]]></code>
26142611
</MoreSpecificImplementedParamType>
2615-
<NullableReturnStatement>
2616-
<code><![CDATA[null]]></code>
2617-
</NullableReturnStatement>
26182612
<RedundantCondition>
26192613
<code><![CDATA[$displayName && (count($displayName) > 0)]]></code>
26202614
<code><![CDATA[is_string($dn)]]></code>

0 commit comments

Comments
 (0)