login: Support logging out of an account

assets/l10n/app_en.arb

@@ -19,6 +19,10 @@
   "@chooseAccountPageTitle": {
     "description": "Title for ChooseAccountPage"
   },
+  "chooseAccountPageLogOutButton": "Log out",
+  "@chooseAccountPageLogOutButton": {
+    "description": "Label for the 'Log out' button for an account on the choose-account page"
+  },
   "chooseAccountButtonAddAnAccount": "Add an account",
   "@chooseAccountButtonAddAnAccount": {
     "description": "Label for ChooseAccountPage button to add an account"

lib/api/route/notifications.dart

@@ -1,4 +1,3 @@
-
 import '../core.dart';
 
 /// https://zulip.com/api/add-fcm-token
@@ -10,6 +9,15 @@ Future<void> addFcmToken(ApiConnection connection, {
   });
 }
 
+/// https://zulip.com/api/remove-fcm-token
+Future<void> removeFcmToken(ApiConnection connection, {
+  required String token,
+}) {
+  return connection.delete('removeFcmToken', (_) {}, 'users/me/android_gcm_reg_id', {
+    'token': RawParameter(token),
+  });
+}
+
 /// https://zulip.com/api/add-apns-token
 Future<void> addApnsToken(ApiConnection connection, {
   required String token,
@@ -20,3 +28,12 @@ Future<void> addApnsToken(ApiConnection connection, {
     'appid': RawParameter(appid),
   });
 }
+
+/// https://zulip.com/api/remove-apns-token
+Future<void> removeApnsToken(ApiConnection connection, {
+  required String token,
+}) {
+  return connection.delete('removeApnsToken', (_) {}, 'users/me/apns_device_token', {
+    'token': RawParameter(token),
+  });
+}

lib/model/store.dart

@@ -78,6 +78,8 @@ abstract class GlobalStore extends ChangeNotifier {
   }
 
   final Map<int, PerAccountStore> _perAccountStores = {};
+
+  int get debugNumPerAccountStoresLoading => _perAccountStoresLoading.length;
   final Map<int, Future<PerAccountStore>> _perAccountStoresLoading = {};
 
   /// The store's per-account data for the given account, if already loaded.
@@ -144,7 +146,21 @@ abstract class GlobalStore extends ChangeNotifier {
   /// This method should be called only by the implementation of [perAccount].
   /// Other callers interested in per-account data should use [perAccount]
   /// and/or [perAccountSync].
-  Future<PerAccountStore> loadPerAccount(int accountId);
+  Future<PerAccountStore> loadPerAccount(int accountId) async {
+    assert(_accounts.containsKey(accountId));
+    final store = await doLoadPerAccount(accountId);
+    if (!_accounts.containsKey(accountId)) {
+      // [removeAccount] was called during [doLoadPerAccount].
+      store.dispose();
+      throw AccountNotFoundException();
+    }
+    return store;
+  }
+
+  /// Load per-account data for the given account, unconditionally.
+  ///
+  /// This method should be called only by [loadPerAccount].
+  Future<PerAccountStore> doLoadPerAccount(int accountId);
 
   // Just the Iterables, not the actual Map, to avoid clients mutating the map.
   // Mutations should go through the setters/mutators below.
@@ -192,10 +208,26 @@ abstract class GlobalStore extends ChangeNotifier {
   /// Update an account in the underlying data store.
   Future<void> doUpdateAccount(int accountId, AccountsCompanion data);
 
+  /// Remove an account from the store.
+  Future<void> removeAccount(int accountId) async {
+    assert(_accounts.containsKey(accountId));
+    await doRemoveAccount(accountId);
+    if (!_accounts.containsKey(accountId)) return; // Already removed.
+    _accounts.remove(accountId);
+    _perAccountStores.remove(accountId)?.dispose();
+    unawaited(_perAccountStoresLoading.remove(accountId));
+    notifyListeners();
+  }
+
+  /// Remove an account from the underlying data store.
+  Future<void> doRemoveAccount(int accountId);
+
   @override
   String toString() => '${objectRuntimeType(this, 'GlobalStore')}#${shortHash(this)}';
 }
 
+class AccountNotFoundException implements Exception {}
+
 /// Store for the user's data for a given Zulip account.
 ///
 /// This should always have a consistent snapshot of the state on the server,
@@ -303,6 +335,14 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
   final GlobalStore _globalStore;
   final ApiConnection connection; // TODO(#135): update zulipFeatureLevel with events
 
+  UpdateMachine? get updateMachine => _updateMachine;
+  UpdateMachine? _updateMachine;
+  set updateMachine(UpdateMachine? value) {
+    assert(_updateMachine == null);
+    assert(value != null);
+    _updateMachine = value;
+  }
+
   bool get isLoading => _isLoading;
   bool _isLoading = false;
   @visibleForTesting
@@ -361,6 +401,10 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
   // Data attached to the self-account on the realm.
 
   final int accountId;
+
+  /// The [Account] this store belongs to.
+  ///
+  /// Will throw if called after [dispose] has been called.
   Account get account => _globalStore.getAccount(accountId)!;
 
   /// Always equal to `account.userId`.
@@ -439,16 +483,24 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
     autocompleteViewManager.reassemble();
   }
 
+  bool _disposed = false;
+
   @override
   void dispose() {
+    assert(!_disposed);
     recentDmConversationsView.dispose();
     unreads.dispose();
     _messages.dispose();
     typingStatus.dispose();
+    updateMachine?.dispose();
+    connection.close();
+    _disposed = true;
     super.dispose();
   }
 
   Future<void> handleEvent(Event event) async {
+    assert(!_disposed);
+
     switch (event) {
       case HeartbeatEvent():
         assert(debugLog("server event: heartbeat"));
@@ -590,6 +642,8 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
   }
 
   Future<void> sendMessage({required MessageDestination destination, required String content}) {
+    assert(!_disposed);
+
     // TODO implement outbox; see design at
     //   https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M3881.20Sending.20outbox.20messages.20is.20fraught.20with.20issues/near/1405739
     return _apiSendMessage(connection,
@@ -682,7 +736,7 @@ class LiveGlobalStore extends GlobalStore {
   final AppDatabase _db;
 
   @override
-  Future<PerAccountStore> loadPerAccount(int accountId) async {
+  Future<PerAccountStore> doLoadPerAccount(int accountId) async {
     final updateMachine = await UpdateMachine.load(this, accountId);
     return updateMachine.store;
   }
@@ -708,6 +762,14 @@ class LiveGlobalStore extends GlobalStore {
     assert(rowsAffected == 1);
   }
 
+  @override
+  Future<void> doRemoveAccount(int accountId) async {
+    final rowsAffected = await (_db.delete(_db.accounts)
+      ..where((a) => a.id.equals(accountId))
+    ).go();
+    assert(rowsAffected == 1);
+  }
+
   @override
   String toString() => '${objectRuntimeType(this, 'LiveGlobalStore')}#${shortHash(this)}';
 }
@@ -722,7 +784,9 @@ class UpdateMachine {
          // case of unauthenticated access to a web-public realm.  We authenticated.
          throw Exception("bad initial snapshot: missing queueId");
        })(),
-       lastEventId = initialSnapshot.lastEventId;
+       lastEventId = initialSnapshot.lastEventId {
+    store.updateMachine = this;
+  }
 
   /// Load the user's data from the server, and start an event queue going.
   ///
@@ -772,6 +836,8 @@ class UpdateMachine {
   final String queueId;
   int lastEventId;
 
+  bool _disposed = false;
+
   static Future<InitialSnapshot> _registerQueueWithRetry(
       ApiConnection connection) async {
     BackoffMachine? backoffMachine;
@@ -850,8 +916,11 @@ class UpdateMachine {
     BackoffMachine? backoffMachine;
 
     while (true) {
+      if (_disposed) return;
+
       if (_debugLoopSignal != null) {
         await _debugLoopSignal!.future;
+        if (_disposed) return;
         assert(() {
           _debugLoopSignal = Completer();
           return true;
@@ -862,13 +931,16 @@ class UpdateMachine {
       try {
         result = await getEvents(store.connection,
           queueId: queueId, lastEventId: lastEventId);
+        if (_disposed) return;
       } catch (e) {
+        if (_disposed) return;
+
         store.isLoading = true;
         switch (e) {
           case ZulipApiException(code: 'BAD_EVENT_QUEUE_ID'):
             assert(debugLog('Lost event queue for $store.  Replacing…'));
+            // This disposes the store, which disposes this update machine.
             await store._globalStore._reloadPerAccount(store.accountId);
-            dispose();
             debugLog('… Event queue replaced.');
             return;
 
@@ -911,6 +983,7 @@ class UpdateMachine {
       final events = result.events;
       for (final event in events) {
         await store.handleEvent(event);
+        if (_disposed) return;
       }
       if (events.isNotEmpty) {
         lastEventId = events.last.id;
@@ -926,6 +999,7 @@ class UpdateMachine {
   // 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> registerNotificationToken() async {
+    assert(!_disposed);
     if (!debugEnableRegisterNotificationToken) {
       return;
     }
@@ -939,8 +1013,18 @@ class UpdateMachine {
     await NotificationService.registerToken(store.connection, token: token);
   }
 
-  void dispose() { // TODO abort long-poll and close ApiConnection
+  /// 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.
+  ///
+  /// To abort polling mid-request, [store]'s [PerAccountStore.connection]
+  /// needs to be closed using [ApiConnection.close], which causes in-progress
+  /// requests to error. [PerAccountStore.dispose] does that.
+  void dispose() {
+    assert(!_disposed);
     NotificationService.instance.token.removeListener(_registerNotificationToken);
+    _disposed = true;
   }
 
   /// In debug mode, controls whether [fetchEmojiData] should

lib/notifications/receive.dart

@@ -159,6 +159,22 @@ class NotificationService {
     }
   }
 
+  static Future<void> unregisterToken(ApiConnection connection, {required String token}) async {
+    switch (defaultTargetPlatform) {
+      case TargetPlatform.android:
+        await removeFcmToken(connection, token: token);
+
+      case TargetPlatform.iOS:
+        await removeApnsToken(connection, token: token);
+
+      case TargetPlatform.linux:
+      case TargetPlatform.macOS:
+      case TargetPlatform.windows:
+      case TargetPlatform.fuchsia:
+        assert(false);
+    }
+  }
+
   static void _onForegroundMessage(FirebaseRemoteMessage message) {
     assert(debugLog("notif message: ${message.data}"));
     _onRemoteMessage(message);

lib/widgets/app.dart

@@ -8,6 +8,8 @@ import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import '../log.dart';
 import '../model/localizations.dart';
 import '../model/narrow.dart';
+import '../model/store.dart';
+import '../notifications/receive.dart';
 import 'about_zulip.dart';
 import 'app_bar.dart';
 import 'dialog.dart';
@@ -221,15 +223,64 @@ class ChooseAccountPage extends StatelessWidget {
     required Widget title,
     Widget? subtitle,
   }) {
+    final designVariables = DesignVariables.of(context);
+    final zulipLocalizations = ZulipLocalizations.of(context);
     return Card(
       clipBehavior: Clip.hardEdge,
       child: ListTile(
         title: title,
         subtitle: subtitle,
+        trailing: PopupMenuButton<AccountItemOverflowMenuItem>(
+          iconColor: designVariables.icon,
+          itemBuilder: (context) => [
+            PopupMenuItem(
+              value: AccountItemOverflowMenuItem.logOut,
+              child: Text(zulipLocalizations.chooseAccountPageLogOutButton)),
+          ],
+          onSelected: (item) async {
+            switch (item) {
+              case AccountItemOverflowMenuItem.logOut: {
+                unawaited(_logOutAccount(context, accountId));
+              }
+            }
+          }),
+        // The default trailing padding with M3 is 24px. Decrease by 12 because
+        // PopupMenuButton adds 12px padding on all sides of the "…" icon.
+        contentPadding: const EdgeInsetsDirectional.only(start: 16, end: 12),
         onTap: () => Navigator.push(context,
           HomePage.buildRoute(accountId: accountId))));
   }
 
+  Future<void> _logOutAccount(BuildContext context, int accountId) async {
+    final globalStore = GlobalStoreWidget.of(context);
+
+    final account = globalStore.getAccount(accountId);
+    if (account == null) return; // TODO(log)
+
+    // Unawaited, to not block removing the account on this request.
+    unawaited(_unregisterToken(globalStore, account));
+
+    await globalStore.removeAccount(accountId);
+  }
+
+  Future<void> _unregisterToken(GlobalStore globalStore, Account account) async {
+    // TODO(#322) use actual acked push token; until #322, this is just null.
+    final token = account.ackedPushToken
+      // Try the current token as a fallback; maybe the server has registered
+      // it and we just haven't recorded that fact in the client.
+      ?? NotificationService.instance.token.value;
+    if (token == null) return;
+
+    final connection = globalStore.apiConnectionFromAccount(account);
+    try {
+      await NotificationService.unregisterToken(connection, token: token);
+    } catch (e) {
+      // TODO retry? handle failures?
+    } finally {
+      connection.close();
+    }
+  }
+
   @override
   Widget build(BuildContext context) {
     final zulipLocalizations = ZulipLocalizations.of(context);
@@ -286,6 +337,8 @@ class ChooseAccountPageOverflowButton extends StatelessWidget {
   }
 }
 
+enum AccountItemOverflowMenuItem { logOut }
+
 class HomePage extends StatelessWidget {
   const HomePage({super.key});
 

lib/widgets/page.dart

@@ -40,6 +40,7 @@ mixin AccountPageRouteMixin<T extends Object?> on PageRoute<T> {
     return PerAccountStoreWidget(
       accountId: accountId,
       placeholder: const LoadingPlaceholderPage(),
+      routeToRemoveOnLogout: this,
       child: super.buildPage(context, animation, secondaryAnimation));
   }
 }