From 8393bddeb9ce3c555ae86c8a0bf68d56bbdf1912 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Mon, 11 Aug 2025 17:00:05 +0200 Subject: [PATCH 1/6] fix: use stable route for comment search results The old version with files.view.index (with dir and scrollto) is opening the folder, but does not open the file or scroll to it. The recommended way to reference a file is files.view.showFile as done by the files search provider and the activites app. OCA\Comments\Search\LegacyProvider and OCA\Comments\Search\Result are deprecated since 20 and thus I took the opportunity to migrate away from it. Signed-off-by: Daniel Kesselberg --- .../lib/Search/CommentsSearchProvider.php | 111 +++++++++++++++--- 1 file changed, 92 insertions(+), 19 deletions(-) diff --git a/apps/comments/lib/Search/CommentsSearchProvider.php b/apps/comments/lib/Search/CommentsSearchProvider.php index 87a658cab1c6c..7ff7d06d4bc97 100644 --- a/apps/comments/lib/Search/CommentsSearchProvider.php +++ b/apps/comments/lib/Search/CommentsSearchProvider.php @@ -6,8 +6,15 @@ * SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ + namespace OCA\Comments\Search; +use OCP\Comments\IComment; +use OCP\Comments\ICommentsManager; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; +use OCP\Files\Node; +use OCP\Files\NotFoundException; use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUser; @@ -16,15 +23,16 @@ use OCP\Search\ISearchQuery; use OCP\Search\SearchResult; use OCP\Search\SearchResultEntry; -use function array_map; -use function pathinfo; +use Psr\Log\LoggerInterface; class CommentsSearchProvider implements IProvider { public function __construct( private IUserManager $userManager, private IL10N $l10n, private IURLGenerator $urlGenerator, - private LegacyProvider $legacyProvider, + private ICommentsManager $commentsManager, + private IRootFolder $rootFolder, + private LoggerInterface $logger, ) { } @@ -45,27 +53,92 @@ public function getOrder(string $route, array $routeParameters): int { } public function search(IUser $user, ISearchQuery $query): SearchResult { + $userFolder = $this->rootFolder->getUserFolder($user->getUID()); + $result = $this->findCommentsBySearchQuery($query, $userFolder); + return SearchResult::complete( $this->l10n->t('Comments'), - array_map(function (Result $result) { - $path = $result->path; - $pathInfo = pathinfo($path); - $isUser = $this->userManager->userExists($result->authorId); + $result + ); + } + + /** + * @return list + */ + private function findCommentsBySearchQuery(ISearchQuery $query, Folder $userFolder): array { + $result = []; + $numComments = 50; + $offset = 0; + + while (count($result) < $numComments) { + $comments = $this->commentsManager->search( + $query->getTerm(), + 'files', + '', + 'comment', + $offset, + $numComments + ); + + foreach ($comments as $comment) { + if ($comment->getActorType() !== 'users') { + continue; + } + + try { + $node = $this->getFileForComment($userFolder, $comment); + } catch (\Throwable $e) { + $this->logger->debug('Found comment for a file, but obtaining the file thrown an exception', ['exception' => $e]); + continue; + } + + $actorId = $comment->getActorId(); + $isUser = $this->userManager->userExists($actorId); + $avatarUrl = $isUser - ? $this->urlGenerator->linkToRouteAbsolute('core.avatar.getAvatar', ['userId' => $result->authorId, 'size' => 42]) - : $this->urlGenerator->linkToRouteAbsolute('core.GuestAvatar.getAvatar', ['guestName' => $result->authorId, 'size' => 42]); - return new SearchResultEntry( + ? $this->urlGenerator->linkToRouteAbsolute('core.avatar.getAvatar', ['userId' => $actorId, 'size' => 42]) + : $this->urlGenerator->linkToRouteAbsolute('core.GuestAvatar.getAvatar', ['guestName' => $actorId, 'size' => 42]); + + $path = $userFolder->getRelativePath($node->getPath()); + + // Use shortened link to centralize the various + // files/folder url redirection in files.View.showFile + $link = $this->urlGenerator->linkToRoute( + 'files.View.showFile', + ['fileid' => $node->getId()] + ); + + $searchResultEntry = new SearchResultEntry( $avatarUrl, - $result->name, - $path, - $this->urlGenerator->linkToRouteAbsolute('files.view.index', [ - 'dir' => $pathInfo['dirname'], - 'scrollto' => $pathInfo['basename'], - ]), + $comment->getMessage(), + ltrim($path, '/'), + $this->urlGenerator->getAbsoluteURL($link), '', - true ); - }, $this->legacyProvider->search($query->getTerm())) - ); + $searchResultEntry->addAttribute('fileId', (string)$node->getId()); + $searchResultEntry->addAttribute('path', $path); + + $result[] = $searchResultEntry; + } + + if (count($comments) < $numComments) { + // Didn't find more comments when we tried to get, so there are no more comments. + break; + } + + $offset += $numComments; + $numComments = 50 - count($result); + } + + return $result; + } + + private function getFileForComment(Folder $userFolder, IComment $comment): Node { + $nodes = $userFolder->getById((int)$comment->getObjectId()); + if (empty($nodes)) { + throw new NotFoundException('File not found'); + } + + return array_shift($nodes); } } From 87337420c5f6c79a796eb21387b9de395351e3b2 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Tue, 12 Aug 2025 13:39:03 +0200 Subject: [PATCH 2/6] fixup! fix: use stable route for comment search results --- build/psalm-baseline.xml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index fd9c1b31a310d..466884118a46d 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -63,18 +63,6 @@ - - - - - - authorId]]> - authorId]]> - authorId]]> - name]]> - path]]> - - From 94f5107f3b1feb3140e2be06d641ef7bc4c0cc24 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Thu, 21 Aug 2025 20:07:36 +0200 Subject: [PATCH 3/6] style: fix typo in test Signed-off-by: Daniel Kesselberg --- apps/dav/tests/unit/Search/EventsSearchProviderTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/dav/tests/unit/Search/EventsSearchProviderTest.php b/apps/dav/tests/unit/Search/EventsSearchProviderTest.php index d14512ba71277..2b91cd145b7bc 100644 --- a/apps/dav/tests/unit/Search/EventsSearchProviderTest.php +++ b/apps/dav/tests/unit/Search/EventsSearchProviderTest.php @@ -269,14 +269,14 @@ public function testSearch(): void { $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('john.doe'); $query = $this->createMock(ISearchQuery::class); - $seachTermFilter = $this->createMock(IFilter::class); - $query->method('getFilter')->willReturnCallback(function ($name) use ($seachTermFilter) { + $searchTermFilter = $this->createMock(IFilter::class); + $query->method('getFilter')->willReturnCallback(function ($name) use ($searchTermFilter) { return match ($name) { - 'term' => $seachTermFilter, + 'term' => $searchTermFilter, default => null, }; }); - $seachTermFilter->method('get')->willReturn('search term'); + $searchTermFilter->method('get')->willReturn('search term'); $query->method('getLimit')->willReturn(5); $query->method('getCursor')->willReturn(20); $this->appManager->expects($this->once()) From b1ab1819394f7121259fd811c3dc4aae4505f64e Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Thu, 21 Aug 2025 20:10:05 +0200 Subject: [PATCH 4/6] fixup! fix: use stable route for comment search results --- apps/comments/lib/Search/CommentsSearchProvider.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/comments/lib/Search/CommentsSearchProvider.php b/apps/comments/lib/Search/CommentsSearchProvider.php index 7ff7d06d4bc97..949a2b60f4e91 100644 --- a/apps/comments/lib/Search/CommentsSearchProvider.php +++ b/apps/comments/lib/Search/CommentsSearchProvider.php @@ -134,11 +134,11 @@ private function findCommentsBySearchQuery(ISearchQuery $query, Folder $userFold } private function getFileForComment(Folder $userFolder, IComment $comment): Node { - $nodes = $userFolder->getById((int)$comment->getObjectId()); - if (empty($nodes)) { + $node = $userFolder->getFirstNodeById((int)$comment->getObjectId()); + if ($node === null) { throw new NotFoundException('File not found'); } - return array_shift($nodes); + return $node; } } From a5fee4aa4ea224808dfd6da7d82d914cbd2174f9 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Thu, 21 Aug 2025 21:09:10 +0200 Subject: [PATCH 5/6] fixup! fix: use stable route for comment search results --- apps/comments/lib/Search/CommentsSearchProvider.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/comments/lib/Search/CommentsSearchProvider.php b/apps/comments/lib/Search/CommentsSearchProvider.php index 949a2b60f4e91..24b3f47b98229 100644 --- a/apps/comments/lib/Search/CommentsSearchProvider.php +++ b/apps/comments/lib/Search/CommentsSearchProvider.php @@ -69,6 +69,7 @@ private function findCommentsBySearchQuery(ISearchQuery $query, Folder $userFold $result = []; $numComments = 50; $offset = 0; + $limit = $numComments; while (count($result) < $numComments) { $comments = $this->commentsManager->search( @@ -77,7 +78,7 @@ private function findCommentsBySearchQuery(ISearchQuery $query, Folder $userFold '', 'comment', $offset, - $numComments + $limit, ); foreach ($comments as $comment) { @@ -121,13 +122,13 @@ private function findCommentsBySearchQuery(ISearchQuery $query, Folder $userFold $result[] = $searchResultEntry; } - if (count($comments) < $numComments) { + if (count($comments) < $limit) { // Didn't find more comments when we tried to get, so there are no more comments. break; } - $offset += $numComments; - $numComments = 50 - count($result); + $offset += $limit; + $limit = $numComments - count($result); } return $result; From 756bdefcd19f415163f9aa675e33f9cc3f66979d Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Thu, 21 Aug 2025 21:30:20 +0200 Subject: [PATCH 6/6] fixup! fix: use stable route for comment search results --- .../Search/CommentsSearchProviderTest.php | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 apps/comments/tests/Unit/Search/CommentsSearchProviderTest.php diff --git a/apps/comments/tests/Unit/Search/CommentsSearchProviderTest.php b/apps/comments/tests/Unit/Search/CommentsSearchProviderTest.php new file mode 100644 index 0000000000000..a3aca625cac74 --- /dev/null +++ b/apps/comments/tests/Unit/Search/CommentsSearchProviderTest.php @@ -0,0 +1,164 @@ +userManager = $this->createMock(IUserManager::class); + $this->l10n = $this->createMock(IL10N::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->commentsManager = $this->createMock(ICommentsManager::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getFirstNodeById')->willReturnCallback(function (int $id) { + if ($id % 4 === 0) { + // Returning null for every fourth file to simulate a file not found case. + return null; + } + $node = $this->createMock(File::class); + $node->method('getId')->willReturn($id); + $node->method('getPath')->willReturn('/' . $id . '.txt'); + return $node; + }); + $userFolder->method('getRelativePath')->willReturnArgument(0); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + + $this->userManager->method('userExists')->willReturn(true); + + $this->l10n->method('t')->willReturnArgument(0); + + $this->provider = new CommentsSearchProvider( + $this->userManager, + $this->l10n, + $this->urlGenerator, + $this->commentsManager, + $this->rootFolder, + new NullLogger(), + ); + } + + public function testGetId(): void { + $this->assertEquals('comments', $this->provider->getId()); + } + + public function testGetName(): void { + $this->l10n->expects($this->once()) + ->method('t') + ->with('Comments') + ->willReturnArgument(0); + + $this->assertEquals('Comments', $this->provider->getName()); + } + + public function testSearch(): void { + $this->commentsManager->method('search')->willReturnCallback(function (string $search, string $objectType, string $objectId, string $verb, int $offset, int $limit = 50) { + // The search method is call until 50 comments are found or there are no more comments to search. + $comments = []; + for ($i = 1; $i <= $limit; $i++) { + $comments[] = $this->mockComment(($offset + $i)); + } + return $comments; + }); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('alice'); + $searchTermFilter = $this->createMock(IFilter::class); + $searchTermFilter->method('get')->willReturn('search term'); + $searchQuery = $this->createMock(ISearchQuery::class); + $searchQuery->method('getFilter')->willReturnCallback(function ($name) use ($searchTermFilter) { + return match ($name) { + 'term' => $searchTermFilter, + default => null, + }; + }); + + $result = $this->provider->search($user, $searchQuery); + $data = $result->jsonSerialize(); + + $this->assertCount(50, $data['entries']); + } + + public function testSearchNoMoreComments(): void { + $this->commentsManager->method('search')->willReturnCallback(function (string $search, string $objectType, string $objectId, string $verb, int $offset, int $limit = 50) { + // Decrease the limit to simulate no more comments to search -> the break case. + if ($offset > 0) { + $limit--; + } + $comments = []; + for ($i = 1; $i <= $limit; $i++) { + $comments[] = $this->mockComment(($offset + $i)); + } + return $comments; + }); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('alice'); + $searchTermFilter = $this->createMock(IFilter::class); + $searchTermFilter->method('get')->willReturn('search term'); + $searchQuery = $this->createMock(ISearchQuery::class); + $searchQuery->method('getFilter')->willReturnCallback(function ($name) use ($searchTermFilter) { + return match ($name) { + 'term' => $searchTermFilter, + default => null, + }; + }); + + + $result = $this->provider->search($user, $searchQuery); + $data = $result->jsonSerialize(); + + $this->assertCount(46, $data['entries']); + } + + private function mockComment(int $id): IComment { + return new Comment([ + 'id' => (string)$id, + 'parent_id' => '0', + 'topmost_parent_id' => '0', + 'children_count' => 0, + 'actor_type' => 'users', + 'actor_id' => 'user' . $id, + 'message' => 'Comment ' . $id, + 'verb' => 'comment', + 'creation_timestamp' => new \DateTime(), + 'latest_child_timestamp' => null, + 'object_type' => 'files', + 'object_id' => (string)$id + ]); + } + +}