diff --git a/lib/model/message.dart b/lib/model/message.dart index 90a0c28840..936bc3bb1f 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -408,29 +408,87 @@ class MessageStoreImpl extends HasChannelStore with MessageStore, _OutboxMessage void reconcileMessages(List messages) { assert(!_disposed); - // What to do when some of the just-fetched messages are already known? + for (int i = 0; i < messages.length; i++) { + final message = messages[i]; + messages[i] = this.messages.update(message.id, + ifAbsent: () => _reconcileUnrecognizedMessage(message), + (current) => _reconcileRecognizedMessage(current, message)); + } + } + + Message _reconcileUnrecognizedMessage(Message incoming) { + if ( + incoming is StreamMessage + && subscriptions[incoming.streamId] == null + ) { + // The message is in an unsubscribed channel. It might grow stale; + // add it to _maybeStaleChannelMessages. + _maybeStaleChannelMessages.add(incoming.id); + } + return _stripMatchFields(incoming); + } + + Message _reconcileRecognizedMessage(Message current, Message incoming) { + // This just-fetched message is one we already know about. // 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.matchContent = null; - message.matchTopic = null; - return message; - }); + // already have. But not always, and we can choose intelligently whether + // to keep the stored version or clobber it with the incoming one. + + bool currentIsMaybeStale = false; + if (incoming is StreamMessage) { + if (subscriptions[incoming.streamId] != null) { + // The incoming version won't grow stale; it's in a subscribed channel. + // Remove it from _maybeStaleChannelMessages if it was there. + currentIsMaybeStale = _maybeStaleChannelMessages.remove(incoming.id); + } else { + assert(_maybeStaleChannelMessages.contains(incoming.id)); + currentIsMaybeStale = true; + } } + + if (currentIsMaybeStale) { + // The event queue is unreliable for this message; the message was in an + // unsubscribed channel when we stored it or sometime since, so the stored + // version might be stale. Refresh it with the fetched version. + return _stripMatchFields(incoming); + } else { + // 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, stick with the version we have. + return current; + } + } + + /// Messages in [messages] whose data stream is or was presumably broken + /// by the message being in an unsubscribed channel. + /// + /// This is the subset of [messages] where the message was + /// in an unsubscribed channel when we added it or sometime since. + /// + /// We don't expect update events for messages in unsubscribed channels, + /// so if some of these maybe-stale messages appear in a fetch, + /// we'll always clobber our stored version with the fetched version. + /// See [reconcileMessages]. + /// + /// (We have seen a few such events, actually -- + /// maybe because the channel only recently became unsubscribed? -- + /// but not consistently, and we're not supposed to rely on them.) + final Set _maybeStaleChannelMessages = {}; + + Message _stripMatchFields(Message message) { + message.matchContent = null; + message.matchTopic = null; + return message; } @override @@ -497,6 +555,30 @@ class MessageStoreImpl extends HasChannelStore with MessageStore, _OutboxMessage ); } + void handleChannelDeleteEvent(ChannelDeleteEvent event) { + final channelIds = event.streams.map((channel) => channel.streamId); + _handleSubscriptionsRemoved(channelIds); + } + + void handleSubscriptionRemoveEvent(SubscriptionRemoveEvent event) { + _handleSubscriptionsRemoved(event.streamIds); + } + + void _handleSubscriptionsRemoved(Iterable channelIds) { + if (channelIds.length > 8) { + assert(channelIds is! Set); + // optimization: https://github.com/zulip/zulip-flutter/pull/1912#discussion_r2479350329 + channelIds = Set.from(channelIds); + } + + // Linear in [messages]. + final affectedKnownMessageIds = messages.values + .where((message) => message is StreamMessage && channelIds.contains(message.streamId)) + .map((message) => message.id); + + _maybeStaleChannelMessages.addAll(affectedKnownMessageIds); + } + void handleUserTopicEvent(UserTopicEvent event) { for (final view in _messageListViews) { view.handleUserTopicEvent(event); @@ -510,10 +592,18 @@ class MessageStoreImpl extends HasChannelStore with MessageStore, _OutboxMessage } void handleMessageEvent(MessageEvent event) { + final message = event.message; + // 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; + // See [reconcileMessages] for reasoning. + messages[message.id] = message; + + if (message is StreamMessage && subscriptions[message.streamId] == null) { + // We didn't expect this event, because the channel is unsubscribed. But + // that doesn't mean we should expect future events about this message. + _maybeStaleChannelMessages.add(message.id); + } _handleMessageEventOutbox(event); @@ -602,6 +692,12 @@ class MessageStoreImpl extends HasChannelStore with MessageStore, _OutboxMessage // See [StreamConversation.displayRecipient] on why the invalidation is // needed. message.conversation.displayRecipient = null; + + if (subscriptions[newStreamId] == null) { + // The message was moved into an unsubscribed channel, which means + // we expect our data on it to get stale. + _maybeStaleChannelMessages.add(messageId); + } } if (newTopic != origTopic) { @@ -624,6 +720,7 @@ class MessageStoreImpl extends HasChannelStore with MessageStore, _OutboxMessage void handleDeleteMessageEvent(DeleteMessageEvent event) { for (final messageId in event.messageIds) { messages.remove(messageId); + _maybeStaleChannelMessages.remove(messageId); _editMessageRequests.remove(messageId); } for (final view in _messageListViews) { diff --git a/lib/model/store.dart b/lib/model/store.dart index e9974894c6..fda6e5b695 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -821,11 +821,17 @@ class PerAccountStore extends PerAccountStoreBase with case ChannelEvent(): assert(debugLog("server event: stream/${event.op}")); + if (event is ChannelDeleteEvent) { + _messages.handleChannelDeleteEvent(event); + } _channels.handleChannelEvent(event); notifyListeners(); case SubscriptionEvent(): assert(debugLog("server event: subscription/${event.op}")); + if (event is SubscriptionRemoveEvent) { + _messages.handleSubscriptionRemoveEvent(event); + } _channels.handleSubscriptionEvent(event); notifyListeners(); diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 4eccf75112..96d95c0c04 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -36,7 +36,7 @@ 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 Subscription? subscription; late PerAccountStore store; late FakeApiConnection connection; // [messageList] is here only for the sake of checking when it notifies. @@ -54,15 +54,20 @@ void main() { /// Initialize [store] and the rest of the test state. Future prepare({ ZulipStream? stream, + bool isChannelSubscribed = true, int? zulipFeatureLevel, }) async { stream ??= eg.stream(streamId: eg.defaultStreamMessageStreamId); - subscription = eg.subscription(stream); final selfAccount = eg.selfAccount.copyWith(zulipFeatureLevel: zulipFeatureLevel); store = eg.store(account: selfAccount, initialSnapshot: eg.initialSnapshot(zulipFeatureLevel: zulipFeatureLevel)); await store.addStream(stream); - await store.addSubscription(subscription); + if (isChannelSubscribed) { + subscription = eg.subscription(stream); + await store.addSubscription(subscription!); + } else { + subscription = null; + } connection = store.connection as FakeApiConnection; notifiedCount = 0; messageList = MessageListView.init(store: store, @@ -498,6 +503,9 @@ void main() { }); group('reconcileMessages', () { + Condition conditionIdentical(T element) => + (it) => it.identicalTo(element); + test('from empty', () async { await prepare(); check(store.messages).isEmpty(); @@ -507,8 +515,7 @@ void main() { final messages = [message1, message2, message3]; store.reconcileMessages(messages); check(messages).deepEquals( - [message1, message2, message3] - .map((m) => (Subject it) => it.identicalTo(m))); + [message1, message2, message3].map(conditionIdentical)); check(store.messages).deepEquals({ for (final m in messages) m.id: m, }); @@ -524,24 +531,144 @@ void main() { final newMessage = eg.streamMessage(); store.reconcileMessages([newMessage]); check(messages).deepEquals( - [message1, message2, message3] - .map((m) => (Subject it) => it.identicalTo(m))); + [message1, message2, message3].map(conditionIdentical)); 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('fetched message with ID already in store.messages', () { + /// Makes a copy of the single message in [MessageStore.messages] + /// by round-tripping through [Message.fromJson] and [Message.toJson]. + /// + /// If that message's [StreamMessage.conversation.displayRecipient] + /// is null, callers must provide a non-null [displayRecipient] + /// to allow [StreamConversation.fromJson] to complete without throwing. + Message copyStoredMessage({String? displayRecipient}) { + final message = store.messages.values.single; + + final json = message.toJson(); + if ( + message is StreamMessage + && message.conversation.displayRecipient == null + ) { + if (displayRecipient == null) throw ArgumentError(); + json['display_recipient'] = displayRecipient; + } + + return Message.fromJson(json); + } + + /// Checks if the single message in [MessageStore.messages] + /// is identical to [message]. + void checkStoredMessageIdenticalTo(Message message) { + check(store.messages) + .deepEquals({message.id: conditionIdentical(message)}); + } + + void checkClobber({Message? withMessageCopy}) { + final messageCopy = withMessageCopy ?? copyStoredMessage(); + store.reconcileMessages([messageCopy]); + checkStoredMessageIdenticalTo(messageCopy); + } + + void checkNoClobber() { + final messageBefore = store.messages.values.single; + store.reconcileMessages([copyStoredMessage()]); + checkStoredMessageIdenticalTo(messageBefore); + } + + test('DM', () async { + await prepare(); + final message = eg.dmMessage(id: 1, from: eg.otherUser, to: [eg.selfUser]); + + store.reconcileMessages([message]); + + // Not clobbering, because the first call didn't mark stale. + checkNoClobber(); + }); + + group('channel message; chooses correctly whether to clobber the stored version', () { + // Exercise the ways we move the message in and out of the "maybe stale" + // state. These include reconcileMessage itself, so sometimes we test + // repeated calls to that with nothing else happening in between. + + test('various conditions', () async { + final channel = eg.stream(); + await prepare(stream: channel, isChannelSubscribed: true); + final message = eg.streamMessage(stream: channel); + + store.reconcileMessages([message]); + + // Not clobbering, because the first call didn't mark stale, + // because the message was in a subscribed channel. + checkNoClobber(); + + await store.removeSubscription(channel.streamId); + // Clobbering because the unsubscribe event marked the message stale. + checkClobber(); + // (Check that reconcileMessage itself didn't unmark as stale.) + checkClobber(); + + await store.addSubscription(eg.subscription(channel)); + // The channel became subscribed, + // but the message's data hasn't been refreshed, so clobber… + checkClobber(); + + // …Now it's been refreshed, by reconcileMessages, so don't clobber. + checkNoClobber(); + + final otherChannel = eg.stream(); + await store.addStream(otherChannel); + check(store.subscriptions[otherChannel.streamId]).isNull(); + await store.handleEvent( + eg.updateMessageEventMoveFrom(origMessages: [message], + newStreamId: otherChannel.streamId)); + // Message was moved to an unsubscribed channel, so clobber. + checkClobber( + withMessageCopy: copyStoredMessage(displayRecipient: otherChannel.name)); + // (Check that reconcileMessage itself didn't unmark as stale.) + checkClobber(); + + // Subscribe, to mark message as not-stale, setting up another check… + await store.addSubscription(eg.subscription(otherChannel)); + + await store.handleEvent(ChannelDeleteEvent(id: 1, streams: [otherChannel])); + // Message was in a channel that became unknown, so clobber. + checkClobber(); + }); + + test('in unsubscribed channel on first call', () async { + await prepare(isChannelSubscribed: false); + final message = eg.streamMessage(); + + store.reconcileMessages([message]); + + checkClobber(); + checkClobber(); + }); + + test('new-message event when in unsubscribed channel', () async { + await prepare(isChannelSubscribed: false); + final message = eg.streamMessage(); + + await store.handleEvent(eg.messageEvent(message)); + + checkClobber(); + checkClobber(); + }); + + test('new-message event when in a subscribed channel', () async { + await prepare(isChannelSubscribed: true); + final message = eg.streamMessage(); + + await store.handleEvent(eg.messageEvent(message)); + + checkNoClobber(); + checkNoClobber(); + }); + }); }); test('matchContent and matchTopic are removed', () async { diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 15a0a7f81e..ef317277ae 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -160,7 +160,7 @@ void main() { final channel = eg.stream(); await prepareComposeBox(tester, narrow: ChannelNarrow(channel.streamId), - streams: [channel], + subscriptions: [eg.subscription(channel)], messages: [eg.streamMessage(stream: channel)]); check(controller).isA() ..topicFocusNode.hasFocus.isFalse() @@ -171,7 +171,7 @@ void main() { final channel = eg.stream(); await prepareComposeBox(tester, narrow: ChannelNarrow(channel.streamId), - streams: [channel], + subscriptions: [eg.subscription(channel)], messages: []); check(controller).isA() .topicFocusNode.hasFocus.isTrue(); @@ -181,7 +181,7 @@ void main() { final channel = eg.stream(); await prepareComposeBox(tester, narrow: TopicNarrow(channel.streamId, eg.t('topic')), - streams: [channel], + subscriptions: [eg.subscription(channel)], messages: [eg.streamMessage(stream: channel, topic: 'topic')]); check(controller).isNotNull().contentFocusNode.hasFocus.isFalse(); }); @@ -190,7 +190,7 @@ void main() { final channel = eg.stream(); await prepareComposeBox(tester, narrow: TopicNarrow(channel.streamId, eg.t('topic')), - streams: [channel], + subscriptions: [eg.subscription(channel)], messages: []); check(controller).isNotNull().contentFocusNode.hasFocus.isTrue(); }); @@ -379,7 +379,7 @@ void main() { addTearDown(MessageStoreImpl.debugReset); final narrow = ChannelNarrow(channel.streamId); - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, narrow: narrow, subscriptions: [eg.subscription(channel)]); await enterTopic(tester, narrow: narrow, topic: 'some topic'); await enterContent(tester, content); } @@ -418,7 +418,7 @@ void main() { addTearDown(MessageStoreImpl.debugReset); final narrow = ChannelNarrow(channel.streamId); - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, narrow: narrow, subscriptions: [eg.subscription(channel)]); await enterTopic(tester, narrow: narrow, topic: topic); await enterContent(tester, 'some content'); } @@ -462,7 +462,7 @@ void main() { await prepareComposeBox(tester, narrow: narrow, otherUsers: [eg.otherUser, eg.thirdUser], - streams: [channel], + subscriptions: [eg.subscription(channel)], mandatoryTopics: mandatoryTopics, zulipFeatureLevel: zulipFeatureLevel); } @@ -736,14 +736,14 @@ void main() { testWidgets('_StreamComposeBox', (tester) async { final channel = eg.stream(); await prepareComposeBox(tester, - narrow: ChannelNarrow(channel.streamId), streams: [channel]); + narrow: ChannelNarrow(channel.streamId), subscriptions: [eg.subscription(channel)]); checkComposeBoxTextFields(tester, expectTopicTextField: true); }); testWidgets('_FixedDestinationComposeBox', (tester) async { final channel = eg.stream(); await prepareComposeBox(tester, - narrow: eg.topicNarrow(channel.streamId, 'topic'), streams: [channel]); + narrow: eg.topicNarrow(channel.streamId, 'topic'), subscriptions: [eg.subscription(channel)]); checkComposeBoxTextFields(tester, expectTopicTextField: false); }); }); @@ -762,7 +762,8 @@ void main() { } testWidgets('smoke TopicNarrow', (tester) async { - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(channel)]); await checkStartTyping(tester, narrow); @@ -786,7 +787,8 @@ void main() { testWidgets('smoke ChannelNarrow', (tester) async { final narrow = ChannelNarrow(channel.streamId); final destinationNarrow = eg.topicNarrow(narrow.streamId, 'test topic'); - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(channel)]); await enterTopic(tester, narrow: narrow, topic: 'test topic'); await checkStartTyping(tester, destinationNarrow); @@ -797,7 +799,8 @@ void main() { }); testWidgets('clearing text sends a "typing stopped" notice', (tester) async { - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(channel)]); await checkStartTyping(tester, narrow); @@ -809,7 +812,8 @@ void main() { testWidgets('hitting send button sends a "typing stopped" notice', (tester) async { MessageStoreImpl.debugOutboxEnable = false; addTearDown(MessageStoreImpl.debugReset); - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(channel)]); await checkStartTyping(tester, narrow); @@ -855,7 +859,8 @@ void main() { testWidgets('for content input, unfocusing sends a "typing stopped" notice', (tester) async { final narrow = ChannelNarrow(channel.streamId); final destinationNarrow = eg.topicNarrow(narrow.streamId, 'test topic'); - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(channel)]); await enterTopic(tester, narrow: narrow, topic: 'test topic'); await checkStartTyping(tester, destinationNarrow); @@ -867,7 +872,8 @@ void main() { }); testWidgets('selection change sends a "typing started" notice', (tester) async { - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(channel)]); await checkStartTyping(tester, narrow); @@ -887,7 +893,8 @@ void main() { }); testWidgets('unfocusing app sends a "typing stopped" notice', (tester) async { - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(channel)]); await checkStartTyping(tester, narrow); @@ -919,8 +926,9 @@ void main() { addTearDown(MessageStoreImpl.debugReset); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - await prepareComposeBox(tester, narrow: eg.topicNarrow(123, 'some topic'), - streams: [eg.stream(streamId: 123)]); + await prepareComposeBox(tester, + narrow: eg.topicNarrow(123, 'some topic'), + subscriptions: [eg.subscription(eg.stream(streamId: 123))]); await enterContent(tester, 'hello world'); @@ -977,7 +985,7 @@ void main() { channel = eg.stream(); final narrow = ChannelNarrow(channel.streamId); await prepareComposeBox(tester, - narrow: narrow, streams: [channel], + narrow: narrow, subscriptions: [eg.subscription(channel)], mandatoryTopics: mandatoryTopics, zulipFeatureLevel: zulipFeatureLevel); @@ -1057,7 +1065,8 @@ void main() { final channel = eg.stream(); final narrow = ChannelNarrow(channel.streamId); - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(channel)]); // (When we check that the send button looks disabled, it should be because // the file is uploading, not a pre-existing reason.) @@ -1175,7 +1184,8 @@ void main() { final channel = eg.stream(); final narrow = eg.topicNarrow(channel.streamId, 'a topic'); - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(channel)]); testBinding.pickFilesResult = FilePickerResult([PlatformFile( readStream: Stream.fromIterable(['asdf'.codeUnits]), @@ -1631,7 +1641,8 @@ void main() { } testWidgets('normal text scale factor', (tester) async { - await prepareComposeBox(tester, narrow: narrow, streams: [stream]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(stream)]); await checkContentInputMaxHeight(tester, maxHeight: verticalPadding + 170, maxVisibleLines: 8); @@ -1640,7 +1651,8 @@ void main() { testWidgets('lower text scale factor', (tester) async { tester.platformDispatcher.textScaleFactorTestValue = 0.8; addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); - await prepareComposeBox(tester, narrow: narrow, streams: [stream]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(stream)]); await checkContentInputMaxHeight(tester, maxHeight: verticalPadding + 170 * 0.8, maxVisibleLines: 8); }); @@ -1648,7 +1660,8 @@ void main() { testWidgets('higher text scale factor', (tester) async { tester.platformDispatcher.textScaleFactorTestValue = 1.5; addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); - await prepareComposeBox(tester, narrow: narrow, streams: [stream]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(stream)]); await checkContentInputMaxHeight(tester, maxHeight: verticalPadding + 170 * 1.5, maxVisibleLines: 8); }); @@ -1656,7 +1669,8 @@ void main() { testWidgets('higher text scale factor exceeding threshold', (tester) async { tester.platformDispatcher.textScaleFactorTestValue = 2; addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); - await prepareComposeBox(tester, narrow: narrow, streams: [stream]); + await prepareComposeBox(tester, + narrow: narrow, subscriptions: [eg.subscription(stream)]); await checkContentInputMaxHeight(tester, maxHeight: verticalPadding + 170 * 1.5, maxVisibleLines: 6); }); @@ -1671,7 +1685,8 @@ void main() { final channel = eg.stream(); await prepareComposeBox(tester, - narrow: eg.topicNarrow(channel.streamId, 'topic'), streams: [channel]); + narrow: eg.topicNarrow(channel.streamId, 'topic'), + subscriptions: [eg.subscription(channel)]); await enterContent(tester, 'some content'); checkContentInputValue(tester, 'some content'); @@ -1769,7 +1784,9 @@ void main() { TypingNotifier.debugEnable = false; addTearDown(TypingNotifier.debugReset); await prepareComposeBox(tester, - narrow: narrow, streams: [channel], otherUsers: otherUsers); + narrow: narrow, + subscriptions: [eg.subscription(channel)], + otherUsers: otherUsers); if (narrow is ChannelNarrow) { connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); @@ -1937,7 +1954,7 @@ void main() { addTearDown(MessageStoreImpl.debugReset); await prepareComposeBox(tester, narrow: narrow, - streams: [channel]); + subscriptions: [eg.subscription(channel)]); await store.addMessages([message, dmMessage]); await tester.pump(); // message list updates } diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index acdde3fd2c..240285a222 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -154,7 +154,7 @@ void main() { testWidgets('MessageListPageState.narrow', (tester) async { final stream = eg.stream(); await setupMessageListPage(tester, narrow: ChannelNarrow(stream.streamId), - streams: [stream], + subscriptions: [eg.subscription(stream)], messages: [eg.streamMessage(stream: stream, content: "

a message

")]); final state = MessageListPage.ancestorOf(tester.element(find.text("a message"))); check(state.narrow).equals(ChannelNarrow(stream.streamId)); @@ -167,7 +167,7 @@ void main() { final topic = eg.defaultRealmEmptyTopicDisplayName; final topicNarrow = eg.topicNarrow(stream.streamId, topic); await setupMessageListPage(tester, narrow: topicNarrow, - streams: [stream], + subscriptions: [eg.subscription(stream)], messages: [eg.streamMessage(stream: stream, topic: topic, content: "

a message

")]); final state = MessageListPage.ancestorOf(tester.element(find.text("a message"))); // The page's narrow has been updated; the topic is "", not "general chat". @@ -177,7 +177,7 @@ void main() { testWidgets('composeBoxState finds compose box', (tester) async { final stream = eg.stream(); await setupMessageListPage(tester, narrow: ChannelNarrow(stream.streamId), - streams: [stream], + subscriptions: [eg.subscription(stream)], messages: [eg.streamMessage(stream: stream, content: "

a message

")]); final state = MessageListPage.ancestorOf(tester.element(find.text("a message"))); check(state.composeBoxState).isNotNull(); @@ -238,7 +238,7 @@ void main() { final channel = eg.stream(); await setupMessageListPage(tester, narrow: eg.topicNarrow(channel.streamId, ''), - streams: [channel], + subscriptions: [eg.subscription(channel)], messageCount: 1); checkAppBarChannelTopic( channel.name, eg.defaultRealmEmptyTopicDisplayName); @@ -281,7 +281,7 @@ void main() { final channel = eg.stream(); await setupMessageListPage(tester, narrow: eg.topicNarrow(channel.streamId, 'hi'), navObservers: [navObserver], - streams: [channel], messageCount: 1); + subscriptions: [eg.subscription(channel)], messageCount: 1); // Clear out initial route. assert(pushedRoutes.length == 1); @@ -298,7 +298,7 @@ void main() { final channel = eg.stream(name: 'channel foo'); await setupMessageListPage(tester, narrow: eg.topicNarrow(channel.streamId, 'topic foo'), - streams: [channel], + subscriptions: [eg.subscription(channel)], messages: [eg.streamMessage(stream: channel, topic: 'topic foo')]); connection.prepare(json: GetStreamTopicsResult(topics: [ @@ -333,7 +333,7 @@ void main() { final channel = eg.stream(name: 'channel foo'); await setupMessageListPage(tester, narrow: ChannelNarrow(channel.streamId), - streams: [channel], + subscriptions: [eg.subscription(channel)], messages: [eg.streamMessage(stream: channel, topic: 'topic foo')]); connection.prepare(json: GetStreamTopicsResult(topics: [ @@ -390,7 +390,7 @@ void main() { final channel = eg.stream(); await setupMessageListPage(tester, narrow: TopicNarrow(channel.streamId, eg.t('topic')), - streams: [channel], + subscriptions: [eg.subscription(channel)], messages: []); check(findPlaceholder).findsOne(); @@ -488,9 +488,11 @@ void main() { group('topic permalink', () { final someStream = eg.stream(); + final someSubscription = eg.subscription(someStream); const someTopic = 'some topic'; final otherStream = eg.stream(); + final otherSubscription = eg.subscription(otherStream); const otherTopic = 'other topic'; testWidgets('with message move', (tester) async { @@ -499,7 +501,7 @@ void main() { narrow: narrow, // server sends the /with/ message in its current, different location messages: [eg.streamMessage(id: 1, stream: otherStream, topic: otherTopic)], - streams: [someStream, otherStream], + subscriptions: [someSubscription, otherSubscription], skipPumpAndSettle: true); await tester.pump(); // global store loaded await tester.pump(); // per-account store loaded @@ -533,7 +535,7 @@ void main() { narrow: narrow, // server sends the /with/ message in its current, different location messages: [eg.streamMessage(id: 1, stream: someStream, topic: someTopic)], - streams: [someStream], + subscriptions: [someSubscription], skipPumpAndSettle: true); await tester.pump(); // global store loaded await tester.pump(); // per-account store loaded @@ -1182,7 +1184,9 @@ void main() { group('Update Narrow on message move', () { const topic = 'foo'; final channel = eg.stream(); + final subscription = eg.subscription(channel); final otherChannel = eg.stream(); + final otherSubscription = eg.subscription(otherChannel); final narrow = eg.topicNarrow(channel.streamId, topic); void prepareGetMessageResponse(List messages) { @@ -1200,7 +1204,10 @@ void main() { testWidgets('compose box send message after move', (tester) async { final message = eg.streamMessage(stream: channel, topic: topic, content: 'Message to move'); - await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel, otherChannel]); + await setupMessageListPage(tester, + narrow: narrow, + messages: [message], + subscriptions: [subscription, otherSubscription]); final channelContentInputFinder = find.descendant( of: find.byType(ComposeAutocomplete), @@ -1240,7 +1247,8 @@ void main() { testWidgets('Move to narrow with existing messages', (tester) async { final message = eg.streamMessage(stream: channel, topic: topic, content: 'Message to move'); - await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel]); + await setupMessageListPage(tester, + narrow: narrow, messages: [message], subscriptions: [subscription]); check(find.textContaining('Existing message').evaluate()).length.equals(0); check(find.textContaining('Message to move').evaluate()).length.equals(1); @@ -1256,7 +1264,8 @@ void main() { testWidgets('show new topic in TopicNarrow after move', (tester) async { final message = eg.streamMessage(stream: channel, topic: topic, content: 'Message to move'); - await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel]); + await setupMessageListPage(tester, + narrow: narrow, messages: [message], subscriptions: [subscription]); prepareGetMessageResponse([message]); await handleMessageMoveEvent([message], 'new topic'); @@ -1315,7 +1324,7 @@ void main() { testWidgets('do not show channel name in ChannelNarrow', (tester) async { await setupMessageListPage(tester, narrow: ChannelNarrow(stream.streamId), - messages: [message], streams: [stream]); + messages: [message], subscriptions: [eg.subscription(stream)]); await tester.pump(); check(findInMessageList('stream name')).length.equals(0); check(findInMessageList('topic name')).length.equals(1); @@ -1324,7 +1333,7 @@ void main() { testWidgets('do not show stream name in TopicNarrow', (tester) async { await setupMessageListPage(tester, narrow: TopicNarrow.ofMessage(message), - messages: [message], streams: [stream]); + messages: [message], subscriptions: [eg.subscription(stream)]); await tester.pump(); check(findInMessageList('stream name')).length.equals(0); check(findInMessageList('topic name')).length.equals(1); @@ -1505,7 +1514,7 @@ void main() { final message = eg.streamMessage(stream: channel, topic: 'topic name'); await setupMessageListPage(tester, narrow: ChannelNarrow(channel.streamId), - streams: [channel], + subscriptions: [eg.subscription(channel)], messages: [message], navObservers: [navObserver]); @@ -1531,7 +1540,7 @@ void main() { final message = eg.streamMessage(stream: channel, topic: 'topic name'); await setupMessageListPage(tester, narrow: TopicNarrow.ofMessage(message), - streams: [channel], + subscriptions: [eg.subscription(channel)], messages: [message], navObservers: [navObserver]); @@ -2136,7 +2145,7 @@ void main() { testWidgets('hidden -> waiting', (tester) async { await setupMessageListPage(tester, - narrow: topicNarrow, streams: [stream], + narrow: topicNarrow, subscriptions: [eg.subscription(stream)], messages: []); await sendMessageAndSucceed(tester); @@ -2152,7 +2161,7 @@ void main() { testWidgets('hidden -> failed, tap to restore message', (tester) async { await setupMessageListPage(tester, - narrow: topicNarrow, streams: [stream], + narrow: topicNarrow, subscriptions: [eg.subscription(stream)], messages: []); // Send a message and fail. Dismiss the error dialog as it pops up. await sendMessageAndFail(tester); @@ -2200,7 +2209,7 @@ void main() { testWidgets('waiting -> waitPeriodExpired, tap to restore message', (tester) async { await setupMessageListPage(tester, - narrow: topicNarrow, streams: [stream], + narrow: topicNarrow, subscriptions: [eg.subscription(stream)], messages: []); await sendMessageAndFail(tester, delay: kSendMessageOfferRestoreWaitPeriod + const Duration(seconds: 1));