mirror of
https://github.com/bitwarden/android.git
synced 2026-08-28 22:45:25 -05:00
PM-33411: bug: Defer early navigation until lifecycle is resumed (#6638)
This commit is contained in:
+2
-2
@@ -4,8 +4,8 @@ import android.os.Parcelable
|
||||
import androidx.compose.ui.graphics.Color
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.bitwarden.ui.platform.base.BackgroundEvent
|
||||
import com.bitwarden.ui.platform.base.BaseViewModel
|
||||
import com.bitwarden.ui.platform.base.DeferredBackgroundEvent
|
||||
import com.bitwarden.ui.platform.base.util.hexToColor
|
||||
import com.bitwarden.ui.platform.components.account.model.AccountSummary
|
||||
import com.bitwarden.ui.platform.components.account.util.initials
|
||||
@@ -541,7 +541,7 @@ sealed class VaultUnlockEvent {
|
||||
/**
|
||||
* Prompts the user for biometrics unlock.
|
||||
*/
|
||||
data class PromptForBiometrics(val cipher: Cipher) : BackgroundEvent, VaultUnlockEvent()
|
||||
data class PromptForBiometrics(val cipher: Cipher) : VaultUnlockEvent(), DeferredBackgroundEvent
|
||||
|
||||
/**
|
||||
* Completes the get credentials request with an error response.
|
||||
|
||||
+1
-1
@@ -1503,7 +1503,7 @@ sealed class SearchAction {
|
||||
*/
|
||||
data class SnackbarDataReceived(
|
||||
val data: BitwardenSnackbarData,
|
||||
) : Internal(), BackgroundEvent
|
||||
) : Internal()
|
||||
|
||||
/**
|
||||
* Indicates a result for updating a cipher during the autofill-and-save process.
|
||||
|
||||
+2
-2
@@ -4,8 +4,8 @@ import androidx.annotation.DrawableRes
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.bitwarden.ui.platform.base.BackgroundEvent
|
||||
import com.bitwarden.ui.platform.base.BaseViewModel
|
||||
import com.bitwarden.ui.platform.base.DeferredBackgroundEvent
|
||||
import com.bitwarden.ui.platform.resource.BitwardenDrawable
|
||||
import com.bitwarden.ui.platform.resource.BitwardenString
|
||||
import com.bitwarden.ui.util.Text
|
||||
@@ -170,7 +170,7 @@ sealed class SettingsEvent {
|
||||
/**
|
||||
* Navigate to the account security screen.
|
||||
*/
|
||||
data object NavigateAccountSecurityShortcut : SettingsEvent(), BackgroundEvent
|
||||
data object NavigateAccountSecurityShortcut : SettingsEvent(), DeferredBackgroundEvent
|
||||
|
||||
/**
|
||||
* Navigate to the appearance screen.
|
||||
|
||||
+4
-4
@@ -2,8 +2,8 @@ package com.x8bit.bitwarden.ui.platform.feature.vaultunlockednavbar
|
||||
|
||||
import androidx.annotation.StringRes
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.bitwarden.ui.platform.base.BackgroundEvent
|
||||
import com.bitwarden.ui.platform.base.BaseViewModel
|
||||
import com.bitwarden.ui.platform.base.DeferredBackgroundEvent
|
||||
import com.bitwarden.ui.platform.resource.BitwardenString
|
||||
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.UserState
|
||||
@@ -276,10 +276,10 @@ sealed class VaultUnlockedNavBarEvent {
|
||||
}
|
||||
|
||||
/**
|
||||
* Shortcut events should to be considered [BackgroundEvent] as they are fired
|
||||
* outside of normal lifecycle aware events and should not be ignored by filter.
|
||||
* Shortcut events should to be considered [DeferredBackgroundEvent] as they are fired
|
||||
* outside normal lifecycle aware events and should not be ignored by filter.
|
||||
*/
|
||||
sealed class Shortcut : VaultUnlockedNavBarEvent(), BackgroundEvent {
|
||||
sealed class Shortcut : VaultUnlockedNavBarEvent(), DeferredBackgroundEvent {
|
||||
/**
|
||||
* Navigate to the Generator screen via a shortcut.
|
||||
*/
|
||||
|
||||
@@ -2289,7 +2289,7 @@ sealed class VaultAction {
|
||||
*/
|
||||
data class SnackbarDataReceive(
|
||||
val data: BitwardenSnackbarData,
|
||||
) : Internal(), BackgroundEvent
|
||||
) : Internal()
|
||||
|
||||
/**
|
||||
* Indicates that the flight recorder data was received.
|
||||
|
||||
+2
-2
@@ -5,8 +5,8 @@ import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.bitwarden.authenticator.data.auth.repository.AuthRepository
|
||||
import com.bitwarden.authenticator.data.platform.repository.model.BiometricsUnlockResult
|
||||
import com.bitwarden.ui.platform.base.BackgroundEvent
|
||||
import com.bitwarden.ui.platform.base.BaseViewModel
|
||||
import com.bitwarden.ui.platform.base.DeferredBackgroundEvent
|
||||
import com.bitwarden.ui.platform.resource.BitwardenString
|
||||
import com.bitwarden.ui.util.Text
|
||||
import com.bitwarden.ui.util.asText
|
||||
@@ -181,7 +181,7 @@ sealed class UnlockEvent {
|
||||
/**
|
||||
* Prompts the user for biometrics unlock.
|
||||
*/
|
||||
data class PromptForBiometrics(val cipher: Cipher) : UnlockEvent(), BackgroundEvent
|
||||
data class PromptForBiometrics(val cipher: Cipher) : UnlockEvent(), DeferredBackgroundEvent
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,8 +1,14 @@
|
||||
package com.bitwarden.ui.platform.base
|
||||
|
||||
/**
|
||||
* Almost all the events in the app involve navigation or toasts. To prevent accidentally
|
||||
* navigating to the same view twice, by default, events are ignored if the view is not currently
|
||||
* resumed. To avoid that restriction, specific events can implement [BackgroundEvent].
|
||||
* Almost all the events in the app involve navigation. To prevent accidentally navigating to the
|
||||
* same view twice, by default, events are ignored if the view is not currently resumed.
|
||||
* To avoid that restriction, specific events can implement [BackgroundEvent].
|
||||
*/
|
||||
interface BackgroundEvent
|
||||
|
||||
/**
|
||||
* This implementation of [BackgroundEvent] allows the event to not be filtered but will force it
|
||||
* to wait until the view is resumed before consuming the event.
|
||||
*/
|
||||
interface DeferredBackgroundEvent : BackgroundEvent
|
||||
|
||||
@@ -6,16 +6,21 @@ import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.compose.LocalLifecycleOwner
|
||||
import com.bitwarden.ui.platform.base.BackgroundEvent
|
||||
import com.bitwarden.ui.platform.base.BaseViewModel
|
||||
import com.bitwarden.ui.platform.base.DeferredBackgroundEvent
|
||||
import kotlinx.coroutines.flow.filter
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.flow.launchIn
|
||||
import kotlinx.coroutines.flow.onEach
|
||||
import kotlinx.coroutines.launch
|
||||
|
||||
/**
|
||||
* Convenience method for observing event flow from [BaseViewModel].
|
||||
*
|
||||
* By default, events will only be consumed when the associated screen is
|
||||
* resumed, to avoid bugs like duplicate navigation calls. To override
|
||||
* this behavior, a given event type can implement [BackgroundEvent].
|
||||
* By default, events will only be consumed when the associated screen is resumed, to avoid bugs
|
||||
* like duplicate navigation calls. To override this behavior, a given event type can implement
|
||||
* [BackgroundEvent] which will allow the event to pass through regardless of the lifecycle state.
|
||||
* Additionally, an event can implement [DeferredBackgroundEvent] which will not be filtered out
|
||||
* based on the lifecycle but will be processed when the screen resumes.
|
||||
*/
|
||||
@Composable
|
||||
fun <E> EventsEffect(
|
||||
@@ -24,12 +29,26 @@ fun <E> EventsEffect(
|
||||
handler: (E) -> Unit,
|
||||
) {
|
||||
LaunchedEffect(key1 = Unit) {
|
||||
viewModel.eventFlow
|
||||
.filter {
|
||||
it is BackgroundEvent ||
|
||||
viewModel
|
||||
.eventFlow
|
||||
.filter { event ->
|
||||
event is BackgroundEvent ||
|
||||
lifecycleOwner.currentState.isAtLeast(Lifecycle.State.RESUMED)
|
||||
}
|
||||
.onEach { handler.invoke(it) }
|
||||
.onEach { event ->
|
||||
if (event is DeferredBackgroundEvent) {
|
||||
// Defer processing this event until the screen is resumed. This is launched
|
||||
// in its own coroutine scope to allow other events to flow unimpeded.
|
||||
launch {
|
||||
lifecycleOwner
|
||||
.currentStateFlow
|
||||
.first { it.isAtLeast(Lifecycle.State.RESUMED) }
|
||||
handler(event)
|
||||
}
|
||||
} else {
|
||||
handler(event)
|
||||
}
|
||||
}
|
||||
.launchIn(this)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
package com.bitwarden.ui.platform.base.util
|
||||
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.LifecycleRegistry
|
||||
import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow
|
||||
import com.bitwarden.ui.platform.base.BackgroundEvent
|
||||
import com.bitwarden.ui.platform.base.BaseComposeTest
|
||||
import com.bitwarden.ui.platform.base.BaseViewModel
|
||||
import com.bitwarden.ui.platform.base.DeferredBackgroundEvent
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import kotlinx.coroutines.flow.MutableSharedFlow
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
class EventsEffectTest : BaseComposeTest() {
|
||||
|
||||
private val mutableEventFlow: MutableSharedFlow<TestEvent> = bufferedMutableSharedFlow()
|
||||
private val viewModel: BaseViewModel<Unit, TestEvent, Unit> = mockk {
|
||||
every { eventFlow } returns mutableEventFlow
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `events are dispatched to handler when lifecycle is RESUMED`() {
|
||||
val handledEvents = mutableListOf<TestEvent>()
|
||||
val lifecycle = createLifecycle(Lifecycle.State.RESUMED)
|
||||
|
||||
setTestContent {
|
||||
EventsEffect(
|
||||
viewModel = viewModel,
|
||||
lifecycleOwner = lifecycle,
|
||||
handler = { handledEvents.add(it) },
|
||||
)
|
||||
}
|
||||
|
||||
composeTestRule.runOnIdle {
|
||||
mutableEventFlow.tryEmit(TestEvent.Regular)
|
||||
}
|
||||
composeTestRule.waitForIdle()
|
||||
|
||||
assertEquals(listOf(TestEvent.Regular), handledEvents)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `events are not dispatched to handler when lifecycle is not RESUMED`() {
|
||||
val handledEvents = mutableListOf<TestEvent>()
|
||||
val lifecycle = createLifecycle(Lifecycle.State.STARTED)
|
||||
|
||||
setTestContent {
|
||||
EventsEffect(
|
||||
viewModel = viewModel,
|
||||
lifecycleOwner = lifecycle,
|
||||
handler = { handledEvents.add(it) },
|
||||
)
|
||||
}
|
||||
|
||||
composeTestRule.runOnIdle {
|
||||
mutableEventFlow.tryEmit(TestEvent.Regular)
|
||||
}
|
||||
composeTestRule.waitForIdle()
|
||||
|
||||
assertEquals(emptyList<TestEvent>(), handledEvents)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `BackgroundEvent is dispatched to handler even when lifecycle is not RESUMED`() {
|
||||
val handledEvents = mutableListOf<TestEvent>()
|
||||
val lifecycle = createLifecycle(Lifecycle.State.STARTED)
|
||||
|
||||
setTestContent {
|
||||
EventsEffect(
|
||||
viewModel = viewModel,
|
||||
lifecycleOwner = lifecycle,
|
||||
handler = { handledEvents.add(it) },
|
||||
)
|
||||
}
|
||||
|
||||
composeTestRule.runOnIdle {
|
||||
mutableEventFlow.tryEmit(TestEvent.Background)
|
||||
}
|
||||
composeTestRule.waitForIdle()
|
||||
|
||||
assertEquals(listOf(TestEvent.Background), handledEvents)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `DeferredBackgroundEvent is not dispatched until lifecycle transitions to RESUMED`() {
|
||||
val handledEvents = mutableListOf<TestEvent>()
|
||||
val lifecycle = createLifecycle(Lifecycle.State.STARTED)
|
||||
|
||||
setTestContent {
|
||||
EventsEffect(
|
||||
viewModel = viewModel,
|
||||
lifecycleOwner = lifecycle,
|
||||
handler = { handledEvents.add(it) },
|
||||
)
|
||||
}
|
||||
|
||||
composeTestRule.runOnIdle {
|
||||
mutableEventFlow.tryEmit(TestEvent.DeferredBackground)
|
||||
}
|
||||
composeTestRule.waitForIdle()
|
||||
|
||||
assertEquals(emptyList<TestEvent>(), handledEvents)
|
||||
|
||||
composeTestRule.runOnIdle {
|
||||
lifecycle.currentState = Lifecycle.State.RESUMED
|
||||
}
|
||||
composeTestRule.waitForIdle()
|
||||
|
||||
assertEquals(listOf(TestEvent.DeferredBackground), handledEvents)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `DeferredBackgroundEvent is dispatched immediately when lifecycle is already RESUMED`() {
|
||||
val handledEvents = mutableListOf<TestEvent>()
|
||||
val lifecycle = createLifecycle(Lifecycle.State.RESUMED)
|
||||
|
||||
setTestContent {
|
||||
EventsEffect(
|
||||
viewModel = viewModel,
|
||||
lifecycleOwner = lifecycle,
|
||||
handler = { handledEvents.add(it) },
|
||||
)
|
||||
}
|
||||
|
||||
composeTestRule.runOnIdle {
|
||||
mutableEventFlow.tryEmit(TestEvent.DeferredBackground)
|
||||
}
|
||||
composeTestRule.waitForIdle()
|
||||
|
||||
assertEquals(listOf(TestEvent.DeferredBackground), handledEvents)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `multiple events are dispatched in order when lifecycle is RESUMED`() {
|
||||
val handledEvents = mutableListOf<TestEvent>()
|
||||
val lifecycle = createLifecycle(Lifecycle.State.RESUMED)
|
||||
|
||||
setTestContent {
|
||||
EventsEffect(
|
||||
viewModel = viewModel,
|
||||
lifecycleOwner = lifecycle,
|
||||
handler = { handledEvents.add(it) },
|
||||
)
|
||||
}
|
||||
|
||||
composeTestRule.runOnIdle {
|
||||
mutableEventFlow.tryEmit(TestEvent.Regular)
|
||||
mutableEventFlow.tryEmit(TestEvent.Background)
|
||||
mutableEventFlow.tryEmit(TestEvent.DeferredBackground)
|
||||
}
|
||||
composeTestRule.waitForIdle()
|
||||
|
||||
assertEquals(
|
||||
listOf(
|
||||
TestEvent.Regular,
|
||||
TestEvent.Background,
|
||||
TestEvent.DeferredBackground,
|
||||
),
|
||||
handledEvents,
|
||||
)
|
||||
}
|
||||
|
||||
private fun createLifecycle(
|
||||
state: Lifecycle.State,
|
||||
): LifecycleRegistry = LifecycleRegistry(mockk()).apply { currentState = state }
|
||||
}
|
||||
|
||||
private sealed class TestEvent {
|
||||
data object Regular : TestEvent()
|
||||
data object Background : TestEvent(), BackgroundEvent
|
||||
data object DeferredBackground : TestEvent(), DeferredBackgroundEvent
|
||||
}
|
||||
Reference in New Issue
Block a user