Skip to content

Commit 5edbc0e

Browse files
committed
msglist: Add fetchNewer method to model
This completes the model layer of #82 and #80: the message list can start at an arbitrary anchor, including a numeric message-ID anchor or AnchorCode.firstUnread, and can fetch more history from there in both directions. Still to do is to work that into the widgets layer. This change is therefore NFC as to the live app: nothing calls this method yet.
1 parent 77a74cc commit 5edbc0e

File tree

3 files changed

+206
-22
lines changed

3 files changed

+206
-22
lines changed

lib/model/message_list.dart

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,42 @@ class MessageListView with ChangeNotifier, _MessageSequence {
673673
});
674674
}
675675

676+
/// Fetch the next batch of newer messages, if applicable.
677+
///
678+
/// If there are no newer messages to fetch (i.e. if [haveNewest]),
679+
/// or if this message list is already busy fetching more messages
680+
/// (i.e. if [busyFetchingMore], which includes backoff from failed requests),
681+
/// then this method does nothing and immediately returns.
682+
/// That makes this method suitable to call frequently, e.g. every frame,
683+
/// whenever it looks likely to be useful to have more messages.
684+
Future<void> fetchNewer() async {
685+
if (haveNewest) return;
686+
if (busyFetchingMore) return;
687+
assert(fetched);
688+
assert(messages.isNotEmpty);
689+
await _fetchMore(
690+
anchor: NumericAnchor(messages.last.id),
691+
numBefore: 0,
692+
numAfter: kMessageListFetchBatchSize,
693+
processResult: (result) {
694+
if (result.messages.isNotEmpty
695+
&& result.messages.first.id == messages.last.id) {
696+
// TODO(server-6): includeAnchor should make this impossible
697+
result.messages.removeAt(0);
698+
}
699+
700+
store.reconcileMessages(result.messages);
701+
store.recentSenders.handleMessages(result.messages); // TODO(#824)
702+
703+
for (final message in result.messages) {
704+
if (_messageVisible(message)) {
705+
_addMessage(message);
706+
}
707+
}
708+
_haveNewest = result.foundNewest;
709+
});
710+
}
711+
676712
Future<void> _fetchMore({
677713
required Anchor anchor,
678714
required int numBefore,

test/example_data.dart

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,24 @@ GetMessagesResult olderGetMessagesResult({
687687
);
688688
}
689689

690+
/// A GetMessagesResult the server might return when we request newer messages.
691+
GetMessagesResult newerGetMessagesResult({
692+
required int anchor,
693+
bool foundAnchor = false, // the value if the server understood includeAnchor false
694+
required bool foundNewest,
695+
bool historyLimited = false,
696+
required List<Message> messages,
697+
}) {
698+
return GetMessagesResult(
699+
anchor: anchor,
700+
foundAnchor: foundAnchor,
701+
foundOldest: false,
702+
foundNewest: foundNewest,
703+
historyLimited: historyLimited,
704+
messages: messages,
705+
);
706+
}
707+
690708
int _nextLocalMessageId = 1;
691709

692710
StreamOutboxMessage streamOutboxMessage({

test/model/message_list_test.dart

Lines changed: 152 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import 'test_store.dart';
2828
const newestResult = eg.newestGetMessagesResult;
2929
const nearResult = eg.nearGetMessagesResult;
3030
const olderResult = eg.olderGetMessagesResult;
31+
const newerResult = eg.newerGetMessagesResult;
3132

3233
void main() {
3334
// Arrange for errors caught within the Flutter framework to be printed
@@ -291,8 +292,8 @@ void main() {
291292
});
292293
});
293294

294-
group('fetchOlder', () {
295-
test('smoke', () async {
295+
group('fetching more', () {
296+
test('fetchOlder smoke', () async {
296297
const narrow = CombinedFeedNarrow();
297298
await prepare(narrow: narrow);
298299
await prepareMessages(foundOldest: false,
@@ -321,14 +322,43 @@ void main() {
321322
);
322323
});
323324

324-
test('nop when already fetching', () async {
325-
await prepare();
326-
await prepareMessages(foundOldest: false,
325+
test('fetchNewer smoke', () async {
326+
const narrow = CombinedFeedNarrow();
327+
await prepare(narrow: narrow, anchor: NumericAnchor(1000));
328+
await prepareMessages(foundOldest: true, foundNewest: false,
327329
messages: List.generate(100, (i) => eg.streamMessage(id: 1000 + i)));
328330

331+
connection.prepare(json: newerResult(
332+
anchor: 1099, foundNewest: false,
333+
messages: List.generate(100, (i) => eg.streamMessage(id: 1100 + i)),
334+
).toJson());
335+
final fetchFuture = model.fetchNewer();
336+
checkNotifiedOnce();
337+
check(model).busyFetchingMore.isTrue();
338+
339+
await fetchFuture;
340+
checkNotifiedOnce();
341+
check(model)
342+
..busyFetchingMore.isFalse()
343+
..messages.length.equals(200);
344+
checkLastRequest(
345+
narrow: narrow.apiEncode(),
346+
anchor: '1099',
347+
includeAnchor: false,
348+
numBefore: 0,
349+
numAfter: kMessageListFetchBatchSize,
350+
allowEmptyTopicName: true,
351+
);
352+
});
353+
354+
test('nop when already fetching older', () async {
355+
await prepare(anchor: NumericAnchor(1000));
356+
await prepareMessages(foundOldest: false, foundNewest: false,
357+
messages: List.generate(201, (i) => eg.streamMessage(id: 900 + i)));
358+
329359
connection.prepare(json: olderResult(
330-
anchor: 1000, foundOldest: false,
331-
messages: List.generate(100, (i) => eg.streamMessage(id: 900 + i)),
360+
anchor: 900, foundOldest: false,
361+
messages: List.generate(100, (i) => eg.streamMessage(id: 800 + i)),
332362
).toJson());
333363
final fetchFuture = model.fetchOlder();
334364
checkNotifiedOnce();
@@ -338,39 +368,90 @@ void main() {
338368
final fetchFuture2 = model.fetchOlder();
339369
checkNotNotified();
340370
check(model).busyFetchingMore.isTrue();
371+
final fetchFuture3 = model.fetchNewer();
372+
checkNotNotified();
373+
check(model)..busyFetchingMore.isTrue()..messages.length.equals(201);
341374

342375
await fetchFuture;
343376
await fetchFuture2;
377+
await fetchFuture3;
344378
// We must not have made another request, because we didn't
345379
// prepare another response and didn't get an exception.
346380
checkNotifiedOnce();
347-
check(model)
348-
..busyFetchingMore.isFalse()
349-
..messages.length.equals(200);
381+
check(model)..busyFetchingMore.isFalse()..messages.length.equals(301);
350382
});
351383

352-
test('nop when already haveOldest true', () async {
353-
await prepare();
354-
await prepareMessages(foundOldest: true, messages:
355-
List.generate(30, (i) => eg.streamMessage()));
384+
test('nop when already fetching newer', () async {
385+
await prepare(anchor: NumericAnchor(1000));
386+
await prepareMessages(foundOldest: false, foundNewest: false,
387+
messages: List.generate(201, (i) => eg.streamMessage(id: 900 + i)));
388+
389+
connection.prepare(json: newerResult(
390+
anchor: 1100, foundNewest: false,
391+
messages: List.generate(100, (i) => eg.streamMessage(id: 1101 + i)),
392+
).toJson());
393+
final fetchFuture = model.fetchNewer();
394+
checkNotifiedOnce();
395+
check(model).busyFetchingMore.isTrue();
396+
397+
// Don't prepare another response.
398+
final fetchFuture2 = model.fetchOlder();
399+
checkNotNotified();
400+
check(model).busyFetchingMore.isTrue();
401+
final fetchFuture3 = model.fetchNewer();
402+
checkNotNotified();
403+
check(model)..busyFetchingMore.isTrue()..messages.length.equals(201);
404+
405+
await fetchFuture;
406+
await fetchFuture2;
407+
await fetchFuture3;
408+
// We must not have made another request, because we didn't
409+
// prepare another response and didn't get an exception.
410+
checkNotifiedOnce();
411+
check(model)..busyFetchingMore.isFalse()..messages.length.equals(301);
412+
});
413+
414+
test('fetchOlder nop when already haveOldest true', () async {
415+
await prepare(anchor: NumericAnchor(1000));
416+
await prepareMessages(foundOldest: true, foundNewest: false, messages:
417+
List.generate(151, (i) => eg.streamMessage(id: 950 + i)));
356418
check(model)
357419
..haveOldest.isTrue()
358-
..messages.length.equals(30);
420+
..messages.length.equals(151);
359421

360422
await model.fetchOlder();
361423
// We must not have made a request, because we didn't
362424
// prepare a response and didn't get an exception.
363425
checkNotNotified();
364426
check(model)
365427
..haveOldest.isTrue()
366-
..messages.length.equals(30);
428+
..messages.length.equals(151);
429+
});
430+
431+
test('fetchNewer nop when already haveNewest true', () async {
432+
await prepare(anchor: NumericAnchor(1000));
433+
await prepareMessages(foundOldest: false, foundNewest: true, messages:
434+
List.generate(151, (i) => eg.streamMessage(id: 950 + i)));
435+
check(model)
436+
..haveNewest.isTrue()
437+
..messages.length.equals(151);
438+
439+
await model.fetchNewer();
440+
// We must not have made a request, because we didn't
441+
// prepare a response and didn't get an exception.
442+
checkNotNotified();
443+
check(model)
444+
..haveNewest.isTrue()
445+
..messages.length.equals(151);
367446
});
368447

369448
test('nop during backoff', () => awaitFakeAsync((async) async {
370449
final olderMessages = List.generate(5, (i) => eg.streamMessage());
371450
final initialMessages = List.generate(5, (i) => eg.streamMessage());
372-
await prepare();
373-
await prepareMessages(foundOldest: false, messages: initialMessages);
451+
final newerMessages = List.generate(5, (i) => eg.streamMessage());
452+
await prepare(anchor: NumericAnchor(initialMessages[2].id));
453+
await prepareMessages(foundOldest: false, foundNewest: false,
454+
messages: initialMessages);
374455
check(connection.takeRequests()).single;
375456

376457
connection.prepare(apiException: eg.apiBadRequest());
@@ -385,20 +466,31 @@ void main() {
385466
check(model).busyFetchingMore.isTrue();
386467
check(connection.lastRequest).isNull();
387468

469+
await model.fetchNewer();
470+
checkNotNotified();
471+
check(model).busyFetchingMore.isTrue();
472+
check(connection.lastRequest).isNull();
473+
388474
// Wait long enough that a first backoff is sure to finish.
389475
async.elapse(const Duration(seconds: 1));
390476
check(model).busyFetchingMore.isFalse();
391477
checkNotifiedOnce();
392478
check(connection.lastRequest).isNull();
393479

394-
connection.prepare(json: olderResult(
395-
anchor: 1000, foundOldest: false, messages: olderMessages).toJson());
480+
connection.prepare(json: olderResult(anchor: initialMessages.first.id,
481+
foundOldest: false, messages: olderMessages).toJson());
396482
await model.fetchOlder();
397483
checkNotified(count: 2);
398484
check(connection.takeRequests()).single;
485+
486+
connection.prepare(json: newerResult(anchor: initialMessages.last.id,
487+
foundNewest: false, messages: newerMessages).toJson());
488+
await model.fetchNewer();
489+
checkNotified(count: 2);
490+
check(connection.takeRequests()).single;
399491
}));
400492

401-
test('handles servers not understanding includeAnchor', () async {
493+
test('fetchOlder handles servers not understanding includeAnchor', () async {
402494
await prepare();
403495
await prepareMessages(foundOldest: false,
404496
messages: List.generate(100, (i) => eg.streamMessage(id: 1000 + i)));
@@ -415,8 +507,25 @@ void main() {
415507
..messages.length.equals(200);
416508
});
417509

510+
test('fetchNewer handles servers not understanding includeAnchor', () async {
511+
await prepare(anchor: NumericAnchor(1000));
512+
await prepareMessages(foundOldest: true, foundNewest: false,
513+
messages: List.generate(101, (i) => eg.streamMessage(id: 1000 + i)));
514+
515+
// The old behavior is to include the anchor message regardless of includeAnchor.
516+
connection.prepare(json: newerResult(
517+
anchor: 1100, foundNewest: false, foundAnchor: true,
518+
messages: List.generate(101, (i) => eg.streamMessage(id: 1100 + i)),
519+
).toJson());
520+
await model.fetchNewer();
521+
checkNotified(count: 2);
522+
check(model)
523+
..busyFetchingMore.isFalse()
524+
..messages.length.equals(201);
525+
});
526+
418527
// TODO(#824): move this test
419-
test('recent senders track all the messages', () async {
528+
test('fetchOlder recent senders track all the messages', () async {
420529
await prepare();
421530
final initialMessages = List.generate(10, (i) => eg.streamMessage(id: 100 + i));
422531
await prepareMessages(foundOldest: false, messages: initialMessages);
@@ -434,6 +543,27 @@ void main() {
434543
recent_senders_test.checkMatchesMessages(store.recentSenders,
435544
[...initialMessages, ...oldMessages]);
436545
});
546+
547+
// TODO(#824): move this test
548+
test('TODO fetchNewer recent senders track all the messages', () async {
549+
await prepare(anchor: NumericAnchor(100));
550+
final initialMessages = List.generate(10, (i) => eg.streamMessage(id: 100 + i));
551+
await prepareMessages(foundOldest: true, foundNewest: false,
552+
messages: initialMessages);
553+
554+
final newMessages = List.generate(10, (i) => eg.streamMessage(id: 110 + i))
555+
// Not subscribed to the stream with id 10.
556+
..add(eg.streamMessage(id: 120, stream: eg.stream(streamId: 10)));
557+
connection.prepare(json: newerResult(
558+
anchor: 100, foundNewest: false,
559+
messages: newMessages,
560+
).toJson());
561+
await model.fetchNewer();
562+
563+
check(model).messages.length.equals(20);
564+
recent_senders_test.checkMatchesMessages(store.recentSenders,
565+
[...initialMessages, ...newMessages]);
566+
});
437567
});
438568

439569
group('MessageEvent', () {

0 commit comments

Comments
 (0)