From 4d9497512e70401cbf559c4749a599a22b9a4693 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 10 Jul 2024 14:46:57 -0400 Subject: [PATCH 1/4] api: Start using 'direct' for MessageType. We still offer support for 'private', which gets converted to 'direct' as well when deserialized. Signed-off-by: Zixuan James Li --- lib/api/model/events.dart | 27 ++++++++++++++++--- lib/api/model/events.g.dart | 19 ++++++------- lib/model/narrow.dart | 2 +- lib/model/unreads.dart | 4 +-- test/api/model/events_checks.dart | 12 +++++++++ test/api/model/events_test.dart | 45 ++++++++++++++++++++++++------- test/example_data.dart | 2 +- test/model/unreads_test.dart | 8 +++--- 8 files changed, 89 insertions(+), 30 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 6d40b7b586..d5c98e4aa3 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -707,6 +707,9 @@ class DeleteMessageEvent extends Event { final List messageIds; // final int messageId; // Not present; we support the bulk_message_deletion capability + // The server never actually sends "direct" here yet (it's "private" instead), + // but we accept both forms for forward-compatibility. + @MessageTypeConverter() final MessageType messageType; final int? streamId; final String? topic; @@ -735,10 +738,25 @@ class DeleteMessageEvent extends Event { /// As in [DeleteMessageEvent.messageType] /// or [UpdateMessageFlagsMessageDetail.type]. -@JsonEnum(fieldRename: FieldRename.snake) +@JsonEnum(alwaysCreate: true) enum MessageType { stream, - private; + direct; +} + +class MessageTypeConverter extends JsonConverter { + const MessageTypeConverter(); + + @override + MessageType fromJson(String json) { + if (json == 'private') json = 'direct'; // TODO(server-future) + return $enumDecode(_$MessageTypeEnumMap, json); + } + + @override + String toJson(MessageType object) { + return _$MessageTypeEnumMap[object]!; + } } /// A Zulip event of type `update_message_flags`. @@ -820,6 +838,9 @@ class UpdateMessageFlagsRemoveEvent extends UpdateMessageFlagsEvent { /// As in [UpdateMessageFlagsRemoveEvent.messageDetails]. @JsonSerializable(fieldRename: FieldRename.snake) class UpdateMessageFlagsMessageDetail { + // The server never actually sends "direct" here yet (it's "private" instead), + // but we accept both forms for forward-compatibility. + @MessageTypeConverter() final MessageType type; final bool? mentioned; final List? userIds; @@ -841,7 +862,7 @@ class UpdateMessageFlagsMessageDetail { case MessageType.stream: result.streamId as int; result.topic as String; - case MessageType.private: + case MessageType.direct: result.userIds as List; } return result; diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index e7bdf5228c..6343c0db88 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -438,7 +438,8 @@ DeleteMessageEvent _$DeleteMessageEventFromJson(Map json) => messageIds: (json['message_ids'] as List) .map((e) => (e as num).toInt()) .toList(), - messageType: $enumDecode(_$MessageTypeEnumMap, json['message_type']), + messageType: + const MessageTypeConverter().fromJson(json['message_type'] as String), streamId: (json['stream_id'] as num?)?.toInt(), topic: json['topic'] as String?, ); @@ -448,16 +449,11 @@ Map _$DeleteMessageEventToJson(DeleteMessageEvent instance) => 'id': instance.id, 'type': instance.type, 'message_ids': instance.messageIds, - 'message_type': _$MessageTypeEnumMap[instance.messageType]!, + 'message_type': const MessageTypeConverter().toJson(instance.messageType), 'stream_id': instance.streamId, 'topic': instance.topic, }; -const _$MessageTypeEnumMap = { - MessageType.stream: 'stream', - MessageType.private: 'private', -}; - UpdateMessageFlagsAddEvent _$UpdateMessageFlagsAddEventFromJson( Map json) => UpdateMessageFlagsAddEvent( @@ -511,7 +507,7 @@ Map _$UpdateMessageFlagsRemoveEventToJson( UpdateMessageFlagsMessageDetail _$UpdateMessageFlagsMessageDetailFromJson( Map json) => UpdateMessageFlagsMessageDetail( - type: $enumDecode(_$MessageTypeEnumMap, json['type']), + type: const MessageTypeConverter().fromJson(json['type'] as String), mentioned: json['mentioned'] as bool?, userIds: (json['user_ids'] as List?) ?.map((e) => (e as num).toInt()) @@ -523,7 +519,7 @@ UpdateMessageFlagsMessageDetail _$UpdateMessageFlagsMessageDetailFromJson( Map _$UpdateMessageFlagsMessageDetailToJson( UpdateMessageFlagsMessageDetail instance) => { - 'type': _$MessageTypeEnumMap[instance.type]!, + 'type': const MessageTypeConverter().toJson(instance.type), 'mentioned': instance.mentioned, 'user_ids': instance.userIds, 'stream_id': instance.streamId, @@ -574,3 +570,8 @@ Map _$HeartbeatEventToJson(HeartbeatEvent instance) => 'id': instance.id, 'type': instance.type, }; + +const _$MessageTypeEnumMap = { + MessageType.stream: 'stream', + MessageType.direct: 'direct', +}; diff --git a/lib/model/narrow.dart b/lib/model/narrow.dart index 9c8cacd06d..26630fc1cf 100644 --- a/lib/model/narrow.dart +++ b/lib/model/narrow.dart @@ -204,7 +204,7 @@ class DmNarrow extends Narrow implements SendableNarrow { UpdateMessageFlagsMessageDetail detail, { required int selfUserId, }) { - assert(detail.type == MessageType.private); + assert(detail.type == MessageType.direct); return DmNarrow.withOtherUsers(detail.userIds!, selfUserId: selfUserId); } diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index c1c375c657..6310141c3b 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -288,7 +288,7 @@ class Unreads extends ChangeNotifier { final streamId = event.streamId!; final topic = event.topic!; _removeAllInStreamTopic(messageIdsSet, streamId, topic); - case MessageType.private: + case MessageType.direct: _slowRemoveAllInDms(messageIdsSet); } @@ -363,7 +363,7 @@ class Unreads extends ChangeNotifier { final topics = (newlyUnreadInStreams[detail.streamId!] ??= {}); final messageIds = (topics[detail.topic!] ??= QueueList()); messageIds.add(messageId); - case MessageType.private: + case MessageType.direct: final narrow = DmNarrow.ofUpdateMessageFlagsMessageDetail(selfUserId: selfUserId, detail); (newlyUnreadInDms[narrow] ??= QueueList()) diff --git a/test/api/model/events_checks.dart b/test/api/model/events_checks.dart index bbbc331621..369091c2f9 100644 --- a/test/api/model/events_checks.dart +++ b/test/api/model/events_checks.dart @@ -60,6 +60,18 @@ extension UpdateMessageEventChecks on Subject { Subject get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage'); } +extension DeleteMessageEventChecks on Subject { + Subject get messageType => has((e) => e.messageType, 'messageType'); +} + +extension UpdateMessageFlagsRemoveEventChecks on Subject { + Subject?> get messageDetails => has((e) => e.messageDetails, 'messageDetails'); +} + +extension UpdateMessageFlagsMessageDetailCheck on Subject { + Subject get type => has((e) => e.type, 'type'); +} + extension HeartbeatEventChecks on Subject { // No properties not covered by Event. } diff --git a/test/api/model/events_test.dart b/test/api/model/events_test.dart index 19d91328cf..f7333378ec 100644 --- a/test/api/model/events_test.dart +++ b/test/api/model/events_test.dart @@ -153,7 +153,16 @@ void main() { })).throws(); }); - test('update_message_flags/remove: require messageDetails in mark-as-unread', () { + test('delete_message: private -> direct', () { + check(DeleteMessageEvent.fromJson({ + 'id': 1, + 'type': 'delete_message', + 'message_ids': [1, 2, 3], + 'message_type': 'private', + })).messageType.equals(MessageType.direct); + }); + + group('update_message_flags/remove', () { final baseJson = { 'id': 1, 'type': 'update_message_flags', @@ -162,14 +171,30 @@ void main() { 'messages': [123], 'all': false, }; - check(() => UpdateMessageFlagsRemoveEvent.fromJson(baseJson)).returnsNormally(); - check(() => UpdateMessageFlagsRemoveEvent.fromJson({ - ...baseJson, 'flag': 'read', - })).throws(); - check(() => UpdateMessageFlagsRemoveEvent.fromJson({ - ...baseJson, - 'flag': 'read', - 'message_details': {'123': {'type': 'private', 'mentioned': false, 'user_ids': [2]}}, - })).returnsNormally(); + final messageDetail = {'type': 'direct', 'mentioned': false, 'user_ids': [2]}; + + test('require messageDetails in mark-as-unread', () { + check(() => UpdateMessageFlagsRemoveEvent.fromJson(baseJson)).returnsNormally(); + check(() => UpdateMessageFlagsRemoveEvent.fromJson({ + ...baseJson, 'flag': 'read', + })).throws(); + check(() => UpdateMessageFlagsRemoveEvent.fromJson({ + ...baseJson, + 'flag': 'read', + 'message_details': {'123': messageDetail}, + })).returnsNormally(); + }); + + test('private -> direct', () { + check(UpdateMessageFlagsRemoveEvent.fromJson({ + ...baseJson, + 'flag': 'read', + 'message_details': { + '123': { + ...messageDetail, + 'type': 'private', + }}})).messageDetails.isNotNull() + .values.single.type.equals(MessageType.direct); + }); }); } diff --git a/test/example_data.dart b/test/example_data.dart index b563ad5e7a..f238bb07e9 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -492,7 +492,7 @@ UpdateMessageFlagsRemoveEvent updateMessageFlagsRemoveEvent( userIds: null, ), DmMessage() => UpdateMessageFlagsMessageDetail( - type: MessageType.private, + type: MessageType.direct, mentioned: mentioned, streamId: null, topic: null, diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index e72b3cbca4..254333fb1f 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -449,7 +449,7 @@ void main() { ), DmMessage() => DeleteMessageEvent( id: 0, - messageType: MessageType.private, + messageType: MessageType.direct, messageIds: [message.id], streamId: null, topic: null, @@ -488,7 +488,7 @@ void main() { model.handleDeleteMessageEvent(DeleteMessageEvent( id: 0, messageIds: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], - messageType: MessageType.private, + messageType: MessageType.direct, streamId: null, topic: null, )); @@ -523,7 +523,7 @@ void main() { model.handleDeleteMessageEvent(DeleteMessageEvent( id: 0, messageIds: [message.id], - messageType: MessageType.private, + messageType: MessageType.direct, streamId: null, topic: null, )); @@ -968,7 +968,7 @@ void main() { ), // message 2 and 3 have their details missing message4.id: UpdateMessageFlagsMessageDetail( - type: MessageType.private, + type: MessageType.direct, mentioned: false, streamId: null, topic: null, From ca4c3d0be8cb48c87d688d10dbe1fa549983b9b8 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Mon, 1 Jul 2024 17:01:45 -0400 Subject: [PATCH 2/4] api: Read typing constants from the server. Signed-off-by: Zixuan James Li --- lib/api/model/initial_snapshot.dart | 11 +++++++++++ lib/api/model/initial_snapshot.g.dart | 18 ++++++++++++++++++ test/example_data.dart | 9 +++++++++ 3 files changed, 38 insertions(+) diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index a98ebef5f1..71b317855c 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -24,6 +24,14 @@ class InitialSnapshot { final List customProfileFields; + // TODO(server-8): Remove the default values. + @JsonKey(defaultValue: 15000) + final int serverTypingStartedExpiryPeriodMilliseconds; + @JsonKey(defaultValue: 5000) + final int serverTypingStoppedWaitPeriodMilliseconds; + @JsonKey(defaultValue: 10000) + final int serverTypingStartedWaitPeriodMilliseconds; + // final List<…> mutedTopics; // TODO(#422) we ignore this feature on older servers final Map realmEmoji; @@ -86,6 +94,9 @@ class InitialSnapshot { required this.zulipMergeBase, required this.alertWords, required this.customProfileFields, + required this.serverTypingStartedExpiryPeriodMilliseconds, + required this.serverTypingStoppedWaitPeriodMilliseconds, + required this.serverTypingStartedWaitPeriodMilliseconds, required this.realmEmoji, required this.recentPrivateConversations, required this.subscriptions, diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index ec8f95ea7b..c47680675d 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -21,6 +21,18 @@ InitialSnapshot _$InitialSnapshotFromJson(Map json) => customProfileFields: (json['custom_profile_fields'] as List) .map((e) => CustomProfileField.fromJson(e as Map)) .toList(), + serverTypingStartedExpiryPeriodMilliseconds: + (json['server_typing_started_expiry_period_milliseconds'] as num?) + ?.toInt() ?? + 15000, + serverTypingStoppedWaitPeriodMilliseconds: + (json['server_typing_stopped_wait_period_milliseconds'] as num?) + ?.toInt() ?? + 5000, + serverTypingStartedWaitPeriodMilliseconds: + (json['server_typing_started_wait_period_milliseconds'] as num?) + ?.toInt() ?? + 10000, realmEmoji: (json['realm_emoji'] as Map).map( (k, e) => MapEntry(k, RealmEmojiItem.fromJson(e as Map)), @@ -74,6 +86,12 @@ Map _$InitialSnapshotToJson(InitialSnapshot instance) => 'zulip_merge_base': instance.zulipMergeBase, 'alert_words': instance.alertWords, 'custom_profile_fields': instance.customProfileFields, + 'server_typing_started_expiry_period_milliseconds': + instance.serverTypingStartedExpiryPeriodMilliseconds, + 'server_typing_stopped_wait_period_milliseconds': + instance.serverTypingStoppedWaitPeriodMilliseconds, + 'server_typing_started_wait_period_milliseconds': + instance.serverTypingStartedWaitPeriodMilliseconds, 'realm_emoji': instance.realmEmoji, 'recent_private_conversations': instance.recentPrivateConversations, 'subscriptions': instance.subscriptions, diff --git a/test/example_data.dart b/test/example_data.dart index f238bb07e9..3da2747eb3 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -533,6 +533,9 @@ InitialSnapshot initialSnapshot({ String? zulipMergeBase, List? alertWords, List? customProfileFields, + int? serverTypingStartedExpiryPeriodMilliseconds, + int? serverTypingStoppedWaitPeriodMilliseconds, + int? serverTypingStartedWaitPeriodMilliseconds, Map? realmEmoji, List? recentPrivateConversations, List? subscriptions, @@ -554,6 +557,12 @@ InitialSnapshot initialSnapshot({ zulipMergeBase: zulipMergeBase ?? recentZulipVersion, alertWords: alertWords ?? ['klaxon'], customProfileFields: customProfileFields ?? [], + serverTypingStartedExpiryPeriodMilliseconds: + serverTypingStartedExpiryPeriodMilliseconds ?? 15000, + serverTypingStoppedWaitPeriodMilliseconds: + serverTypingStoppedWaitPeriodMilliseconds ?? 5000, + serverTypingStartedWaitPeriodMilliseconds: + serverTypingStartedWaitPeriodMilliseconds ?? 10000, realmEmoji: realmEmoji ?? {}, recentPrivateConversations: recentPrivateConversations ?? [], subscriptions: subscriptions ?? [], // TODO add subscriptions to default From fb36a8a21050423ac148fcb3efe581ae0dae90ad Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Mon, 1 Jul 2024 18:08:42 -0400 Subject: [PATCH 3/4] api: Add typing events. Signed-off-by: Zixuan James Li --- lib/api/model/events.dart | 69 ++++++++++++++++++++++++++++++- lib/api/model/events.g.dart | 28 +++++++++++++ test/api/model/events_checks.dart | 8 ++++ test/api/model/events_test.dart | 45 ++++++++++++++++++++ 4 files changed, 148 insertions(+), 2 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index d5c98e4aa3..7ef38b4260 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -62,6 +62,7 @@ sealed class Event { case 'remove': return UpdateMessageFlagsRemoveEvent.fromJson(json); default: return UnexpectedEvent.fromJson(json); } + case 'typing': return TypingEvent.fromJson(json); case 'reaction': return ReactionEvent.fromJson(json); case 'heartbeat': return HeartbeatEvent.fromJson(json); // TODO add many more event types @@ -736,8 +737,9 @@ class DeleteMessageEvent extends Event { Map toJson() => _$DeleteMessageEventToJson(this); } -/// As in [DeleteMessageEvent.messageType] -/// or [UpdateMessageFlagsMessageDetail.type]. +/// As in [DeleteMessageEvent.messageType], +/// [UpdateMessageFlagsMessageDetail.type], +/// or [TypingEvent.messageType]. @JsonEnum(alwaysCreate: true) enum MessageType { stream, @@ -871,6 +873,69 @@ class UpdateMessageFlagsMessageDetail { Map toJson() => _$UpdateMessageFlagsMessageDetailToJson(this); } +/// A Zulip event of type `typing`: +/// https://zulip.com/api/get-events#typing-start +/// https://zulip.com/api/get-events#typing-stop +@JsonSerializable(fieldRename: FieldRename.snake) +class TypingEvent extends Event { + @override + @JsonKey(includeToJson: true) + String get type => 'typing'; + + final TypingOp op; + @MessageTypeConverter() + final MessageType messageType; + @JsonKey(readValue: _readSenderId) + final int senderId; + @JsonKey(name: 'recipients', fromJson: _recipientIdsFromJson) + final List? recipientIds; + final int? streamId; + final String? topic; + + TypingEvent({ + required super.id, + required this.op, + required this.messageType, + required this.senderId, + required this.recipientIds, + required this.streamId, + required this.topic, + }); + + static Object? _readSenderId(Map json, String key) { + return (json['sender'] as Map)['user_id']; + } + + static List? _recipientIdsFromJson(Object? json) { + if (json == null) return null; + return (json as List).map( + (item) => (item as Map)['user_id'] as int).toList(); + } + + factory TypingEvent.fromJson(Map json) { + final result = _$TypingEventFromJson(json); + // Crunchy-shell validation + switch (result.messageType) { + case MessageType.stream: + result.streamId as int; + result.topic as String; + case MessageType.direct: + result.recipientIds as List; + } + return result; + } + + @override + Map toJson() => _$TypingEventToJson(this); +} + +/// As in [TypingEvent.op]. +@JsonEnum(fieldRename: FieldRename.snake) +enum TypingOp { + start, + stop +} + /// A Zulip event of type `reaction`, with op `add` or `remove`. /// /// See: diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 6343c0db88..0dbf71f6e8 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -526,6 +526,34 @@ Map _$UpdateMessageFlagsMessageDetailToJson( 'topic': instance.topic, }; +TypingEvent _$TypingEventFromJson(Map json) => TypingEvent( + id: (json['id'] as num).toInt(), + op: $enumDecode(_$TypingOpEnumMap, json['op']), + messageType: + const MessageTypeConverter().fromJson(json['message_type'] as String), + senderId: (TypingEvent._readSenderId(json, 'sender_id') as num).toInt(), + recipientIds: TypingEvent._recipientIdsFromJson(json['recipients']), + streamId: (json['stream_id'] as num?)?.toInt(), + topic: json['topic'] as String?, + ); + +Map _$TypingEventToJson(TypingEvent instance) => + { + 'id': instance.id, + 'type': instance.type, + 'op': _$TypingOpEnumMap[instance.op]!, + 'message_type': const MessageTypeConverter().toJson(instance.messageType), + 'sender_id': instance.senderId, + 'recipients': instance.recipientIds, + 'stream_id': instance.streamId, + 'topic': instance.topic, + }; + +const _$TypingOpEnumMap = { + TypingOp.start: 'start', + TypingOp.stop: 'stop', +}; + ReactionEvent _$ReactionEventFromJson(Map json) => ReactionEvent( id: (json['id'] as num).toInt(), diff --git a/test/api/model/events_checks.dart b/test/api/model/events_checks.dart index 369091c2f9..f95c6b4362 100644 --- a/test/api/model/events_checks.dart +++ b/test/api/model/events_checks.dart @@ -72,6 +72,14 @@ extension UpdateMessageFlagsMessageDetailCheck on Subject get type => has((e) => e.type, 'type'); } +extension TypingEventChecks on Subject { + Subject get messageType => has((e) => e.messageType, 'messageType'); + Subject get senderId => has((e) => e.senderId, 'senderId'); + Subject?> get recipientIds => has((e) => e.recipientIds, 'recipientIds'); + Subject get streamId => has((e) => e.streamId, 'streamId'); + Subject get topic => has((e) => e.topic, 'topic'); +} + extension HeartbeatEventChecks on Subject { // No properties not covered by Event. } diff --git a/test/api/model/events_test.dart b/test/api/model/events_test.dart index f7333378ec..d722e3229c 100644 --- a/test/api/model/events_test.dart +++ b/test/api/model/events_test.dart @@ -197,4 +197,49 @@ void main() { .values.single.type.equals(MessageType.direct); }); }); + + group('typing status event', () { + final baseJson = { + 'id': 1, + 'type': 'typing', + 'op': 'start', + 'sender': {'user_id': 123, 'email': '123@example.com'}, + }; + + final directMessageJson = { + ...baseJson, + 'message_type': 'direct', + 'recipients': [1, 2, 3].map((e) => {'user_id': e, 'email': '$e@example.com'}).toList(), + }; + + test('direct message typing events', () { + check(TypingEvent.fromJson(directMessageJson)) + ..recipientIds.isNotNull().deepEquals([1, 2, 3]) + ..senderId.equals(123); + }); + + test('private type missing recipient', () { + check(() => TypingEvent.fromJson({ + ...baseJson, 'message_type': 'private'})).throws(); + }); + + test('private -> direct', () { + check(TypingEvent.fromJson({ + ...directMessageJson, + 'message_type': 'private', + })).messageType.equals(MessageType.direct); + }); + + test('stream type missing streamId/topic', () { + check(() => TypingEvent.fromJson({ + ...baseJson, 'message_type': 'stream', 'stream_id': 123, 'topic': 'foo'})) + .returnsNormally(); + check(() => TypingEvent.fromJson({ + ...baseJson, 'message_type': 'stream'})).throws(); + check(() => TypingEvent.fromJson({ + ...baseJson, 'message_type': 'stream', 'topic': 'foo'})).throws(); + check(() => TypingEvent.fromJson({ + ...baseJson, 'message_type': 'stream', 'stream_id': 123})).throws(); + }); + }); } From 5148c45e8e85c378d2e16d73238a09d3a5494293 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 2 Jul 2024 18:01:39 -0400 Subject: [PATCH 4/4] model: Add model for handling typing status events. The map of typist-id-to-timer maps design was inspired by the web app's implementation of typing status. We use a nested map instead to avoid introducing a custom hashing algorithm, and to make it easier to query typists in the same narrow. See also: https://github.com/zulip/zulip/blob/09bad82131abdedf253d53b4cb44c8b95f6a49f1/static/js/typing_data.js Signed-off-by: Zixuan James Li --- lib/model/store.dart | 12 ++ lib/model/typing_status.dart | 83 +++++++++++ test/example_data.dart | 16 +++ test/model/typing_status_test.dart | 221 +++++++++++++++++++++++++++++ 4 files changed, 332 insertions(+) create mode 100644 lib/model/typing_status.dart create mode 100644 test/model/typing_status_test.dart diff --git a/lib/model/store.dart b/lib/model/store.dart index b9b3be843b..86d0c8714d 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -24,6 +24,7 @@ import 'message.dart'; import 'message_list.dart'; import 'recent_dm_conversations.dart'; import 'stream.dart'; +import 'typing_status.dart'; import 'unreads.dart'; export 'package:drift/drift.dart' show Value; @@ -242,6 +243,10 @@ class PerAccountStore extends ChangeNotifier with StreamStore, MessageStore { .followedBy(initialSnapshot.realmNonActiveUsers) .followedBy(initialSnapshot.crossRealmBots) .map((user) => MapEntry(user.userId, user))), + typingStatus: TypingStatus( + selfUserId: account.userId, + typingStartedExpiryPeriod: Duration(milliseconds: initialSnapshot.serverTypingStartedExpiryPeriodMilliseconds), + ), streams: streams, messages: MessageStoreImpl(), unreads: Unreads( @@ -266,6 +271,7 @@ class PerAccountStore extends ChangeNotifier with StreamStore, MessageStore { required this.selfUserId, required this.userSettings, required this.users, + required this.typingStatus, required StreamStoreImpl streams, required MessageStoreImpl messages, required this.unreads, @@ -319,6 +325,8 @@ class PerAccountStore extends ChangeNotifier with StreamStore, MessageStore { final Map users; + final TypingStatus typingStatus; + //////////////////////////////// // Streams, topics, and stuff about them. @@ -383,6 +391,7 @@ class PerAccountStore extends ChangeNotifier with StreamStore, MessageStore { recentDmConversationsView.dispose(); unreads.dispose(); _messages.dispose(); + typingStatus.dispose(); super.dispose(); } @@ -485,6 +494,9 @@ class PerAccountStore extends ChangeNotifier with StreamStore, MessageStore { assert(debugLog("server event: update_message_flags/${event.op} ${event.flag.toJson()}")); _messages.handleUpdateMessageFlagsEvent(event); unreads.handleUpdateMessageFlagsEvent(event); + } else if (event is TypingEvent) { + assert(debugLog("server event: typing/${event.op} ${event.messageType}")); + typingStatus.handleTypingEvent(event); } else if (event is ReactionEvent) { assert(debugLog("server event: reaction/${event.op}")); _messages.handleReactionEvent(event); diff --git a/lib/model/typing_status.dart b/lib/model/typing_status.dart new file mode 100644 index 0000000000..d4637c12e6 --- /dev/null +++ b/lib/model/typing_status.dart @@ -0,0 +1,83 @@ +import 'dart:async'; + +import 'package:flutter/foundation.dart'; + +import '../api/model/events.dart'; +import 'narrow.dart'; + +/// The model for tracking the typing status organized by narrows. +/// +/// Listeners are notified when a typist is added or removed from any narrow. +class TypingStatus extends ChangeNotifier { + TypingStatus({ + required this.selfUserId, + required this.typingStartedExpiryPeriod, + }); + + final int selfUserId; + final Duration typingStartedExpiryPeriod; + + Iterable get debugActiveNarrows => _timerMapsByNarrow.keys; + + Iterable typistIdsInNarrow(SendableNarrow narrow) => + _timerMapsByNarrow[narrow]?.keys ?? []; + + // Using SendableNarrow as the key covers the narrows + // where typing notifications are supported (topics and DMs). + final Map> _timerMapsByNarrow = {}; + + @override + void dispose() { + for (final timersByTypistId in _timerMapsByNarrow.values) { + for (final timer in timersByTypistId.values) { + timer.cancel(); + } + } + _timerMapsByNarrow.clear(); + super.dispose(); + } + + bool _addTypist(SendableNarrow narrow, int typistUserId) { + final narrowTimerMap = _timerMapsByNarrow[narrow] ??= {}; + final typistTimer = narrowTimerMap[typistUserId]; + final isNewTypist = typistTimer == null; + typistTimer?.cancel(); + narrowTimerMap[typistUserId] = Timer(typingStartedExpiryPeriod, () { + if (_removeTypist(narrow, typistUserId)) { + notifyListeners(); + } + }); + return isNewTypist; + } + + bool _removeTypist(SendableNarrow narrow, int typistUserId) { + final narrowTimerMap = _timerMapsByNarrow[narrow]; + final typistTimer = narrowTimerMap?.remove(typistUserId); + if (typistTimer == null) { + return false; + } + typistTimer.cancel(); + if (narrowTimerMap!.isEmpty) _timerMapsByNarrow.remove(narrow); + return true; + } + + void handleTypingEvent(TypingEvent event) { + SendableNarrow narrow = switch (event.messageType) { + MessageType.direct => DmNarrow( + allRecipientIds: event.recipientIds!..sort(), selfUserId: selfUserId), + MessageType.stream => TopicNarrow(event.streamId!, event.topic!), + }; + + bool hasUpdate = false; + switch (event.op) { + case TypingOp.start: + hasUpdate = _addTypist(narrow, event.senderId); + case TypingOp.stop: + hasUpdate = _removeTypist(narrow, event.senderId); + } + + if (hasUpdate) { + notifyListeners(); + } + } +} diff --git a/test/example_data.dart b/test/example_data.dart index 3da2747eb3..481c208533 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -504,6 +504,22 @@ UpdateMessageFlagsRemoveEvent updateMessageFlagsRemoveEvent( }))); } +TypingEvent typingEvent(SendableNarrow narrow, TypingOp op, int senderId) { + switch (narrow) { + case TopicNarrow(): + return TypingEvent(id: 1, op: op, senderId: senderId, + messageType: MessageType.stream, + streamId: narrow.streamId, + topic: narrow.topic, + recipientIds: null); + case DmNarrow(): + return TypingEvent(id: 1, op: op, senderId: senderId, + messageType: MessageType.direct, + recipientIds: narrow.allRecipientIds, + streamId: null, + topic: null); + } +} ReactionEvent reactionEvent(Reaction reaction, ReactionOp op, int messageId) { return ReactionEvent( diff --git a/test/model/typing_status_test.dart b/test/model/typing_status_test.dart new file mode 100644 index 0000000000..162e89f23f --- /dev/null +++ b/test/model/typing_status_test.dart @@ -0,0 +1,221 @@ +import 'package:checks/checks.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/model/events.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/narrow.dart'; +import 'package:zulip/model/typing_status.dart'; + +import '../example_data.dart' as eg; +import '../fake_async.dart'; + +void main() { + late TypingStatus model; + late int notifiedCount; + + void checkNotNotified() { + check(notifiedCount).equals(0); + } + + void checkNotifiedOnce() { + check(notifiedCount).equals(1); + notifiedCount = 0; + } + + void handleTypingEvent(SendableNarrow narrow, TypingOp op, User sender) { + assert(sender != eg.selfUser); + model.handleTypingEvent(eg.typingEvent(narrow, op, sender.userId)); + } + + void checkTypists(Map> typistsByNarrow) { + final actualTypistsByNarrow = >{}; + for (final narrow in model.debugActiveNarrows) { + actualTypistsByNarrow[narrow] = model.typistIdsInNarrow(narrow); + } + check(actualTypistsByNarrow).deepEquals( + typistsByNarrow.map((k, v) => MapEntry(k, v.map((e) => e.userId)))); + } + + void prepareModel({ + int? selfUserId, + Map> typistsByNarrow = const {}, + }) { + model = TypingStatus( + selfUserId: selfUserId ?? eg.selfUser.userId, + typingStartedExpiryPeriod: const Duration(milliseconds: 15000)); + check(model.debugActiveNarrows).isEmpty(); + notifiedCount = 0; + model.addListener(() => notifiedCount += 1); + + typistsByNarrow.forEach((narrow, typists) { + for (final typist in typists) { + handleTypingEvent(narrow, TypingOp.start, typist); + checkNotifiedOnce(); + } + }); + checkTypists(typistsByNarrow); + } + + final stream = eg.stream(); + final topicNarrow = TopicNarrow(stream.streamId, 'foo'); + + final dmNarrow = DmNarrow.withUser(eg.otherUser.userId, selfUserId: eg.selfUser.userId); + final groupNarrow = DmNarrow.withOtherUsers( + [eg.otherUser.userId, eg.thirdUser.userId], selfUserId: eg.selfUser.userId); + + test('dispose', () => awaitFakeAsync((async) async { + prepareModel(typistsByNarrow: {dmNarrow: [eg.otherUser]}); + + model.dispose(); + checkTypists({}); + check(async.pendingTimers).isEmpty(); + })); + + group('handle typing start events', () { + test('add typists in separate narrows', () { + prepareModel(); + + handleTypingEvent(dmNarrow, TypingOp.start, eg.otherUser); + checkTypists({dmNarrow: [eg.otherUser]}); + checkNotifiedOnce(); + + handleTypingEvent(groupNarrow, TypingOp.start, eg.thirdUser); + checkTypists({dmNarrow: [eg.otherUser], groupNarrow: [eg.thirdUser]}); + checkNotifiedOnce(); + + handleTypingEvent(topicNarrow, TypingOp.start, eg.fourthUser); + checkTypists({ + dmNarrow: [eg.otherUser], + groupNarrow: [eg.thirdUser], + topicNarrow: [eg.fourthUser]}); + checkNotifiedOnce(); + }); + + test('add a typist in the same narrow', () { + prepareModel(typistsByNarrow: {groupNarrow: [eg.otherUser]}); + + handleTypingEvent(groupNarrow, TypingOp.start, eg.thirdUser); + checkTypists({groupNarrow: [eg.otherUser, eg.thirdUser]}); + checkNotifiedOnce(); + }); + + test('sort dm recipients', () { + prepareModel(selfUserId: 5); + final recipientIds = [5, 4, 10, 8, 2, 1]; + + final eventUnsorted = TypingEvent(id: 1, op: TypingOp.start, + senderId: 1, + messageType: MessageType.direct, + recipientIds: recipientIds, + streamId: null, topic: null); + // DmNarrow's constructor expects the recipient IDs to be sorted, + // and [model.handleTypingEvent] should handle that. + model.handleTypingEvent(eventUnsorted); + check(model.typistIdsInNarrow( + DmNarrow(allRecipientIds: recipientIds..sort(), selfUserId: 5), + )).single.equals(1); + }); + }); + + group('handle typing stop events', () { + test('remove a typist from an unknown narrow', () { + prepareModel(typistsByNarrow: {groupNarrow: [eg.otherUser]}); + + handleTypingEvent(dmNarrow, TypingOp.stop, eg.otherUser); + checkTypists({groupNarrow: [eg.otherUser]}); + checkNotNotified(); + }); + + test('remove an unknown typist from a known narrow', () { + prepareModel(typistsByNarrow: {groupNarrow: [eg.otherUser]}); + + handleTypingEvent(groupNarrow, TypingOp.stop, eg.thirdUser); + checkTypists({groupNarrow: [eg.otherUser]}); + checkNotNotified(); + }); + + test('remove one of two typists in the same narrow', () { + prepareModel(typistsByNarrow: { + groupNarrow: [eg.otherUser, eg.thirdUser], + }); + + handleTypingEvent(groupNarrow, TypingOp.stop, eg.otherUser); + checkTypists({groupNarrow: [eg.thirdUser]}); + checkNotifiedOnce(); + }); + + test('remove all typists in a narrow', () { + prepareModel(typistsByNarrow: {dmNarrow: [eg.otherUser]}); + + handleTypingEvent(dmNarrow, TypingOp.stop, eg.otherUser); + checkTypists({}); + checkNotifiedOnce(); + }); + + test('remove typists from different narrows', () { + prepareModel(typistsByNarrow: { + dmNarrow: [eg.otherUser], + groupNarrow: [eg.thirdUser], + topicNarrow: [eg.fourthUser], + }); + + handleTypingEvent(groupNarrow, TypingOp.stop, eg.thirdUser); + checkTypists({dmNarrow: [eg.otherUser], topicNarrow: [eg.fourthUser]}); + checkNotifiedOnce(); + + handleTypingEvent(dmNarrow, TypingOp.stop, eg.otherUser); + checkTypists({topicNarrow: [eg.fourthUser]}); + checkNotifiedOnce(); + + handleTypingEvent(topicNarrow, TypingOp.stop, eg.fourthUser); + checkTypists({}); + checkNotifiedOnce(); + }); + }); + + group('typing start expiry period', () { + test('typist is removed when the expiry period ends', () => awaitFakeAsync((async) async { + prepareModel(typistsByNarrow: {dmNarrow: [eg.otherUser]}); + + async.elapse(const Duration(seconds: 5)); + checkTypists({dmNarrow: [eg.otherUser]}); + checkNotNotified(); + + async.elapse(const Duration(seconds: 10)); + checkTypists({}); + check(async.pendingTimers).isEmpty(); + checkNotifiedOnce(); + })); + + test('early typing stop event cancels the timer', () => awaitFakeAsync((async) async { + prepareModel(typistsByNarrow: {dmNarrow: [eg.otherUser]}); + check(async.pendingTimers).length.equals(1); + + handleTypingEvent(dmNarrow, TypingOp.stop, eg.otherUser); + checkTypists({}); + check(async.pendingTimers).isEmpty(); + checkNotifiedOnce(); + })); + + test('repeated typing start event resets the timer', () => awaitFakeAsync((async) async { + prepareModel(typistsByNarrow: {dmNarrow: [eg.otherUser]}); + check(async.pendingTimers).length.equals(1); + + async.elapse(const Duration(seconds: 10)); + checkTypists({dmNarrow: [eg.otherUser]}); + // The timer should restart from the event. + handleTypingEvent(dmNarrow, TypingOp.start, eg.otherUser); + check(async.pendingTimers).length.equals(1); + checkNotNotified(); + + async.elapse(const Duration(seconds: 10)); + checkTypists({dmNarrow: [eg.otherUser]}); + check(async.pendingTimers).length.equals(1); + checkNotNotified(); + + async.elapse(const Duration(seconds: 5)); + checkTypists({}); + check(async.pendingTimers).isEmpty(); + checkNotifiedOnce(); + })); + }); +}