diff --git a/lib/model/store.dart b/lib/model/store.dart index 1aed416810..0db97feabe 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -602,14 +602,12 @@ class UpdateMachine { /// In the future this might load an old snapshot from local storage first. 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); final stopwatch = Stopwatch()..start(); - final initialSnapshot = await registerQueue(connection); // TODO retry + final initialSnapshot = await _registerQueueWithRetry(connection); final t = (stopwatch..stop()).elapsed; - // TODO log the time better - if (kDebugMode) print("initial fetch time: ${t.inMilliseconds}ms"); + assert(debugLog("initial fetch time: ${t.inMilliseconds}ms")); final store = PerAccountStore.fromInitialSnapshot( globalStore: globalStore, @@ -630,6 +628,22 @@ class UpdateMachine { final String queueId; int lastEventId; + static Future _registerQueueWithRetry( + ApiConnection connection) async { + BackoffMachine? backoffMachine; + while (true) { + try { + return await registerQueue(connection); + } catch (e) { + assert(debugLog('Error fetching initial snapshot: $e\n' + 'Backing off, then will retry…')); + // TODO tell user if initial-fetch errors persist, or look non-transient + await (backoffMachine ??= BackoffMachine()).wait(); + assert(debugLog('… Backoff wait complete, retrying initial fetch.')); + } + } + } + Completer? _debugLoopSignal; /// In debug mode, causes the polling loop to pause before the next @@ -705,6 +719,26 @@ 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 @@ -713,6 +747,9 @@ class UpdateMachine { // TODO(#322) save acked token, to dedupe updating it on the server // TODO(#323) track the registerFcmToken/etc request, warn if not succeeding Future registerNotificationToken() async { + if (!debugEnableRegisterNotificationToken) { + return; + } NotificationService.instance.token.addListener(_registerNotificationToken); await _registerNotificationToken(); } diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index cc33f9bbbd..801320cb57 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -3,6 +3,28 @@ import 'dart:ui'; import 'package:checks/checks.dart'; import 'package:zulip/api/model/model.dart'; +extension UserChecks on Subject { + Subject get userId => has((x) => x.userId, 'userId'); + Subject get deliveryEmailStaleDoNotUse => has((x) => x.deliveryEmailStaleDoNotUse, 'deliveryEmailStaleDoNotUse'); + Subject get email => has((x) => x.email, 'email'); + Subject get fullName => has((x) => x.fullName, 'fullName'); + Subject get dateJoined => has((x) => x.dateJoined, 'dateJoined'); + Subject get isActive => has((x) => x.isActive, 'isActive'); + Subject get isOwner => has((x) => x.isOwner, 'isOwner'); + Subject get isAdmin => has((x) => x.isAdmin, 'isAdmin'); + Subject get isGuest => has((x) => x.isGuest, 'isGuest'); + Subject get isBillingAdmin => has((x) => x.isBillingAdmin, 'isBillingAdmin'); + Subject get isBot => has((x) => x.isBot, 'isBot'); + Subject get botType => has((x) => x.botType, 'botType'); + Subject get botOwnerId => has((x) => x.botOwnerId, 'botOwnerId'); + Subject get role => has((x) => x.role, 'role'); + Subject get timezone => has((x) => x.timezone, 'timezone'); + Subject get avatarUrl => has((x) => x.avatarUrl, 'avatarUrl'); + Subject get avatarVersion => has((x) => x.avatarVersion, 'avatarVersion'); + Subject?> get profileData => has((x) => x.profileData, 'profileData'); + Subject get isSystemBot => has((x) => x.isSystemBot, 'isSystemBot'); +} + extension ZulipStreamChecks on Subject { Subject get canRemoveSubscribersGroup => has((e) => e.canRemoveSubscribersGroup, 'canRemoveSubscribersGroup'); } diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 38eff362a8..f113b46a6e 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -12,6 +12,7 @@ import 'package:zulip/model/store.dart'; import 'package:zulip/notifications.dart'; import '../api/fake_api.dart'; +import '../api/model/model_checks.dart'; import '../example_data.dart' as eg; import '../fake_async.dart'; import '../stdlib_checks.dart'; @@ -135,6 +136,76 @@ void main() { }); }); + group('UpdateMachine.load', () { + late TestGlobalStore globalStore; + late FakeApiConnection connection; + + Future prepareStore() async { + globalStore = TestGlobalStore(accounts: []); + await globalStore.insertAccount(eg.selfAccount.toCompanion(false)); + connection = (globalStore.apiConnectionFromAccount(eg.selfAccount) + as FakeApiConnection); + UpdateMachine.debugEnableRegisterNotificationToken = false; + addTearDown(() => UpdateMachine.debugEnableRegisterNotificationToken = true); + } + + void checkLastRequest() { + check(connection.takeLastRequest()).isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/register'); + } + + test('smoke', () => awaitFakeAsync((async) async { + await prepareStore(); + final users = [eg.selfUser, eg.otherUser]; + connection.prepare(json: eg.initialSnapshot(realmUsers: users).toJson()); + final updateMachine = await UpdateMachine.load( + globalStore, eg.selfAccount.id); + updateMachine.debugPauseLoop(); + + // TODO UpdateMachine.debugPauseLoop is too late to prevent first poll attempt; + // the polling retry catches the resulting NetworkException from lack of + // `connection.prepare`, so that doesn't fail the test, but it does + // clobber the recorded registerQueue request so we can't check it. + // checkLastRequest(); + + check(updateMachine.store.users.values).unorderedMatches( + users.map((expected) => (it) => it.fullName.equals(expected.fullName))); + })); + + test('retries registerQueue on NetworkError', () => awaitFakeAsync((async) async { + await prepareStore(); + + // Try to load, inducing an error in the request. + connection.prepare(exception: Exception('failed')); + final future = UpdateMachine.load(globalStore, eg.selfAccount.id); + bool complete = false; + future.whenComplete(() => complete = true); + async.flushMicrotasks(); + checkLastRequest(); + check(complete).isFalse(); + + // 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. + final users = [eg.selfUser, eg.otherUser]; + connection.prepare(json: eg.initialSnapshot(realmUsers: users).toJson()); + final updateMachine = await future; + updateMachine.debugPauseLoop(); + check(complete).isTrue(); + // checkLastRequest(); TODO UpdateMachine.debugPauseLoop was too late; see comment above + check(updateMachine.store.users.values).unorderedMatches( + users.map((expected) => (it) => it.fullName.equals(expected.fullName))); + })); + + // TODO test UpdateMachine.load starts polling loop + // TODO test UpdateMachine.load calls registerNotificationToken + }); + group('UpdateMachine.poll', () { late TestGlobalStore globalStore; late UpdateMachine updateMachine;