From 632248484279ff7b9c4ae14555870dff3af51d83 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 14 Aug 2024 03:31:45 -0400 Subject: [PATCH 1/6] message test [nfc]: Move handleDeleteMessageEvent to match the order the events are defined. Signed-off-by: Zixuan James Li --- test/model/message_test.dart | 66 ++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 249317310c..d9d459818e 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -369,6 +369,39 @@ void main() { }); }); + group('handleDeleteMessageEvent', () { + test('delete an unknown message', () async { + final message1 = eg.streamMessage(); + final message2 = eg.streamMessage(); + await prepare(); + await prepareMessages([message1]); + await store.handleEvent(eg.deleteMessageEvent([message2])); + checkNotNotified(); + check(store).messages.values.single.id.equals(message1.id); + }); + + test('delete messages', () async { + final message1 = eg.streamMessage(); + final message2 = eg.streamMessage(); + await prepare(); + await prepareMessages([message1, message2]); + await store.handleEvent(eg.deleteMessageEvent([message1, message2])); + checkNotifiedOnce(); + check(store).messages.isEmpty(); + }); + + test('delete an unknown message with a known message', () async { + final message1 = eg.streamMessage(); + final message2 = eg.streamMessage(); + final message3 = eg.streamMessage(); + await prepare(); + await prepareMessages([message1, message2]); + await store.handleEvent(eg.deleteMessageEvent([message2, message3])); + checkNotifiedOnce(); + check(store).messages.values.single.id.equals(message1.id); + }); + }); + group('handleUpdateMessageFlagsEvent', () { UpdateMessageFlagsAddEvent mkAddEvent( MessageFlag flag, @@ -473,39 +506,6 @@ void main() { }); }); - group('handleDeleteMessageEvent', () { - test('delete an unknown message', () async { - final message1 = eg.streamMessage(); - final message2 = eg.streamMessage(); - await prepare(); - await prepareMessages([message1]); - await store.handleEvent(eg.deleteMessageEvent([message2])); - checkNotNotified(); - check(store).messages.values.single.id.equals(message1.id); - }); - - test('delete messages', () async { - final message1 = eg.streamMessage(); - final message2 = eg.streamMessage(); - await prepare(); - await prepareMessages([message1, message2]); - await store.handleEvent(eg.deleteMessageEvent([message1, message2])); - checkNotifiedOnce(); - check(store).messages.isEmpty(); - }); - - test('delete an unknown message with a known message', () async { - final message1 = eg.streamMessage(); - final message2 = eg.streamMessage(); - final message3 = eg.streamMessage(); - await prepare(); - await prepareMessages([message1, message2]); - await store.handleEvent(eg.deleteMessageEvent([message2, message3])); - checkNotifiedOnce(); - check(store).messages.values.single.id.equals(message1.id); - }); - }); - group('handleReactionEvent', () { test('add reaction', () async { final originalMessage = eg.streamMessage(reactions: []); From 153097856e5f06845b5c2314a5f976f9d08e050f Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 14 Aug 2024 03:15:11 -0400 Subject: [PATCH 2/6] submessage: Extract PollOptionKey. Semantically, we expect strings of a specific shape to be the key for options. This make it clearer to the reader without complicating things with extra validation. Signed-off-by: Zixuan James Li --- lib/api/model/submessage.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index 900f8dbbde..f9f22a01b8 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -173,7 +173,7 @@ sealed class PollEventSubmessage extends SubmessageData { /// /// For options that are a part of the initial [PollWidgetData], the /// [senderId] should be `null`. - static String optionKey({required int? senderId, required int idx}) => + static PollOptionKey optionKey({required int? senderId, required int idx}) => // "canned" is a canonical constant coined by the web client: // https://github.com/zulip/zulip/blob/40f59a05c/web/shared/src/poll_data.ts#L238 '${senderId ?? 'canned'},$idx'; @@ -205,6 +205,8 @@ enum PollEventSubmessageType { .map((key, value) => MapEntry(value, key)); } +typedef PollOptionKey = String; + /// A poll event when an option is added. /// /// See: https://github.com/zulip/zulip/blob/40f59a05c/web/shared/src/poll_data.ts#L112-L159 @@ -264,7 +266,7 @@ class PollVoteEventSubmessage extends PollEventSubmessage { /// The key of the affected option. /// /// See [PollEventSubmessage.optionKey]. - final String key; + final PollOptionKey key; @JsonKey(name: 'vote', unknownEnumValue: PollVoteOp.unknown) final PollVoteOp op; From 2d41e6624d6f5a1ee9fa6f312c922c2a9a6f1847 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 14 Aug 2024 03:17:08 -0400 Subject: [PATCH 3/6] submessage [nfc]: Minor wording change to a comment. Signed-off-by: Zixuan James Li --- lib/api/model/submessage.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index f9f22a01b8..3920aadeef 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -38,7 +38,7 @@ class Submessage { // We cannot parse the String into one of the [SubmessageData] classes because // information from other submessages are required. Specifically, we need: // * the index of this submessage in [Message.submessages]; - // * the [WidgetType] of the first [Message.submessages]. + // * the parsed [WidgetType] from the first [Message.submessages]. final String content; factory Submessage.fromJson(Map json) => From dcdc491a2746738b75d7fd1b9b95e341def1168f Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 14 Aug 2024 03:23:22 -0400 Subject: [PATCH 4/6] submessage test [nfc]: Extract pollWidgetData. The goal is to make the relevant context of the test data local to the tests, and keep the example data as boring as possible. We also combine the smoke test into the test for invalid widget types, contrasting it with the failing cases. Signed-off-by: Zixuan James Li --- lib/api/model/submessage.dart | 6 +++++- test/api/model/submessage_test.dart | 19 +++++++++++-------- test/example_data.dart | 15 ++++++++------- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index 3920aadeef..c9544d40f8 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -65,7 +65,9 @@ enum SubmessageType { /// * Zero or more submessages with content [PollEventSubmessage] if the /// message is a poll (i.e. if the first submessage was a [PollWidgetData]), /// and similarly for other types of widgets. -sealed class SubmessageData {} +sealed class SubmessageData { + Object? toJson(); +} /// The data encoded in a submessage to make the message a Zulip widget. /// @@ -87,6 +89,7 @@ sealed class WidgetData extends SubmessageData { }; } + @override Object? toJson(); } @@ -188,6 +191,7 @@ sealed class PollEventSubmessage extends SubmessageData { } } + @override Map toJson(); } diff --git a/test/api/model/submessage_test.dart b/test/api/model/submessage_test.dart index a42e4cf1cb..4d049307b7 100644 --- a/test/api/model/submessage_test.dart +++ b/test/api/model/submessage_test.dart @@ -3,6 +3,7 @@ import 'package:test/scaffolding.dart'; import 'package:zulip/api/model/submessage.dart'; import '../../example_data.dart' as eg; +import '../../stdlib_checks.dart'; import 'submessage_checks.dart'; void main() { @@ -27,23 +28,25 @@ void main() { }); }); - test('smoke WidgetData', () { - check(WidgetData.fromJson(eg.pollWidgetDataFavoriteLetter)).isA() + test('invalid widget_type -> UnsupportedWidgetData/throw', () { + final pollWidgetData = deepToJson(eg.pollWidgetData( + question: 'example question', + options: ['A', 'B', 'C'], + )) as Map; + + check(WidgetData.fromJson(pollWidgetData)).isA() ..widgetType.equals(WidgetType.poll) ..extraData.which((x) => x - ..question.equals('favorite letter') + ..question.equals('example question') ..options.deepEquals(['A', 'B', 'C']) ); - }); - - test('invalid widget_type -> UnsupportedWidgetData/throw', () { check(WidgetData.fromJson({ - ...eg.pollWidgetDataFavoriteLetter, + ...pollWidgetData, 'widget_type': 'unknown_foo', })).isA(); check(() => WidgetData.fromJson({ - ...eg.pollWidgetDataFavoriteLetter, + ...pollWidgetData, 'widget_type': 123, })).throws(); }); diff --git a/test/example_data.dart b/test/example_data.dart index 1716869f35..3e3762a051 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -3,6 +3,7 @@ import 'dart:math'; 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/api/model/submessage.dart'; import 'package:zulip/api/route/realm.dart'; import 'package:zulip/api/route/channels.dart'; import 'package:zulip/model/narrow.dart'; @@ -405,13 +406,13 @@ DmMessage dmMessage({ }) as Map); } -const pollWidgetDataFavoriteLetter = { - 'widget_type': 'poll', - 'extra_data': { - 'question': 'favorite letter', - 'options': ['A', 'B', 'C'], - } -}; +PollWidgetData pollWidgetData({ + required String question, + required List options, +}) { + return PollWidgetData( + extraData: PollWidgetExtraData(question: question, options: options)); +} //////////////////////////////////////////////////////////////// // Aggregate data structures. From 5af5c76a861231af8115e7518e34f8fe44d73702 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Mon, 22 Jul 2024 19:14:17 -0400 Subject: [PATCH 5/6] api: Construct polls data store from submessages. For now, we only consider the case when the submessages describe a poll, and disgard them otherwise. It will be a simple refactor later to support other Zulip widgets like todo lists, by extracting a common ancestors for such widget data structures. The `Poll` data structure will become useful when we support submessage events, where updates to a poll can happen. Some more comments are added here instead of earlier because of their references to `SubmessageData`. Signed-off-by: Zixuan James Li --- lib/api/model/model.dart | 11 ++ lib/api/model/model.g.dart | 6 +- lib/api/model/submessage.dart | 151 ++++++++++++++++++++++++++ test/api/model/model_checks.dart | 2 + test/api/model/submessage_checks.dart | 10 ++ test/api/model/submessage_test.dart | 3 + test/example_data.dart | 20 ++++ test/model/message_test.dart | 118 ++++++++++++++++++++ 8 files changed, 319 insertions(+), 2 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 75edd26b70..bf60ab6adb 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -3,6 +3,7 @@ import 'package:json_annotation/json_annotation.dart'; import 'events.dart'; import 'initial_snapshot.dart'; import 'reaction.dart'; +import 'submessage.dart'; export 'json.dart' show JsonNullable; export 'reaction.dart'; @@ -533,6 +534,9 @@ sealed class Message { final String senderRealmStr; @JsonKey(name: 'subject') String topic; + /// Poll data if "submessages" describe a poll, `null` otherwise. + @JsonKey(name: 'submessages', readValue: _readPoll, fromJson: Poll.fromJson, toJson: Poll.toJson) + Poll? poll; final int timestamp; String get type; @@ -564,6 +568,13 @@ sealed class Message { return list.map((raw) => MessageFlag.fromRawString(raw as String)).toList(); } + static Poll? _readPoll(Map json, String key) { + return Submessage.parseSubmessagesJson( + json['submessages'] as List? ?? [], + messageSenderId: (json['sender_id'] as num).toInt(), + ); + } + Message({ required this.client, required this.content, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 2c9ac0163c..87c24b1c2f 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -282,7 +282,7 @@ StreamMessage _$StreamMessageFromJson(Map json) { matchTopic: json['match_subject'] as String?, displayRecipient: json['display_recipient'] as String?, streamId: (json['stream_id'] as num).toInt(), - ); + )..poll = Poll.fromJson(Message._readPoll(json, 'submessages')); } Map _$StreamMessageToJson(StreamMessage instance) { @@ -301,6 +301,7 @@ Map _$StreamMessageToJson(StreamMessage instance) { 'sender_id': instance.senderId, 'sender_realm_str': instance.senderRealmStr, 'subject': instance.topic, + 'submessages': Poll.toJson(instance.poll), 'timestamp': instance.timestamp, 'flags': instance.flags, 'match_content': instance.matchContent, @@ -360,7 +361,7 @@ DmMessage _$DmMessageFromJson(Map json) => DmMessage( matchTopic: json['match_subject'] as String?, displayRecipient: const DmRecipientListConverter() .fromJson(json['display_recipient'] as List), - ); + )..poll = Poll.fromJson(Message._readPoll(json, 'submessages')); Map _$DmMessageToJson(DmMessage instance) => { 'client': instance.client, @@ -377,6 +378,7 @@ Map _$DmMessageToJson(DmMessage instance) => { 'sender_id': instance.senderId, 'sender_realm_str': instance.senderRealmStr, 'subject': instance.topic, + 'submessages': Poll.toJson(instance.poll), 'timestamp': instance.timestamp, 'flags': instance.flags, 'match_content': instance.matchContent, diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index c9544d40f8..749a88b98b 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -1,5 +1,9 @@ +import 'dart:convert'; + import 'package:json_annotation/json_annotation.dart'; +import '../../log.dart'; + part 'submessage.g.dart'; /// Data used for Zulip "widgets" within messages, like polls and todo lists. @@ -41,6 +45,30 @@ class Submessage { // * the parsed [WidgetType] from the first [Message.submessages]. final String content; + /// Parse a JSON list into a [Poll]. + // TODO: Use a generalized return type when supporting other Zulip widgets. + static Poll? parseSubmessagesJson(List json, { + required int messageSenderId, + }) { + final submessages = json.map((e) => Submessage.fromJson(e as Map)).toList(); + if (submessages.isEmpty) return null; + + assert(submessages.first.senderId == messageSenderId); + + final widgetData = WidgetData.fromJson(jsonDecode(submessages.first.content)); + switch (widgetData) { + case PollWidgetData(): + return Poll.fromSubmessages( + widgetData: widgetData, + pollEventSubmessages: submessages.skip(1), + messageSenderId: messageSenderId, + ); + case UnsupportedWidgetData(): + assert(debugLog('Unsupported widgetData: ${widgetData.json}')); + return null; + } + } + factory Submessage.fromJson(Map json) => _$SubmessageFromJson(json); @@ -319,3 +347,126 @@ class UnknownPollEventSubmessage extends PollEventSubmessage { @override Map toJson() => json; } + +/// States of a poll Zulip widget. +/// +/// See also: +/// - https://zulip.com/help/create-a-poll +/// - https://github.com/zulip/zulip/blob/304d948416465c1a085122af5d752f03d6797003/web/shared/src/poll_data.ts +class Poll { + /// Construct a poll from submessages. + /// + /// For a poll Zulip widget, the first submessage's content contains a + /// [PollWidgetData], and all the following submessages' content each contains + /// a [PollEventSubmessage]. + factory Poll.fromSubmessages({ + required PollWidgetData widgetData, + required Iterable pollEventSubmessages, + required int messageSenderId, + }) { + final poll = Poll._( + messageSenderId: messageSenderId, + question: widgetData.extraData.question, + options: widgetData.extraData.options, + ); + + for (final submessage in pollEventSubmessages) { + final event = PollEventSubmessage.fromJson(jsonDecode(submessage.content) as Map); + poll._applyEvent(submessage.senderId, event); + } + return poll; + } + + Poll._({ + required this.messageSenderId, + required this.question, + required List options, + }) { + for (int index = 0; index < options.length; index += 1) { + // Initial poll options use a placeholder senderId. + // See [PollEventSubmessage.optionKey] for details. + _addOption(senderId: null, idx: index, option: options[index]); + } + } + + final int messageSenderId; + String question; + + /// The limit of options any single user can add to a poll. + /// + /// See https://github.com/zulip/zulip/blob/304d948416465c1a085122af5d752f03d6797003/web/shared/src/poll_data.ts#L69-L71 + static const _maxIdx = 1000; + + Iterable get options => _options.values; + /// Contains the text of all options from [_options]. + final Set _existingOptionTexts = {}; + final Map _options = {}; + + void _applyEvent(int senderId, PollEventSubmessage event) { + switch (event) { + case PollNewOptionEventSubmessage(): + _addOption(senderId: senderId, idx: event.idx, option: event.option); + + case PollQuestionEventSubmessage(): + if (senderId != messageSenderId) { + // Only the message owner can edit the question. + assert(debugLog('unexpected poll data: user $senderId is not allowed to edit the question')); // TODO(log) + return; + } + + question = event.question; + + case PollVoteEventSubmessage(): + final option = _options[event.key]; + if (option == null) { + assert(debugLog('vote for unknown key ${event.key}')); // TODO(log) + return; + } + + switch (event.op) { + case PollVoteOp.add: + option.voters.add(senderId); + case PollVoteOp.remove: + option.voters.remove(senderId); + case PollVoteOp.unknown: + assert(debugLog('unknown vote op ${event.op}')); // TODO(log) + } + + case UnknownPollEventSubmessage(): + } + } + + void _addOption({required int? senderId, required int idx, required String option}) { + if (idx > _maxIdx || idx < 0) return; + + // The web client suppresses duplicate options, which can be created through + // the /poll command as there is no server-side validation. + if (_existingOptionTexts.contains(option)) return; + + final key = PollEventSubmessage.optionKey(senderId: senderId, idx: idx); + assert(!_options.containsKey(key)); + _options[key] = PollOption(text: option); + _existingOptionTexts.add(option); + } + + static Poll? fromJson(Object? json) { + // [Submessage.parseSubmessagesJson] does all the heavy lifting for parsing. + return json as Poll?; + } + + static List toJson(Poll? poll) { + // Rather than maintaining a up-to-date submessages list, return as if it is + // empty, because we are not sending the submessages to the server anyway. + return []; + } +} + +class PollOption { + PollOption({required this.text}); + + final String text; + final Set voters = {}; + + @override + String toString() => 'PollOption(text: $text, voters: {${voters.join(', ')}})'; +} diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index 71d199e864..2e5e8e584b 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -1,5 +1,6 @@ import 'package:checks/checks.dart'; import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/model/submessage.dart'; extension UserChecks on Subject { Subject get userId => has((x) => x.userId, 'userId'); @@ -39,6 +40,7 @@ extension MessageChecks on Subject { Subject get senderId => has((e) => e.senderId, 'senderId'); Subject get senderRealmStr => has((e) => e.senderRealmStr, 'senderRealmStr'); Subject get topic => has((e) => e.topic, 'topic'); + Subject get poll => has((e) => e.poll, 'poll'); Subject get timestamp => has((e) => e.timestamp, 'timestamp'); Subject get type => has((e) => e.type, 'type'); Subject> get flags => has((e) => e.flags, 'flags'); diff --git a/test/api/model/submessage_checks.dart b/test/api/model/submessage_checks.dart index a234f8cc56..0b7eed2230 100644 --- a/test/api/model/submessage_checks.dart +++ b/test/api/model/submessage_checks.dart @@ -37,3 +37,13 @@ extension PollVoteEventChecks on Subject { Subject get key => has((e) => e.key, 'key'); Subject get op => has((e) => e.op, 'op'); } + +extension PollChecks on Subject { + Subject get question => has((e) => e.question, 'question'); + Subject> get options => has((e) => e.options, 'options'); +} + +extension PollOptionChecks on Subject { + Subject get text => has((e) => e.text, 'text'); + Subject> get voters => has((e) => e.voters, 'voters'); +} diff --git a/test/api/model/submessage_test.dart b/test/api/model/submessage_test.dart index 4d049307b7..1c9c6f198c 100644 --- a/test/api/model/submessage_test.dart +++ b/test/api/model/submessage_test.dart @@ -114,4 +114,7 @@ void main() { 'key': PollEventSubmessage.optionKey(senderId: null, idx: 0) })).isA().op.equals(PollVoteOp.unknown); }); + + // Parsing polls with PollEventSubmessages are tested in + // `test/model/message_test.dart` in the "handleSubmessageEvent" test. } diff --git a/test/example_data.dart b/test/example_data.dart index 3e3762a051..39c31f9ddb 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -1,3 +1,4 @@ +import 'dart:convert'; import 'dart:math'; import 'package:zulip/api/model/events.dart'; @@ -343,6 +344,7 @@ StreamMessage streamMessage({ List? reactions, int? timestamp, List? flags, + List? submessages, }) { _checkPositive(id, 'message ID'); final effectiveStream = stream ?? _stream(streamId: defaultStreamMessageStreamId); @@ -362,6 +364,7 @@ StreamMessage streamMessage({ 'id': id ?? _nextMessageId(), 'last_edit_timestamp': lastEditTimestamp, 'subject': topic ?? 'example topic', + 'submessages': submessages ?? [], 'timestamp': timestamp ?? 1678139636, 'type': 'stream', }) as Map); @@ -386,6 +389,7 @@ DmMessage dmMessage({ int? lastEditTimestamp, int? timestamp, List? flags, + List? submessages, }) { _checkPositive(id, 'message ID'); assert(!to.any((user) => user.userId == from.userId)); @@ -401,6 +405,7 @@ DmMessage dmMessage({ 'id': id ?? _nextMessageId(), 'last_edit_timestamp': lastEditTimestamp, 'subject': '', + 'submessages': submessages ?? [], 'timestamp': timestamp ?? 1678139636, 'type': 'private', }) as Map); @@ -414,6 +419,21 @@ PollWidgetData pollWidgetData({ extraData: PollWidgetExtraData(question: question, options: options)); } +Submessage submessage({ + SubmessageType? msgType, + required SubmessageData? content, + int? senderId, +}) { + return Submessage( + msgType: msgType ?? SubmessageType.widget, + content: jsonEncode(content), + senderId: senderId ?? selfUser.userId, + ); +} + +PollOption pollOption({required String text, required Iterable voters}) => + PollOption(text: text)..voters.addAll(voters); + //////////////////////////////////////////////////////////////// // Aggregate data structures. // diff --git a/test/model/message_test.dart b/test/model/message_test.dart index d9d459818e..616fd89e95 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -1,13 +1,17 @@ +import 'dart:convert'; + import 'package:checks/checks.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/model/submessage.dart'; import 'package:zulip/model/message_list.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import '../api/fake_api.dart'; import '../api/model/model_checks.dart'; +import '../api/model/submessage_checks.dart'; import '../example_data.dart' as eg; import '../stdlib_checks.dart'; import 'message_list_test.dart'; @@ -52,10 +56,14 @@ void main() { /// Perform the initial message fetch for [messageList]. /// /// The test case must have already called [prepare] to initialize the state. + /// + /// This does not support submessages. Use [prepareMessageWithSubmessages] + /// instead if needed. Future prepareMessages( List messages, { bool foundOldest = false, }) async { + assert(messages.every((message) => message.poll == null)); connection.prepare(json: newestResult(foundOldest: foundOldest, messages: messages).toJson()); await messageList.fetchInitial(); @@ -576,4 +584,114 @@ void main() { .reactions.isNotNull().jsonEquals([eg.unicodeEmojiReaction]); }); }); + + group('handle Poll related events', () { + Condition conditionPollOption(String text, {Iterable? voters}) => + (it) => it.isA()..text.equals(text)..voters.deepEquals(voters ?? []); + + Subject checkPoll(Message message) => + check(store.messages[message.id]).isNotNull().poll.isNotNull(); + + group('handleMessageEvent with initial submessages', () { + late Message message; + + final defaultPollWidgetData = eg.pollWidgetData( + question: 'example question', + options: ['foo', 'bar'], + ); + + final defaultOptionConditions = [ + conditionPollOption('foo'), + conditionPollOption('bar'), + ]; + + Future handlePollMessageEvent({ + SubmessageData? widgetData, + List<(User sender, PollEventSubmessage event)> events = const [], + }) async { + message = eg.streamMessage(sender: eg.otherUser, submessages: [ + eg.submessage( + content: widgetData ?? defaultPollWidgetData, + senderId: eg.otherUser.userId), + for (final (sender, event) in events) + eg.submessage(content: event, senderId: sender.userId), + ]); + + await prepare(); + await store.handleEvent(MessageEvent(id: 0, message: message)); + } + + test('smoke', () async { + await handlePollMessageEvent(); + checkPoll(message) + ..question.equals(defaultPollWidgetData.extraData.question) + ..options.deepEquals(defaultOptionConditions); + }); + + test('contains new question event', () async { + await handlePollMessageEvent(events: [ + (eg.otherUser, PollQuestionEventSubmessage(question: 'new question')), + ]); + checkPoll(message) + ..question.equals('new question') + ..options.deepEquals(defaultOptionConditions); + }); + + test('contains new option event', () async { + await handlePollMessageEvent(events: [ + (eg.otherUser, PollNewOptionEventSubmessage(idx: 3, option: 'baz')), + (eg.selfUser, PollNewOptionEventSubmessage(idx: 0, option: 'quz')), + ]); + checkPoll(message) + ..question.equals(defaultPollWidgetData.extraData.question) + ..options.deepEquals([ + ...defaultOptionConditions, + conditionPollOption('baz'), + conditionPollOption('quz'), + ]); + }); + + test('contains vote events on initial canned options', () async { + await handlePollMessageEvent(events: [ + (eg.otherUser, PollVoteEventSubmessage(key: 'canned,1', op: PollVoteOp.add)), + (eg.otherUser, PollVoteEventSubmessage(key: 'canned,2', op: PollVoteOp.add)), + (eg.otherUser, PollVoteEventSubmessage(key: 'canned,2', op: PollVoteOp.remove)), + (eg.selfUser, PollVoteEventSubmessage(key: 'canned,1', op: PollVoteOp.add)), + ]); + checkPoll(message) + ..question.equals(defaultPollWidgetData.extraData.question) + ..options.deepEquals([ + conditionPollOption('foo'), + conditionPollOption('bar', voters: [eg.otherUser.userId, eg.selfUser.userId]), + ]); + }); + + test('contains vote events on post-creation options', () async { + await handlePollMessageEvent(events: [ + (eg.otherUser, PollNewOptionEventSubmessage(idx: 0, option: 'baz')), + (eg.otherUser, PollVoteEventSubmessage(key: '${eg.otherUser.userId},0', op: PollVoteOp.add)), + (eg.selfUser, PollVoteEventSubmessage(key: '${eg.otherUser.userId},0', op: PollVoteOp.add)), + ]); + checkPoll(message) + ..question.equals(defaultPollWidgetData.extraData.question) + ..options.deepEquals([ + ...defaultOptionConditions, + conditionPollOption('baz', voters: [eg.otherUser.userId, eg.selfUser.userId]), + ]); + }); + + test('content with invalid widget_type', () async { + message = eg.streamMessage(sender: eg.otherUser, submessages: [ + Submessage( + msgType: SubmessageType.widget, + content: jsonEncode({'widget_type': 'other'}), + senderId: eg.otherUser.userId, + ), + ]); + await prepare(); + await store.handleEvent(MessageEvent(id: 0, message: message)); + check(store.messages[message.id]).isNotNull().poll.isNull(); + }); + }); + }); } From fd10d4f232e06e86d69e67fa338c89031279b62a Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 19 Jul 2024 14:11:29 -0400 Subject: [PATCH 6/6] event: Handle submessage event for polls. We could potentially avoid notifying listeners in some cases when handling submessage event, but it wouldn't be critically necessary. Signed-off-by: Zixuan James Li --- lib/api/model/events.dart | 37 ++++++ lib/api/model/events.g.dart | 27 ++++ lib/api/model/submessage.dart | 12 ++ lib/model/message.dart | 19 +++ lib/model/store.dart | 4 + test/example_data.dart | 15 +++ test/model/message_test.dart | 225 ++++++++++++++++++++++++++++++++++ 7 files changed, 339 insertions(+) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 6d5fde6895..ec270c3d3c 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -3,6 +3,7 @@ import 'package:json_annotation/json_annotation.dart'; import '../../model/algorithms.dart'; import 'json.dart'; import 'model.dart'; +import 'submessage.dart'; part 'events.g.dart'; @@ -63,6 +64,7 @@ sealed class Event { case 'remove': return UpdateMessageFlagsRemoveEvent.fromJson(json); default: return UnexpectedEvent.fromJson(json); } + case 'submessage': return SubmessageEvent.fromJson(json); case 'typing': return TypingEvent.fromJson(json); case 'reaction': return ReactionEvent.fromJson(json); case 'heartbeat': return HeartbeatEvent.fromJson(json); @@ -942,6 +944,41 @@ class UpdateMessageFlagsMessageDetail { Map toJson() => _$UpdateMessageFlagsMessageDetailToJson(this); } +/// A Zulip event of type `submessage`: https://zulip.com/api/get-events#submessage +@JsonSerializable(fieldRename: FieldRename.snake) +class SubmessageEvent extends Event { + @override + @JsonKey(includeToJson: true) + String get type => 'submessage'; + + @JsonKey(unknownEnumValue: SubmessageType.unknown) + final SubmessageType msgType; + /// [SubmessageData] encoded in JSON. + // We cannot parse the String into one of the [SubmessageData] classes because + // information from other submessages are required. Specifically, we need + // the parsed [WidgetData] from the first [Message.submessages] of the + // corresponding message. + final String content; + final int messageId; + final int senderId; + final int submessageId; + + SubmessageEvent({ + required super.id, + required this.msgType, + required this.content, + required this.messageId, + required this.senderId, + required this.submessageId, + }); + + factory SubmessageEvent.fromJson(Map json) => + _$SubmessageEventFromJson(json); + + @override + Map toJson() => _$SubmessageEventToJson(this); +} + /// A Zulip event of type `typing`: /// https://zulip.com/api/get-events#typing-start /// https://zulip.com/api/get-events#typing-stop diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 9226278d26..7aed0879c7 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -570,6 +570,33 @@ Map _$UpdateMessageFlagsMessageDetailToJson( 'topic': instance.topic, }; +SubmessageEvent _$SubmessageEventFromJson(Map json) => + SubmessageEvent( + id: (json['id'] as num).toInt(), + msgType: $enumDecode(_$SubmessageTypeEnumMap, json['msg_type'], + unknownValue: SubmessageType.unknown), + content: json['content'] as String, + messageId: (json['message_id'] as num).toInt(), + senderId: (json['sender_id'] as num).toInt(), + submessageId: (json['submessage_id'] as num).toInt(), + ); + +Map _$SubmessageEventToJson(SubmessageEvent instance) => + { + 'id': instance.id, + 'type': instance.type, + 'msg_type': _$SubmessageTypeEnumMap[instance.msgType]!, + 'content': instance.content, + 'message_id': instance.messageId, + 'sender_id': instance.senderId, + 'submessage_id': instance.submessageId, + }; + +const _$SubmessageTypeEnumMap = { + SubmessageType.widget: 'widget', + SubmessageType.unknown: 'unknown', +}; + TypingEvent _$TypingEventFromJson(Map json) => TypingEvent( id: (json['id'] as num).toInt(), op: $enumDecode(_$TypingOpEnumMap, json['op']), diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index 749a88b98b..6538793e8b 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -3,6 +3,7 @@ import 'dart:convert'; import 'package:json_annotation/json_annotation.dart'; import '../../log.dart'; +import 'events.dart'; part 'submessage.g.dart'; @@ -402,6 +403,17 @@ class Poll { final Set _existingOptionTexts = {}; final Map _options = {}; + void handleSubmessageEvent(SubmessageEvent event) { + final PollEventSubmessage? pollEventSubmessage; + try { + pollEventSubmessage = PollEventSubmessage.fromJson(jsonDecode(event.content) as Map); + } catch (e) { + assert(debugLog('Malformed submessage event data for poll: $e\n${jsonEncode(event)}')); // TODO(log) + return; + } + _applyEvent(event.senderId, pollEventSubmessage); + } + void _applyEvent(int senderId, PollEventSubmessage event) { switch (event) { case PollNewOptionEventSubmessage(): diff --git a/lib/model/message.dart b/lib/model/message.dart index 2f08519bf5..1c3cc5a3b9 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -1,3 +1,5 @@ +import 'dart:convert'; + import '../api/model/events.dart'; import '../api/model/model.dart'; import '../log.dart'; @@ -310,4 +312,21 @@ class MessageStoreImpl with MessageStore { view.notifyListenersIfMessagePresent(event.messageId); } } + + void handleSubmessageEvent(SubmessageEvent event) { + final message = messages[event.messageId]; + if (message == null) return; + + final poll = message.poll; + if (poll == null) { + assert(debugLog('Missing poll for submessage event:\n${jsonEncode(event)}')); // TODO(log) + return; + } + + poll.handleSubmessageEvent(event); + + for (final view in _messageListViews) { + view.notifyListenersIfMessagePresent(event.messageId); + } + } } diff --git a/lib/model/store.dart b/lib/model/store.dart index bdd6c0d9a5..7c7d89fa87 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -537,6 +537,10 @@ class PerAccountStore extends ChangeNotifier with ChannelStore, MessageStore { _messages.handleUpdateMessageFlagsEvent(event); unreads.handleUpdateMessageFlagsEvent(event); + case SubmessageEvent(): + assert(debugLog("server event: submessage ${event.content}")); + _messages.handleSubmessageEvent(event); + case TypingEvent(): assert(debugLog("server event: typing/${event.op} ${event.messageType}")); typingStatus.handleTypingEvent(event); diff --git a/test/example_data.dart b/test/example_data.dart index 39c31f9ddb..efb711a352 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -647,6 +647,21 @@ UpdateMessageFlagsRemoveEvent updateMessageFlagsRemoveEvent( }))); } +SubmessageEvent submessageEvent( + int messageId, + int senderId, { + required SubmessageData? content, +}) { + return SubmessageEvent( + id: 0, + msgType: SubmessageType.widget, + content: jsonEncode(content), + messageId: messageId, + senderId: senderId, + submessageId: 100, + ); +} + TypingEvent typingEvent(SendableNarrow narrow, TypingOp op, int senderId) { switch (narrow) { case TopicNarrow(): diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 616fd89e95..2e0dd38ebb 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -592,6 +592,231 @@ void main() { Subject checkPoll(Message message) => check(store.messages[message.id]).isNotNull().poll.isNotNull(); + group('handleSubmessageEvent', () { + Future preparePollMessage({ + String? question, + List? options, + User? messageSender, + }) async { + final effectiveMessageSender = messageSender ?? eg.selfUser; + final message = eg.streamMessage(sender: effectiveMessageSender); + final submessages = [ + eg.submessage(senderId: effectiveMessageSender.userId, + content: eg.pollWidgetData( + question: question ?? 'example question', + options: (options != null) + ? options.map((e) => e.text).toList() + : ['foo', 'bar'])), + if (options != null) + for (int i = 0; i < options.length; i++) + ...[ + for (final voter in options[i].voters) + eg.submessage(senderId: voter, + content: PollVoteEventSubmessage( + key: PollEventSubmessage.optionKey(senderId: null, idx: i), + op: PollVoteOp.add)), + ], + ]; + await prepare(); + // Perform a single-message initial message fetch for [messageList] with + // submessages. + connection.prepare(json: + newestResult(foundOldest: true, messages: []).toJson() + ..['messages'] = [{ + ...message.toJson(), + "submessages": submessages.map(deepToJson).toList(), + }]); + await messageList.fetchInitial(); + checkNotifiedOnce(); + return message; + } + + test('message is unknown', () async { + await prepare(); + await store.handleEvent(eg.submessageEvent(1000, eg.selfUser.userId, + content: PollQuestionEventSubmessage(question: 'New question'))); + checkNotNotified(); + }); + + test('message has no submessages', () async { + final message = eg.streamMessage(); + await prepare(); + await prepareMessages([message]); + await store.handleEvent(eg.submessageEvent(message.id, eg.otherUser.userId, + content: PollQuestionEventSubmessage(question: 'New question'))); + checkNotNotified(); + check(store.messages[message.id]).isNotNull().poll.isNull(); + }); + + test('ignore submessage event with malformed content', () async { + final message = await preparePollMessage(question: 'Old question'); + await store.handleEvent(SubmessageEvent( + id: 0, msgType: SubmessageType.widget, submessageId: 123, + messageId: message.id, + senderId: eg.selfUser.userId, + content: jsonEncode({ + 'type': 'question', + // Invalid type for question + 'question': 100, + }))); + checkNotifiedOnce(); + checkPoll(message).question.equals('Old question'); + }); + + group('question event', () { + test('update question', () async { + final message = await preparePollMessage(question: 'Old question'); + await store.handleEvent(eg.submessageEvent(message.id, eg.selfUser.userId, + content: PollQuestionEventSubmessage(question: 'New question'))); + checkNotifiedOnce(); + checkPoll(message).question.equals('New question'); + }); + + test('unauthorized question edits', () async { + final message = await preparePollMessage( + question: 'Old question', + messageSender: eg.otherUser, + ); + checkPoll(message).question.equals('Old question'); + await store.handleEvent(eg.submessageEvent(message.id, eg.selfUser.userId, + content: PollQuestionEventSubmessage(question: 'edit'))); + checkPoll(message).question.equals('Old question'); + }); + }); + + group('new option event', () { + late Message message; + + Future handleNewOptionEvent(User sender, { + required String option, + required int idx, + }) async { + await store.handleEvent(eg.submessageEvent(message.id, sender.userId, + content: PollNewOptionEventSubmessage(option: option, idx: idx))); + checkNotifiedOnce(); + } + + test('add option', () async { + message = await preparePollMessage( + options: [eg.pollOption(text: 'bar', voters: [])]); + await handleNewOptionEvent(eg.otherUser, option: 'baz', idx: 0); + checkPoll(message).options.deepEquals([ + conditionPollOption('bar'), + conditionPollOption('baz'), + ]); + }); + + test('option with duplicate text ignored', () async { + message = await preparePollMessage( + options: [eg.pollOption(text: 'existing', voters: [])]); + checkPoll(message).options.deepEquals([conditionPollOption('existing')]); + await handleNewOptionEvent(eg.otherUser, option: 'existing', idx: 0); + checkPoll(message).options.deepEquals([conditionPollOption('existing')]); + }); + + test('option index limit exceeded', () async{ + message = await preparePollMessage( + question: 'favorite number', + options: List.generate(1001, (i) => eg.pollOption(text: '$i', voters: [])), + ); + checkPoll(message).options.length.equals(1001); + await handleNewOptionEvent(eg.otherUser, option: 'baz', idx: 1001); + checkPoll(message).options.length.equals(1001); + }); + }); + + group('vote event', () { + late Message message; + + Future handleVoteEvent(String key, PollVoteOp op, User voter) async { + await store.handleEvent(eg.submessageEvent(message.id, voter.userId, + content: PollVoteEventSubmessage(key: key, op: op))); + checkNotifiedOnce(); + } + + test('add votes', () async { + message = await preparePollMessage(); + + String optionKey(int index) => + PollEventSubmessage.optionKey(senderId: null, idx: index); + + await handleVoteEvent(optionKey(0), PollVoteOp.add, eg.otherUser); + checkPoll(message).options.deepEquals([ + conditionPollOption('foo', voters: [eg.otherUser.userId]), + conditionPollOption('bar', voters: []), + ]); + + await handleVoteEvent(optionKey(1), PollVoteOp.add, eg.otherUser); + checkPoll(message).options.deepEquals([ + conditionPollOption('foo', voters: [eg.otherUser.userId]), + conditionPollOption('bar', voters: [eg.otherUser.userId]), + ]); + + await handleVoteEvent(optionKey(0), PollVoteOp.add, eg.selfUser); + checkPoll(message).options.deepEquals([ + conditionPollOption('foo', voters: [eg.otherUser.userId, eg.selfUser.userId]), + conditionPollOption('bar', voters: [eg.otherUser.userId]), + ]); + }); + + test('remove votes', () async { + message = await preparePollMessage(options: [ + eg.pollOption(text: 'foo', voters: [eg.otherUser.userId, eg.selfUser.userId]), + eg.pollOption(text: 'bar', voters: [eg.selfUser.userId]), + ]); + + String optionKey(int index) => + PollEventSubmessage.optionKey(senderId: null, idx: index); + + await handleVoteEvent(optionKey(0), PollVoteOp.remove, eg.otherUser); + checkPoll(message).options.deepEquals([ + conditionPollOption('foo', voters: [eg.selfUser.userId]), + conditionPollOption('bar', voters: [eg.selfUser.userId]), + ]); + + await handleVoteEvent(optionKey(1), PollVoteOp.remove, eg.selfUser); + checkPoll(message).options.deepEquals([ + conditionPollOption('foo', voters: [eg.selfUser.userId]), + conditionPollOption('bar', voters: []), + ]); + }); + + test('vote for unknown options', () async { + message = await preparePollMessage(options: [ + eg.pollOption(text: 'foo', voters: [eg.selfUser.userId]), + eg.pollOption(text: 'bar', voters: []), + ]); + + final unknownOptionKey = PollEventSubmessage.optionKey( + senderId: eg.selfUser.userId, + idx: 10, + ); + + await handleVoteEvent(unknownOptionKey, PollVoteOp.remove, eg.selfUser); + checkPoll(message).options.deepEquals([ + conditionPollOption('foo', voters: [eg.selfUser.userId]), + conditionPollOption('bar', voters: []), + ]); + + await handleVoteEvent(unknownOptionKey, PollVoteOp.add, eg.selfUser); + checkPoll(message).options.deepEquals([ + conditionPollOption('foo', voters: [eg.selfUser.userId]), + conditionPollOption('bar', voters: []), + ]); + }); + + test('ignore invalid vote op', () async { + message = await preparePollMessage( + options: [eg.pollOption(text: 'foo', voters: [])]); + checkPoll(message).options.deepEquals([conditionPollOption('foo')]); + await handleVoteEvent( + PollEventSubmessage.optionKey(senderId: null, idx: 0), + PollVoteOp.unknown, eg.otherUser); + checkPoll(message).options.deepEquals([conditionPollOption('foo')]); + }); + }); + }); + group('handleMessageEvent with initial submessages', () { late Message message;