Merge pull request #33614 from nextcloud/perf/noid/user-displayname-cache-for-comment-mentions

Use user displayname cache for comment mentions

2 files changed, 32 insertions, 58 deletions

diff --git a/apps/comments/lib/Notification/Notifier.php b/apps/comments/lib/Notification/Notifier.php
index 7c6a40133ee..4ddb7295bfe 100644
--- a/apps/comments/lib/Notification/Notifier.php
+++ b/apps/comments/lib/Notification/Notifier.php
@@ -100,9 +100,9 @@ class Notifier implements INotifier {
 		$displayName = $comment->getActorId();
 		$isDeletedActor = $comment->getActorType() === ICommentsManager::DELETED_USER;
 		if ($comment->getActorType() === 'users') {
-			$commenter = $this->userManager->get($comment->getActorId());
-			if ($commenter instanceof IUser) {
-				$displayName = $commenter->getDisplayName();
+			$commenter = $this->userManager->getDisplayName($comment->getActorId());
+			if ($commenter !== null) {
+				$displayName = $commenter;
 			}
 		}
 
@@ -171,8 +171,8 @@ class Notifier implements INotifier {
 		$mentions = $comment->getMentions();
 		foreach ($mentions as $mention) {
 			if ($mention['type'] === 'user') {
-				$user = $this->userManager->get($mention['id']);
-				if (!$user instanceof IUser) {
+				$userDisplayName = $this->userManager->getDisplayName($mention['id']);
+				if ($userDisplayName === null) {
 					continue;
 				}
 			}
diff --git a/apps/comments/tests/Unit/Notification/NotifierTest.php b/apps/comments/tests/Unit/Notification/NotifierTest.php
index 330530f0000..ecd22ffd9e3 100644
--- a/apps/comments/tests/Unit/Notification/NotifierTest.php
+++ b/apps/comments/tests/Unit/Notification/NotifierTest.php
@@ -39,27 +39,28 @@ use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
 use OCP\Notification\INotification;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class NotifierTest extends TestCase {
 
 	/** @var Notifier */
 	protected $notifier;
-	/** @var IFactory|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IFactory|MockObject */
 	protected $l10nFactory;
-	/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IL10N|MockObject */
 	protected $l;
-	/** @var IRootFolder|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IRootFolder|MockObject */
 	protected $folder;
-	/** @var ICommentsManager|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var ICommentsManager|MockObject */
 	protected $commentsManager;
-	/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IURLGenerator|MockObject */
 	protected $url;
-	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IUserManager|MockObject */
 	protected $userManager;
-	/** @var INotification|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var INotification|MockObject */
 	protected $notification;
-	/** @var IComment|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IComment|MockObject */
 	protected $comment;
 	/** @var string */
 	protected $lc = 'tlh_KX';
@@ -97,15 +98,7 @@ class NotifierTest extends TestCase {
 		$displayName = 'Huraga';
 		$message = '@Huraga mentioned you in a comment on "Gre\'thor.odp"';
 
-		/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */
-		$user = $this->createMock(IUser::class);
-		$user->expects($this->once())
-			->method('getDisplayName')
-			->willReturn($displayName);
-		/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $you */
-		$you = $this->createMock(IUser::class);
-
-		/** @var Node|\PHPUnit\Framework\MockObject\MockObject $node */
+		/** @var Node|MockObject $node */
 		$node = $this->createMock(Node::class);
 		$node
 			->expects($this->atLeastOnce())
@@ -213,10 +206,10 @@ class NotifierTest extends TestCase {
 
 		$this->userManager
 			->expects($this->exactly(2))
-			->method('get')
+			->method('getDisplayName')
 			->willReturnMap([
-				['huraga', $user],
-				['you', $you],
+				['huraga', $displayName],
+				['you', 'You'],
 			]);
 
 		$this->notifier->prepare($this->notification, $this->lc);
@@ -226,10 +219,7 @@ class NotifierTest extends TestCase {
 		$fileName = 'Gre\'thor.odp';
 		$message = 'You were mentioned on "Gre\'thor.odp", in a comment by a user that has since been deleted';
 
-		/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $you */
-		$you = $this->createMock(IUser::class);
-
-		/** @var Node|\PHPUnit\Framework\MockObject\MockObject $node */
+		/** @var Node|MockObject $node */
 		$node = $this->createMock(Node::class);
 		$node
 			->expects($this->atLeastOnce())
@@ -334,9 +324,11 @@ class NotifierTest extends TestCase {
 
 		$this->userManager
 			->expects($this->once())
-			->method('get')
-			->with('you')
-			->willReturn($you);
+			->method('getDisplayName')
+			->willReturnMap([
+				['huraga', null],
+				['you', 'You'],
+			]);
 
 		$this->notifier->prepare($this->notification, $this->lc);
 	}
@@ -373,7 +365,7 @@ class NotifierTest extends TestCase {
 
 		$this->userManager
 			->expects($this->never())
-			->method('get');
+			->method('getDisplayName');
 
 		$this->notifier->prepare($this->notification, $this->lc);
 	}
@@ -411,7 +403,7 @@ class NotifierTest extends TestCase {
 
 		$this->userManager
 			->expects($this->never())
-			->method('get');
+			->method('getDisplayName');
 
 		$this->notifier->prepare($this->notification, $this->lc);
 	}
@@ -422,12 +414,6 @@ class NotifierTest extends TestCase {
 
 		$displayName = 'Huraga';
 
-		/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */
-		$user = $this->createMock(IUser::class);
-		$user->expects($this->once())
-			->method('getDisplayName')
-			->willReturn($displayName);
-
 		$this->folder
 			->expects($this->never())
 			->method('getById');
@@ -472,9 +458,9 @@ class NotifierTest extends TestCase {
 
 		$this->userManager
 			->expects($this->once())
-			->method('get')
+			->method('getDisplayName')
 			->with('huraga')
-			->willReturn($user);
+			->willReturn($displayName);
 
 		$this->notifier->prepare($this->notification, $this->lc);
 	}
@@ -485,12 +471,6 @@ class NotifierTest extends TestCase {
 
 		$displayName = 'Huraga';
 
-		/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */
-		$user = $this->createMock(IUser::class);
-		$user->expects($this->once())
-			->method('getDisplayName')
-			->willReturn($displayName);
-
 		$this->folder
 			->expects($this->never())
 			->method('getById');
@@ -536,9 +516,9 @@ class NotifierTest extends TestCase {
 
 		$this->userManager
 			->expects($this->once())
-			->method('get')
+			->method('getDisplayName')
 			->with('huraga')
-			->willReturn($user);
+			->willReturn($displayName);
 
 		$this->notifier->prepare($this->notification, $this->lc);
 	}
@@ -549,12 +529,6 @@ class NotifierTest extends TestCase {
 
 		$displayName = 'Huraga';
 
-		/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */
-		$user = $this->createMock(IUser::class);
-		$user->expects($this->once())
-			->method('getDisplayName')
-			->willReturn($displayName);
-
 		$userFolder = $this->createMock(Folder::class);
 		$this->folder->expects($this->once())
 			->method('getUserFolder')
@@ -609,9 +583,9 @@ class NotifierTest extends TestCase {
 
 		$this->userManager
 			->expects($this->once())
-			->method('get')
+			->method('getDisplayName')
 			->with('huraga')
-			->willReturn($user);
+			->willReturn($displayName);
 
 		$this->notifier->prepare($this->notification, $this->lc);
 	}