diff --git a/lib/api/core.dart b/lib/api/core.dart index 04671c40e3..320454b12b 100644 --- a/lib/api/core.dart +++ b/lib/api/core.dart @@ -116,12 +116,24 @@ 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}")); - addAuth(request); + 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); + } + if (overrideUserAgent != null) { request.headers['User-Agent'] = overrideUserAgent; } else { 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/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/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 309f12f287..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,9 +756,15 @@ 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 - updateMachine.registerNotificationToken(); + unawaited(updateMachine.registerNotificationToken()); return updateMachine; } @@ -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 @@ -865,26 +918,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 +943,46 @@ 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. + /// + /// 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)}'; } diff --git a/test/api/core_test.dart b/test/api/core_test.dart index b3a40fdb0e..2cbda4feb3 100644 --- a/test/api/core_test.dart +++ b/test/api/core_test.dart @@ -19,6 +19,79 @@ import '../example_data.dart' as eg; void main() { TestZulipBinding.ensureInitialized(); + group('auth', () { + 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: {}); + 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('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() + .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 { 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); + }); + }); + }); +} 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 ?? [], 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;