Skip to content

Commit

Permalink
Merge pull request #3886 from element-hq/feature/bma/fixSendQueueCrash
Browse files Browse the repository at this point in the history
fix : protect some usages of client to avoid crashes
  • Loading branch information
ganfra authored Nov 22, 2024
2 parents e76f7fb + 0261739 commit 20674dd
Show file tree
Hide file tree
Showing 14 changed files with 156 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.encryption.RecoveryState
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.sync.SlidingSyncVersion
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
import io.element.android.libraries.preferences.api.store.EnableNativeSlidingSyncUseCase
Expand Down Expand Up @@ -102,10 +103,7 @@ class LoggedInPresenter @Inject constructor(
}
}
LoggedInEvents.CheckSlidingSyncProxyAvailability -> coroutineScope.launch {
// Force the user to log out if they were using the proxy sliding sync and it's no longer available, but native sliding sync is.
forceNativeSlidingSyncMigration = !matrixClient.isUsingNativeSlidingSync() &&
matrixClient.isNativeSlidingSyncSupported() &&
!matrixClient.isSlidingSyncProxySupported()
forceNativeSlidingSyncMigration = matrixClient.forceNativeSlidingSyncMigration().getOrDefault(false)
}
LoggedInEvents.LogoutAndMigrateToNativeSlidingSync -> coroutineScope.launch {
// Enable native sliding sync if it wasn't already the case
Expand All @@ -125,6 +123,18 @@ class LoggedInPresenter @Inject constructor(
)
}

// Force the user to log out if they were using the proxy sliding sync and it's no longer available, but native sliding sync is.
private suspend fun MatrixClient.forceNativeSlidingSyncMigration(): Result<Boolean> = runCatching {
val currentSlidingSyncVersion = currentSlidingSyncVersion().getOrThrow()
if (currentSlidingSyncVersion == SlidingSyncVersion.Proxy) {
val availableSlidingSyncVersions = availableSlidingSyncVersions().getOrThrow()
availableSlidingSyncVersions.contains(SlidingSyncVersion.Native) &&
!availableSlidingSyncVersions.contains(SlidingSyncVersion.Proxy)
} else {
false
}
}

private suspend fun ensurePusherIsRegistered(pusherRegistrationState: MutableState<AsyncData<Unit>>) {
Timber.tag(pusherTag.value).d("Ensure pusher is registered")
val currentPushProvider = pushService.getCurrentPushProvider()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.encryption.RecoveryState
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.sync.SlidingSyncVersion
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
import io.element.android.libraries.matrix.test.AN_EXCEPTION
Expand Down Expand Up @@ -501,9 +502,8 @@ class LoggedInPresenterTest {
// - The sliding sync proxy is no longer supported
// - The native sliding sync is supported
val matrixClient = FakeMatrixClient(
isUsingNativeSlidingSyncLambda = { false },
isSlidingSyncProxySupportedLambda = { false },
isNativeSlidingSyncSupportedLambda = { true },
currentSlidingSyncVersionLambda = { Result.success(SlidingSyncVersion.Proxy) },
availableSlidingSyncVersionsLambda = { Result.success(listOf(SlidingSyncVersion.Native)) },
)
val presenter = createLoggedInPresenter(matrixClient = matrixClient)
moleculeFlow(RecompositionMode.Immediate) {
Expand All @@ -521,9 +521,8 @@ class LoggedInPresenterTest {
@Test
fun `present - CheckSlidingSyncProxyAvailability will not force the migration if native sliding sync is not supported too`() = runTest {
val matrixClient = FakeMatrixClient(
isUsingNativeSlidingSyncLambda = { false },
isSlidingSyncProxySupportedLambda = { false },
isNativeSlidingSyncSupportedLambda = { false },
currentSlidingSyncVersionLambda = { Result.success(SlidingSyncVersion.Proxy) },
availableSlidingSyncVersionsLambda = { Result.success(emptyList()) },
)
val presenter = createLoggedInPresenter(matrixClient = matrixClient)
moleculeFlow(RecompositionMode.Immediate) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import io.element.android.features.roomlist.impl.search.RoomListSearchEvents
import io.element.android.features.roomlist.impl.search.RoomListSearchState
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.core.bool.orFalse
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
import io.element.android.libraries.designsystem.utils.snackbar.collectSnackbarMessageAsState
import io.element.android.libraries.featureflag.api.FeatureFlagService
Expand All @@ -51,6 +50,7 @@ import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.encryption.RecoveryState
import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.sync.SlidingSyncVersion
import io.element.android.libraries.matrix.api.timeline.ReceiptType
import io.element.android.libraries.preferences.api.store.SessionPreferencesStore
import io.element.android.libraries.push.api.notifications.NotificationCleaner
Expand Down Expand Up @@ -231,10 +231,7 @@ class RoomListPresenter @Inject constructor(
}
}
val needsSlidingSyncMigration by produceState(false) {
value = runCatching {
// Note: this can fail when the session is destroyed from another client.
client.isNativeSlidingSyncSupported() && !client.isUsingNativeSlidingSync()
}.getOrNull().orFalse()
value = client.needsSlidingSyncMigration().getOrDefault(false)
}
val securityBannerState by rememberSecurityBannerState(securityBannerDismissed, needsSlidingSyncMigration)
return when {
Expand Down Expand Up @@ -315,6 +312,19 @@ class RoomListPresenter @Inject constructor(
}
}

/**
* Checks if the user needs to migrate to a native sliding sync version.
*/
private suspend fun MatrixClient.needsSlidingSyncMigration(): Result<Boolean> = runCatching {
val currentSlidingSyncVersion = currentSlidingSyncVersion().getOrThrow()
if (currentSlidingSyncVersion != SlidingSyncVersion.Native) {
val availableSlidingSyncVersions = availableSlidingSyncVersions().getOrThrow()
availableSlidingSyncVersions.contains(SlidingSyncVersion.Native)
} else {
false
}
}

private var currentUpdateVisibleRangeJob: Job? = null
private fun CoroutineScope.updateVisibleRange(range: IntRange) {
currentUpdateVisibleRangeJob?.cancel()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import io.element.android.libraries.matrix.api.room.preview.RoomPreviewInfo
import io.element.android.libraries.matrix.api.roomdirectory.RoomDirectoryService
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.RoomSummary
import io.element.android.libraries.matrix.api.sync.SlidingSyncVersion
import io.element.android.libraries.matrix.api.sync.SyncService
import io.element.android.libraries.matrix.api.user.MatrixSearchUserResults
import io.element.android.libraries.matrix.api.user.MatrixUser
Expand Down Expand Up @@ -145,14 +146,15 @@ interface MatrixClient : Closeable {
suspend fun getUrl(url: String): Result<String>
suspend fun getRoomPreviewInfo(roomIdOrAlias: RoomIdOrAlias, serverNames: List<String>): Result<RoomPreviewInfo>

/** Returns `true` if the home server supports native sliding sync. */
suspend fun isNativeSlidingSyncSupported(): Boolean

/** Returns `true` if the home server supports sliding sync using a proxy. */
suspend fun isSlidingSyncProxySupported(): Boolean
/**
* Returns the currently used sliding sync version.
*/
suspend fun currentSlidingSyncVersion(): Result<SlidingSyncVersion>

/** Returns `true` if the current session is using native sliding sync, `false` if it's using a proxy. */
fun isUsingNativeSlidingSync(): Boolean
/**
* Returns the available sliding sync versions for the current user.
*/
suspend fun availableSlidingSyncVersions(): Result<List<SlidingSyncVersion>>

fun canDeactivateAccount(): Boolean
suspend fun deactivateAccount(password: String, eraseData: Boolean): Result<Unit>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright 2024 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only
* Please see LICENSE in the repository root for full details.
*/

package io.element.android.libraries.matrix.api.sync

sealed interface SlidingSyncVersion {
data object None : SlidingSyncVersion
data object Proxy : SlidingSyncVersion
data object Native : SlidingSyncVersion
}
Loading

0 comments on commit 20674dd

Please sign in to comment.