Skip to content

Commit 31deb52

Browse files
committed
autocomplete: Sort user-mention autocomplete results
The @-mention autocomplete shows relevant results in the following order of priority: * Recent conversation users * Recent DM users * Alphabetical order Fixes: #228
1 parent 1a5425e commit 31deb52

6 files changed

+308
-4
lines changed

lib/model/algorithms.dart

+43
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,46 @@ QueueList<int> setUnion(Iterable<int> xs, Iterable<int> ys) {
116116
}
117117
return result;
118118
}
119+
120+
extension PossibleIndex<T extends num> on List<T> {
121+
/// The index of [element] in this sorted list.
122+
///
123+
/// Uses binary search to find the index, so the list must be sorted,
124+
/// otherwise the result is unpredictable.
125+
///
126+
/// If [element] is found, returns its index (0-based).
127+
///
128+
/// ```dart
129+
/// final somePrimes = [11, 13, 17, 23, 27];
130+
/// final index = somePrimes.possibleIndexOf(17); // 2
131+
/// ```
132+
/// If [element] is not found, returns the possible index (1-based)
133+
/// with a negative sign, where if the [element] is inserted,
134+
/// the order of the list is maintained.
135+
///
136+
/// ```dart
137+
/// final somePrimes = [11, 13, 17, 23, 27];
138+
/// int index = somePrimes.possibleIndexOf(7); // -1
139+
/// index = somePrimes.possibleIndexOf(19); // -4
140+
/// ```
141+
///
142+
/// Note: Using a negative 1-based index will differentiate between the case where
143+
/// the element is found at index 0 and the case where the element is not
144+
/// found, but should be placed at the very start of the list (index -1).
145+
int possibleIndexOf(T element) {
146+
int low = 0;
147+
int high = length - 1;
148+
149+
while (high >= low) {
150+
int mid = (low + high) ~/ 2;
151+
if (element == this[mid]) {
152+
return mid;
153+
} else if (element < this[mid]) {
154+
high = mid - 1;
155+
} else {
156+
low = mid + 1;
157+
}
158+
}
159+
return -low - 1;
160+
}
161+
}

lib/model/autocomplete.dart

+89-3
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ class MentionAutocompleteView extends ChangeNotifier {
183183
@override
184184
void dispose() {
185185
store.autocompleteViewManager.unregisterMentionAutocomplete(this);
186+
_sortedUsers = null;
186187
// We cancel in-progress computations by checking [hasListeners] between tasks.
187188
// After [super.dispose] is called, [hasListeners] returns false.
188189
// TODO test that logic (may involve detecting an unhandled Future rejection; how?)
@@ -206,6 +207,7 @@ class MentionAutocompleteView extends ChangeNotifier {
206207
/// Called in particular when we get a [RealmUserEvent].
207208
void refreshStaleUserResults() {
208209
if (_query != null) {
210+
_sortedUsers = null;
209211
_startSearch(_query!);
210212
}
211213
}
@@ -244,11 +246,89 @@ class MentionAutocompleteView extends ChangeNotifier {
244246
notifyListeners();
245247
}
246248

249+
List<User>? _sortedUsers;
250+
251+
int compareByRelevance({
252+
required User userA,
253+
required User userB,
254+
required int? streamId,
255+
required String? topic,
256+
}) {
257+
// TODO(#234): give preference to "all", "everyone" or "stream".
258+
259+
// TODO(#618): give preference to subscribed users first.
260+
261+
if (streamId != null) {
262+
final conversationPrecedence = store.recentSenders.compareByRecency(
263+
userA: userA,
264+
userB: userB,
265+
streamId: streamId,
266+
topic: topic);
267+
if (conversationPrecedence != 0) {
268+
return conversationPrecedence;
269+
}
270+
}
271+
272+
final dmPrecedence = store.recentDmConversationsView.compareByDms(userA, userB);
273+
if (dmPrecedence != 0) {
274+
return dmPrecedence;
275+
}
276+
277+
if (!userA.isBot && userB.isBot) {
278+
return -1;
279+
} else if (userA.isBot && !userB.isBot) {
280+
return 1;
281+
}
282+
283+
final userAName = store.autocompleteViewManager.autocompleteDataCache
284+
.lowercasedNameForUserName(userA.fullName);
285+
final userBName = store.autocompleteViewManager.autocompleteDataCache
286+
.lowercasedNameForUserName(userB.fullName);
287+
return userAName.compareTo(userBName);
288+
}
289+
290+
List<User> sortByRelevance({
291+
required List<User> users,
292+
required Narrow narrow,
293+
}) {
294+
switch (narrow) {
295+
case StreamNarrow():
296+
users.sort((userA, userB) => compareByRelevance(
297+
userA: userA,
298+
userB: userB,
299+
streamId: narrow.streamId,
300+
topic: null));
301+
case TopicNarrow():
302+
users.sort((userA, userB) => compareByRelevance(
303+
userA: userA,
304+
userB: userB,
305+
streamId: narrow.streamId,
306+
topic: narrow.topic));
307+
case DmNarrow():
308+
users.sort((userA, userB) => compareByRelevance(
309+
userA: userA,
310+
userB: userB,
311+
streamId: null,
312+
topic: null));
313+
case AllMessagesNarrow():
314+
// do nothing in this case for now
315+
}
316+
return users;
317+
}
318+
319+
void _sortUsers() {
320+
final users = store.users.values.toList();
321+
_sortedUsers = sortByRelevance(users: users, narrow: narrow);
322+
}
323+
247324
Future<List<MentionAutocompleteResult>?> _computeResults(MentionAutocompleteQuery query) async {
248325
final List<MentionAutocompleteResult> results = [];
249-
final Iterable<User> users = store.users.values;
250326

251-
final iterator = users.iterator;
327+
if (_sortedUsers == null) {
328+
_sortUsers();
329+
}
330+
331+
final iterator = _sortedUsers!.iterator;
252332
bool isDone = false;
253333
while (!isDone) {
254334
// CPU perf: End this task; enqueue a new one for resuming this work
@@ -270,7 +350,7 @@ class MentionAutocompleteView extends ChangeNotifier {
270350
}
271351
}
272352
}
273-
return results; // TODO(#228) sort for most relevant first
353+
return results;
274354
}
275355
}
276356

@@ -339,6 +419,12 @@ class AutocompleteDataCache {
339419
void invalidateUser(int userId) {
340420
_nameWordsByUser.remove(userId);
341421
}
422+
423+
final Map<String, String> _lowercasedNamesByUserName = {};
424+
425+
String lowercasedNameForUserName(String name) {
426+
return _lowercasedNamesByUserName[name] ??= name.toLowerCase();
427+
}
342428
}
343429

344430
sealed class MentionAutocompleteResult {}

lib/model/message_list.dart

+1
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
381381
for (final message in result.messages) {
382382
if (_messageVisible(message)) {
383383
_addMessage(message);
384+
store.recentSenders.processAsStreamMessage(message);
384385
}
385386
}
386387
_fetched = true;

lib/model/recent_dm_conversations.dart

+29-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ class RecentDmConversationsView extends ChangeNotifier {
3030
required this.map,
3131
required this.sorted,
3232
required this.selfUserId,
33-
});
33+
}) {
34+
_computeLatestMessagesWithUsers();
35+
}
3436

3537
/// The latest message ID in each conversation.
3638
final Map<DmNarrow, int> map;
@@ -40,6 +42,32 @@ class RecentDmConversationsView extends ChangeNotifier {
4042

4143
final int selfUserId;
4244

45+
/// Map from user ID to the latest message ID in any conversation with the user.
46+
///
47+
/// Both 1:1 and group DM conversations are considered.
48+
/// The self-user ID is excluded even if there is a self-DM conversation.
49+
///
50+
/// (The identified message was not necessarily sent by the identified user;
51+
/// it might have been sent by anyone in its conversation.)
52+
final Map<int, int> _latestMessageByRecipient = {};
53+
54+
void _computeLatestMessagesWithUsers() {
55+
for (final dmNarrow in sorted) {
56+
for (final userId in dmNarrow.otherRecipientIds) {
57+
if (!_latestMessageByRecipient.containsKey(userId)) {
58+
_latestMessageByRecipient[userId] = map[dmNarrow]!;
59+
}
60+
}
61+
}
62+
}
63+
64+
int compareByDms(User userA, User userB) {
65+
final aLatestMessageId = _latestMessageByRecipient[userA.userId] ?? -1;
66+
final bLatestMessageId = _latestMessageByRecipient[userB.userId] ?? -1;
67+
68+
return bLatestMessageId - aLatestMessageId;
69+
}
70+
4371
/// Insert the key at the proper place in [sorted].
4472
///
4573
/// Optimized, taking O(1) time, for the case where that place is the start,

lib/model/recent_senders.dart

+141
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import '../api/model/model.dart';
2+
import 'algorithms.dart';
3+
4+
/// A data structure to keep track of message ids.
5+
class MessageIdTracker {
6+
final List<int> _ids = [];
7+
8+
void add(int id) {
9+
if (_ids.isEmpty) {
10+
_ids.add(id);
11+
} else {
12+
int i = _ids.possibleIndexOf(id);
13+
if (i >= 0) { // the [id] already exists, so do not add it.
14+
return;
15+
}
16+
i = -i - 1; // change the index back to 0-based for readability.
17+
if (i == _ids.length) { // the [id] should be added to the end of the list.
18+
_ids.add(id);
19+
} else {
20+
_ids.insert(i, id);
21+
}
22+
}
23+
}
24+
25+
// TODO: remove
26+
27+
/// The maximum id in the tracker list.
28+
///
29+
/// Returns -1 if the tracker list is empty.
30+
int get maxId => _ids.isNotEmpty ? _ids.last : -1;
31+
32+
@override
33+
String toString() {
34+
return _ids.toString();
35+
}
36+
}
37+
38+
/// A data structure to keep track of stream and topic messages.
39+
///
40+
/// The owner should call [clear] in order to free resources.
41+
class RecentSenders {
42+
// streamSenders[streamId][senderId] = IdTracker
43+
final Map<int, Map<int, MessageIdTracker>> _streamSenders = {};
44+
45+
// topicSenders[streamId][topic][senderId] = IdTracker
46+
final Map<int, Map<String, Map<int, MessageIdTracker>>> _topicSenders = {};
47+
48+
void clear() {
49+
_streamSenders.clear();
50+
_topicSenders.clear();
51+
}
52+
53+
int _latestMessageIdOfSenderInStream({required int streamId, required int senderId}) {
54+
return _streamSenders[streamId]?[senderId]?.maxId ?? -1;
55+
}
56+
57+
int _latestMessageIdOfSenderInTopic({
58+
required int streamId,
59+
required String topic,
60+
required int senderId,
61+
}) {
62+
return _topicSenders[streamId]?[topic]?[senderId]?.maxId ?? -1;
63+
}
64+
65+
void _addMessageInStream({
66+
required int streamId,
67+
required int senderId,
68+
required messageId,
69+
}) {
70+
final senderMap = _streamSenders[streamId] ?? {};
71+
final idTracker = senderMap[senderId] ?? MessageIdTracker();
72+
_streamSenders[streamId] = senderMap;
73+
senderMap[senderId] = idTracker;
74+
idTracker.add(messageId);
75+
}
76+
77+
void _addMessageInTopic({
78+
required int streamId,
79+
required String topic,
80+
required int senderId,
81+
required messageId,
82+
}) {
83+
final topicMap = _topicSenders[streamId] ?? {};
84+
final senderMap = topicMap[topic] ?? {};
85+
final idTracker = senderMap[senderId] ?? MessageIdTracker();
86+
_topicSenders[streamId] = topicMap;
87+
topicMap[topic] = senderMap;
88+
senderMap[senderId] = idTracker;
89+
idTracker.add(messageId);
90+
}
91+
92+
/// Extracts and keeps track of the necessary data from a [message] only
93+
/// if it is a stream message.
94+
void processAsStreamMessage(Message message) {
95+
if (message is! StreamMessage) {
96+
return;
97+
}
98+
99+
final streamId = message.streamId;
100+
final topic = message.subject;
101+
final senderId = message.senderId;
102+
final messageId = message.id;
103+
104+
_addMessageInStream(
105+
streamId: streamId, senderId: senderId, messageId: messageId);
106+
_addMessageInTopic(
107+
streamId: streamId,
108+
topic: topic,
109+
senderId: senderId,
110+
messageId: messageId);
111+
}
112+
113+
// TODO: removeMessageInTopic
114+
115+
int compareByRecency({
116+
required User userA,
117+
required User userB,
118+
required int streamId,
119+
required String? topic,
120+
}) {
121+
int aMessageId, bMessageId;
122+
123+
if (topic != null) {
124+
aMessageId = _latestMessageIdOfSenderInTopic(
125+
streamId: streamId, topic: topic, senderId: userA.userId);
126+
bMessageId = _latestMessageIdOfSenderInTopic(
127+
streamId: streamId, topic: topic, senderId: userB.userId);
128+
129+
if (aMessageId != bMessageId) {
130+
return bMessageId - aMessageId;
131+
}
132+
}
133+
134+
aMessageId =
135+
_latestMessageIdOfSenderInStream(streamId: streamId, senderId: userA.userId);
136+
bMessageId =
137+
_latestMessageIdOfSenderInStream(streamId: streamId, senderId: userB.userId);
138+
139+
return bMessageId - aMessageId;
140+
}
141+
}

0 commit comments

Comments
 (0)