Skip to content

Commit 55092c8

Browse files
committed
message: Avoid double-sends after send-message request succeeds
This implements the waitPeriodExpired -> waiting state transition. GitHub discussion: #1472 (comment)
1 parent 3e1a9ce commit 55092c8

File tree

2 files changed

+48
-8
lines changed

2 files changed

+48
-8
lines changed

lib/model/message.dart

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -530,12 +530,14 @@ const kSendMessageOfferRestoreWaitPeriod = Duration(seconds: 10); // TODO(#1441
530530
/// [MessageStore.sendMessage] call and before its eventual deletion.
531531
///
532532
/// ```
533-
/// Got an [ApiRequestException].
534-
/// ┌──────┬──────────┬─────────────► failed
535-
/// (create) │ │ │ │
536-
/// └► hidden waiting waitPeriodExpired ──┴──────────────► (delete)
537-
/// │ ▲ │ ▲ User restores
538-
/// └──────┘ └──────┘ the draft.
533+
/// Got an [ApiRequestException].
534+
/// ┌──────┬────────────────────────────┬──────────► failed
535+
/// │ │ │ │
536+
/// │ │ [sendMessage] │ │
537+
/// (create) │ │ request succeeds. │ │
538+
/// └► hidden waiting ◄─────────────── waitPeriodExpired ──┴─────► (delete)
539+
/// │ ▲ │ ▲ User restores
540+
/// └──────┘ └─────────────────────┘ the draft.
539541
/// Debounce [sendMessage] request
540542
/// timed out. not finished when
541543
/// wait period timed out.
@@ -559,7 +561,8 @@ enum OutboxMessageState {
559561
/// outbox message is shown to the user.
560562
///
561563
/// This state can be reached after staying in [hidden] for
562-
/// [kLocalEchoDebounceDuration].
564+
/// [kLocalEchoDebounceDuration], or when the request succeeds after the
565+
/// outbox message reaches [OutboxMessageState.waitPeriodExpired].
563566
waiting,
564567

565568
/// The [sendMessage] HTTP request did not finish in time and the user is
@@ -717,7 +720,8 @@ mixin _OutboxMessageStore on PerAccountStoreBase {
717720
final isStateTransitionValid = switch (newState) {
718721
OutboxMessageState.hidden => false,
719722
OutboxMessageState.waiting =>
720-
oldState == OutboxMessageState.hidden,
723+
oldState == OutboxMessageState.hidden
724+
|| oldState == OutboxMessageState.waitPeriodExpired,
721725
OutboxMessageState.waitPeriodExpired =>
722726
oldState == OutboxMessageState.waiting,
723727
OutboxMessageState.failed =>
@@ -803,6 +807,14 @@ mixin _OutboxMessageStore on PerAccountStoreBase {
803807
// Cancel the timer that would have had us start presuming that the
804808
// send might have failed.
805809
_outboxMessageWaitPeriodTimers.remove(localMessageId)?.cancel();
810+
if (_outboxMessages[localMessageId]!.state
811+
== OutboxMessageState.waitPeriodExpired) {
812+
// The user was offered to restore the message since the request did not
813+
// complete for a while. Since the request was successful, we expect the
814+
// message event to arrive eventually. Stop inviting the the user to
815+
// retry, to avoid double-sends.
816+
_updateOutboxMessage(localMessageId, newState: OutboxMessageState.waiting);
817+
}
806818
}
807819

808820
TopicName _processTopicLikeServer(TopicName topic, {

test/model/message_test.dart

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,34 @@ void main() {
202202
checkState().equals(OutboxMessageState.waiting);
203203
}));
204204

205+
test('waitPeriodExpired -> waiting and never return to waitPeriodExpired', () => awaitFakeAsync((async) async {
206+
await prepare(stream: stream);
207+
await prepareMessages([eg.streamMessage(stream: stream)]);
208+
// Set up a [sendMessage] request that succeeds after enough delay,
209+
// for the outbox message to reach the waitPeriodExpired state.
210+
// TODO extract helper to add prepare an outbox message with a delayed
211+
// successful [sendMessage] request if we have more tests like this
212+
connection.prepare(json: SendMessageResult(id: 1).toJson(),
213+
delay: kSendMessageOfferRestoreWaitPeriod + Duration(seconds: 1));
214+
final future = store.sendMessage(
215+
destination: streamDestination, content: 'content');
216+
async.elapse(kSendMessageOfferRestoreWaitPeriod);
217+
checkState().equals(OutboxMessageState.waitPeriodExpired);
218+
219+
// Wait till the [sendMessage] request succeeds.
220+
async.elapse(Duration(seconds: 1));
221+
await future;
222+
checkState().equals(OutboxMessageState.waiting);
223+
224+
// Wait till we reach at least [kSendMessageOfferRestoreWaitPeriod] after
225+
// returning to the waiting state.
226+
async.elapse(kSendMessageOfferRestoreWaitPeriod);
227+
async.flushTimers();
228+
// The outbox message should stay in the waiting state;
229+
// it should not transition to waitPeriodExpired.
230+
checkState().equals(OutboxMessageState.waiting);
231+
}));
232+
205233
group('… -> failed', () {
206234
test('hidden -> failed', () => awaitFakeAsync((async) async {
207235
await prepareOutboxMessageToFailAfterDelay(Duration.zero);

0 commit comments

Comments
 (0)