From 0445e187dfe320895acabd33d9adc1971aa783c5 Mon Sep 17 00:00:00 2001
From: Nik Clayton <nik@ngo.org.uk>
Date: Sun, 10 Mar 2024 12:25:12 +0100
Subject: [PATCH] fix: Ensure logging out accounts completes (#515)

The account logout process could fail due to API exceptions; network
errors for example, or if the user had already revoked the app's token
for that account. This would prevent the rest of the logout process
(cleaning database, etc) from completing.

Fix this by ignoring network errors during the logout process, and
always cleaning up account content in the database.

Fix a related issue where a deleted account might be recreated in a
partial state if the account's visible position was saved after it was
deleted. The recreated account couldn't do anything as it had no tokens,
but is very confusing.
---
 .../java/app/pachli/usecase/LogoutUsecase.kt  | 15 ++++++++-----
 .../pachli/core/accounts/AccountManager.kt    | 21 ++++++++++++++++---
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/app/src/main/java/app/pachli/usecase/LogoutUsecase.kt b/app/src/main/java/app/pachli/usecase/LogoutUsecase.kt
index bd4483ab9..fc0dae630 100644
--- a/app/src/main/java/app/pachli/usecase/LogoutUsecase.kt
+++ b/app/src/main/java/app/pachli/usecase/LogoutUsecase.kt
@@ -14,6 +14,7 @@ import app.pachli.core.network.retrofit.MastodonApi
 import app.pachli.util.removeShortcut
 import dagger.hilt.android.qualifiers.ApplicationContext
 import javax.inject.Inject
+import timber.log.Timber
 
 class LogoutUsecase @Inject constructor(
     @ApplicationContext private val context: Context,
@@ -37,11 +38,15 @@ class LogoutUsecase @Inject constructor(
             val clientId = activeAccount.clientId
             val clientSecret = activeAccount.clientSecret
             if (clientId != null && clientSecret != null) {
-                api.revokeOAuthToken(
-                    clientId = clientId,
-                    clientSecret = clientSecret,
-                    token = activeAccount.accessToken,
-                )
+                try {
+                    api.revokeOAuthToken(
+                        clientId = clientId,
+                        clientSecret = clientSecret,
+                        token = activeAccount.accessToken,
+                    )
+                } catch (e: Exception) {
+                    Timber.e(e, "Could not revoke OAuth token, continuing")
+                }
             }
 
             // disable push notifications
diff --git a/core/accounts/src/main/kotlin/app/pachli/core/accounts/AccountManager.kt b/core/accounts/src/main/kotlin/app/pachli/core/accounts/AccountManager.kt
index 02dabce97..76ccf94e3 100644
--- a/core/accounts/src/main/kotlin/app/pachli/core/accounts/AccountManager.kt
+++ b/core/accounts/src/main/kotlin/app/pachli/core/accounts/AccountManager.kt
@@ -130,10 +130,25 @@ class AccountManager @Inject constructor(
      * @param account the account to save
      */
     fun saveAccount(account: AccountEntity) {
-        if (account.id != 0L) {
-            Timber.d("saveAccount: saving account with id %d", account.id)
-            accountDao.insertOrReplace(account)
+        if (account.id == 0L) {
+            Timber.e("Trying to save account with ID = 0, ignoring")
+            return
         }
+
+        // Work around saveAccount() being called after account deletion
+        // For example:
+        // - Have two accounts, A and B, signed in with A, looking at home timeline for A
+        // - Log out of A. This triggers deletion of account A from the database
+        // - Shortly afterwards the timeline activity/fragment ends, and it tries to save
+        //   the visible ID back to the database, which creates the AccountEntity record
+        //   that was just deleted, but in a partial state.
+        if (accounts.find { it.id == account.id } == null) {
+            Timber.e("Trying to save account with ID = %d which does not exist, ignoring", account.id)
+            return
+        }
+
+        Timber.d("saveAccount: saving account with id %d", account.id)
+        accountDao.insertOrReplace(account)
     }
 
     /**