Skip to content

Commit 681e2d4

Browse files
nickvergessennickvergessen
authored
Merge pull request #14024 from nextcloud/bugfix/14011/paginate-search-results
fix(search): Paginate unified search results
2 parents 494ec1c + fc3779c commit 681e2d4

File tree

3 files changed

+153
-22
lines changed

3 files changed

+153
-22
lines changed

‎lib/Search/ConversationSearch.php

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,17 @@ public function getOrder(string $route, array $routeParameters): ?int {
6666
}
6767

6868
/**
69+
* Search for user's conversations
70+
*
71+
* Cursor is the conversation token
72+
* Results are sorted by display name and then conversation token
73+
*
6974
* @inheritDoc
7075
*/
7176
public function search(IUser $user, ISearchQuery $query): SearchResult {
7277
$rooms = $this->manager->getRoomsForUser($user->getUID());
7378

79+
$cursorKey = null;
7480
$result = [];
7581
foreach ($rooms as $room) {
7682
if ($room->getType() === Room::TYPE_CHANGELOG) {
@@ -82,18 +88,19 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {
8288
$parameters['token'] === $room->getToken() &&
8389
str_starts_with($query->getRoute(), Application::APP_ID . '.')) {
8490
// Don't search the current conversation.
85-
//User most likely looks for other things with the same name
91+
// User most likely looks for other things with the same name
8692
continue;
8793
}
8894

95+
$displayName = $room->getDisplayName($user->getUID());
8996
if ($room->getType() === Room::TYPE_ONE_TO_ONE || $room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) {
9097
$otherUserId = str_replace(
9198
json_encode($user->getUID()),
9299
'',
93100
$room->getName()
94101
);
95102
if (stripos($otherUserId, $query->getTerm()) === false
96-
&& stripos($room->getDisplayName($user->getUID()), $query->getTerm()) === false) {
103+
&& stripos($displayName, $query->getTerm()) === false) {
97104
// Neither name nor displayname (one-to-one) match, skip
98105
continue;
99106
}
@@ -103,7 +110,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {
103110

104111
$entry = new SearchResultEntry(
105112
$this->avatarService->getAvatarUrl($room),
106-
$room->getDisplayName($user->getUID()),
113+
$displayName,
107114
'',
108115
$this->url->linkToRouteAbsolute('spreed.Page.showCall', ['token' => $room->getToken()]),
109116
'',
@@ -112,12 +119,62 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {
112119

113120
$entry->addAttribute('conversation', $room->getToken());
114121

115-
$result[] = $entry;
122+
$result[strtolower($displayName . '#' . $room->getToken())] = $entry;
123+
124+
if ($query->getCursor() === $room->getToken()) {
125+
$cursorKey = strtolower($displayName . '#' . $room->getToken());
126+
}
127+
}
128+
129+
if (count($result) <= $query->getLimit()) {
130+
return SearchResult::complete(
131+
$this->l->t('Conversations'),
132+
array_values($result),
133+
);
116134
}
135+
ksort($result);
136+
137+
$newCursorWithName = '#';
138+
if ($cursorKey) {
139+
$foundCursor = false;
140+
$filteredResults = [];
141+
$lastPossibleCursor = '#';
142+
foreach ($result as $key => $entry) {
143+
if ($cursorKey === $key) {
144+
$foundCursor = true;
145+
continue;
146+
}
147+
if (!$foundCursor) {
148+
continue;
149+
}
150+
151+
if (count($filteredResults) === $query->getLimit()) {
152+
// We already have enough results, but there are more,
153+
// so we add the cursor for the next request.
154+
$newCursorWithName = $lastPossibleCursor;
155+
break;
156+
}
157+
158+
$filteredResults[] = $entry;
159+
$lastPossibleCursor = $key;
160+
}
161+
} else {
162+
$filteredResults = array_slice($result, 0, $query->getLimit());
163+
// Next page starts at the last result
164+
$newCursorWithName = array_key_last($filteredResults);
165+
}
166+
167+
// Cursor is the token only (to survive renamed),
168+
// but the array key is `display name#token`, so we split by the #
169+
// and get the last part which is the token.
170+
// If it's empty, there is no cursor for a next page
171+
$parts = explode('#', $newCursorWithName);
172+
$newCursor = end($parts);
117173

118-
return SearchResult::complete(
174+
return SearchResult::paginated(
119175
$this->l->t('Conversations'),
120-
$result
176+
array_values($filteredResults),
177+
$newCursor
121178
);
122179
}
123180
}

‎tests/integration/features/bootstrap/FeatureContext.php

Lines changed: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3241,31 +3241,71 @@ protected function compareDataResponse(?TableNode $formData = null) {
32413241
/**
32423242
* @param TableNode|null $formData
32433243
*/
3244-
protected function compareSearchResponse(?TableNode $formData = null) {
3245-
$messages = $this->getDataFromResponse($this->response)['entries'];
3244+
protected function compareSearchResponse(?TableNode $formData = null, ?string $expectedCursor = null) {
3245+
$data = $this->getDataFromResponse($this->response);
3246+
$results = $data['entries'];
3247+
3248+
if ($expectedCursor !== null) {
3249+
Assert::assertSame($expectedCursor, $data['cursor']);
3250+
}
32463251

32473252
if ($formData === null) {
3248-
Assert::assertEmpty($messages);
3253+
Assert::assertEmpty($results);
32493254
return;
32503255
}
32513256

3252-
$expected = array_map(static function (array $message) {
3253-
$message['attributes.conversation'] = self::$identifierToToken[$message['attributes.conversation']];
3254-
$message['attributes.messageId'] = self::$textToMessageId[$message['attributes.messageId']];
3255-
return $message;
3257+
$expected = array_map(static function (array $result) {
3258+
if (isset($result['attributes.conversation'])) {
3259+
$result['attributes.conversation'] = self::$identifierToToken[$result['attributes.conversation']];
3260+
}
3261+
if (isset($result['attributes.messageId'])) {
3262+
$result['attributes.messageId'] = self::$textToMessageId[$result['attributes.messageId']];
3263+
}
3264+
return $result;
32563265
}, $formData->getHash());
32573266

32583267
$count = count($expected);
3259-
Assert::assertCount($count, $messages, 'Message count does not match');
3260-
3261-
Assert::assertEquals($expected, array_map(static function ($message) {
3262-
return [
3263-
'title' => $message['title'],
3264-
'subline' => $message['subline'],
3265-
'attributes.conversation' => $message['attributes']['conversation'],
3266-
'attributes.messageId' => $message['attributes']['messageId'],
3268+
Assert::assertCount($count, $results, 'Result count does not match');
3269+
3270+
Assert::assertEquals($expected, array_map(static function ($actual) {
3271+
$compare = [
3272+
'title' => $actual['title'],
3273+
'subline' => $actual['subline'],
32673274
];
3268-
}, $messages));
3275+
if (isset($actual['attributes']['conversation'])) {
3276+
$compare['attributes.conversation'] = $actual['attributes']['conversation'];
3277+
}
3278+
if (isset($actual['attributes']['messageId'])) {
3279+
$compare['attributes.messageId'] = $actual['attributes']['messageId'];
3280+
}
3281+
return $compare;
3282+
}, $results));
3283+
}
3284+
3285+
/**
3286+
* @Then /^user "([^"]*)" searches for conversations with "([^"]*)"(?: offset "([^"]*)")? limit (\d+)(?: expected cursor "([^"]*)")?$/
3287+
*
3288+
* @param string $user
3289+
* @param string $search
3290+
* @param int $limit
3291+
*/
3292+
public function userSearchesRooms(string $user, string $search, string $offset, int $limit, string $expectedCursor, ?TableNode $formData = null): void {
3293+
$searchUrl = '/search/providers/talk-conversations/search?limit=' . $limit;
3294+
if ($offset && array_key_exists($offset, self::$identifierToToken)) {
3295+
$searchUrl .= '&cursor=' . self::$identifierToToken[$offset];
3296+
}
3297+
3298+
$searchUrl .= '&term=' . $search;
3299+
3300+
$this->setCurrentUser($user);
3301+
$this->sendRequest('GET', $searchUrl);
3302+
$this->assertStatusCode($this->response, 200);
3303+
3304+
if ($expectedCursor !== null) {
3305+
$expectedCursor = self::$identifierToToken[$expectedCursor] ?? '';
3306+
}
3307+
3308+
$this->compareSearchResponse($formData, $expectedCursor);
32693309
}
32703310

32713311
/**

‎tests/integration/features/conversation-5/search.feature

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
Feature: conversation-5/search
2+
3+
Background:
4+
Given user "participant1" exists
5+
6+
Scenario: Search for conversations with cursor and limit
7+
Given user "participant1" creates room "Room 1" (v4)
8+
| roomType | 2 |
9+
| roomName | Room 1 |
10+
Given user "participant1" creates room "Room 2" (v4)
11+
| roomType | 2 |
12+
| roomName | room 2 |
13+
Given user "participant1" creates room "Room 3" (v4)
14+
| roomType | 2 |
15+
| roomName | ROOM 3 |
16+
Given user "participant1" creates room "Room 4" (v4)
17+
| roomType | 2 |
18+
| roomName | Room 4 |
19+
Given user "participant1" creates room "Room 5" (v4)
20+
| roomType | 2 |
21+
| roomName | Room 5 |
22+
Given user "participant1" creates room "Room 6" (v4)
23+
| roomType | 2 |
24+
| roomName | Room 6 |
25+
And user "participant1" searches for conversations with "o" limit 1 expected cursor "Room 1"
26+
| title | subline | attributes.conversation |
27+
| Room 1 | | Room 1 |
28+
And user "participant1" searches for conversations with "o" offset "Room 4" limit 1 expected cursor "Room 5"
29+
| title | subline | attributes.conversation |
30+
| Room 5 | | Room 5 |
31+
And user "participant1" searches for conversations with "o" offset "Room 4" limit 5 expected cursor ""
32+
| title | subline | attributes.conversation |
33+
| Room 5 | | Room 5 |
34+
| Room 6 | | Room 6 |

0 commit comments

Comments
 (0)