Skip to content

Commit dd0fe75

Browse files
PIG208PIG208
committed
draft; Support outbox managed by MessageStore, w/ flag
1 parent ca723fa commit dd0fe75

File tree

4 files changed

+167
-10
lines changed

4 files changed

+167
-10
lines changed

‎lib/model/message.dart

+74-8
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ class OutboxMessage {
4646
final MessageDestination destination;
4747
final String content;
4848

49+
/// Whether the OutboxMessage will be hidden to [MessageListView] or not.
50+
///
51+
/// When set to false with [unhide], this cannot be toggle back to true again.
52+
bool get hidden => _hidden;
53+
bool _hidden = true;
54+
void unhide() {
55+
assert(_hidden);
56+
_hidden = false;
57+
}
58+
4959
OutboxMessageLifecycle get state => _state;
5060
OutboxMessageLifecycle _state;
5161
set state(OutboxMessageLifecycle value) {
@@ -148,14 +158,57 @@ class MessageStoreImpl with MessageStore {
148158
}
149159

150160
@override
151-
Future<void> sendMessage({required MessageDestination destination, required String content}) {
152-
// TODO implement outbox; see design at
153-
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M3881.20Sending.20outbox.20messages.20is.20fraught.20with.20issues/near/1405739
154-
return _apiSendMessage(connection,
155-
destination: destination,
156-
content: content,
157-
readBySender: true,
158-
);
161+
Future<void> sendMessage({required MessageDestination destination, required String content}) async {
162+
final outboxMessage = OutboxMessage._(destination: destination, content: content);
163+
assert(!outboxMessages.containsKey(outboxMessage.localMessageId));
164+
outboxMessages[outboxMessage.localMessageId] = outboxMessage;
165+
// The need:
166+
// hide some outbox messages;
167+
// comply with the state diagram;
168+
// debounce until a timeout.
169+
170+
// The outbox message only become visible to views after
171+
// [kLocalEchoDebounceDuration].
172+
Future<void>.delayed(kLocalEchoDebounceDuration, () {
173+
if (!outboxMessages.containsKey(outboxMessage.localMessageId)) {
174+
// The outbox message was deleted, one such reason can be that the
175+
// corresponding "message" event arrived quickly.
176+
return;
177+
}
178+
if (!outboxMessage.hidden) {
179+
return;
180+
}
181+
outboxMessage.unhide();
182+
for (final view in _messageListViews) {
183+
view.handleOutboxMessage(outboxMessage);
184+
}
185+
});
186+
187+
try {
188+
await _apiSendMessage(connection,
189+
destination: destination,
190+
content: content,
191+
readBySender: true,
192+
queueId: 'TODO queue_id',
193+
localId: outboxMessage.localMessageId);
194+
} catch (e) {
195+
outboxMessage.unhide();
196+
_updateOutboxMessage(outboxMessage,
197+
newState: OutboxMessageLifecycle.failed);
198+
// TODO handle 4xx errors
199+
rethrow;
200+
}
201+
_updateOutboxMessage(outboxMessage,
202+
newState: OutboxMessageLifecycle.sent);
203+
}
204+
205+
void _updateOutboxMessage(OutboxMessage outboxMessage, {
206+
required OutboxMessageLifecycle newState,
207+
}) {
208+
outboxMessage.state = newState;
209+
for (final view in _messageListViews) {
210+
view.notifyListenersIfOutboxMessagePresent(outboxMessage.localMessageId);
211+
}
159212
}
160213

161214
@override
@@ -193,11 +246,24 @@ class MessageStoreImpl with MessageStore {
193246
// See [fetchedMessages] for reasoning.
194247
messages[event.message.id] = event.message;
195248

249+
final localMessageId = event.localMessageId;
250+
if (localMessageId != null) {
251+
_removeOutboxMessage(localMessageId: localMessageId);
252+
}
253+
196254
for (final view in _messageListViews) {
197255
view.handleMessageEvent(event);
198256
}
199257
}
200258

259+
void _removeOutboxMessage({required int localMessageId}) {
260+
final removed = outboxMessages.remove(localMessageId);
261+
if (removed == null) return;
262+
for (final view in _messageListViews) {
263+
view.handleRemoveOutboxMessage(localMessageId);
264+
}
265+
}
266+
201267
void handleUpdateMessageEvent(UpdateMessageEvent event) {
202268
assert(event.messageIds.contains(event.messageId), "See https://github.com/zulip/zulip-flutter/pull/753#discussion_r1649463633");
203269
_handleUpdateMessageEventTimestamp(event);

‎lib/model/message_list.dart

+15
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import '../api/route/messages.dart';
1010
import 'algorithms.dart';
1111
import 'channel.dart';
1212
import 'content.dart';
13+
import 'message.dart';
1314
import 'narrow.dart';
1415
import 'store.dart';
1516

@@ -625,6 +626,14 @@ class MessageListView with ChangeNotifier, _MessageSequence {
625626
}
626627
}
627628

629+
void handleOutboxMessage(OutboxMessage outboxMessage) {
630+
// TODO handle
631+
}
632+
633+
void handleRemoveOutboxMessage(int localMessageId) {
634+
// TODO handle
635+
}
636+
628637
void handleUserTopicEvent(UserTopicEvent event) {
629638
switch (_canAffectVisibility(event)) {
630639
case VisibilityEffect.none:
@@ -786,6 +795,12 @@ class MessageListView with ChangeNotifier, _MessageSequence {
786795
}
787796
}
788797

798+
/// Notify listeners if the given outbox message is present in this view.
799+
void notifyListenersIfOutboxMessagePresent(int localMessageId) {
800+
// TODO handle
801+
notifyListeners();
802+
}
803+
789804
/// Called when the app is reassembled during debugging, e.g. for hot reload.
790805
///
791806
/// This will redo from scratch any computations we can, such as parsing

‎test/example_data.dart

+2-2
Original file line numberDiff line numberDiff line change
@@ -620,8 +620,8 @@ UserTopicEvent userTopicEvent(
620620
);
621621
}
622622

623-
MessageEvent messageEvent(Message message) =>
624-
MessageEvent(id: 0, message: message, localMessageId: null);
623+
MessageEvent messageEvent(Message message, {int? localMessageId}) =>
624+
MessageEvent(id: 0, message: message, localMessageId: localMessageId);
625625

626626
DeleteMessageEvent deleteMessageEvent(List<StreamMessage> messages) {
627627
assert(messages.isNotEmpty);

‎test/model/message_test.dart

+76
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import 'package:test/scaffolding.dart';
55
import 'package:zulip/api/model/events.dart';
66
import 'package:zulip/api/model/model.dart';
77
import 'package:zulip/api/model/submessage.dart';
8+
import 'package:zulip/api/route/messages.dart';
9+
import 'package:zulip/model/message.dart';
810
import 'package:zulip/model/message_list.dart';
911
import 'package:zulip/model/narrow.dart';
1012
import 'package:zulip/model/store.dart';
@@ -13,7 +15,9 @@ import '../api/fake_api.dart';
1315
import '../api/model/model_checks.dart';
1416
import '../api/model/submessage_checks.dart';
1517
import '../example_data.dart' as eg;
18+
import '../fake_async.dart';
1619
import '../stdlib_checks.dart';
20+
import 'message_checks.dart';
1721
import 'message_list_test.dart';
1822
import 'store_checks.dart';
1923
import 'test_store.dart';
@@ -77,6 +81,78 @@ void main() {
7781
checkNotified(count: messageList.fetched ? messages.length : 0);
7882
}
7983

84+
group('sendMessage', () {
85+
const destination =
86+
StreamDestination(eg.defaultStreamMessageStreamId, TopicName('some topic'));
87+
88+
test('message sent successfully, message event arrives on time', () async {
89+
await prepare();
90+
91+
connection.prepare(json: SendMessageResult(id: 1).toJson(),
92+
delay: Duration.zero);
93+
final future = store.sendMessage(destination: destination, content: 'content');
94+
final outboxMessage = store.outboxMessages.values.single;
95+
check(outboxMessage)
96+
..state.equals(OutboxMessageLifecycle.sending)
97+
..hidden.isTrue();
98+
checkNotNotified();
99+
100+
await future;
101+
check(outboxMessage)
102+
..state.equals(OutboxMessageLifecycle.sent)
103+
..hidden.isTrue();
104+
checkNotifiedOnce();
105+
106+
await store.handleEvent(eg.messageEvent(
107+
eg.streamMessage(), localMessageId: outboxMessage.localMessageId));
108+
check(store.outboxMessages).isEmpty();
109+
// TODO uncomment once message list support is added
110+
// checkNotifiedOnce();
111+
});
112+
113+
test('message sent successfully, message event arrives after debounce timeout', () => awaitFakeAsync((async) async {
114+
await prepare();
115+
116+
connection.prepare(json: SendMessageResult(id: 1).toJson());
117+
await store.sendMessage(destination: destination, content: 'content');
118+
final outboxMessage = store.outboxMessages.values.single;
119+
check(outboxMessage)
120+
..state.equals(OutboxMessageLifecycle.sent)
121+
..hidden.isTrue();
122+
checkNotifiedOnce();
123+
124+
async.elapse(kLocalEchoDebounceDuration);
125+
check(outboxMessage)
126+
..state.equals(OutboxMessageLifecycle.sent)
127+
..hidden.isFalse();
128+
129+
await store.handleEvent(eg.messageEvent(
130+
eg.streamMessage(), localMessageId: outboxMessage.localMessageId));
131+
check(store.outboxMessages).isEmpty();
132+
// TODO uncomment once message list support is added
133+
// checkNotifiedOnce();
134+
}));
135+
136+
test('message failed to send', () async {
137+
await prepare();
138+
139+
connection.prepare(apiException: eg.apiBadRequest(),
140+
delay: Duration.zero);
141+
final future = store.sendMessage(destination: destination, content: 'content');
142+
final outboxMessage = store.outboxMessages.values.single;
143+
check(outboxMessage)
144+
..state.equals(OutboxMessageLifecycle.sending)
145+
..hidden.isTrue();
146+
checkNotNotified();
147+
148+
await check(future).throws();
149+
check(outboxMessage)
150+
..state.equals(OutboxMessageLifecycle.failed)
151+
..hidden.isFalse();
152+
checkNotifiedOnce();
153+
});
154+
});
155+
80156
group('reconcileMessages', () {
81157
test('from empty', () async {
82158
await prepare();

0 commit comments

Comments
 (0)