push_device [nfc]: Auto-register from PushDeviceManager, vs UpdateMachine

Author: gnprice

lib/model/push_device.dart

@@ -5,7 +5,12 @@ import 'store.dart';
 /// 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});
+  PushDeviceManager({required super.core}) {
+    if (!debugAutoRegisterToken) {
+      return;
+    }
+    registerToken();
+  }
 
   bool _disposed = false;
 
@@ -27,9 +32,6 @@ class PushDeviceManager extends PerAccountStoreBase {
   // TODO(#322) save acked token, to dedupe updating it on the server
   // TODO(#323) track the addFcmToken/etc request, warn if not succeeding
   Future<void> registerToken() async {
-    if (!debugEnableRegisterToken) {
-      return;
-    }
     NotificationService.instance.token.addListener(_registerToken);
     await _registerToken();
   }
@@ -40,22 +42,22 @@ class PushDeviceManager extends PerAccountStoreBase {
     await NotificationService.instance.registerToken(connection);
   }
 
-  /// In debug mode, controls whether [registerToken] should
-  /// have its normal effect.
+  /// In debug mode, controls whether [registerToken] should be called
+  /// immediately in the constructor.
   ///
   /// Outside of debug mode, this is always true and the setter has no effect.
-  static bool get debugEnableRegisterToken {
+  static bool get debugAutoRegisterToken {
     bool result = true;
     assert(() {
-      result = _debugEnableRegisterToken;
+      result = _debugAutoRegisterToken;
       return true;
     }());
     return result;
   }
-  static bool _debugEnableRegisterToken = true;
-  static set debugEnableRegisterToken(bool value) {
+  static bool _debugAutoRegisterToken = true;
+  static set debugAutoRegisterToken(bool value) {
     assert(() {
-      _debugEnableRegisterToken = value;
+      _debugAutoRegisterToken = value;
       return true;
     }());
   }

lib/model/store.dart

@@ -1123,7 +1123,6 @@ class UpdateMachine {
       //   serverEmojiDataUrl are already unsupported at time of writing.)
       unawaited(updateMachine.fetchEmojiData(initialSnapshot.serverEmojiDataUrl!));
     }
-    unawaited(store.pushDevices.registerToken());
     store.presence.start();
     return updateMachine;
   }

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.debugAutoRegisterToken = false;
+      addTearDown(() => PushDeviceManager.debugAutoRegisterToken = true);
       await prepare();
       testBinding.firebaseMessagingInitialToken = '123';
       addTearDown(NotificationService.debugReset);

test/model/push_device_test.dart

@@ -16,10 +16,14 @@ void main() {
   TestZulipBinding.ensureInitialized();
 
   group('registerToken', () {
+    // TODO test registerToken gets called on constructing an instance
+
     late PushDeviceManager model;
     late FakeApiConnection connection;
 
     void prepareStore() {
+      PushDeviceManager.debugAutoRegisterToken = false;
+      addTearDown(() => PushDeviceManager.debugAutoRegisterToken = true);
       final store = eg.store();
       model = store.pushDevices;
       connection = store.connection as FakeApiConnection;

test/model/store_test.dart

@@ -17,10 +17,8 @@ import 'package:zulip/api/route/realm.dart';
 import 'package:zulip/log.dart';
 import 'package:zulip/model/actions.dart';
 import 'package:zulip/model/presence.dart';
-import 'package:zulip/model/push_device.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';
@@ -137,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();
   }));
 
@@ -163,7 +158,7 @@ void main() {
 
     await check(future).throws<AccountNotFoundException>();
     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();
   }));
@@ -182,7 +177,7 @@ void main() {
 
     await check(future).throws<AccountNotFoundException>();
     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();
   }));
@@ -203,7 +198,7 @@ void main() {
 
     await check(future).throws<AccountNotFoundException>();
     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();
   }));
@@ -224,7 +219,7 @@ void main() {
 
     await check(future).throws<AccountNotFoundException>();
     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();
   }));
@@ -246,7 +241,7 @@ void main() {
 
     await check(future).throws<AccountNotFoundException>();
     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();
   }));
@@ -270,7 +265,7 @@ void main() {
 
     await check(future).throws<AccountNotFoundException>();
     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();
   }));
@@ -294,7 +289,7 @@ void main() {
 
     await check(future).throws<AccountNotFoundException>();
     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();
   }));
@@ -484,8 +479,6 @@ void main() {
         as FakeApiConnection);
       UpdateMachine.debugEnableFetchEmojiData = false;
       addTearDown(() => UpdateMachine.debugEnableFetchEmojiData = true);
-      PushDeviceManager.debugEnableRegisterToken = false;
-      addTearDown(() => PushDeviceManager.debugEnableRegisterToken = true);
     }
 
     void checkLastRequest() {
@@ -572,7 +565,6 @@ void main() {
     }));
 
     // TODO test UpdateMachine.load starts polling loop
-    // TODO test UpdateMachine.load calls [PushDeviceManager.registerToken]
   });
 
   group('UpdateMachine.fetchEmojiData', () {

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.debugAutoRegisterToken = false;
+    addTearDown(() => PushDeviceManager.debugAutoRegisterToken = true);
     await NotificationService.instance.start();
   }