recent_senders: Handle moved messages

This is similar to how we add support to handling moves for unreads
(commit 34e6201), also with optimizations to avoid making unnecessary
copies of message IDs when the entire conversation is moved (e.g.
resolve/unresolve topic).

An alternative approach to this that we didn't take is extracting
helpers from handleMessages and handleDeleteMessageEvent and combining
the two to handle moves, like web does
(https://github.com/zulip/zulip/blob/bd04a30b/web/src/recent_senders.ts#L165-L190),
but creating a dedicated helper makes it more sraightforward to
optimize for our performance need.

Fixes: zulip#901

Author: PIG208

lib/model/recent_senders.dart

@@ -68,6 +68,45 @@ class RecentSenders {
       [senderId] ??= MessageIdTracker()).add(messageId);
   }
 
+  void handleUpdateMessageEvent(UpdateMessageEvent event, Map<int, Message> cachedMessages) {
+    if (event.moveData == null) {
+      // No move; nothing to do.
+      return;
+    }
+    final UpdateMessageMoveData(
+      :origStreamId, :newStreamId, :origTopic, :newTopic) = event.moveData!;
+
+    final messagesBySender = _groupStreamMessageIdsBySender(event.messageIds, cachedMessages);
+    final sendersInChannel = streamSenders[origStreamId];
+    final topicsInChannel = topicSenders[origStreamId];
+    final sendersInTopic = topicsInChannel?[origTopic];
+    for (final MapEntry(key: senderId, value: messages) in messagesBySender.entries) {
+      if (newStreamId != origStreamId) {
+        final streamTracker = sendersInChannel?[senderId];
+        final movedMessagesInStreamTracker = streamTracker?.popAll(messages);
+        if (streamTracker?.maxId == null) sendersInChannel?.remove(senderId);
+        if (movedMessagesInStreamTracker != null) {
+          ((streamSenders[newStreamId] ??= {})
+            [senderId] ??= MessageIdTracker()).addAll(movedMessagesInStreamTracker);
+        }
+      }
+
+      // This is an invariant of [UpdateMessageMoveData].
+      assert(newStreamId != origStreamId || newTopic != origTopic);
+
+      final topicTracker = sendersInTopic?[senderId];
+      final movedMessagesInTopicTracker = topicTracker?.popAll(messages);
+      if (topicTracker?.maxId == null) sendersInTopic?.remove(senderId);
+      if (movedMessagesInTopicTracker != null) {
+        (((topicSenders[newStreamId] ??= {})[newTopic] ??= {})
+          [senderId] ??= MessageIdTracker()).addAll(movedMessagesInTopicTracker);
+      }
+    }
+    if (sendersInChannel?.isEmpty ?? false) streamSenders.remove(origStreamId);
+    if (sendersInTopic?.isEmpty ?? false) topicsInChannel?.remove(origTopic);
+    if (topicsInChannel?.isEmpty ?? false) topicSenders.remove(origStreamId);
+  }
+
   void handleDeleteMessageEvent(DeleteMessageEvent event, Map<int, Message> cachedMessages) {
     if (event.messageType != MessageType.stream) return;
 

lib/model/store.dart

@@ -754,6 +754,7 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, UserStore, Channel
 
       case UpdateMessageEvent():
         assert(debugLog("server event: update_message ${event.messageId}"));
+        recentSenders.handleUpdateMessageEvent(event, messages);
         _messages.handleUpdateMessageEvent(event);
         unreads.handleUpdateMessageEvent(event);
 

test/model/recent_senders_test.dart

@@ -2,7 +2,9 @@ import 'package:checks/checks.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/model/recent_senders.dart';
+import 'package:zulip/model/store.dart';
 import '../example_data.dart' as eg;
+import 'test_store.dart';
 
 /// [messages] should be sorted by [id] ascending.
 void checkMatchesMessages(RecentSenders model, List<Message> messages) {
@@ -142,6 +144,126 @@ void main() {
     });
   });
 
+  group('RecentSenders.handleUpdateMessageUpdate', () {
+    late PerAccountStore store;
+    late RecentSenders model;
+
+    final origChannel = eg.stream(); final newChannel = eg.stream();
+    final origTopic = 'origTopic';   final newTopic = 'newTopic';
+    final userX = eg.user();         final userY = eg.user();
+
+    Future<void> prepare(List<Message> messages) async {
+      store = eg.store();
+      await store.addMessages(messages);
+      await store.addStreams([origChannel, newChannel]);
+      await store.addUsers([userX, userY]);
+      model = store.recentSenders;
+    }
+
+    List<StreamMessage> copyMessagesWith(Iterable<StreamMessage> messages, {
+      ZulipStream? newChannel,
+      String? newTopic,
+    }) {
+      assert(newChannel != null || newTopic != null);
+      return messages.map((message) => StreamMessage.fromJson(
+        message.toJson()
+          ..['stream_id'] = newChannel?.streamId ?? message.streamId
+          // See [StreamMessage.displayRecipient] for why this is needed.
+          ..['display_recipient'] = newChannel?.name ?? message.displayRecipient!
+
+          ..['subject'] = newTopic ?? message.topic
+      )).toList();
+    }
+
+    test('move a conversation exactly', () async {
+      final messages = List.generate(10, (i) => eg.streamMessage(
+        stream: origChannel, topic: origTopic, sender: userX));
+      await prepare(messages);
+      final streamSenderIdsBefore = model.streamSenders
+        [origChannel.streamId]![userX.userId]!.ids;
+      final topicSenderIdsBefore = model.topicSenders
+        [origChannel.streamId]![TopicName(origTopic)]![userX.userId]!.ids;
+
+      await store.handleEvent(eg.updateMessageEventMoveFrom(
+        origMessages: messages,
+        newStreamId: newChannel.streamId,
+        newTopicStr: newTopic));
+      checkMatchesMessages(model, copyMessagesWith(
+        messages, newChannel: newChannel, newTopic: newTopic));
+      // Check we avoided creating a new list for the moved message IDs.
+      final streamSenderIdsAfter = model.streamSenders
+        [newChannel.streamId]![userX.userId]!.ids;
+      final topicSenderIdsAfter = model.topicSenders
+        [newChannel.streamId]![TopicName(newTopic)]![userX.userId]!.ids;
+      check(streamSenderIdsBefore).identicalTo(streamSenderIdsAfter);
+      check(topicSenderIdsBefore).identicalTo(topicSenderIdsAfter);
+    });
+
+    test('move a conversation partially to a different channel', () async {
+      final messages = List.generate(10, (i) => eg.streamMessage(
+        stream: origChannel, topic: origTopic));
+      final movedMessages = messages.take(5).toList();
+      final otherMessages = messages.skip(5);
+      await prepare(messages);
+
+      await store.handleEvent(eg.updateMessageEventMoveFrom(
+        origMessages: movedMessages,
+        newStreamId: newChannel.streamId));
+      checkMatchesMessages(model, [
+        ...copyMessagesWith(movedMessages, newChannel: newChannel),
+        ...otherMessages,
+      ]);
+    });
+
+    test('move a conversation partially to a different topic, within the same channel', () async {
+      final messages = List.generate(10, (i) => eg.streamMessage(
+        stream: origChannel, topic: origTopic, sender: userX));
+      final movedMessages = messages.take(5).toList();
+      final otherMessages = messages.skip(5);
+      await prepare(messages);
+      final streamSenderIdsBefore = model.streamSenders
+        [origChannel.streamId]![userX.userId]!.ids;
+
+      await store.handleEvent(eg.updateMessageEventMoveFrom(
+        origMessages: movedMessages,
+        newTopicStr: newTopic));
+      checkMatchesMessages(model, [
+        ...copyMessagesWith(movedMessages, newTopic: newTopic),
+        ...otherMessages,
+      ]);
+
+      // Check that we did not touch stream message IDs tracker
+      // when there wasn't a stream move.
+      final streamSenderIdsAfter = model.streamSenders
+        [origChannel.streamId]![userX.userId]!.ids;
+      check(streamSenderIdsBefore).identicalTo(streamSenderIdsAfter);
+    });
+
+    test('move a conversation with multiple senders', () async {
+      final messages = [
+        eg.streamMessage(stream: origChannel, topic: origTopic, sender: userX),
+        eg.streamMessage(stream: origChannel, topic: origTopic, sender: userX),
+        eg.streamMessage(stream: origChannel, topic: origTopic, sender: userY),
+      ];
+      await prepare(messages);
+
+      await store.handleEvent(eg.updateMessageEventMoveFrom(
+        origMessages: messages,
+        newStreamId: newChannel.streamId));
+      checkMatchesMessages(model, copyMessagesWith(
+        messages, newChannel: newChannel));
+    });
+
+    test('message edit update without move', () async {
+      final messages = List.generate(10, (i) => eg.streamMessage(
+        stream: origChannel, topic: origTopic));
+      await prepare(messages);
+
+      await store.handleEvent(eg.updateMessageEditEvent(messages[0]));
+      checkMatchesMessages(model, messages);
+    });
+  });
+
   test('RecentSenders.handleDeleteMessageEvent', () {
     final model = RecentSenders();
     final stream = eg.stream();