channel: Keep get-stream-topics data up-to-date

Fixes: zulip#1499

Author: PIG208

lib/model/channel.dart

@@ -1,8 +1,13 @@
+import 'dart:async';
+
+import 'package:collection/collection.dart';
 import 'package:flutter/foundation.dart';
 
 import '../api/model/events.dart';
 import '../api/model/initial_snapshot.dart';
 import '../api/model/model.dart';
+import '../api/route/channels.dart';
+import 'store.dart';
 
 /// The portion of [PerAccountStore] for channels, topics, and stuff about them.
 ///
@@ -34,6 +39,22 @@ mixin ChannelStore {
   /// and [streamsByName].
   Map<int, Subscription> get subscriptions;
 
+  /// Fetch topics in a stream from the server.
+  ///
+  /// The results from the last successful fetch
+  /// can be retrieved with [topicMaxIdsInStream].
+  Future<void> fetchTopics(int streamId);
+
+  /// Pairs of the known topics and its latest message ID, in the given stream.
+  ///
+  /// The result is guaranteed to be sorted by maxId, and the topics are
+  /// guaranteed to be distinct.  In some cases when message moves occur,
+  /// the same maxId can be present in multiple topics.
+  ///
+  /// Returns null if the data has never been fetched yet.
+  /// To fetch it from the server, use [fetchTopics].
+  List<TopicMaxId>? topicMaxIdsInStream(int streamId);
+
   /// The visibility policy that the self-user has for the given topic.
   ///
   /// This does not incorporate the user's channel-level policy,
@@ -208,6 +229,31 @@ class ChannelStoreImpl extends PerAccountStoreBase with ChannelStore {
   @override
   final Map<int, Subscription> subscriptions;
 
+  /// Maps indexed by stream IDs, of the the known latest message IDs in each
+  /// topic.
+  ///
+  /// For example: `_topicMaxIdsByStream[stream.id][topic] = maxId`.
+  final Map<int, Map<TopicName, int>> _topicMaxIdsByStream = {};
+
+  @override
+  Future<void> fetchTopics(int streamId) async {
+    final result = await getStreamTopics(connection, streamId: streamId);
+    _topicMaxIdsByStream[streamId] = {
+      for (final GetStreamTopicsEntry(:name, :maxId) in result.topics)
+        name: maxId,
+    };
+  }
+
+  @override
+  List<TopicMaxId>? topicMaxIdsInStream(int streamId) {
+    final topicMaxIdsInStream = _topicMaxIdsByStream[streamId];
+    if (topicMaxIdsInStream == null) return null;
+    return [
+      for (final MapEntry(:key, :value) in topicMaxIdsInStream.entries)
+        (topic: key, maxId: value)
+    ].sortedBy((value) => -value.maxId);
+  }
+
   @override
   Map<int, Map<TopicName, UserTopicVisibilityPolicy>> get debugTopicVisibility => topicVisibility;
 
@@ -374,4 +420,48 @@ class ChannelStoreImpl extends PerAccountStoreBase with ChannelStore {
       forStream[event.topicName] = visibilityPolicy;
     }
   }
+
+  /// Handle a [MessageEvent], returning whether listeners should be notified.
+  bool handleMessageEvent(MessageEvent event) {
+    if (event.message is! StreamMessage) return false;
+    final StreamMessage(:streamId, :topic) = event.message as StreamMessage;
+
+    final topicMaxIdsInStream = _topicMaxIdsByStream[streamId];
+    if (topicMaxIdsInStream == null) return false;
+    assert(!topicMaxIdsInStream.containsKey(topic)
+           || topicMaxIdsInStream[topic]! < event.message.id);
+    topicMaxIdsInStream[topic] = event.message.id;
+    return true;
+  }
+
+  /// Handle a [UpdateMessageEvent], returning whether listeners should be
+  /// notified.
+  bool handleUpdateMessageEvent(UpdateMessageEvent event) {
+    if (event.moveData == null) return false;
+    final UpdateMessageMoveData(
+      :origStreamId, :origTopic, :newStreamId, :newTopic, :propagateMode,
+    ) = event.moveData!;
+    bool shouldNotify = false;
+
+    final topicMaxIdsInOrigStream = _topicMaxIdsByStream[origStreamId];
+    if (propagateMode == PropagateMode.changeAll
+        && topicMaxIdsInOrigStream != null) {
+      shouldNotify = topicMaxIdsInOrigStream.remove(origTopic) != null;
+    }
+
+    final topicMaxIdsInNewStream = _topicMaxIdsByStream[newStreamId];
+    if (topicMaxIdsInNewStream != null) {
+      final movedMaxId = event.messageIds.max;
+      if (!topicMaxIdsInNewStream.containsKey(newTopic)
+          || topicMaxIdsInNewStream[newTopic]! < movedMaxId) {
+        topicMaxIdsInNewStream[newTopic] = movedMaxId;
+        shouldNotify = true;
+      }
+    }
+
+    return shouldNotify;
+  }
 }
+
+/// A topic and its known latest message ID.
+typedef TopicMaxId = ({TopicName topic, int maxId});

lib/model/store.dart

@@ -693,6 +693,12 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
   Map<String, ZulipStream> get streamsByName => _channels.streamsByName;
   @override
   Map<int, Subscription> get subscriptions => _channels.subscriptions;
+
+  @override
+  Future<void> fetchTopics(int streamId) => _channels.fetchTopics(streamId);
+  @override
+  List<TopicMaxId>? topicMaxIdsInStream(int streamId) =>
+    _channels.topicMaxIdsInStream(streamId);
   @override
   UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, TopicName topic) =>
     _channels.topicVisibilityPolicy(streamId, topic);
@@ -890,13 +896,15 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
         unreads.handleMessageEvent(event);
         recentDmConversationsView.handleMessageEvent(event);
         recentSenders.handleMessage(event.message); // TODO(#824)
+        if (_channels.handleMessageEvent(event)) notifyListeners();
         // When adding anything here (to handle [MessageEvent]),
         // it probably belongs in [reconcileMessages] too.
 
       case UpdateMessageEvent():
         assert(debugLog("server event: update_message ${event.messageId}"));
         _messages.handleUpdateMessageEvent(event);
         unreads.handleUpdateMessageEvent(event);
+        if (_channels.handleUpdateMessageEvent(event)) notifyListeners();
 
       case DeleteMessageEvent():
         assert(debugLog("server event: delete_message ${event.messageIds}"));

lib/widgets/topic_list.dart

@@ -1,8 +1,8 @@
 import 'package:flutter/material.dart';
 
 import '../api/model/model.dart';
-import '../api/route/channels.dart';
 import '../generated/l10n/zulip_localizations.dart';
+import '../model/channel.dart';
 import '../model/narrow.dart';
 import '../model/unreads.dart';
 import 'action_sheet.dart';
@@ -128,9 +128,6 @@ class _TopicList extends StatefulWidget {
 
 class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMixin {
   Unreads? unreadsModel;
-  // TODO(#1499): store the results on [ChannelStore], and keep them
-  //   up-to-date by handling events
-  List<GetStreamTopicsEntry>? lastFetchedTopics;
 
   @override
   void onNewStore() {
@@ -156,26 +153,26 @@ class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMi
     // Do nothing when the fetch fails; the topic-list will stay on
     // the loading screen, until the user navigates away and back.
     // TODO(design) show a nice error message on screen when this fails
-    final store = PerAccountStoreWidget.of(context);
-    final result = await getStreamTopics(store.connection,
-      streamId: widget.streamId);
+    await PerAccountStoreWidget.of(context).fetchTopics(widget.streamId);
     if (!mounted) return;
     setState(() {
-      lastFetchedTopics = result.topics;
+      // TODO maybe notify listeners from store.fetchTopics
+      // The actuall state lives in the PerAccountStore
     });
   }
 
   @override
   Widget build(BuildContext context) {
-    if (lastFetchedTopics == null) {
+    final store = PerAccountStoreWidget.of(context);
+    if (store.topicMaxIdsInStream(widget.streamId) == null) {
       return const Center(child: CircularProgressIndicator());
     }
 
     // TODO(design) handle the rare case when `lastFetchTopics` is empty
 
     // This is adapted from parts of the build method on [_InboxPageState].
     final topicItems = <_TopicItemData>[];
-    for (final GetStreamTopicsEntry(:maxId, name: topic) in lastFetchedTopics!) {
+    for (final TopicMaxId(:topic, :maxId) in store.topicMaxIdsInStream(widget.streamId)!) {
       final unreadMessageIds =
         unreadsModel!.streams[widget.streamId]?[topic] ?? <int>[];
       final countInTopic = unreadMessageIds.length;
@@ -185,9 +182,6 @@ class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMi
         topic: topic,
         unreadCount: countInTopic,
         hasMention: hasMention,
-        // `lastFetchedTopics.maxId` can become outdated when a new message
-        // arrives or when there are message moves, until we re-fetch.
-        // TODO(#1499): track changes to this
         maxId: maxId,
       ));
     }