msglist [nfc]: Introduce haveNewest in model, always true for now

Author: gnprice

lib/model/message_list.dart

@@ -94,7 +94,7 @@ mixin _MessageSequence {
   ///
   /// This may or may not represent all the message history that
   /// conceptually belongs in this message list.
-  /// That information is expressed in [fetched] and [haveOldest].
+  /// That information is expressed in [fetched], [haveOldest], [haveNewest].
   ///
   /// See also [middleMessage], an index which divides this list
   /// into a top slice and a bottom slice.
@@ -121,11 +121,19 @@ mixin _MessageSequence {
 
   /// Whether we know we have the oldest messages for this narrow.
   ///
-  /// (Currently we always have the newest messages for the narrow,
-  /// once [fetched] is true, because we start from the newest.)
+  /// See also [haveNewest].
   bool get haveOldest => _haveOldest;
   bool _haveOldest = false;
 
+  /// Whether we know we have the newest messages for this narrow.
+  ///
+  /// (Currently this is always true once [fetched] is true,
+  /// because we start from the newest.)
+  ///
+  /// See also [haveOldest].
+  bool get haveNewest => _haveNewest;
+  bool _haveNewest = false;
+
   /// Whether this message list is currently busy when it comes to
   /// fetching more messages.
   ///
@@ -158,7 +166,7 @@ mixin _MessageSequence {
   /// before, between, or after the messages.
   ///
   /// This information is completely derived from [messages] and
-  /// the flags [haveOldest] and [busyFetchingMore].
+  /// the flags [haveOldest], [haveNewest], and [busyFetchingMore].
   /// It exists as an optimization, to memoize that computation.
   ///
   /// See also [middleItem], an index which divides this list
@@ -315,6 +323,7 @@ mixin _MessageSequence {
     messages.clear();
     middleMessage = 0;
     _haveOldest = false;
+    _haveNewest = false;
     _status = FetchingStatus.unstarted;
     _fetchBackoffMachine = null;
     contents.clear();
@@ -534,7 +543,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   Future<void> fetchInitial() async {
     // TODO(#80): fetch from anchor firstUnread, instead of newest
     // TODO(#82): fetch from a given message ID as anchor
-    assert(!fetched && !haveOldest && !busyFetchingMore);
+    assert(!fetched && !haveOldest && !haveNewest && !busyFetchingMore);
     assert(messages.isEmpty && contents.isEmpty);
     _setStatus(FetchingStatus.fetchInitial, was: FetchingStatus.unstarted);
     // TODO schedule all this in another isolate
@@ -561,6 +570,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
       // Now [middleMessage] is the last message (the one just added).
     }
     _haveOldest = result.foundOldest;
+    _haveNewest = true; // TODO(#82)
     _setStatus(FetchingStatus.idle, was: FetchingStatus.fetchInitial);
   }
 
@@ -715,8 +725,16 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     if (!narrow.containsMessage(message) || !_messageVisible(message)) {
       return;
     }
-    if (!fetched) {
-      // TODO mitigate this fetch/event race: save message to add to list later
+    if (!haveNewest) {
+      // This message list's [messages] doesn't yet reach the new end
+      // of the narrow's message history.  (Either [fetchInitial] hasn't yet
+      // completed, or if it has then it was in the middle of history and no
+      // subsequent fetch has reached the end.)
+      // So this still-newer message doesn't belong.
+      // Leave it to be found by a subsequent fetch when appropriate.
+      // TODO mitigate this fetch/event race: save message to add to list later,
+      //   in case the fetch that reaches the end is already ongoing and
+      //   didn't include this message.
       return;
     }
     // TODO insert in middle instead, when appropriate

test/model/message_list_test.dart

@@ -150,7 +150,8 @@ void main() {
         checkNotifiedOnce();
         check(model)
           ..messages.length.equals(kMessageListFetchBatchSize)
-          ..haveOldest.isFalse();
+          ..haveOldest.isFalse()
+          ..haveNewest.isTrue();
         checkLastRequest(
           narrow: narrow.apiEncode(),
           anchor: 'newest',
@@ -180,7 +181,8 @@ void main() {
       checkNotifiedOnce();
       check(model)
         ..messages.length.equals(30)
-        ..haveOldest.isTrue();
+        ..haveOldest.isTrue()
+        ..haveNewest.isTrue();
     });
 
     test('no messages found', () async {
@@ -194,7 +196,8 @@ void main() {
       check(model)
         ..fetched.isTrue()
         ..messages.isEmpty()
-        ..haveOldest.isTrue();
+        ..haveOldest.isTrue()
+        ..haveNewest.isTrue();
     });
 
     // TODO(#824): move this test
@@ -417,6 +420,10 @@ void main() {
       check(model).messages.length.equals(30);
     });
 
+    test('while in mid-history', () async {
+    }, skip: true, // TODO(#82): not yet possible to exercise this case
+    );
+
     test('before fetch', () async {
       final stream = eg.stream();
       await prepare(narrow: ChannelNarrow(stream.streamId));
@@ -2139,9 +2146,10 @@ void checkInvariants(MessageListView model) {
     check(model)
       ..messages.isEmpty()
       ..haveOldest.isFalse()
+      ..haveNewest.isFalse()
       ..busyFetchingMore.isFalse();
   }
-  if (model.haveOldest) {
+  if (model.haveOldest && model.haveNewest) {
     check(model).busyFetchingMore.isFalse();
   }
 
@@ -2286,5 +2294,6 @@ extension MessageListViewChecks on Subject<MessageListView> {
   Subject<int> get middleItem => has((x) => x.middleItem, 'middleItem');
   Subject<bool> get fetched => has((x) => x.fetched, 'fetched');
   Subject<bool> get haveOldest => has((x) => x.haveOldest, 'haveOldest');
+  Subject<bool> get haveNewest => has((x) => x.haveNewest, 'haveNewest');
   Subject<bool> get busyFetchingMore => has((x) => x.busyFetchingMore, 'busyFetchingMore');
 }