Skip to content

Commit fc9147f

Browse files
committed
autocomplete: Introduce a field for sorted users
This field will be used to maintain a list of sorted users based on the most relevant autocomplete criteria in the upcoming commits. This commit also removes the retry logic for an in-progress query computation as there is no longer the cause that would necessitate such a retry.
1 parent d0c9e52 commit fc9147f

File tree

2 files changed

+22
-48
lines changed

2 files changed

+22
-48
lines changed

lib/model/autocomplete.dart

+22-17
Original file line numberDiff line numberDiff line change
@@ -164,20 +164,35 @@ class AutocompleteViewManager {
164164
/// * When the object will no longer be used, call [dispose] to free
165165
/// resources on the [PerAccountStore].
166166
class MentionAutocompleteView extends ChangeNotifier {
167-
MentionAutocompleteView._({required this.store, required this.narrow});
167+
MentionAutocompleteView._({
168+
required this.store,
169+
required this.narrow,
170+
required this.sortedUsers,
171+
});
168172

169173
factory MentionAutocompleteView.init({
170174
required PerAccountStore store,
171175
required Narrow narrow,
172176
}) {
173-
final view = MentionAutocompleteView._(store: store, narrow: narrow);
177+
final users = store.users.values.toList();
178+
final sortedUsers = sortByRelevance(users: users);
179+
final view = MentionAutocompleteView._(
180+
store: store,
181+
narrow: narrow,
182+
sortedUsers: sortedUsers,
183+
);
174184
store.autocompleteViewManager.registerMentionAutocomplete(view);
175185
return view;
176186
}
187+
188+
static List<User> sortByRelevance({required List<User> users}) {
189+
return users; // TODO(#228) sort for most relevant first
190+
}
177191

178192
@override
179193
void dispose() {
180194
store.autocompleteViewManager.unregisterMentionAutocomplete(this);
195+
sortedUsers.clear();
181196
// We cancel in-progress computations by checking [hasListeners] between tasks.
182197
// After [super.dispose] is called, [hasListeners] returns false.
183198
// TODO test that logic (may involve detecting an unhandled Future rejection; how?)
@@ -186,6 +201,7 @@ class MentionAutocompleteView extends ChangeNotifier {
186201

187202
final PerAccountStore store;
188203
final Narrow narrow;
204+
final List<User> sortedUsers;
189205

190206
MentionAutocompleteQuery? get query => _query;
191207
MentionAutocompleteQuery? _query;
@@ -209,17 +225,7 @@ class MentionAutocompleteView extends ChangeNotifier {
209225
List<MentionAutocompleteResult> _results = [];
210226

211227
_startSearch(MentionAutocompleteQuery query) async {
212-
List<MentionAutocompleteResult>? newResults;
213-
214-
while (true) {
215-
try {
216-
newResults = await _computeResults(query);
217-
break;
218-
} on ConcurrentModificationError {
219-
// Retry
220-
// TODO backoff?
221-
}
222-
}
228+
final newResults = await _computeResults(query);
223229

224230
if (newResults == null) {
225231
// Query was old; new search is in progress. Or, no listeners to notify.
@@ -232,9 +238,8 @@ class MentionAutocompleteView extends ChangeNotifier {
232238

233239
Future<List<MentionAutocompleteResult>?> _computeResults(MentionAutocompleteQuery query) async {
234240
final List<MentionAutocompleteResult> results = [];
235-
final Iterable<User> users = store.users.values;
236241

237-
final iterator = users.iterator;
242+
final iterator = sortedUsers.iterator;
238243
bool isDone = false;
239244
while (!isDone) {
240245
// CPU perf: End this task; enqueue a new one for resuming this work
@@ -245,7 +250,7 @@ class MentionAutocompleteView extends ChangeNotifier {
245250
}
246251

247252
for (int i = 0; i < 1000; i++) {
248-
if (!iterator.moveNext()) { // Can throw ConcurrentModificationError
253+
if (!iterator.moveNext()) {
249254
isDone = true;
250255
break;
251256
}
@@ -256,7 +261,7 @@ class MentionAutocompleteView extends ChangeNotifier {
256261
}
257262
}
258263
}
259-
return results; // TODO(#228) sort for most relevant first
264+
return results;
260265
}
261266
}
262267

test/model/autocomplete_test.dart

-31
Original file line numberDiff line numberDiff line change
@@ -274,37 +274,6 @@ void main() {
274274
}
275275
});
276276

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);
284-
285-
bool done = false;
286-
view.addListener(() { done = true; });
287-
view.query = MentionAutocompleteQuery('User 10000');
288-
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
301-
await Future(() {});
302-
check(view.results).single
303-
.isA<UserMentionAutocompleteResult>()
304-
.userId.equals(10000);
305-
}
306-
});
307-
308277
group('MentionAutocompleteQuery.testUser', () {
309278
doCheck(String rawQuery, User user, bool expected) {
310279
final result = MentionAutocompleteQuery(rawQuery)

0 commit comments

Comments
 (0)