From 87c4ffb7a7aeea565207e85655b53795bd5b22e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= <sebastian@mobanisto.de>
Date: Wed, 24 Aug 2022 20:01:22 +0200
Subject: [PATCH] Move db check to StatusManager

---
 .../android/MailboxNotificationManager.kt     |   4 +-
 .../mailbox/android/StatusManager.kt          |  24 +++-
 .../briarproject/mailbox/android/UiUtils.kt   |  15 ---
 .../mailbox/android/ui/MailboxViewModel.kt    |   8 --
 .../mailbox/android/ui/MainActivity.kt        | 103 +++++++-----------
 .../mailbox/android/ui/StartupFragment.kt     |   7 +-
 6 files changed, 67 insertions(+), 94 deletions(-)

diff --git a/mailbox-android/src/main/java/org/briarproject/mailbox/android/MailboxNotificationManager.kt b/mailbox-android/src/main/java/org/briarproject/mailbox/android/MailboxNotificationManager.kt
index 3e97c4c0..833fa852 100644
--- a/mailbox-android/src/main/java/org/briarproject/mailbox/android/MailboxNotificationManager.kt
+++ b/mailbox-android/src/main/java/org/briarproject/mailbox/android/MailboxNotificationManager.kt
@@ -39,12 +39,14 @@ import org.briarproject.mailbox.R
 import org.briarproject.mailbox.android.StatusManager.ErrorClockSkew
 import org.briarproject.mailbox.android.StatusManager.ErrorNoNetwork
 import org.briarproject.mailbox.android.StatusManager.MailboxAppState
+import org.briarproject.mailbox.android.StatusManager.NeedOnboarding
 import org.briarproject.mailbox.android.StatusManager.NotStarted
 import org.briarproject.mailbox.android.StatusManager.StartedSettingUp
 import org.briarproject.mailbox.android.StatusManager.StartedSetupComplete
 import org.briarproject.mailbox.android.StatusManager.Starting
 import org.briarproject.mailbox.android.StatusManager.Stopped
 import org.briarproject.mailbox.android.StatusManager.Stopping
+import org.briarproject.mailbox.android.StatusManager.Undecided
 import org.briarproject.mailbox.android.StatusManager.Wiping
 import org.briarproject.mailbox.android.ui.MainActivity
 import javax.inject.Inject
@@ -113,7 +115,7 @@ class MailboxNotificationManager @Inject constructor(
                     setContentTitle(ctx.getString(R.string.notification_mailbox_title_offline))
                     setContentText(ctx.getString(R.string.notification_mailbox_content_clock_skew))
                 }
-                NotStarted, Stopping, Stopped, Wiping ->
+                Undecided, NeedOnboarding, NotStarted, Stopping, Stopped, Wiping ->
                     error("No notifications when lifecycle not running")
             }
             setSmallIcon(R.drawable.ic_notification_foreground)
diff --git a/mailbox-android/src/main/java/org/briarproject/mailbox/android/StatusManager.kt b/mailbox-android/src/main/java/org/briarproject/mailbox/android/StatusManager.kt
index 29df60ca..831763a2 100644
--- a/mailbox-android/src/main/java/org/briarproject/mailbox/android/StatusManager.kt
+++ b/mailbox-android/src/main/java/org/briarproject/mailbox/android/StatusManager.kt
@@ -31,6 +31,7 @@ import kotlinx.coroutines.flow.SharingStarted.Companion.Lazily
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
+import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.stateIn
@@ -42,6 +43,7 @@ import org.briarproject.mailbox.core.setup.SetupComplete
 import org.briarproject.mailbox.core.setup.SetupManager
 import org.briarproject.mailbox.core.tor.TorPlugin
 import org.briarproject.mailbox.core.tor.TorState
+import org.slf4j.LoggerFactory
 import javax.inject.Inject
 import kotlin.math.min
 
@@ -54,6 +56,11 @@ class StatusManager @Inject constructor(
     torPlugin: TorPlugin,
 ) {
 
+    companion object {
+        private val LOG = LoggerFactory.getLogger(StatusManager::class.java)
+    }
+
+    private val haveDb = flow { emit(setupManager.hasDb) }
     private val lifecycleState: StateFlow<LifecycleState> =
         lifecycleManager.lifecycleStateFlow
     private val torPluginState: StateFlow<TorState> = torPlugin.state
@@ -63,6 +70,8 @@ class StatusManager @Inject constructor(
      * Possible values for [appState]
      */
     sealed class MailboxAppState
+    object Undecided : MailboxAppState()
+    object NeedOnboarding : MailboxAppState()
     object NotStarted : MailboxAppState()
     data class Starting(val status: String) : MailboxAppState()
     data class StartedSettingUp(val qrCode: Bitmap) : MailboxAppState()
@@ -73,12 +82,16 @@ class StatusManager @Inject constructor(
     object Stopping : MailboxAppState()
     object Stopped : MailboxAppState()
 
+    private val statesWithoutNotification =
+        setOf(Undecided, NeedOnboarding, NotStarted, Wiping, Stopping, Stopped)
+
     @Suppress("OPT_IN_USAGE")
     val appState: Flow<MailboxAppState> = combine(
-        lifecycleState, torPluginState, setupComplete
-    ) { ls, ts, sc ->
+        haveDb, lifecycleState, torPluginState, setupComplete
+    ) { db, ls, ts, sc ->
+        LOG.info("combining: $db $ls ${ts.javaClass.simpleName} $sc")
         when {
-            ls == LifecycleState.NOT_STARTED -> NotStarted
+            ls == LifecycleState.NOT_STARTED -> if (!db) NeedOnboarding else NotStarted
             ls == LifecycleState.WIPING -> Wiping
             ls == LifecycleState.STOPPING -> Stopping
             ls == LifecycleState.STOPPED -> Stopped
@@ -109,10 +122,11 @@ class StatusManager @Inject constructor(
     }.flowOn(Dispatchers.IO)
         .distinctUntilChanged()
         .onEach { state ->
-            if (state != NotStarted && state != Wiping && state != Stopping && state != Stopped)
+            if (!statesWithoutNotification.contains(state)) {
                 notificationManager.onMailboxAppStateChanged(state)
+            }
         }
-        .stateIn(GlobalScope, Lazily, NotStarted)
+        .stateIn(GlobalScope, Lazily, Undecided)
 
     private fun getString(@StringRes resId: Int, vararg formatArgs: Any?): String {
         return context.getString(resId, *formatArgs)
diff --git a/mailbox-android/src/main/java/org/briarproject/mailbox/android/UiUtils.kt b/mailbox-android/src/main/java/org/briarproject/mailbox/android/UiUtils.kt
index 714bc45a..d2ab020f 100644
--- a/mailbox-android/src/main/java/org/briarproject/mailbox/android/UiUtils.kt
+++ b/mailbox-android/src/main/java/org/briarproject/mailbox/android/UiUtils.kt
@@ -29,9 +29,6 @@ import android.text.format.DateUtils.FORMAT_SHOW_DATE
 import android.text.format.DateUtils.WEEK_IN_MILLIS
 import android.text.format.DateUtils.getRelativeDateTimeString
 import android.text.format.DateUtils.getRelativeTimeSpanString
-import androidx.lifecycle.LifecycleOwner
-import androidx.lifecycle.LiveData
-import androidx.lifecycle.Observer
 import org.briarproject.mailbox.R
 
 object UiUtils {
@@ -54,16 +51,4 @@ object UiUtils {
             else -> getRelativeTimeSpanString(time, now, MIN_DATE_RESOLUTION, flags).toString()
         }
     }
-
-    fun <T> LiveData<T>.observeOnce(lifecycleOwner: LifecycleOwner, observer: Observer<T>) {
-        observe(
-            lifecycleOwner,
-            object : Observer<T> {
-                override fun onChanged(t: T?) {
-                    observer.onChanged(t)
-                    removeObserver(this)
-                }
-            }
-        )
-    }
 }
diff --git a/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/MailboxViewModel.kt b/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/MailboxViewModel.kt
index c9a1aabe..c0bcb4d4 100644
--- a/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/MailboxViewModel.kt
+++ b/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/MailboxViewModel.kt
@@ -28,9 +28,7 @@ import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.asLiveData
 import androidx.lifecycle.distinctUntilChanged
 import dagger.hilt.android.lifecycle.HiltViewModel
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.StateFlow
-import kotlinx.coroutines.withContext
 import org.briarproject.android.dontkillmelib.DozeHelper
 import org.briarproject.mailbox.android.MailboxPreferences
 import org.briarproject.mailbox.android.MailboxService
@@ -38,7 +36,6 @@ import org.briarproject.mailbox.android.StatusManager
 import org.briarproject.mailbox.core.lifecycle.LifecycleManager
 import org.briarproject.mailbox.core.lifecycle.LifecycleManager.LifecycleState
 import org.briarproject.mailbox.core.settings.MetadataManager
-import org.briarproject.mailbox.core.setup.SetupManager
 import org.briarproject.mailbox.core.system.AndroidExecutor
 import org.briarproject.mailbox.core.system.DozeWatchdog
 import org.briarproject.mailbox.core.util.LogUtils.info
@@ -51,7 +48,6 @@ class MailboxViewModel @Inject constructor(
     private val dozeHelper: DozeHelper,
     private val dozeWatchdog: DozeWatchdog,
     private val lifecycleManager: LifecycleManager,
-    private val setupManager: SetupManager,
     statusManager: StatusManager,
     metadataManager: MetadataManager,
     private val mailboxPreferences: MailboxPreferences,
@@ -85,10 +81,6 @@ class MailboxViewModel @Inject constructor(
 
     val lifecycleState: StateFlow<LifecycleState> = lifecycleManager.lifecycleStateFlow
 
-    suspend fun hasDb(): Boolean = withContext(Dispatchers.IO) {
-        setupManager.hasDb
-    }
-
     val appState = statusManager.appState
 
     val lastAccess: LiveData<Long> = metadataManager.ownerConnectionTime.asLiveData()
diff --git a/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/MainActivity.kt b/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/MainActivity.kt
index 2c20e839..1507e0b7 100644
--- a/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/MainActivity.kt
+++ b/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/MainActivity.kt
@@ -24,13 +24,9 @@ import android.os.Bundle
 import androidx.activity.viewModels
 import androidx.appcompat.app.AlertDialog
 import androidx.appcompat.app.AppCompatActivity
-import androidx.lifecycle.Lifecycle.State.DESTROYED
-import androidx.lifecycle.MutableLiveData
-import androidx.lifecycle.lifecycleScope
 import androidx.navigation.NavController
 import androidx.navigation.fragment.NavHostFragment
 import dagger.hilt.android.AndroidEntryPoint
-import kotlinx.coroutines.launch
 import org.briarproject.android.dontkillmelib.DozeUtils.needsDozeWhitelisting
 import org.briarproject.mailbox.NavMainDirections.actionGlobalClockSkewFragment
 import org.briarproject.mailbox.NavMainDirections.actionGlobalDoNotKillMeFragment
@@ -46,14 +42,15 @@ import org.briarproject.mailbox.R
 import org.briarproject.mailbox.android.StatusManager.ErrorClockSkew
 import org.briarproject.mailbox.android.StatusManager.ErrorNoNetwork
 import org.briarproject.mailbox.android.StatusManager.MailboxAppState
+import org.briarproject.mailbox.android.StatusManager.NeedOnboarding
 import org.briarproject.mailbox.android.StatusManager.NotStarted
 import org.briarproject.mailbox.android.StatusManager.StartedSettingUp
 import org.briarproject.mailbox.android.StatusManager.StartedSetupComplete
 import org.briarproject.mailbox.android.StatusManager.Starting
 import org.briarproject.mailbox.android.StatusManager.Stopped
 import org.briarproject.mailbox.android.StatusManager.Stopping
+import org.briarproject.mailbox.android.StatusManager.Undecided
 import org.briarproject.mailbox.android.StatusManager.Wiping
-import org.briarproject.mailbox.android.UiUtils.observeOnce
 import org.briarproject.mailbox.core.lifecycle.LifecycleManager.LifecycleState.NOT_STARTED
 import org.briarproject.mailbox.core.util.LogUtils.info
 import org.slf4j.LoggerFactory.getLogger
@@ -74,6 +71,8 @@ class MainActivity : AppCompatActivity() {
         navHostFragment.navController
     }
 
+    var hadBeenStartedOnSave = false
+
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         LOG.info("onCreate()")
@@ -90,33 +89,50 @@ class MainActivity : AppCompatActivity() {
         }
 
         viewModel.doNotKillComplete.observe(this) { complete ->
-            if (complete && nav.currentDestination?.id == R.id.doNotKillMeFragment) nav.navigate(
-                actionGlobalStartupFragment()
-            )
+            if (complete && nav.currentDestination?.id == R.id.doNotKillMeFragment) {
+                nav.navigate(actionGlobalStartupFragment())
+            }
         }
 
         LOG.info { "do we have a saved instance state? " + (savedInstanceState != null) }
+        hadBeenStartedOnSave =
+            savedInstanceState?.getBoolean(BUNDLE_LIFECYCLE_HAS_STARTED) ?: false
 
-        val haveDb = MutableLiveData<Boolean>()
-
-        haveDb.observeOnce(this) { hasDb ->
-            val hadBeenStartedOnSave =
-                savedInstanceState?.getBoolean(BUNDLE_LIFECYCLE_HAS_STARTED) ?: false
-            LOG.info { "do we have a db? $hasDb, had been started on save: $hadBeenStartedOnSave" }
-            onDbChecked(hasDb, hadBeenStartedOnSave)
-            launchAndRepeatWhileStarted {
-                viewModel.appState.collect { onAppStateChanged(it) }
-            }
-        }
-
-        lifecycleScope.launch {
-            haveDb.value = viewModel.hasDb()
+        launchAndRepeatWhileStarted {
+            viewModel.appState.collect { onAppStateChanged(it) }
         }
     }
 
     private fun onAppStateChanged(state: MailboxAppState) {
+        LOG.info("state: ${state.javaClass.simpleName}")
+        // Catch the situation where we come back to the activity after a remote wipe has happened
+        // while the app was in the background and gets restored from the recent app list after
+        // wiping and stopping has already completed.
+        // In this case onSaveInstanceState() has written true to the bundle but there's no db
+        // any longer.
+        if (hadBeenStartedOnSave && state == NeedOnboarding) {
+            finish()
+            startActivity(Intent(this, WipeCompleteActivity::class.java))
+            return
+        }
         when (state) {
-            NotStarted -> {} // do not navigate anywhere yet
+            Undecided -> {} // Do nothing yet
+            NeedOnboarding -> if (nav.currentDestination?.id == R.id.initFragment)
+                nav.navigate(actionGlobalOnboardingContainer())
+            NotStarted -> if (viewModel.needToShowDoNotKillMeFragment) {
+                // Consult whether any condition changed that requires us to show the do-not-kill
+                // fragment (again).
+                nav.navigate(actionGlobalDoNotKillMeFragment())
+            } else {
+                // We have a db and don't need to show the do-not-kill fragment, let's start
+                // the lifecycle.
+                nav.navigate(actionGlobalStartupFragment())
+            }
+            // It is important to navigate here with from various fragments. The normal case is
+            // that we come from the init fragment, do-not-kill fragment or the onboarding fragment.
+            // However, when the service got killed and the app has been restored with a different
+            // UI state such as the qr code screen or the status screen, then we also want to
+            // navigate to the startup fragment.
             is Starting -> if (nav.currentDestination?.id != R.id.startupFragment)
                 nav.navigate(actionGlobalStartupFragment())
             is StartedSettingUp -> if (nav.currentDestination?.id != R.id.qrCodeFragment)
@@ -139,47 +155,6 @@ class MainActivity : AppCompatActivity() {
         }
     }
 
-    private fun onDbChecked(hasDb: Boolean, hadBeenStartedOnSave: Boolean) {
-        if (lifecycle.currentState == DESTROYED) {
-            return
-        }
-        // Catch the situation where we come back to the activity after a remote wipe has happened
-        // while the app was in the background and gets restored from the recent app list after
-        // wiping and stopping has already completed.
-        // In this case onSaveInstanceState() has written true to the bundle but there's no db
-        // any longer.
-        if (!hasDb && hadBeenStartedOnSave) {
-            finish()
-            startActivity(Intent(this, WipeCompleteActivity::class.java))
-            return
-        }
-
-        // It is important to navigate here with and without a saved instance state. The normal
-        // case is that we do not have a saved instance state (when the app has just been started).
-        // However, when the service got killed and the app has been restored with a different UI
-        // state such as the qr code screen or the status screen, then we want to check the
-        // situation and navigate accordingly. Another corner case is when the device gets rotated
-        // very quickly during startup, where we end up here with a saved instance state but no
-        // db yet.
-        if (!hasDb) {
-            // Otherwise, if we don't have a db yet, we need to move from the init fragment to
-            // onboarding.
-            if (nav.currentDestination?.id == R.id.initFragment)
-                nav.navigate(actionGlobalOnboardingContainer())
-        } else {
-            // We do have a db already
-            if (viewModel.needToShowDoNotKillMeFragment) {
-                // Consult whether any condition changed that requires us to show the do-not-kill
-                // fragment (again).
-                nav.navigate(actionGlobalDoNotKillMeFragment())
-            } else {
-                // We have a db and don't need to show the do-not-kill fragment, let's start
-                // the lifecycle.
-                nav.navigate(actionGlobalStartupFragment())
-            }
-        }
-    }
-
     override fun onSaveInstanceState(outState: Bundle) {
         super.onSaveInstanceState(outState)
         outState.putBoolean(
diff --git a/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/StartupFragment.kt b/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/StartupFragment.kt
index 42ea8b19..f12dee24 100644
--- a/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/StartupFragment.kt
+++ b/mailbox-android/src/main/java/org/briarproject/mailbox/android/ui/StartupFragment.kt
@@ -27,14 +27,18 @@ import android.widget.TextView
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.activityViewModels
 import dagger.hilt.android.AndroidEntryPoint
-import kotlinx.coroutines.flow.collect
 import org.briarproject.mailbox.R
 import org.briarproject.mailbox.android.StatusManager.MailboxAppState
 import org.briarproject.mailbox.android.StatusManager.Starting
+import org.slf4j.LoggerFactory
 
 @AndroidEntryPoint
 class StartupFragment : Fragment() {
 
+    companion object {
+        private val LOG = LoggerFactory.getLogger(StartupFragment::class.java)
+    }
+
     private val viewModel: MailboxViewModel by activityViewModels()
     private lateinit var statusTextView: TextView
 
@@ -57,6 +61,7 @@ class StartupFragment : Fragment() {
     }
 
     private fun onAppStateChanged(state: MailboxAppState) {
+        LOG.info("state: ${state.javaClass.simpleName}")
         if (state is Starting) {
             statusTextView.text = state.status
         }
-- 
GitLab