diff --git a/lib/model/compose.dart b/lib/model/compose.dart index ec8fdcb3dd..b59a3efcc7 100644 --- a/lib/model/compose.dart +++ b/lib/model/compose.dart @@ -142,7 +142,7 @@ String quoteAndReplyPlaceholder(PerAccountStore store, { final sender = store.users[message.senderId]; assert(sender != null); final url = narrowLink(store, - SendableNarrow.ofMessage(message, selfUserId: store.account.userId), + SendableNarrow.ofMessage(message, selfUserId: store.selfUserId), nearMessageId: message.id); // See note in [quoteAndReply] about asking `mention` to omit the | part. return '${mention(sender!, silent: true)} ${inlineLink('said', url)}: ' // TODO(i18n) ? @@ -164,7 +164,7 @@ String quoteAndReply(PerAccountStore store, { final sender = store.users[message.senderId]; assert(sender != null); final url = narrowLink(store, - SendableNarrow.ofMessage(message, selfUserId: store.account.userId), + SendableNarrow.ofMessage(message, selfUserId: store.selfUserId), nearMessageId: message.id); // Could ask `mention` to omit the | part unless the mention is ambiguous… // but that would mean a linear scan through all users, and the extra noise diff --git a/lib/model/database.dart b/lib/model/database.dart index fb13b3e6e2..bd8f243d7f 100644 --- a/lib/model/database.dart +++ b/lib/model/database.dart @@ -7,10 +7,25 @@ import 'package:path_provider/path_provider.dart'; part 'database.g.dart'; +/// The table of [Account] records in the app's database. class Accounts extends Table { + /// The ID of this account in the app's local database. + /// + /// This uniquely identifies the account within this install of the app, + /// and never changes for a given account. It has no meaning to the server, + /// though, or anywhere else outside this install of the app. Column get id => integer().autoIncrement()(); + /// The URL of the Zulip realm this account is on. + /// + /// This corresponds to [GetServerSettingsResult.realmUrl]. + /// It never changes for a given account. Column get realmUrl => text().map(const UriConverter())(); + + /// The Zulip user ID of this account. + /// + /// This is the identifier the server uses for the account. + /// It never changes for a given account. Column get userId => integer()(); Column get email => text()(); diff --git a/lib/model/database.g.dart b/lib/model/database.g.dart index d493eafae6..52d9c00c0c 100644 --- a/lib/model/database.g.dart +++ b/lib/model/database.g.dart @@ -182,8 +182,23 @@ class $AccountsTable extends Accounts with TableInfo<$AccountsTable, Account> { } class Account extends DataClass implements Insertable { + /// The ID of this account in the app's local database. + /// + /// This uniquely identifies the account within this install of the app, + /// and never changes for a given account. It has no meaning to the server, + /// though, or anywhere else outside this install of the app. final int id; + + /// The URL of the Zulip realm this account is on. + /// + /// This corresponds to [GetServerSettingsResult.realmUrl]. + /// It never changes for a given account. final Uri realmUrl; + + /// The Zulip user ID of this account. + /// + /// This is the identifier the server uses for the account. + /// It never changes for a given account. final int userId; final String email; final String apiKey; diff --git a/lib/model/internal_link.dart b/lib/model/internal_link.dart index 15fc0cbeb7..91cb1c6df1 100644 --- a/lib/model/internal_link.dart +++ b/lib/model/internal_link.dart @@ -95,23 +95,13 @@ Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) { fragment.write('/near/$nearMessageId'); } - return store.account.realmUrl.replace(fragment: fragment.toString()); -} - -/// Create a new `Uri` object in relation to a given realmUrl. -/// -/// Returns `null` if `urlString` could not be parsed as a `Uri`. -Uri? tryResolveOnRealmUrl(String urlString, Uri realmUrl) { - try { - return realmUrl.resolve(urlString); - } on FormatException { - return null; - } + return store.realmUrl.replace(fragment: fragment.toString()); } /// A [Narrow] from a given URL, on `store`'s realm. /// -/// `url` must already be passed through [tryResolveOnRealmUrl]. +/// `url` must already be a result from [PerAccountStore.tryResolveUrl] +/// on `store`. /// /// Returns `null` if any of the operator/operand pairs are invalid. /// @@ -124,7 +114,7 @@ Uri? tryResolveOnRealmUrl(String urlString, Uri realmUrl) { /// #narrow/stream/1-announce/stream/1-announce (duplicated operator) // TODO(#252): handle all valid narrow links, returning a search narrow Narrow? parseInternalLink(Uri url, PerAccountStore store) { - if (!_isInternalLink(url, store.account.realmUrl)) return null; + if (!_isInternalLink(url, store.realmUrl)) return null; final (category, segments) = _getCategoryAndSegmentsFromFragment(url.fragment); switch (category) { @@ -197,7 +187,7 @@ Narrow? _interpretNarrowSegments(List segments, PerAccountStore store) { if (dmElement != null) { if (streamElement != null || topicElement != null) return null; - return DmNarrow.withUsers(dmElement.operand, selfUserId: store.account.userId); + return DmNarrow.withUsers(dmElement.operand, selfUserId: store.selfUserId); } else if (streamElement != null) { final streamId = streamElement.operand; if (topicElement != null) { diff --git a/lib/model/store.dart b/lib/model/store.dart index aafc30aaa7..1aed416810 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -111,9 +111,7 @@ abstract class GlobalStore extends ChangeNotifier { } // It's up to us. Start loading. - final account = getAccount(accountId); - assert(account != null, 'Account not found on global store'); - future = loadPerAccount(account!); + future = loadPerAccount(accountId); _perAccountStoresLoading[accountId] = future; store = await future; _setPerAccount(accountId, store); @@ -121,12 +119,11 @@ abstract class GlobalStore extends ChangeNotifier { return store; } - Future _reloadPerAccount(Account account) async { - assert(identical(_accounts[account.id], account)); - assert(_perAccountStores.containsKey(account.id)); - assert(!_perAccountStoresLoading.containsKey(account.id)); - final store = await loadPerAccount(account); - _setPerAccount(account.id, store); + Future _reloadPerAccount(int accountId) async { + assert(_perAccountStores.containsKey(accountId)); + assert(!_perAccountStoresLoading.containsKey(accountId)); + final store = await loadPerAccount(accountId); + _setPerAccount(accountId, store); } void _setPerAccount(int accountId, PerAccountStore store) { @@ -141,7 +138,7 @@ abstract class GlobalStore extends ChangeNotifier { /// This method should be called only by the implementation of [perAccount]. /// Other callers interested in per-account data should use [perAccount] /// and/or [perAccountSync]. - Future loadPerAccount(Account account); + Future loadPerAccount(int accountId); // Just the Iterables, not the actual Map, to avoid clients mutating the map. // Mutations should go through the setters/mutators below. @@ -191,33 +188,36 @@ class PerAccountStore extends ChangeNotifier with StreamStore { /// but it may have already been used for other requests. factory PerAccountStore.fromInitialSnapshot({ required GlobalStore globalStore, - required Account account, + required int accountId, ApiConnection? connection, required InitialSnapshot initialSnapshot, }) { + final account = globalStore.getAccount(accountId)!; connection ??= globalStore.apiConnectionFromAccount(account); final streams = StreamStoreImpl(initialSnapshot: initialSnapshot); return PerAccountStore._( globalStore: globalStore, - account: account, connection: connection, + realmUrl: account.realmUrl, zulipVersion: initialSnapshot.zulipVersion, maxFileUploadSizeMib: initialSnapshot.maxFileUploadSizeMib, realmDefaultExternalAccounts: initialSnapshot.realmDefaultExternalAccounts, realmEmoji: initialSnapshot.realmEmoji, customProfileFields: _sortCustomProfileFields(initialSnapshot.customProfileFields), + accountId: accountId, + selfUserId: account.userId, userSettings: initialSnapshot.userSettings, - unreads: Unreads( - initial: initialSnapshot.unreadMsgs, - selfUserId: account.userId, - streamStore: streams, - ), users: Map.fromEntries( initialSnapshot.realmUsers .followedBy(initialSnapshot.realmNonActiveUsers) .followedBy(initialSnapshot.crossRealmBots) .map((user) => MapEntry(user.userId, user))), streams: streams, + unreads: Unreads( + initial: initialSnapshot.unreadMsgs, + selfUserId: account.userId, + streamStore: streams, + ), recentDmConversationsView: RecentDmConversationsView( initial: initialSnapshot.recentPrivateConversations, selfUserId: account.userId), ); @@ -225,42 +225,69 @@ class PerAccountStore extends ChangeNotifier with StreamStore { PerAccountStore._({ required GlobalStore globalStore, - required this.account, required this.connection, + required this.realmUrl, required this.zulipVersion, required this.maxFileUploadSizeMib, required this.realmDefaultExternalAccounts, required this.realmEmoji, required this.customProfileFields, + required this.accountId, + required this.selfUserId, required this.userSettings, - required this.unreads, required this.users, required streams, + required this.unreads, required this.recentDmConversationsView, - }) : _globalStore = globalStore, + }) : assert(selfUserId == globalStore.getAccount(accountId)!.userId), + assert(realmUrl == globalStore.getAccount(accountId)!.realmUrl), + assert(realmUrl == connection.realmUrl), + _globalStore = globalStore, _streams = streams; - final GlobalStore _globalStore; + //////////////////////////////////////////////////////////////// + // Data. - final Account account; - final ApiConnection connection; // TODO(#135): update zulipFeatureLevel with events + //////////////////////////////// + // Where data comes from in the first place. - // TODO(#135): Keep all this data updated by handling Zulip events from the server. + final GlobalStore _globalStore; + final ApiConnection connection; // TODO(#135): update zulipFeatureLevel with events + //////////////////////////////// // 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); + final String zulipVersion; // TODO get from account; update there on initial snapshot final int maxFileUploadSizeMib; // No event for this. final Map realmDefaultExternalAccounts; Map realmEmoji; List customProfileFields; + //////////////////////////////// // Data attached to the self-account on the realm. + + final int accountId; + Account get account => _globalStore.getAccount(accountId)!; + + /// Always equal to `account.userId`. + final int selfUserId; + final UserSettings? userSettings; // TODO(server-5) - final Unreads unreads; + //////////////////////////////// // Users and data about them. + final Map users; + //////////////////////////////// // Streams, topics, and stuff about them. @override @@ -278,7 +305,10 @@ class PerAccountStore extends ChangeNotifier with StreamStore { @visibleForTesting StreamStoreImpl get debugStreamStore => _streams; - // TODO lots more data. When adding, be sure to update handleEvent too. + //////////////////////////////// + // Messages, and summaries of messages. + + final Unreads unreads; final RecentDmConversationsView recentDmConversationsView; @@ -294,8 +324,14 @@ class PerAccountStore extends ChangeNotifier with StreamStore { assert(removed); } + //////////////////////////////// + // Other digests of data. + final AutocompleteViewManager autocompleteViewManager = AutocompleteViewManager(); + // End of data. + //////////////////////////////////////////////////////////////// + /// 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 @@ -464,6 +500,16 @@ class PerAccountStore extends ChangeNotifier with StreamStore { } const _apiSendMessage = sendMessage; // Bit ugly; for alternatives, see: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20PerAccountStore.20methods/near/1545809 +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 [GlobalStore] that uses a live server and live, persistent local database. /// @@ -517,8 +563,8 @@ class LiveGlobalStore extends GlobalStore { final AppDatabase _db; @override - Future loadPerAccount(Account account) async { - final updateMachine = await UpdateMachine.load(this, account); + Future loadPerAccount(int accountId) async { + final updateMachine = await UpdateMachine.load(this, accountId); return updateMachine.store; } @@ -554,7 +600,8 @@ class UpdateMachine { /// Load the user's data from the server, and start an event queue going. /// /// In the future this might load an old snapshot from local storage first. - static Future load(GlobalStore globalStore, Account account) async { + static Future load(GlobalStore globalStore, int accountId) async { + final account = globalStore.getAccount(accountId)!; // TODO test UpdateMachine.load, now that it uses [GlobalStore.apiConnection] final connection = globalStore.apiConnectionFromAccount(account); @@ -566,7 +613,7 @@ class UpdateMachine { final store = PerAccountStore.fromInitialSnapshot( globalStore: globalStore, - account: account, + accountId: accountId, connection: connection, initialSnapshot: initialSnapshot, ); @@ -624,7 +671,7 @@ class UpdateMachine { switch (e) { case ZulipApiException(code: 'BAD_EVENT_QUEUE_ID'): assert(debugLog('Lost event queue for $store. Replacing…')); - await store._globalStore._reloadPerAccount(store.account); + await store._globalStore._reloadPerAccount(store.accountId); dispose(); debugLog('… Event queue replaced.'); return; diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 7ca86d3779..ba03831fe2 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -26,12 +26,11 @@ void showMessageActionSheet({required BuildContext context, required Message mes // any message list, so that's fine. final isComposeBoxOffered = MessageListPage.composeBoxControllerOf(context) != null; - final selfUserId = store.account.userId; final hasThumbsUpReactionVote = message.reactions ?.aggregated.any((reactionWithVotes) => reactionWithVotes.reactionType == ReactionType.unicodeEmoji && reactionWithVotes.emojiCode == '1f44d' - && reactionWithVotes.userIds.contains(selfUserId)) + && reactionWithVotes.userIds.contains(store.selfUserId)) ?? false; showDraggableScrollableModalBottomSheet( diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index a153125e49..75c7fd90e2 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -265,7 +265,7 @@ class HomePage extends StatelessWidget { const SizedBox(height: 12), Text.rich(TextSpan( text: 'Connected to: ', - children: [bold(store.account.realmUrl.toString())])), + children: [bold(store.realmUrl.toString())])), Text.rich(TextSpan( text: 'Zulip server version: ', children: [bold(store.zulipVersion)])), diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index e2a45923eb..1f70556f86 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -9,7 +9,6 @@ import '../api/model/model.dart'; import '../model/binding.dart'; import '../model/content.dart'; import '../model/internal_link.dart'; -import '../model/store.dart'; import 'code_block.dart'; import 'dialog.dart'; import 'icons.dart'; @@ -261,10 +260,11 @@ class MessageImage extends StatelessWidget { final src = node.srcUrl; final store = PerAccountStoreWidget.of(context); - final resolvedSrc = resolveUrl(src, store.account); + final resolvedSrc = store.tryResolveUrl(src); + // TODO if src fails to parse, show an explicit "broken image" return GestureDetector( - onTap: () { + onTap: resolvedSrc == null ? null : () { // TODO(log) Navigator.of(context).push(getLightboxRoute( context: context, message: message, src: resolvedSrc)); }, @@ -281,7 +281,7 @@ class MessageImage extends StatelessWidget { child: SizedBox( height: 100, width: 150, - child: LightboxHero( + child: resolvedSrc == null ? null : LightboxHero( message: message, src: resolvedSrc, child: RealmContentNetworkImage( @@ -717,7 +717,7 @@ class MessageImageEmoji extends StatelessWidget { @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final resolvedSrc = resolveUrl(node.src, store.account); + final resolvedSrc = store.tryResolveUrl(node.src); const size = 20.0; @@ -730,12 +730,13 @@ class MessageImageEmoji extends StatelessWidget { // Web's css makes this seem like it should be -0.5, but that looks // too low. top: -1.5, - child: RealmContentNetworkImage( - resolvedSrc, - filterQuality: FilterQuality.medium, - width: size, - height: size, - )), + child: resolvedSrc == null ? const SizedBox.shrink() // TODO(log) + : RealmContentNetworkImage( + resolvedSrc, + filterQuality: FilterQuality.medium, + width: size, + height: size, + )), ]); } } @@ -786,7 +787,7 @@ void _launchUrl(BuildContext context, String urlString) async { } final store = PerAccountStoreWidget.of(context); - final url = tryResolveOnRealmUrl(urlString, store.account.realmUrl); + final url = store.tryResolveUrl(urlString); if (url == null) { // TODO(log) await showError(context, null); return; @@ -964,7 +965,7 @@ class AvatarImage extends StatelessWidget { final resolvedUrl = switch (user.avatarUrl) { null => null, // TODO(#255): handle computing gravatars - var avatarUrl => resolveUrl(avatarUrl, store.account), + var avatarUrl => store.tryResolveUrl(avatarUrl), }; return (resolvedUrl == null) ? const SizedBox.shrink() @@ -1000,14 +1001,6 @@ class AvatarShape extends StatelessWidget { // Small helpers. // -/// Resolve `url` to `account`'s realm, if relative -// This may dissolve when we start passing around URLs as [Uri] objects instead -// of strings. -Uri resolveUrl(String url, Account account) { - final realmUrl = account.realmUrl; - return realmUrl.resolve(url); // TODO handle if fails to parse -} - InlineSpan _errorUnimplemented(UnimplementedNode node) { // For now this shows error-styled HTML code even in release mode, // because release mode isn't yet about general users but developer demos, diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index 947cd226c1..ba7fc7a4a4 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -73,14 +73,13 @@ class ReactionChip extends StatelessWidget { final emojiset = store.userSettings?.emojiset ?? Emojiset.google; - final selfUserId = store.account.userId; - final selfVoted = userIds.contains(selfUserId); + final selfVoted = userIds.contains(store.selfUserId); final label = showName // TODO(i18n): List formatting, like you can do in JavaScript: // new Intl.ListFormat('ja').format(['Chris', 'Greg', 'Alya', 'Shu']) // // 'Chris、Greg、Alya、Shu' ? userIds.map((id) { - return id == selfUserId + return id == store.selfUserId ? 'You' : store.users[id]?.fullName ?? '(unknown user)'; // TODO(i18n) }).join(', ') @@ -320,7 +319,7 @@ class _ImageEmoji extends StatelessWidget { if (parsedSrc == null) { // TODO(log) return _textFallback; } - final resolved = store.account.realmUrl.resolveUri(parsedSrc); + final resolved = store.realmUrl.resolveUri(parsedSrc); // Unicode and text emoji get scaled; it would look weird if image emoji didn't. final size = _squareEmojiScalerClamped(context).scale(_squareEmojiSize); diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index a1e706946f..8aa5172efc 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -335,7 +335,7 @@ class _DmItem extends StatelessWidget { @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final selfUser = store.users[store.account.userId]!; + final selfUser = store.users[store.selfUserId]!; final title = switch (narrow.otherRecipientIds) { // TODO dedupe with [RecentDmConversationsItem] [] => selfUser.fullName, diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index a957b29cfc..3a5ca5fae5 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -745,7 +745,7 @@ class DmRecipientHeader extends StatelessWidget { final String title; if (message.allRecipientIds.length > 1) { title = zulipLocalizations.messageListGroupYouAndOthers(message.allRecipientIds - .where((id) => id != store.account.userId) + .where((id) => id != store.selfUserId) .map((id) => store.users[id]?.fullName ?? zulipLocalizations.unknownUserName) .sorted() .join(", ")); @@ -757,7 +757,7 @@ class DmRecipientHeader extends StatelessWidget { return GestureDetector( onTap: () => Navigator.push(context, MessageListPage.buildRoute(context: context, - narrow: DmNarrow.ofMessage(message, selfUserId: store.account.userId))), + narrow: DmNarrow.ofMessage(message, selfUserId: store.selfUserId))), child: ColoredBox( color: _kDmRecipientHeaderColor, child: Padding( diff --git a/lib/widgets/profile.dart b/lib/widgets/profile.dart index 055772017e..6a46e5779d 100644 --- a/lib/widgets/profile.dart +++ b/lib/widgets/profile.dart @@ -57,7 +57,7 @@ class ProfilePage extends StatelessWidget { FilledButton.icon( onPressed: () => Navigator.push(context, MessageListPage.buildRoute(context: context, - narrow: DmNarrow.withUser(userId, selfUserId: store.account.userId))), + narrow: DmNarrow.withUser(userId, selfUserId: store.selfUserId))), icon: const Icon(Icons.email), label: Text(zulipLocalizations.profileButtonSendDirectMessage)), ]; diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index dac1d638c5..aafdc8c42f 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -87,7 +87,7 @@ class RecentDmConversationsItem extends StatelessWidget { @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final selfUser = store.users[store.account.userId]!; + final selfUser = store.users[store.selfUserId]!; final String title; final Widget avatar; diff --git a/test/example_data.dart b/test/example_data.dart index ae3dfcac14..be976bce84 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -494,7 +494,7 @@ PerAccountStore store({Account? account, InitialSnapshot? initialSnapshot}) { final effectiveAccount = account ?? selfAccount; return PerAccountStore.fromInitialSnapshot( globalStore: globalStore(accounts: [effectiveAccount]), - account: effectiveAccount, + accountId: effectiveAccount.id, initialSnapshot: initialSnapshot ?? _initialSnapshot(), ); } diff --git a/test/model/compose_test.dart b/test/model/compose_test.dart index ad68a70d10..202a36050e 100644 --- a/test/model/compose_test.dart +++ b/test/model/compose_test.dart @@ -225,9 +225,9 @@ hello test('AllMessagesNarrow', () { final store = eg.store(); check(narrowLink(store, const AllMessagesNarrow())) - .equals(store.account.realmUrl.resolve('#narrow')); + .equals(store.realmUrl.resolve('#narrow')); check(narrowLink(store, const AllMessagesNarrow(), nearMessageId: 1)) - .equals(store.account.realmUrl.resolve('#narrow/near/1')); + .equals(store.realmUrl.resolve('#narrow/near/1')); }); test('StreamNarrow / TopicNarrow', () { @@ -244,7 +244,7 @@ hello ? StreamNarrow(streamId) : TopicNarrow(streamId, topic); check(narrowLink(store, narrow, nearMessageId: nearMessageId)) - .equals(store.account.realmUrl.resolve(expectedFragment)); + .equals(store.realmUrl.resolve(expectedFragment)); } checkNarrow(streamId: 1, name: 'announce', '#narrow/stream/1-announce'); @@ -275,10 +275,10 @@ hello final store = eg.store(); final narrow = DmNarrow(allRecipientIds: allRecipientIds, selfUserId: selfUserId); check(narrowLink(store, narrow, nearMessageId: nearMessageId)) - .equals(store.account.realmUrl.resolve(expectedFragment)); + .equals(store.realmUrl.resolve(expectedFragment)); store.connection.zulipFeatureLevel = 176; check(narrowLink(store, narrow, nearMessageId: nearMessageId)) - .equals(store.account.realmUrl.resolve(legacyExpectedFragment)); + .equals(store.realmUrl.resolve(legacyExpectedFragment)); } checkNarrow(allRecipientIds: [1], selfUserId: 1, diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 4b00a42510..572e176157 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -111,6 +111,13 @@ class ContentExample { const ImageEmojiNode( src: '/user_avatars/2/emoji/images/204.png', alt: ':flutter:')); + static final emojiCustomInvalidUrl = ContentExample.inline( + 'custom emoji with invalid URL', + null, // hypothetical, to test for a risk of crashing + '

:invalid:

', + const ImageEmojiNode( + src: '::not a URL::', alt: ':invalid:')); + static final emojiZulipExtra = ContentExample.inline( 'Zulip extra emoji', ":zulip:", @@ -268,6 +275,17 @@ class ContentExample { ]), ]); + static const imageInvalidUrl = ContentExample( + 'single image with invalid URL', + null, // hypothetical, to test for a risk of crashing + '
' + '' + '
', [ + ImageNodeList([ + ImageNode(srcUrl: '::not a URL::'), + ]), + ]); + static const imageCluster = ContentExample( 'multiple images', "https://chat.zulip.org/user_avatars/2/realm/icon.png?version=3\nhttps://chat.zulip.org/user_avatars/2/realm/icon.png?version=4", @@ -580,6 +598,7 @@ void main() { testParseExample(ContentExample.emojiUnicodeMultiCodepoint); testParseExample(ContentExample.emojiUnicodeLiteral); testParseExample(ContentExample.emojiCustom); + testParseExample(ContentExample.emojiCustomInvalidUrl); testParseExample(ContentExample.emojiZulipExtra); testParseExample(ContentExample.mathInline); @@ -751,6 +770,7 @@ void main() { testParseExample(ContentExample.mathBlockInQuote); testParseExample(ContentExample.imageSingle); + testParseExample(ContentExample.imageInvalidUrl); testParseExample(ContentExample.imageCluster); testParseExample(ContentExample.imageClusterThenContent); testParseExample(ContentExample.imageMultipleClusters); diff --git a/test/model/internal_link_test.dart b/test/model/internal_link_test.dart index c80c41ca76..e06c8590af 100644 --- a/test/model/internal_link_test.dart +++ b/test/model/internal_link_test.dart @@ -38,7 +38,7 @@ void main() { store ??= setupStore(realmUrl: realmUrl, streams: streams, users: users); for (final testCase in testCases) { final String urlString = testCase.$1; - final Uri url = tryResolveOnRealmUrl(urlString, realmUrl)!; + final Uri url = store.tryResolveUrl(urlString)!; final Narrow? expected = testCase.$2; test(urlString, () { check(parseInternalLink(url, store!)).equals(expected); @@ -134,7 +134,7 @@ void main() { final Uri realmUrl = testCase.$4; test('${expected ? 'accepts': 'rejects'} $description: $urlString', () { final store = setupStore(realmUrl: realmUrl, streams: streams); - final url = tryResolveOnRealmUrl(urlString, realmUrl)!; + final url = store.tryResolveUrl(urlString)!; final result = parseInternalLink(url, store); check(result != null).equals(expected); }); diff --git a/test/model/store_test.dart b/test/model/store_test.dart index cd8e13f03e..38eff362a8 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -28,12 +28,12 @@ void main() { final accounts = [account1, account2]; final globalStore = LoadingTestGlobalStore(accounts: accounts); List> completers(int accountId) => - globalStore.completers[accounts[accountId - 1]]!; + globalStore.completers[accountId]!; final future1 = globalStore.perAccount(1); final store1 = PerAccountStore.fromInitialSnapshot( globalStore: globalStore, - account: account1, + accountId: 1, initialSnapshot: eg.initialSnapshot(), ); completers(1).single.complete(store1); @@ -44,7 +44,7 @@ void main() { final future2 = globalStore.perAccount(2); final store2 = PerAccountStore.fromInitialSnapshot( globalStore: globalStore, - account: account2, + accountId: 2, initialSnapshot: eg.initialSnapshot(), ); completers(2).single.complete(store2); @@ -61,7 +61,7 @@ void main() { final accounts = [account1, account2]; final globalStore = LoadingTestGlobalStore(accounts: accounts); List> completers(int accountId) => - globalStore.completers[accounts[accountId - 1]]!; + globalStore.completers[accountId]!; final future1a = globalStore.perAccount(1); final future1b = globalStore.perAccount(1); @@ -71,12 +71,12 @@ void main() { final future2 = globalStore.perAccount(2); final store1 = PerAccountStore.fromInitialSnapshot( globalStore: globalStore, - account: account1, + accountId: 1, initialSnapshot: eg.initialSnapshot(), ); final store2 = PerAccountStore.fromInitialSnapshot( globalStore: globalStore, - account: account2, + accountId: 2, initialSnapshot: eg.initialSnapshot(), ); completers(1).single.complete(store1); @@ -94,14 +94,14 @@ void main() { final accounts = [account1, account2]; final globalStore = LoadingTestGlobalStore(accounts: accounts); List> completers(int accountId) => - globalStore.completers[accounts[accountId - 1]]!; + globalStore.completers[accountId]!; check(globalStore.perAccountSync(1)).isNull(); final future1 = globalStore.perAccount(1); check(globalStore.perAccountSync(1)).isNull(); final store1 = PerAccountStore.fromInitialSnapshot( globalStore: globalStore, - account: account1, + accountId: 1, initialSnapshot: eg.initialSnapshot(), ); completers(1).single.complete(store1); @@ -215,7 +215,7 @@ void main() { await prepareStore(); updateMachine.debugPauseLoop(); updateMachine.poll(); - check(globalStore.perAccountSync(store.account.id)).identicalTo(store); + check(globalStore.perAccountSync(store.accountId)).identicalTo(store); // Let the server expire the event queue. connection.prepare(httpStatus: 400, json: { @@ -228,7 +228,7 @@ void main() { await Future.delayed(Duration.zero); // The global store has a new store. - check(globalStore.perAccountSync(store.account.id)).not((it) => it.identicalTo(store)); + check(globalStore.perAccountSync(store.accountId)).not((it) => it.identicalTo(store)); updateFromGlobalStore(); // The new UpdateMachine updates the new store. @@ -388,12 +388,12 @@ void main() { class LoadingTestGlobalStore extends TestGlobalStore { LoadingTestGlobalStore({required super.accounts}); - Map>> completers = {}; + Map>> completers = {}; @override - Future loadPerAccount(Account account) { + Future loadPerAccount(int accountId) { final completer = Completer(); - (completers[account] ??= []).add(completer); + (completers[accountId] ??= []).add(completer); return completer.future; } } diff --git a/test/model/test_store.dart b/test/model/test_store.dart index e16a84d932..f170f01900 100644 --- a/test/model/test_store.dart +++ b/test/model/test_store.dart @@ -90,14 +90,14 @@ class TestGlobalStore extends GlobalStore { } @override - Future loadPerAccount(Account account) { - final initialSnapshot = _initialSnapshots[account.id]!; + Future loadPerAccount(int accountId) { + final initialSnapshot = _initialSnapshots[accountId]!; final store = PerAccountStore.fromInitialSnapshot( globalStore: this, - account: account, + accountId: accountId, initialSnapshot: initialSnapshot, ); - updateMachines[account.id] = UpdateMachine.fromInitialSnapshot( + updateMachines[accountId] = UpdateMachine.fromInitialSnapshot( store: store, initialSnapshot: initialSnapshot); return Future.value(store); } diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 0f130f9229..f408854442 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -8,6 +8,7 @@ import 'package:url_launcher/url_launcher.dart'; import 'package:zulip/api/core.dart'; import 'package:zulip/model/content.dart'; import 'package:zulip/model/narrow.dart'; +import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; @@ -16,6 +17,8 @@ import 'package:zulip/widgets/store.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; import '../model/content_test.dart'; +import '../model/test_store.dart'; +import '../stdlib_checks.dart'; import '../test_images.dart'; import '../test_navigation.dart'; import 'dialog_checks.dart'; @@ -55,6 +58,21 @@ void main() { }); } + /// Set [debugNetworkImageHttpClientProvider] to return a constant image. + /// + /// Returns the [FakeImageHttpClient] that handles the requests. + /// + /// The caller must set [debugNetworkImageHttpClientProvider] back to null + /// before the end of the test. + FakeImageHttpClient prepareBoringImageHttpClient() { + final httpClient = FakeImageHttpClient(); + debugNetworkImageHttpClientProvider = () => httpClient; + httpClient.request.response + ..statusCode = HttpStatus.ok + ..content = kSolidBlueAvatar; + return httpClient; + } + group('Heading', () { testWidgets('plain h6', (tester) async { await prepareContentBare(tester, @@ -73,6 +91,98 @@ void main() { testContentSmoke(ContentExample.quotation); + group('MessageImage, MessageImageList', () { + Future prepareContent(WidgetTester tester, String html) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + prepareBoringImageHttpClient(); + + await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp( + home: PerAccountStoreWidget(accountId: eg.selfAccount.id, + child: MessageContent( + message: eg.streamMessage(content: html), + content: parseContent(html)))))); + await tester.pump(); // global store + await tester.pump(); // per-account store + debugNetworkImageHttpClientProvider = null; + } + + testWidgets('single image', (tester) async { + const example = ContentExample.imageSingle; + await prepareContent(tester, example.html); + final expectedImages = (example.expectedNodes[0] as ImageNodeList).images; + final images = tester.widgetList( + find.byType(RealmContentNetworkImage)); + check(images.map((i) => i.src.toString()).toList()) + .deepEquals(expectedImages.map((n) => n.srcUrl)); + }); + + testWidgets('image with invalid src URL', (tester) async { + const example = ContentExample.imageInvalidUrl; + await prepareContent(tester, example.html); + // The image indeed has an invalid URL. + final expectedImages = (example.expectedNodes[0] as ImageNodeList).images; + check(() => Uri.parse(expectedImages.single.srcUrl)).throws(); + check(tryResolveUrl(eg.realmUrl, expectedImages.single.srcUrl)).isNull(); + // The MessageImage has shown up, but it doesn't attempt a RealmContentNetworkImage. + check(tester.widgetList(find.byType(MessageImage))).isNotEmpty(); + check(tester.widgetList(find.byType(RealmContentNetworkImage))).isEmpty(); + }); + + testWidgets('multiple images', (tester) async { + const example = ContentExample.imageCluster; + await prepareContent(tester, example.html); + final expectedImages = (example.expectedNodes[1] as ImageNodeList).images; + final images = tester.widgetList( + find.byType(RealmContentNetworkImage)); + check(images.map((i) => i.src.toString()).toList()) + .deepEquals(expectedImages.map((n) => n.srcUrl)); + }); + + testWidgets('content after image cluster', (tester) async { + const example = ContentExample.imageClusterThenContent; + await prepareContent(tester, example.html); + final expectedImages = (example.expectedNodes[1] as ImageNodeList).images; + final images = tester.widgetList( + find.byType(RealmContentNetworkImage)); + check(images.map((i) => i.src.toString()).toList()) + .deepEquals(expectedImages.map((n) => n.srcUrl)); + }); + + testWidgets('multiple clusters of images', (tester) async { + const example = ContentExample.imageMultipleClusters; + await prepareContent(tester, example.html); + final expectedImages = (example.expectedNodes[1] as ImageNodeList).images + + (example.expectedNodes[4] as ImageNodeList).images; + final images = tester.widgetList( + find.byType(RealmContentNetworkImage)); + check(images.map((i) => i.src.toString()).toList()) + .deepEquals(expectedImages.map((n) => n.srcUrl)); + }); + + testWidgets('image as immediate child in implicit paragraph', (tester) async { + const example = ContentExample.imageInImplicitParagraph; + await prepareContent(tester, example.html); + final expectedImages = ((example.expectedNodes[0] as ListNode) + .items[0][0] as ImageNodeList).images; + final images = tester.widgetList( + find.byType(RealmContentNetworkImage)); + check(images.map((i) => i.src.toString()).toList()) + .deepEquals(expectedImages.map((n) => n.srcUrl)); + }); + + testWidgets('image cluster in implicit paragraph', (tester) async { + const example = ContentExample.imageClusterInImplicitParagraph; + await prepareContent(tester, example.html); + final expectedImages = ((example.expectedNodes[0] as ListNode) + .items[0][1] as ImageNodeList).images; + final images = tester.widgetList( + find.byType(RealmContentNetworkImage)); + check(images.map((i) => i.src.toString()).toList()) + .deepEquals(expectedImages.map((n) => n.srcUrl)); + }); + }); + group("CodeBlock", () { testContentSmoke(ContentExample.codeBlockPlain); testContentSmoke(ContentExample.codeBlockHighlightedShort); @@ -252,96 +362,36 @@ void main() { tester.widget(find.textContaining(RegExp(r'^(Tue, Jan 30|Wed, Jan 31), 2024, \d+:\d\d [AP]M$'))); }); - group('MessageImages', () { - final message = eg.streamMessage(); - + group('MessageImageEmoji', () { Future prepareContent(WidgetTester tester, String html) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - final httpClient = FakeImageHttpClient(); + prepareBoringImageHttpClient(); - debugNetworkImageHttpClientProvider = () => httpClient; - httpClient.request.response - ..statusCode = HttpStatus.ok - ..content = kSolidBlueAvatar; - - await tester.pumpWidget( - MaterialApp( - home: Directionality( - textDirection: TextDirection.ltr, - child: GlobalStoreWidget( - child: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: MessageContent( - message: message, - content: parseContent(html))))))); + await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp( + home: PerAccountStoreWidget(accountId: eg.selfAccount.id, + child: BlockContentList(nodes: parseContent(html).nodes))))); await tester.pump(); // global store await tester.pump(); // per-account store - debugNetworkImageHttpClientProvider = null; } - testWidgets('single image', (tester) async { - const example = ContentExample.imageSingle; - await prepareContent(tester, example.html); - final expectedImages = (example.expectedNodes[0] as ImageNodeList).images; - final images = tester.widgetList( - find.byType(RealmContentNetworkImage)); - check(images.map((i) => i.src.toString()).toList()) - .deepEquals(expectedImages.map((n) => n.srcUrl)); - }); - - testWidgets('multiple images', (tester) async { - const example = ContentExample.imageCluster; - await prepareContent(tester, example.html); - final expectedImages = (example.expectedNodes[1] as ImageNodeList).images; - final images = tester.widgetList( - find.byType(RealmContentNetworkImage)); - check(images.map((i) => i.src.toString()).toList()) - .deepEquals(expectedImages.map((n) => n.srcUrl)); - }); - - testWidgets('content after image cluster', (tester) async { - const example = ContentExample.imageClusterThenContent; - await prepareContent(tester, example.html); - final expectedImages = (example.expectedNodes[1] as ImageNodeList).images; - final images = tester.widgetList( - find.byType(RealmContentNetworkImage)); - check(images.map((i) => i.src.toString()).toList()) - .deepEquals(expectedImages.map((n) => n.srcUrl)); - }); - - testWidgets('multiple clusters of images', (tester) async { - const example = ContentExample.imageMultipleClusters; - await prepareContent(tester, example.html); - final expectedImages = (example.expectedNodes[1] as ImageNodeList).images - + (example.expectedNodes[4] as ImageNodeList).images; - final images = tester.widgetList( - find.byType(RealmContentNetworkImage)); - check(images.map((i) => i.src.toString()).toList()) - .deepEquals(expectedImages.map((n) => n.srcUrl)); + testWidgets('smoke: custom emoji', (tester) async { + await prepareContent(tester, ContentExample.emojiCustom.html); + tester.widget(find.byType(MessageImageEmoji)); + debugNetworkImageHttpClientProvider = null; }); - testWidgets('image as immediate child in implicit paragraph', (tester) async { - const example = ContentExample.imageInImplicitParagraph; - await prepareContent(tester, example.html); - final expectedImages = ((example.expectedNodes[0] as ListNode) - .items[0][0] as ImageNodeList).images; - final images = tester.widgetList( - find.byType(RealmContentNetworkImage)); - check(images.map((i) => i.src.toString()).toList()) - .deepEquals(expectedImages.map((n) => n.srcUrl)); + testWidgets('smoke: custom emoji with invalid URL', (tester) async { + await prepareContent(tester, ContentExample.emojiCustomInvalidUrl.html); + final url = tester.widget(find.byType(MessageImageEmoji)).node.src; + check(() => Uri.parse(url)).throws(); + debugNetworkImageHttpClientProvider = null; }); - testWidgets('image cluster in implicit paragraph', (tester) async { - const example = ContentExample.imageClusterInImplicitParagraph; - await prepareContent(tester, example.html); - final expectedImages = ((example.expectedNodes[0] as ListNode) - .items[0][1] as ImageNodeList).images; - final images = tester.widgetList( - find.byType(RealmContentNetworkImage)); - check(images.map((i) => i.src.toString()).toList()) - .deepEquals(expectedImages.map((n) => n.srcUrl)); + testWidgets('smoke: Zulip extra emoji', (tester) async { + await prepareContent(tester, ContentExample.emojiZulipExtra.html); + tester.widget(find.byType(MessageImageEmoji)); + debugNetworkImageHttpClientProvider = null; }); }); @@ -349,14 +399,10 @@ void main() { final authHeaders = authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey); Future>> actualHeaders(WidgetTester tester, Uri src) async { - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - final httpClient = FakeImageHttpClient(); - debugNetworkImageHttpClientProvider = () => httpClient; - httpClient.request.response - ..statusCode = HttpStatus.ok - ..content = kSolidBlueAvatar; + final httpClient = prepareBoringImageHttpClient(); await tester.pumpWidget(GlobalStoreWidget( child: PerAccountStoreWidget(accountId: eg.selfAccount.id, @@ -388,4 +434,47 @@ void main() { check(tester.takeException()).isA(); }); }); + + group('AvatarImage', () { + late PerAccountStore store; + + Future actualUrl(WidgetTester tester, String avatarUrl) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final user = eg.user(avatarUrl: avatarUrl); + store.addUser(user); + + prepareBoringImageHttpClient(); + await tester.pumpWidget(GlobalStoreWidget( + child: PerAccountStoreWidget(accountId: eg.selfAccount.id, + child: AvatarImage(userId: user.userId)))); + await tester.pump(); + await tester.pump(); + tester.widget(find.byType(AvatarImage)); + final widgets = tester.widgetList( + find.byType(RealmContentNetworkImage)); + return widgets.firstOrNull?.src; + } + + testWidgets('smoke with absolute URL', (tester) async { + const avatarUrl = 'https://example/avatar.png'; + check(await actualUrl(tester, avatarUrl)).isNotNull() + .asString.equals(avatarUrl); + debugNetworkImageHttpClientProvider = null; + }); + + testWidgets('smoke with relative URL', (tester) async { + const avatarUrl = '/avatar.png'; + check(await actualUrl(tester, avatarUrl)) + .equals(store.tryResolveUrl(avatarUrl)!); + debugNetworkImageHttpClientProvider = null; + }); + + testWidgets('smoke with invalid URL', (tester) async { + const avatarUrl = '::not a URL::'; + check(await actualUrl(tester, avatarUrl)).isNull(); + debugNetworkImageHttpClientProvider = null; + }); + }); } diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 33202bf3e1..b8ee2e0e84 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -458,7 +458,7 @@ void main() { check(findAvatarImageWidget(tester)).isNull(); } else { check(findAvatarImageWidget(tester)).isNotNull() - .src.equals(resolveUrl(avatarUrl, eg.selfAccount)); + .src.equals(eg.selfAccount.realmUrl.resolve(avatarUrl)); } } diff --git a/test/widgets/store_test.dart b/test/widgets/store_test.dart index 580ab9fd8e..a4b8500ba4 100644 --- a/test/widgets/store_test.dart +++ b/test/widgets/store_test.dart @@ -35,7 +35,7 @@ class MyWidgetWithMixinState extends State with PerAccountSto @override Widget build(BuildContext context) { final brightness = Theme.of(context).brightness; - final accountId = PerAccountStoreWidget.of(context).account.id; + final accountId = PerAccountStoreWidget.of(context).accountId; return Text('brightness: $brightness; accountId: $accountId'); } } @@ -87,7 +87,7 @@ void main() { child: Builder( builder: (context) { final store = PerAccountStoreWidget.of(context); - return Text('found store, account: ${store.account.id}'); + return Text('found store, account: ${store.accountId}'); }))))); await tester.pump(); await tester.pump(); @@ -109,7 +109,7 @@ void main() { child: Builder( builder: (context) { final store = PerAccountStoreWidget.of(context); - return Text('found store, account: ${store.account.id}'); + return Text('found store, account: ${store.accountId}'); }))))); // First, the global store has to load. @@ -137,7 +137,7 @@ void main() { child: Builder( builder: (context) { final store = PerAccountStoreWidget.of(context); - return Text('found store, account: ${store.account.id}'); + return Text('found store, account: ${store.accountId}'); }))))); // (... even one that really is separate, with its own fresh state node ...)