Skip to content

Commit 3612e86

Browse files
committed
message: Consider unsubscribed channels in reconcileMessages
This fixes the "fourth buggy behavior" in #1798: #1798 (comment) Fixes-partly: #1798
1 parent e600864 commit 3612e86

File tree

2 files changed

+46
-18
lines changed

2 files changed

+46
-18
lines changed

lib/model/message.dart

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -410,21 +410,37 @@ class MessageStoreImpl extends HasChannelStore with MessageStore, _OutboxMessage
410410
//
411411
// If the fetched message reflects changes we haven't yet heard from the
412412
// event queue, then it doesn't much matter which version we use: we'll
413-
// soon get the corresponding events and apply the changes anyway.
413+
// soon get the corresponding events and apply the changes anyway. [1]
414414
// But if it lacks changes we've already heard from the event queue, then
415415
// we won't hear those events again; the only way to wind up with an
416416
// updated message is to use the version we have, that already reflects
417-
// those events' changes. So we always stick with the version we have.
417+
// those events' changes. So we always stick with the version we have. [2]
418+
//
419+
// [1] Actually, we don't expect an event if the message is in an
420+
// unsubscribed channel. See [2] and issue #1798 ("fourth buggy behavior").
421+
// [2] Since we don't expect events for messages in unsubscribed channels,
422+
// we actually stick with the *fetched* version of those messages,
423+
// since fetching is the only reliable way to get updates.
418424
for (int i = 0; i < messages.length; i++) {
419425
final message = messages[i];
426+
427+
if (message is StreamMessage && subscriptions[message.streamId] == null) {
428+
this.messages[message.id] = _stripMatchFields(message);
429+
continue;
430+
}
431+
420432
messages[i] = this.messages.putIfAbsent(message.id, () {
421-
message.matchContent = null;
422-
message.matchTopic = null;
423-
return message;
433+
return _stripMatchFields(message);
424434
});
425435
}
426436
}
427437

438+
Message _stripMatchFields(Message message) {
439+
message.matchContent = null;
440+
message.matchTopic = null;
441+
return message;
442+
}
443+
428444
@override
429445
bool? getEditMessageErrorStatus(int messageId) {
430446
assert(!_disposed);

test/model/message_test.dart

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -533,19 +533,31 @@ void main() {
533533
});
534534
});
535535

536-
test('on ID collision, new message does not clobber old in store.messages', () async {
537-
await prepare();
538-
final message = eg.streamMessage(id: 1, content: '<p>foo</p>');
539-
await addMessages([message]);
540-
check(store.messages).deepEquals({1: message});
541-
final newMessage = eg.streamMessage(id: 1, content: '<p>bar</p>');
542-
final messages = [newMessage];
543-
store.reconcileMessages(messages);
544-
check(messages).deepEquals(
545-
// (We'll check more messages in an upcoming commit.)
546-
[message].map(conditionIdentical));
547-
check(store.messages).deepEquals({1: message});
548-
});
536+
test(
537+
'on fetched message with ID already in store.messages, '
538+
'fetched does not clobber existing except in unsubscribed channel',
539+
() async {
540+
await prepare();
541+
542+
final otherChannel = eg.stream();
543+
await store.addStream(otherChannel);
544+
check(store.subscriptions[otherChannel.streamId]).isNull();
545+
546+
final message1 = eg.streamMessage(id: 1, content: '<p>foo</p>');
547+
final message2 = eg.streamMessage(id: 2, content: '<p>foo</p>', stream: otherChannel);
548+
final message3 = eg.streamMessage(id: 3, content: '<p>foo</p>');
549+
await addMessages([message1, message2, message3]);
550+
check(store.messages).deepEquals({1: message1, 2: message2, 3: message3});
551+
final newMessage1 = eg.streamMessage(id: 1, content: '<p>bar</p>');
552+
final newMessage2 = eg.streamMessage(id: 2, content: '<p>bar</p>', stream: otherChannel);
553+
final newMessage3 = eg.streamMessage(id: 3, content: '<p>bar</p>');
554+
final messages = [newMessage1, newMessage2, newMessage3];
555+
store.reconcileMessages(messages);
556+
check(messages).deepEquals(
557+
[message1, newMessage2, message3].map(conditionIdentical));
558+
check(store.messages)
559+
.deepEquals({1: message1, 2: newMessage2, 3: message3});
560+
});
549561

550562
test('matchContent and matchTopic are removed', () async {
551563
await prepare();

0 commit comments

Comments
 (0)