Merge pull request #33615 from nextcloud/perf/noid/user-displayname-cache-for-activity-providers

Use user name cache in activity providers

apps/comments/lib/Activity/Provider.php

@@ -43,8 +43,6 @@ class Provider implements IProvider {
 	protected ICommentsManager $commentsManager;
 	protected IUserManager $userManager;
 	protected IManager $activityManager;
-	/** @var string[] */
-	protected array $displayNames = [];
 
 	public function __construct(IFactory $languageFactory, IURLGenerator $url, ICommentsManager $commentsManager, IUserManager $userManager, IManager $activityManager) {
 		$this->languageFactory = $languageFactory;
@@ -213,22 +211,10 @@ class Provider implements IProvider {
 	}
 
 	protected function generateUserParameter(string $uid): array {
-		if (!isset($this->displayNames[$uid])) {
-			$this->displayNames[$uid] = $this->getDisplayName($uid);
-		}
-
 		return [
 			'type' => 'user',
 			'id' => $uid,
-			'name' => $this->displayNames[$uid],
+			'name' => $this->userManager->getDisplayName($uid) ?? $uid,
 		];
 	}
-
-	protected function getDisplayName(string $uid): string {
-		$user = $this->userManager->get($uid);
-		if ($user instanceof IUser) {
-			return $user->getDisplayName();
-		}
-		return $uid;
-	}
 }

apps/dav/lib/CalDAV/Activity/Provider/Base.php

@@ -112,34 +112,14 @@ abstract class Base implements IProvider {
 		];
 	}
 
-	/**
-	 * @param string $uid
-	 * @return array
-	 */
-	protected function generateUserParameter($uid) {
-		if (!isset($this->userDisplayNames[$uid])) {
-			$this->userDisplayNames[$uid] = $this->getUserDisplayName($uid);
-		}
-
+	protected function generateUserParameter(string $uid): array {
 		return [
 			'type' => 'user',
 			'id' => $uid,
-			'name' => $this->userDisplayNames[$uid],
+			'name' => $this->userManager->getDisplayName($uid) ?? $uid,
 		];
 	}
 
-	/**
-	 * @param string $uid
-	 * @return string
-	 */
-	protected function getUserDisplayName($uid) {
-		$user = $this->userManager->get($uid);
-		if ($user instanceof IUser) {
-			return $user->getDisplayName();
-		}
-		return $uid;
-	}
-
 	/**
 	 * @param string $gid
 	 * @return array

apps/dav/lib/CardDAV/Activity/Provider/Base.php

@@ -98,34 +98,14 @@ abstract class Base implements IProvider {
 		];
 	}
 
-	/**
-	 * @param string $uid
-	 * @return array
-	 */
 	protected function generateUserParameter(string $uid): array {
-		if (!isset($this->userDisplayNames[$uid])) {
-			$this->userDisplayNames[$uid] = $this->getUserDisplayName($uid);
-		}
-
 		return [
 			'type' => 'user',
 			'id' => $uid,
-			'name' => $this->userDisplayNames[$uid],
+			'name' => $this->userManager->getDisplayName($uid) ?? $uid,
 		];
 	}
 
-	/**
-	 * @param string $uid
-	 * @return string
-	 */
-	protected function getUserDisplayName(string $uid): string {
-		$user = $this->userManager->get($uid);
-		if ($user instanceof IUser) {
-			return $user->getDisplayName();
-		}
-		return $uid;
-	}
-
 	/**
 	 * @param string $gid
 	 * @return array

apps/dav/tests/unit/CalDAV/Activity/Provider/BaseTest.php

@@ -160,41 +160,4 @@ class BaseTest extends TestCase {
 			'name' => $gid,
 		], $this->invokePrivate($this->provider, 'generateGroupParameter', [$gid]));
 	}
-
-	public function dataGenerateUserParameter() {
-		$u1 = $this->createMock(IUser::class);
-		$u1->expects($this->any())
-			->method('getDisplayName')
-			->willReturn('User 1');
-		return [
-			['u1', 'User 1', $u1],
-			['u2', 'u2', null],
-		];
-	}
-
-	/**
-	 * @dataProvider dataGenerateUserParameter
-	 * @param string $uid
-	 * @param string $displayName
-	 * @param IUser|null $user
-	 */
-	public function testGenerateUserParameter(string $uid, string $displayName, ?IUser $user) {
-		$this->userManager->expects($this->once())
-			->method('get')
-			->with($uid)
-			->willReturn($user);
-
-		$this->assertEquals([
-			'type' => 'user',
-			'id' => $uid,
-			'name' => $displayName,
-		], $this->invokePrivate($this->provider, 'generateUserParameter', [$uid]));
-
-		// Test caching (only 1 user manager invocation allowed)
-		$this->assertEquals([
-			'type' => 'user',
-			'id' => $uid,
-			'name' => $displayName,
-		], $this->invokePrivate($this->provider, 'generateUserParameter', [$uid]));
-	}
 }

apps/files/lib/Activity/Provider.php

@@ -527,12 +527,12 @@ class Provider implements IProvider {
 	 */
 	protected function getUser($uid) {
 		// First try local user
-		$user = $this->userManager->get($uid);
-		if ($user instanceof IUser) {
+		$displayName = $this->userManager->getDisplayName($uid);
+		if ($displayName !== null) {
 			return [
 				'type' => 'user',
-				'id' => $user->getUID(),
-				'name' => $user->getDisplayName(),
+				'id' => $uid,
+				'name' => $displayName,
 			];
 		}
 

apps/files/tests/Activity/ProviderTest.php

@@ -177,17 +177,10 @@ class ProviderTest extends TestCase {
 		$provider = $this->getProvider();
 
 		if ($userDisplayName !== null) {
-			$user = $this->createMock(IUser::class);
-			$user->expects($this->once())
-				->method('getUID')
-				->willReturn($uid);
-			$user->expects($this->once())
-				->method('getDisplayName')
-				->willReturn($userDisplayName);
 			$this->userManager->expects($this->once())
-				->method('get')
+				->method('getDisplayName')
 				->with($uid)
-				->willReturn($user);
+				->willReturn($userDisplayName);
 		}
 		if ($cloudIdData !== null) {
 			$this->cloudIdManager->expects($this->once())

apps/files_sharing/lib/Activity/Providers/Base.php

@@ -177,12 +177,12 @@ abstract class Base implements IProvider {
 	 */
 	protected function getUser($uid) {
 		// First try local user
-		$user = $this->userManager->get($uid);
-		if ($user instanceof IUser) {
+		$displayName = $this->userManager->getDisplayName($uid);
+		if ($displayName !== null) {
 			return [
 				'type' => 'user',
-				'id' => $user->getUID(),
-				'name' => $user->getDisplayName(),
+				'id' => $uid,
+				'name' => $displayName,
 			];
 		}
 

apps/settings/lib/Activity/GroupProvider.php

@@ -51,8 +51,6 @@ class GroupProvider implements IProvider {
 
 	/** @var string[] */
 	protected $groupDisplayNames = [];
-	/** @var string[] */
-	protected $userDisplayNames = [];
 
 
 	public function __construct(L10nFactory $l10n,
@@ -169,32 +167,11 @@ class GroupProvider implements IProvider {
 		return $gid;
 	}
 
-	/**
-	 * @param string $uid
-	 * @return array
-	 */
 	protected function generateUserParameter(string $uid): array {
-		if (!isset($this->displayNames[$uid])) {
-			$this->userDisplayNames[$uid] = $this->getDisplayName($uid);
-		}
-
 		return [
 			'type' => 'user',
 			'id' => $uid,
-			'name' => $this->userDisplayNames[$uid],
+			'name' => $this->userManager->getDisplayName($uid) ?? $uid,
 		];
 	}
-
-	/**
-	 * @param string $uid
-	 * @return string
-	 */
-	protected function getDisplayName(string $uid): string {
-		$user = $this->userManager->get($uid);
-		if ($user instanceof IUser) {
-			return $user->getDisplayName();
-		} else {
-			return $uid;
-		}
-	}
 }

apps/settings/lib/Activity/Provider.php

@@ -66,9 +66,6 @@ class Provider implements IProvider {
 	/** @var IManager */
 	private $activityManager;
 
-	/** @var string[] cached displayNames - key is the UID and value the displayname */
-	protected $displayNames = [];
-
 	public function __construct(IFactory $languageFactory,
 								IURLGenerator $url,
 								IUserManager $userManager,
@@ -206,23 +203,10 @@ class Provider implements IProvider {
 	}
 
 	protected function generateUserParameter(string $uid): array {
-		if (!isset($this->displayNames[$uid])) {
-			$this->displayNames[$uid] = $this->getDisplayName($uid);
-		}
-
 		return [
 			'type' => 'user',
 			'id' => $uid,
-			'name' => $this->displayNames[$uid],
+			'name' => $this->userManager->getDisplayName($uid) ?? $uid,
 		];
 	}
-
-	protected function getDisplayName(string $uid): string {
-		$user = $this->userManager->get($uid);
-		if ($user instanceof IUser) {
-			return $user->getDisplayName();
-		}
-
-		return $uid;
-	}
 }

apps/sharebymail/lib/Activity.php

@@ -54,9 +54,6 @@ class Activity implements IProvider {
 	/** @var IContactsManager */
 	protected $contactsManager;
 
-	/** @var array */
-	protected $displayNames = [];
-
 	/** @var array */
 	protected $contactNames = [];
 
@@ -346,14 +343,10 @@ class Activity implements IProvider {
 	 * @return array
 	 */
 	protected function generateUserParameter($uid) {
-		if (!isset($this->displayNames[$uid])) {
-			$this->displayNames[$uid] = $this->getDisplayName($uid);
-		}
-
 		return [
 			'type' => 'user',
 			'id' => $uid,
-			'name' => $this->displayNames[$uid],
+			'name' => $this->userManager->getDisplayName($uid) ?? $uid,
 		];
 	}
 
@@ -381,17 +374,4 @@ class Activity implements IProvider {
 
 		return $email;
 	}
-
-	/**
-	 * @param string $uid
-	 * @return string
-	 */
-	protected function getDisplayName($uid) {
-		$user = $this->userManager->get($uid);
-		if ($user instanceof IUser) {
-			return $user->getDisplayName();
-		} else {
-			return $uid;
-		}
-	}
 }

apps/systemtags/lib/Activity/Provider.php

@@ -55,9 +55,6 @@ class Provider implements IProvider {
 	/** @var IUserManager */
 	protected $userManager;
 
-	/** @var string[] */
-	protected $displayNames = [];
-
 	/**
 	 * @param IFactory $languageFactory
 	 * @param IURLGenerator $url
@@ -334,15 +331,11 @@ class Provider implements IProvider {
 		];
 	}
 
-	protected function getUserParameter($uid) {
-		if (!isset($this->displayNames[$uid])) {
-			$this->displayNames[$uid] = $this->getDisplayName($uid);
-		}
-
+	protected function getUserParameter(string $uid): array {
 		return [
 			'type' => 'user',
 			'id' => $uid,
-			'name' => $this->displayNames[$uid],
+			'name' => $this->userManager->getDisplayName($uid) ?? $uid,
 		];
 	}
 
@@ -355,17 +348,4 @@ class Provider implements IProvider {
 			return $this->l->t('%s (invisible)', $parameter['name']);
 		}
 	}
-
-	/**
-	 * @param string $uid
-	 * @return string
-	 */
-	protected function getDisplayName($uid) {
-		$user = $this->userManager->get($uid);
-		if ($user instanceof IUser) {
-			return $user->getDisplayName();
-		} else {
-			return $uid;
-		}
-	}
 }