Skip to content

Commit a084caf

Browse files
committed
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. 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.
1 parent 449f326 commit a084caf

File tree

3 files changed

+75
-29
lines changed

3 files changed

+75
-29
lines changed

lib/model/topicmap.dart

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import '../api/model/model.dart';
2+
3+
class TopicMap<T> {
4+
final Map<String, _KeyValue<T>> _topicMessageMap = {};
5+
6+
int get size => _topicMessageMap.length;
7+
8+
T? get(TopicName key) {
9+
final mapping = _topicMessageMap[_munge(key)];
10+
return mapping?.v;
11+
}
12+
13+
void set(TopicName key, T value) {
14+
_topicMessageMap[_munge(key)] = _KeyValue(k: key, v: value);
15+
}
16+
17+
TopicName? getSomeTopicVersion(TopicName key){
18+
return _topicMessageMap[_munge(key)]?.k;
19+
}
20+
bool has(TopicName key) => _topicMessageMap.containsKey(_munge(key));
21+
22+
bool delete(TopicName key) => _topicMessageMap.remove(_munge(key)) != null;
23+
24+
Iterable<TopicName> keys() => _topicMessageMap.values.map((kv) => kv.k);
25+
26+
Iterable<T> values() => _topicMessageMap.values.map((kv) => kv.v);
27+
28+
Iterable<MapEntry<TopicName, T>> get entries =>
29+
_topicMessageMap.values.map((kv) => MapEntry(kv.k, kv.v));
30+
31+
void clear() => _topicMessageMap.clear();
32+
33+
String _munge(TopicName key) => key.toString().toLowerCase();
34+
}
35+
36+
class _KeyValue<T> {
37+
final TopicName k;
38+
final T v;
39+
40+
_KeyValue({required this.k, required this.v});
41+
}

lib/model/unreads.dart

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import '../log.dart';
1010
import 'algorithms.dart';
1111
import 'narrow.dart';
1212
import 'channel.dart';
13+
import 'topicmap.dart';
1314

1415
/// The view-model for unread messages.
1516
///
@@ -40,14 +41,14 @@ class Unreads extends ChangeNotifier {
4041
required int selfUserId,
4142
required ChannelStore channelStore,
4243
}) {
43-
final streams = <int, Map<TopicName, QueueList<int>>>{};
44+
final streams = <int, TopicMap<QueueList<int>>>{};
4445
final dms = <DmNarrow, QueueList<int>>{};
4546
final mentions = Set.of(initial.mentions);
4647

4748
for (final unreadChannelSnapshot in initial.channels) {
4849
final streamId = unreadChannelSnapshot.streamId;
4950
final topic = unreadChannelSnapshot.topic;
50-
(streams[streamId] ??= {})[topic] = QueueList.from(unreadChannelSnapshot.unreadMessageIds);
51+
(streams[streamId] ??= TopicMap<QueueList<int>>()).set(topic,QueueList.from(unreadChannelSnapshot.unreadMessageIds));
5152
}
5253

5354
for (final unreadDmSnapshot in initial.dms) {
@@ -86,7 +87,7 @@ class Unreads extends ChangeNotifier {
8687
// int count;
8788

8889
/// Unread stream messages, as: stream ID → topic → message IDs (sorted).
89-
final Map<int, Map<TopicName, QueueList<int>>> streams;
90+
final Map<int, TopicMap<QueueList<int>>> streams;
9091

9192
/// Unread DM messages, as: DM narrow → message IDs (sorted).
9293
final Map<DmNarrow, QueueList<int>> dms;
@@ -187,7 +188,7 @@ class Unreads extends ChangeNotifier {
187188

188189
int countInTopicNarrow(int streamId, TopicName topic) {
189190
final topics = streams[streamId];
190-
return topics?[topic]?.length ?? 0;
191+
return topics?.size ?? 0;
191192
}
192193

193194
int countInDmNarrow(DmNarrow narrow) => dms[narrow]?.length ?? 0;
@@ -443,26 +444,30 @@ class Unreads extends ChangeNotifier {
443444
// TODO use efficient lookups
444445
bool _slowIsPresentInStreams(int messageId) {
445446
return streams.values.any(
446-
(topics) => topics.values.any(
447+
(topics) => topics.values().any(
447448
(messageIds) => messageIds.contains(messageId),
448449
),
449450
);
450451
}
451452

452453
void _addLastInStreamTopic(int messageId, int streamId, TopicName topic) {
453-
((streams[streamId] ??= {})[topic] ??= QueueList()).addLast(messageId);
454+
if ((streams[streamId] ??= TopicMap<QueueList<int>>()).get(topic) == null) {
455+
streams[streamId]?.set(topic, QueueList<int>());
456+
}
457+
(streams[streamId] ??= TopicMap<QueueList<int>>()).get(topic)?.addLast(messageId);
454458
}
455459

456460
// [messageIds] must be sorted ascending and without duplicates.
457461
void _addAllInStreamTopic(QueueList<int> messageIds, int streamId, TopicName topic) {
458-
final topics = streams[streamId] ??= {};
459-
topics.update(topic,
460-
ifAbsent: () => messageIds,
461-
// setUnion dedupes existing and incoming unread IDs,
462-
// so we tolerate zulip/zulip#22164, fixed in 6.0
463-
// TODO(server-6) remove 6.0 comment
464-
(existing) => setUnion(existing, messageIds),
465-
);
462+
final topics = streams[streamId] ??= TopicMap<QueueList<int>>();
463+
if (topics.has(topic)) {
464+
// If the topic already exists, update its message ids with setUnion
465+
topics.set(topic, setUnion(topics.get(topic) ?? QueueList<int>(), messageIds));
466+
} else {
467+
// If the topic does not exist, insert the new messageIds list
468+
topics.set(topic, messageIds);
469+
}
470+
466471
}
467472

468473
// TODO use efficient model lookups
@@ -477,9 +482,9 @@ class Unreads extends ChangeNotifier {
477482
}
478483
}
479484
for (final topic in newlyEmptyTopics) {
480-
topics.remove(topic);
485+
topics.delete(topic);
481486
}
482-
if (topics.isEmpty) {
487+
if (topics.size==0) {
483488
newlyEmptyStreams.add(streamId);
484489
}
485490
}
@@ -491,14 +496,14 @@ class Unreads extends ChangeNotifier {
491496
void _removeAllInStreamTopic(Set<int> incomingMessageIds, int streamId, TopicName topic) {
492497
final topics = streams[streamId];
493498
if (topics == null) return;
494-
final messageIds = topics[topic];
499+
final messageIds = topics.get(topic);
495500
if (messageIds == null) return;
496501

497502
// ([QueueList] doesn't have a `removeAll`)
498503
messageIds.removeWhere((id) => incomingMessageIds.contains(id));
499504
if (messageIds.isEmpty) {
500-
topics.remove(topic);
501-
if (topics.isEmpty) {
505+
topics.delete(topic);
506+
if (topics.size==0) {
502507
streams.remove(streamId);
503508
}
504509
}

pubspec.lock

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -291,10 +291,10 @@ packages:
291291
dependency: "direct dev"
292292
description:
293293
name: fake_async
294-
sha256: "6a95e56b2449df2273fd8c45a662d6947ce1ebb7aafe80e550a3f68297f3cacc"
294+
sha256: "5368f224a74523e8d2e7399ea1638b37aecfca824a3cc4dfdf77bf1fa905ac44"
295295
url: "https://pub.dev"
296296
source: hosted
297-
version: "1.3.2"
297+
version: "1.3.3"
298298
ffi:
299299
dependency: transitive
300300
description:
@@ -607,10 +607,10 @@ packages:
607607
dependency: "direct main"
608608
description:
609609
name: intl
610-
sha256: d6f56758b7d3014a48af9701c085700aac781a92a87a62b1333b46d8879661cf
610+
sha256: "3df61194eb431efc39c4ceba583b95633a403f46c9fd341e550ce0bfa50e9aa5"
611611
url: "https://pub.dev"
612612
source: hosted
613-
version: "0.19.0"
613+
version: "0.20.2"
614614
io:
615615
dependency: transitive
616616
description:
@@ -647,10 +647,10 @@ packages:
647647
dependency: transitive
648648
description:
649649
name: leak_tracker
650-
sha256: c35baad643ba394b40aac41080300150a4f08fd0fd6a10378f8f7c6bc161acec
650+
sha256: "6bb818ecbdffe216e81182c2f0714a2e62b593f4a4f13098713ff1685dfb6ab0"
651651
url: "https://pub.dev"
652652
source: hosted
653-
version: "10.0.8"
653+
version: "10.0.9"
654654
leak_tracker_flutter_testing:
655655
dependency: transitive
656656
description:
@@ -1244,10 +1244,10 @@ packages:
12441244
dependency: transitive
12451245
description:
12461246
name: vm_service
1247-
sha256: "0968250880a6c5fe7edc067ed0a13d4bae1577fe2771dcf3010d52c4a9d3ca14"
1247+
sha256: ddfa8d30d89985b96407efce8acbdd124701f96741f2d981ca860662f1c0dc02
12481248
url: "https://pub.dev"
12491249
source: hosted
1250-
version: "14.3.1"
1250+
version: "15.0.0"
12511251
wakelock_plus:
12521252
dependency: "direct main"
12531253
description:
@@ -1300,10 +1300,10 @@ packages:
13001300
dependency: transitive
13011301
description:
13021302
name: webdriver
1303-
sha256: "3d773670966f02a646319410766d3b5e1037efb7f07cc68f844d5e06cd4d61c8"
1303+
sha256: "2f3a14ca026957870cfd9c635b83507e0e51d8091568e90129fbf805aba7cade"
13041304
url: "https://pub.dev"
13051305
source: hosted
1306-
version: "3.0.4"
1306+
version: "3.1.0"
13071307
webkit_inspection_protocol:
13081308
dependency: transitive
13091309
description:

0 commit comments

Comments
 (0)