Skip to content

Commit b84c664

Browse files
committed
wip; message: Create an outbox message on send; manage its states
While we are creating these outbox messages, they are in no way user-visible changes since they don't end up in message list views right now. Similar to TypingNotifier.debugEnabled, we add MessageStoreImpl.debugOutboxEnabled for tests that do not intend to cover outbox messages.
1 parent 9a9ea38 commit b84c664

File tree

11 files changed

+597
-16
lines changed

11 files changed

+597
-16
lines changed

lib/api/model/model.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,13 @@ class DmRecipient extends Recipient {
559559
DmRecipient({required this.allRecipientIds})
560560
: assert(isSortedWithoutDuplicates(allRecipientIds.toList()));
561561

562+
factory DmRecipient.fromDmDestination(DmDestination destination, {
563+
required int selfUserId,
564+
}) {
565+
assert(destination.userIds.contains(selfUserId));
566+
return DmRecipient(allRecipientIds: destination.userIds);
567+
}
568+
562569
/// The user IDs of all users in the thread, sorted numerically.
563570
///
564571
/// This lists the sender as well as all (other) recipients, and it

lib/model/message.dart

Lines changed: 276 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1+
import 'dart:async';
2+
import 'dart:collection';
13
import 'dart:convert';
24

5+
import 'package:flutter/foundation.dart';
6+
37
import '../api/model/events.dart';
48
import '../api/model/model.dart';
59
import '../api/route/messages.dart';
@@ -8,12 +12,129 @@ import 'message_list.dart';
812
import 'store.dart';
913

1014
const _apiSendMessage = sendMessage; // Bit ugly; for alternatives, see: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20PerAccountStore.20methods/near/1545809
15+
const kLocalEchoDebounceDuration = Duration(milliseconds: 300);
16+
17+
/// States outlining where an [OutboxMessage] is, in its lifecycle.
18+
///
19+
/// ```
20+
//// ┌─────────────────────────────────────┐
21+
/// │ Event received, │
22+
/// Send │ or we abandoned │
23+
/// immediately. │ 200. the queue. ▼
24+
/// (create) ──────────────► sending ──────► sent ────────────────► (delete)
25+
/// │ ▲
26+
/// │ 4xx or User │
27+
/// │ other error. cancels. │
28+
/// └────────► failed ────────────────────┘
29+
/// ```
30+
enum OutboxMessageLifecycle {
31+
sending,
32+
sent,
33+
failed,
34+
}
35+
36+
/// A message sent by the self-user.
37+
sealed class OutboxMessage<T extends Recipient> implements MessageBase<T> {
38+
/// Construct a new [OutboxMessage] with a unique [localMessageId].
39+
OutboxMessage({
40+
required this.localMessageId,
41+
required int selfUserId,
42+
required this.content,
43+
}) : senderId = selfUserId,
44+
timestamp = (DateTime.timestamp().millisecondsSinceEpoch / 1000).toInt(),
45+
_state = OutboxMessageLifecycle.sending;
46+
47+
static OutboxMessage fromDestination(MessageDestination destination, {
48+
required int localMessageId,
49+
required int selfUserId,
50+
required String content,
51+
}) {
52+
return switch (destination) {
53+
StreamDestination() => StreamOutboxMessage(
54+
localMessageId: localMessageId,
55+
selfUserId: selfUserId,
56+
recipient: StreamRecipient(destination.streamId, destination.topic),
57+
content: content,
58+
),
59+
DmDestination() => DmOutboxMessage(
60+
localMessageId: localMessageId,
61+
selfUserId: selfUserId,
62+
recipient: DmRecipient.fromDmDestination(destination, selfUserId: selfUserId),
63+
content: content,
64+
),
65+
};
66+
}
67+
68+
@override
69+
int? get id => null;
70+
71+
@override
72+
final int senderId;
73+
@override
74+
final int timestamp;
75+
final String content;
76+
77+
/// ID corresponding to [MessageEvent.localMessageId], which identifies a
78+
/// locally echoed message.
79+
final int localMessageId;
80+
81+
OutboxMessageLifecycle get state => _state;
82+
OutboxMessageLifecycle _state;
83+
set state(OutboxMessageLifecycle value) {
84+
// See [OutboxMessageLifecycle] for valid state transitions.
85+
switch (value) {
86+
case OutboxMessageLifecycle.sending:
87+
assert(false);
88+
case OutboxMessageLifecycle.sent:
89+
case OutboxMessageLifecycle.failed:
90+
assert(_state == OutboxMessageLifecycle.sending);
91+
}
92+
_state = value;
93+
}
94+
95+
/// Whether the [OutboxMessage] will be hidden to [MessageListView] or not.
96+
///
97+
/// When set to false with [unhide], this cannot be toggled back to true again.
98+
bool get hidden => _hidden;
99+
bool _hidden = true;
100+
void unhide() {
101+
assert(_hidden);
102+
_hidden = false;
103+
}
104+
}
105+
106+
class StreamOutboxMessage extends OutboxMessage<StreamRecipient> {
107+
StreamOutboxMessage({
108+
required super.localMessageId,
109+
required super.selfUserId,
110+
required this.recipient,
111+
required super.content,
112+
});
113+
114+
@override
115+
final StreamRecipient recipient;
116+
}
117+
118+
class DmOutboxMessage extends OutboxMessage<DmRecipient> {
119+
DmOutboxMessage({
120+
required super.localMessageId,
121+
required super.selfUserId,
122+
required this.recipient,
123+
required super.content,
124+
});
125+
126+
@override
127+
final DmRecipient recipient;
128+
}
11129

12130
/// The portion of [PerAccountStore] for messages and message lists.
13131
mixin MessageStore {
14132
/// All known messages, indexed by [Message.id].
15133
Map<int, Message> get messages;
16134

135+
/// Messages sent by the user, indexed by [OutboxMessage.localMessageId].
136+
Map<int, OutboxMessage> get outboxMessages;
137+
17138
Set<MessageListView> get debugMessageListViews;
18139

19140
void registerMessageList(MessageListView view);
@@ -24,6 +145,11 @@ mixin MessageStore {
24145
required String content,
25146
});
26147

148+
/// Remove from [outboxMessages] given the [localMessageId].
149+
///
150+
/// The message to remove must already exist.
151+
void removeOutboxMessage(int localMessageId);
152+
27153
/// Reconcile a batch of just-fetched messages with the store,
28154
/// mutating the list.
29155
///
@@ -41,11 +167,28 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
41167
MessageStoreImpl({required super.core})
42168
// There are no messages in InitialSnapshot, so we don't have
43169
// a use case for initializing MessageStore with nonempty [messages].
44-
: messages = {};
170+
: messages = {},
171+
_outboxMessages = {};
172+
173+
/// A fresh ID to use for [OutboxMessage.localMessageId], unique within the
174+
/// current event queue.
175+
int _nextLocalMessageId = 0;
45176

46177
@override
47178
final Map<int, Message> messages;
48179

180+
@override
181+
late final UnmodifiableMapView<int, OutboxMessage> outboxMessages =
182+
UnmodifiableMapView(_outboxMessages);
183+
final Map<int, OutboxMessage> _outboxMessages;
184+
185+
/// The timers for debouncing outbox messages, indexed by
186+
/// [OutboxMessage.localMessageId].
187+
///
188+
/// It is guaranteed that all timers tracked here are active, and each
189+
/// corresponds to an outbox message where [OutboxMessage.hidden] is true.
190+
final Map<int, Timer> _outboxMessageDebounceTimers = {};
191+
49192
final Set<MessageListView> _messageListViews = {};
50193

51194
@override
@@ -84,17 +227,135 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
84227
// [InheritedNotifier] to rebuild in the next frame) before the owner's
85228
// `dispose` or `onNewStore` is called. Discussion:
86229
// https://chat.zulip.org/#narrow/channel/243-mobile-team/topic/MessageListView.20lifecycle/near/2086893
230+
231+
for (final timer in _outboxMessageDebounceTimers.values) {
232+
timer.cancel();
233+
}
234+
_outboxMessageDebounceTimers.clear();
87235
}
88236

89237
@override
90-
Future<void> sendMessage({required MessageDestination destination, required String content}) {
91-
// TODO implement outbox; see design at
92-
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M3881.20Sending.20outbox.20messages.20is.20fraught.20with.20issues/near/1405739
93-
return _apiSendMessage(connection,
94-
destination: destination,
95-
content: content,
96-
readBySender: true,
97-
);
238+
Future<void> sendMessage({required MessageDestination destination, required String content}) async {
239+
if (!debugOutboxEnabled) {
240+
await _apiSendMessage(connection,
241+
destination: destination,
242+
content: content,
243+
readBySender: true);
244+
return;
245+
}
246+
247+
final localMessageId = _nextLocalMessageId++;
248+
assert(!outboxMessages.containsKey(localMessageId));
249+
assert(!_outboxMessageDebounceTimers.containsKey(localMessageId));
250+
_outboxMessages[localMessageId] = OutboxMessage.fromDestination(destination,
251+
localMessageId: localMessageId,
252+
selfUserId: selfUserId,
253+
content: content);
254+
_outboxMessageDebounceTimers[localMessageId] =
255+
Timer(kLocalEchoDebounceDuration, () => _unhideOutboxMessage(localMessageId));
256+
257+
try {
258+
await _apiSendMessage(connection,
259+
destination: destination,
260+
content: content,
261+
readBySender: true,
262+
queueId: queueId,
263+
localId: localMessageId.toString());
264+
_updateOutboxMessage(localMessageId, newState: OutboxMessageLifecycle.sent);
265+
} catch (e) {
266+
final outboxMessage = outboxMessages[localMessageId];
267+
if (outboxMessage == null) {
268+
// The message event arrived while the message was being sent;
269+
// nothing else to do here.
270+
assert(!_outboxMessageDebounceTimers.containsKey(localMessageId));
271+
rethrow;
272+
}
273+
// This should be called before `_unhideOutboxMessage(localMessageId)`
274+
// to avoid unnecessarily notifying the listeners twice.
275+
_updateOutboxMessage(localMessageId, newState: OutboxMessageLifecycle.failed);
276+
if (outboxMessage.hidden) {
277+
_unhideOutboxMessage(localMessageId);
278+
}
279+
rethrow;
280+
}
281+
}
282+
283+
@override
284+
void removeOutboxMessage(int localMessageId) {
285+
final removed = _outboxMessages.remove(localMessageId);
286+
_outboxMessageDebounceTimers.remove(localMessageId)?.cancel();
287+
if (removed == null) {
288+
assert(false, 'Removing unknown outbox message with localMessageId: $localMessageId');
289+
return;
290+
}
291+
for (final view in _messageListViews) {
292+
view.removeOutboxMessageIfExists(removed);
293+
}
294+
}
295+
296+
/// Unhide the [OutboxMessage] with the given [localMessageId]
297+
/// and notify listeners.
298+
///
299+
/// The outbox message must exist and have been hidden.
300+
void _unhideOutboxMessage(int localMessageId) {
301+
final outboxMessage = outboxMessages[localMessageId];
302+
final debounceTimer = _outboxMessageDebounceTimers.remove(localMessageId);
303+
assert(debounceTimer != null,
304+
'Every hidden outbox message should have a debounce timer.');
305+
if (outboxMessage == null || debounceTimer == null) {
306+
assert(false);
307+
return;
308+
}
309+
outboxMessage.unhide();
310+
debounceTimer.cancel();
311+
for (final view in _messageListViews) {
312+
view.handleOutboxMessage(outboxMessage);
313+
}
314+
}
315+
316+
/// Update the state of the [OutboxMessage] with the given [localMessageId],
317+
/// and notify listeners if necessary.
318+
///
319+
/// This is a no-op if no outbox message with [localMessageId] exists.
320+
void _updateOutboxMessage(int localMessageId, {
321+
required OutboxMessageLifecycle newState,
322+
}) {
323+
final outboxMessage = outboxMessages[localMessageId];
324+
if (outboxMessage == null) {
325+
return;
326+
}
327+
outboxMessage.state = newState;
328+
if (outboxMessage.hidden) {
329+
return;
330+
}
331+
for (final view in _messageListViews) {
332+
view.notifyListenersIfOutboxMessagePresent(localMessageId);
333+
}
334+
}
335+
336+
/// In debug mode, controls whether outbox messages should be created when
337+
/// [sendMessage] is called.
338+
///
339+
/// Outside of debug mode, this is always true and the setter has no effect.
340+
static bool get debugOutboxEnabled {
341+
bool result = true;
342+
assert(() {
343+
result = _debugOutboxEnabled;
344+
return true;
345+
}());
346+
return result;
347+
}
348+
static bool _debugOutboxEnabled = true;
349+
static set debugOutboxEnabled(bool value) {
350+
assert(() {
351+
_debugOutboxEnabled = value;
352+
return true;
353+
}());
354+
}
355+
356+
@visibleForTesting
357+
static void debugReset() {
358+
_debugOutboxEnabled = true;
98359
}
99360

100361
@override
@@ -132,6 +393,12 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
132393
// See [fetchedMessages] for reasoning.
133394
messages[event.message.id] = event.message;
134395

396+
if (event.localMessageId != null) {
397+
final localMessageId = int.parse(event.localMessageId!, radix: 10);
398+
_outboxMessages.remove(localMessageId);
399+
_outboxMessageDebounceTimers.remove(localMessageId)?.cancel();
400+
}
401+
135402
for (final view in _messageListViews) {
136403
view.handleMessageEvent(event);
137404
}

lib/model/message_list.dart

Lines changed: 17 additions & 0 deletions
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

@@ -650,6 +651,17 @@ class MessageListView with ChangeNotifier, _MessageSequence {
650651
}
651652
}
652653

654+
void handleOutboxMessage(OutboxMessage outboxMessage) {
655+
// TODO: implement this
656+
}
657+
658+
/// Remove the [outboxMessage] from the view.
659+
///
660+
/// This is a no-op if the message is not found.
661+
void removeOutboxMessageIfExists(OutboxMessage outboxMessage) {
662+
// TODO: implement this
663+
}
664+
653665
void handleUserTopicEvent(UserTopicEvent event) {
654666
switch (_canAffectVisibility(event)) {
655667
case VisibilityEffect.none:
@@ -811,6 +823,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
811823
}
812824
}
813825

826+
/// Notify listeners if the given outbox message is present in this view.
827+
void notifyListenersIfOutboxMessagePresent(int localMessageId) {
828+
// TODO: implement this
829+
}
830+
814831
/// Called when the app is reassembled during debugging, e.g. for hot reload.
815832
///
816833
/// This will redo from scratch any computations we can, such as parsing

0 commit comments

Comments
 (0)