From 3f2413bc95e72381c89e219c24cc1d5377d6ba92 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Tue, 21 May 2024 13:41:18 +0200 Subject: [PATCH] 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. --- changelog.d/2884.bugfix | 1 + .../impl/root/PreferencesRootPresenter.kt | 2 +- .../roomlist/impl/RoomListPresenterTests.kt | 4 ++-- .../impl/VerifySelfSessionPresenter.kt | 3 +-- .../impl/VerifySelfSessionPresenterTests.kt | 3 +-- .../indicator/impl/DefaultIndicatorService.kt | 2 +- .../verification/SessionVerificationService.kt | 10 ++-------- .../RustSessionVerificationService.kt | 12 ++++++++---- .../FakeSessionVerificationService.kt | 15 ++++----------- 9 files changed, 21 insertions(+), 31 deletions(-) create mode 100644 changelog.d/2884.bugfix diff --git a/changelog.d/2884.bugfix b/changelog.d/2884.bugfix new file mode 100644 index 0000000000..f2efefe69e --- /dev/null +++ b/changelog.d/2884.bugfix @@ -0,0 +1 @@ +Session falsely displayed as 'verified' with no internet connection. diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt index 20c28427a8..86992d4ac7 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt @@ -74,7 +74,7 @@ class PreferencesRootPresenter @Inject constructor( } // 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() diff --git a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt index 053b4e51a8..e682e6608d 100644 --- a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt +++ b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt @@ -140,7 +140,7 @@ class RoomListPresenterTests { }.test { val initialState = awaitItem() assertThat(initialState.showAvatarIndicator).isTrue() - sessionVerificationService.givenCanVerifySession(false) + sessionVerificationService.givenNeedsSessionVerification(false) encryptionService.emitBackupState(BackupState.ENABLED) val finalState = awaitItem() assertThat(finalState.showAvatarIndicator).isFalse() @@ -282,7 +282,7 @@ class RoomListPresenterTests { roomListService = roomListService, encryptionService = encryptionService, sessionVerificationService = FakeSessionVerificationService().apply { - givenCanVerifySession(false) + givenNeedsSessionVerification(false) }, syncService = FakeSyncService(initialState = SyncState.Running) ) diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt index b31cbd0162..ed7342eb60 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt @@ -25,7 +25,6 @@ import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope -import androidx.compose.runtime.setValue import com.freeletics.flowredux.compose.rememberStateAndDispatch import io.element.android.features.preferences.api.store.SessionPreferencesStore import io.element.android.libraries.architecture.AsyncData @@ -61,7 +60,7 @@ class VerifySelfSessionPresenter @Inject constructor( val recoveryState by encryptionService.recoveryStateStateFlow.collectAsState() val stateAndDispatch = stateMachine.rememberStateAndDispatch() 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 { derivedStateOf { when { diff --git a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTests.kt b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTests.kt index 3dd391da16..1a27891bef 100644 --- a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTests.kt +++ b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTests.kt @@ -296,8 +296,7 @@ class VerifySelfSessionPresenterTests { @Test fun `present - When verification is not needed, the flow is completed`() = runTest { val service = FakeSessionVerificationService().apply { - givenCanVerifySession(false) - givenIsReady(true) + givenNeedsSessionVerification(false) givenVerifiedStatus(SessionVerifiedStatus.Verified) givenVerificationFlowState(VerificationFlowState.Finished) } diff --git a/libraries/indicator/impl/src/main/kotlin/io/element/android/libraries/indicator/impl/DefaultIndicatorService.kt b/libraries/indicator/impl/src/main/kotlin/io/element/android/libraries/indicator/impl/DefaultIndicatorService.kt index aae0f1f913..6b72b31d19 100644 --- a/libraries/indicator/impl/src/main/kotlin/io/element/android/libraries/indicator/impl/DefaultIndicatorService.kt +++ b/libraries/indicator/impl/src/main/kotlin/io/element/android/libraries/indicator/impl/DefaultIndicatorService.kt @@ -38,7 +38,7 @@ class DefaultIndicatorService @Inject constructor( ) : IndicatorService { @Composable override fun showRoomListTopBarIndicator(): State { - val canVerifySession by sessionVerificationService.canVerifySessionFlow.collectAsState(initial = false) + val canVerifySession by sessionVerificationService.needsSessionVerification.collectAsState(initial = false) val settingChatBackupIndicator = showSettingChatBackupIndicator() return remember { diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/verification/SessionVerificationService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/verification/SessionVerificationService.kt index 8d22fc174e..01687d754a 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/verification/SessionVerificationService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/verification/SessionVerificationService.kt @@ -26,12 +26,6 @@ interface SessionVerificationService { */ val verificationFlowState: StateFlow - /** - * 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 - /** * Returns whether the current verification status is either: [SessionVerifiedStatus.Unknown], [SessionVerifiedStatus.NotVerified] * or [SessionVerifiedStatus.Verified]. @@ -39,9 +33,9 @@ interface SessionVerificationService { val sessionVerifiedStatus: StateFlow /** - * 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 + val needsSessionVerification: Flow /** * Request verification of the current session. diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt index 88403e8928..5b6d960d09 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt @@ -30,8 +30,8 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -80,10 +80,14 @@ class RustSessionVerificationService( private val _sessionVerifiedStatus = MutableStateFlow(SessionVerifiedStatus.Unknown) override val sessionVerifiedStatus: StateFlow = _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 -> - isReady && verificationStatus == SessionVerifiedStatus.NotVerified + override val needsSessionVerification = sessionVerifiedStatus.map { verificationStatus -> + verificationStatus == SessionVerifiedStatus.NotVerified } init { diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/verification/FakeSessionVerificationService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/verification/FakeSessionVerificationService.kt index cde77d7c61..780758acbe 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/verification/FakeSessionVerificationService.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/verification/FakeSessionVerificationService.kt @@ -25,17 +25,14 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow class FakeSessionVerificationService : SessionVerificationService { - private val _isReady = MutableStateFlow(false) private val _sessionVerifiedStatus = MutableStateFlow(SessionVerifiedStatus.Unknown) private var _verificationFlowState = MutableStateFlow(VerificationFlowState.Initial) - private var _canVerifySessionFlow = MutableStateFlow(true) + private var _needsSessionVerification = MutableStateFlow(true) var shouldFail = false override val verificationFlowState: StateFlow = _verificationFlowState override val sessionVerifiedStatus: StateFlow = _sessionVerifiedStatus - override val canVerifySessionFlow: Flow = _canVerifySessionFlow - - override val isReady: StateFlow = _isReady + override val needsSessionVerification: Flow = _needsSessionVerification override suspend fun requestVerification() { if (!shouldFail) { @@ -85,12 +82,8 @@ class FakeSessionVerificationService : SessionVerificationService { _verificationFlowState.value = state } - fun givenCanVerifySession(canVerify: Boolean) { - _canVerifySessionFlow.value = canVerify - } - - fun givenIsReady(value: Boolean) { - _isReady.value = value + fun givenNeedsSessionVerification(needsVerification: Boolean) { + _needsSessionVerification.value = needsVerification } override suspend fun reset() {