Skip to content

autocomplete: Add and test MentionAutocompleteView #102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
216 changes: 213 additions & 3 deletions lib/model/autocomplete.dart
Original file line number Diff line number Diff line change
@@ -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<MentionAutocompleteView> _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<MentionAutocompleteResult> get results => _results;
List<MentionAutocompleteResult> _results = [];

_startSearch(MentionAutocompleteQuery query) async {
List<MentionAutocompleteResult>? 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<List<MentionAutocompleteResult>?> _computeResults(MentionAutocompleteQuery query) async {
final List<MentionAutocompleteResult> results = [];
final Iterable<User> 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)
Expand All @@ -8,12 +180,11 @@ class MentionAutocompleteQuery {

final List<String> _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<String> nameWords = user.fullName.toLowerCase().split(' ');
final List<String> nameWords = cache.nameWordsForUser(user);

int nameWordsIndex = 0;
int queryWordsIndex = 0;
Expand All @@ -40,3 +211,42 @@ class MentionAutocompleteQuery {
@override
int get hashCode => Object.hash('MentionAutocompleteQuery', raw);
}

class AutocompleteDataCache {
final Map<int, List<String>> _nameWordsByUser = {};

List<String> 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;
}
22 changes: 15 additions & 7 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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()}");
Expand Down
2 changes: 1 addition & 1 deletion pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ packages:
source: hosted
version: "2.7.0"
fake_async:
dependency: transitive
dependency: "direct dev"
description:
name: fake_async
sha256: "511392330127add0b769b75a987850d136345d9227c6b94c96a04cf4a391bf78"
Expand Down
1 change: 1 addition & 0 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 21 additions & 7 deletions test/example_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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: '[email protected]',
email: '[email protected]', // TODO generate example emails
fullName: fullName ?? 'A user',// TODO generate example names
email: email ?? '[email protected]', // TODO generate example emails
fullName: fullName ?? 'A user', // TODO generate example names
dateJoined: '2023-04-28',
isActive: true,
isOwner: false,
Expand All @@ -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,
Expand Down Expand Up @@ -108,3 +114,11 @@ final InitialSnapshot initialSnapshot = InitialSnapshot(
realmNonActiveUsers: [],
crossRealmBots: [],
);

PerAccountStore store() {
return PerAccountStore.fromInitialSnapshot(
account: selfAccount,
connection: FakeApiConnection.fromAccount(selfAccount),
initialSnapshot: initialSnapshot,
);
}
6 changes: 6 additions & 0 deletions test/model/autocomplete_checks.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import 'package:checks/checks.dart';
import 'package:zulip/model/autocomplete.dart';

extension UserMentionAutocompleteResultChecks on Subject<UserMentionAutocompleteResult> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(FWIW I'd be inclined to put this one inside the individual test file, down at the end after main, because it seems likely we'll only want it within the one file and because it'd be easy to move later if we want it in several files after all. But in a separate file is fine too.)

Subject<int> get userId => has((r) => r.userId, 'userId');
}
Loading