Skip to content

Commit 7354ce7

Browse files
committed
autocomplete: Terminate the already-running search when there is a new one
1 parent efe3686 commit 7354ce7

File tree

3 files changed

+116
-32
lines changed

3 files changed

+116
-32
lines changed

lib/model/autocomplete.dart

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -235,14 +235,11 @@ class MentionAutocompleteView extends ChangeNotifier {
235235
_startSearch(MentionAutocompleteQuery query) async {
236236
List<MentionAutocompleteResult>? newResults;
237237

238-
while (true) {
239-
try {
240-
newResults = await _computeResults(query);
241-
break;
242-
} on ConcurrentModificationError {
243-
// Retry
244-
// TODO backoff?
245-
}
238+
try {
239+
newResults = await _computeResults(query);
240+
} on ConcurrentModificationError {
241+
// Terminate this search as there is a new search going on
242+
return;
246243
}
247244

248245
if (newResults == null) {

lib/model/store.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,16 @@ class PerAccountStore extends ChangeNotifier with StreamStore {
448448
user.profileData = null;
449449
}
450450
}
451+
// The following two lines will eventually not change [users], but are here to
452+
// trigger a modification to the collection as only changing the properties
453+
// of [user] does not signal a modification of [users] where needed.
454+
//
455+
// One example of this is [MentionAutocompleteView._computeResults] which
456+
// throws a `ConcurrentModificationError` when there is a modification done
457+
// to [users].
458+
users[-1] = user;
459+
users.remove(-1);
460+
451461
autocompleteViewManager.handleRealmUserUpdateEvent(event);
452462
notifyListeners();
453463
} else if (event is StreamEvent) {

test/model/autocomplete_test.dart

Lines changed: 101 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:test/scaffolding.dart';
88
import 'package:zulip/api/model/model.dart';
99
import 'package:zulip/model/autocomplete.dart';
1010
import 'package:zulip/model/narrow.dart';
11+
import 'package:zulip/model/store.dart';
1112
import 'package:zulip/widgets/compose_box.dart';
1213

1314
import '../example_data.dart' as eg;
@@ -274,35 +275,111 @@ void main() {
274275
}
275276
});
276277

277-
test('MentionAutocompleteView mutating store.users while in progress causes retry', () async {
278-
const narrow = CombinedFeedNarrow();
279-
final store = eg.store();
280-
for (int i = 0; i < 1500; i++) {
281-
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
282-
}
283-
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
278+
group('MentionAutocompleteView recomputes results', () {
279+
Future<void> doCheck({
280+
required List<User> users,
281+
required String rawQuery,
282+
required Future<void> Function(PerAccountStore store) act,
283+
required void Function(Iterable<MentionAutocompleteResult>) expect,
284+
}) async {
285+
const narrow = CombinedFeedNarrow();
286+
final store = eg.store();
287+
await store.addUsers(users);
288+
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
284289

285-
bool done = false;
286-
view.addListener(() { done = true; });
287-
view.query = MentionAutocompleteQuery('User 10000');
290+
bool done = false;
291+
view.addListener(() { done = true; });
292+
view.query = MentionAutocompleteQuery(rawQuery);
288293

289-
await Future(() {});
290-
check(done).isFalse();
291-
await store.addUser(eg.user(userId: 10000, email: '[email protected]', fullName: 'User 10000'));
292-
await Future(() {});
293-
check(done).isFalse();
294-
await Future(() {});
295-
check(done).isTrue();
296-
check(view.results).single
297-
.isA<UserMentionAutocompleteResult>()
298-
.userId.equals(10000);
299-
// new result sticks; no "zombie" result from `store.users` pre-mutation
300-
for (int i = 0; i < 10; i++) { // for good measure
301294
await Future(() {});
302-
check(view.results).single
295+
check(done).isFalse();
296+
297+
await act(store);
298+
299+
await Future(() {});
300+
check(done).isFalse();
301+
await Future(() {});
302+
check(done).isTrue();
303+
expect(view.results);
304+
// new result sticks; no "zombie" result
305+
for (int i = 0; i < 10; i++) { // for good measure
306+
await Future(() {});
307+
expect(view.results);
308+
}
309+
}
310+
311+
List<User> generateUsers({required int count}) {
312+
return List.generate(
313+
count,
314+
(i) => eg.user(
315+
userId: i,
316+
email: 'user$i@example.com',
317+
fullName: 'User $i'));
318+
}
319+
320+
test('RealmUserAddEvent', () async {
321+
final users = generateUsers(count: 1500);
322+
323+
Future<void> act(PerAccountStore store) async {
324+
await store.addUser(eg.user(
325+
userId: 10000,
326+
327+
fullName: 'User 10000'));
328+
}
329+
330+
void expect(Iterable<MentionAutocompleteResult> results) {
331+
check(results).single
303332
.isA<UserMentionAutocompleteResult>()
304333
.userId.equals(10000);
305-
}
334+
}
335+
336+
await doCheck(
337+
users: users,
338+
rawQuery: 'User 10000',
339+
act: act,
340+
expect: expect,
341+
);
342+
});
343+
344+
test('RealmUserRemoveEvent', () async {
345+
final users = generateUsers(count: 1500);
346+
347+
Future<void> act(PerAccountStore store) async {
348+
await store.removeUser(1495);
349+
}
350+
351+
void expect(Iterable<MentionAutocompleteResult> results) {
352+
check(results).isEmpty();
353+
}
354+
355+
await doCheck(
356+
users: users,
357+
rawQuery: 'User 1495',
358+
act: act,
359+
expect: expect,
360+
);
361+
});
362+
363+
test('RealmUserUpdateEvent', () async {
364+
final users = generateUsers(count: 1500);
365+
366+
Future<void> act(PerAccountStore store) async {
367+
await store.updateUser(1, fullName: 'Updated User 1');
368+
}
369+
370+
void expect(Iterable<MentionAutocompleteResult> results) {
371+
check(results).single
372+
.isA<UserMentionAutocompleteResult>()
373+
.userId.equals(1);
374+
}
375+
376+
await doCheck(
377+
users: users,
378+
rawQuery: 'Updated User 1',
379+
act: act,
380+
expect: expect,
381+
);
382+
});
306383
});
307384

308385
group('MentionAutocompleteQuery.testUser', () {

0 commit comments

Comments
 (0)