Skip to content

Move more data to PerAccountStoreBase #1467

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 14 commits into from
Apr 8, 2025
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
21 changes: 9 additions & 12 deletions lib/model/emoji.dart
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,12 @@ mixin EmojiStore {
/// Generally the only code that should need this class is [PerAccountStore]
/// itself. Other code accesses this functionality through [PerAccountStore],
/// or through the mixin [EmojiStore] which describes its interface.
class EmojiStoreImpl with EmojiStore {
class EmojiStoreImpl extends PerAccountStoreBase with EmojiStore {
EmojiStoreImpl({
required this.realmUrl,
required super.core,
required this.allRealmEmoji,
}) : _serverEmojiData = null; // TODO(#974) maybe start from a hard-coded baseline

/// The same as [PerAccountStore.realmUrl].
final Uri realmUrl;

/// The realm's custom emoji, indexed by their [RealmEmojiItem.emojiCode],
/// including deactivated emoji not available for new uses.
///
Expand Down Expand Up @@ -195,19 +192,19 @@ class EmojiStoreImpl with EmojiStore {
required String? stillUrl,
required String emojiName,
}) {
final source = Uri.tryParse(sourceUrl);
if (source == null) return TextEmojiDisplay(emojiName: emojiName);
final resolvedUrl = this.tryResolveUrl(sourceUrl);
if (resolvedUrl == null) return TextEmojiDisplay(emojiName: emojiName);

Uri? still;
Uri? resolvedStillUrl;
if (stillUrl != null) {
still = Uri.tryParse(stillUrl);
if (still == null) return TextEmojiDisplay(emojiName: emojiName);
resolvedStillUrl = this.tryResolveUrl(stillUrl);
if (resolvedStillUrl == null) return TextEmojiDisplay(emojiName: emojiName);
}

return ImageEmojiDisplay(
emojiName: emojiName,
resolvedUrl: realmUrl.resolveUri(source),
resolvedStillUrl: still == null ? null : realmUrl.resolveUri(still),
resolvedUrl: resolvedUrl,
resolvedStillUrl: resolvedStillUrl,
);
}

Expand Down
13 changes: 6 additions & 7 deletions lib/model/recent_dm_conversations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,19 @@ import '../api/model/initial_snapshot.dart';
import '../api/model/model.dart';
import '../api/model/events.dart';
import 'narrow.dart';
import 'store.dart';

/// A view-model for the recent-DM-conversations UI.
///
/// This maintains the list of recent DM conversations,
/// plus additional data in order to efficiently maintain the list.
class RecentDmConversationsView extends ChangeNotifier {
class RecentDmConversationsView extends PerAccountStoreBase with ChangeNotifier {
factory RecentDmConversationsView({
required CorePerAccountStore core,
required List<RecentDmConversation> initial,
required int selfUserId,
}) {
final entries = initial.map((conversation) => MapEntry(
DmNarrow.ofRecentDmConversation(conversation, selfUserId: selfUserId),
DmNarrow.ofRecentDmConversation(conversation, selfUserId: core.selfUserId),
conversation.maxMessageId,
)).toList()..sort((a, b) => -a.value.compareTo(b.value));

Expand All @@ -33,18 +34,18 @@ class RecentDmConversationsView extends ChangeNotifier {
}

return RecentDmConversationsView._(
core: core,
map: Map.fromEntries(entries),
sorted: QueueList.from(entries.map((e) => e.key)),
latestMessagesByRecipient: latestMessagesByRecipient,
selfUserId: selfUserId,
);
}

RecentDmConversationsView._({
required super.core,
required this.map,
required this.sorted,
required this.latestMessagesByRecipient,
required this.selfUserId,
});

/// The latest message ID in each conversation.
Expand All @@ -62,8 +63,6 @@ class RecentDmConversationsView extends ChangeNotifier {
/// it might have been sent by anyone in its conversation.)
final Map<int, int> latestMessagesByRecipient;

final int selfUserId;

/// Insert the key at the proper place in [sorted].
///
/// Optimized, taking O(1) time, for the case where that place is the start,
Expand Down
138 changes: 83 additions & 55 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,31 @@ abstract class GlobalStore extends ChangeNotifier {
class AccountNotFoundException implements Exception {}

/// A bundle of items that are useful to [PerAccountStore] and its substores.
///
/// Each instance of this class is constructed as part of constructing a
/// [PerAccountStore] instance,
/// and is shared by that [PerAccountStore] and its substores.
/// Calling [PerAccountStore.dispose] also disposes the [CorePerAccountStore]
/// (for example, it calls [ApiConnection.dispose] on [connection]).
class CorePerAccountStore {
CorePerAccountStore({required this.connection});
CorePerAccountStore._({
required GlobalStore globalStore,
required this.connection,
required this.accountId,
required this.selfUserId,
}) : _globalStore = globalStore,
assert(connection.realmUrl == globalStore.getAccount(accountId)!.realmUrl);

final GlobalStore _globalStore;
final ApiConnection connection; // TODO(#135): update zulipFeatureLevel with events
final int accountId;

// This isn't strictly needed as a field; it could be a getter
// that uses `_globalStore.getAccount(accountId)`.
// But we denormalize it here to save a hash-table lookup every time
// the self-user ID is needed, which can be often.
// It never changes on the account; see [GlobalStore.updateAccount].
final int selfUserId;
}

/// A base class for [PerAccountStore] and its substores,
Expand All @@ -342,7 +363,54 @@ abstract class PerAccountStoreBase {

final CorePerAccountStore _core;

////////////////////////////////
// Where data comes from in the first place.

GlobalStore get _globalStore => _core._globalStore;

ApiConnection get connection => _core.connection;

////////////////////////////////
// Data attached to the realm or the server.

/// Always equal to `account.realmUrl` and `connection.realmUrl`.
Uri get realmUrl => connection.realmUrl;

/// Resolve [reference] as a URL relative to [realmUrl].
///
/// This returns null if [reference] fails to parse as a URL.
Uri? tryResolveUrl(String reference) => _tryResolveUrl(realmUrl, reference);

////////////////////////////////
// Data attached to the self-account on the realm.

int get accountId => _core.accountId;

/// The [Account] this store belongs to.
///
/// Will throw if the account has been removed from the global store,
/// which is possible only if [PerAccountStore.dispose] has been called
/// on this store.
Account get account => _globalStore.getAccount(accountId)!;

/// The user ID of the "self-user",
/// i.e. the account the person using this app is logged into.
///
/// This always equals the [Account.userId] on [account].
///
/// For the corresponding [User] object, see [UserStore.selfUser].
int get selfUserId => _core.selfUserId;
}

const _tryResolveUrl = tryResolveUrl;

/// Like [Uri.resolve], but on failure return null instead of throwing.
Uri? tryResolveUrl(Uri baseUrl, String reference) {
try {
return baseUrl.resolve(reference);
} on FormatException {
return null;
}
}

/// Store for the user's data for a given Zulip account.
Expand Down Expand Up @@ -385,14 +453,16 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
throw Exception("bad initial snapshot: missing queueId");
}

final realmUrl = account.realmUrl;
final core = CorePerAccountStore(connection: connection);
final core = CorePerAccountStore._(
globalStore: globalStore,
connection: connection,
accountId: accountId,
selfUserId: account.userId,
);
final channels = ChannelStoreImpl(initialSnapshot: initialSnapshot);
return PerAccountStore._(
globalStore: globalStore,
core: core,
queueId: queueId,
realmUrl: realmUrl,
realmWildcardMentionPolicy: initialSnapshot.realmWildcardMentionPolicy,
realmMandatoryTopics: initialSnapshot.realmMandatoryTopics,
realmWaitingPeriodThreshold: initialSnapshot.realmWaitingPeriodThreshold,
Expand All @@ -402,41 +472,35 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
customProfileFields: _sortCustomProfileFields(initialSnapshot.customProfileFields),
emailAddressVisibility: initialSnapshot.emailAddressVisibility,
emoji: EmojiStoreImpl(
realmUrl: realmUrl, allRealmEmoji: initialSnapshot.realmEmoji),
accountId: accountId,
core: core, allRealmEmoji: initialSnapshot.realmEmoji),
userSettings: initialSnapshot.userSettings,
typingNotifier: TypingNotifier(
connection: connection,
core: core,
typingStoppedWaitPeriod: Duration(
milliseconds: initialSnapshot.serverTypingStoppedWaitPeriodMilliseconds),
typingStartedWaitPeriod: Duration(
milliseconds: initialSnapshot.serverTypingStartedWaitPeriodMilliseconds),
),
users: UserStoreImpl(
selfUserId: account.userId,
initialSnapshot: initialSnapshot),
typingStatus: TypingStatus(
selfUserId: account.userId,
users: UserStoreImpl(core: core, initialSnapshot: initialSnapshot),
typingStatus: TypingStatus(core: core,
typingStartedExpiryPeriod: Duration(milliseconds: initialSnapshot.serverTypingStartedExpiryPeriodMilliseconds),
),
channels: channels,
messages: MessageStoreImpl(core: core),
unreads: Unreads(
initial: initialSnapshot.unreadMsgs,
selfUserId: account.userId,
core: core,
channelStore: channels,
),
recentDmConversationsView: RecentDmConversationsView(
initial: initialSnapshot.recentPrivateConversations, selfUserId: account.userId),
recentDmConversationsView: RecentDmConversationsView(core: core,
initial: initialSnapshot.recentPrivateConversations),
recentSenders: RecentSenders(),
);
}

PerAccountStore._({
required GlobalStore globalStore,
required super.core,
required this.queueId,
required this.realmUrl,
required this.realmWildcardMentionPolicy,
required this.realmMandatoryTopics,
required this.realmWaitingPeriodThreshold,
Expand All @@ -446,7 +510,6 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
required this.customProfileFields,
required this.emailAddressVisibility,
required EmojiStoreImpl emoji,
required this.accountId,
required this.userSettings,
required this.typingNotifier,
required UserStoreImpl users,
Expand All @@ -456,11 +519,7 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
required this.unreads,
required this.recentDmConversationsView,
required this.recentSenders,
}) : assert(realmUrl == globalStore.getAccount(accountId)!.realmUrl),
assert(realmUrl == core.connection.realmUrl),
assert(emoji.realmUrl == realmUrl),
_globalStore = globalStore,
_realmEmptyTopicDisplayName = realmEmptyTopicDisplayName,
}) : _realmEmptyTopicDisplayName = realmEmptyTopicDisplayName,
_emoji = emoji,
_users = users,
_channels = channels,
Expand All @@ -472,8 +531,6 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
////////////////////////////////
// Where data comes from in the first place.

final GlobalStore _globalStore;

final String queueId;
UpdateMachine? get updateMachine => _updateMachine;
UpdateMachine? _updateMachine;
Expand All @@ -495,14 +552,6 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
////////////////////////////////
// Data attached to the realm or the server.

/// Always equal to `account.realmUrl` and `connection.realmUrl`.
final Uri realmUrl;

/// Resolve [reference] as a URL relative to [realmUrl].
///
/// This returns null if [reference] fails to parse as a URL.
Uri? tryResolveUrl(String reference) => _tryResolveUrl(realmUrl, reference);

/// Always equal to `connection.zulipFeatureLevel`
/// and `account.zulipFeatureLevel`.
int get zulipFeatureLevel => connection.zulipFeatureLevel!;
Expand Down Expand Up @@ -560,23 +609,13 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
////////////////////////////////
// Data attached to the self-account on the realm.

final int accountId;

/// The [Account] this store belongs to.
///
/// Will throw if called after [dispose] has been called.
Account get account => _globalStore.getAccount(accountId)!;

final UserSettings? userSettings; // TODO(server-5)

final TypingNotifier typingNotifier;

////////////////////////////////
// Users and data about them.

@override
int get selfUserId => _users.selfUserId;

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

Expand Down Expand Up @@ -882,17 +921,6 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
String toString() => '${objectRuntimeType(this, 'PerAccountStore')}#${shortHash(this)}';
}

const _tryResolveUrl = tryResolveUrl;

/// Like [Uri.resolve], but on failure return null instead of throwing.
Uri? tryResolveUrl(Uri baseUrl, String reference) {
try {
return baseUrl.resolve(reference);
} on FormatException {
return null;
}
}

/// A [GlobalStoreBackend] that uses a live, persistent local database.
///
/// Used as part of a [LiveGlobalStore].
Expand Down
12 changes: 5 additions & 7 deletions lib/model/typing_status.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,21 @@ import 'dart:async';

import 'package:flutter/foundation.dart';

import '../api/core.dart';
import '../api/model/events.dart';
import '../api/route/typing.dart';
import 'binding.dart';
import 'narrow.dart';
import 'store.dart';

/// The model for tracking the typing status organized by narrows.
///
/// Listeners are notified when a typist is added or removed from any narrow.
class TypingStatus extends ChangeNotifier {
class TypingStatus extends PerAccountStoreBase with ChangeNotifier {
Copy link
Collaborator

Choose a reason for hiding this comment

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

store [nfc]: Get selfUserId on substores from PerAccountStoreBase

This might be splitting hairs, but before this commit, we haven't treated TypingStatus as a "substore" in the same sense as the others: MessageStore and its MessageStoreImpl, EmojiStore and its EmojiStoreImpl, etc.; is it OK to start treating TypingStatus as a substore in just the way this commit does?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's not structured in the same way as those two and UserStore and ChannelStore. Instead it more resembles Unreads, RecentSenders, or RecentDmConversationsView (two of which also appear in this commit).

The actual MessageStoreImpl or EmojiStoreImpl etc., though, is very similar to TypingStatus or Unreads etc.:

  • It's associated with a particular PerAccountStore, and is effectively part of the implementation of PerAccountStore.
  • It in turn needs access to some of the core pieces of PerAccountStore, like the ApiConnection or self-user ID etc.

I think that first point makes a good definition for the term "substore".

TypingStatus({
required this.selfUserId,
required super.core,
required this.typingStartedExpiryPeriod,
});

final int selfUserId;
final Duration typingStartedExpiryPeriod;

Iterable<SendableNarrow> get debugActiveNarrows => _timerMapsByNarrow.keys;
Expand Down Expand Up @@ -93,14 +92,13 @@ class TypingStatus extends ChangeNotifier {
/// See also:
/// * https://github.com/zulip/zulip/blob/52a9846cdf4abfbe937a94559690d508e95f4065/web/shared/src/typing_status.ts
/// * https://zulip.readthedocs.io/en/latest/subsystems/typing-indicators.html
class TypingNotifier {
class TypingNotifier extends PerAccountStoreBase {
TypingNotifier({
required this.connection,
required super.core,
required this.typingStoppedWaitPeriod,
required this.typingStartedWaitPeriod,
});

final ApiConnection connection;
final Duration typingStoppedWaitPeriod;
final Duration typingStartedWaitPeriod;

Expand Down
Loading