Skip to content

Commit b37892f

Browse files
PIG208PIG208
committed
wip; message: Create an outbox message on send; manage its states
TODO fix broken tests; more tests, documentation. This does not debounce new outbox messages. That will be handled in a later commit.
1 parent 354f0d4 commit b37892f

File tree

5 files changed

+134
-12
lines changed

5 files changed

+134
-12
lines changed

‎lib/model/message.dart

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'dart:convert';
22

33
import '../api/core.dart';
4+
import '../api/exception.dart';
45
import '../api/model/events.dart';
56
import '../api/model/model.dart';
67
import '../api/route/messages.dart';
@@ -145,14 +146,23 @@ mixin MessageStore {
145146
}
146147

147148
class MessageStoreImpl with MessageStore {
148-
MessageStoreImpl({required this.connection})
149+
MessageStoreImpl({
150+
required this.connection,
151+
required this.queueId,
152+
required this.selfUserId,
153+
})
149154
// There are no messages in InitialSnapshot, so we don't have
150155
// a use case for initializing MessageStore with nonempty [messages].
151156
: messages = {},
152157
outboxMessages = {};
153158

154159
final ApiConnection connection;
155160

161+
/// The event queue ID, for local-echoing, as in [InitialSnapshot.queueId].
162+
final String queueId;
163+
164+
final int selfUserId;
165+
156166
@override
157167
final Map<int, Message> messages;
158168

@@ -200,14 +210,46 @@ class MessageStoreImpl with MessageStore {
200210
}
201211

202212
@override
203-
Future<void> sendMessage({required MessageDestination destination, required String content}) {
204-
// TODO implement outbox; see design at
205-
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M3881.20Sending.20outbox.20messages.20is.20fraught.20with.20issues/near/1405739
206-
return _apiSendMessage(connection,
207-
destination: destination,
208-
content: content,
209-
readBySender: true,
210-
);
213+
Future<void> sendMessage({required MessageDestination destination, required String content}) async {
214+
final outboxMessage = OutboxMessage.fromDestination(
215+
destination, selfUserId: selfUserId, content: content);
216+
final localMessageId = outboxMessage.localMessageId;
217+
assert(!outboxMessages.containsKey(localMessageId));
218+
// TODO debounce new outbox messages
219+
outboxMessages[localMessageId] = outboxMessage;
220+
for (final view in _messageListViews) {
221+
view.handleOutboxMessage(outboxMessage);
222+
}
223+
224+
try {
225+
await _apiSendMessage(connection,
226+
destination: destination,
227+
content: content,
228+
readBySender: true,
229+
queueId: queueId,
230+
localId: localMessageId);
231+
_updateOutboxMessage(
232+
localMessageId: localMessageId, newState: OutboxMessageLifecycle.sent);
233+
} on ApiRequestException {
234+
_updateOutboxMessage(
235+
localMessageId: localMessageId, newState: OutboxMessageLifecycle.failed);
236+
rethrow;
237+
}
238+
}
239+
240+
void _updateOutboxMessage({
241+
required int localMessageId,
242+
required OutboxMessageLifecycle newState,
243+
}) {
244+
final outboxMessage = outboxMessages[localMessageId];
245+
assert(outboxMessage != null);
246+
if (outboxMessage == null) {
247+
return;
248+
}
249+
outboxMessage.state = newState;
250+
for (final view in _messageListViews) {
251+
view.handleUpdateOutboxMessage(localMessageId);
252+
}
211253
}
212254

213255
@override
@@ -245,6 +287,8 @@ class MessageStoreImpl with MessageStore {
245287
// See [fetchedMessages] for reasoning.
246288
messages[event.message.id] = event.message;
247289

290+
outboxMessages.remove(event.localMessageId);
291+
248292
for (final view in _messageListViews) {
249293
view.handleMessageEvent(event);
250294
}

‎lib/model/store.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,11 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, UserStore, Channel
374374
typingStartedExpiryPeriod: Duration(milliseconds: initialSnapshot.serverTypingStartedExpiryPeriodMilliseconds),
375375
),
376376
channels: channels,
377-
messages: MessageStoreImpl(connection: connection),
377+
messages: MessageStoreImpl(
378+
connection: connection,
379+
queueId: queueId,
380+
selfUserId: account.userId,
381+
),
378382
unreads: Unreads(
379383
initial: initialSnapshot.unreadMsgs,
380384
selfUserId: account.userId,

‎test/example_data.dart

Lines changed: 2 additions & 2 deletions
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_checks.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:zulip/model/message.dart';
3+
4+
extension OutboxMessageChecks on Subject<OutboxMessage> {
5+
Subject<OutboxMessageLifecycle> get state => has((x) => x.state, 'state');
6+
}

‎test/model/message_test.dart

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import 'dart:convert';
22

33
import 'package:checks/checks.dart';
4+
import 'package:http/http.dart' as http;
45
import 'package:test/scaffolding.dart';
56
import 'package:zulip/api/model/events.dart';
67
import 'package:zulip/api/model/model.dart';
78
import 'package:zulip/api/model/submessage.dart';
9+
import 'package:zulip/api/route/messages.dart';
10+
import 'package:zulip/model/message.dart';
811
import 'package:zulip/model/message_list.dart';
912
import 'package:zulip/model/narrow.dart';
1013
import 'package:zulip/model/store.dart';
@@ -13,7 +16,9 @@ import '../api/fake_api.dart';
1316
import '../api/model/model_checks.dart';
1417
import '../api/model/submessage_checks.dart';
1518
import '../example_data.dart' as eg;
19+
import '../fake_async.dart';
1620
import '../stdlib_checks.dart';
21+
import 'message_checks.dart';
1722
import 'message_list_test.dart';
1823
import 'store_checks.dart';
1924
import 'test_store.dart';
@@ -77,6 +82,69 @@ void main() {
7782
checkNotified(count: messageList.fetched ? messages.length : 0);
7883
}
7984

85+
group('sendMessage', () {
86+
const destination =
87+
StreamDestination(eg.defaultStreamMessageStreamId, TopicName('some topic'));
88+
89+
test('message sent successfully, message event arrives on time', () async {
90+
await prepare();
91+
92+
connection.prepare(json: SendMessageResult(id: 1).toJson(),
93+
delay: Duration.zero);
94+
final future = store.sendMessage(destination: destination, content: 'content');
95+
final outboxMessage = store.outboxMessages.values.single;
96+
check(outboxMessage).state.equals(OutboxMessageLifecycle.sending);
97+
checkNotNotified();
98+
check(connection.lastRequest).isA<http.Request>()
99+
..bodyFields['queue_id'].equals(store.queueId)
100+
..bodyFields['local_id'].equals('${outboxMessage.localMessageId}');
101+
102+
await future;
103+
check(outboxMessage).state.equals(OutboxMessageLifecycle.sent);
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).state.equals(OutboxMessageLifecycle.sent);
120+
checkNotifiedOnce();
121+
122+
async.elapse(kLocalEchoDebounceDuration);
123+
check(outboxMessage).state.equals(OutboxMessageLifecycle.sent);
124+
125+
await store.handleEvent(eg.messageEvent(
126+
eg.streamMessage(), localMessageId: outboxMessage.localMessageId));
127+
check(store.outboxMessages).isEmpty();
128+
// TODO uncomment once message list support is added
129+
// checkNotifiedOnce();
130+
}));
131+
132+
test('message failed to send', () async {
133+
await prepare();
134+
135+
connection.prepare(apiException: eg.apiBadRequest(),
136+
delay: Duration.zero);
137+
final future = store.sendMessage(destination: destination, content: 'content');
138+
final outboxMessage = store.outboxMessages.values.single;
139+
check(outboxMessage).state.equals(OutboxMessageLifecycle.sending);
140+
checkNotNotified();
141+
142+
await check(future).throws();
143+
check(outboxMessage).state.equals(OutboxMessageLifecycle.failed);
144+
checkNotifiedOnce();
145+
});
146+
});
147+
80148
group('reconcileMessages', () {
81149
test('from empty', () async {
82150
await prepare();

0 commit comments

Comments
 (0)