AppNav: introduce a owner param so we avoid crash on AppNavigationState when switching quickly between screens
This commit is contained in:
@@ -56,10 +56,7 @@ import io.element.android.libraries.matrix.api.core.RoomId
|
|||||||
import io.element.android.libraries.matrix.ui.di.MatrixUIBindings
|
import io.element.android.libraries.matrix.ui.di.MatrixUIBindings
|
||||||
import io.element.android.services.appnavstate.api.AppNavigationStateService
|
import io.element.android.services.appnavstate.api.AppNavigationStateService
|
||||||
import kotlinx.coroutines.CoroutineScope
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.Dispatchers
|
|
||||||
import kotlinx.coroutines.runBlocking
|
|
||||||
import kotlinx.parcelize.Parcelize
|
import kotlinx.parcelize.Parcelize
|
||||||
import kotlin.coroutines.coroutineContext
|
|
||||||
|
|
||||||
@ContributesNode(AppScope::class)
|
@ContributesNode(AppScope::class)
|
||||||
class LoggedInFlowNode @AssistedInject constructor(
|
class LoggedInFlowNode @AssistedInject constructor(
|
||||||
@@ -110,17 +107,17 @@ class LoggedInFlowNode @AssistedInject constructor(
|
|||||||
val imageLoaderFactory = bindings<MatrixUIBindings>().loggedInImageLoaderFactory()
|
val imageLoaderFactory = bindings<MatrixUIBindings>().loggedInImageLoaderFactory()
|
||||||
Coil.setImageLoader(imageLoaderFactory)
|
Coil.setImageLoader(imageLoaderFactory)
|
||||||
inputs.matrixClient.startSync()
|
inputs.matrixClient.startSync()
|
||||||
appNavigationStateService.onNavigateToSession(inputs.matrixClient.sessionId)
|
appNavigationStateService.onNavigateToSession(id, inputs.matrixClient.sessionId)
|
||||||
// TODO We do not support Space yet, so directly navigate to main space
|
// TODO We do not support Space yet, so directly navigate to main space
|
||||||
appNavigationStateService.onNavigateToSpace(MAIN_SPACE)
|
appNavigationStateService.onNavigateToSpace(id, MAIN_SPACE)
|
||||||
loggedInFlowProcessor.observeEvents(coroutineScope)
|
loggedInFlowProcessor.observeEvents(coroutineScope)
|
||||||
},
|
},
|
||||||
onDestroy = {
|
onDestroy = {
|
||||||
val imageLoaderFactory = bindings<MatrixUIBindings>().notLoggedInImageLoaderFactory()
|
val imageLoaderFactory = bindings<MatrixUIBindings>().notLoggedInImageLoaderFactory()
|
||||||
Coil.setImageLoader(imageLoaderFactory)
|
Coil.setImageLoader(imageLoaderFactory)
|
||||||
plugins<LifecycleCallback>().forEach { it.onFlowReleased(inputs.matrixClient) }
|
plugins<LifecycleCallback>().forEach { it.onFlowReleased(inputs.matrixClient) }
|
||||||
appNavigationStateService.onLeavingSpace()
|
appNavigationStateService.onLeavingSpace(id)
|
||||||
appNavigationStateService.onLeavingSession()
|
appNavigationStateService.onLeavingSession(id)
|
||||||
loggedInFlowProcessor.stopObserving()
|
loggedInFlowProcessor.stopObserving()
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -82,13 +82,13 @@ class RoomFlowNode @AssistedInject constructor(
|
|||||||
onCreate = {
|
onCreate = {
|
||||||
Timber.v("OnCreate")
|
Timber.v("OnCreate")
|
||||||
plugins<LifecycleCallback>().forEach { it.onFlowCreated(inputs.room) }
|
plugins<LifecycleCallback>().forEach { it.onFlowCreated(inputs.room) }
|
||||||
appNavigationStateService.onNavigateToRoom(inputs.room.roomId)
|
appNavigationStateService.onNavigateToRoom(id, inputs.room.roomId)
|
||||||
},
|
},
|
||||||
onDestroy = {
|
onDestroy = {
|
||||||
Timber.v("OnDestroy")
|
Timber.v("OnDestroy")
|
||||||
inputs.room.close()
|
inputs.room.close()
|
||||||
plugins<LifecycleCallback>().forEach { it.onFlowReleased(inputs.room) }
|
plugins<LifecycleCallback>().forEach { it.onFlowReleased(inputs.room) }
|
||||||
appNavigationStateService.onLeavingRoom()
|
appNavigationStateService.onLeavingRoom(id)
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -21,26 +21,34 @@ import io.element.android.libraries.matrix.api.core.SessionId
|
|||||||
import io.element.android.libraries.matrix.api.core.SpaceId
|
import io.element.android.libraries.matrix.api.core.SpaceId
|
||||||
import io.element.android.libraries.matrix.api.core.ThreadId
|
import io.element.android.libraries.matrix.api.core.ThreadId
|
||||||
|
|
||||||
sealed interface AppNavigationState {
|
/**
|
||||||
object Root : AppNavigationState
|
* Can represent the current global app navigation state.
|
||||||
|
* @param owner mostly a Node identifier associated with the state.
|
||||||
|
*/
|
||||||
|
sealed class AppNavigationState(open val owner: String) {
|
||||||
|
object Root : AppNavigationState("ROOT")
|
||||||
|
|
||||||
data class Session(
|
data class Session(
|
||||||
|
override val owner: String,
|
||||||
val sessionId: SessionId,
|
val sessionId: SessionId,
|
||||||
) : AppNavigationState
|
) : AppNavigationState(owner)
|
||||||
|
|
||||||
data class Space(
|
data class Space(
|
||||||
|
override val owner: String,
|
||||||
// Can be fake value, if no space is selected
|
// Can be fake value, if no space is selected
|
||||||
val spaceId: SpaceId,
|
val spaceId: SpaceId,
|
||||||
val parentSession: Session,
|
val parentSession: Session,
|
||||||
) : AppNavigationState
|
) : AppNavigationState(owner)
|
||||||
|
|
||||||
data class Room(
|
data class Room(
|
||||||
|
override val owner: String,
|
||||||
val roomId: RoomId,
|
val roomId: RoomId,
|
||||||
val parentSpace: Space,
|
val parentSpace: Space,
|
||||||
) : AppNavigationState
|
) : AppNavigationState(owner)
|
||||||
|
|
||||||
data class Thread(
|
data class Thread(
|
||||||
|
override val owner: String,
|
||||||
val threadId: ThreadId,
|
val threadId: ThreadId,
|
||||||
val parentRoom: Room,
|
val parentRoom: Room,
|
||||||
) : AppNavigationState
|
) : AppNavigationState(owner)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -25,15 +25,15 @@ import kotlinx.coroutines.flow.StateFlow
|
|||||||
interface AppNavigationStateService {
|
interface AppNavigationStateService {
|
||||||
val appNavigationStateFlow: StateFlow<AppNavigationState>
|
val appNavigationStateFlow: StateFlow<AppNavigationState>
|
||||||
|
|
||||||
fun onNavigateToSession(sessionId: SessionId)
|
fun onNavigateToSession(owner: String, sessionId: SessionId)
|
||||||
fun onLeavingSession()
|
fun onLeavingSession(owner: String)
|
||||||
|
|
||||||
fun onNavigateToSpace(spaceId: SpaceId)
|
fun onNavigateToSpace(owner: String, spaceId: SpaceId)
|
||||||
fun onLeavingSpace()
|
fun onLeavingSpace(owner: String)
|
||||||
|
|
||||||
fun onNavigateToRoom(roomId: RoomId)
|
fun onNavigateToRoom(owner: String, roomId: RoomId)
|
||||||
fun onLeavingRoom()
|
fun onLeavingRoom(owner: String)
|
||||||
|
|
||||||
fun onNavigateToThread(threadId: ThreadId)
|
fun onNavigateToThread(owner: String, threadId: ThreadId)
|
||||||
fun onLeavingThread()
|
fun onLeavingThread(owner: String)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -40,11 +40,10 @@ private val loggerTag = LoggerTag("Navigation")
|
|||||||
@SingleIn(AppScope::class)
|
@SingleIn(AppScope::class)
|
||||||
class DefaultAppNavigationStateService @Inject constructor() : AppNavigationStateService {
|
class DefaultAppNavigationStateService @Inject constructor() : AppNavigationStateService {
|
||||||
|
|
||||||
private val currentAppNavigationState = MutableStateFlow<AppNavigationState>(AppNavigationState.Root)
|
private val currentAppNavigationState: MutableStateFlow<AppNavigationState> = MutableStateFlow(AppNavigationState.Root)
|
||||||
|
|
||||||
override val appNavigationStateFlow: StateFlow<AppNavigationState> = currentAppNavigationState
|
override val appNavigationStateFlow: StateFlow<AppNavigationState> = currentAppNavigationState
|
||||||
|
|
||||||
override fun onNavigateToSession(sessionId: SessionId) {
|
override fun onNavigateToSession(owner: String, sessionId: SessionId) {
|
||||||
val currentValue = currentAppNavigationState.value
|
val currentValue = currentAppNavigationState.value
|
||||||
Timber.tag(loggerTag.value).d("Navigating to session $sessionId. Current state: $currentValue")
|
Timber.tag(loggerTag.value).d("Navigating to session $sessionId. Current state: $currentValue")
|
||||||
val newValue: AppNavigationState.Session = when (currentValue) {
|
val newValue: AppNavigationState.Session = when (currentValue) {
|
||||||
@@ -52,53 +51,54 @@ class DefaultAppNavigationStateService @Inject constructor() : AppNavigationStat
|
|||||||
is AppNavigationState.Space,
|
is AppNavigationState.Space,
|
||||||
is AppNavigationState.Room,
|
is AppNavigationState.Room,
|
||||||
is AppNavigationState.Thread,
|
is AppNavigationState.Thread,
|
||||||
AppNavigationState.Root -> AppNavigationState.Session(sessionId)
|
is AppNavigationState.Root -> AppNavigationState.Session(owner, sessionId)
|
||||||
}
|
}
|
||||||
currentAppNavigationState.value = newValue
|
currentAppNavigationState.value = newValue
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun onNavigateToSpace(spaceId: SpaceId) {
|
override fun onNavigateToSpace(owner: String, spaceId: SpaceId) {
|
||||||
val currentValue = currentAppNavigationState.value
|
val currentValue = currentAppNavigationState.value
|
||||||
Timber.tag(loggerTag.value).d("Navigating to space $spaceId. Current state: $currentValue")
|
Timber.tag(loggerTag.value).d("Navigating to space $spaceId. Current state: $currentValue")
|
||||||
val newValue: AppNavigationState.Space = when (currentValue) {
|
val newValue: AppNavigationState.Space = when (currentValue) {
|
||||||
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
||||||
is AppNavigationState.Session -> AppNavigationState.Space(spaceId, currentValue)
|
is AppNavigationState.Session -> AppNavigationState.Space(owner, spaceId, currentValue)
|
||||||
is AppNavigationState.Space -> AppNavigationState.Space(spaceId, currentValue.parentSession)
|
is AppNavigationState.Space -> AppNavigationState.Space(owner, spaceId, currentValue.parentSession)
|
||||||
is AppNavigationState.Room -> AppNavigationState.Space(spaceId, currentValue.parentSpace.parentSession)
|
is AppNavigationState.Room -> AppNavigationState.Space(owner, spaceId, currentValue.parentSpace.parentSession)
|
||||||
is AppNavigationState.Thread -> AppNavigationState.Space(spaceId, currentValue.parentRoom.parentSpace.parentSession)
|
is AppNavigationState.Thread -> AppNavigationState.Space(owner, spaceId, currentValue.parentRoom.parentSpace.parentSession)
|
||||||
}
|
}
|
||||||
currentAppNavigationState.value = newValue
|
currentAppNavigationState.value = newValue
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun onNavigateToRoom(roomId: RoomId) {
|
override fun onNavigateToRoom(owner: String, roomId: RoomId) {
|
||||||
val currentValue = currentAppNavigationState.value
|
val currentValue = currentAppNavigationState.value
|
||||||
Timber.tag(loggerTag.value).d("Navigating to room $roomId. Current state: $currentValue")
|
Timber.tag(loggerTag.value).d("Navigating to room $roomId. Current state: $currentValue")
|
||||||
val newValue: AppNavigationState.Room = when (currentValue) {
|
val newValue: AppNavigationState.Room = when (currentValue) {
|
||||||
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
||||||
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
||||||
is AppNavigationState.Space -> AppNavigationState.Room(roomId, currentValue)
|
is AppNavigationState.Space -> AppNavigationState.Room(owner, roomId, currentValue)
|
||||||
is AppNavigationState.Room -> AppNavigationState.Room(roomId, currentValue.parentSpace)
|
is AppNavigationState.Room -> AppNavigationState.Room(owner, roomId, currentValue.parentSpace)
|
||||||
is AppNavigationState.Thread -> AppNavigationState.Room(roomId, currentValue.parentRoom.parentSpace)
|
is AppNavigationState.Thread -> AppNavigationState.Room(owner, roomId, currentValue.parentRoom.parentSpace)
|
||||||
}
|
}
|
||||||
currentAppNavigationState.value = newValue
|
currentAppNavigationState.value = newValue
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun onNavigateToThread(threadId: ThreadId) {
|
override fun onNavigateToThread(owner: String, threadId: ThreadId) {
|
||||||
val currentValue = currentAppNavigationState.value
|
val currentValue = currentAppNavigationState.value
|
||||||
Timber.tag(loggerTag.value).d("Navigating to thread $threadId. Current state: $currentValue")
|
Timber.tag(loggerTag.value).d("Navigating to thread $threadId. Current state: $currentValue")
|
||||||
val newValue: AppNavigationState.Thread = when (currentValue) {
|
val newValue: AppNavigationState.Thread = when (currentValue) {
|
||||||
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
||||||
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
||||||
is AppNavigationState.Space -> error("onNavigateToRoom() must be called first")
|
is AppNavigationState.Space -> error("onNavigateToRoom() must be called first")
|
||||||
is AppNavigationState.Room -> AppNavigationState.Thread(threadId, currentValue)
|
is AppNavigationState.Room -> AppNavigationState.Thread(owner, threadId, currentValue)
|
||||||
is AppNavigationState.Thread -> AppNavigationState.Thread(threadId, currentValue.parentRoom)
|
is AppNavigationState.Thread -> AppNavigationState.Thread(owner, threadId, currentValue.parentRoom)
|
||||||
}
|
}
|
||||||
currentAppNavigationState.value = newValue
|
currentAppNavigationState.value = newValue
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun onLeavingThread() {
|
override fun onLeavingThread(owner: String) {
|
||||||
val currentValue = currentAppNavigationState.value
|
val currentValue = currentAppNavigationState.value
|
||||||
Timber.tag(loggerTag.value).d("Leaving thread. Current state: $currentValue")
|
Timber.tag(loggerTag.value).d("Leaving thread. Current state: $currentValue")
|
||||||
|
if (!currentValue.assertOwner(owner)) return
|
||||||
val newValue: AppNavigationState.Room = when (currentValue) {
|
val newValue: AppNavigationState.Room = when (currentValue) {
|
||||||
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
||||||
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
||||||
@@ -109,9 +109,10 @@ class DefaultAppNavigationStateService @Inject constructor() : AppNavigationStat
|
|||||||
currentAppNavigationState.value = newValue
|
currentAppNavigationState.value = newValue
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun onLeavingRoom() {
|
override fun onLeavingRoom(owner: String) {
|
||||||
val currentValue = currentAppNavigationState.value
|
val currentValue = currentAppNavigationState.value
|
||||||
Timber.tag(loggerTag.value).d("Leaving room. Current state: $currentValue")
|
Timber.tag(loggerTag.value).d("Leaving room. Current state: $currentValue")
|
||||||
|
if (!currentValue.assertOwner(owner)) return
|
||||||
val newValue: AppNavigationState.Space = when (currentValue) {
|
val newValue: AppNavigationState.Space = when (currentValue) {
|
||||||
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
||||||
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
||||||
@@ -122,9 +123,10 @@ class DefaultAppNavigationStateService @Inject constructor() : AppNavigationStat
|
|||||||
currentAppNavigationState.value = newValue
|
currentAppNavigationState.value = newValue
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun onLeavingSpace() {
|
override fun onLeavingSpace(owner: String) {
|
||||||
val currentValue = currentAppNavigationState.value
|
val currentValue = currentAppNavigationState.value
|
||||||
Timber.tag(loggerTag.value).d("Leaving space. Current state: $currentValue")
|
Timber.tag(loggerTag.value).d("Leaving space. Current state: $currentValue")
|
||||||
|
if (!currentValue.assertOwner(owner)) return
|
||||||
val newValue: AppNavigationState.Session = when (currentValue) {
|
val newValue: AppNavigationState.Session = when (currentValue) {
|
||||||
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
AppNavigationState.Root -> error("onNavigateToSession() must be called first")
|
||||||
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
is AppNavigationState.Session -> error("onNavigateToSpace() must be called first")
|
||||||
@@ -135,9 +137,18 @@ class DefaultAppNavigationStateService @Inject constructor() : AppNavigationStat
|
|||||||
currentAppNavigationState.value = newValue
|
currentAppNavigationState.value = newValue
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun onLeavingSession() {
|
override fun onLeavingSession(owner: String) {
|
||||||
val currentValue = currentAppNavigationState.value
|
val currentValue = currentAppNavigationState.value
|
||||||
Timber.tag(loggerTag.value).d("Leaving session. Current state: $currentValue")
|
Timber.tag(loggerTag.value).d("Leaving session. Current state: $currentValue")
|
||||||
|
if (!currentValue.assertOwner(owner)) return
|
||||||
currentAppNavigationState.value = AppNavigationState.Root
|
currentAppNavigationState.value = AppNavigationState.Root
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun AppNavigationState.assertOwner(owner: String): Boolean {
|
||||||
|
if (this.owner != owner) {
|
||||||
|
Timber.tag(loggerTag.value).d("Can't leave current state as the owner is not the same (current = ${this.owner}, new = $owner)")
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return true
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -30,23 +30,31 @@ import kotlinx.coroutines.test.runTest
|
|||||||
import org.junit.Assert.assertThrows
|
import org.junit.Assert.assertThrows
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
|
|
||||||
|
private const val aSessionOwner = "aSessionOwner"
|
||||||
|
private const val aSpaceOwner = "aSpaceOwner"
|
||||||
|
private const val aRoomOwner = "aRoomOwner"
|
||||||
|
private const val aThreadOwner = "aThreadOwner"
|
||||||
|
|
||||||
class DefaultAppNavigationStateServiceTest {
|
class DefaultAppNavigationStateServiceTest {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testNavigation() = runTest {
|
fun testNavigation() = runTest {
|
||||||
val service = DefaultAppNavigationStateService()
|
val service = DefaultAppNavigationStateService()
|
||||||
service.onNavigateToSession(A_SESSION_ID)
|
service.onNavigateToSession(aSessionOwner, A_SESSION_ID)
|
||||||
service.onNavigateToSpace(A_SPACE_ID)
|
service.onNavigateToSpace(aSpaceOwner, A_SPACE_ID)
|
||||||
service.onNavigateToRoom(A_ROOM_ID)
|
service.onNavigateToRoom(aRoomOwner, A_ROOM_ID)
|
||||||
service.onNavigateToThread(A_THREAD_ID)
|
service.onNavigateToThread(aThreadOwner, A_THREAD_ID)
|
||||||
assertThat(service.appNavigationStateFlow.first()).isEqualTo(
|
assertThat(service.appNavigationStateFlow.first()).isEqualTo(
|
||||||
AppNavigationState.Thread(
|
AppNavigationState.Thread(
|
||||||
A_THREAD_ID,
|
aThreadOwner, A_THREAD_ID,
|
||||||
AppNavigationState.Room(
|
AppNavigationState.Room(
|
||||||
|
aRoomOwner,
|
||||||
A_ROOM_ID,
|
A_ROOM_ID,
|
||||||
AppNavigationState.Space(
|
AppNavigationState.Space(
|
||||||
|
aSpaceOwner,
|
||||||
A_SPACE_ID,
|
A_SPACE_ID,
|
||||||
AppNavigationState.Session(
|
AppNavigationState.Session(
|
||||||
|
aSessionOwner,
|
||||||
A_SESSION_ID
|
A_SESSION_ID
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
@@ -58,6 +66,6 @@ class DefaultAppNavigationStateServiceTest {
|
|||||||
@Test
|
@Test
|
||||||
fun testFailure() = runTest {
|
fun testFailure() = runTest {
|
||||||
val service = DefaultAppNavigationStateService()
|
val service = DefaultAppNavigationStateService()
|
||||||
assertThrows(IllegalStateException::class.java) { service.onNavigateToSpace(A_SPACE_ID) }
|
assertThrows(IllegalStateException::class.java) { service.onNavigateToSpace(aSpaceOwner, A_SPACE_ID) }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user