unreads: Fix topic case-sensitivity.

Maps canoncalized topic names to
a variant of the actual topic name.

This is achieved by introducing a TopicMap class
similar to FoldDict in web.

I referred to the FoldDict code in
zulip/zulip as @gnprice suggested.
Thanks to the folks who wrote it!

See https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/Expected.20behaviour.20for.20case.20insensitivity.20for.20unreads.20.23F980/near/2076806

Fixes #980.

Author: apoorvapendse

lib/model/channel.dart

@@ -361,3 +361,51 @@ class ChannelStoreImpl with ChannelStore {
     }
   }
 }
+
+class TopicMap<T> {
+  final Map<String, _KeyValue<T>> _topicMessageMap = {};
+
+  int get size => _topicMessageMap.length;
+
+  T? get(TopicName key) {
+    return _topicMessageMap[_munge(key)]?.v;
+  }
+
+  void set(TopicName key, T value) {
+    _topicMessageMap[_munge(key)] = _KeyValue(k: key, v: value);
+  }
+
+  bool containsKey(TopicName key) => _topicMessageMap.containsKey(_munge(key));
+
+  bool remove(TopicName key) => _topicMessageMap.remove(_munge(key)) != null;
+
+  Iterable<TopicName> keys() => _topicMessageMap.values.map((kv) => kv.k);
+
+  Iterable<T> values() => _topicMessageMap.values.map((kv) => kv.v);
+
+  Iterable<MapEntry<TopicName, T>> get entries =>
+      _topicMessageMap.entries.map((kv) => MapEntry(kv.value.k, kv.value.v));
+
+  void clear() => _topicMessageMap.clear();
+
+  String _munge(TopicName key) => key.toString().toLowerCase();
+}
+
+extension TopicMapExtensions<T> on TopicMap<T> {
+  // Converts this `TopicMap<T>` into a plain `Map<TopicName, T>`,
+  // making it easier to do standard deep equality checks in tests.
+  Map<TopicName, T> toMap() {
+    final result = <TopicName, T>{};
+    for (final entry in entries) {
+      result[entry.key] = entry.value;
+    }
+    return result;
+  }
+}
+
+class _KeyValue<T> {
+  final TopicName k;
+  final T v;
+
+  _KeyValue({required this.k, required this.v});
+}

lib/model/unreads.dart

@@ -40,14 +40,14 @@ class Unreads extends ChangeNotifier {
     required int selfUserId,
     required ChannelStore channelStore,
   }) {
-    final streams = <int, Map<TopicName, QueueList<int>>>{};
+    final streams = <int, TopicMap<QueueList<int>>>{};
     final dms = <DmNarrow, QueueList<int>>{};
     final mentions = Set.of(initial.mentions);
 
     for (final unreadChannelSnapshot in initial.channels) {
       final streamId = unreadChannelSnapshot.streamId;
       final topic = unreadChannelSnapshot.topic;
-      (streams[streamId] ??= {})[topic] = QueueList.from(unreadChannelSnapshot.unreadMessageIds);
+      (streams[streamId] ??= TopicMap<QueueList<int>>()).set(topic,QueueList.from(unreadChannelSnapshot.unreadMessageIds));
     }
 
     for (final unreadDmSnapshot in initial.dms) {
@@ -86,7 +86,7 @@ class Unreads extends ChangeNotifier {
   // int count;
 
   /// Unread stream messages, as: stream ID → topic → message IDs (sorted).
-  final Map<int, Map<TopicName, QueueList<int>>> streams;
+  final Map<int, TopicMap<QueueList<int>>> streams;
 
   /// Unread DM messages, as: DM narrow → message IDs (sorted).
   final Map<DmNarrow, QueueList<int>> dms;
@@ -187,7 +187,9 @@ class Unreads extends ChangeNotifier {
 
   int countInTopicNarrow(int streamId, TopicName topic) {
     final topics = streams[streamId];
-    return topics?[topic]?.length ?? 0;
+    if (topics == null) return 0;
+    final qlist = topics.get(topic);
+    return qlist?.length ?? 0;
   }
 
   int countInDmNarrow(DmNarrow narrow) => dms[narrow]?.length ?? 0;
@@ -443,26 +445,30 @@ class Unreads extends ChangeNotifier {
   // TODO use efficient lookups
   bool _slowIsPresentInStreams(int messageId) {
     return streams.values.any(
-      (topics) => topics.values.any(
+      (topics) => topics.values().any(
         (messageIds) => messageIds.contains(messageId),
       ),
     );
   }
 
   void _addLastInStreamTopic(int messageId, int streamId, TopicName topic) {
-    ((streams[streamId] ??= {})[topic] ??= QueueList()).addLast(messageId);
+    if ((streams[streamId] ??= TopicMap<QueueList<int>>()).get(topic) == null) {
+      streams[streamId]?.set(topic, QueueList<int>());
+    }
+    (streams[streamId] ??= TopicMap<QueueList<int>>()).get(topic)?.addLast(messageId);
   }
 
   // [messageIds] must be sorted ascending and without duplicates.
   void _addAllInStreamTopic(QueueList<int> messageIds, int streamId, TopicName topic) {
-    final topics = streams[streamId] ??= {};
-    topics.update(topic,
-      ifAbsent: () => messageIds,
-      // setUnion dedupes existing and incoming unread IDs,
-      // so we tolerate zulip/zulip#22164, fixed in 6.0
-      // TODO(server-6) remove 6.0 comment
-      (existing) => setUnion(existing, messageIds),
-    );
+    final topics = streams[streamId] ??= TopicMap<QueueList<int>>();
+    if (topics.containsKey(topic)) {
+      // If the topic already exists, update its message ids with setUnion
+    topics.set(topic, setUnion(topics.get(topic) ?? QueueList<int>(), messageIds));
+    } else {
+      // If the topic does not exist, insert the new messageIds list
+    topics.set(topic, messageIds);
+    }
+
   }
 
   // TODO use efficient model lookups
@@ -479,7 +485,7 @@ class Unreads extends ChangeNotifier {
       for (final topic in newlyEmptyTopics) {
         topics.remove(topic);
       }
-      if (topics.isEmpty) {
+      if (topics.size==0) {
         newlyEmptyStreams.add(streamId);
       }
     }
@@ -491,14 +497,14 @@ class Unreads extends ChangeNotifier {
   void _removeAllInStreamTopic(Set<int> incomingMessageIds, int streamId, TopicName topic) {
     final topics = streams[streamId];
     if (topics == null) return;
-    final messageIds = topics[topic];
+    final messageIds = topics.get(topic);
     if (messageIds == null) return;
 
     // ([QueueList] doesn't have a `removeAll`)
     messageIds.removeWhere((id) => incomingMessageIds.contains(id));
     if (messageIds.isEmpty) {
       topics.remove(topic);
-      if (topics.isEmpty) {
+      if (topics.size==0) {
         streams.remove(streamId);
       }
     }

pubspec.lock

@@ -291,10 +291,10 @@ packages:
     dependency: "direct dev"
     description:
       name: fake_async
-      sha256: "6a95e56b2449df2273fd8c45a662d6947ce1ebb7aafe80e550a3f68297f3cacc"
+      sha256: "5368f224a74523e8d2e7399ea1638b37aecfca824a3cc4dfdf77bf1fa905ac44"
       url: "https://pub.dev"
     source: hosted
-    version: "1.3.2"
+    version: "1.3.3"
   ffi:
     dependency: transitive
     description:
@@ -607,10 +607,10 @@ packages:
     dependency: "direct main"
     description:
       name: intl
-      sha256: d6f56758b7d3014a48af9701c085700aac781a92a87a62b1333b46d8879661cf
+      sha256: "3df61194eb431efc39c4ceba583b95633a403f46c9fd341e550ce0bfa50e9aa5"
       url: "https://pub.dev"
     source: hosted
-    version: "0.19.0"
+    version: "0.20.2"
   io:
     dependency: transitive
     description:
@@ -647,10 +647,10 @@ packages:
     dependency: transitive
     description:
       name: leak_tracker
-      sha256: c35baad643ba394b40aac41080300150a4f08fd0fd6a10378f8f7c6bc161acec
+      sha256: "6bb818ecbdffe216e81182c2f0714a2e62b593f4a4f13098713ff1685dfb6ab0"
       url: "https://pub.dev"
     source: hosted
-    version: "10.0.8"
+    version: "10.0.9"
   leak_tracker_flutter_testing:
     dependency: transitive
     description:
@@ -1244,10 +1244,10 @@ packages:
     dependency: transitive
     description:
       name: vm_service
-      sha256: "0968250880a6c5fe7edc067ed0a13d4bae1577fe2771dcf3010d52c4a9d3ca14"
+      sha256: ddfa8d30d89985b96407efce8acbdd124701f96741f2d981ca860662f1c0dc02
       url: "https://pub.dev"
     source: hosted
-    version: "14.3.1"
+    version: "15.0.0"
   wakelock_plus:
     dependency: "direct main"
     description:
@@ -1300,10 +1300,10 @@ packages:
     dependency: transitive
     description:
       name: webdriver
-      sha256: "3d773670966f02a646319410766d3b5e1037efb7f07cc68f844d5e06cd4d61c8"
+      sha256: "2f3a14ca026957870cfd9c635b83507e0e51d8091568e90129fbf805aba7cade"
       url: "https://pub.dev"
     source: hosted
-    version: "3.0.4"
+    version: "3.1.0"
   webkit_inspection_protocol:
     dependency: transitive
     description:

test/model/channel_test.dart

@@ -1,9 +1,11 @@
 
 import 'package:checks/checks.dart';
+import 'package:collection/collection.dart';
 import 'package:test/scaffolding.dart';
 import 'package:zulip/api/model/events.dart';
 import 'package:zulip/api/model/initial_snapshot.dart';
 import 'package:zulip/api/model/model.dart';
+import 'package:zulip/model/algorithms.dart';
 import 'package:zulip/model/store.dart';
 import 'package:zulip/model/channel.dart';
 
@@ -394,4 +396,118 @@ void main() {
         .equals(UserTopicVisibilityPolicy.none);
     });
   });
+    group('TopicMap with QueueList<int>', () {
+    test('initially empty', () {
+      final topicMap = TopicMap<QueueList<int>>();
+      check(topicMap.size).equals(0);
+      check(topicMap.keys()).isEmpty();
+      check(topicMap.values()).isEmpty();
+      check(topicMap.entries).isEmpty();
+      check(topicMap.toMap()).deepEquals({});
+    });
+
+    test('set and get values', () {
+      final topicMap = TopicMap<QueueList<int>>();
+      final topic = TopicName('Topic1');
+      final value = QueueList<int>();
+      value.addAll([1, 2, 3]);
+      topicMap.set(topic, value);
+
+      check(topicMap.size).equals(1);
+      check(topicMap.get(topic)).equals(value);
+    });
+
+    test('containsKey and remove', () {
+      final topicMap = TopicMap<QueueList<int>>();
+      final topic = TopicName('Math');
+      final value = QueueList<int>();
+      value.addAll([3, 1, 4]);
+      topicMap.set(topic, value);
+
+      check(topicMap.containsKey(topic)).isTrue();
+      // Removing should return true.
+      check(topicMap.remove(topic)).isTrue();
+      check(topicMap.containsKey(topic)).isFalse();
+      check(topicMap.size).equals(0);
+      // Removing a non-existent key returns false.
+      check(topicMap.remove(topic)).isFalse();
+    });
+
+    test('keys, values, and entries', () {
+      final topicMap = TopicMap<QueueList<int>>();
+      final topicA = TopicName('A');
+      final topicB = TopicName('B');
+      final valueA = QueueList<int>();
+      valueA.addAll([10, 20]);
+      final valueB = QueueList<int>();
+      valueB.addAll([30,40]);
+      topicMap.set(topicA, valueA);
+      topicMap.set(topicB, valueB);
+
+      final keys = topicMap.keys().toList();
+      check(keys.length).equals(2);
+      check(keys).contains(topicA);
+      check(keys).contains(topicB);
+
+      final values = topicMap.values().toList();
+      check(values.length).equals(2);
+      check(values).contains(valueA);
+      check(values).contains(valueB);
+
+      final entries = topicMap.entries.toList();
+      check(entries.length).equals(2);
+      for (final entry in entries) {
+        if (entry.key == topicA) {
+          check(entry.value).equals(valueA);
+        } else if (entry.key == topicB) {
+          check(entry.value).equals(valueB);
+        }
+      }
+    });
+
+    test('clear and toMap', () {
+      final topicMap = TopicMap<QueueList<int>>();
+      final topic1 = TopicName('One');
+      final topic2 = TopicName('Two');
+      final value1 = QueueList<int>();
+      final value2 = QueueList<int>();
+      value1.add(1);
+      value2.add(2);
+      topicMap.set(topic1, value1);
+      topicMap.set(topic2, value2);
+
+      check(topicMap.size).equals(2);
+      check(topicMap.toMap()).deepEquals({
+        topic1: value1,
+        topic2: value2,
+      });
+
+      topicMap.clear();
+      check(topicMap.size).equals(0);
+      check(topicMap.toMap()).deepEquals({});
+    });
+
+    test('key normalization is case-insensitive', () {
+      final topicMap = TopicMap<QueueList<int>>();
+      // Using two TopicName instances that differ only by case.
+      final topicUpper = TopicName('HELLO');
+      final topicLower = TopicName('hello');
+      final valueLower = QueueList<int>();
+      final valueUpper = QueueList<int>();
+      valueUpper.add(42);
+      valueLower.add(32);
+      topicMap.set(topicLower, valueLower);
+      topicMap.set(topicUpper, setUnion(topicMap.get(topicLower) as Iterable<int>, valueUpper));
+      // Since _munge lowercases the key, using a different case should return the same value.
+      final list1 = topicMap.get(topicUpper);
+      final list2 = setUnion(valueUpper, valueLower);
+      check(list1?.length).equals(list2.length);
+      for ( int i = 0; i < list1!.length; i++ ) {
+        check(list1[i]).equals(list2[i]);
+      }
+
+      check(topicMap.toMap().containsKey(TopicName("HELLO"))).isTrue();
+      check(topicMap.toMap().containsKey(TopicName("hello"))).isFalse();
+    });
+  });
 }

test/model/unreads_checks.dart

@@ -3,9 +3,17 @@ import 'package:collection/collection.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/model/narrow.dart';
 import 'package:zulip/model/unreads.dart';
+import 'package:zulip/model/channel.dart';
 
 extension UnreadsChecks on Subject<Unreads> {
-  Subject<Map<int, Map<TopicName, QueueList<int>>>> get streams => has((u) => u.streams, 'streams');
+  Subject<Map<int, Map<TopicName, QueueList<int>>>> get streams => has(
+    (unreads) {
+      return unreads.streams.map(
+        (streamId, topicMap) => MapEntry(streamId, topicMap.toMap()),
+      );
+    },
+    'streams',
+  );
   Subject<Map<DmNarrow, QueueList<int>>> get dms => has((u) => u.dms, 'dms');
   Subject<Set<int>> get mentions => has((u) => u.mentions, 'mentions');
   Subject<bool> get oldUnreadsMissing => has((u) => u.oldUnreadsMissing, 'oldUnreadsMissing');