Skip to content

Commit

Permalink
user [nfc]: Make the actual users Map private
Browse files Browse the repository at this point in the history
This gives somewhat better encapsulation -- other code isn't expected
to add or remove items from this map.
  • Loading branch information
gnprice committed Feb 25, 2025
1 parent dadf9de commit 30c64a0
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 27 deletions.
5 changes: 4 additions & 1 deletion lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,10 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, UserStore, Channel
int get selfUserId => _users.selfUserId;

@override
Map<int, User> get users => _users.users;
User? getUser(int userId) => _users.getUser(userId);

@override
Iterable<User> get allUsers => _users.allUsers;

final UserStoreImpl _users;

Expand Down
49 changes: 24 additions & 25 deletions lib/model/user.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ mixin UserStore {
/// For the corresponding [User] object, see [selfUser].
int get selfUserId;

/// All known users in the realm, by [User.userId].
/// The user with the given ID, if that user is known.
///
/// There may be other users not found in this map, for multiple reasons:
/// There may be other users that are perfectly real but are
/// not known to the app, for multiple reasons:
///
/// * The self-user may not have permission to see all the users in the
/// realm, for example because the self-user is a guest.
Expand All @@ -27,33 +28,26 @@ mixin UserStore {
/// Those may therefore refer to users for which we have yet to see the
/// [RealmUserAddEvent], or have already handled a [RealmUserRemoveEvent].
///
/// Code that looks up a user in this map should therefore always handle
/// Code that looks up a user here should therefore always handle
/// the possibility that the user is not found (except
/// where there is a specific reason to know the user should be found).
/// Consider using [userDisplayName].
Map<int, User> get users;

/// The [User] object for the "self-user",
/// i.e. the account the person using this app is logged into.
///
/// When only the user ID is needed, see [selfUserId].
User get selfUser => getUser(selfUserId)!;

/// The user with the given ID, if that user is known.
///
/// There may be perfectly real users that are not known,
/// so callers must handle that possibility.
/// For details, see [users].
User? getUser(int userId) => users[userId];
User? getUser(int userId);

/// All known users in the realm.
///
/// This may have a large number of elements, like tens of thousands.
/// Consider [getUser] or other alternatives to iterating through this.
///
/// There may be perfectly real users which are not known
/// and so are not found here. For details, see [users].
Iterable<User> get allUsers => users.values;
/// and so are not found here. For details, see [getUser].
Iterable<User> get allUsers;

/// The [User] object for the "self-user",
/// i.e. the account the person using this app is logged into.
///
/// When only the user ID is needed, see [selfUserId].
User get selfUser => getUser(selfUserId)!;

/// The name to show the given user as in the UI, even for unknown users.
///
Expand All @@ -69,7 +63,7 @@ mixin UserStore {

/// The name to show for the given message's sender in the UI.
///
/// If the user is known (see [users]), this is their current [User.fullName].
/// If the user is known (see [getUser]), this is their current [User.fullName].
/// If unknown, this uses the fallback value conveniently provided on the
/// [Message] object itself, namely [Message.senderFullName].
///
Expand All @@ -90,7 +84,7 @@ class UserStoreImpl with UserStore {
UserStoreImpl({
required this.selfUserId,
required InitialSnapshot initialSnapshot,
}) : users = Map.fromEntries(
}) : _users = Map.fromEntries(
initialSnapshot.realmUsers
.followedBy(initialSnapshot.realmNonActiveUsers)
.followedBy(initialSnapshot.crossRealmBots)
Expand All @@ -99,19 +93,24 @@ class UserStoreImpl with UserStore {
@override
final int selfUserId;

final Map<int, User> _users;

@override
User? getUser(int userId) => _users[userId];

@override
final Map<int, User> users;
Iterable<User> get allUsers => _users.values;

void handleRealmUserEvent(RealmUserEvent event) {
switch (event) {
case RealmUserAddEvent():
users[event.person.userId] = event.person;
_users[event.person.userId] = event.person;

case RealmUserRemoveEvent():
users.remove(event.userId);
_users.remove(event.userId);

case RealmUserUpdateEvent():
final user = users[event.userId];
final user = _users[event.userId];
if (user == null) {
return; // TODO log
}
Expand Down
1 change: 0 additions & 1 deletion test/model/store_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ extension PerAccountStoreChecks on Subject<PerAccountStore> {
Subject<Account> get account => has((x) => x.account, 'account');
Subject<int> get selfUserId => has((x) => x.selfUserId, 'selfUserId');
Subject<UserSettings?> get userSettings => has((x) => x.userSettings, 'userSettings');
Subject<Map<int, User>> get users => has((x) => x.users, 'users');
Subject<Map<int, ZulipStream>> get streams => has((x) => x.streams, 'streams');
Subject<Map<String, ZulipStream>> get streamsByName => has((x) => x.streamsByName, 'streamsByName');
Subject<Map<int, Subscription>> get subscriptions => has((x) => x.subscriptions, 'subscriptions');
Expand Down

0 comments on commit 30c64a0

Please sign in to comment.