Merge pull request #33615 from nextcloud/perf/noid/user-displayname-c…

…ache-for-activity-providers Use user name cache in activity providers
nextcloud · Aug 19, 2022 · 71065f5 · 71065f5
2 parents 7d7f7ab + 7e11778
commit 71065f5
Show file tree

Hide file tree

Showing 11 changed files with 19 additions and 196 deletions.
diff --git a/apps/comments/lib/Activity/Provider.php b/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 @@ protected function generateFileParameter(int $id, string $path): array {
  }
 
  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;
- }
 }
diff --git a/apps/dav/lib/CalDAV/Activity/Provider/Base.php b/apps/dav/lib/CalDAV/Activity/Provider/Base.php
@@ -112,34 +112,14 @@ protected function generateLegacyCalendarParameter($id, $name) {
  ];
  }
 
- /**
- * @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

diff --git a/apps/dav/lib/CardDAV/Activity/Provider/Base.php b/apps/dav/lib/CardDAV/Activity/Provider/Base.php
@@ -98,34 +98,14 @@ protected function generateAddressbookParameter(array $data, IL10N $l): array {
  ];
  }
 
- /**
- * @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

diff --git a/apps/dav/tests/unit/CalDAV/Activity/Provider/BaseTest.php b/apps/dav/tests/unit/CalDAV/Activity/Provider/BaseTest.php
@@ -160,41 +160,4 @@ public function testGenerateGroupParameter(string $gid) {
  '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]));
- }
 }
diff --git a/apps/files/lib/Activity/Provider.php b/apps/files/lib/Activity/Provider.php
@@ -527,12 +527,12 @@ protected function getParentEndToEndEncryptionContainer(Folder $userFolder, Node
  */
  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,
  ];
  }
 

diff --git a/apps/files/tests/Activity/ProviderTest.php b/apps/files/tests/Activity/ProviderTest.php
@@ -177,17 +177,10 @@ public function testGetUser(string $uid, ?string $userDisplayName, ?array $cloud
  $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())

diff --git a/apps/files_sharing/lib/Activity/Providers/Base.php b/apps/files_sharing/lib/Activity/Providers/Base.php
@@ -177,12 +177,12 @@ protected function getFile($parameter, IEvent $event = null) {
  */
  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,
  ];
  }
 

diff --git a/apps/settings/lib/Activity/GroupProvider.php b/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 @@ protected function getGroupDisplayName(string $gid): string {
  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;
- }
- }
 }
diff --git a/apps/settings/lib/Activity/Provider.php b/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 @@ protected function setSubjects(IEvent $event, string $subject, array $parameters
  }
 
  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;
- }
 }
diff --git a/apps/sharebymail/lib/Activity.php b/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 @@ protected function generateEmailParameter($email) {
  * @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 @@ protected function getContactName($email) {
 
  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;
- }
- }
 }
diff --git a/apps/systemtags/lib/Activity/Provider.php b/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 @@ protected function getSystemTagParameter($parameter) {
  ];
  }
 
- 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 @@ protected function generatePlainSystemTag(array $parameter) {
  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;
- }
- }
 }