Skip to content

Commit 3550d87

Browse files
chrisbobbegnprice
chrisbobbe
authored and
gnprice
committed
store: Add RecentDmConversationsView view-model
Toward #119, the list of recent DM conversations. Much of this code was transcribed from zulip-mobile; in particular, from: src/pm-conversations/pmConversationsModel.js src/pm-conversations/__tests__/pmConversationsModel-test.js
1 parent 606975b commit 3550d87

5 files changed

+294
-1
lines changed

Diff for: ‎lib/model/narrow.dart

+9
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11

2+
import '../api/model/initial_snapshot.dart';
23
import '../api/model/model.dart';
34
import '../api/model/narrow.dart';
45
import '../api/route/messages.dart';
@@ -152,6 +153,14 @@ class DmNarrow extends Narrow implements SendableNarrow {
152153
);
153154
}
154155

156+
/// A [DmNarrow] from an item in [InitialSnapshot.recentPrivateConversations].
157+
factory DmNarrow.ofRecentDmConversation(RecentDmConversation conversation, {required int selfUserId}) {
158+
return DmNarrow(
159+
allRecipientIds: [...conversation.userIds, selfUserId]..sort(),
160+
selfUserId: selfUserId,
161+
);
162+
}
163+
155164
/// The user IDs of everyone in the conversation, sorted.
156165
///
157166
/// Each message in the conversation is sent by one of these users

Diff for: ‎lib/model/recent_dm_conversations.dart

+126
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import 'package:collection/collection.dart';
2+
import 'package:flutter/foundation.dart';
3+
4+
import '../api/model/initial_snapshot.dart';
5+
import '../api/model/model.dart';
6+
import '../api/model/events.dart';
7+
import 'narrow.dart';
8+
9+
/// A view-model for the recent-DM-conversations UI.
10+
///
11+
/// This maintains the list of recent DM conversations,
12+
/// plus additional data in order to efficiently maintain the list.
13+
class RecentDmConversationsView extends ChangeNotifier {
14+
factory RecentDmConversationsView({
15+
required List<RecentDmConversation> initial,
16+
required selfUserId,
17+
}) {
18+
final entries = initial.map((conversation) => MapEntry(
19+
DmNarrow.ofRecentDmConversation(conversation, selfUserId: selfUserId),
20+
conversation.maxMessageId,
21+
)).toList()..sort((a, b) => -a.value.compareTo(b.value));
22+
return RecentDmConversationsView._(
23+
map: Map.fromEntries(entries),
24+
sorted: QueueList.from(entries.map((e) => e.key)),
25+
selfUserId: selfUserId,
26+
);
27+
}
28+
29+
RecentDmConversationsView._({
30+
required this.map,
31+
required this.sorted,
32+
required this.selfUserId,
33+
});
34+
35+
/// The latest message ID in each conversation.
36+
final Map<DmNarrow, int> map;
37+
38+
/// The [DmNarrow] keys of the map, sorted by latest message descending.
39+
final QueueList<DmNarrow> sorted;
40+
41+
final int selfUserId;
42+
43+
/// Insert the key at the proper place in [sorted].
44+
///
45+
/// Optimized, taking O(1) time, for the case where that place is the start,
46+
/// because that's the common case for a new message.
47+
/// May take O(n) time in general.
48+
void _insertSorted(DmNarrow key, int msgId) {
49+
final i = sorted.indexWhere((k) => map[k]! < msgId);
50+
// QueueList is a deque, with O(1) to add at start or end.
51+
switch (i) {
52+
case == 0:
53+
sorted.addFirst(key);
54+
case < 0:
55+
sorted.addLast(key);
56+
default:
57+
sorted.insert(i, key);
58+
}
59+
}
60+
61+
/// Handle [MessageEvent], updating [map] and [sorted].
62+
///
63+
/// Can take linear time in general. That sounds inefficient...
64+
/// but it's what the webapp does, so must not be catastrophic. 🤷
65+
/// (In fact the webapp calls `Array#sort`,
66+
/// which takes at *least* linear time, and may be 𝛳(N log N).)
67+
///
68+
/// The point of the event is that we're learning about the message
69+
/// in real time immediately after it was sent --
70+
/// so the overwhelmingly common case is that the message
71+
/// is newer than any existing message we know about. (*)
72+
/// That's therefore the case we optimize for,
73+
/// particularly in the helper _insertSorted.
74+
///
75+
/// (*) In fact at present that's the only possible case.
76+
/// The alternative will become possible when we have the analogue of
77+
/// zulip-mobile's FETCH_MESSAGES_COMPLETE, with fetches done for a
78+
/// [MessageListView] reporting their messages back via the [PerAccountStore]
79+
/// to all our central data structures.
80+
/// Then that can race with a new-message event: for example,
81+
/// say we get a fetch-messages result that includes the just-sent
82+
/// message 1002, and only after that get the event about message 1001,
83+
/// sent moments earlier. The event queue always delivers events in order, so
84+
/// even the race is possible only because we fetch messages outside of the
85+
/// event queue.
86+
void handleMessageEvent(MessageEvent event) {
87+
final message = event.message;
88+
if (message is! DmMessage) {
89+
return;
90+
}
91+
final key = DmNarrow.ofMessage(message, selfUserId: selfUserId);
92+
final prev = map[key];
93+
if (prev == null) {
94+
// The conversation is new. Add to both `map` and `sorted`.
95+
map[key] = message.id;
96+
_insertSorted(key, message.id);
97+
} else if (prev >= message.id) {
98+
// The conversation already has a newer message.
99+
// This should be impossible as long as we only listen for messages coming
100+
// through the event system, which sends events in order.
101+
// Anyway, do nothing.
102+
} else {
103+
// The conversation needs to be (a) updated in `map`...
104+
map[key] = message.id;
105+
106+
// ... and (b) possibly moved around in `sorted` to keep the list sorted.
107+
final i = sorted.indexOf(key);
108+
assert(i >= 0, 'key in map should be in sorted');
109+
if (i == 0) {
110+
// The conversation was already the latest, so no reordering needed.
111+
// (This is likely a common case in practice -- happens every time
112+
// the user gets several DMs in a row in the same thread -- so good to
113+
// optimize.)
114+
} else {
115+
// It wasn't the latest. Just handle the general case.
116+
sorted.removeAt(i); // linear time, ouch
117+
_insertSorted(key, message.id);
118+
}
119+
}
120+
notifyListeners();
121+
}
122+
123+
// TODO update from messages loaded in message lists. When doing so,
124+
// review handleMessageEvent so it acknowledges the subtle races that can
125+
// happen when taking data from outside the event system.
126+
}

Diff for: ‎lib/model/store.dart

+8-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import '../log.dart';
1616
import 'autocomplete.dart';
1717
import 'database.dart';
1818
import 'message_list.dart';
19+
import 'recent_dm_conversations.dart';
1920

2021
export 'package:drift/drift.dart' show Value;
2122
export 'database.dart' show Account, AccountsCompanion;
@@ -158,7 +159,9 @@ class PerAccountStore extends ChangeNotifier {
158159
streams = Map.fromEntries(initialSnapshot.streams.map(
159160
(stream) => MapEntry(stream.streamId, stream))),
160161
subscriptions = Map.fromEntries(initialSnapshot.subscriptions.map(
161-
(subscription) => MapEntry(subscription.streamId, subscription)));
162+
(subscription) => MapEntry(subscription.streamId, subscription))),
163+
recentDmConversationsView = RecentDmConversationsView(
164+
initial: initialSnapshot.recentPrivateConversations, selfUserId: account.userId);
162165

163166
final Account account;
164167
final ApiConnection connection; // TODO(#135): update zulipFeatureLevel with events
@@ -178,6 +181,9 @@ class PerAccountStore extends ChangeNotifier {
178181

179182
// TODO lots more data. When adding, be sure to update handleEvent too.
180183

184+
// TODO call [RecentDmConversationsView.dispose] in [dispose]
185+
final RecentDmConversationsView recentDmConversationsView;
186+
181187
final Set<MessageListView> _messageListViews = {};
182188

183189
void registerMessageList(MessageListView view) {
@@ -260,6 +266,7 @@ class PerAccountStore extends ChangeNotifier {
260266
notifyListeners();
261267
} else if (event is MessageEvent) {
262268
assert(debugLog("server event: message ${jsonEncode(event.message.toJson())}"));
269+
recentDmConversationsView.handleMessageEvent(event);
263270
for (final view in _messageListViews) {
264271
view.maybeAddMessage(event.message);
265272
}

Diff for: ‎test/model/recent_dm_conversations_checks.dart

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:collection/collection.dart';
3+
import 'package:zulip/model/narrow.dart';
4+
import 'package:zulip/model/recent_dm_conversations.dart';
5+
6+
extension RecentDmConversationsViewChecks on Subject<RecentDmConversationsView> {
7+
Subject<Map<DmNarrow, int>> get map => has((v) => v.map, 'map');
8+
Subject<QueueList<DmNarrow>> get sorted => has((v) => v.sorted, 'sorted');
9+
}

Diff for: ‎test/model/recent_dm_conversations_test.dart

+142
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:test/scaffolding.dart';
3+
import 'package:zulip/api/model/events.dart';
4+
import 'package:zulip/api/model/initial_snapshot.dart';
5+
import 'package:zulip/model/narrow.dart';
6+
import 'package:zulip/model/recent_dm_conversations.dart';
7+
8+
import '../example_data.dart' as eg;
9+
import 'recent_dm_conversations_checks.dart';
10+
11+
void main() {
12+
group('RecentDmConversationsView', () {
13+
/// Get a [DmNarrow] from a list of recipient IDs excluding self.
14+
DmNarrow key(userIds) {
15+
return DmNarrow(
16+
allRecipientIds: [eg.selfUser.userId, ...userIds]..sort(),
17+
selfUserId: eg.selfUser.userId,
18+
);
19+
}
20+
21+
test('construct from initial data', () {
22+
check(RecentDmConversationsView(selfUserId: eg.selfUser.userId,
23+
initial: []))
24+
..map.isEmpty()
25+
..sorted.isEmpty();
26+
27+
check(RecentDmConversationsView(selfUserId: eg.selfUser.userId,
28+
initial: [
29+
RecentDmConversation(userIds: [], maxMessageId: 200),
30+
RecentDmConversation(userIds: [1], maxMessageId: 100),
31+
RecentDmConversation(userIds: [2, 1], maxMessageId: 300), // userIds out of order
32+
]))
33+
..map.deepEquals({
34+
key([1, 2]): 300,
35+
key([]): 200,
36+
key([1]): 100,
37+
})
38+
..sorted.deepEquals([key([1, 2]), key([]), key([1])]);
39+
});
40+
41+
group('message event (new message)', () {
42+
setupView() {
43+
return RecentDmConversationsView(selfUserId: eg.selfUser.userId,
44+
initial: [
45+
RecentDmConversation(userIds: [1], maxMessageId: 200),
46+
RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
47+
]);
48+
}
49+
50+
test('(check base state)', () {
51+
// This is here mostly for checked documentation of what
52+
// setupView returns, to help in reading the other test cases.
53+
check(setupView())
54+
..map.deepEquals({
55+
key([1]): 200,
56+
key([1, 2]): 100,
57+
})
58+
..sorted.deepEquals([key([1]), key([1, 2])]);
59+
});
60+
61+
test('stream message -> do nothing', () {
62+
bool listenersNotified = false;
63+
final expected = setupView();
64+
check(setupView()
65+
..addListener(() { listenersNotified = true; })
66+
..handleMessageEvent(MessageEvent(id: 1, message: eg.streamMessage()))
67+
) ..map.deepEquals(expected.map)
68+
..sorted.deepEquals(expected.sorted);
69+
check(listenersNotified).isFalse();
70+
});
71+
72+
test('new conversation, newest message', () {
73+
bool listenersNotified = false;
74+
final message = eg.dmMessage(id: 300, from: eg.selfUser, to: [eg.user(userId: 2)]);
75+
check(setupView()
76+
..addListener(() { listenersNotified = true; })
77+
..handleMessageEvent(MessageEvent(id: 1, message: message))
78+
) ..map.deepEquals({
79+
key([2]): 300,
80+
key([1]): 200,
81+
key([1, 2]): 100,
82+
})
83+
..sorted.deepEquals([key([2]), key([1]), key([1, 2])]);
84+
check(listenersNotified).isTrue();
85+
});
86+
87+
test('new conversation, not newest message', () {
88+
bool listenersNotified = false;
89+
final message = eg.dmMessage(id: 150, from: eg.selfUser, to: [eg.user(userId: 2)]);
90+
check(setupView()
91+
..addListener(() { listenersNotified = true; })
92+
..handleMessageEvent(MessageEvent(id: 1, message: message))
93+
) ..map.deepEquals({
94+
key([1]): 200,
95+
key([2]): 150,
96+
key([1, 2]): 100,
97+
})
98+
..sorted.deepEquals([key([1]), key([2]), key([1, 2])]);
99+
check(listenersNotified).isTrue();
100+
});
101+
102+
test('existing conversation, newest message', () {
103+
bool listenersNotified = false;
104+
final message = eg.dmMessage(id: 300, from: eg.selfUser,
105+
to: [eg.user(userId: 1), eg.user(userId: 2)]);
106+
check(setupView()
107+
..addListener(() { listenersNotified = true; })
108+
..handleMessageEvent(MessageEvent(id: 1, message: message))
109+
) ..map.deepEquals({
110+
key([1, 2]): 300,
111+
key([1]): 200,
112+
})
113+
..sorted.deepEquals([key([1, 2]), key([1])]);
114+
check(listenersNotified).isTrue();
115+
});
116+
117+
test('existing newest conversation, newest message', () {
118+
bool listenersNotified = false;
119+
final message = eg.dmMessage(id: 300, from: eg.selfUser, to: [eg.user(userId: 1)]);
120+
check(setupView()
121+
..addListener(() { listenersNotified = true; })
122+
..handleMessageEvent(MessageEvent(id: 1, message: message))
123+
) ..map.deepEquals({
124+
key([1]): 300,
125+
key([1, 2]): 100,
126+
})
127+
..sorted.deepEquals([key([1]), key([1, 2])]);
128+
check(listenersNotified).isTrue();
129+
});
130+
131+
test('existing conversation, not newest in conversation', () {
132+
final message = eg.dmMessage(id: 99, from: eg.selfUser,
133+
to: [eg.user(userId: 1), eg.user(userId: 2)]);
134+
final expected = setupView();
135+
check(setupView()
136+
..handleMessageEvent(MessageEvent(id: 1, message: message))
137+
) ..map.deepEquals(expected.map)
138+
..sorted.deepEquals(expected.sorted);
139+
});
140+
});
141+
});
142+
}

0 commit comments

Comments
 (0)