diff --git a/lib/model/push_device.dart b/lib/model/push_device.dart new file mode 100644 index 0000000000..0b82b3c24a --- /dev/null +++ b/lib/model/push_device.dart @@ -0,0 +1,93 @@ +import 'dart:async'; + +import '../notifications/receive.dart'; +import 'store.dart'; + +/// Manages telling the server this device's push token, +/// and tracking the server's responses on the status of push devices. +// TODO(#1764) do that tracking of responses +class PushDeviceManager extends PerAccountStoreBase { + PushDeviceManager({required super.core}) { + _registerTokenAndSubscribe(); + } + + bool _disposed = false; + + /// Cleans up resources and tells the instance not to make new API requests. + /// + /// After this is called, the instance is not in a usable state + /// and should be abandoned. + void dispose() { + assert(!_disposed); + NotificationService.instance.token.removeListener(_registerToken); + _disposed = true; + } + + /// Send this client's notification token to the server, now and if it changes. + // TODO(#322) save acked token, to dedupe updating it on the server + // TODO(#323) track the addFcmToken/etc request, warn if not succeeding + void _registerTokenAndSubscribe() async { + _debugMaybePause(); + if (_debugRegisterTokenProceed != null) { + await _debugRegisterTokenProceed!.future; + } + + NotificationService.instance.token.addListener(_registerToken); + await _registerToken(); + + _debugRegisterTokenCompleted?.complete(); + } + + Future _registerToken() async { + // TODO it would be nice to register the token before even registerQueue: + // https://github.com/zulip/zulip-flutter/pull/325#discussion_r1365982807 + await NotificationService.instance.registerToken(connection); + } + + Completer? _debugRegisterTokenProceed; + Completer? _debugRegisterTokenCompleted; + + void _debugMaybePause() { + assert(() { + if (debugAutoPause) { + _debugRegisterTokenProceed = Completer(); + _debugRegisterTokenCompleted = Completer(); + } + return true; + }()); + } + + /// Unpause registering the token (after [debugAutoPause]), + /// returning a future that completes when any immediate request is completed. + /// + /// This has no effect if [debugAutoPause] was false + /// when this instance was constructed, + /// and therefore no effect outside of debug mode. + Future debugUnpauseRegisterToken() async { + _debugRegisterTokenProceed!.complete(); + await _debugRegisterTokenCompleted!.future; + } + + /// In debug mode, controls whether new instances should pause + /// before registering the token with the server. + /// + /// When paused, token registration can be unpaused + /// with [debugUnpauseRegisterToken]. + /// + /// Outside of debug mode, this is always false and the setter has no effect. + static bool get debugAutoPause { + bool result = false; + assert(() { + result = _debugAutoPause; + return true; + }()); + return result; + } + static bool _debugAutoPause = false; + static set debugAutoPause(bool value) { + assert(() { + _debugAutoPause = value; + return true; + }()); + } +} diff --git a/lib/model/store.dart b/lib/model/store.dart index a5da6aa3f8..fe503da36e 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -17,7 +17,6 @@ import '../api/route/events.dart'; import '../api/backoff.dart'; import '../api/route/realm.dart'; import '../log.dart'; -import '../notifications/receive.dart'; import 'actions.dart'; import 'autocomplete.dart'; import 'database.dart'; @@ -25,6 +24,7 @@ import 'emoji.dart'; import 'localizations.dart'; import 'message.dart'; import 'presence.dart'; +import 'push_device.dart'; import 'realm.dart'; import 'recent_dm_conversations.dart'; import 'recent_senders.dart'; @@ -529,6 +529,7 @@ class PerAccountStore extends PerAccountStoreBase with emoji: EmojiStoreImpl(core: core, allRealmEmoji: initialSnapshot.realmEmoji), userSettings: initialSnapshot.userSettings, + pushDevices: PushDeviceManager(core: core), savedSnippets: SavedSnippetStoreImpl(core: core, savedSnippets: initialSnapshot.savedSnippets ?? []), typingNotifier: TypingNotifier(realm: realm), @@ -552,6 +553,7 @@ class PerAccountStore extends PerAccountStoreBase with required RealmStoreImpl realm, required EmojiStoreImpl emoji, required this.userSettings, + required this.pushDevices, required SavedSnippetStoreImpl savedSnippets, required this.typingNotifier, required UserStoreImpl users, @@ -623,6 +625,8 @@ class PerAccountStore extends PerAccountStoreBase with final UserSettings userSettings; + final PushDeviceManager pushDevices; + @override Map get savedSnippets => _savedSnippets.savedSnippets; final SavedSnippetStoreImpl _savedSnippets; @@ -714,6 +718,7 @@ class PerAccountStore extends PerAccountStoreBase with presence.dispose(); typingStatus.dispose(); typingNotifier.dispose(); + pushDevices.dispose(); updateMachine?.dispose(); connection.close(); _disposed = true; @@ -1118,9 +1123,6 @@ class UpdateMachine { // 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()); store.presence.start(); return updateMachine; } @@ -1505,28 +1507,6 @@ class UpdateMachine { store.realmUrl.toString(), error.toString())); } - /// 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 - /// mean we actually sent the token). Make it just `void` once we fix the - /// one test that relies on the future. - // TODO(#322) save acked token, to dedupe updating it on the server - // TODO(#323) track the addFcmToken/etc request, warn if not succeeding - Future registerNotificationToken() async { - assert(!_disposed); - if (!debugEnableRegisterNotificationToken) { - return; - } - NotificationService.instance.token.addListener(_registerNotificationToken); - await _registerNotificationToken(); - } - - Future _registerNotificationToken() async { - final token = NotificationService.instance.token.value; - if (token == null) return; - await NotificationService.registerToken(store.connection, token: token); - } - /// Cleans up resources and tells the instance not to make new API requests. /// /// After this is called, the instance is not in a usable state @@ -1537,7 +1517,6 @@ class UpdateMachine { /// requests to error. [PerAccountStore.dispose] does that. void dispose() { assert(!_disposed); - NotificationService.instance.token.removeListener(_registerNotificationToken); _disposed = true; } @@ -1561,26 +1540,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; - }()); - } - @override String toString() => '${objectRuntimeType(this, 'UpdateMachine')}#${shortHash(this)}'; } diff --git a/lib/notifications/receive.dart b/lib/notifications/receive.dart index 212b0f5f0d..3930645747 100644 --- a/lib/notifications/receive.dart +++ b/lib/notifications/receive.dart @@ -147,7 +147,10 @@ class NotificationService { token.value = value; } - static Future registerToken(ApiConnection connection, {required String token}) async { + Future registerToken(ApiConnection connection) async { + final token = this.token.value; + if (token == null) return; + switch (defaultTargetPlatform) { case TargetPlatform.android: await addFcmToken(connection, token: token); diff --git a/test/model/actions_test.dart b/test/model/actions_test.dart index e7ba1339dd..c28bd76fec 100644 --- a/test/model/actions_test.dart +++ b/test/model/actions_test.dart @@ -7,6 +7,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart' as http_testing; import 'package:zulip/model/actions.dart'; +import 'package:zulip/model/push_device.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/notifications/receive.dart'; @@ -147,6 +148,8 @@ void main() { })); test('notifications are removed after logout', () => awaitFakeAsync((async) async { + PushDeviceManager.debugAutoPause = true; + addTearDown(() => PushDeviceManager.debugAutoPause = false); await prepare(); testBinding.firebaseMessagingInitialToken = '123'; addTearDown(NotificationService.debugReset); @@ -180,6 +183,7 @@ void main() { test('fallback to current token if acked is missing', () => awaitFakeAsync((async) async { await prepare(ackedPushToken: null); + addTearDown(NotificationService.debugReset); NotificationService.instance.token = ValueNotifier('asdf'); final newConnection = separateConnection() @@ -193,6 +197,7 @@ void main() { test('no error if acked token and current token both missing', () => awaitFakeAsync((async) async { await prepare(ackedPushToken: null); + addTearDown(NotificationService.debugReset); NotificationService.instance.token = ValueNotifier(null); final newConnection = separateConnection(); diff --git a/test/model/push_device_test.dart b/test/model/push_device_test.dart new file mode 100644 index 0000000000..32299d6cff --- /dev/null +++ b/test/model/push_device_test.dart @@ -0,0 +1,130 @@ +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/model/push_device.dart'; +import 'package:zulip/notifications/receive.dart'; + +import '../api/fake_api.dart'; +import '../example_data.dart' as eg; +import '../fake_async.dart'; +import 'binding.dart'; +import 'store_test.dart'; +import '../stdlib_checks.dart'; + +void main() { + TestZulipBinding.ensureInitialized(); + + group('register token', () { + late PushDeviceManager model; + late FakeApiConnection connection; + + void prepareStore() { + PushDeviceManager.debugAutoPause = true; + addTearDown(() => PushDeviceManager.debugAutoPause = false); + final store = eg.store(); + model = store.pushDevices; + connection = store.connection as FakeApiConnection; + } + + void checkLastRequestApns({required String token, required String appid}) { + check(connection.takeRequests()).single.isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/users/me/apns_device_token') + ..bodyFields.deepEquals({'token': token, 'appid': appid}); + } + + void checkLastRequestFcm({required String token}) { + check(connection.takeRequests()).single.isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/users/me/android_gcm_reg_id') + ..bodyFields.deepEquals({'token': token}); + } + + testAndroidIos('token already known', () => awaitFakeAsync((async) async { + // This tests the case where [NotificationService.start] has already + // learned the token before the store is created. + // (This is probably the common case.) + addTearDown(testBinding.reset); + testBinding.firebaseMessagingInitialToken = '012abc'; + testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.zulip.flutter'); + addTearDown(NotificationService.debugReset); + await NotificationService.instance.start(); + + // On store startup, send the token. + prepareStore(); + connection.prepare(json: {}); + await model.debugUnpauseRegisterToken(); + if (defaultTargetPlatform == TargetPlatform.android) { + checkLastRequestFcm(token: '012abc'); + } else { + checkLastRequestApns(token: '012abc', appid: 'com.zulip.flutter'); + } + + if (defaultTargetPlatform == TargetPlatform.android) { + // If the token changes, send it again. + testBinding.firebaseMessaging.setToken('456def'); + connection.prepare(json: {}); + async.flushMicrotasks(); + checkLastRequestFcm(token: '456def'); + } + })); + + testAndroidIos('token initially unknown', () => awaitFakeAsync((async) async { + // This tests the case where the store is created while our + // request for the token is still pending. + addTearDown(testBinding.reset); + testBinding.firebaseMessagingInitialToken = '012abc'; + testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.zulip.flutter'); + addTearDown(NotificationService.debugReset); + final startFuture = NotificationService.instance.start(); + + // TODO this test is a bit brittle in its interaction with asynchrony; + // to fix, probably extend TestZulipBinding to control when getToken finishes. + // + // The aim here is to first wait for `model.debugUnpauseRegisterToken` + // to complete whatever it's going to do; then check no request was made; + // and only after that wait for `NotificationService.start` to finish, + // including its `getToken` call. + + // On store startup, send nothing (because we have nothing to send). + prepareStore(); + await model.debugUnpauseRegisterToken(); + check(connection.lastRequest).isNull(); + + // When the token later appears, send it. + connection.prepare(json: {}); + await startFuture; + async.flushMicrotasks(); + if (defaultTargetPlatform == TargetPlatform.android) { + checkLastRequestFcm(token: '012abc'); + } else { + checkLastRequestApns(token: '012abc', appid: 'com.zulip.flutter'); + } + + if (defaultTargetPlatform == TargetPlatform.android) { + // If the token subsequently changes, send it again. + testBinding.firebaseMessaging.setToken('456def'); + connection.prepare(json: {}); + async.flushMicrotasks(); + checkLastRequestFcm(token: '456def'); + } + })); + + test('on iOS, use provided app ID from packageInfo', () => awaitFakeAsync((async) async { + final origTargetPlatform = debugDefaultTargetPlatformOverride; + addTearDown(() => debugDefaultTargetPlatformOverride = origTargetPlatform); + debugDefaultTargetPlatformOverride = TargetPlatform.iOS; + addTearDown(testBinding.reset); + testBinding.firebaseMessagingInitialToken = '012abc'; + testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.example.test'); + addTearDown(NotificationService.debugReset); + await NotificationService.instance.start(); + + prepareStore(); + connection.prepare(json: {}); + await model.debugUnpauseRegisterToken(); + checkLastRequestApns(token: '012abc', appid: 'com.example.test'); + })); + }); +} diff --git a/test/model/store_test.dart b/test/model/store_test.dart index d3c3e9c4e4..e522174507 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -19,7 +19,6 @@ import 'package:zulip/model/actions.dart'; import 'package:zulip/model/presence.dart'; import 'package:zulip/model/server_support.dart'; import 'package:zulip/model/store.dart'; -import 'package:zulip/notifications/receive.dart'; import '../api/fake_api.dart'; import '../api/model/model_checks.dart'; @@ -136,17 +135,14 @@ void main() { })); test('GlobalStore.perAccount loading succeeds', () => awaitFakeAsync((async) async { - NotificationService.instance.token = ValueNotifier('asdf'); - addTearDown(NotificationService.debugReset); - final globalStore = UpdateMachineTestGlobalStore(accounts: [eg.selfAccount]); final connection = globalStore.apiConnectionFromAccount(eg.selfAccount) as FakeApiConnection; final future = globalStore.perAccount(eg.selfAccount.id); check(connection.takeRequests()).length.equals(1); // register request await future; - // poll, server-emoji-data, register-token requests - check(connection.takeRequests()).length.equals(3); + // poll and server-emoji-data requests + check(connection.takeRequests()).length.equals(2); check(connection).isOpen.isTrue(); })); @@ -162,7 +158,7 @@ void main() { await check(future).throws(); check(globalStore.takeDoRemoveAccountCalls()).single.equals(eg.selfAccount.id); - // no poll, server-emoji-data, or register-token requests + // no poll or other follow-up requests check(connection.takeRequests()).isEmpty(); check(connection).isOpen.isFalse(); })); @@ -181,7 +177,7 @@ void main() { await check(future).throws(); check(globalStore.takeDoRemoveAccountCalls()).single.equals(eg.selfAccount.id); - // no poll, server-emoji-data, or register-token requests + // no poll or other follow-up requests check(connection.takeRequests()).isEmpty(); check(connection).isOpen.isFalse(); })); @@ -202,7 +198,7 @@ void main() { await check(future).throws(); check(globalStore.takeDoRemoveAccountCalls()).isEmpty(); - // no poll, server-emoji-data, or register-token requests + // no poll or other follow-up requests check(connection.takeRequests()).isEmpty(); check(connection).isOpen.isFalse(); })); @@ -223,7 +219,7 @@ void main() { await check(future).throws(); check(globalStore.takeDoRemoveAccountCalls()).isEmpty(); - // no poll, server-emoji-data, or register-token requests + // no poll or other follow-up requests check(connection.takeRequests()).isEmpty(); check(connection).isOpen.isFalse(); })); @@ -245,7 +241,7 @@ void main() { await check(future).throws(); check(globalStore.takeDoRemoveAccountCalls()).isEmpty(); - // no poll, server-emoji-data, or register-token requests + // no poll or other follow-up requests check(connection.takeRequests()).isEmpty(); check(connection).isOpen.isFalse(); })); @@ -269,7 +265,7 @@ void main() { await check(future).throws(); check(globalStore.takeDoRemoveAccountCalls()).isEmpty(); - // no poll, server-emoji-data, or register-token requests + // no poll or other follow-up requests check(connection.takeRequests()).isEmpty(); check(connection).isOpen.isFalse(); })); @@ -293,7 +289,7 @@ void main() { await check(future).throws(); check(globalStore.takeDoRemoveAccountCalls()).isEmpty(); - // no retry-register, poll, server-emoji-data, or register-token requests + // no retry-register, poll, or other follow-up requests check(connection.takeRequests()).isEmpty(); check(connection).isOpen.isFalse(); })); @@ -483,8 +479,6 @@ void main() { as FakeApiConnection); UpdateMachine.debugEnableFetchEmojiData = false; addTearDown(() => UpdateMachine.debugEnableFetchEmojiData = true); - UpdateMachine.debugEnableRegisterNotificationToken = false; - addTearDown(() => UpdateMachine.debugEnableRegisterNotificationToken = true); } void checkLastRequest() { @@ -571,7 +565,6 @@ void main() { })); // TODO test UpdateMachine.load starts polling loop - // TODO test UpdateMachine.load calls registerNotificationToken }); group('UpdateMachine.fetchEmojiData', () { @@ -1160,116 +1153,6 @@ void main() { })); }); - group('UpdateMachine.registerNotificationToken', () { - late UpdateMachine updateMachine; - late FakeApiConnection connection; - - void prepareStore() { - updateMachine = eg.updateMachine(); - connection = updateMachine.store.connection as FakeApiConnection; - } - - void checkLastRequestApns({required String token, required String appid}) { - check(connection.takeRequests()).single.isA() - ..method.equals('POST') - ..url.path.equals('/api/v1/users/me/apns_device_token') - ..bodyFields.deepEquals({'token': token, 'appid': appid}); - } - - void checkLastRequestFcm({required String token}) { - check(connection.takeRequests()).single.isA() - ..method.equals('POST') - ..url.path.equals('/api/v1/users/me/android_gcm_reg_id') - ..bodyFields.deepEquals({'token': token}); - } - - testAndroidIos('token already known', () => awaitFakeAsync((async) async { - // This tests the case where [NotificationService.start] has already - // learned the token before the store is created. - // (This is probably the common case.) - addTearDown(testBinding.reset); - testBinding.firebaseMessagingInitialToken = '012abc'; - testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.zulip.flutter'); - addTearDown(NotificationService.debugReset); - await NotificationService.instance.start(); - - // On store startup, send the token. - prepareStore(); - connection.prepare(json: {}); - await updateMachine.registerNotificationToken(); - if (defaultTargetPlatform == TargetPlatform.android) { - checkLastRequestFcm(token: '012abc'); - } else { - checkLastRequestApns(token: '012abc', appid: 'com.zulip.flutter'); - } - - if (defaultTargetPlatform == TargetPlatform.android) { - // If the token changes, send it again. - testBinding.firebaseMessaging.setToken('456def'); - connection.prepare(json: {}); - async.flushMicrotasks(); - checkLastRequestFcm(token: '456def'); - } - })); - - testAndroidIos('token initially unknown', () => awaitFakeAsync((async) async { - // This tests the case where the store is created while our - // request for the token is still pending. - addTearDown(testBinding.reset); - testBinding.firebaseMessagingInitialToken = '012abc'; - testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.zulip.flutter'); - addTearDown(NotificationService.debugReset); - final startFuture = NotificationService.instance.start(); - - // TODO this test is a bit brittle in its interaction with asynchrony; - // to fix, probably extend TestZulipBinding to control when getToken finishes. - // - // The aim here is to first wait for `store.registerNotificationToken` - // to complete whatever it's going to do; then check no request was made; - // and only after that wait for `NotificationService.start` to finish, - // including its `getToken` call. - - // On store startup, send nothing (because we have nothing to send). - prepareStore(); - await updateMachine.registerNotificationToken(); - check(connection.lastRequest).isNull(); - - // When the token later appears, send it. - connection.prepare(json: {}); - await startFuture; - async.flushMicrotasks(); - if (defaultTargetPlatform == TargetPlatform.android) { - checkLastRequestFcm(token: '012abc'); - } else { - checkLastRequestApns(token: '012abc', appid: 'com.zulip.flutter'); - } - - if (defaultTargetPlatform == TargetPlatform.android) { - // If the token subsequently changes, send it again. - testBinding.firebaseMessaging.setToken('456def'); - connection.prepare(json: {}); - async.flushMicrotasks(); - checkLastRequestFcm(token: '456def'); - } - })); - - test('on iOS, use provided app ID from packageInfo', () => awaitFakeAsync((async) async { - final origTargetPlatform = debugDefaultTargetPlatformOverride; - addTearDown(() => debugDefaultTargetPlatformOverride = origTargetPlatform); - debugDefaultTargetPlatformOverride = TargetPlatform.iOS; - addTearDown(testBinding.reset); - testBinding.firebaseMessagingInitialToken = '012abc'; - testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.example.test'); - addTearDown(NotificationService.debugReset); - await NotificationService.instance.start(); - - prepareStore(); - connection.prepare(json: {}); - await updateMachine.registerNotificationToken(); - checkLastRequestApns(token: '012abc', appid: 'com.example.test'); - })); - }); - group('ZulipVersionData', () { group('fromMalformedServerResponseException', () { test('replace missing feature level with 0', () async { diff --git a/test/notifications/open_test.dart b/test/notifications/open_test.dart index ef034f29d6..649d07d9ce 100644 --- a/test/notifications/open_test.dart +++ b/test/notifications/open_test.dart @@ -10,6 +10,7 @@ import 'package:zulip/host/notifications.dart'; import 'package:zulip/model/database.dart'; import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; +import 'package:zulip/model/push_device.dart'; import 'package:zulip/notifications/open.dart'; import 'package:zulip/notifications/receive.dart'; import 'package:zulip/widgets/app.dart'; @@ -80,6 +81,8 @@ void main() { testBinding.firebaseMessagingInitialToken = '012abc'; addTearDown(NotificationService.debugReset); NotificationService.debugBackgroundIsolateIsLive = false; + PushDeviceManager.debugAutoPause = true; + addTearDown(() => PushDeviceManager.debugAutoPause = false); await NotificationService.instance.start(); } diff --git a/test/notifications/receive_test.dart b/test/notifications/receive_test.dart index 3b7907b1c2..55546dcc26 100644 --- a/test/notifications/receive_test.dart +++ b/test/notifications/receive_test.dart @@ -20,8 +20,7 @@ void main() { // are tested end-to-end in `display_test.dart`, by posting FCM messages // to the respective streams and checking that the right logic then runs. - // The token logic is tested end-to-end in `test/model/store_test.dart` in the - // `UpdateMachine.registerNotificationToken` tests. + // The token logic is tested end-to-end in `test/model/push_device_test.dart`. group('permissions', () { testWidgets('request permission', (tester) async {