From fe604ec81a6b11312082f438c684e5b10f4930bd Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 16 Sep 2024 13:47:35 -0700 Subject: [PATCH 1/7] api [nfc]: Explicitly check origin in ApiConnection.send This is NFC because all existing call sites (all of which are in the other methods on this class) do always pass URLs that are based on realmUrl and preserve its origin. --- lib/api/core.dart | 7 +++++++ test/api/core_test.dart | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/lib/api/core.dart b/lib/api/core.dart index 04671c40e3..d2fb43286f 100644 --- a/lib/api/core.dart +++ b/lib/api/core.dart @@ -121,7 +121,14 @@ class ApiConnection { assert(debugLog("${request.method} ${request.url}")); + if (request.url.origin != realmUrl.origin) { + // No caller should get here with a URL whose origin isn't the realm's. + // If this does happen, it's important not to proceed, because we'd be + // sending the user's auth credentials. + throw StateError("ApiConnection.send called on off-realm URL"); + } addAuth(request); + if (overrideUserAgent != null) { request.headers['User-Agent'] = overrideUserAgent; } else { diff --git a/test/api/core_test.dart b/test/api/core_test.dart index b3a40fdb0e..c2f0088ed8 100644 --- a/test/api/core_test.dart +++ b/test/api/core_test.dart @@ -19,6 +19,46 @@ import '../example_data.dart' as eg; void main() { TestZulipBinding.ensureInitialized(); + group('auth', () { + Future makeRequest(String realmUrl, String requestUrl) { + final account = eg.account(user: eg.selfUser, realmUrl: Uri.parse(realmUrl)); + return FakeApiConnection.with_(account: account, (connection) async { + connection.prepare(json: {}); + await connection.send(kExampleRouteName, (json) => json, + http.Request('GET', Uri.parse(requestUrl))); + return connection.lastRequest!; + }); + } + + test('send rejects off-realm URL', () async { + Future checkAllow(String realmUrl, String requestUrl) async { + check(await makeRequest(realmUrl, requestUrl)) + .isA() + .url.asString.equals(requestUrl); + } + + Future checkDeny(String realmUrl, String requestUrl) async { + await check(makeRequest(realmUrl, requestUrl)) + .throws(); + } + + // Baseline: normal requests are allowed. + checkAllow('https://chat.example', 'https://chat.example/api/v1/example/route'); + checkAllow('https://chat.example', 'https://chat.example/path'); + + // Mismatched origins are not allowed. + checkDeny ('https://chat.example', 'https://chat.example.evil/path'); + checkDeny ('https://chat.example', 'https://evil.chat.example/path'); + checkDeny ('https://chat.example', 'https://chat.example:444/path'); + checkDeny ('https://chat.example', 'http://chat.example/path'); + + // Less-expected scenarios that do have matching origins are also allowed. + checkAllow('https://chat.example', 'https://chat.example/'); + checkAllow('https://chat.example/', 'https://chat.example/path'); + checkAllow(r'https:/\chat.example', 'https://chat.example/path'); + }); + }); + test('ApiConnection.get', () async { void checkRequest(Map? params, String expectedRelativeUrl) { finish(FakeApiConnection.with_(account: eg.selfAccount, (connection) async { From fa9db6504e01822fc2b94477b3211a333d5cf70d Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 16 Sep 2024 13:56:16 -0700 Subject: [PATCH 2/7] api: Add useAuth flag on ApiConnection.send This will let us reuse ApiConnection for some specialty endpoints that aren't authenticated (or don't use the normal Zulip auth mechanism). --- lib/api/core.dart | 19 +++++++++++------- test/api/core_test.dart | 43 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/lib/api/core.dart b/lib/api/core.dart index d2fb43286f..320454b12b 100644 --- a/lib/api/core.dart +++ b/lib/api/core.dart @@ -116,18 +116,23 @@ class ApiConnection { bool _isOpen = true; Future send(String routeName, T Function(Map) fromJson, - http.BaseRequest request, {String? overrideUserAgent}) async { + http.BaseRequest request, { + bool useAuth = true, + String? overrideUserAgent, + }) async { assert(_isOpen); assert(debugLog("${request.method} ${request.url}")); - if (request.url.origin != realmUrl.origin) { - // No caller should get here with a URL whose origin isn't the realm's. - // If this does happen, it's important not to proceed, because we'd be - // sending the user's auth credentials. - throw StateError("ApiConnection.send called on off-realm URL"); + if (useAuth) { + if (request.url.origin != realmUrl.origin) { + // No caller should get here with a URL whose origin isn't the realm's. + // If this does happen, it's important not to proceed, because we'd be + // sending the user's auth credentials. + throw StateError("ApiConnection.send called with useAuth on off-realm URL"); + } + addAuth(request); } - addAuth(request); if (overrideUserAgent != null) { request.headers['User-Agent'] = overrideUserAgent; diff --git a/test/api/core_test.dart b/test/api/core_test.dart index c2f0088ed8..2cbda4feb3 100644 --- a/test/api/core_test.dart +++ b/test/api/core_test.dart @@ -20,17 +20,50 @@ void main() { TestZulipBinding.ensureInitialized(); group('auth', () { - Future makeRequest(String realmUrl, String requestUrl) { - final account = eg.account(user: eg.selfUser, realmUrl: Uri.parse(realmUrl)); + Future makeRequest(String realmUrl, String requestUrl, { + bool? useAuth, + }) { + final account = eg.account(user: eg.selfUser, apiKey: eg.selfAccount.apiKey, + realmUrl: Uri.parse(realmUrl)); return FakeApiConnection.with_(account: account, (connection) async { connection.prepare(json: {}); - await connection.send(kExampleRouteName, (json) => json, - http.Request('GET', Uri.parse(requestUrl))); + final request = http.Request('GET', Uri.parse(requestUrl)); + if (useAuth == null) { + // Null means use the default. We don't repeat the default on this + // test helper (as one normally would in non-test code), because that + // would mean we weren't checking what the default is. + await connection.send(kExampleRouteName, (json) => json, request); + } else { + await connection.send(kExampleRouteName, (json) => json, + useAuth: useAuth, request); + } return connection.lastRequest!; }); } - test('send rejects off-realm URL', () async { + test('auth headers sent by default', () async { + check(await makeRequest('https://chat.example', 'https://chat.example/path')) + .isA().headers.deepEquals({ + ...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey), + ...kFallbackUserAgentHeader, + }); + check(await makeRequest('https://chat.example', 'https://chat.example/path', + useAuth: true)) + .isA().headers.deepEquals({ + ...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey), + ...kFallbackUserAgentHeader, + }); + }); + + test('auth headers omitted if useAuth false', () async { + check(await makeRequest('https://chat.example', 'https://chat.example/path', + useAuth: false)) + .isA().headers.deepEquals({ + ...kFallbackUserAgentHeader, + }); + }); + + test('send rejects off-realm URL (with default useAuth)', () async { Future checkAllow(String realmUrl, String requestUrl) async { check(await makeRequest(realmUrl, requestUrl)) .isA() From 0f4f5971631de8986440ff7aafc1c945dc526c88 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 16 Sep 2024 14:00:07 -0700 Subject: [PATCH 3/7] api: Add serverEmojiDataUrl in initial snapshot --- lib/api/model/initial_snapshot.dart | 3 +++ lib/api/model/initial_snapshot.g.dart | 4 ++++ test/example_data.dart | 3 +++ 3 files changed, 10 insertions(+) diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index f9de6fb1b0..6f45d1a050 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -68,6 +68,8 @@ class InitialSnapshot { final int maxFileUploadSizeMib; + final Uri? serverEmojiDataUrl; // TODO(server-6) + @JsonKey(readValue: _readUsersIsActiveFallbackTrue) final List realmUsers; @JsonKey(readValue: _readUsersIsActiveFallbackFalse) @@ -117,6 +119,7 @@ class InitialSnapshot { required this.userTopics, required this.realmDefaultExternalAccounts, required this.maxFileUploadSizeMib, + required this.serverEmojiDataUrl, required this.realmUsers, required this.realmNonActiveUsers, required this.crossRealmBots, diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index 01dc1ea850..80cf0f6cc8 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -64,6 +64,9 @@ InitialSnapshot _$InitialSnapshotFromJson(Map json) => k, RealmDefaultExternalAccount.fromJson(e as Map)), ), maxFileUploadSizeMib: (json['max_file_upload_size_mib'] as num).toInt(), + serverEmojiDataUrl: json['server_emoji_data_url'] == null + ? null + : Uri.parse(json['server_emoji_data_url'] as String), realmUsers: (InitialSnapshot._readUsersIsActiveFallbackTrue(json, 'realm_users') as List) @@ -105,6 +108,7 @@ Map _$InitialSnapshotToJson(InitialSnapshot instance) => 'user_topics': instance.userTopics, 'realm_default_external_accounts': instance.realmDefaultExternalAccounts, 'max_file_upload_size_mib': instance.maxFileUploadSizeMib, + 'server_emoji_data_url': instance.serverEmojiDataUrl?.toString(), 'realm_users': instance.realmUsers, 'realm_non_active_users': instance.realmNonActiveUsers, 'cross_realm_bots': instance.crossRealmBots, diff --git a/test/example_data.dart b/test/example_data.dart index b4bd5159f0..5601e8356d 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -791,6 +791,7 @@ InitialSnapshot initialSnapshot({ List? userTopics, Map? realmDefaultExternalAccounts, int? maxFileUploadSizeMib, + Uri? serverEmojiDataUrl, List? realmUsers, List? realmNonActiveUsers, List? crossRealmBots, @@ -823,6 +824,8 @@ InitialSnapshot initialSnapshot({ userTopics: userTopics, realmDefaultExternalAccounts: realmDefaultExternalAccounts ?? {}, maxFileUploadSizeMib: maxFileUploadSizeMib ?? 25, + serverEmojiDataUrl: serverEmojiDataUrl + ?? realmUrl.replace(path: '/static/emoji.json'), realmUsers: realmUsers ?? [], realmNonActiveUsers: realmNonActiveUsers ?? [], crossRealmBots: crossRealmBots ?? [], From 66fb237abb1e286a7a1fd80a6d708417c673d6a8 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 16 Sep 2024 14:11:20 -0700 Subject: [PATCH 4/7] api: Add route fetchServerEmojiData --- lib/api/route/realm.dart | 48 ++++++++++++++++++++++++++++++++++ lib/api/route/realm.g.dart | 13 +++++++++ test/api/route/realm_test.dart | 48 ++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 test/api/route/realm_test.dart diff --git a/lib/api/route/realm.dart b/lib/api/route/realm.dart index aca71e85b5..a43c2e9921 100644 --- a/lib/api/route/realm.dart +++ b/lib/api/route/realm.dart @@ -1,6 +1,8 @@ +import 'package:http/http.dart' as http; import 'package:json_annotation/json_annotation.dart'; import '../core.dart'; +import '../model/initial_snapshot.dart'; part 'realm.g.dart'; @@ -94,3 +96,49 @@ class ExternalAuthenticationMethod { Map toJson() => _$ExternalAuthenticationMethodToJson(this); } + +/// Fetch data from the URL described by [InitialSnapshot.serverEmojiDataUrl]. +/// +/// This request is unauthenticated, and the URL need not be on the realm. +/// The given [ApiConnection] is used for providing a `User-Agent` header +/// and for handling errors. +/// +/// For docs, search for "server_emoji" +/// in . +Future fetchServerEmojiData(ApiConnection connection, { + required Uri emojiDataUrl, +}) { + // TODO(#950): cache server responses on fetchServerEmojiData + + // This nontraditional endpoint doesn't conform to all the usual Zulip API + // protocols: https://zulip.com/api/rest-error-handling + // notably the `{ code, msg, result }` format for errors. + // So in the case of an error, the generic Zulip API error-handling logic + // in [ApiConnection.send] will throw [MalformedServerResponseException] + // in some cases where "malformed" isn't quite the right description. + // We'll just tolerate that. + // If it really mattered, we could refactor [ApiConnection] to accommodate. + // + // Similarly, there's no `"result": "success"` in the response. + // Fortunately none of our code looks for that in the first place. + return connection.send('fetchServerEmojiData', ServerEmojiData.fromJson, + useAuth: false, + http.Request('GET', emojiDataUrl)); +} + +/// The server's data describing its list of Unicode emoji +/// and its names for them. +/// +/// For docs, search for "server_emoji" +/// in . +@JsonSerializable(fieldRename: FieldRename.snake) +class ServerEmojiData { + final Map> codeToNames; + + ServerEmojiData({required this.codeToNames}); + + factory ServerEmojiData.fromJson(Map json) => + _$ServerEmojiDataFromJson(json); + + Map toJson() => _$ServerEmojiDataToJson(this); +} diff --git a/lib/api/route/realm.g.dart b/lib/api/route/realm.g.dart index 8957739f30..e34fbc9d0c 100644 --- a/lib/api/route/realm.g.dart +++ b/lib/api/route/realm.g.dart @@ -72,3 +72,16 @@ Map _$ExternalAuthenticationMethodToJson( 'login_url': instance.loginUrl, 'signup_url': instance.signupUrl, }; + +ServerEmojiData _$ServerEmojiDataFromJson(Map json) => + ServerEmojiData( + codeToNames: (json['code_to_names'] as Map).map( + (k, e) => + MapEntry(k, (e as List).map((e) => e as String).toList()), + ), + ); + +Map _$ServerEmojiDataToJson(ServerEmojiData instance) => + { + 'code_to_names': instance.codeToNames, + }; diff --git a/test/api/route/realm_test.dart b/test/api/route/realm_test.dart new file mode 100644 index 0000000000..c1cc18b98b --- /dev/null +++ b/test/api/route/realm_test.dart @@ -0,0 +1,48 @@ +import 'package:checks/checks.dart'; +import 'package:http/http.dart' as http; +import 'package:test/scaffolding.dart'; +import 'package:zulip/api/core.dart'; +import 'package:zulip/api/route/realm.dart'; + +import '../../stdlib_checks.dart'; +import '../fake_api.dart'; + +void main() { + group('fetchServerEmojiData', () { + Future checkFetchServerEmojiData(FakeApiConnection connection, { + required Uri emojiDataUrl, + }) async { + final result = await fetchServerEmojiData(connection, + emojiDataUrl: emojiDataUrl); + check(connection.lastRequest).isA() + ..method.equals('GET') + ..url.equals(emojiDataUrl) + ..headers.deepEquals(kFallbackUserAgentHeader); + return result; + } + + final fakeResult = ServerEmojiData(codeToNames: { + '1f642': ['smile'], + '1f34a': ['orange', 'tangerine', 'mandarin'], + }); + + test('smoke', () { + return FakeApiConnection.with_((connection) async { + connection.prepare(json: fakeResult.toJson()); + check(await checkFetchServerEmojiData(connection, + emojiDataUrl: connection.realmUrl.resolve('/static/emoji.json') + )).jsonEquals(fakeResult); + }); + }); + + test('off-realm is OK', () { + return FakeApiConnection.with_( + realmUrl: Uri.parse('https://chat.example'), (connection) async { + connection.prepare(json: fakeResult.toJson()); + check(await checkFetchServerEmojiData(connection, + emojiDataUrl: Uri.parse('https://elsewhere.example/static/emoji.json') + )).jsonEquals(fakeResult); + }); + }); + }); +} From ce01d068f18d6e7daf7a22435f1e2f226da39918 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 30 Sep 2024 16:13:54 -0700 Subject: [PATCH 5/7] store [nfc]: Clarify registerNotificationToken is unawaited --- lib/model/store.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/model/store.dart b/lib/model/store.dart index 309f12f287..981144d1a8 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -748,7 +748,7 @@ class UpdateMachine { updateMachine.poll(); // TODO do registerNotificationToken before registerQueue: // https://github.com/zulip/zulip-flutter/pull/325#discussion_r1365982807 - updateMachine.registerNotificationToken(); + unawaited(updateMachine.registerNotificationToken()); return updateMachine; } From e963af53a62ae0d5076c738488421703be630e8c Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 1 Oct 2024 19:20:01 -0700 Subject: [PATCH 6/7] store [nfc]: Move debugEnableRegisterNotificationToken to end of class We'll need another of these for fetching emoji data. Let's move them out from amidst all the main logic, to keep down the clutter. --- lib/model/store.dart | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/model/store.dart b/lib/model/store.dart index 981144d1a8..f6469539dc 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -865,26 +865,6 @@ class UpdateMachine { } } - /// In debug mode, controls whether [registerNotificationToken] should - /// have its normal effect. - /// - /// Outside of debug mode, this is always true and the setter has no effect. - static bool get debugEnableRegisterNotificationToken { - bool result = true; - assert(() { - result = _debugEnableRegisterNotificationToken; - return true; - }()); - return result; - } - static bool _debugEnableRegisterNotificationToken = true; - static set debugEnableRegisterNotificationToken(bool value) { - assert(() { - _debugEnableRegisterNotificationToken = value; - return true; - }()); - } - /// Send this client's notification token to the server, now and if it changes. /// /// TODO The returned future isn't especially meaningful (it may or may not @@ -910,6 +890,26 @@ class UpdateMachine { NotificationService.instance.token.removeListener(_registerNotificationToken); } + /// In debug mode, controls whether [registerNotificationToken] should + /// have its normal effect. + /// + /// Outside of debug mode, this is always true and the setter has no effect. + static bool get debugEnableRegisterNotificationToken { + bool result = true; + assert(() { + result = _debugEnableRegisterNotificationToken; + return true; + }()); + return result; + } + static bool _debugEnableRegisterNotificationToken = true; + static set debugEnableRegisterNotificationToken(bool value) { + assert(() { + _debugEnableRegisterNotificationToken = value; + return true; + }()); + } + @override String toString() => '${objectRuntimeType(this, 'UpdateMachine')}#${shortHash(this)}'; } From f5d09c4cb6090aa205bd9a7ef8e8b492f095aa4e Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 17 Sep 2024 14:36:21 -0700 Subject: [PATCH 7/7] emoji: Fetch server emoji data (about Unicode emoji) This completes the data we'll need for use in #669, which in turn will let us offer an emoji picker for reactions and for use inside message content. The doc on fetchEmojiData is based on a comment at the corresponding code in the legacy zulip-mobile app, in src/events/eventActions.js . --- lib/model/emoji.dart | 24 ++++++++++++- lib/model/store.dart | 73 ++++++++++++++++++++++++++++++++++++++ test/model/store_test.dart | 67 ++++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+), 1 deletion(-) diff --git a/lib/model/emoji.dart b/lib/model/emoji.dart index 688cec6549..2eea7fcde2 100644 --- a/lib/model/emoji.dart +++ b/lib/model/emoji.dart @@ -1,6 +1,7 @@ import '../api/model/events.dart'; import '../api/model/initial_snapshot.dart'; import '../api/model/model.dart'; +import '../api/route/realm.dart'; /// An emoji, described by how to display it in the UI. sealed class EmojiDisplay { @@ -61,6 +62,12 @@ mixin EmojiStore { required String emojiCode, required String emojiName, }); + + // TODO cut debugServerEmojiData once we can query for lists of emoji; + // have tests make those queries end-to-end + Map>? get debugServerEmojiData; + + void setServerEmojiData(ServerEmojiData data); } /// The implementation of [EmojiStore] that does the work. @@ -72,7 +79,7 @@ class EmojiStoreImpl with EmojiStore { EmojiStoreImpl({ required this.realmUrl, required this.realmEmoji, - }); + }) : _serverEmojiData = null; // TODO(#974) maybe start from a hard-coded baseline /// The same as [PerAccountStore.realmUrl]. final Uri realmUrl; @@ -131,6 +138,21 @@ class EmojiStoreImpl with EmojiStore { ); } + @override + Map>? get debugServerEmojiData => _serverEmojiData; + + /// The server's list of Unicode emoji and names for them, + /// from [ServerEmojiData]. + /// + /// This is null until [UpdateMachine.fetchEmojiData] finishes + /// retrieving the data. + Map>? _serverEmojiData; + + @override + void setServerEmojiData(ServerEmojiData data) { + _serverEmojiData = data.codeToNames; + } + void handleRealmEmojiUpdateEvent(RealmEmojiUpdateEvent event) { realmEmoji = event.realmEmoji; } diff --git a/lib/model/store.dart b/lib/model/store.dart index f6469539dc..8cc25f2759 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -16,6 +16,7 @@ import '../api/model/model.dart'; import '../api/route/events.dart'; import '../api/route/messages.dart'; import '../api/backoff.dart'; +import '../api/route/realm.dart'; import '../log.dart'; import '../notifications/receive.dart'; import 'autocomplete.dart'; @@ -345,6 +346,15 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess emojiType: emojiType, emojiCode: emojiCode, emojiName: emojiName); } + @override + Map>? get debugServerEmojiData => _emoji.debugServerEmojiData; + + @override + void setServerEmojiData(ServerEmojiData data) { + _emoji.setServerEmojiData(data); + notifyListeners(); + } + EmojiStoreImpl _emoji; //////////////////////////////// @@ -746,6 +756,12 @@ class UpdateMachine { final updateMachine = UpdateMachine.fromInitialSnapshot( store: store, initialSnapshot: initialSnapshot); updateMachine.poll(); + if (initialSnapshot.serverEmojiDataUrl != null) { + // TODO(server-6): If the server is ancient, just skip trying to have + // a list of its emoji. (The old servers that don't provide + // serverEmojiDataUrl are already unsupported at time of writing.) + unawaited(updateMachine.fetchEmojiData(initialSnapshot.serverEmojiDataUrl!)); + } // TODO do registerNotificationToken before registerQueue: // https://github.com/zulip/zulip-flutter/pull/325#discussion_r1365982807 unawaited(updateMachine.registerNotificationToken()); @@ -772,6 +788,43 @@ class UpdateMachine { } } + /// Fetch emoji data from the server, and update the store with the result. + /// + /// This functions a lot like [registerQueue] and the surrounding logic + /// in [load] above, but it's unusual in that we've separated it out. + /// Effectively it's data that *would have* been in the [registerQueue] + /// response, except that we pulled it out to its own endpoint as part of + /// a caching strategy, because the data changes infrequently. + /// + /// Conveniently (a) this deferred fetch doesn't cause any fetch/event race, + /// because this data doesn't get updated by events anyway (it can change + /// only on a server restart); and (b) we don't need this data for displaying + /// messages or anything else, only for certain UIs like the emoji picker, + /// so it's fine that we go without it for a while. + Future fetchEmojiData(Uri serverEmojiDataUrl) async { + if (!debugEnableFetchEmojiData) return; + BackoffMachine? backoffMachine; + ServerEmojiData data; + while (true) { + try { + data = await fetchServerEmojiData(store.connection, + emojiDataUrl: serverEmojiDataUrl); + assert(debugLog('Got emoji data: ${data.codeToNames.length} emoji')); + break; + } catch (e) { + assert(debugLog('Error fetching emoji data: $e\n' // TODO(log) + 'Backing off, then will retry…')); + // The emoji data is a lot less urgent than the initial fetch, + // or even the event-queue poll request. So wait longer. + backoffMachine ??= BackoffMachine(firstBound: const Duration(seconds: 2), + maxBound: const Duration(minutes: 2)); + await backoffMachine.wait(); + } + } + + store.setServerEmojiData(data); + } + Completer? _debugLoopSignal; /// In debug mode, causes the polling loop to pause before the next @@ -890,6 +943,26 @@ class UpdateMachine { NotificationService.instance.token.removeListener(_registerNotificationToken); } + /// In debug mode, controls whether [fetchEmojiData] should + /// have its normal effect. + /// + /// Outside of debug mode, this is always true and the setter has no effect. + static bool get debugEnableFetchEmojiData { + bool result = true; + assert(() { + result = _debugEnableFetchEmojiData; + return true; + }()); + return result; + } + static bool _debugEnableFetchEmojiData = true; + static set debugEnableFetchEmojiData(bool value) { + assert(() { + _debugEnableFetchEmojiData = value; + return true; + }()); + } + /// In debug mode, controls whether [registerNotificationToken] should /// have its normal effect. /// diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 615dd27dc6..01ace52e33 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -4,10 +4,12 @@ import 'package:checks/checks.dart'; import 'package:flutter/foundation.dart'; import 'package:http/http.dart' as http; import 'package:test/scaffolding.dart'; +import 'package:zulip/api/core.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/events.dart'; import 'package:zulip/api/route/messages.dart'; +import 'package:zulip/api/route/realm.dart'; import 'package:zulip/model/message_list.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; @@ -230,6 +232,8 @@ void main() { await globalStore.insertAccount(account.toCompanion(false)); connection = (globalStore.apiConnectionFromAccount(account) as FakeApiConnection); + UpdateMachine.debugEnableFetchEmojiData = false; + addTearDown(() => UpdateMachine.debugEnableFetchEmojiData = true); UpdateMachine.debugEnableRegisterNotificationToken = false; addTearDown(() => UpdateMachine.debugEnableRegisterNotificationToken = true); } @@ -317,6 +321,69 @@ void main() { // TODO test UpdateMachine.load calls registerNotificationToken }); + group('UpdateMachine.fetchEmojiData', () { + late UpdateMachine updateMachine; + late PerAccountStore store; + late FakeApiConnection connection; + + void prepareStore() { + updateMachine = eg.updateMachine(); + store = updateMachine.store; + connection = store.connection as FakeApiConnection; + } + + final emojiDataUrl = Uri.parse('https://cdn.example/emoji.json'); + final data = { + '1f642': ['smile'], + '1f34a': ['orange', 'tangerine', 'mandarin'], + }; + + void checkLastRequest() { + check(connection.takeRequests()).single.isA() + ..method.equals('GET') + ..url.equals(emojiDataUrl) + ..headers.deepEquals(kFallbackUserAgentHeader); + } + + test('happy case', () => awaitFakeAsync((async) async { + prepareStore(); + check(store.debugServerEmojiData).isNull(); + + connection.prepare(json: ServerEmojiData(codeToNames: data).toJson()); + await updateMachine.fetchEmojiData(emojiDataUrl); + checkLastRequest(); + check(store.debugServerEmojiData).deepEquals(data); + })); + + test('retries on failure', () => awaitFakeAsync((async) async { + prepareStore(); + check(store.debugServerEmojiData).isNull(); + + // Try to fetch, inducing an error in the request. + connection.prepare(exception: Exception('failed')); + final future = updateMachine.fetchEmojiData(emojiDataUrl); + bool complete = false; + future.whenComplete(() => complete = true); + async.flushMicrotasks(); + checkLastRequest(); + check(complete).isFalse(); + check(store.debugServerEmojiData).isNull(); + + // The retry doesn't happen immediately; there's a timer. + check(async.pendingTimers).length.equals(1); + async.elapse(Duration.zero); + check(connection.lastRequest).isNull(); + check(async.pendingTimers).length.equals(1); + + // After a timer, we retry. + connection.prepare(json: ServerEmojiData(codeToNames: data).toJson()); + await future; + check(complete).isTrue(); + checkLastRequest(); + check(store.debugServerEmojiData).deepEquals(data); + })); + }); + group('UpdateMachine.poll', () { late TestGlobalStore globalStore; late UpdateMachine updateMachine;