Skip to content

Commit

Permalink
fix: serverConfig and notification crashes right after user becomes i…
Browse files Browse the repository at this point in the history
…nvalid [WPB-6552] [WPB-6233] (#2684)
  • Loading branch information
saleniuk committed Feb 13, 2024
1 parent 7914307 commit ceb0052
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 20 deletions.
24 changes: 18 additions & 6 deletions app/src/main/kotlin/com/wire/android/GlobalObserversManager.kt
Expand Up @@ -91,14 +91,26 @@ class GlobalObserversManager @Inject constructor(
}

coreLogic.getGlobalScope().observeValidAccounts()
.combine(persistentStatusesFlow) { list, persistentStatuses ->
val persistentStatusesMap = persistentStatuses.associate { it.userId to it.isPersistentWebSocketEnabled }
/*
Intersect both lists as they can be slightly out of sync because both lists can be updated at slightly different times.
When user is logged out, at this time one of them can still contain this invalid user - make sure that it's ignored.
When user is logged in, at this time one of them can still not contain this new user - ignore for now,
the user will be handled correctly in the next iteration when the second list becomes updated as well.
*/
list.map { (selfUser, _) -> selfUser }
.filter { persistentStatusesMap.containsKey(it.id) }
.map { it to persistentStatusesMap.getValue(it.id) }
}
.distinctUntilChanged()
.combine(persistentStatusesFlow, ::Pair)
.collect { (list, persistentStatuses) ->
notificationChannelsManager.createUserNotificationChannels(list.map { it.first })
.collectLatest {
// create notification channels for all valid users
notificationChannelsManager.createUserNotificationChannels(it.map { it.first })

list.map { it.first.id }
// do not observe notifications for users with PersistentWebSocketEnabled, it will be done in PersistentWebSocketService
.filter { userId -> persistentStatuses.none { it.userId == userId && it.isPersistentWebSocketEnabled } }
// do not observe notifications for users with PersistentWebSocketEnabled, it will be done in PersistentWebSocketService
it.filter { (_, isPersistentWebSocketEnabled) -> !isPersistentWebSocketEnabled }
.map { (selfUser, _) -> selfUser.id }
.run {
notificationManager.observeNotificationsAndCallsWhileRunning(this, scope)
}
Expand Down
Expand Up @@ -37,12 +37,8 @@ class ObserveAppLockConfigUseCase @Inject constructor(
) {
operator fun invoke(): Flow<AppLockConfig> = channelFlow {
coreLogic.getGlobalScope().session.currentSessionFlow().collectLatest { sessionResult ->
when (sessionResult) {
is CurrentSessionResult.Failure -> {
send(AppLockConfig.Disabled(DEFAULT_APP_LOCK_TIMEOUT))
}

is CurrentSessionResult.Success -> {
when {
sessionResult is CurrentSessionResult.Success && sessionResult.accountInfo.isValid() -> {
val userId = sessionResult.accountInfo.userId
val appLockTeamFeatureConfigFlow =
coreLogic.getSessionScope(userId).appLockTeamFeatureConfigObserver
Expand All @@ -67,6 +63,10 @@ class ObserveAppLockConfigUseCase @Inject constructor(
send(it)
}
}

else -> {
send(AppLockConfig.Disabled(DEFAULT_APP_LOCK_TIMEOUT))
}
}
}
}
Expand Down
Expand Up @@ -18,6 +18,7 @@

package com.wire.android.notification

import androidx.annotation.VisibleForTesting
import com.wire.android.R
import com.wire.android.appLogger
import com.wire.android.di.KaliumCoreLogic
Expand All @@ -36,6 +37,7 @@ import com.wire.kalium.logic.data.notification.LocalNotificationMessage
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.feature.message.MarkMessagesAsNotifiedUseCase
import com.wire.kalium.logic.feature.session.CurrentSessionResult
import com.wire.kalium.logic.feature.session.DoesValidSessionExistResult
import com.wire.kalium.logic.feature.session.GetAllSessionsResult
import com.wire.kalium.logic.feature.user.E2EIRequiredResult
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -244,9 +246,8 @@ class WireNotificationManager @Inject constructor(
return
}

// start observing notifications only for new users
userIds
.filter { observingJobs.userJobs[it]?.isAllActive() != true }
// start observing notifications only for new users with valid session and without active jobs
newUsersWithValidSessionAndWithoutActiveJobs(userIds) { observingJobs.userJobs[it]?.isAllActive() == true }
.forEach { userId ->
val jobs = UserObservingJobs(
currentScreenJob = scope.launch(dispatcherProvider.default()) {
Expand All @@ -271,6 +272,20 @@ class WireNotificationManager @Inject constructor(
}
}

@VisibleForTesting
internal suspend fun newUsersWithValidSessionAndWithoutActiveJobs(
userIds: List<UserId>,
hasActiveJobs: (UserId) -> Boolean
): List<UserId> = userIds
.filter { !hasActiveJobs(it) }
.filter {
// double check if the valid session for the given user still exists
when (val result = coreLogic.getGlobalScope().doesValidSessionExist(it)) {
is DoesValidSessionExistResult.Success -> result.doesValidSessionExist
else -> false
}
}

private fun stopObservingForUser(userId: UserId, observingJobs: ObservingJobs) {
messagesNotificationManager.hideAllNotificationsForUser(userId)
observingJobs.userJobs[userId]?.cancelAll()
Expand Down
Expand Up @@ -71,8 +71,9 @@ class ProximitySensorManager @Inject constructor(
override fun onSensorChanged(event: SensorEvent) {
appCoroutineScope.launch {
coreLogic.get().globalScope {
when (val currentSession = currentSession.get().invoke()) {
is CurrentSessionResult.Success -> {
val currentSession = currentSession.get().invoke()
when {
currentSession is CurrentSessionResult.Success && currentSession.accountInfo.isValid() -> {
val userId = currentSession.accountInfo.userId
val isCallRunning = coreLogic.get().getSessionScope(userId).calls.isCallRunning()
val distance = event.values.first()
Expand All @@ -92,8 +93,10 @@ class ProximitySensorManager @Inject constructor(
}
}

else -> {
// NO SESSION - Nothing to do
else -> { // NO SESSION - just release in case it's still held
if (wakeLock.isHeld) {
wakeLock.release()
}
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions app/src/test/kotlin/com/wire/android/GlobalObserversManagerTest.kt
Expand Up @@ -146,6 +146,23 @@ class GlobalObserversManagerTest {
coVerify(exactly = 0) { arrangement.messageScope.deleteEphemeralMessageEndDate() }
}

@Test
fun `given validAccounts and persistentStatuses are out of sync, when setting up notifications, then ignore invalid users`() {
val validAccountsList = listOf(TestUser.SELF_USER)
val persistentStatusesList = listOf(
PersistentWebSocketStatus(TestUser.SELF_USER.id, false),
PersistentWebSocketStatus(TestUser.USER_ID.copy(value = "something else"), true)
)
val (arrangement, manager) = Arrangement()
.withValidAccounts(validAccountsList.map { it to null })
.withPersistentWebSocketConnectionStatuses(persistentStatusesList)
.arrange()
manager.observe()
coVerify(exactly = 1) {
arrangement.notificationChannelsManager.createUserNotificationChannels(listOf(TestUser.SELF_USER))
}
}

private class Arrangement {

@MockK
Expand Down
Expand Up @@ -22,6 +22,7 @@ import com.wire.android.datastore.GlobalDataStore
import com.wire.kalium.logic.CoreLogic
import com.wire.kalium.logic.configuration.AppLockTeamConfig
import com.wire.kalium.logic.data.auth.AccountInfo
import com.wire.kalium.logic.data.logout.LogoutReason
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.feature.UserSessionScope
import com.wire.kalium.logic.feature.applock.AppLockTeamFeatureConfigObserver
Expand Down Expand Up @@ -54,6 +55,22 @@ class ObserveAppLockConfigUseCaseTest {
}
}

@Test
fun givenInvalidSession_whenObservingAppLock_thenSendDisabledStatus() = runTest {
val (_, useCase) = Arrangement()
.withInvalidSession()
.arrange()

val result = useCase.invoke()

result.test {
val appLockStatus = awaitItem()

assertEquals(AppLockConfig.Disabled(timeout), appLockStatus)
awaitComplete()
}
}

@Test
fun givenValidSessionAndAppLockedByTeam_whenObservingAppLock_thenSendEnforcedByTeamStatus() =
runTest {
Expand Down Expand Up @@ -142,6 +159,11 @@ class ObserveAppLockConfigUseCaseTest {
flowOf(CurrentSessionResult.Failure.SessionNotFound)
}

fun withInvalidSession() = apply {
coEvery { coreLogic.getGlobalScope().session.currentSessionFlow() } returns
flowOf(CurrentSessionResult.Success(accountInfoInvalid))
}

fun withValidSession() = apply {
coEvery { coreLogic.getGlobalScope().session.currentSessionFlow() } returns
flowOf(CurrentSessionResult.Success(accountInfo))
Expand Down Expand Up @@ -177,7 +199,9 @@ class ObserveAppLockConfigUseCaseTest {
}

companion object {
private val accountInfo = AccountInfo.Valid(UserId("userId", "domain"))
private val userId = UserId("userId", "domain")
private val accountInfo = AccountInfo.Valid(userId)
private val accountInfoInvalid = AccountInfo.Invalid(userId, LogoutReason.DELETED_ACCOUNT)
private val timeout = 60.seconds
}
}
Expand Up @@ -55,6 +55,7 @@ import com.wire.kalium.logic.feature.message.MessageScope
import com.wire.kalium.logic.feature.message.Result
import com.wire.kalium.logic.feature.session.CurrentSessionFlowUseCase
import com.wire.kalium.logic.feature.session.CurrentSessionResult
import com.wire.kalium.logic.feature.session.DoesValidSessionExistResult
import com.wire.kalium.logic.feature.session.GetAllSessionsResult
import com.wire.kalium.logic.feature.session.GetSessionsUseCase
import com.wire.kalium.logic.feature.user.E2EIRequiredResult
Expand Down Expand Up @@ -83,6 +84,7 @@ import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import kotlinx.datetime.Instant
import org.amshove.kluent.internal.assertEquals
import org.junit.jupiter.api.Test
import kotlin.time.Duration.Companion.minutes

Expand Down Expand Up @@ -696,6 +698,51 @@ class WireNotificationManagerTest {
}
}

@Test
fun givenSessionExistsForTheUserAndNoActiveJobs_whenGettingUsersToObserve_thenReturnThatUser() =
runTest(dispatcherProvider.main()) {
// given
val userId = provideUserId()
val (_, manager) = Arrangement()
.withDoesValidSessionExistResult(userId, DoesValidSessionExistResult.Success(true))
.arrange()
val hasActiveJobs: (UserId) -> Boolean = { false }
// when
val result = manager.newUsersWithValidSessionAndWithoutActiveJobs(listOf(userId), hasActiveJobs)
// then
assertEquals(listOf(userId), result)
}

@Test
fun givenSessionExistsForTheUserButWithActiveJobs_whenGettingUsersToObserve_thenDoNotReturnThatUser() =
runTest(dispatcherProvider.main()) {
// given
val userId = provideUserId()
val (_, manager) = Arrangement()
.withDoesValidSessionExistResult(userId, DoesValidSessionExistResult.Success(true))
.arrange()
val hasActiveJobs: (UserId) -> Boolean = { true }
// when
val result = manager.newUsersWithValidSessionAndWithoutActiveJobs(listOf(userId), hasActiveJobs)
// then
assertEquals(listOf(), result)
}

@Test
fun givenSessionDoesNotExistForTheUserAndNoActiveJobs_whenGettingUsersToObserve_thenDoNotReturnThatUser() =
runTest(dispatcherProvider.main()) {
// given
val userId = provideUserId()
val (_, manager) = Arrangement()
.withDoesValidSessionExistResult(userId, DoesValidSessionExistResult.Success(false))
.arrange()
val hasActiveJobs: (UserId) -> Boolean = { false }
// when
val result = manager.newUsersWithValidSessionAndWithoutActiveJobs(listOf(userId), hasActiveJobs)
// then
assertEquals(listOf(), result)
}

private inner class Arrangement {
@MockK
lateinit var coreLogic: CoreLogic
Expand Down Expand Up @@ -813,6 +860,7 @@ class WireNotificationManagerTest {
every { servicesManager.startOngoingCallService() } returns Unit
every { servicesManager.stopOngoingCallService() } returns Unit
every { pingRinger.ping(any(), any()) } returns Unit
coEvery { globalKaliumScope.doesValidSessionExist.invoke(any()) } returns DoesValidSessionExistResult.Success(true)
}

private fun mockSpecificUserSession(
Expand Down Expand Up @@ -890,6 +938,10 @@ class WireNotificationManagerTest {
coEvery { observeE2EIRequired.invoke() } returns flowOf(result)
}

fun withDoesValidSessionExistResult(userId: UserId, result: DoesValidSessionExistResult) = apply {
coEvery { globalKaliumScope.doesValidSessionExist.invoke(userId) } returns result
}

fun arrange() = this to wireNotificationManager
}

Expand Down

0 comments on commit ceb0052

Please sign in to comment.