Skip to content

Commit

Permalink
Revert "store [nfc]: Change users from Map<int, User> to a new wr…
Browse files Browse the repository at this point in the history
…apper `MapNotifier<int, User>`"

This reverts commit f074645.
  • Loading branch information
gnprice committed May 14, 2024
1 parent bfb2d1d commit 46f3723
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 41 deletions.
14 changes: 1 addition & 13 deletions lib/model/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -341,11 +341,6 @@ class MentionAutocompleteView extends ChangeNotifier {
_sortedUsers = sortByRelevance(users: users, narrow: narrow);
}

bool _areUsersModified = false;
void _usersModified() {
_areUsersModified = true;
}

Future<List<MentionAutocompleteResult>?> _computeResults(MentionAutocompleteQuery query) async {
final List<MentionAutocompleteResult> results = [];

Expand All @@ -354,7 +349,6 @@ class MentionAutocompleteView extends ChangeNotifier {
}

final iterator = _sortedUsers!.iterator;
store.users.addListener(_usersModified);
bool isDone = false;
while (!isDone) {
// CPU perf: End this task; enqueue a new one for resuming this work
Expand All @@ -365,12 +359,7 @@ class MentionAutocompleteView extends ChangeNotifier {
}

for (int i = 0; i < 1000; i++) {
if (_areUsersModified) {
_areUsersModified = false;
_sortedUsers = null;
throw ConcurrentModificationError();
}
if (!iterator.moveNext()) {
if (!iterator.moveNext()) { // Can throw ConcurrentModificationError
isDone = true;
break;
}
Expand All @@ -381,7 +370,6 @@ class MentionAutocompleteView extends ChangeNotifier {
}
}
}
store.users.removeListener(_usersModified);
return results;
}
}
Expand Down
25 changes: 2 additions & 23 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -172,26 +172,6 @@ abstract class GlobalStore extends ChangeNotifier {
String toString() => '${objectRuntimeType(this, 'GlobalStore')}#${shortHash(this)}';
}

/// A wrapper class for `Map<K, V>`.
///
/// This can be listened to for any modifications done to `Map<K, V>`.
class MapNotifier<K, V> extends ValueNotifier<Map<K, V>> {
MapNotifier(super._value);

Iterable<V> get values => value.values;

V? operator [](Object? key) => value[key];

void operator []=(K key, V value) {
this.value[key] = value;
notifyListeners();
}

V? remove(Object? key) {
return value.remove(key);
}
}

/// Store for the user's data for a given Zulip account.
///
/// This should always have a consistent snapshot of the state on the server,
Expand Down Expand Up @@ -256,15 +236,14 @@ class PerAccountStore extends ChangeNotifier with StreamStore {
required this.accountId,
required this.selfUserId,
required this.userSettings,
required Map<int, User> users,
required this.users,
required streams,
required this.unreads,
required this.recentDmConversationsView,
}) : assert(selfUserId == globalStore.getAccount(accountId)!.userId),
assert(realmUrl == globalStore.getAccount(accountId)!.realmUrl),
assert(realmUrl == connection.realmUrl),
_globalStore = globalStore,
users = MapNotifier(users),
_streams = streams;

////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -307,7 +286,7 @@ class PerAccountStore extends ChangeNotifier with StreamStore {
////////////////////////////////
// Users and data about them.

final MapNotifier<int, User> users;
final Map<int, User> users;

final RecentSenders recentSenders = RecentSenders();

Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class _ComposeAutocompleteState extends State<ComposeAutocomplete> with PerAccou
case UserMentionAutocompleteResult(:var userId):
// TODO(i18n) language-appropriate space character; check active keyboard?
// (maybe handle centrally in `widget.controller`)
replacementString = '${mention(store.users[userId]!, silent: intent.query.silent, users: store.users.value)} ';
replacementString = '${mention(store.users[userId]!, silent: intent.query.silent, users: store.users)} ';
}

widget.controller.value = intent.textEditingValue.replaced(
Expand Down
6 changes: 3 additions & 3 deletions test/model/compose_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -314,17 +314,17 @@ hello
test('`users` passed; has two users with same fullName', () {
final store = eg.store();
store.addUsers([user, eg.user(userId: 5), eg.user(userId: 234, fullName: user.fullName)]);
check(mention(user, silent: true, users: store.users.value)).equals('@_**Full Name|123**');
check(mention(user, silent: true, users: store.users)).equals('@_**Full Name|123**');
});
test('`users` passed; has two same-name users but one of them is deactivated', () {
final store = eg.store();
store.addUsers([user, eg.user(userId: 5), eg.user(userId: 234, fullName: user.fullName, isActive: false)]);
check(mention(user, silent: true, users: store.users.value)).equals('@_**Full Name|123**');
check(mention(user, silent: true, users: store.users)).equals('@_**Full Name|123**');
});
test('`users` passed; user has unique fullName', () {
final store = eg.store();
store.addUsers([user, eg.user(userId: 234, fullName: 'Another Name')]);
check(mention(user, silent: true, users: store.users.value)).equals('@_**Full Name**');
check(mention(user, silent: true, users: store.users)).equals('@_**Full Name**');
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/widgets/autocomplete_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ void main() {
await tester.tap(find.text('User Three'));
await tester.pump();
check(tester.widget<TextField>(composeInputFinder).controller!.text)
.contains(mention(user3, users: store.users.value));
.contains(mention(user3, users: store.users));
checkUserShown(user1, store, expected: false);
checkUserShown(user2, store, expected: false);
checkUserShown(user3, store, expected: false);
Expand Down

0 comments on commit 46f3723

Please sign in to comment.