msglist: Support outbox messages in message list

This adds some overhead in magnitude of O(1) (where the constant is
the number of outbox messages in a view, expected to be small) on
message event handling.

We add outboxMessages as a list independent from messages on
_MessageSequence.  Because outbox messages are not rendered
(the raw content is shown as plain text), we leave the 1-1
relationship between `messages` and `contents` unchanged.

When computing `items`, we now start to look at `outboxMessages` as
well, with the guarantee that the items related to outbox messages
always come after those for other messages.  Look for places that call
`_processOutboxMessage(int index)` for references, and the changes to
`checkInvariants` on how this affects the message list invariants.

`addOutboxMessage` is similar to `handleMessage`.  However, outbox
messages do not rely on the fetched state, i.e. they can be
synchronously updated when the message list view was first
initialized.

Author: PIG208

lib/model/message.dart

@@ -542,6 +542,8 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes
       }
     }
 
+    // TODO predict outbox message moves using propagateMode
+
     for (final view in _messageListViews) {
       view.messagesMoved(messageMove: messageMove, messageIds: event.messageIds);
     }

lib/model/message_list.dart

@@ -63,6 +63,21 @@ class MessageListMessageItem extends MessageListMessageBaseItem {
   });
 }
 
+class MessageListOutboxMessageItem extends MessageListMessageBaseItem {
+  @override
+  final OutboxMessage message;
+  @override
+  final ZulipContent content;
+
+  MessageListOutboxMessageItem(
+    this.message, {
+    required super.showSender,
+    required super.isLastInBlock,
+  }) : content = ZulipContent(nodes: [
+    ParagraphNode(links: [], nodes: [TextNode(message.content)]),
+  ]);
+}
+
 /// Indicates the app is loading more messages at the top.
 // TODO(#80): or loading at the bottom, by adding a [MessageListDirection.newer]
 class MessageListLoadingItem extends MessageListItem {
@@ -90,7 +105,16 @@ mixin _MessageSequence {
   /// See also [contents] and [items].
   final List<Message> messages = [];
 
-  /// Whether [messages] and [items] represent the results of a fetch.
+  /// The messages sent by the self-user.
+  ///
+  /// See also [items].
+  ///
+  /// Usually this should not have that many items, so we do not anticipate
+  /// performance issues with unoptimized O(N) iterations through this list.
+  final List<OutboxMessage> outboxMessages = [];
+
+  /// Whether [messages], [outboxMessages], and [items] represent the results
+  /// of a fetch.
   ///
   /// This allows the UI to distinguish "still working on fetching messages"
   /// from "there are in fact no messages here".
@@ -142,11 +166,12 @@ mixin _MessageSequence {
   /// The messages and their siblings in the UI, in order.
   ///
   /// This has a [MessageListMessageItem] corresponding to each element
-  /// of [messages], in order.  It may have additional items interspersed
-  /// before, between, or after the messages.
+  /// of [messages], followed by each element in [outboxMessages] in order.
+  /// It may have additional items interspersed before, between, or after the
+  /// messages.
   ///
-  /// This information is completely derived from [messages] and
-  /// the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown].
+  /// This information is completely derived from [messages], [outboxMessages]
+  /// and the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown].
   /// It exists as an optimization, to memoize that computation.
   final QueueList<MessageListItem> items = QueueList();
 
@@ -168,9 +193,10 @@ mixin _MessageSequence {
         }
       case MessageListRecipientHeaderItem(:var message):
       case MessageListDateSeparatorItem(:var message):
-        if (message.id == null)                  return 1;  // TODO(#1441): test
+        if (message.id == null)                  return 1;
         return message.id! <= messageId ? -1 : 1;
       case MessageListMessageItem(:var message): return message.id.compareTo(messageId);
+      case MessageListOutboxMessageItem():       return 1;
     }
   }
 
@@ -274,10 +300,46 @@ mixin _MessageSequence {
     _reprocessAll();
   }
 
+  /// Append [outboxMessage] to [outboxMessages], and update derived data
+  /// accordingly.
+  ///
+  /// The caller is responsible for ensuring this is an appropriate thing to do
+  /// given [narrow], our state of being caught up, and other concerns.
+  void _addOutboxMessage(OutboxMessage outboxMessage) {
+    assert(!outboxMessages.contains(outboxMessage));
+    outboxMessages.add(outboxMessage);
+    _processOutboxMessage(outboxMessages.length - 1);
+  }
+
+  /// Remove the [outboxMessage] from the view.
+  ///
+  /// Returns true if the outbox message was removed, false otherwise.
+  bool _removeOutboxMessage(OutboxMessage outboxMessage) {
+    if (!outboxMessages.remove(outboxMessage)) {
+      return false;
+    }
+    _reprocessOutboxMessages();
+    return true;
+  }
+
+  /// Remove all outbox messages that satisfy [test] from [outboxMessages].
+  ///
+  /// Returns true if any outbox messages were removed, false otherwise.
+  bool _removeOutboxMessagesWhere(bool Function(OutboxMessage) test) {
+    final count = outboxMessages.length;
+    outboxMessages.removeWhere(test);
+    if (outboxMessages.length == count) {
+      return false;
+    }
+    _reprocessOutboxMessages();
+    return true;
+  }
+
   /// Reset all [_MessageSequence] data, and cancel any active fetches.
   void _reset() {
     generation += 1;
     messages.clear();
+    outboxMessages.clear();
     _fetched = false;
     _haveOldest = false;
     _fetchingOlder = false;
@@ -301,7 +363,8 @@ mixin _MessageSequence {
   ///
   /// Returns whether an item has been appended or not.
   ///
-  /// The caller must append a [MessageListMessageBaseItem] after this.
+  /// The caller must append a [MessageListMessageBaseItem] for [message]
+  /// after this.
   bool _maybeAppendAuxillaryItem(MessageBase message, {
     required MessageBase? prevMessage,
   }) {
@@ -328,6 +391,7 @@ mixin _MessageSequence {
   /// The previous messages in the list must already have been processed.
   /// This message must already have been parsed and reflected in [contents].
   void _processMessage(int index) {
+    assert(items.lastOrNull is! MessageListOutboxMessageItem);
     final prevMessage = index == 0 ? null : messages[index - 1];
     final message = messages[index];
     final content = contents[index];
@@ -338,6 +402,20 @@ mixin _MessageSequence {
       isLastInBlock: true));
   }
 
+  /// Append to [items] based on the index-th outbox message.
+  ///
+  /// All [messages] and previous messages in [outboxMessages] must already have
+  /// been processed.
+  void _processOutboxMessage(int index) {
+    final prevMessage = index == 0 ? messages.lastOrNull : outboxMessages[index - 1];
+    final message = outboxMessages[index];
+
+    final appended = _maybeAppendAuxillaryItem(message, prevMessage: prevMessage);
+    items.add(MessageListOutboxMessageItem(message,
+      showSender: appended || prevMessage?.senderId != message.senderId,
+      isLastInBlock: true));
+  }
+
   /// Update [items] to include markers at start and end as appropriate.
   void _updateEndMarkers() {
     assert(fetched);
@@ -362,12 +440,55 @@ mixin _MessageSequence {
     }
   }
 
-  /// Recompute [items] from scratch, based on [messages], [contents], and flags.
+  /// Remove items associated with [outboxMessages] from [items].
+  ///
+  /// This is designed to be idempotent; repeated calls will not change the
+  /// content of [items].
+  ///
+  /// This is efficient due to the expected small size of [outboxMessages].
+  void _removeOutboxMessageItems() {
+    // This loop relies on the assumption that all items that follow
+    // the last [MessageListMessageItem] are derived from outbox messages.
+    // If there is no [MessageListMessageItem] at all,
+    // this will end up removing end markers.
+    while (items.isNotEmpty && items.last is! MessageListMessageItem) {
+      items.removeLast();
+    }
+    assert(items.none((e) => e is MessageListOutboxMessageItem));
+
+    if (items.isNotEmpty) {
+      final lastItem = items.last as MessageListMessageItem;
+      lastItem.isLastInBlock = true;
+    }
+
+    if (fetched) {
+      // Restore the end markers in case they were removed; only do so when
+      // [fetched] is true, since the markers are not there otherwise.
+      _updateEndMarkers();
+    }
+  }
+
+  /// Recompute the portion of [items] derived from outbox messages,
+  /// based on [outboxMessages] and [messages].
+  ///
+  /// All [messages] should have been processed when this is called.
+  void _reprocessOutboxMessages() {
+    _removeOutboxMessageItems();
+    for (var i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
+  }
+
+  /// Recompute [items] from scratch, based on [messages], [contents],
+  /// [outboxMessages] and flags.
   void _reprocessAll() {
     items.clear();
     for (var i = 0; i < messages.length; i++) {
       _processMessage(i);
     }
+    for (var i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
     _updateEndMarkers();
   }
 }
@@ -412,7 +533,9 @@ class MessageListView with ChangeNotifier, _MessageSequence {
 
   factory MessageListView.init(
       {required PerAccountStore store, required Narrow narrow}) {
-    final view = MessageListView._(store: store, narrow: narrow);
+    final view = MessageListView._(store: store, narrow: narrow)
+      .._syncOutboxMessages()
+      .._reprocessOutboxMessages();
     store.registerMessageList(view);
     return view;
   }
@@ -510,11 +633,13 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     _adjustNarrowForTopicPermalink(result.messages.firstOrNull);
     store.reconcileMessages(result.messages);
     store.recentSenders.handleMessages(result.messages); // TODO(#824)
+    _removeOutboxMessageItems();
     for (final message in result.messages) {
       if (_messageVisible(message)) {
         _addMessage(message);
       }
     }
+    _reprocessOutboxMessages();
     _fetched = true;
     _haveOldest = result.foundOldest;
     _updateEndMarkers();
@@ -621,16 +746,51 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
   }
 
+  bool _shouldAddOutboxMessage(OutboxMessage outboxMessage, {
+    bool wasUnmuted = false,
+  }) {
+    return !outboxMessage.hidden
+      && narrow.containsMessage(outboxMessage)
+      && (_messageVisible(outboxMessage) || wasUnmuted);
+  }
+
+  /// Copy outbox messages from the store, keeping the ones belong to the view.
+  ///
+  /// This does not recompute [items].  The caller is expected to call
+  /// [_reprocessOutboxMessages] later to keep [items] up-to-date.
+  ///
+  /// This assumes that [outboxMessages] is empty.
+  void _syncOutboxMessages() {
+    assert(outboxMessages.isEmpty);
+    for (final outboxMessage in store.outboxMessages.values) {
+      if (_shouldAddOutboxMessage(outboxMessage)) {
+        outboxMessages.add(outboxMessage);
+      }
+    }
+  }
+
   /// Add [outboxMessage] if it belongs to the view.
   void addOutboxMessage(OutboxMessage outboxMessage) {
-    // TODO(#1441) implement this
+    assert(outboxMessages.none(
+      (message) => message.localMessageId == outboxMessage.localMessageId));
+    if (_shouldAddOutboxMessage(outboxMessage)) {
+      _addOutboxMessage(outboxMessage);
+      if (fetched) {
+        // Only need to notify listeners when [fetched] is true, because
+        // otherwise the message list just shows a loading indicator with
+        // no other items.
+        notifyListeners();
+      }
+    }
   }
 
   /// Remove the [outboxMessage] from the view.
   ///
   /// This is a no-op if the message is not found.
   void removeOutboxMessage(OutboxMessage outboxMessage) {
-    // TODO(#1441) implement this
+    if (_removeOutboxMessage(outboxMessage)) {
+      notifyListeners();
+    }
   }
 
   void handleUserTopicEvent(UserTopicEvent event) {
@@ -639,10 +799,17 @@ class MessageListView with ChangeNotifier, _MessageSequence {
         return;
 
       case VisibilityEffect.muted:
-        if (_removeMessagesWhere((message) =>
-            (message is StreamMessage
-             && message.streamId == event.streamId
-             && message.topic == event.topicName))) {
+        bool removed = _removeOutboxMessagesWhere((message) =>
+          message is StreamOutboxMessage
+          && message.conversation.streamId == event.streamId
+          && message.conversation.topic == event.topicName);
+
+        removed |= _removeMessagesWhere((message) =>
+          message is StreamMessage
+          && message.streamId == event.streamId
+          && message.topic == event.topicName);
+
+        if (removed) {
           notifyListeners();
         }
 
@@ -655,6 +822,18 @@ class MessageListView with ChangeNotifier, _MessageSequence {
           notifyListeners();
           fetchInitial();
         }
+
+        outboxMessages.clear();
+        for (final outboxMessage in store.outboxMessages.values) {
+          if (_shouldAddOutboxMessage(
+            outboxMessage,
+            wasUnmuted: outboxMessage is StreamOutboxMessage
+              && outboxMessage.conversation.streamId == event.streamId
+              && outboxMessage.conversation.topic == event.topicName,
+          )) {
+            outboxMessages.add(outboxMessage);
+          }
+        }
     }
   }
 
@@ -668,14 +847,27 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   void handleMessageEvent(MessageEvent event) {
     final message = event.message;
     if (!narrow.containsMessage(message) || !_messageVisible(message)) {
+      assert(event.localMessageId == null || outboxMessages.none((message) =>
+        message.localMessageId == int.parse(event.localMessageId!, radix: 10)));
       return;
     }
     if (!_fetched) {
       // TODO mitigate this fetch/event race: save message to add to list later
       return;
     }
+    // We always remove all outbox message items
+    // to ensure that message items come before them.
+    _removeOutboxMessageItems();
     // TODO insert in middle instead, when appropriate
     _addMessage(message);
+    if (event.localMessageId != null) {
+      final localMessageId = int.parse(event.localMessageId!);
+      // [outboxMessages] is epxected to be short, so removing the corresponding
+      // outbox message and reprocessing them all in linear time is efficient.
+      outboxMessages.removeWhere(
+        (message) => message.localMessageId == localMessageId);
+    }
+    _reprocessOutboxMessages();
     notifyListeners();
   }
 
@@ -707,6 +899,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     // TODO in cases where we do have data to do better, do better.
     _reset();
     notifyListeners();
+    _syncOutboxMessages();
     fetchInitial();
   }
 
@@ -722,6 +915,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
       case PropagateMode.changeLater:
         narrow = newNarrow;
         _reset();
+        _syncOutboxMessages();
         fetchInitial();
       case PropagateMode.changeOne:
     }
@@ -796,7 +990,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
 
   /// Notify listeners if the given outbox message is present in this view.
   void notifyListenersIfOutboxMessagePresent(int localMessageId) {
-    // TODO(#1441) implement this
+    final isAnyPresent =
+      outboxMessages.any((message) => message.localMessageId == localMessageId);
+    if (isAnyPresent) {
+      notifyListeners();
+    }
   }
 
   /// Called when the app is reassembled during debugging, e.g. for hot reload.