Session falsely displayed as 'verified' with no internet connection (#2884)

* Session falsely displayed as 'verified' with no internet connection
	- Remove the need to wait for `isReady` for `SessionVerificationService.canVerifySessionFlow` to fix this.
	- Rename `SessionVerificationService.canVerifySessionFlow` to `needsSessionVerification`.
	- Make `isReady` private.
This commit is contained in:
Jorge Martin Espinosa 2024-05-21 13:41:18 +02:00 committed by GitHub
parent bce1a30249
commit 0e05a0e4ed
9 changed files with 21 additions and 31 deletions

1
changelog.d/2884.bugfix Normal file
View file

@ -0,0 +1 @@
Session falsely displayed as 'verified' with no internet connection.

View file

@ -74,7 +74,7 @@ class PreferencesRootPresenter @Inject constructor(
} }
// We should display the 'complete verification' option if the current session can be verified // We should display the 'complete verification' option if the current session can be verified
val canVerifyUserSession by sessionVerificationService.canVerifySessionFlow.collectAsState(false) val canVerifyUserSession by sessionVerificationService.needsSessionVerification.collectAsState(false)
val showSecureBackupIndicator by indicatorService.showSettingChatBackupIndicator() val showSecureBackupIndicator by indicatorService.showSettingChatBackupIndicator()

View file

@ -140,7 +140,7 @@ class RoomListPresenterTests {
}.test { }.test {
val initialState = awaitItem() val initialState = awaitItem()
assertThat(initialState.showAvatarIndicator).isTrue() assertThat(initialState.showAvatarIndicator).isTrue()
sessionVerificationService.givenCanVerifySession(false) sessionVerificationService.givenNeedsSessionVerification(false)
encryptionService.emitBackupState(BackupState.ENABLED) encryptionService.emitBackupState(BackupState.ENABLED)
val finalState = awaitItem() val finalState = awaitItem()
assertThat(finalState.showAvatarIndicator).isFalse() assertThat(finalState.showAvatarIndicator).isFalse()
@ -282,7 +282,7 @@ class RoomListPresenterTests {
roomListService = roomListService, roomListService = roomListService,
encryptionService = encryptionService, encryptionService = encryptionService,
sessionVerificationService = FakeSessionVerificationService().apply { sessionVerificationService = FakeSessionVerificationService().apply {
givenCanVerifySession(false) givenNeedsSessionVerification(false)
}, },
syncService = FakeSyncService(initialState = SyncState.Running) syncService = FakeSyncService(initialState = SyncState.Running)
) )

View file

@ -25,7 +25,6 @@ import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import com.freeletics.flowredux.compose.rememberStateAndDispatch import com.freeletics.flowredux.compose.rememberStateAndDispatch
import io.element.android.features.preferences.api.store.SessionPreferencesStore import io.element.android.features.preferences.api.store.SessionPreferencesStore
import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.AsyncData
@ -61,7 +60,7 @@ class VerifySelfSessionPresenter @Inject constructor(
val recoveryState by encryptionService.recoveryStateStateFlow.collectAsState() val recoveryState by encryptionService.recoveryStateStateFlow.collectAsState()
val stateAndDispatch = stateMachine.rememberStateAndDispatch() val stateAndDispatch = stateMachine.rememberStateAndDispatch()
val skipVerification by sessionPreferencesStore.isSessionVerificationSkipped().collectAsState(initial = false) val skipVerification by sessionPreferencesStore.isSessionVerificationSkipped().collectAsState(initial = false)
val needsVerification by sessionVerificationService.canVerifySessionFlow.collectAsState(initial = true) val needsVerification by sessionVerificationService.needsSessionVerification.collectAsState(initial = true)
val verificationFlowStep by remember { val verificationFlowStep by remember {
derivedStateOf { derivedStateOf {
when { when {

View file

@ -296,8 +296,7 @@ class VerifySelfSessionPresenterTests {
@Test @Test
fun `present - When verification is not needed, the flow is completed`() = runTest { fun `present - When verification is not needed, the flow is completed`() = runTest {
val service = FakeSessionVerificationService().apply { val service = FakeSessionVerificationService().apply {
givenCanVerifySession(false) givenNeedsSessionVerification(false)
givenIsReady(true)
givenVerifiedStatus(SessionVerifiedStatus.Verified) givenVerifiedStatus(SessionVerifiedStatus.Verified)
givenVerificationFlowState(VerificationFlowState.Finished) givenVerificationFlowState(VerificationFlowState.Finished)
} }

View file

@ -38,7 +38,7 @@ class DefaultIndicatorService @Inject constructor(
) : IndicatorService { ) : IndicatorService {
@Composable @Composable
override fun showRoomListTopBarIndicator(): State<Boolean> { override fun showRoomListTopBarIndicator(): State<Boolean> {
val canVerifySession by sessionVerificationService.canVerifySessionFlow.collectAsState(initial = false) val canVerifySession by sessionVerificationService.needsSessionVerification.collectAsState(initial = false)
val settingChatBackupIndicator = showSettingChatBackupIndicator() val settingChatBackupIndicator = showSettingChatBackupIndicator()
return remember { return remember {

View file

@ -26,12 +26,6 @@ interface SessionVerificationService {
*/ */
val verificationFlowState: StateFlow<VerificationFlowState> val verificationFlowState: StateFlow<VerificationFlowState>
/**
* The internal service that checks verification can only run after the initial sync.
* This [StateFlow] will notify consumers when the service is ready to be used.
*/
val isReady: StateFlow<Boolean>
/** /**
* Returns whether the current verification status is either: [SessionVerifiedStatus.Unknown], [SessionVerifiedStatus.NotVerified] * Returns whether the current verification status is either: [SessionVerifiedStatus.Unknown], [SessionVerifiedStatus.NotVerified]
* or [SessionVerifiedStatus.Verified]. * or [SessionVerifiedStatus.Verified].
@ -39,9 +33,9 @@ interface SessionVerificationService {
val sessionVerifiedStatus: StateFlow<SessionVerifiedStatus> val sessionVerifiedStatus: StateFlow<SessionVerifiedStatus>
/** /**
* Returns whether the current session needs to be verified and the SDK is ready to start the verification. * Returns whether the current session needs to be verified.
*/ */
val canVerifySessionFlow: Flow<Boolean> val needsSessionVerification: Flow<Boolean>
/** /**
* Request verification of the current session. * Request verification of the current session.

View file

@ -30,8 +30,8 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
@ -80,10 +80,14 @@ class RustSessionVerificationService(
private val _sessionVerifiedStatus = MutableStateFlow<SessionVerifiedStatus>(SessionVerifiedStatus.Unknown) private val _sessionVerifiedStatus = MutableStateFlow<SessionVerifiedStatus>(SessionVerifiedStatus.Unknown)
override val sessionVerifiedStatus: StateFlow<SessionVerifiedStatus> = _sessionVerifiedStatus.asStateFlow() override val sessionVerifiedStatus: StateFlow<SessionVerifiedStatus> = _sessionVerifiedStatus.asStateFlow()
override val isReady = isSyncServiceReady.stateIn(sessionCoroutineScope, SharingStarted.Eagerly, false) /**
* The internal service that checks verification can only run after the initial sync.
* This [StateFlow] will notify consumers when the service is ready to be used.
*/
private val isReady = isSyncServiceReady.stateIn(sessionCoroutineScope, SharingStarted.Eagerly, false)
override val canVerifySessionFlow = combine(sessionVerifiedStatus, isReady) { verificationStatus, isReady -> override val needsSessionVerification = sessionVerifiedStatus.map { verificationStatus ->
isReady && verificationStatus == SessionVerifiedStatus.NotVerified verificationStatus == SessionVerifiedStatus.NotVerified
} }
init { init {

View file

@ -25,17 +25,14 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
class FakeSessionVerificationService : SessionVerificationService { class FakeSessionVerificationService : SessionVerificationService {
private val _isReady = MutableStateFlow(false)
private val _sessionVerifiedStatus = MutableStateFlow<SessionVerifiedStatus>(SessionVerifiedStatus.Unknown) private val _sessionVerifiedStatus = MutableStateFlow<SessionVerifiedStatus>(SessionVerifiedStatus.Unknown)
private var _verificationFlowState = MutableStateFlow<VerificationFlowState>(VerificationFlowState.Initial) private var _verificationFlowState = MutableStateFlow<VerificationFlowState>(VerificationFlowState.Initial)
private var _canVerifySessionFlow = MutableStateFlow(true) private var _needsSessionVerification = MutableStateFlow(true)
var shouldFail = false var shouldFail = false
override val verificationFlowState: StateFlow<VerificationFlowState> = _verificationFlowState override val verificationFlowState: StateFlow<VerificationFlowState> = _verificationFlowState
override val sessionVerifiedStatus: StateFlow<SessionVerifiedStatus> = _sessionVerifiedStatus override val sessionVerifiedStatus: StateFlow<SessionVerifiedStatus> = _sessionVerifiedStatus
override val canVerifySessionFlow: Flow<Boolean> = _canVerifySessionFlow override val needsSessionVerification: Flow<Boolean> = _needsSessionVerification
override val isReady: StateFlow<Boolean> = _isReady
override suspend fun requestVerification() { override suspend fun requestVerification() {
if (!shouldFail) { if (!shouldFail) {
@ -85,12 +82,8 @@ class FakeSessionVerificationService : SessionVerificationService {
_verificationFlowState.value = state _verificationFlowState.value = state
} }
fun givenCanVerifySession(canVerify: Boolean) { fun givenNeedsSessionVerification(needsVerification: Boolean) {
_canVerifySessionFlow.value = canVerify _needsSessionVerification.value = needsVerification
}
fun givenIsReady(value: Boolean) {
_isReady.value = value
} }
override suspend fun reset() { override suspend fun reset() {