Skip to content

Commit d0344b9

Browse files
committed
autocomplete [nfc]: Factor out generic AutocompleteView
The mechanism of query and results is generic to the idea of autocomplete in general, it's not specific to autocomplete on @-mentions vs. topics vs. anything else.
1 parent 0bf8dfb commit d0344b9

File tree

3 files changed

+137
-96
lines changed

3 files changed

+137
-96
lines changed

lib/model/autocomplete.dart

Lines changed: 134 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import 'narrow.dart';
88
import 'store.dart';
99

1010
extension ComposeContentAutocomplete on ComposeContentController {
11-
AutocompleteIntent? autocompleteIntent() {
11+
AutocompleteIntent<MentionAutocompleteQuery>? autocompleteIntent() {
1212
if (!selection.isValid || !selection.isNormalized) {
1313
// We don't require [isCollapsed] to be true because we've seen that
1414
// autocorrect and even backspace involve programmatically expanding the
@@ -69,7 +69,7 @@ final RegExp mentionAutocompleteMarkerRegex = (() {
6969
})();
7070

7171
/// The content controller's recognition that the user might want autocomplete UI.
72-
class AutocompleteIntent {
72+
class AutocompleteIntent<Q extends AutocompleteQuery> {
7373
AutocompleteIntent({
7474
required this.syntaxStart,
7575
required this.query,
@@ -91,7 +91,7 @@ class AutocompleteIntent {
9191
// that use a custom/subclassed [TextEditingValue], so that's not convenient.
9292
final int syntaxStart;
9393

94-
final MentionAutocompleteQuery query; // TODO other autocomplete query types
94+
final Q query;
9595

9696
/// The [TextEditingValue] whose text [syntaxStart] refers to.
9797
final TextEditingValue textEditingValue;
@@ -151,83 +151,34 @@ class AutocompleteViewManager {
151151
// void dispose() { … }
152152
}
153153

154-
/// A view-model for a mention-autocomplete interaction.
154+
/// A view-model for an autocomplete interaction.
155155
///
156156
/// The owner of one of these objects must call [dispose] when the object
157157
/// will no longer be used, in order to free resources on the [PerAccountStore].
158158
///
159159
/// Lifecycle:
160-
/// * Create with [init].
160+
/// * Create an instance of a concrete subtype.
161161
/// * Add listeners with [addListener].
162162
/// * Use the [query] setter to start a search for a query.
163163
/// * On reassemble, call [reassemble].
164164
/// * When the object will no longer be used, call [dispose] to free
165165
/// resources on the [PerAccountStore].
166-
class MentionAutocompleteView extends ChangeNotifier {
167-
MentionAutocompleteView._({
168-
required this.store,
169-
required this.narrow,
170-
required this.sortedUsers,
171-
});
166+
abstract class AutocompleteView<Q extends AutocompleteQuery, R extends AutocompleteResult> extends ChangeNotifier {
172167

173-
factory MentionAutocompleteView.init({
174-
required PerAccountStore store,
175-
required Narrow narrow,
176-
}) {
177-
final view = MentionAutocompleteView._(
178-
store: store,
179-
narrow: narrow,
180-
sortedUsers: _usersByRelevance(store: store, narrow: narrow),
181-
);
182-
store.autocompleteViewManager.registerMentionAutocomplete(view);
183-
return view;
184-
}
168+
/// This could be used to transform results after they've been
169+
/// computed for sorting, filtering etc.
170+
final List<R> Function(List<R> results)? resultsFilter;
171+
final PerAccountStore store;
185172

186-
static List<User> _usersByRelevance({
187-
required PerAccountStore store,
188-
required Narrow narrow,
189-
}) {
190-
assert(narrow is! CombinedFeedNarrow);
191-
return store.users.values.toList()
192-
..sort((userA, userB) => compareByDms(userA, userB, store: store));
193-
}
173+
AutocompleteView({this.resultsFilter, required this.store});
194174

195-
/// Determines which of the two users is more recent in DM conversations.
196-
///
197-
/// Returns a negative number if [userA] is more recent than [userB],
198-
/// returns a positive number if [userB] is more recent than [userA],
199-
/// and returns `0` if both [userA] and [userB] are equally recent
200-
/// or there is no DM exchanged with them whatsoever.
201-
@visibleForTesting
202-
static int compareByDms(User userA, User userB, {required PerAccountStore store}) {
203-
final recentDms = store.recentDmConversationsView;
204-
final aLatestMessageId = recentDms.latestMessagesByRecipient[userA.userId];
205-
final bLatestMessageId = recentDms.latestMessagesByRecipient[userB.userId];
175+
Iterable<Object> getDataForQuery(Q query);
206176

207-
return switch((aLatestMessageId, bLatestMessageId)) {
208-
(int a, int b) => -a.compareTo(b),
209-
(int(), _) => -1,
210-
(_, int()) => 1,
211-
_ => 0,
212-
};
213-
}
177+
R? testItem(Q query, Object item);
214178

215-
@override
216-
void dispose() {
217-
store.autocompleteViewManager.unregisterMentionAutocomplete(this);
218-
// We cancel in-progress computations by checking [hasListeners] between tasks.
219-
// After [super.dispose] is called, [hasListeners] returns false.
220-
// TODO test that logic (may involve detecting an unhandled Future rejection; how?)
221-
super.dispose();
222-
}
223-
224-
final PerAccountStore store;
225-
final Narrow narrow;
226-
final List<User> sortedUsers;
227-
228-
MentionAutocompleteQuery? get query => _query;
229-
MentionAutocompleteQuery? _query;
230-
set query(MentionAutocompleteQuery? query) {
179+
Q? get query => _query;
180+
Q? _query;
181+
set query(Q? query) {
231182
_query = query;
232183
if (query != null) {
233184
_startSearch(query);
@@ -243,11 +194,22 @@ class MentionAutocompleteView extends ChangeNotifier {
243194
}
244195
}
245196

246-
Iterable<MentionAutocompleteResult> get results => _results;
247-
List<MentionAutocompleteResult> _results = [];
197+
Iterable<R> get results => _results;
198+
List<R> _results = [];
199+
200+
Future<void> _startSearch(Q query) async {
201+
List<R>? newResults;
202+
203+
while (true) {
204+
try {
205+
newResults = await _computeResults(query);
206+
break;
207+
} on ConcurrentModificationError {
208+
// Retry
209+
// TODO backoff?
210+
}
211+
}
248212

249-
Future<void> _startSearch(MentionAutocompleteQuery query) async {
250-
final newResults = await _computeResults(query);
251213
if (newResults == null) {
252214
// Query was old; new search is in progress. Or, no listeners to notify.
253215
return;
@@ -257,9 +219,11 @@ class MentionAutocompleteView extends ChangeNotifier {
257219
notifyListeners();
258220
}
259221

260-
Future<List<MentionAutocompleteResult>?> _computeResults(MentionAutocompleteQuery query) async {
261-
final List<MentionAutocompleteResult> results = [];
262-
final iterator = sortedUsers.iterator;
222+
Future<List<R>?> _computeResults(Q query) async {
223+
final List<R> results = [];
224+
final Iterable<Object> data = getDataForQuery(query);
225+
226+
final iterator = data.iterator;
263227
bool isDone = false;
264228
while (!isDone) {
265229
// CPU perf: End this task; enqueue a new one for resuming this work
@@ -274,41 +238,89 @@ class MentionAutocompleteView extends ChangeNotifier {
274238
isDone = true;
275239
break;
276240
}
277-
278-
final User user = iterator.current;
279-
if (query.testUser(user, store.autocompleteViewManager.autocompleteDataCache)) {
280-
results.add(UserMentionAutocompleteResult(userId: user.userId));
281-
}
241+
final Object item = iterator.current;
242+
final result = testItem(query, item);
243+
if (result != null) results.add(result);
282244
}
283245
}
284-
return results;
246+
return resultsFilter?.call(results) ?? results;
285247
}
286248
}
287249

288-
class MentionAutocompleteQuery {
289-
MentionAutocompleteQuery(this.raw, {this.silent = false})
290-
: _lowercaseWords = raw.toLowerCase().split(' ');
250+
class MentionAutocompleteView extends AutocompleteView<MentionAutocompleteQuery, MentionAutocompleteResult> {
251+
final Narrow narrow;
252+
final List<User> sortedUsers;
291253

292-
final String raw;
254+
MentionAutocompleteView.init({
255+
required super.store,
256+
required this.narrow,
257+
}) : sortedUsers = _usersByRelevance(store: store, narrow: narrow)
258+
{
259+
store.autocompleteViewManager.registerMentionAutocomplete(this);
260+
}
293261

294-
/// Whether the user wants a silent mention (@_query, vs. @query).
295-
final bool silent;
262+
static List<User> _usersByRelevance({
263+
required PerAccountStore store,
264+
required Narrow narrow,
265+
}) {
266+
assert(narrow is! CombinedFeedNarrow);
267+
return store.users.values.toList()
268+
..sort((userA, userB) => compareByDms(userA, userB, store: store));
269+
}
296270

297-
final List<String> _lowercaseWords;
271+
@override
272+
Iterable<Object> getDataForQuery(MentionAutocompleteQuery query) {
273+
return sortedUsers;
274+
}
298275

299-
bool testUser(User user, AutocompleteDataCache cache) {
300-
// TODO(#236) test email too, not just name
276+
@override
277+
MentionAutocompleteResult? testItem(MentionAutocompleteQuery query, Object item) {
278+
if (item is User) {
279+
if (query.testUser(item, store.autocompleteViewManager.autocompleteDataCache)) {
280+
return UserMentionAutocompleteResult(userId: item.userId);
281+
}
282+
}
283+
return null;
284+
}
301285

302-
if (!user.isActive) return false;
286+
/// Determines which of the two users is more recent in DM conversations.
287+
///
288+
/// Returns a negative number if [userA] is more recent than [userB],
289+
/// returns a positive number if [userB] is more recent than [userA],
290+
/// and returns `0` if both [userA] and [userB] are equally recent
291+
/// or there is no DM exchanged with them whatsoever.
292+
@visibleForTesting
293+
static int compareByDms(User userA, User userB, {required PerAccountStore store}) {
294+
final recentDms = store.recentDmConversationsView;
295+
final aLatestMessageId = recentDms.latestMessagesByRecipient[userA.userId];
296+
final bLatestMessageId = recentDms.latestMessagesByRecipient[userB.userId];
303297

304-
return _testName(user, cache);
298+
return switch((aLatestMessageId, bLatestMessageId)) {
299+
(int a, int b) => -a.compareTo(b),
300+
(int(), _) => -1,
301+
(_, int()) => 1,
302+
_ => 0,
303+
};
305304
}
306305

307-
bool _testName(User user, AutocompleteDataCache cache) {
308-
// TODO(#237) test with diacritics stripped, where appropriate
306+
@override
307+
void dispose() {
308+
store.autocompleteViewManager.unregisterMentionAutocomplete(this);
309+
// We cancel in-progress computations by checking [hasListeners] between tasks.
310+
// After [super.dispose] is called, [hasListeners] returns false.
311+
// TODO test that logic (may involve detecting an unhandled Future rejection; how?)
312+
super.dispose();
313+
}
314+
}
309315

310-
final List<String> nameWords = cache.nameWordsForUser(user);
316+
abstract class AutocompleteQuery {
317+
final String raw;
318+
final List<String> _lowercaseWords;
311319

320+
AutocompleteQuery(this.raw) : _lowercaseWords = raw.toLowerCase().split(' ');
321+
322+
bool _testContainsQueryWords(List<String> nameWords) {
323+
// TODO(#237) test with diacritics stripped, where appropriate
312324
int nameWordsIndex = 0;
313325
int queryWordsIndex = 0;
314326
while (true) {
@@ -326,6 +338,33 @@ class MentionAutocompleteQuery {
326338
}
327339
}
328340

341+
@override
342+
String toString() {
343+
return '${objectRuntimeType(this, 'AutocompleteQuery')}(raw: $raw})';
344+
}
345+
346+
@override
347+
bool operator ==(Object other) {
348+
return other is AutocompleteQuery && other.raw == raw;
349+
}
350+
351+
@override
352+
int get hashCode => Object.hash('AutocompleteQuery', raw);
353+
}
354+
355+
class MentionAutocompleteQuery extends AutocompleteQuery {
356+
/// Whether the user wants a silent mention (@_query, vs. @query).
357+
final bool silent;
358+
359+
MentionAutocompleteQuery(super.raw, {this.silent = false});
360+
361+
bool testUser(User user, AutocompleteDataCache cache) {
362+
// TODO(#236) test email too, not just name
363+
if (!user.isActive) return false;
364+
365+
return _testContainsQueryWords(cache.nameWordsForUser(user));
366+
}
367+
329368
@override
330369
String toString() {
331370
return '${objectRuntimeType(this, 'MentionAutocompleteQuery')}(raw: $raw, silent: $silent})';
@@ -352,7 +391,9 @@ class AutocompleteDataCache {
352391
}
353392
}
354393

355-
sealed class MentionAutocompleteResult {}
394+
class AutocompleteResult {}
395+
396+
sealed class MentionAutocompleteResult extends AutocompleteResult {}
356397

357398
class UserMentionAutocompleteResult extends MentionAutocompleteResult {
358399
UserMentionAutocompleteResult({required this.userId});

test/model/autocomplete_checks.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ import 'package:zulip/model/autocomplete.dart';
33
import 'package:zulip/widgets/compose_box.dart';
44

55
extension ComposeContentControllerChecks on Subject<ComposeContentController> {
6-
Subject<AutocompleteIntent?> get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent');
6+
Subject<AutocompleteIntent<MentionAutocompleteQuery>?> get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent');
77
}
88

9-
extension AutocompleteIntentChecks on Subject<AutocompleteIntent> {
9+
extension AutocompleteIntentChecks on Subject<AutocompleteIntent<MentionAutocompleteQuery>> {
1010
Subject<int> get syntaxStart => has((i) => i.syntaxStart, 'syntaxStart');
1111
Subject<MentionAutocompleteQuery> get query => has((i) => i.query, 'query');
1212
}

test/widgets/compose_box_checks.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ extension ComposeContentControllerChecks on Subject<ComposeContentController> {
66
Subject<List<ContentValidationError>> get validationErrors => has((c) => c.validationErrors, 'validationErrors');
77
}
88

9-
extension AutocompleteIntentChecks on Subject<AutocompleteIntent> {
9+
extension AutocompleteIntentChecks on Subject<AutocompleteIntent<MentionAutocompleteQuery>> {
1010
Subject<int> get syntaxStart => has((i) => i.syntaxStart, 'syntaxStart');
1111
Subject<MentionAutocompleteQuery> get query => has((i) => i.query, 'query');
1212
}

0 commit comments

Comments
 (0)