Skip to content

Commit 8ea4c8d

Browse files
committed
msglist: Make initial fetch from any anchor, in model
This is NFC as to the live app, because we continue to always set the anchor to AnchorCode.newest there.
1 parent 7b0e405 commit 8ea4c8d

File tree

3 files changed

+108
-27
lines changed

3 files changed

+108
-27
lines changed

lib/model/message_list.dart

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import 'content.dart';
1313
import 'narrow.dart';
1414
import 'store.dart';
1515

16+
export '../api/route/messages.dart' show Anchor, AnchorCode, NumericAnchor;
17+
1618
/// The number of messages to fetch in each request.
1719
const kMessageListFetchBatchSize = 100; // TODO tune
1820

@@ -126,9 +128,6 @@ mixin _MessageSequence {
126128

127129
/// Whether we know we have the newest messages for this narrow.
128130
///
129-
/// (Currently this is always true once [fetched] is true,
130-
/// because we start from the newest.)
131-
///
132131
/// See also [haveOldest].
133132
bool get haveNewest => _haveNewest;
134133
bool _haveNewest = false;
@@ -418,14 +417,20 @@ bool _sameDay(DateTime date1, DateTime date2) {
418417
/// * When the object will no longer be used, call [dispose] to free
419418
/// resources on the [PerAccountStore].
420419
class MessageListView with ChangeNotifier, _MessageSequence {
421-
factory MessageListView.init(
422-
{required PerAccountStore store, required Narrow narrow}) {
423-
return MessageListView._(store: store, narrow: narrow)
420+
factory MessageListView.init({
421+
required PerAccountStore store,
422+
required Narrow narrow,
423+
Anchor anchor = AnchorCode.newest, // TODO(#82): make required, for explicitness
424+
}) {
425+
return MessageListView._(store: store, narrow: narrow, anchor: anchor)
424426
.._register();
425427
}
426428

427-
MessageListView._({required this.store, required Narrow narrow})
428-
: _narrow = narrow;
429+
MessageListView._({
430+
required this.store,
431+
required Narrow narrow,
432+
required Anchor anchor,
433+
}) : _narrow = narrow, _anchor = anchor;
429434

430435
final PerAccountStore store;
431436

@@ -435,6 +440,17 @@ class MessageListView with ChangeNotifier, _MessageSequence {
435440
Narrow get narrow => _narrow;
436441
Narrow _narrow;
437442

443+
/// The anchor point this message list starts from in the message history.
444+
///
445+
/// This is passed to the server in the get-messages request
446+
/// sent by [fetchInitial].
447+
/// That includes not only the original [fetchInitial] call made by
448+
/// the message-list widget, but any additional [fetchInitial] calls
449+
/// which might be made internally by this class in order to
450+
/// fetch the messages from scratch, e.g. after certain events.
451+
Anchor get anchor => _anchor;
452+
final Anchor _anchor;
453+
438454
void _register() {
439455
store.registerMessageList(this);
440456
}
@@ -520,16 +536,14 @@ class MessageListView with ChangeNotifier, _MessageSequence {
520536

521537
/// Fetch messages, starting from scratch.
522538
Future<void> fetchInitial() async {
523-
// TODO(#80): fetch from anchor firstUnread, instead of newest
524-
// TODO(#82): fetch from a given message ID as anchor
525539
assert(!fetched && !haveOldest && !haveNewest && !busyFetchingMore);
526540
assert(messages.isEmpty && contents.isEmpty);
527541
_setStatus(FetchingStatus.fetchInitial, was: FetchingStatus.unstarted);
528542
// TODO schedule all this in another isolate
529543
final generation = this.generation;
530544
final result = await getMessages(store.connection,
531545
narrow: narrow.apiEncode(),
532-
anchor: AnchorCode.newest,
546+
anchor: anchor,
533547
numBefore: kMessageListFetchBatchSize,
534548
numAfter: kMessageListFetchBatchSize,
535549
allowEmptyTopicName: true,
@@ -541,12 +555,20 @@ class MessageListView with ChangeNotifier, _MessageSequence {
541555
store.reconcileMessages(result.messages);
542556
store.recentSenders.handleMessages(result.messages); // TODO(#824)
543557

544-
// We'll make the bottom slice start at the last visible message, if any.
558+
// The bottom slice will start at the "anchor message".
559+
// This is the first visible message at or past [anchor] if any,
560+
// else the last visible message if any. [reachedAnchor] helps track that.
561+
bool reachedAnchor = false;
545562
for (final message in result.messages) {
546563
if (!_messageVisible(message)) continue;
547-
middleMessage = messages.length;
564+
if (!reachedAnchor) {
565+
// Push the previous message into the top slice.
566+
middleMessage = messages.length;
567+
// We could interpret [anchor] for ourselves; but the server has already
568+
// done that work, reducing it to an int, `result.anchor`. So use that.
569+
reachedAnchor = message.id >= result.anchor;
570+
}
548571
_addMessage(message);
549-
// Now [middleMessage] is the last message (the one just added).
550572
}
551573
_haveOldest = result.foundOldest;
552574
_haveNewest = result.foundNewest;

lib/widgets/message_list.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,11 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
476476
}
477477

478478
void _initModel(PerAccountStore store) {
479-
_model = MessageListView.init(store: store, narrow: widget.narrow);
479+
// TODO(#82): get anchor as page/route argument, instead of using newest
480+
// TODO(#80): default to anchor firstUnread, instead of newest
481+
final anchor = AnchorCode.newest;
482+
_model = MessageListView.init(store: store,
483+
narrow: widget.narrow, anchor: anchor);
480484
model.addListener(_modelChanged);
481485
model.fetchInitial();
482486
}

test/model/message_list_test.dart

Lines changed: 67 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,18 @@ void main() {
6767
void checkNotifiedOnce() => checkNotified(count: 1);
6868

6969
/// Initialize [model] and the rest of the test state.
70-
Future<void> prepare({Narrow narrow = const CombinedFeedNarrow()}) async {
70+
Future<void> prepare({
71+
Narrow narrow = const CombinedFeedNarrow(),
72+
Anchor anchor = AnchorCode.newest,
73+
}) async {
7174
final stream = eg.stream(streamId: eg.defaultStreamMessageStreamId);
7275
subscription = eg.subscription(stream);
7376
store = eg.store();
7477
await store.addStream(stream);
7578
await store.addSubscription(subscription);
7679
connection = store.connection as FakeApiConnection;
7780
notifiedCount = 0;
78-
model = MessageListView.init(store: store, narrow: narrow)
81+
model = MessageListView.init(store: store, narrow: narrow, anchor: anchor)
7982
..addListener(() {
8083
checkInvariants(model);
8184
notifiedCount++;
@@ -88,11 +91,18 @@ void main() {
8891
///
8992
/// The test case must have already called [prepare] to initialize the state.
9093
Future<void> prepareMessages({
91-
required bool foundOldest,
94+
bool? foundOldest,
95+
bool? foundNewest,
96+
int? anchorMessageId,
9297
required List<Message> messages,
9398
}) async {
94-
connection.prepare(json:
95-
newestResult(foundOldest: foundOldest, messages: messages).toJson());
99+
final result = eg.getMessagesResult(
100+
anchor: model.anchor == AnchorCode.firstUnread
101+
? NumericAnchor(anchorMessageId!) : model.anchor,
102+
foundOldest: foundOldest,
103+
foundNewest: foundNewest,
104+
messages: messages);
105+
connection.prepare(json: result.toJson());
96106
await model.fetchInitial();
97107
checkNotifiedOnce();
98108
}
@@ -187,11 +197,7 @@ void main() {
187197
});
188198

189199
test('early in history', () async {
190-
// For now, this gets a response that isn't realistic for the
191-
// request it sends, to simulate when we start sending requests
192-
// that would make this response realistic.
193-
// TODO(#82): send appropriate fetch request
194-
await prepare();
200+
await prepare(anchor: NumericAnchor(1000));
195201
connection.prepare(json: nearResult(
196202
anchor: 1000, foundOldest: true, foundNewest: false,
197203
messages: List.generate(111, (i) => eg.streamMessage(id: 990 + i)),
@@ -219,6 +225,26 @@ void main() {
219225
..haveNewest.isTrue();
220226
});
221227

228+
group('sends proper anchor', () {
229+
Future<void> checkFetchWithAnchor(Anchor anchor) async {
230+
await prepare(anchor: anchor);
231+
// This prepared response isn't entirely realistic, depending on the anchor.
232+
// That's OK; these particular tests don't use the details of the response.
233+
connection.prepare(json:
234+
newestResult(foundOldest: true, messages: []).toJson());
235+
await model.fetchInitial();
236+
checkNotifiedOnce();
237+
check(connection.lastRequest).isA<http.Request>()
238+
.url.queryParameters['anchor']
239+
.equals(anchor.toJson());
240+
}
241+
242+
test('oldest', () => checkFetchWithAnchor(AnchorCode.oldest));
243+
test('firstUnread', () => checkFetchWithAnchor(AnchorCode.firstUnread));
244+
test('newest', () => checkFetchWithAnchor(AnchorCode.newest));
245+
test('numeric', () => checkFetchWithAnchor(NumericAnchor(12345)));
246+
});
247+
222248
// TODO(#824): move this test
223249
test('recent senders track all the messages', () async {
224250
const narrow = CombinedFeedNarrow();
@@ -1695,8 +1721,9 @@ void main() {
16951721
..middleMessage.equals(0);
16961722
});
16971723

1698-
test('on fetchInitial not empty', () async {
1699-
await prepare(narrow: const CombinedFeedNarrow());
1724+
test('on fetchInitial, anchor past end', () async {
1725+
await prepare(narrow: const CombinedFeedNarrow(),
1726+
anchor: AnchorCode.newest);
17001727
final stream1 = eg.stream();
17011728
final stream2 = eg.stream();
17021729
await store.addStreams([stream1, stream2]);
@@ -1719,6 +1746,34 @@ void main() {
17191746
.equals(messages[messages.length - 2].id);
17201747
});
17211748

1749+
test('on fetchInitial, anchor in middle', () async {
1750+
final s1 = eg.stream();
1751+
final s2 = eg.stream();
1752+
final messages = [
1753+
eg.streamMessage(id: 1, stream: s1), eg.streamMessage(id: 2, stream: s2),
1754+
eg.streamMessage(id: 3, stream: s1), eg.streamMessage(id: 4, stream: s2),
1755+
eg.streamMessage(id: 5, stream: s1), eg.streamMessage(id: 6, stream: s2),
1756+
eg.streamMessage(id: 7, stream: s1), eg.streamMessage(id: 8, stream: s2),
1757+
];
1758+
final anchorId = 4;
1759+
1760+
await prepare(narrow: const CombinedFeedNarrow(),
1761+
anchor: NumericAnchor(anchorId));
1762+
await store.addStreams([s1, s2]);
1763+
await store.addSubscription(eg.subscription(s1));
1764+
await store.addSubscription(eg.subscription(s2, isMuted: true));
1765+
await prepareMessages(foundOldest: true, foundNewest: true,
1766+
messages: messages);
1767+
// The anchor message is the first visible message with ID at least anchorId…
1768+
check(model)
1769+
..messages[model.middleMessage - 1].id.isLessThan(anchorId)
1770+
..messages[model.middleMessage].id.isGreaterOrEqual(anchorId);
1771+
// … even though a non-visible message actually had anchorId itself.
1772+
check(messages[3].id)
1773+
..equals(anchorId)
1774+
..isLessThan(model.messages[model.middleMessage].id);
1775+
});
1776+
17221777
/// Like [prepareMessages], but arrange for the given top and bottom slices.
17231778
Future<void> prepareMessageSplit(List<Message> top, List<Message> bottom, {
17241779
bool foundOldest = true,

0 commit comments

Comments
 (0)