Skip to content

Commit b34a68f

Browse files
PIG208gnprice
authored andcommitted
poll: Make Poll a ChangeNotifier
Previously, we would notify all message list views containing the message when the poll got updated through a submessage event. By making Poll a ChangeNotifier, we can limit that notification to only its listeners to improve performance. A side-effect of this change is that we no longer notify listeners if the event is malformed. Signed-off-by: Zixuan James Li <[email protected]>
1 parent 36ad5ac commit b34a68f

File tree

3 files changed

+30
-12
lines changed

3 files changed

+30
-12
lines changed

lib/api/model/submessage.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'dart:convert';
22

3+
import 'package:flutter/foundation.dart';
34
import 'package:json_annotation/json_annotation.dart';
45

56
import '../../log.dart';
@@ -354,7 +355,7 @@ class UnknownPollEventSubmessage extends PollEventSubmessage {
354355
/// See also:
355356
/// - https://zulip.com/help/create-a-poll
356357
/// - https://github.com/zulip/zulip/blob/304d948416465c1a085122af5d752f03d6797003/web/shared/src/poll_data.ts
357-
class Poll {
358+
class Poll extends ChangeNotifier {
358359
/// Construct a poll from submessages.
359360
///
360361
/// For a poll Zulip widget, the first submessage's content contains a
@@ -412,6 +413,7 @@ class Poll {
412413
return;
413414
}
414415
_applyEvent(event.senderId, pollEventSubmessage);
416+
notifyListeners();
415417
}
416418

417419
void _applyEvent(int senderId, PollEventSubmessage event) {

lib/model/message.dart

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -331,10 +331,8 @@ class MessageStoreImpl with MessageStore {
331331
return;
332332
}
333333

334+
// Live-updates for polls should not rebuild the message lists.
335+
// [Poll] is responsible for notifying the affected listeners.
334336
poll.handleSubmessageEvent(event);
335-
336-
for (final view in _messageListViews) {
337-
view.notifyListenersIfMessagePresent(event.messageId);
338-
}
339337
}
340338
}

test/model/message_test.dart

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,18 @@ void main() {
604604
Subject<Poll> checkPoll(Message message) =>
605605
check(store.messages[message.id]).isNotNull().poll.isNotNull();
606606

607+
late int pollNotifiedCount;
608+
609+
void checkPollNotified({required int count}) {
610+
check(pollNotifiedCount).equals(count);
611+
pollNotifiedCount = 0;
612+
// This captures any unchecked [messageList] notifications, to verify
613+
// that poll live-updates do not trigger broader rebuilds.
614+
checkNotNotified();
615+
}
616+
void checkPollNotNotified() => checkPollNotified(count: 0);
617+
void checkPollNotifiedOnce() => checkPollNotified(count: 1);
618+
607619
group('handleSubmessageEvent', () {
608620
Future<Message> preparePollMessage({
609621
String? question,
@@ -640,6 +652,10 @@ void main() {
640652
}]);
641653
await messageList.fetchInitial();
642654
checkNotifiedOnce();
655+
pollNotifiedCount = 0;
656+
store.messages[message.id]!.poll!.addListener(() {
657+
pollNotifiedCount++;
658+
});
643659
return message;
644660
}
645661

@@ -671,7 +687,7 @@ void main() {
671687
// Invalid type for question
672688
'question': 100,
673689
})));
674-
checkNotifiedOnce();
690+
checkPollNotNotified();
675691
checkPoll(message).question.equals('Old question');
676692
});
677693

@@ -680,7 +696,7 @@ void main() {
680696
final message = await preparePollMessage(question: 'Old question');
681697
await store.handleEvent(eg.submessageEvent(message.id, eg.selfUser.userId,
682698
content: PollQuestionEventSubmessage(question: 'New question')));
683-
checkNotifiedOnce();
699+
checkPollNotifiedOnce();
684700
checkPoll(message).question.equals('New question');
685701
});
686702

@@ -705,7 +721,7 @@ void main() {
705721
}) async {
706722
await store.handleEvent(eg.submessageEvent(message.id, sender.userId,
707723
content: PollNewOptionEventSubmessage(option: option, idx: idx)));
708-
checkNotifiedOnce();
724+
checkPollNotifiedOnce();
709725
}
710726

711727
test('add option', () async {
@@ -743,7 +759,7 @@ void main() {
743759
Future<void> handleVoteEvent(String key, PollVoteOp op, User voter) async {
744760
await store.handleEvent(eg.submessageEvent(message.id, voter.userId,
745761
content: PollVoteEventSubmessage(key: key, op: op)));
746-
checkNotifiedOnce();
762+
checkPollNotifiedOnce();
747763
}
748764

749765
test('add votes', () async {
@@ -821,9 +837,11 @@ void main() {
821837
message = await preparePollMessage(
822838
options: [eg.pollOption(text: 'foo', voters: [])]);
823839
checkPoll(message).options.deepEquals([conditionPollOption('foo')]);
824-
await handleVoteEvent(
825-
PollEventSubmessage.optionKey(senderId: null, idx: 0),
826-
PollVoteOp.unknown, eg.otherUser);
840+
await store.handleEvent(eg.submessageEvent(message.id, eg.otherUser.userId,
841+
content: PollVoteEventSubmessage(
842+
key: PollEventSubmessage.optionKey(senderId: null, idx: 0),
843+
op: PollVoteOp.unknown)));
844+
checkPollNotNotified();
827845
checkPoll(message).options.deepEquals([conditionPollOption('foo')]);
828846
});
829847
});

0 commit comments

Comments
 (0)