diff --git a/lib/model/message.dart b/lib/model/message.dart new file mode 100644 index 0000000000..6396ef28ed --- /dev/null +++ b/lib/model/message.dart @@ -0,0 +1,196 @@ +import '../api/model/events.dart'; +import '../api/model/model.dart'; +import 'message_list.dart'; + +/// The portion of [PerAccountStore] for messages and message lists. +mixin MessageStore { + /// All known messages, indexed by [Message.id]. + Map get messages; + + void registerMessageList(MessageListView view); + void unregisterMessageList(MessageListView view); + + /// Reconcile a batch of just-fetched messages with the store, + /// mutating the list. + /// + /// This is called after a [getMessages] request to report the result + /// to the store. + /// + /// The list's length will not change, but some entries may be replaced + /// by a different [Message] object with the same [Message.id]. + /// All [Message] objects in the resulting list will be present in + /// [this.messages]. + void reconcileMessages(List messages); +} + +class MessageStoreImpl with MessageStore { + MessageStoreImpl() + // There are no messages in InitialSnapshot, so we don't have + // a use case for initializing MessageStore with nonempty [messages]. + : messages = {}; + + @override + final Map messages; + + final Set _messageListViews = {}; + + @override + void registerMessageList(MessageListView view) { + final added = _messageListViews.add(view); + assert(added); + } + + @override + void unregisterMessageList(MessageListView view) { + final removed = _messageListViews.remove(view); + assert(removed); + } + + void reassemble() { + for (final view in _messageListViews) { + view.reassemble(); + } + } + + void dispose() { + for (final view in _messageListViews) { + view.dispose(); + } + } + + @override + void reconcileMessages(List messages) { + // What to do when some of the just-fetched messages are already known? + // This is common and normal: in particular it happens when one message list + // overlaps another, e.g. a stream and a topic within it. + // + // Most often, the just-fetched message will look just like the one we + // already have. But they can differ: message fetching happens out of band + // from the event queue, so there's inherently a race. + // + // If the fetched message reflects changes we haven't yet heard from the + // event queue, then it doesn't much matter which version we use: we'll + // soon get the corresponding events and apply the changes anyway. + // But if it lacks changes we've already heard from the event queue, then + // we won't hear those events again; the only way to wind up with an + // updated message is to use the version we have, that already reflects + // those events' changes. So we always stick with the version we have. + for (int i = 0; i < messages.length; i++) { + final message = messages[i]; + messages[i] = this.messages.putIfAbsent(message.id, () => message); + } + } + + void handleMessageEvent(MessageEvent event) { + // If the message is one we already know about (from a fetch), + // clobber it with the one from the event system. + // See [fetchedMessages] for reasoning. + messages[event.message.id] = event.message; + + for (final view in _messageListViews) { + view.handleMessageEvent(event); + } + } + + void handleUpdateMessageEvent(UpdateMessageEvent event) { + _handleUpdateMessageEventContent(event); + // TODO(#150): Handle message moves. The views' recipient headers + // may need updating, and consequently showSender too. + } + + void _handleUpdateMessageEventContent(UpdateMessageEvent event) { + final message = messages[event.messageId]; + if (message == null) return; + + // TODO(server-5): Cut this fallback; rely on renderingOnly from FL 114 + final isRenderingOnly = event.renderingOnly ?? (event.userId == null); + if (event.editTimestamp != null && !isRenderingOnly) { + // A rendering-only update gets omitted from the message edit history, + // and [Message.lastEditTimestamp] is the last timestamp of that history. + // So on a rendering-only update, the timestamp doesn't get updated. + message.lastEditTimestamp = event.editTimestamp; + } + + message.flags = event.flags; + + if (event.renderedContent != null) { + assert(message.contentType == 'text/html', + "Message contentType was ${message.contentType}; expected text/html."); + message.content = event.renderedContent!; + } + + if (event.isMeMessage != null) { + message.isMeMessage = event.isMeMessage!; + } + + for (final view in _messageListViews) { + view.messageContentChanged(event.messageId); + } + } + + void handleDeleteMessageEvent(DeleteMessageEvent event) { + // TODO handle DeleteMessageEvent, particularly in MessageListView + } + + void handleUpdateMessageFlagsEvent(UpdateMessageFlagsEvent event) { + final isAdd = switch (event) { + UpdateMessageFlagsAddEvent() => true, + UpdateMessageFlagsRemoveEvent() => false, + }; + + if (isAdd && (event as UpdateMessageFlagsAddEvent).all) { + for (final message in messages.values) { + message.flags.add(event.flag); + } + + for (final view in _messageListViews) { + if (view.messages.isEmpty) continue; + view.notifyListeners(); + } + } else { + bool anyMessageFound = false; + for (final messageId in event.messages) { + final message = messages[messageId]; + if (message == null) continue; // a message we don't know about yet + anyMessageFound = true; + + isAdd + ? message.flags.add(event.flag) + : message.flags.remove(event.flag); + } + if (anyMessageFound) { + for (final view in _messageListViews) { + view.notifyListenersIfAnyMessagePresent(event.messages); + } + } + } + } + + void handleReactionEvent(ReactionEvent event) { + final message = messages[event.messageId]; + if (message == null) return; + + switch (event.op) { + case ReactionOp.add: + (message.reactions ??= Reactions([])).add(Reaction( + emojiName: event.emojiName, + emojiCode: event.emojiCode, + reactionType: event.reactionType, + userId: event.userId, + )); + case ReactionOp.remove: + if (message.reactions == null) { // TODO(log) + return; + } + message.reactions!.remove( + reactionType: event.reactionType, + emojiCode: event.emojiCode, + userId: event.userId, + ); + } + + for (final view in _messageListViews) { + view.notifyListenersIfMessagePresent(event.messageId); + } + } +} diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 3dce9ac584..09c780f35e 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -378,6 +378,7 @@ class MessageListView with ChangeNotifier, _MessageSequence { numBefore: kMessageListFetchBatchSize, numAfter: 0, ); + store.reconcileMessages(result.messages); for (final message in result.messages) { if (_messageVisible(message)) { _addMessage(message); @@ -413,6 +414,8 @@ class MessageListView with ChangeNotifier, _MessageSequence { result.messages.removeLast(); } + store.reconcileMessages(result.messages); + final fetchedMessages = _allMessagesVisible ? result.messages // Avoid unnecessarily copying the list. : result.messages.where(_messageVisible); @@ -426,10 +429,9 @@ class MessageListView with ChangeNotifier, _MessageSequence { } } - /// Add [message] to this view, if it belongs here. - /// - /// Called in particular when we get a [MessageEvent]. - void maybeAddMessage(Message message) { + /// Add [MessageEvent.message] to this view, if it belongs here. + void handleMessageEvent(MessageEvent event) { + final message = event.message; if (!narrow.containsMessage(message) || !_messageVisible(message)) { return; } @@ -442,104 +444,35 @@ class MessageListView with ChangeNotifier, _MessageSequence { notifyListeners(); } - static void _applyChangesToMessage(UpdateMessageEvent event, Message message) { - // TODO(server-5): Cut this fallback; rely on renderingOnly from FL 114 - final isRenderingOnly = event.renderingOnly ?? (event.userId == null); - if (event.editTimestamp != null && !isRenderingOnly) { - // A rendering-only update gets omitted from the message edit history, - // and [Message.lastEditTimestamp] is the last timestamp of that history. - // So on a rendering-only update, the timestamp doesn't get updated. - message.lastEditTimestamp = event.editTimestamp; - } - - message.flags = event.flags; - - if (event.renderedContent != null) { - assert(message.contentType == 'text/html', - "Message contentType was ${message.contentType}; expected text/html."); - message.content = event.renderedContent!; - } - - if (event.isMeMessage != null) { - message.isMeMessage = event.isMeMessage!; - } + // Repeal the `@protected` annotation that applies on the base implementation, + // so we can call this method from [MessageStoreImpl]. + @override + void notifyListeners() { + super.notifyListeners(); } - /// Update the message the given event applies to, if present in this view. - /// - /// This method only handles the case where the message's contents - /// were changed, and ignores any changes to its stream or topic. - /// - /// TODO(#150): Handle message moves. - // NB that when handling message moves (#150), recipient headers - // may need updating, and consequently showSender too. - void maybeUpdateMessage(UpdateMessageEvent event) { - final idx = _findMessageWithId(event.messageId); - if (idx == -1) { - return; + /// Notify listeners if the given message is present in this view. + void notifyListenersIfMessagePresent(int messageId) { + final index = _findMessageWithId(messageId); + if (index != -1) { + notifyListeners(); } - - _applyChangesToMessage(event, messages[idx]); - _reparseContent(idx); - notifyListeners(); } - void maybeUpdateMessageFlags(UpdateMessageFlagsEvent event) { - final isAdd = switch (event) { - UpdateMessageFlagsAddEvent() => true, - UpdateMessageFlagsRemoveEvent() => false, - }; - - bool didUpdateAny = false; - if (isAdd && (event as UpdateMessageFlagsAddEvent).all) { - for (final message in messages) { - message.flags.add(event.flag); - didUpdateAny = true; - } - } else { - for (final messageId in event.messages) { - final index = _findMessageWithId(messageId); - if (index != -1) { - final message = messages[index]; - isAdd ? message.flags.add(event.flag) : message.flags.remove(event.flag); - didUpdateAny = true; - } - } - } - if (!didUpdateAny) { - return; + /// Notify listeners if any of the given messages is present in this view. + void notifyListenersIfAnyMessagePresent(Iterable messageIds) { + final isAnyPresent = messageIds.any((id) => _findMessageWithId(id) != -1); + if (isAnyPresent) { + notifyListeners(); } - - notifyListeners(); } - void maybeUpdateMessageReactions(ReactionEvent event) { - final index = _findMessageWithId(event.messageId); - if (index == -1) { - return; - } - - final message = messages[index]; - switch (event.op) { - case ReactionOp.add: - (message.reactions ??= Reactions([])).add(Reaction( - emojiName: event.emojiName, - emojiCode: event.emojiCode, - reactionType: event.reactionType, - userId: event.userId, - )); - case ReactionOp.remove: - if (message.reactions == null) { // TODO(log) - return; - } - message.reactions!.remove( - reactionType: event.reactionType, - emojiCode: event.emojiCode, - userId: event.userId, - ); + void messageContentChanged(int messageId) { + final index = _findMessageWithId(messageId); + if (index != -1) { + _reparseContent(index); + notifyListeners(); } - - notifyListeners(); } /// Called when the app is reassembled during debugging, e.g. for hot reload. diff --git a/lib/model/store.dart b/lib/model/store.dart index 3e31b3bfaa..baae805051 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -20,6 +20,7 @@ import '../log.dart'; import '../notifications/receive.dart'; import 'autocomplete.dart'; import 'database.dart'; +import 'message.dart'; import 'message_list.dart'; import 'recent_dm_conversations.dart'; import 'stream.dart'; @@ -199,7 +200,7 @@ abstract class GlobalStore extends ChangeNotifier { /// This class does not attempt to poll an event queue /// to keep the data up to date. For that behavior, see /// [UpdateMachine]. -class PerAccountStore extends ChangeNotifier with StreamStore { +class PerAccountStore extends ChangeNotifier with StreamStore, MessageStore { /// Construct a store for the user's data, starting from the given snapshot. /// /// The global store must already have been updated with @@ -242,6 +243,7 @@ class PerAccountStore extends ChangeNotifier with StreamStore { .followedBy(initialSnapshot.crossRealmBots) .map((user) => MapEntry(user.userId, user))), streams: streams, + messages: MessageStoreImpl(), unreads: Unreads( initial: initialSnapshot.unreadMsgs, selfUserId: account.userId, @@ -265,13 +267,15 @@ class PerAccountStore extends ChangeNotifier with StreamStore { required this.userSettings, required this.users, required StreamStoreImpl streams, + required MessageStoreImpl messages, required this.unreads, required this.recentDmConversationsView, }) : assert(selfUserId == globalStore.getAccount(accountId)!.userId), assert(realmUrl == globalStore.getAccount(accountId)!.realmUrl), assert(realmUrl == connection.realmUrl), _globalStore = globalStore, - _streams = streams; + _streams = streams, + _messages = messages; //////////////////////////////////////////////////////////////// // Data. @@ -336,21 +340,26 @@ class PerAccountStore extends ChangeNotifier with StreamStore { //////////////////////////////// // Messages, and summaries of messages. - final Unreads unreads; - - final RecentDmConversationsView recentDmConversationsView; + @override + Map get messages => _messages.messages; + @override + void registerMessageList(MessageListView view) => + _messages.registerMessageList(view); + @override + void unregisterMessageList(MessageListView view) => + _messages.unregisterMessageList(view); + @override + void reconcileMessages(List messages) { + _messages.reconcileMessages(messages); + // TODO(#649) notify [unreads] of the just-fetched messages + // TODO(#650) notify [recentDmConversationsView] of the just-fetched messages + } - final Set _messageListViews = {}; + final MessageStoreImpl _messages; - void registerMessageList(MessageListView view) { - final added = _messageListViews.add(view); - assert(added); - } + final Unreads unreads; - void unregisterMessageList(MessageListView view) { - final removed = _messageListViews.remove(view); - assert(removed); - } + final RecentDmConversationsView recentDmConversationsView; //////////////////////////////// // Other digests of data. @@ -365,19 +374,15 @@ class PerAccountStore extends ChangeNotifier with StreamStore { /// This will redo from scratch any computations we can, such as parsing /// message contents. It won't repeat network requests. void reassemble() { - for (final view in _messageListViews) { - view.reassemble(); - } + _messages.reassemble(); autocompleteViewManager.reassemble(); } @override void dispose() { - unreads.dispose(); recentDmConversationsView.dispose(); - for (final view in _messageListViews.toList()) { - view.dispose(); - } + unreads.dispose(); + _messages.dispose(); super.dispose(); } @@ -464,32 +469,26 @@ class PerAccountStore extends ChangeNotifier with StreamStore { notifyListeners(); } else if (event is MessageEvent) { assert(debugLog("server event: message ${jsonEncode(event.message.toJson())}")); - recentDmConversationsView.handleMessageEvent(event); - for (final view in _messageListViews) { - view.maybeAddMessage(event.message); - } + _messages.handleMessageEvent(event); unreads.handleMessageEvent(event); + recentDmConversationsView.handleMessageEvent(event); + // When adding anything here (to handle [MessageEvent]), + // it probably belongs in [reconcileMessages] too. } else if (event is UpdateMessageEvent) { assert(debugLog("server event: update_message ${event.messageId}")); - for (final view in _messageListViews) { - view.maybeUpdateMessage(event); - } + _messages.handleUpdateMessageEvent(event); unreads.handleUpdateMessageEvent(event); } else if (event is DeleteMessageEvent) { assert(debugLog("server event: delete_message ${event.messageIds}")); - // TODO handle in message lists + _messages.handleDeleteMessageEvent(event); unreads.handleDeleteMessageEvent(event); } else if (event is UpdateMessageFlagsEvent) { assert(debugLog("server event: update_message_flags/${event.op} ${event.flag.toJson()}")); - for (final view in _messageListViews) { - view.maybeUpdateMessageFlags(event); - } + _messages.handleUpdateMessageFlagsEvent(event); unreads.handleUpdateMessageFlagsEvent(event); } else if (event is ReactionEvent) { assert(debugLog("server event: reaction/${event.op}")); - for (final view in _messageListViews) { - view.maybeUpdateMessageReactions(event); - } + _messages.handleReactionEvent(event); } else if (event is UnexpectedEvent) { assert(debugLog("server event: ${jsonEncode(event.toJson())}")); // TODO log better } else { diff --git a/test/example_data.dart b/test/example_data.dart index 2ea23f9bf0..67ba1aaa3c 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -457,6 +457,19 @@ UpdateMessageFlagsRemoveEvent updateMessageFlagsRemoveEvent( }))); } + +ReactionEvent reactionEvent(Reaction reaction, ReactionOp op, int messageId) { + return ReactionEvent( + id: 1, + op: op, + emojiName: reaction.emojiName, + emojiCode: reaction.emojiCode, + reactionType: reaction.reactionType, + userId: reaction.userId, + messageId: messageId, + ); +} + //////////////////////////////////////////////////////////////// // The entire per-account or global state. // diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 3ba9d49c3f..c818970c30 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -7,6 +7,7 @@ import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/model/narrow.dart'; import 'package:zulip/api/route/messages.dart'; +import 'package:zulip/model/algorithms.dart'; import 'package:zulip/model/content.dart'; import 'package:zulip/model/message_list.dart'; import 'package:zulip/model/narrow.dart'; @@ -19,7 +20,7 @@ import '../stdlib_checks.dart'; import 'content_checks.dart'; import 'test_store.dart'; -void main() async { +void main() { // These variables are the common state operated on by each test. // Each test case calls [prepare] to initialize them. late Subscription subscription; @@ -233,19 +234,20 @@ void main() async { ..messages.length.equals(200); }); - test('maybeAddMessage', () async { + test('MessageEvent', () async { final stream = eg.stream(); await prepare(narrow: StreamNarrow(stream.streamId)); await prepareMessages(foundOldest: true, messages: List.generate(30, (i) => eg.streamMessage(stream: stream))); check(model).messages.length.equals(30); - model.maybeAddMessage(eg.streamMessage(stream: stream)); + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(stream: stream))); checkNotifiedOnce(); check(model).messages.length.equals(31); }); - test('maybeAddMessage, not in narrow', () async { + test('MessageEvent, not in narrow', () async { final stream = eg.stream(streamId: 123); await prepare(narrow: StreamNarrow(stream.streamId)); await prepareMessages(foundOldest: true, messages: @@ -253,281 +255,217 @@ void main() async { check(model).messages.length.equals(30); final otherStream = eg.stream(streamId: 234); - model.maybeAddMessage(eg.streamMessage(stream: otherStream)); + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(stream: otherStream))); checkNotNotified(); check(model).messages.length.equals(30); }); - test('maybeAddMessage, before fetch', () async { + test('MessageEvent, before fetch', () async { final stream = eg.stream(); await prepare(narrow: StreamNarrow(stream.streamId)); - model.maybeAddMessage(eg.streamMessage(stream: stream)); + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(stream: stream))); checkNotNotified(); check(model).fetched.isFalse(); checkInvariants(model); }); - group('maybeUpdateMessage', () { - test('update a message', () async { - final originalMessage = eg.streamMessage( - content: "

Hello, world

"); - final updateEvent = eg.updateMessageEditEvent(originalMessage, - flags: [MessageFlag.starred], - renderedContent: "

Hello, edited

", - editTimestamp: 99999, - isMeMessage: true, - ); - await prepare(); - await prepareMessages(foundOldest: true, messages: [originalMessage]); - - final message = model.messages.single; - check(message) - ..content.not((it) => it.equals(updateEvent.renderedContent!)) - ..lastEditTimestamp.isNull() - ..flags.not((it) => it.deepEquals(updateEvent.flags)) - ..isMeMessage.not((it) => it.equals(updateEvent.isMeMessage!)); - - model.maybeUpdateMessage(updateEvent); + group('notifyListenersIfMessagePresent', () { + test('message present', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMessages(foundOldest: false, + messages: List.generate(100, (i) => eg.streamMessage(id: 100 + i))); + model.notifyListenersIfMessagePresent(150); checkNotifiedOnce(); - check(model).messages.single - ..identicalTo(message) - ..content.equals(updateEvent.renderedContent!) - ..lastEditTimestamp.equals(updateEvent.editTimestamp) - ..flags.equals(updateEvent.flags) - ..isMeMessage.equals(updateEvent.isMeMessage!); }); - test('ignore when message not present', () async { - final originalMessage = eg.streamMessage( - content: "

Hello, world

"); - final updateEvent = eg.updateMessageEditEvent(originalMessage, - messageId: originalMessage.id + 1, - renderedContent: "

Hello, edited

", - ); - await prepare(); - await prepareMessages(foundOldest: true, messages: [originalMessage]); + test('message absent', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMessages(foundOldest: false, + messages: List.generate(100, (i) => eg.streamMessage(id: 100 + i)) + .where((m) => m.id != 150).toList()); + model.notifyListenersIfMessagePresent(150); + checkNotNotified(); + }); - model.maybeUpdateMessage(updateEvent); + test('message absent (older than window)', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMessages(foundOldest: false, + messages: List.generate(100, (i) => eg.streamMessage(id: 100 + i))); + model.notifyListenersIfMessagePresent(50); checkNotNotified(); - check(model).messages.single - ..content.equals(originalMessage.content) - ..content.not((it) => it.equals(updateEvent.renderedContent!)); }); - // TODO(server-5): Cut legacy case for rendering-only message update - Future checkRenderingOnly({required bool legacy}) async { - final originalMessage = eg.streamMessage( - lastEditTimestamp: 78492, - content: "

Hello, world

"); - final updateEvent = eg.updateMessageEditEvent(originalMessage, - renderedContent: "

Hello, world

Some link preview
", - editTimestamp: 99999, - renderingOnly: legacy ? null : true, - userId: null, - ); - await prepare(); - await prepareMessages(foundOldest: true, messages: [originalMessage]); - final message = model.messages.single; + test('message absent (newer than window)', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMessages(foundOldest: false, + messages: List.generate(100, (i) => eg.streamMessage(id: 100 + i))); + model.notifyListenersIfMessagePresent(250); + checkNotNotified(); + }); + }); - model.maybeUpdateMessage(updateEvent); + group('notifyListenersIfAnyMessagePresent', () { + final messages = List.generate(100, (i) => eg.streamMessage(id: 100 + i)); + + test('all messages present', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMessages(foundOldest: false, messages: messages); + model.notifyListenersIfAnyMessagePresent([150, 151, 152]); checkNotifiedOnce(); - check(model).messages.single - ..identicalTo(message) - // Content is updated... - ..content.equals(updateEvent.renderedContent!) - // ... edit timestamp is not. - ..lastEditTimestamp.equals(originalMessage.lastEditTimestamp) - ..lastEditTimestamp.not((it) => it.equals(updateEvent.editTimestamp)); - } + }); - test('rendering-only update does not change timestamp', () async { - await checkRenderingOnly(legacy: false); + test('some messages present', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMessages(foundOldest: false, + messages: messages.where((m) => m.id != 151).toList()); + model.notifyListenersIfAnyMessagePresent([150, 151, 152]); + checkNotifiedOnce(); }); - test('rendering-only update does not change timestamp (for old server versions)', () async { - await checkRenderingOnly(legacy: true); + test('no messages present', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMessages(foundOldest: false, messages: + messages.where((m) => ![150, 151, 152].contains(m.id)).toList()); + model.notifyListenersIfAnyMessagePresent([150, 151, 152]); + checkNotNotified(); }); + }); - group('ReactionEvent handling', () { - ReactionEvent mkEvent(Reaction reaction, ReactionOp op, int messageId) { - return ReactionEvent( - id: 1, - op: op, - emojiName: reaction.emojiName, - emojiCode: reaction.emojiCode, - reactionType: reaction.reactionType, - userId: reaction.userId, - messageId: messageId, - ); - } + group('messageContentChanged', () { + test('message present', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMessages(foundOldest: false, + messages: List.generate(10, (i) => eg.streamMessage(id: 10 + i))); - test('add reaction', () async { - final originalMessage = eg.streamMessage(reactions: []); - await prepare(); - await prepareMessages(foundOldest: true, messages: [originalMessage]); - final message = model.messages.single; + final message = model.messages[5]; + await store.handleEvent(eg.updateMessageEditEvent(message, + renderedContent: '${message.content}

edited.generate(10, + (i) => eg.streamMessage(id: 10 + i, stream: stream)); + check(messagesInNarrow.every(narrow.containsMessage)).isTrue(); - test('remove reaction', () async { - final eventReaction = Reaction(reactionType: ReactionType.unicodeEmoji, - emojiName: 'wave', emojiCode: '1f44b', userId: 1); - - // Same emoji, different user. Not to be removed. - final reaction2 = Reaction(reactionType: ReactionType.unicodeEmoji, - emojiName: 'wave', emojiCode: '1f44b', userId: 2); - - // Same user, different emoji. Not to be removed. - final reaction3 = Reaction(reactionType: ReactionType.unicodeEmoji, - emojiName: 'working_on_it', emojiCode: '1f6e0', userId: 1); - - // Same user, same emojiCode, different emojiName. To be removed: servers - // key on user, message, reaction type, and emoji code, but not emoji name. - // So we mimic that behavior; see discussion: - // https://github.com/zulip/zulip-flutter/pull/256#discussion_r1284865099 - final reaction4 = Reaction(reactionType: ReactionType.unicodeEmoji, - emojiName: 'hello', emojiCode: '1f44b', userId: 1); - - final originalMessage = eg.streamMessage( - reactions: [reaction2, reaction3, reaction4]); - await prepare(); - await prepareMessages(foundOldest: true, messages: [originalMessage]); - final message = model.messages.single; - - model.maybeUpdateMessageReactions( - mkEvent(eventReaction, ReactionOp.remove, originalMessage.id)); - checkNotifiedOnce(); - check(model).messages.single - ..identicalTo(message) - ..reactions.isNotNull().jsonEquals([reaction2, reaction3]); - }); + final messageNotInNarrow = eg.dmMessage(id: 100, from: eg.otherUser, to: [eg.selfUser]); + check(narrow.containsMessage(messageNotInNarrow)).isFalse(); - test('remove reaction; message is not in list', () async { - final someMessage = eg.streamMessage(reactions: [eg.unicodeEmojiReaction]); - await prepare(); - await prepareMessages(foundOldest: true, messages: [someMessage]); - model.maybeUpdateMessageReactions( - mkEvent(eg.unicodeEmojiReaction, ReactionOp.remove, 1000)); - checkNotNotified(); - check(model).messages.single.reactions.isNotNull().jsonEquals([eg.unicodeEmojiReaction]); - }); + await prepareMessages(foundOldest: false, messages: messagesInNarrow); + await store.addMessage(messageNotInNarrow); + + await store.handleEvent(eg.updateMessageEditEvent(messageNotInNarrow, + renderedContent: '${messageNotInNarrow.content}

edited messageIds, { - bool all = false, - }) { - return UpdateMessageFlagsAddEvent( - id: 1, - flag: flag, - messages: messageIds, - all: all, - ); - } + group('regression tests for #455', () { + test('reaction events handled once, even when message is in two message lists', () async { + final stream = eg.stream(); + store = eg.store(); + await store.addStream(stream); + await store.addSubscription(eg.subscription(stream)); + connection = store.connection as FakeApiConnection; + + int notifiedCount1 = 0; + final model1 = MessageListView.init(store: store, + narrow: StreamNarrow(stream.streamId)) + ..addListener(() => notifiedCount1++); + + int notifiedCount2 = 0; + final model2 = MessageListView.init(store: store, + narrow: TopicNarrow(stream.streamId, 'hello')) + ..addListener(() => notifiedCount2++); + + for (final m in [model1, model2]) { + connection.prepare(json: newestResult( + foundOldest: false, + messages: [eg.streamMessage(stream: stream, topic: 'hello')]).toJson()); + await m.fetchInitial(); + } - const mkRemoveEvent = eg.updateMessageFlagsRemoveEvent; + final message = eg.streamMessage(stream: stream, topic: 'hello'); + await store.handleEvent(MessageEvent(id: 0, message: message)); - group('add flag', () { - test('not in list', () async { - await prepare(); - final message = eg.streamMessage(flags: []); - await prepareMessages(foundOldest: true, messages: [message]); - model.maybeUpdateMessageFlags(mkAddEvent(MessageFlag.read, [2])); - checkNotNotified(); - check(model).messages.single.flags.deepEquals([]); - }); + await store.handleEvent( + eg.reactionEvent(eg.unicodeEmojiReaction, ReactionOp.add, message.id)); - test('affected message, unaffected message, absent message', () async { - await prepare(); - final message1 = eg.streamMessage(flags: []); - final message2 = eg.streamMessage(flags: []); - await prepareMessages(foundOldest: true, messages: [message1, message2]); - model.maybeUpdateMessageFlags(mkAddEvent(MessageFlag.read, [message2.id, 3])); - checkNotifiedOnce(); - check(model).messages - ..[0].flags.deepEquals([]) - ..[1].flags.deepEquals([MessageFlag.read]); - }); + check(notifiedCount1).equals(3); // fetch, new-message event, reaction event + check(notifiedCount2).equals(3); // fetch, new-message event, reaction event - test('all: true, list non-empty', () async { - await prepare(); - final message1 = eg.streamMessage(flags: []); - final message2 = eg.streamMessage(flags: []); - await prepareMessages(foundOldest: true, messages: [message1, message2]); - model.maybeUpdateMessageFlags(mkAddEvent(MessageFlag.read, [], all: true)); - checkNotifiedOnce(); - check(model).messages - ..[0].flags.deepEquals([MessageFlag.read]) - ..[1].flags.deepEquals([MessageFlag.read]); - }); + check(model1.messages.last) + ..identicalTo(model2.messages.last) + ..reactions.isNotNull().total.equals(1); + }); - test('all: true, list empty', () async { - await prepare(); - await prepareMessages(foundOldest: true, messages: []); - model.maybeUpdateMessageFlags(mkAddEvent(MessageFlag.read, [], all: true)); - checkNotNotified(); - }); + Future checkApplied({ + required Event Function(Message) mkEvent, + required void Function(Subject) doCheckMessageAfterFetch, + }) async { + final stream = eg.stream(); + store = eg.store(); + await store.addStream(stream); + await store.addSubscription(eg.subscription(stream)); + connection = store.connection as FakeApiConnection; - test('other flags not clobbered', () async { - final message = eg.streamMessage(flags: [MessageFlag.starred]); - await prepare(); - await prepareMessages(foundOldest: true, messages: [message]); - model.maybeUpdateMessageFlags(mkAddEvent(MessageFlag.read, [message.id])); - checkNotifiedOnce(); - check(model).messages.single.flags.deepEquals([MessageFlag.starred, MessageFlag.read]); - }); - }); + final message = eg.streamMessage(stream: stream); - group('remove flag', () { - test('not in list', () async { - await prepare(); - final message = eg.streamMessage(flags: [MessageFlag.read]); - await prepareMessages(foundOldest: true, messages: [message]); - model.maybeUpdateMessageFlags(mkAddEvent(MessageFlag.read, [2])); - checkNotNotified(); - check(model).messages.single.flags.deepEquals([MessageFlag.read]); - }); + await store.addMessage(Message.fromJson(message.toJson())); + await store.handleEvent(mkEvent(message)); - test('affected message, unaffected message, absent message', () async { - await prepare(); - final message1 = eg.streamMessage(flags: [MessageFlag.read]); - final message2 = eg.streamMessage(flags: [MessageFlag.read]); - final message3 = eg.streamMessage(flags: [MessageFlag.read]); - await prepareMessages(foundOldest: true, messages: [message1, message2]); - model.maybeUpdateMessageFlags(mkRemoveEvent(MessageFlag.read, [message2, message3])); - checkNotifiedOnce(); - check(model).messages - ..[0].flags.deepEquals([MessageFlag.read]) - ..[1].flags.deepEquals([]); - }); + // init msglist *after* event was handled + model = MessageListView.init(store: store, narrow: const CombinedFeedNarrow()); + checkInvariants(model); - test('other flags not affected', () async { - final message = eg.streamMessage(flags: [MessageFlag.starred, MessageFlag.read]); - await prepare(); - await prepareMessages(foundOldest: true, messages: [message]); - model.maybeUpdateMessageFlags(mkRemoveEvent(MessageFlag.read, [message])); - checkNotifiedOnce(); - check(model).messages.single.flags.deepEquals([MessageFlag.starred]); - }); + connection.prepare(json: + newestResult(foundOldest: false, messages: [message]).toJson()); + await model.fetchInitial(); + checkInvariants(model); + doCheckMessageAfterFetch( + check(model).messages.single + ..id.equals(message.id) + ); + } + + test('ReactionEvent is applied even when message not in any msglists', () async { + await checkApplied( + mkEvent: (message) => + eg.reactionEvent(eg.unicodeEmojiReaction, ReactionOp.add, message.id), + doCheckMessageAfterFetch: + (messageSubject) => messageSubject.reactions.isNotNull().total.equals(1), + ); + }); + + test('UpdateMessageEvent (edit) is applied even when message not in any msglists', () async { + await checkApplied( + mkEvent: (message) { + final newContent = '${message.content}

edited

'; + return eg.updateMessageEditEvent(message, + renderedContent: newContent); + }, + doCheckMessageAfterFetch: + (messageSubject) => messageSubject.content.endsWith('

edited

'), + ); + }); + + test('UpdateMessageFlagsEvent is applied even when message not in any msglists', () async { + await checkApplied( + mkEvent: (message) => UpdateMessageFlagsAddEvent( + id: 1, + flag: MessageFlag.starred, + messages: [message.id], + all: false, + ), + doCheckMessageAfterFetch: + (messageSubject) => messageSubject.flags.contains(MessageFlag.starred), + ); }); }); @@ -536,7 +474,8 @@ void main() async { await prepare(narrow: StreamNarrow(stream.streamId)); await prepareMessages(foundOldest: true, messages: List.generate(30, (i) => eg.streamMessage(stream: stream))); - model.maybeAddMessage(eg.streamMessage(stream: stream)); + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(stream: stream))); checkNotifiedOnce(); check(model).messages.length.equals(31); @@ -591,24 +530,29 @@ void main() async { check(model.messages.map((m) => m.id)) .deepEquals(expected..insertAll(0, [101, 103, 105])); - // … and on maybeAddMessage. - model.maybeAddMessage(eg.streamMessage(id: 301, stream: stream1, topic: 'A')); + // … and on MessageEvent. + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(id: 301, stream: stream1, topic: 'A'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(301)); - model.maybeAddMessage(eg.streamMessage(id: 302, stream: stream1, topic: 'B')); + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(id: 302, stream: stream1, topic: 'B'))); checkNotNotified(); check(model.messages.map((m) => m.id)).deepEquals(expected); - model.maybeAddMessage(eg.streamMessage(id: 303, stream: stream2, topic: 'C')); + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(id: 303, stream: stream2, topic: 'C'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(303)); - model.maybeAddMessage(eg.streamMessage(id: 304, stream: stream2, topic: 'D')); + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(id: 304, stream: stream2, topic: 'D'))); checkNotNotified(); check(model.messages.map((m) => m.id)).deepEquals(expected); - model.maybeAddMessage(eg.dmMessage(id: 305, from: eg.otherUser, to: [eg.selfUser])); + await store.handleEvent(MessageEvent(id: 0, + message: eg.dmMessage(id: 305, from: eg.otherUser, to: [eg.selfUser]))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(305)); }); @@ -643,16 +587,19 @@ void main() async { check(model.messages.map((m) => m.id)) .deepEquals(expected..insertAll(0, [101, 102])); - // … and on maybeAddMessage. - model.maybeAddMessage(eg.streamMessage(id: 301, stream: stream, topic: 'A')); + // … and on MessageEvent. + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(id: 301, stream: stream, topic: 'A'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(301)); - model.maybeAddMessage(eg.streamMessage(id: 302, stream: stream, topic: 'B')); + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(id: 302, stream: stream, topic: 'B'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(302)); - model.maybeAddMessage(eg.streamMessage(id: 303, stream: stream, topic: 'C')); + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(id: 303, stream: stream, topic: 'C'))); checkNotNotified(); check(model.messages.map((m) => m.id)).deepEquals(expected); }); @@ -682,8 +629,9 @@ void main() async { check(model.messages.map((m) => m.id)) .deepEquals(expected..insertAll(0, [101])); - // … and on maybeAddMessage. - model.maybeAddMessage(eg.streamMessage(id: 301, stream: stream, topic: 'A')); + // … and on MessageEvent. + await store.handleEvent(MessageEvent(id: 0, + message: eg.streamMessage(id: 301, stream: stream, topic: 'A'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(301)); }); @@ -736,25 +684,25 @@ void main() async { await model.fetchOlder(); checkNotified(count: 2); - // Then test maybeAddMessage, where a new header is needed… - model.maybeAddMessage(streamMessage(13)); + // Then test MessageEvent, where a new header is needed… + await store.handleEvent(MessageEvent(id: 0, message: streamMessage(13))); checkNotifiedOnce(); // … and where it's not. - model.maybeAddMessage(streamMessage(14)); + await store.handleEvent(MessageEvent(id: 0, message: streamMessage(14))); checkNotifiedOnce(); - // Then test maybeUpdateMessage, where a header is and remains needed… + // Then test UpdateMessageEvent edits, where a header is and remains needed… UpdateMessageEvent updateEvent(Message message) => eg.updateMessageEditEvent( message, renderedContent: '${message.content}

edited

', ); - model.maybeUpdateMessage(updateEvent(model.messages.first)); + await store.handleEvent(updateEvent(model.messages.first)); checkNotifiedOnce(); - model.maybeUpdateMessage(updateEvent(model.messages[model.messages.length - 2])); + await store.handleEvent(updateEvent(model.messages[model.messages.length - 2])); checkNotifiedOnce(); // … and where it's not. - model.maybeUpdateMessage(updateEvent(model.messages.last)); + await store.handleEvent(updateEvent(model.messages.last)); checkNotifiedOnce(); // Then test reassemble. @@ -780,7 +728,7 @@ void main() async { // Until then, we always compute this sequentially from oldest to newest. // So we just need to exercise the different cases of the logic for // whether the sender should be shown, but the difference between - // fetchInitial and maybeAddMessage etc. doesn't matter. + // fetchInitial and handleMessageEvent etc. doesn't matter. const t1 = 1693602618; const t2 = t1 + 86400; @@ -919,6 +867,7 @@ void checkInvariants(MessageListView model) { } for (final message in model.messages) { + check(model.store.messages)[message.id].isNotNull().identicalTo(message); check(model.narrow.containsMessage(message)).isTrue(); if (message is! StreamMessage) continue; @@ -934,9 +883,8 @@ void checkInvariants(MessageListView model) { } } - for (int i = 0; i < model.messages.length - 1; i++) { - check(model.messages[i].id).isLessThan(model.messages[i+1].id); - } + check(isSortedWithoutDuplicates(model.messages.map((m) => m.id).toList())) + .isTrue(); check(model).contents.length.equals(model.messages.length); for (int i = 0; i < model.contents.length; i++) { diff --git a/test/model/message_test.dart b/test/model/message_test.dart new file mode 100644 index 0000000000..dad8e15783 --- /dev/null +++ b/test/model/message_test.dart @@ -0,0 +1,420 @@ +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/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 '../example_data.dart' as eg; +import '../stdlib_checks.dart'; +import 'message_list_test.dart'; +import 'store_checks.dart'; +import 'test_store.dart'; + +void main() { + // These "late" variables are the common state operated on by each test. + // Each test case calls [prepare] to initialize them. + late Subscription subscription; + late PerAccountStore store; + late FakeApiConnection connection; + // [messageList] is here only for the sake of checking when it notifies. + // For anything deeper than that, use `message_list_test.dart`. + late MessageListView messageList; + late int notifiedCount; + + void checkNotified({required int count}) { + check(notifiedCount).equals(count); + notifiedCount = 0; + } + void checkNotNotified() => checkNotified(count: 0); + void checkNotifiedOnce() => checkNotified(count: 1); + + /// Initialize [store] and the rest of the test state. + Future prepare({Narrow narrow = const CombinedFeedNarrow()}) async { + final stream = eg.stream(); + subscription = eg.subscription(stream); + store = eg.store(); + await store.addStream(stream); + await store.addSubscription(subscription); + connection = store.connection as FakeApiConnection; + notifiedCount = 0; + messageList = MessageListView.init(store: store, narrow: narrow) + ..addListener(() { + notifiedCount++; + }); + check(messageList).fetched.isFalse(); + checkNotNotified(); + } + + /// Perform the initial message fetch for [messageList]. + /// + /// The test case must have already called [prepare] to initialize the state. + Future prepareMessages( + List messages, { + bool foundOldest = false, + }) async { + connection.prepare(json: + newestResult(foundOldest: foundOldest, messages: messages).toJson()); + await messageList.fetchInitial(); + checkNotifiedOnce(); + } + + Future addMessages(Iterable messages) async { + for (final m in messages) { + await store.handleEvent(MessageEvent(id: 0, message: m)); + } + checkNotified(count: messageList.fetched ? messages.length : 0); + } + + group('reconcileMessages', () { + test('from empty', () async { + await prepare(); + check(store.messages).isEmpty(); + final message1 = eg.streamMessage(); + final message2 = eg.streamMessage(); + final message3 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]); + final messages = [message1, message2, message3]; + store.reconcileMessages(messages); + check(messages).deepEquals( + [message1, message2, message3] + .map((m) => (Subject it) => it.identicalTo(m))); + check(store.messages).deepEquals({ + for (final m in messages) m.id: m, + }); + }); + + test('from not-empty', () async { + await prepare(); + final message1 = eg.streamMessage(); + final message2 = eg.streamMessage(); + final message3 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]); + final messages = [message1, message2, message3]; + await addMessages(messages); + final newMessage = eg.streamMessage(); + store.reconcileMessages([newMessage]); + check(messages).deepEquals( + [message1, message2, message3] + .map((m) => (Subject it) => it.identicalTo(m))); + check(store.messages).deepEquals({ + for (final m in messages) m.id: m, + newMessage.id: newMessage, + }); + }); + + test('on ID collision, new message does not clobber old in store.messages', () async { + await prepare(); + final message = eg.streamMessage(id: 1, content: '

foo

'); + await addMessages([message]); + check(store.messages).deepEquals({1: message}); + final newMessage = eg.streamMessage(id: 1, content: '

bar

'); + final messages = [newMessage]; + store.reconcileMessages(messages); + check(messages).single.identicalTo(message); + check(store.messages).deepEquals({1: message}); + }); + }); + + group('handleMessageEvent', () { + test('from empty', () async { + await prepare(); + check(store.messages).isEmpty(); + + final newMessage = eg.streamMessage(); + await store.handleEvent(MessageEvent(id: 1, message: newMessage)); + check(store.messages).deepEquals({ + newMessage.id: newMessage, + }); + }); + + test('from not-empty', () async { + await prepare(); + final messages = [ + eg.streamMessage(), + eg.streamMessage(), + eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]), + ]; + await addMessages(messages); + check(store.messages).deepEquals({ + for (final m in messages) m.id: m, + }); + + final newMessage = eg.streamMessage(); + await store.handleEvent(MessageEvent(id: 1, message: newMessage)); + check(store.messages).deepEquals({ + for (final m in messages) m.id: m, + newMessage.id: newMessage, + }); + }); + + test('new message clobbers old on ID collision', () async { + await prepare(); + final message = eg.streamMessage(id: 1, content: '

foo

'); + await addMessages([message]); + check(store.messages).deepEquals({1: message}); + + final newMessage = eg.streamMessage(id: 1, content: '

bar

'); + await store.handleEvent(MessageEvent(id: 1, message: newMessage)); + check(store.messages).deepEquals({1: newMessage}); + }); + }); + + group('handleUpdateMessageEvent', () { + test('update a message', () async { + final originalMessage = eg.streamMessage( + content: "

Hello, world

"); + final updateEvent = eg.updateMessageEditEvent(originalMessage, + flags: [MessageFlag.starred], + renderedContent: "

Hello, edited

", + editTimestamp: 99999, + isMeMessage: true, + ); + await prepare(); + await prepareMessages([originalMessage]); + + final message = store.messages.values.single; + check(message) + ..content.not((it) => it.equals(updateEvent.renderedContent!)) + ..lastEditTimestamp.isNull() + ..flags.not((it) => it.deepEquals(updateEvent.flags)) + ..isMeMessage.not((it) => it.equals(updateEvent.isMeMessage!)); + + await store.handleEvent(updateEvent); + checkNotifiedOnce(); + check(store).messages.values.single + ..identicalTo(message) + ..content.equals(updateEvent.renderedContent!) + ..lastEditTimestamp.equals(updateEvent.editTimestamp) + ..flags.equals(updateEvent.flags) + ..isMeMessage.equals(updateEvent.isMeMessage!); + }); + + test('ignore when message unknown', () async { + final originalMessage = eg.streamMessage( + content: "

Hello, world

"); + final updateEvent = eg.updateMessageEditEvent(originalMessage, + messageId: originalMessage.id + 1, + renderedContent: "

Hello, edited

", + ); + await prepare(); + await prepareMessages([originalMessage]); + + await store.handleEvent(updateEvent); + checkNotNotified(); + check(store).messages.values.single + ..content.equals(originalMessage.content) + ..content.not((it) => it.equals(updateEvent.renderedContent!)); + }); + + // TODO(server-5): Cut legacy case for rendering-only message update + Future checkRenderingOnly({required bool legacy}) async { + final originalMessage = eg.streamMessage( + lastEditTimestamp: 78492, + content: "

Hello, world

"); + final updateEvent = eg.updateMessageEditEvent(originalMessage, + renderedContent: "

Hello, world

Some link preview
", + editTimestamp: 99999, + renderingOnly: legacy ? null : true, + userId: null, + ); + await prepare(); + await prepareMessages([originalMessage]); + final message = store.messages.values.single; + + await store.handleEvent(updateEvent); + checkNotifiedOnce(); + check(store).messages.values.single + ..identicalTo(message) + // Content is updated... + ..content.equals(updateEvent.renderedContent!) + // ... edit timestamp is not. + ..lastEditTimestamp.equals(originalMessage.lastEditTimestamp) + ..lastEditTimestamp.not((it) => it.equals(updateEvent.editTimestamp)); + } + + test('rendering-only update does not change timestamp', () async { + await checkRenderingOnly(legacy: false); + }); + + test('rendering-only update does not change timestamp (for old server versions)', () async { + await checkRenderingOnly(legacy: true); + }); + }); + + group('handleUpdateMessageFlagsEvent', () { + UpdateMessageFlagsAddEvent mkAddEvent( + MessageFlag flag, + List messageIds, { + bool all = false, + }) { + return UpdateMessageFlagsAddEvent( + id: 1, + flag: flag, + messages: messageIds, + all: all, + ); + } + + const mkRemoveEvent = eg.updateMessageFlagsRemoveEvent; + + group('add flag', () { + test('message is unknown', () async { + await prepare(); + final message = eg.streamMessage(flags: []); + await prepareMessages([message]); + await store.handleEvent(mkAddEvent(MessageFlag.read, [2])); + checkNotNotified(); + check(store).messages.values.single.flags.deepEquals([]); + }); + + test('affected message, unaffected message, absent message', () async { + await prepare(); + final message1 = eg.streamMessage(flags: []); + final message2 = eg.streamMessage(flags: []); + await prepareMessages([message1, message2]); + await store.handleEvent(mkAddEvent(MessageFlag.read, [message2.id, 3])); + checkNotifiedOnce(); + check(store).messages + ..[message1.id].flags.deepEquals([]) + ..[message2.id].flags.deepEquals([MessageFlag.read]); + }); + + test('all: true; we have some known messages', () async { + await prepare(); + final message1 = eg.streamMessage(flags: []); + final message2 = eg.streamMessage(flags: []); + await prepareMessages([message1, message2]); + await store.handleEvent(mkAddEvent(MessageFlag.read, [], all: true)); + checkNotifiedOnce(); + check(store).messages + ..[message1.id].flags.deepEquals([MessageFlag.read]) + ..[message2.id].flags.deepEquals([MessageFlag.read]); + }); + + test('all: true; we don\'t know about any messages', () async { + await prepare(); + await prepareMessages([]); + await store.handleEvent(mkAddEvent(MessageFlag.read, [], all: true)); + checkNotNotified(); + }); + + test('other flags not clobbered', () async { + final message = eg.streamMessage(flags: [MessageFlag.starred]); + await prepare(); + await prepareMessages([message]); + await store.handleEvent(mkAddEvent(MessageFlag.read, [message.id])); + checkNotifiedOnce(); + check(store).messages.values + .single.flags.deepEquals([MessageFlag.starred, MessageFlag.read]); + }); + }); + + group('remove flag', () { + test('message is unknown', () async { + await prepare(); + final message = eg.streamMessage(flags: [MessageFlag.read]); + await prepareMessages([message]); + await store.handleEvent(mkAddEvent(MessageFlag.read, [2])); + checkNotNotified(); + check(store).messages.values + .single.flags.deepEquals([MessageFlag.read]); + }); + + test('affected message, unaffected message, absent message', () async { + await prepare(); + final message1 = eg.streamMessage(flags: [MessageFlag.read]); + final message2 = eg.streamMessage(flags: [MessageFlag.read]); + final message3 = eg.streamMessage(flags: [MessageFlag.read]); + await prepareMessages([message1, message2]); + await store.handleEvent(mkRemoveEvent(MessageFlag.read, [message2, message3])); + checkNotifiedOnce(); + check(store).messages + ..[message1.id].flags.deepEquals([MessageFlag.read]) + ..[message2.id].flags.deepEquals([]); + }); + + test('other flags not affected', () async { + final message = eg.streamMessage(flags: [MessageFlag.starred, MessageFlag.read]); + await prepare(); + await prepareMessages([message]); + await store.handleEvent(mkRemoveEvent(MessageFlag.read, [message])); + checkNotifiedOnce(); + check(store).messages.values + .single.flags.deepEquals([MessageFlag.starred]); + }); + }); + }); + + group('handleReactionEvent', () { + test('add reaction', () async { + final originalMessage = eg.streamMessage(reactions: []); + await prepare(); + await prepareMessages([originalMessage]); + final message = store.messages.values.single; + + await store.handleEvent( + eg.reactionEvent(eg.unicodeEmojiReaction, ReactionOp.add, originalMessage.id)); + checkNotifiedOnce(); + check(store.messages).values.single + ..identicalTo(message) + ..reactions.isNotNull().jsonEquals([eg.unicodeEmojiReaction]); + }); + + test('add reaction; message is unknown', () async { + final someMessage = eg.streamMessage(reactions: []); + await prepare(); + await prepareMessages([someMessage]); + await store.handleEvent( + eg.reactionEvent(eg.unicodeEmojiReaction, ReactionOp.add, 1000)); + checkNotNotified(); + check(store.messages).values.single + .reactions.isNull(); + }); + + test('remove reaction', () async { + final eventReaction = Reaction(reactionType: ReactionType.unicodeEmoji, + emojiName: 'wave', emojiCode: '1f44b', userId: 1); + + // Same emoji, different user. Not to be removed. + final reaction2 = Reaction(reactionType: ReactionType.unicodeEmoji, + emojiName: 'wave', emojiCode: '1f44b', userId: 2); + + // Same user, different emoji. Not to be removed. + final reaction3 = Reaction(reactionType: ReactionType.unicodeEmoji, + emojiName: 'working_on_it', emojiCode: '1f6e0', userId: 1); + + // Same user, same emojiCode, different emojiName. To be removed: servers + // key on user, message, reaction type, and emoji code, but not emoji name. + // So we mimic that behavior; see discussion: + // https://github.com/zulip/zulip-flutter/pull/256#discussion_r1284865099 + final reaction4 = Reaction(reactionType: ReactionType.unicodeEmoji, + emojiName: 'hello', emojiCode: '1f44b', userId: 1); + + final originalMessage = eg.streamMessage( + reactions: [reaction2, reaction3, reaction4]); + await prepare(); + await prepareMessages([originalMessage]); + final message = store.messages.values.single; + + await store.handleEvent( + eg.reactionEvent(eventReaction, ReactionOp.remove, originalMessage.id)); + checkNotifiedOnce(); + check(store.messages).values.single + ..identicalTo(message) + ..reactions.isNotNull().jsonEquals([reaction2, reaction3]); + }); + + test('remove reaction; message is unknown', () async { + final someMessage = eg.streamMessage(reactions: [eg.unicodeEmojiReaction]); + await prepare(); + await prepareMessages([someMessage]); + await store.handleEvent( + eg.reactionEvent(eg.unicodeEmojiReaction, ReactionOp.remove, 1000)); + checkNotNotified(); + check(store.messages).values.single + .reactions.isNotNull().jsonEquals([eg.unicodeEmojiReaction]); + }); + }); +} diff --git a/test/model/store_checks.dart b/test/model/store_checks.dart index b1ee950fe2..e019dfac6d 100644 --- a/test/model/store_checks.dart +++ b/test/model/store_checks.dart @@ -42,6 +42,7 @@ extension PerAccountStoreChecks on Subject { Subject> get streams => has((x) => x.streams, 'streams'); Subject> get streamsByName => has((x) => x.streamsByName, 'streamsByName'); Subject> get subscriptions => has((x) => x.subscriptions, 'subscriptions'); + Subject> get messages => has((x) => x.messages, 'messages'); Subject get unreads => has((x) => x.unreads, 'unreads'); Subject get recentDmConversationsView => has((x) => x.recentDmConversationsView, 'recentDmConversationsView'); Subject get autocompleteViewManager => has((x) => x.autocompleteViewManager, 'autocompleteViewManager'); diff --git a/test/model/test_store.dart b/test/model/test_store.dart index 285e620d4f..d68b6c1a46 100644 --- a/test/model/test_store.dart +++ b/test/model/test_store.dart @@ -154,4 +154,14 @@ extension PerAccountStoreTestExtension on PerAccountStore { visibilityPolicy: visibilityPolicy, )); } + + Future addMessage(Message message) async { + await handleEvent(MessageEvent(id: 1, message: message)); + } + + Future addMessages(Iterable messages) async { + for (final message in messages) { + await addMessage(message); + } + } }