From 967ebca281209c0896ef29bfe2cb8a46558d1688 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Mon, 13 May 2024 23:25:39 +0430 Subject: [PATCH 01/11] recent-dm-conversations: Add `latestMessagesByRecipient` data structure This data structure is used to keep track of the latest message of each recipient in all DM conversations. --- lib/model/recent_dm_conversations.dart | 45 +++++++++++++- .../model/recent_dm_conversations_checks.dart | 2 + test/model/recent_dm_conversations_test.dart | 61 ++++++++++++++++--- 3 files changed, 96 insertions(+), 12 deletions(-) diff --git a/lib/model/recent_dm_conversations.dart b/lib/model/recent_dm_conversations.dart index 5fd6fcf9ee..d1918b1960 100644 --- a/lib/model/recent_dm_conversations.dart +++ b/lib/model/recent_dm_conversations.dart @@ -1,3 +1,5 @@ +import 'dart:math'; + import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; @@ -19,9 +21,22 @@ class RecentDmConversationsView extends ChangeNotifier { DmNarrow.ofRecentDmConversation(conversation, selfUserId: selfUserId), conversation.maxMessageId, )).toList()..sort((a, b) => -a.value.compareTo(b.value)); + final map = Map.fromEntries(entries); + final sorted = QueueList.from(entries.map((e) => e.key)); + + final msgsByUser = {}; + for (final entry in entries) { + final dmNarrow = entry.key; + final msg = entry.value; + for (final userId in dmNarrow.otherRecipientIds) { + // only take the latest message of a user among all the conversations. + msgsByUser.putIfAbsent(userId, () => msg); + } + } return RecentDmConversationsView._( - map: Map.fromEntries(entries), - sorted: QueueList.from(entries.map((e) => e.key)), + map: map, + sorted: sorted, + latestMessagesByRecipient: msgsByUser, selfUserId: selfUserId, ); } @@ -29,6 +44,7 @@ class RecentDmConversationsView extends ChangeNotifier { RecentDmConversationsView._({ required this.map, required this.sorted, + required this.latestMessagesByRecipient, required this.selfUserId, }); @@ -38,8 +54,24 @@ class RecentDmConversationsView extends ChangeNotifier { /// The [DmNarrow] keys of [map], sorted by latest message descending. final QueueList sorted; + /// Map from user ID to the latest message ID in any conversation with the user. + /// + /// Both 1:1 and group DM conversations are considered. + /// The self-user ID is excluded even if there is a self-DM conversation. + /// + /// (The identified message was not necessarily sent by the identified user; + /// it might have been sent by anyone in its conversation.) + final Map latestMessagesByRecipient; + final int selfUserId; + int compareByDms(User userA, User userB) { + final aLatestMessageId = latestMessagesByRecipient[userA.userId] ?? -1; + final bLatestMessageId = latestMessagesByRecipient[userB.userId] ?? -1; + + return bLatestMessageId.compareTo(aLatestMessageId); + } + /// Insert the key at the proper place in [sorted]. /// /// Optimized, taking O(1) time, for the case where that place is the start, @@ -58,7 +90,7 @@ class RecentDmConversationsView extends ChangeNotifier { } } - /// Handle [MessageEvent], updating [map] and [sorted]. + /// Handle [MessageEvent], updating [map], [sorted], and [latestMessagesByRecipient]. /// /// Can take linear time in general. That sounds inefficient... /// but it's what the webapp does, so must not be catastrophic. 🤷 @@ -117,6 +149,13 @@ class RecentDmConversationsView extends ChangeNotifier { _insertSorted(key, message.id); } } + for (final recipient in key.otherRecipientIds) { + latestMessagesByRecipient.update( + recipient, + (latestMessageId) => max(message.id, latestMessageId), + ifAbsent: () => message.id, + ); + } notifyListeners(); } diff --git a/test/model/recent_dm_conversations_checks.dart b/test/model/recent_dm_conversations_checks.dart index af92ca25ac..bbb78593f9 100644 --- a/test/model/recent_dm_conversations_checks.dart +++ b/test/model/recent_dm_conversations_checks.dart @@ -6,4 +6,6 @@ import 'package:zulip/model/recent_dm_conversations.dart'; extension RecentDmConversationsViewChecks on Subject { Subject> get map => has((v) => v.map, 'map'); Subject> get sorted => has((v) => v.sorted, 'sorted'); + Subject> get latestMessagesByRecipient => has( + (v) => v.latestMessagesByRecipient, 'latestMessagesByRecipient'); } diff --git a/test/model/recent_dm_conversations_test.dart b/test/model/recent_dm_conversations_test.dart index c09ca53527..a3f01b4d35 100644 --- a/test/model/recent_dm_conversations_test.dart +++ b/test/model/recent_dm_conversations_test.dart @@ -22,7 +22,8 @@ void main() { check(RecentDmConversationsView(selfUserId: eg.selfUser.userId, initial: [])) ..map.isEmpty() - ..sorted.isEmpty(); + ..sorted.isEmpty() + ..latestMessagesByRecipient.isEmpty(); check(RecentDmConversationsView(selfUserId: eg.selfUser.userId, initial: [ @@ -35,7 +36,8 @@ void main() { key([]): 200, key([1]): 100, }) - ..sorted.deepEquals([key([1, 2]), key([]), key([1])]); + ..sorted.deepEquals([key([1, 2]), key([]), key([1])]) + ..latestMessagesByRecipient.deepEquals({1: 300, 2: 300}); }); group('message event (new message)', () { @@ -55,7 +57,8 @@ void main() { key([1]): 200, key([1, 2]): 100, }) - ..sorted.deepEquals([key([1]), key([1, 2])]); + ..sorted.deepEquals([key([1]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 200, 2: 100}); }); test('stream message -> do nothing', () { @@ -65,7 +68,8 @@ void main() { ..addListener(() { listenersNotified = true; }) ..handleMessageEvent(MessageEvent(id: 1, message: eg.streamMessage())) ) ..map.deepEquals(expected.map) - ..sorted.deepEquals(expected.sorted); + ..sorted.deepEquals(expected.sorted) + ..latestMessagesByRecipient.deepEquals(expected.latestMessagesByRecipient); check(listenersNotified).isFalse(); }); @@ -80,7 +84,8 @@ void main() { key([1]): 200, key([1, 2]): 100, }) - ..sorted.deepEquals([key([2]), key([1]), key([1, 2])]); + ..sorted.deepEquals([key([2]), key([1]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 200, 2: 300}); check(listenersNotified).isTrue(); }); @@ -95,7 +100,8 @@ void main() { key([2]): 150, key([1, 2]): 100, }) - ..sorted.deepEquals([key([1]), key([2]), key([1, 2])]); + ..sorted.deepEquals([key([1]), key([2]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 200, 2: 150}); check(listenersNotified).isTrue(); }); @@ -110,7 +116,8 @@ void main() { key([1, 2]): 300, key([1]): 200, }) - ..sorted.deepEquals([key([1, 2]), key([1])]); + ..sorted.deepEquals([key([1, 2]), key([1])]) + ..latestMessagesByRecipient.deepEquals({1: 300, 2: 300}); check(listenersNotified).isTrue(); }); @@ -124,7 +131,8 @@ void main() { key([1]): 300, key([1, 2]): 100, }) - ..sorted.deepEquals([key([1]), key([1, 2])]); + ..sorted.deepEquals([key([1]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 300, 2: 100}); check(listenersNotified).isTrue(); }); @@ -135,7 +143,42 @@ void main() { check(setupView() ..handleMessageEvent(MessageEvent(id: 1, message: message)) ) ..map.deepEquals(expected.map) - ..sorted.deepEquals(expected.sorted); + ..sorted.deepEquals(expected.sorted) + ..latestMessagesByRecipient.deepEquals(expected.latestMessagesByRecipient); + }); + + test('new conversation with one existing and one new user, newest message', () { + bool listenersNotified = false; + final message = eg.dmMessage(id: 300, from: eg.selfUser, + to: [eg.user(userId: 1), eg.user(userId: 3)]); + check(setupView() + ..addListener(() { listenersNotified = true; }) + ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ) ..map.deepEquals({ + key([1, 3]): 300, + key([1]): 200, + key([1, 2]): 100, + }) + ..sorted.deepEquals([key([1, 3]), key([1]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 300, 2: 100, 3: 300}); + check(listenersNotified).isTrue(); + }); + + test('new conversation with one existing and one new user, not newest message', () { + bool listenersNotified = false; + final message = eg.dmMessage(id: 150, from: eg.selfUser, + to: [eg.user(userId: 1), eg.user(userId: 3)]); + check(setupView() + ..addListener(() { listenersNotified = true; }) + ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ) ..map.deepEquals({ + key([1]): 200, + key([1, 3]): 150, + key([1, 2]): 100, + }) + ..sorted.deepEquals([key([1]), key([1, 3]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 200, 2: 100, 3: 150}); + check(listenersNotified).isTrue(); }); }); }); From 0621c0e92502d4d0c4f1b22b245f32ab9f983cd1 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Fri, 17 May 2024 00:04:39 +0430 Subject: [PATCH 02/11] autocomplete [nfc]: Refactor the code for refreshing stale user results --- lib/model/autocomplete.dart | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 9b8a81efd4..bc3575bc78 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -125,23 +125,26 @@ class AutocompleteViewManager { assert(removed); } - void handleRealmUserAddEvent(RealmUserAddEvent event) { + /// Recomputes the autocomplete results for users. + /// + /// Calls [MentionAutocompleteView.refreshStaleUserResults] for all that are registered. + void _refreshStaleUserResults() { for (final view in _mentionAutocompleteViews) { view.refreshStaleUserResults(); } } + void handleRealmUserAddEvent(RealmUserAddEvent event) { + _refreshStaleUserResults(); + } + void handleRealmUserRemoveEvent(RealmUserRemoveEvent event) { - for (final view in _mentionAutocompleteViews) { - view.refreshStaleUserResults(); - } + _refreshStaleUserResults(); autocompleteDataCache.invalidateUser(event.userId); } void handleRealmUserUpdateEvent(RealmUserUpdateEvent event) { - for (final view in _mentionAutocompleteViews) { - view.refreshStaleUserResults(); - } + _refreshStaleUserResults(); autocompleteDataCache.invalidateUser(event.userId); } From 2b825abb99ba2735925e11fce091dd6be58308db Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Fri, 17 May 2024 00:06:14 +0430 Subject: [PATCH 03/11] autocomplete [nfc]: Introduce a field for sorted users This field will be used to maintain a list of sorted users based on the most relevant autocomplete criteria in the upcoming commits. Co-authored-by: Greg Price --- lib/model/autocomplete.dart | 40 +++++++++++++++++++++++++++++++++---- lib/model/message_list.dart | 2 ++ lib/model/store.dart | 1 + 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index bc3575bc78..003ab6a3a3 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -148,6 +148,14 @@ class AutocompleteViewManager { autocompleteDataCache.invalidateUser(event.userId); } + void handleMessageEvent(MessageEvent event) { + _refreshStaleUserResults(); + } + + void handleOlderMessages() { + _refreshStaleUserResults(); + } + /// Called when the app is reassembled during debugging, e.g. for hot reload. /// /// Calls [MentionAutocompleteView.reassemble] for all that are registered. @@ -193,6 +201,7 @@ class MentionAutocompleteView extends ChangeNotifier { @override void dispose() { store.autocompleteViewManager.unregisterMentionAutocomplete(this); + _sortedUsers = null; // We cancel in-progress computations by checking [hasListeners] between tasks. // After [super.dispose] is called, [hasListeners] returns false. // TODO test that logic (may involve detecting an unhandled Future rejection; how?) @@ -216,6 +225,7 @@ class MentionAutocompleteView extends ChangeNotifier { /// Called in particular when we get a [RealmUserEvent]. void refreshStaleUserResults() { if (_query != null) { + _sortedUsers = null; _startSearch(_query!); } } @@ -225,6 +235,7 @@ class MentionAutocompleteView extends ChangeNotifier { /// This will redo the search from scratch for the current query, if any. void reassemble() { if (_query != null) { + _sortedUsers = null; _startSearch(_query!); } } @@ -254,22 +265,43 @@ class MentionAutocompleteView extends ChangeNotifier { notifyListeners(); } + List? _sortedUsers; + + List sortByRelevance({required List users}) { + return users; + } + + void _sortUsers() { + final users = store.users.values.toList(); + _sortedUsers = sortByRelevance(users: users); + } + Future?> _computeResults(MentionAutocompleteQuery query) async { final List results = []; - final Iterable users = store.users.values; - final iterator = users.iterator; + if (_sortedUsers == null) { + _sortUsers(); + } + + final sortedUsers = _sortedUsers!; + final iterator = sortedUsers.iterator; bool isDone = false; while (!isDone) { // CPU perf: End this task; enqueue a new one for resuming this work await Future(() {}); + if (_sortedUsers != sortedUsers) { + // The list of users this loop has been working from has become stale. + // Abort so _startSearch can retry with the new list. + throw ConcurrentModificationError(); + } + if (query != _query || !hasListeners) { // false if [dispose] has been called. return null; } for (int i = 0; i < 1000; i++) { - if (!iterator.moveNext()) { // Can throw ConcurrentModificationError + if (!iterator.moveNext()) { isDone = true; break; } @@ -280,7 +312,7 @@ class MentionAutocompleteView extends ChangeNotifier { } } } - return results; // TODO(#228) sort for most relevant first + return results; } } diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index f0d48368d0..77798e4467 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -417,6 +417,8 @@ class MessageListView with ChangeNotifier, _MessageSequence { ? result.messages // Avoid unnecessarily copying the list. : result.messages.where(_messageVisible); + store.autocompleteViewManager.handleOlderMessages(); + _insertAllMessages(0, fetchedMessages); _haveOldest = result.foundOldest; } finally { diff --git a/lib/model/store.dart b/lib/model/store.dart index a5d288fe02..a516b879d6 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -437,6 +437,7 @@ class PerAccountStore extends ChangeNotifier with StreamStore { } else if (event is MessageEvent) { assert(debugLog("server event: message ${jsonEncode(event.message.toJson())}")); recentDmConversationsView.handleMessageEvent(event); + autocompleteViewManager.handleMessageEvent(event); for (final view in _messageListViews) { view.maybeAddMessage(event.message); } From 09d33518247e30e3879bbdd7069526e710e52fc5 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 10:59:20 +0430 Subject: [PATCH 04/11] autocomplete: Add "recent DM conversations" criterion In @-mention autocomplete, users are suggested based on: 1. Recent DM conversations. Fixes part of #228 --- lib/model/autocomplete.dart | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 003ab6a3a3..01d794e24c 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -267,13 +267,34 @@ class MentionAutocompleteView extends ChangeNotifier { List? _sortedUsers; - List sortByRelevance({required List users}) { + int compareByRelevance({ + required User userA, + required User userB, + }) { + final dmPrecedence = store.recentDmConversationsView.compareByDms(userA, userB); + return dmPrecedence; + } + + List sortByRelevance({ + required List users, + required Narrow narrow, + }) { + switch (narrow) { + case StreamNarrow(): + case TopicNarrow(): + case DmNarrow(): + users.sort((userA, userB) => compareByRelevance( + userA: userA, + userB: userB)); + case AllMessagesNarrow(): + // do nothing in this case for now + } return users; } void _sortUsers() { final users = store.users.values.toList(); - _sortedUsers = sortByRelevance(users: users); + _sortedUsers = sortByRelevance(users: users, narrow: narrow); } Future?> _computeResults(MentionAutocompleteQuery query) async { From 4c93f1726eca231040fe3c30a7459aa2bb467447 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Mon, 13 May 2024 23:36:02 +0430 Subject: [PATCH 05/11] recent-senders: Add the new `MessageIdTracker` and `RecentSenders` data structures These data structures are used to keep track of user messages in topics and streams. --- lib/model/recent_senders.dart | 156 ++++++++++++++++++++++++++++ test/model/recent_senders_test.dart | 129 +++++++++++++++++++++++ 2 files changed, 285 insertions(+) create mode 100644 lib/model/recent_senders.dart create mode 100644 test/model/recent_senders_test.dart diff --git a/lib/model/recent_senders.dart b/lib/model/recent_senders.dart new file mode 100644 index 0000000000..dbd4e3d739 --- /dev/null +++ b/lib/model/recent_senders.dart @@ -0,0 +1,156 @@ +import 'package:collection/collection.dart'; +import 'package:flutter/foundation.dart'; + +import '../api/model/model.dart'; + +class MessageIdTracker { + final List _ids = []; + + void add(int id) { + if (_ids.isEmpty) { + _ids.add(id); + } else { + int i = lowerBound(_ids, id); + if (i < _ids.length && _ids[i] == id) return; // the [id] already exists, so do not add it. + _ids.insert(i, id); + } + } + + // TODO: remove + + /// The maximum id in the tracker list. + /// + /// Returns -1 if the tracker list is empty. + int get maxId => _ids.isNotEmpty ? _ids.last : -1; + + /// Getter for the tracked message IDs. + /// + /// This is intended for testing purposes only. + List get idsForTesting => _ids; + + @override + bool operator == (covariant MessageIdTracker other) { + if (identical(this, other)) return true; + + return _ids.equals(other._ids); + } + + @override + int get hashCode => Object.hashAll(_ids); +} + +/// A data structure to keep track of stream and topic messages. +/// +/// The owner should call [clear] in order to free resources. +class RecentSenders { + // streamSenders[streamId][senderId] = IdTracker + final Map> _streamSenders = {}; + + // topicSenders[streamId][topic][senderId] = IdTracker + final Map>> _topicSenders = {}; + + /// Whether stream senders and topic senders are both empty. + @visibleForTesting + bool get debugIsEmpty => _streamSenders.isEmpty && _topicSenders.isEmpty; + + /// Whether stream senders and topic senders are both not empty. + @visibleForTesting + bool get debugIsNotEmpty => _streamSenders.isNotEmpty && _topicSenders.isNotEmpty; + + void clear() { + _streamSenders.clear(); + _topicSenders.clear(); + } + + int _latestMessageIdOfSenderInStream({required int streamId, required int senderId}) { + return _streamSenders[streamId]?[senderId]?.maxId ?? -1; + } + + int _latestMessageIdOfSenderInTopic({ + required int streamId, + required String topic, + required int senderId, + }) { + return _topicSenders[streamId]?[topic]?[senderId]?.maxId ?? -1; + } + + void _addMessageInStream({ + required int streamId, + required int senderId, + required int messageId, + }) { + final sendersMap = _streamSenders[streamId] ??= {}; + final idTracker = sendersMap[senderId] ??= MessageIdTracker(); + idTracker.add(messageId); + } + + void _addMessageInTopic({ + required int streamId, + required String topic, + required int senderId, + required int messageId, + }) { + final topicsMap = _topicSenders[streamId] ??= {}; + final sendersMap = topicsMap[topic] ??= {}; + final idTracker = sendersMap[senderId] ??= MessageIdTracker(); + idTracker.add(messageId); + } + + /// Extracts and keeps track of the necessary data from a [message] only + /// if it is a stream message. + void handleMessage(Message message) { + if (message is! StreamMessage) { + return; + } + + final streamId = message.streamId; + final topic = message.subject; + final senderId = message.senderId; + final messageId = message.id; + + _addMessageInStream( + streamId: streamId, senderId: senderId, messageId: messageId); + _addMessageInTopic( + streamId: streamId, + topic: topic, + senderId: senderId, + messageId: messageId); + } + + // TODO: removeMessageInTopic + + /// Determines which of the two users has more recent activity. + /// + /// First checks for the activity in [topic] if provided. + /// + /// If no [topic] is provided, or the activity in the topic is the same (which + /// is extremely rare) or there is no activity in the topic at all, then + /// checks for the activity in the stream with [streamId]. + /// + /// Returns a negative number if [userA] has more recent activity than [userB], + /// returns a positive number if [userB] has more recent activity than [userA], + /// and returns `0` if both [userA] and [userB] have the same recent activity + /// (which is extremely rare) or has no activity at all. + int compareByRecency( + User userA, + User userB, { + required int streamId, + required String? topic, + }) { + if (topic != null) { + final aMessageId = _latestMessageIdOfSenderInTopic( + streamId: streamId, topic: topic, senderId: userA.userId); + final bMessageId = _latestMessageIdOfSenderInTopic( + streamId: streamId, topic: topic, senderId: userB.userId); + + final result = bMessageId.compareTo(aMessageId); + if (result != 0) return result; + } + + final aMessageId = + _latestMessageIdOfSenderInStream(streamId: streamId, senderId: userA.userId); + final bMessageId = + _latestMessageIdOfSenderInStream(streamId: streamId, senderId: userB.userId); + return bMessageId.compareTo(aMessageId); + } +} diff --git a/test/model/recent_senders_test.dart b/test/model/recent_senders_test.dart new file mode 100644 index 0000000000..191bf6d536 --- /dev/null +++ b/test/model/recent_senders_test.dart @@ -0,0 +1,129 @@ +import 'package:checks/checks.dart'; +import 'package:test/scaffolding.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/recent_senders.dart'; +import '../example_data.dart' as eg; + +void main() { + group('MessageIdTracker', () { + late MessageIdTracker idTracker; + + void prepare() { + idTracker = MessageIdTracker(); + } + + test('starts with no ids', () { + prepare(); + check(idTracker.idsForTesting).isEmpty(); + }); + + test('calling add(id) adds the same id to the tracker just one time', () { + prepare(); + check(idTracker.idsForTesting).isEmpty(); + idTracker.add(1); + idTracker.add(1); + check(idTracker.idsForTesting.singleOrNull).equals(1); + }); + + test('ids are sorted ascendingly, with maxId pointing to the last element', () { + prepare(); + check(idTracker.idsForTesting).isEmpty(); + idTracker.add(1); + idTracker.add(9); + idTracker.add(-1); + idTracker.add(5); + check(idTracker.idsForTesting).deepEquals([-1, 1, 5, 9]); + check(idTracker.maxId).equals(idTracker.idsForTesting.last); + }); + }); + + group('RecentSenders', () { + late RecentSenders recentSenders; + + void prepare() { + recentSenders = RecentSenders(); + } + + test('starts with no stream or topic senders', () { + prepare(); + check(recentSenders.debugIsEmpty).equals(true); + }); + + test('only processes a stream message', () { + prepare(); + final dmMessage = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]); + recentSenders.handleMessage(dmMessage); + check(recentSenders.debugIsEmpty).equals(true); + + final streamMessage = eg.streamMessage(); + recentSenders.handleMessage(streamMessage); + check(recentSenders.debugIsNotEmpty).equals(true); + }); + + group('compareByRecency', () { + final userA = eg.otherUser; + final userB = eg.thirdUser; + final stream = eg.stream(); + const topic1 = 'topic1'; + const topic2 = 'topic2'; + + void fillWithMessages(List messages) { + for (final message in messages) { + recentSenders.handleMessage(message); + } + } + + /// Determines the priority between [userA] and [userB] based on their activity. + /// + /// The activity is first looked for in [topic] then in [stream]. + /// + /// Returns a negative number if [userA] has more recent activity, + /// returns a positive number if [userB] has more recent activity, and + /// returns `0` if the activity is the same or there is no activity at all. + int priority({required String? topic}) { + return recentSenders.compareByRecency( + userA, + userB, + streamId: stream.streamId, + topic: topic, + ); + } + + test('prioritizes the user with more recent activity in the topic', () { + final userAMessage = eg.streamMessage(sender: userA, stream: stream, topic: topic1); + final userBMessage = eg.streamMessage(sender: userB, stream: stream, topic: topic1); + prepare(); + fillWithMessages([userAMessage, userBMessage]); + final priorityInTopic1 = priority(topic: topic1); + check(priorityInTopic1).isGreaterThan(0); // [userB] is more recent in topic1. + }); + + test('prioritizes the user with more recent activity in the stream ' + 'if there is no activity in the topic from both users', () { + final userAMessage = eg.streamMessage(sender: userA, stream: stream, topic: topic1); + final userBMessage = eg.streamMessage(sender: userB, stream: stream, topic: topic1); + prepare(); + fillWithMessages([userAMessage, userBMessage]); + final priorityInTopic2 = priority(topic: topic2); + check(priorityInTopic2).isGreaterThan(0); // [userB] is more recent in the stream. + }); + + test('prioritizes the user with more recent activity in the stream ' + 'if there is no topic provided', () { + final userAMessage = eg.streamMessage(sender: userA, stream: stream, topic: topic1); + final userBMessage = eg.streamMessage(sender: userB, stream: stream, topic: topic2); + prepare(); + fillWithMessages([userAMessage, userBMessage]); + final priorityInStream = priority(topic: null); + check(priorityInStream).isGreaterThan(0); // [userB] is more recent in the stream. + }); + + test('prioritizes none of the users if there is no activity in the stream from both users', () { + prepare(); + fillWithMessages([]); + final priorityInStream = priority(topic: null); + check(priorityInStream).equals(0); // none of the users has activity in the stream. + }); + }); + }); +} From 35b392885e9a3c6505041bb225f4d6d26cd608d4 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 10:38:54 +0430 Subject: [PATCH 06/11] store: Add `RecentSenders` data structure to `store.dart` --- lib/model/store.dart | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/model/store.dart b/lib/model/store.dart index a516b879d6..dd6a3fb294 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -21,6 +21,7 @@ import 'autocomplete.dart'; import 'database.dart'; import 'message_list.dart'; import 'recent_dm_conversations.dart'; +import 'recent_senders.dart'; import 'stream.dart'; import 'unreads.dart'; @@ -287,6 +288,8 @@ class PerAccountStore extends ChangeNotifier with StreamStore { final Map users; + final RecentSenders recentSenders = RecentSenders(); + //////////////////////////////// // Streams, topics, and stuff about them. @@ -347,6 +350,7 @@ class PerAccountStore extends ChangeNotifier with StreamStore { void dispose() { unreads.dispose(); recentDmConversationsView.dispose(); + recentSenders.clear(); for (final view in _messageListViews.toList()) { view.dispose(); } @@ -436,6 +440,7 @@ class PerAccountStore extends ChangeNotifier with StreamStore { notifyListeners(); } else if (event is MessageEvent) { assert(debugLog("server event: message ${jsonEncode(event.message.toJson())}")); + recentSenders.handleMessage(event.message); recentDmConversationsView.handleMessageEvent(event); autocompleteViewManager.handleMessageEvent(event); for (final view in _messageListViews) { From 2996ce5112bf7bbd57c0ccb7eabec2cb4822dc70 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Thu, 16 May 2024 22:04:44 +0430 Subject: [PATCH 07/11] autocomplete: Add "recent activity in current stream" criterion In @-mention autocomplete, users are suggested based on: 1. Recent activity in the current stream. 2. Recent DM conversations. --- lib/model/autocomplete.dart | 26 +++++++++++++++++++++++++- lib/model/message_list.dart | 4 ++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 01d794e24c..a7f272f816 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -270,7 +270,19 @@ class MentionAutocompleteView extends ChangeNotifier { int compareByRelevance({ required User userA, required User userB, + required int? streamId, + required String? topic, }) { + if (streamId != null) { + final conversationPrecedence = store.recentSenders.compareByRecency( + userA, + userB, + streamId: streamId, + topic: topic); + if (conversationPrecedence != 0) { + return conversationPrecedence; + } + } final dmPrecedence = store.recentDmConversationsView.compareByDms(userA, userB); return dmPrecedence; } @@ -281,11 +293,23 @@ class MentionAutocompleteView extends ChangeNotifier { }) { switch (narrow) { case StreamNarrow(): + users.sort((userA, userB) => compareByRelevance( + userA: userA, + userB: userB, + streamId: narrow.streamId, + topic: null)); case TopicNarrow(): + users.sort((userA, userB) => compareByRelevance( + userA: userA, + userB: userB, + streamId: narrow.streamId, + topic: narrow.topic)); case DmNarrow(): users.sort((userA, userB) => compareByRelevance( userA: userA, - userB: userB)); + userB: userB, + streamId: null, + topic: null)); case AllMessagesNarrow(): // do nothing in this case for now } diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 77798e4467..e538b90ded 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -381,6 +381,7 @@ class MessageListView with ChangeNotifier, _MessageSequence { for (final message in result.messages) { if (_messageVisible(message)) { _addMessage(message); + store.recentSenders.handleMessage(message); } } _fetched = true; @@ -417,6 +418,9 @@ class MessageListView with ChangeNotifier, _MessageSequence { ? result.messages // Avoid unnecessarily copying the list. : result.messages.where(_messageVisible); + for (final message in fetchedMessages) { + store.recentSenders.handleMessage(message); + } store.autocompleteViewManager.handleOlderMessages(); _insertAllMessages(0, fetchedMessages); From cc715e6da9e45952297b0187f0180b936af85375 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 11:02:29 +0430 Subject: [PATCH 08/11] autocomplete: Add "human vs. bot users" criterion In @-mention autocomplete, users are suggested based on: 1. Recent activity in the current stream. 2. Recent DM conversations. 3. Human vs. Bot users. --- lib/model/autocomplete.dart | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index a7f272f816..224fcc6ff0 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -284,7 +284,16 @@ class MentionAutocompleteView extends ChangeNotifier { } } final dmPrecedence = store.recentDmConversationsView.compareByDms(userA, userB); - return dmPrecedence; + if (dmPrecedence != 0) { + return dmPrecedence; + } + + if (!userA.isBot && userB.isBot) { + return -1; + } else if (userA.isBot && !userB.isBot) { + return 1; + } + return 0; } List sortByRelevance({ From 54489935e462efe883277b30550b277f2c3b7b37 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 11:07:04 +0430 Subject: [PATCH 09/11] autocomplete [nfc]: Support caching normalized user names in `AutocompleteDataCache` --- lib/model/autocomplete.dart | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 224fcc6ff0..504b59da12 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -426,13 +426,21 @@ class MentionAutocompleteQuery { } class AutocompleteDataCache { + final Map _normalizedNamesByUser = {}; + + /// The lowercase `fullName` of [user]. + String normalizedNameForUser(User user) { + return _normalizedNamesByUser[user.userId] ??= user.fullName.toLowerCase(); + } + final Map> _nameWordsByUser = {}; List nameWordsForUser(User user) { - return _nameWordsByUser[user.userId] ??= user.fullName.toLowerCase().split(' '); + return _nameWordsByUser[user.userId] ??= normalizedNameForUser(user).split(' '); } void invalidateUser(int userId) { + _normalizedNamesByUser.remove(userId); _nameWordsByUser.remove(userId); } } From 2b5711be1984efc3f792c39505abeb4e3a2e1b99 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 11:09:00 +0430 Subject: [PATCH 10/11] autocomplete: Add "alphabetical order" criterion In @-mention autocomplete, users are suggested based on: 1. Recent activity in the current stream. 2. Recent DM conversations. 3. Human vs. Bot users. 4. Alphabetical order. Fixes: #228 --- lib/model/autocomplete.dart | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 504b59da12..6443e8e393 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -293,7 +293,12 @@ class MentionAutocompleteView extends ChangeNotifier { } else if (userA.isBot && !userB.isBot) { return 1; } - return 0; + + final userAName = store.autocompleteViewManager.autocompleteDataCache + .normalizedNameForUser(userA); + final userBName = store.autocompleteViewManager.autocompleteDataCache + .normalizedNameForUser(userB); + return userAName.compareTo(userBName); } List sortByRelevance({ From c710a5c33ad8bae3d8e6e155effc4f1212655a26 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 11:10:58 +0430 Subject: [PATCH 11/11] autocomplete [nfc]: Add TODO comments for future autocomplete criteria --- lib/model/autocomplete.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 6443e8e393..658327d60c 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -273,6 +273,10 @@ class MentionAutocompleteView extends ChangeNotifier { required int? streamId, required String? topic, }) { + // TODO(#234): give preference to "all", "everyone" or "stream". + + // TODO(#618): give preference to subscribed users first. + if (streamId != null) { final conversationPrecedence = store.recentSenders.compareByRecency( userA,