diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index d35ce8f26a..5b0522f42c 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -1,4 +1,176 @@ +import 'package:flutter/foundation.dart'; + +import '../api/model/events.dart'; import '../api/model/model.dart'; +import 'narrow.dart'; +import 'store.dart'; + +/// A per-account manager for the view-models of autocomplete interactions. +/// +/// There should be exactly one of these per PerAccountStore. +/// +/// Since this manages a cache of user data, the handleRealmUser…Event functions +/// must be called as appropriate. +/// +/// On reassemble, call [reassemble]. +class AutocompleteViewManager { + final Set _mentionAutocompleteViews = {}; + + AutocompleteDataCache autocompleteDataCache = AutocompleteDataCache(); + + void registerMentionAutocomplete(MentionAutocompleteView view) { + final added = _mentionAutocompleteViews.add(view); + assert(added); + } + + void unregisterMentionAutocomplete(MentionAutocompleteView view) { + final removed = _mentionAutocompleteViews.remove(view); + assert(removed); + } + + void handleRealmUserAddEvent(RealmUserAddEvent event) { + for (final view in _mentionAutocompleteViews) { + view.refreshStaleUserResults(); + } + } + + void handleRealmUserRemoveEvent(RealmUserRemoveEvent event) { + for (final view in _mentionAutocompleteViews) { + view.refreshStaleUserResults(); + } + autocompleteDataCache.invalidateUser(event.userId); + } + + void handleRealmUserUpdateEvent(RealmUserUpdateEvent event) { + for (final view in _mentionAutocompleteViews) { + view.refreshStaleUserResults(); + } + autocompleteDataCache.invalidateUser(event.userId); + } + + /// Called when the app is reassembled during debugging, e.g. for hot reload. + /// + /// Calls [MentionAutocompleteView.reassemble] for all that are registered. + /// + void reassemble() { + for (final view in _mentionAutocompleteViews) { + view.reassemble(); + } + } +} + +/// A view-model for a mention-autocomplete interaction. +/// +/// The owner of one of these objects must call [dispose] when the object +/// will no longer be used, in order to free resources on the [PerAccountStore]. +/// +/// Lifecycle: +/// * Create with [init]. +/// * Add listeners with [addListener]. +/// * Use the [query] setter to start a search for a query. +/// * On reassemble, call [reassemble]. +/// * When the object will no longer be used, call [dispose] to free +/// resources on the [PerAccountStore]. +class MentionAutocompleteView extends ChangeNotifier { + MentionAutocompleteView._({required this.store, required this.narrow}); + + factory MentionAutocompleteView.init({ + required PerAccountStore store, + required Narrow narrow, + }) { + final view = MentionAutocompleteView._(store: store, narrow: narrow); + store.autocompleteViewManager.registerMentionAutocomplete(view); + return view; + } + + @override + void dispose() { + store.autocompleteViewManager.unregisterMentionAutocomplete(this); + // TODO cancel in-progress computations if possible + super.dispose(); + } + + final PerAccountStore store; + final Narrow narrow; + + MentionAutocompleteQuery? _currentQuery; + set query(MentionAutocompleteQuery query) { + _currentQuery = query; + _startSearch(query); + } + + /// Recompute user results for the current query, if any. + /// + /// Called in particular when we get a [RealmUserEvent]. + void refreshStaleUserResults() { + if (_currentQuery != null) { + _startSearch(_currentQuery!); + } + } + + /// Called when the app is reassembled during debugging, e.g. for hot reload. + /// + /// This will redo the search from scratch for the current query, if any. + void reassemble() { + if (_currentQuery != null) { + _startSearch(_currentQuery!); + } + } + + Iterable get results => _results; + List _results = []; + + _startSearch(MentionAutocompleteQuery query) async { + List? newResults; + + while (true) { + try { + newResults = await _computeResults(query); + break; + } on ConcurrentModificationError { + // Retry + // TODO backoff? + } + } + + if (newResults == null) { + // Query was old; new search is in progress. + return; + } + + _results = newResults; + notifyListeners(); + } + + Future?> _computeResults(MentionAutocompleteQuery query) async { + final List results = []; + final Iterable users = store.users.values; + + final iterator = users.iterator; + bool isDone = false; + while (!isDone) { + // CPU perf: End this task; enqueue a new one for resuming this work + await Future(() {}); + + if (query != _currentQuery) { + return null; + } + + for (int i = 0; i < 1000; i++) { + if (!iterator.moveNext()) { // Can throw ConcurrentModificationError + isDone = true; + break; + } + + final User user = iterator.current; + if (query.testUser(user, store.autocompleteViewManager.autocompleteDataCache)) { + results.add(UserMentionAutocompleteResult(userId: user.userId)); + } + } + } + return results; + } +} class MentionAutocompleteQuery { MentionAutocompleteQuery(this.raw) @@ -8,12 +180,11 @@ class MentionAutocompleteQuery { final List _lowercaseWords; - bool testUser(User user) { + bool testUser(User user, AutocompleteDataCache cache) { // TODO test email too, not just name // TODO test with diacritics stripped, where appropriate - // TODO cache, elsewhere - final List nameWords = user.fullName.toLowerCase().split(' '); + final List nameWords = cache.nameWordsForUser(user); int nameWordsIndex = 0; int queryWordsIndex = 0; @@ -40,3 +211,42 @@ class MentionAutocompleteQuery { @override int get hashCode => Object.hash('MentionAutocompleteQuery', raw); } + +class AutocompleteDataCache { + final Map> _nameWordsByUser = {}; + + List nameWordsForUser(User user) { + return _nameWordsByUser[user.userId] ??= user.fullName.toLowerCase().split(' '); + } + + void invalidateUser(int userId) { + _nameWordsByUser.remove(userId); + } +} + +abstract class MentionAutocompleteResult {} + +class UserMentionAutocompleteResult extends MentionAutocompleteResult { + UserMentionAutocompleteResult({required this.userId}); + + final int userId; +} + +enum WildcardMentionType { + all, + everyone, + stream, +} + +class WildcardMentionAutocompleteResult extends MentionAutocompleteResult { + WildcardMentionAutocompleteResult({required this.type}); + + final WildcardMentionType type; +} + + +class UserGroupMentionAutocompleteResult extends MentionAutocompleteResult { + UserGroupMentionAutocompleteResult({required this.userGroupId}); + + final int userGroupId; +} diff --git a/lib/model/store.dart b/lib/model/store.dart index 5def307d63..3981975792 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -12,6 +12,8 @@ import '../api/model/initial_snapshot.dart'; import '../api/model/model.dart'; import '../api/route/events.dart'; import '../api/route/messages.dart'; +import '../log.dart'; +import 'autocomplete.dart'; import 'database.dart'; import 'message_list.dart'; @@ -177,6 +179,8 @@ class PerAccountStore extends ChangeNotifier { assert(removed); } + final AutocompleteViewManager autocompleteViewManager = AutocompleteViewManager(); + /// Called when the app is reassembled during debugging, e.g. for hot reload. /// /// This will redo from scratch any computations we can, such as parsing @@ -185,24 +189,27 @@ class PerAccountStore extends ChangeNotifier { for (final view in _messageListViews) { view.reassemble(); } + autocompleteViewManager.reassemble(); } void handleEvent(Event event) { if (event is HeartbeatEvent) { - debugPrint("server event: heartbeat"); + assert(debugLog("server event: heartbeat")); } else if (event is AlertWordsEvent) { - debugPrint("server event: alert_words"); + assert(debugLog("server event: alert_words")); // We don't yet store this data, so there's nothing to update. } else if (event is RealmUserAddEvent) { - debugPrint("server event: realm_user/add"); + assert(debugLog("server event: realm_user/add")); users[event.person.userId] = event.person; + autocompleteViewManager.handleRealmUserAddEvent(event); notifyListeners(); } else if (event is RealmUserRemoveEvent) { - debugPrint("server event: realm_user/remove"); + assert(debugLog("server event: realm_user/remove")); users.remove(event.userId); + autocompleteViewManager.handleRealmUserRemoveEvent(event); notifyListeners(); } else if (event is RealmUserUpdateEvent) { - debugPrint("server event: realm_user/update"); + assert(debugLog("server event: realm_user/update")); final user = users[event.userId]; if (user == null) { return; // TODO log @@ -225,14 +232,15 @@ class PerAccountStore extends ChangeNotifier { profileData.remove(update.id); } } + autocompleteViewManager.handleRealmUserUpdateEvent(event); notifyListeners(); } else if (event is MessageEvent) { - debugPrint("server event: message ${jsonEncode(event.message.toJson())}"); + assert(debugLog("server event: message ${jsonEncode(event.message.toJson())}")); for (final view in _messageListViews) { view.maybeAddMessage(event.message); } } else if (event is UnexpectedEvent) { - debugPrint("server event: ${jsonEncode(event.toJson())}"); // TODO log better + assert(debugLog("server event: ${jsonEncode(event.toJson())}")); // TODO log better } else { // TODO(dart-3): Use a sealed class / pattern-matching to exclude this. throw Exception("Event object of impossible type: ${event.toString()}"); diff --git a/pubspec.lock b/pubspec.lock index be4f342e51..2212ef6682 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -274,7 +274,7 @@ packages: source: hosted version: "2.7.0" fake_async: - dependency: transitive + dependency: "direct dev" description: name: fake_async sha256: "511392330127add0b769b75a987850d136345d9227c6b94c96a04cf4a391bf78" diff --git a/pubspec.yaml b/pubspec.yaml index 94b47f7c10..36485f3395 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -71,6 +71,7 @@ dev_dependencies: test: ^1.23.1 checks: ^0.2.2 drift_dev: ^2.5.2 + fake_async: ^1.3.1 # For information on the generic Dart part of this file, see the # following page: https://dart.dev/tools/pub/pubspec diff --git a/test/example_data.dart b/test/example_data.dart index aa335d1760..8efa4809df 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -2,17 +2,19 @@ import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/model/store.dart'; +import 'api/fake_api.dart'; + final Uri realmUrl = Uri.parse('https://chat.example/'); const String recentZulipVersion = '6.1'; const int recentZulipFeatureLevel = 164; -User user({int? userId, String? fullName}) { +User user({int? userId, String? email, String? fullName}) { return User( userId: userId ?? 123, // TODO generate example IDs deliveryEmailStaleDoNotUse: 'name@example.com', - email: 'name@example.com', // TODO generate example emails - fullName: fullName ?? 'A user',// TODO generate example names + email: email ?? 'name@example.com', // TODO generate example emails + fullName: fullName ?? 'A user', // TODO generate example names dateJoined: '2023-04-28', isActive: true, isOwner: false, @@ -28,28 +30,32 @@ User user({int? userId, String? fullName}) { ); } +final User selfUser = user(fullName: 'Self User', email: 'self@example', userId: 123); final Account selfAccount = Account( id: 1001, realmUrl: realmUrl, - email: 'self@example', + email: selfUser.email, apiKey: 'asdfqwer', - userId: 123, + userId: selfUser.userId, zulipFeatureLevel: recentZulipFeatureLevel, zulipVersion: recentZulipVersion, zulipMergeBase: recentZulipVersion, ); +final User otherUser = user(fullName: 'Other User', email: 'other@example', userId: 234); final Account otherAccount = Account( id: 1002, realmUrl: realmUrl, - email: 'other@example', + email: otherUser.email, apiKey: 'sdfgwert', - userId: 234, + userId: otherUser.userId, zulipFeatureLevel: recentZulipFeatureLevel, zulipVersion: recentZulipVersion, zulipMergeBase: recentZulipVersion, ); +final User thirdUser = user(fullName: 'Third User', email: 'third@example', userId: 345); + final _messagePropertiesBase = { 'is_me_message': false, 'last_edit_timestamp': null, @@ -108,3 +114,11 @@ final InitialSnapshot initialSnapshot = InitialSnapshot( realmNonActiveUsers: [], crossRealmBots: [], ); + +PerAccountStore store() { + return PerAccountStore.fromInitialSnapshot( + account: selfAccount, + connection: FakeApiConnection.fromAccount(selfAccount), + initialSnapshot: initialSnapshot, + ); +} diff --git a/test/model/autocomplete_checks.dart b/test/model/autocomplete_checks.dart new file mode 100644 index 0000000000..33fdfbc9ff --- /dev/null +++ b/test/model/autocomplete_checks.dart @@ -0,0 +1,6 @@ +import 'package:checks/checks.dart'; +import 'package:zulip/model/autocomplete.dart'; + +extension UserMentionAutocompleteResultChecks on Subject { + Subject get userId => has((r) => r.userId, 'userId'); +} diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index 29b7530d56..d4432de9f5 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -1,14 +1,161 @@ +import 'dart:async'; + import 'package:checks/checks.dart'; +import 'package:fake_async/fake_async.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/model/autocomplete.dart'; +import 'package:zulip/model/narrow.dart'; import '../example_data.dart' as eg; +import 'test_store.dart'; +import 'autocomplete_checks.dart'; void main() { + test('MentionAutocompleteView misc', () async { + const narrow = AllMessagesNarrow(); + final store = eg.store() + ..addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]); + final view = MentionAutocompleteView.init(store: store, narrow: narrow); + + bool done = false; + view.addListener(() { done = true; }); + view.query = MentionAutocompleteQuery('Third'); + await Future(() {}); + check(done).isTrue(); + check(view.results).single + .isA() + .userId.equals(eg.thirdUser.userId); + }); + + test('MentionAutocompleteView not starve timers', () { + fakeAsync((binding) { + const narrow = AllMessagesNarrow(); + final store = eg.store() + ..addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]); + final view = MentionAutocompleteView.init(store: store, narrow: narrow); + + bool searchDone = false; + view.addListener(() { + searchDone = true; + }); + + // Schedule a timer event with zero delay. + // This stands in for a user interaction, or frame rendering timer, + // placing an urgent task on the event queue. + bool timerDone = false; + Timer(const Duration(), () { + timerDone = true; + // The timer should go first, before the search does its work. + check(searchDone).isFalse(); + }); + + view.query = MentionAutocompleteQuery('Third'); + check(timerDone).isFalse(); + check(searchDone).isFalse(); + + binding.elapse(const Duration(seconds: 1)); + + check(timerDone).isTrue(); + check(searchDone).isTrue(); + check(view.results).single + .isA() + .userId.equals(eg.thirdUser.userId); + }); + }); + + test('MentionAutocompleteView yield between batches of 1000', () async { + const narrow = AllMessagesNarrow(); + final store = eg.store(); + for (int i = 0; i < 2500; i++) { + store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i')); + } + final view = MentionAutocompleteView.init(store: store, narrow: narrow); + + bool done = false; + view.addListener(() { done = true; }); + view.query = MentionAutocompleteQuery('User 2222'); + + await Future(() {}); + check(done).isFalse(); + await Future(() {}); + check(done).isFalse(); + await Future(() {}); + check(done).isTrue(); + check(view.results).single + .isA() + .userId.equals(2222); + }); + + test('MentionAutocompleteView new query during computation replaces old', () async { + const narrow = AllMessagesNarrow(); + final store = eg.store(); + for (int i = 0; i < 1500; i++) { + store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i')); + } + final view = MentionAutocompleteView.init(store: store, narrow: narrow); + + bool done = false; + view.addListener(() { done = true; }); + view.query = MentionAutocompleteQuery('User 1111'); + + await Future(() {}); + check(done).isFalse(); + view.query = MentionAutocompleteQuery('User 0'); + + // …new query goes through all batches + await Future(() {}); + check(done).isFalse(); + await Future(() {}); + check(done).isTrue(); // new result is set + check(view.results).single + .isA() + .userId.equals(0); + + // new result sticks; it isn't clobbered with old query's result + for (int i = 0; i < 10; i++) { // for good measure + await Future(() {}); + check(view.results).single + .isA() + .userId.equals(0); + } + }); + + test('MentionAutocompleteView mutating store.users while in progress causes retry', () async { + const narrow = AllMessagesNarrow(); + final store = eg.store(); + for (int i = 0; i < 1500; i++) { + store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i')); + } + final view = MentionAutocompleteView.init(store: store, narrow: narrow); + + bool done = false; + view.addListener(() { done = true; }); + view.query = MentionAutocompleteQuery('User 10000'); + + await Future(() {}); + check(done).isFalse(); + store.addUser(eg.user(userId: 10000, email: 'user10000@example.com', fullName: 'User 10000')); + await Future(() {}); + check(done).isFalse(); + await Future(() {}); + check(done).isTrue(); + check(view.results).single + .isA() + .userId.equals(10000); + // new result sticks; no "zombie" result from `store.users` pre-mutation + for (int i = 0; i < 10; i++) { // for good measure + await Future(() {}); + check(view.results).single + .isA() + .userId.equals(10000); + } + }); + test('MentionAutocompleteQuery.testUser', () { doCheck(String rawQuery, User user, bool expected) { - final result = MentionAutocompleteQuery(rawQuery).testUser(user); + final result = MentionAutocompleteQuery(rawQuery) + .testUser(user, AutocompleteDataCache()); expected ? check(result).isTrue() : check(result).isFalse(); } diff --git a/test/model/test_store.dart b/test/model/test_store.dart new file mode 100644 index 0000000000..e56c19c5c2 --- /dev/null +++ b/test/model/test_store.dart @@ -0,0 +1,15 @@ +import 'package:zulip/api/model/events.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/store.dart'; + +extension PerAccountStoreTestExtension on PerAccountStore { + void addUser(User user) { + handleEvent(RealmUserAddEvent(id: 1, person: user)); + } + + void addUsers(Iterable users) { + for (final user in users) { + addUser(user); + } + } +}