Skip to content

Commit ce98562

Browse files
gnpricechrisbobbe
authored andcommitted
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 44b49be commit ce98562

File tree

3 files changed

+112
-34
lines changed

3 files changed

+112
-34
lines changed

lib/model/message_list.dart

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

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

@@ -127,9 +129,6 @@ mixin _MessageSequence {
127129

128130
/// Whether we know we have the newest messages for this narrow.
129131
///
130-
/// (Currently this is always true once [fetched] is true,
131-
/// because we start from the newest.)
132-
///
133132
/// See also [haveOldest].
134133
bool get haveNewest => _haveNewest;
135134
bool _haveNewest = false;
@@ -448,14 +447,20 @@ bool _sameDay(DateTime date1, DateTime date2) {
448447
/// * When the object will no longer be used, call [dispose] to free
449448
/// resources on the [PerAccountStore].
450449
class MessageListView with ChangeNotifier, _MessageSequence {
451-
factory MessageListView.init(
452-
{required PerAccountStore store, required Narrow narrow}) {
453-
return MessageListView._(store: store, narrow: narrow)
450+
factory MessageListView.init({
451+
required PerAccountStore store,
452+
required Narrow narrow,
453+
Anchor anchor = AnchorCode.newest, // TODO(#82): make required, for explicitness
454+
}) {
455+
return MessageListView._(store: store, narrow: narrow, anchor: anchor)
454456
.._register();
455457
}
456458

457-
MessageListView._({required this.store, required Narrow narrow})
458-
: _narrow = narrow;
459+
MessageListView._({
460+
required this.store,
461+
required Narrow narrow,
462+
required Anchor anchor,
463+
}) : _narrow = narrow, _anchor = anchor;
459464

460465
final PerAccountStore store;
461466

@@ -465,6 +470,17 @@ class MessageListView with ChangeNotifier, _MessageSequence {
465470
Narrow get narrow => _narrow;
466471
Narrow _narrow;
467472

473+
/// The anchor point this message list starts from in the message history.
474+
///
475+
/// This is passed to the server in the get-messages request
476+
/// sent by [fetchInitial].
477+
/// That includes not only the original [fetchInitial] call made by
478+
/// the message-list widget, but any additional [fetchInitial] calls
479+
/// which might be made internally by this class in order to
480+
/// fetch the messages from scratch, e.g. after certain events.
481+
Anchor get anchor => _anchor;
482+
final Anchor _anchor;
483+
468484
void _register() {
469485
store.registerMessageList(this);
470486
}
@@ -550,16 +566,14 @@ class MessageListView with ChangeNotifier, _MessageSequence {
550566

551567
/// Fetch messages, starting from scratch.
552568
Future<void> fetchInitial() async {
553-
// TODO(#80): fetch from anchor firstUnread, instead of newest
554-
// TODO(#82): fetch from a given message ID as anchor
555569
assert(!fetched && !haveOldest && !haveNewest && !busyFetchingMore);
556570
assert(messages.isEmpty && contents.isEmpty);
557571
_setStatus(FetchingStatus.fetchInitial, was: FetchingStatus.unstarted);
558572
// TODO schedule all this in another isolate
559573
final generation = this.generation;
560574
final result = await getMessages(store.connection,
561575
narrow: narrow.apiEncode(),
562-
anchor: AnchorCode.newest,
576+
anchor: anchor,
563577
numBefore: kMessageListFetchBatchSize,
564578
numAfter: kMessageListFetchBatchSize,
565579
allowEmptyTopicName: true,
@@ -571,12 +585,20 @@ class MessageListView with ChangeNotifier, _MessageSequence {
571585
store.reconcileMessages(result.messages);
572586
store.recentSenders.handleMessages(result.messages); // TODO(#824)
573587

574-
// We'll make the bottom slice start at the last visible message, if any.
588+
// The bottom slice will start at the "anchor message".
589+
// This is the first visible message at or past [anchor] if any,
590+
// else the last visible message if any. [reachedAnchor] helps track that.
591+
bool reachedAnchor = false;
575592
for (final message in result.messages) {
576593
if (!_messageVisible(message)) continue;
577-
middleMessage = messages.length;
594+
if (!reachedAnchor) {
595+
// Push the previous message into the top slice.
596+
middleMessage = messages.length;
597+
// We could interpret [anchor] for ourselves; but the server has already
598+
// done that work, reducing it to an int, `result.anchor`. So use that.
599+
reachedAnchor = message.id >= result.anchor;
600+
}
578601
_addMessage(message);
579-
// Now [middleMessage] is the last message (the one just added).
580602
}
581603
_haveOldest = result.foundOldest;
582604
_haveNewest = result.foundNewest;

lib/widgets/message_list.dart

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

519519
void _initModel(PerAccountStore store) {
520-
_model = MessageListView.init(store: store, narrow: widget.narrow);
520+
// TODO(#82): get anchor as page/route argument, instead of using newest
521+
// TODO(#80): default to anchor firstUnread, instead of newest
522+
final anchor = AnchorCode.newest;
523+
_model = MessageListView.init(store: store,
524+
narrow: widget.narrow, anchor: anchor);
521525
model.addListener(_modelChanged);
522526
model.fetchInitial();
523527
}

test/model/message_list_test.dart

Lines changed: 71 additions & 19 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();
@@ -441,13 +467,10 @@ void main() {
441467

442468
test('while in mid-history', () async {
443469
final stream = eg.stream();
444-
await prepare(narrow: ChannelNarrow(stream.streamId));
445-
connection.prepare(json: nearResult(
446-
anchor: 1000, foundOldest: true, foundNewest: false,
447-
messages: List.generate(30,
448-
(i) => eg.streamMessage(id: 1000 + i, stream: stream))).toJson());
449-
await model.fetchInitial();
450-
checkNotifiedOnce();
470+
await prepare(narrow: ChannelNarrow(stream.streamId),
471+
anchor: NumericAnchor(1000));
472+
await prepareMessages(foundOldest: true, foundNewest: false, messages:
473+
List.generate(30, (i) => eg.streamMessage(id: 1000 + i, stream: stream)));
451474

452475
check(model).messages.length.equals(30);
453476
await store.addMessage(eg.streamMessage(stream: stream));
@@ -1711,8 +1734,9 @@ void main() {
17111734
..middleMessage.equals(0);
17121735
});
17131736

1714-
test('on fetchInitial not empty', () async {
1715-
await prepare(narrow: const CombinedFeedNarrow());
1737+
test('on fetchInitial, anchor past end', () async {
1738+
await prepare(narrow: const CombinedFeedNarrow(),
1739+
anchor: AnchorCode.newest);
17161740
final stream1 = eg.stream();
17171741
final stream2 = eg.stream();
17181742
await store.addStreams([stream1, stream2]);
@@ -1735,6 +1759,34 @@ void main() {
17351759
.equals(messages[messages.length - 2].id);
17361760
});
17371761

1762+
test('on fetchInitial, anchor in middle', () async {
1763+
final s1 = eg.stream();
1764+
final s2 = eg.stream();
1765+
final messages = [
1766+
eg.streamMessage(id: 1, stream: s1), eg.streamMessage(id: 2, stream: s2),
1767+
eg.streamMessage(id: 3, stream: s1), eg.streamMessage(id: 4, stream: s2),
1768+
eg.streamMessage(id: 5, stream: s1), eg.streamMessage(id: 6, stream: s2),
1769+
eg.streamMessage(id: 7, stream: s1), eg.streamMessage(id: 8, stream: s2),
1770+
];
1771+
final anchorId = 4;
1772+
1773+
await prepare(narrow: const CombinedFeedNarrow(),
1774+
anchor: NumericAnchor(anchorId));
1775+
await store.addStreams([s1, s2]);
1776+
await store.addSubscription(eg.subscription(s1));
1777+
await store.addSubscription(eg.subscription(s2, isMuted: true));
1778+
await prepareMessages(foundOldest: true, foundNewest: true,
1779+
messages: messages);
1780+
// The anchor message is the first visible message with ID at least anchorId…
1781+
check(model)
1782+
..messages[model.middleMessage - 1].id.isLessThan(anchorId)
1783+
..messages[model.middleMessage].id.isGreaterOrEqual(anchorId);
1784+
// … even though a non-visible message actually had anchorId itself.
1785+
check(messages[3].id)
1786+
..equals(anchorId)
1787+
..isLessThan(model.messages[model.middleMessage].id);
1788+
});
1789+
17381790
/// Like [prepareMessages], but arrange for the given top and bottom slices.
17391791
Future<void> prepareMessageSplit(List<Message> top, List<Message> bottom, {
17401792
bool foundOldest = true,

0 commit comments

Comments
 (0)