From b17cc73ad8ef84e05471d6cfc62997e210f1c507 Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Tue, 27 Aug 2024 19:25:25 +0200 Subject: [PATCH] fix: app registering many config sync jobs [WPB-10234] (#3275) (#3390) Co-authored-by: Yamil Medina --- .../android/di/accountScoped/UserModule.kt | 6 +- .../wire/android/ui/home/AppSyncViewModel.kt | 65 +++++++--- .../sync/FeatureFlagNotificationViewModel.kt | 2 +- .../android/ui/home/AppSyncViewModelTest.kt | 115 ++++++++++++++++++ .../FeatureFlagNotificationViewModelTest.kt | 4 +- kalium | 2 +- 6 files changed, 173 insertions(+), 21 deletions(-) create mode 100644 app/src/test/kotlin/com/wire/android/ui/home/AppSyncViewModelTest.kt diff --git a/app/src/main/kotlin/com/wire/android/di/accountScoped/UserModule.kt b/app/src/main/kotlin/com/wire/android/di/accountScoped/UserModule.kt index 3fa653b074f..3d4a174eae2 100644 --- a/app/src/main/kotlin/com/wire/android/di/accountScoped/UserModule.kt +++ b/app/src/main/kotlin/com/wire/android/di/accountScoped/UserModule.kt @@ -26,7 +26,7 @@ import com.wire.kalium.logic.feature.asset.GetAssetSizeLimitUseCase import com.wire.kalium.logic.feature.asset.GetAvatarAssetUseCase import com.wire.kalium.logic.feature.client.FinalizeMLSClientAfterE2EIEnrollment import com.wire.kalium.logic.feature.conversation.GetAllContactsNotInConversationUseCase -import com.wire.kalium.logic.feature.e2ei.CertificateRevocationListCheckWorker +import com.wire.kalium.logic.feature.e2ei.SyncCertificateRevocationListUseCase import com.wire.kalium.logic.feature.e2ei.usecase.GetMLSClientIdentityUseCase import com.wire.kalium.logic.feature.e2ei.usecase.GetMembersE2EICertificateStatusesUseCase import com.wire.kalium.logic.feature.e2ei.usecase.IsOtherUserE2EIVerifiedUseCase @@ -232,8 +232,8 @@ class UserModule { @ViewModelScoped @Provides - fun provideCertificateRevocationListCheckWorker(userScope: UserScope): CertificateRevocationListCheckWorker = - userScope.certificateRevocationListCheckWorker + fun provideCertificateRevocationListCheckWorker(userScope: UserScope): SyncCertificateRevocationListUseCase = + userScope.syncCertificateRevocationListUseCase @ViewModelScoped @Provides diff --git a/app/src/main/kotlin/com/wire/android/ui/home/AppSyncViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/home/AppSyncViewModel.kt index 79b2a2cbfb9..7988a39e302 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/AppSyncViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/AppSyncViewModel.kt @@ -17,33 +17,70 @@ */ package com.wire.android.ui.home -import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import com.wire.android.navigation.SavedStateViewModel -import com.wire.kalium.logic.feature.e2ei.CertificateRevocationListCheckWorker +import com.wire.android.appLogger +import com.wire.kalium.logic.feature.e2ei.SyncCertificateRevocationListUseCase import com.wire.kalium.logic.feature.e2ei.usecase.ObserveCertificateRevocationForSelfClientUseCase import com.wire.kalium.logic.feature.featureConfig.FeatureFlagsSyncWorker import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.Job +import kotlinx.coroutines.joinAll import kotlinx.coroutines.launch +import kotlinx.datetime.Clock +import kotlinx.datetime.Instant import javax.inject.Inject +import kotlin.time.Duration +import kotlin.time.Duration.Companion.minutes @HiltViewModel class AppSyncViewModel @Inject constructor( - override val savedStateHandle: SavedStateHandle, - private val certificateRevocationListCheckWorker: CertificateRevocationListCheckWorker, + private val syncCertificateRevocationListUseCase: SyncCertificateRevocationListUseCase, private val observeCertificateRevocationForSelfClient: ObserveCertificateRevocationForSelfClientUseCase, - private val featureFlagsSyncWorker: FeatureFlagsSyncWorker -) : SavedStateViewModel(savedStateHandle) { + private val featureFlagsSyncWorker: FeatureFlagsSyncWorker, +) : ViewModel() { + + private val minIntervalBetweenPulls: Duration = MIN_INTERVAL_BETWEEN_PULLS + + private var lastPullInstant: Instant? = null + private var syncDataJob: Job? = null fun startSyncingAppConfig() { - viewModelScope.launch { - certificateRevocationListCheckWorker.execute() - } - viewModelScope.launch { - observeCertificateRevocationForSelfClient.invoke() + if (isSyncing()) return + + val now = Clock.System.now() + if (isPullTooRecent(now)) return + + lastPullInstant = now + syncDataJob = viewModelScope.launch { + runSyncTasks() } - viewModelScope.launch { - featureFlagsSyncWorker.execute() + } + + private fun isSyncing(): Boolean { + return syncDataJob?.isActive == true + } + + private fun isPullTooRecent(now: Instant): Boolean { + return lastPullInstant?.let { lastPull -> + lastPull + minIntervalBetweenPulls > now + } ?: false + } + + @Suppress("TooGenericExceptionCaught") + private suspend fun runSyncTasks() { + try { + listOf( + viewModelScope.launch { syncCertificateRevocationListUseCase() }, + viewModelScope.launch { featureFlagsSyncWorker.execute() }, + viewModelScope.launch { observeCertificateRevocationForSelfClient.invoke() } + ).joinAll() + } catch (e: Exception) { + appLogger.e("Error while syncing app config", e) } } + + companion object { + val MIN_INTERVAL_BETWEEN_PULLS = 60.minutes + } } diff --git a/app/src/main/kotlin/com/wire/android/ui/home/sync/FeatureFlagNotificationViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/home/sync/FeatureFlagNotificationViewModel.kt index a136ff5ff50..b8a465f4720 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/sync/FeatureFlagNotificationViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/sync/FeatureFlagNotificationViewModel.kt @@ -215,7 +215,7 @@ class FeatureFlagNotificationViewModel @Inject constructor( } private suspend fun observeCallEndedBecauseOfConversationDegraded(userId: UserId) = - coreLogic.getSessionScope(userId).calls.observeEndCallDialog().collect { + coreLogic.getSessionScope(userId).calls.observeEndCallDueToDegradationDialog().collect { featureFlagState = featureFlagState.copy(showCallEndedBecauseOfConversationDegraded = true) } diff --git a/app/src/test/kotlin/com/wire/android/ui/home/AppSyncViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/AppSyncViewModelTest.kt new file mode 100644 index 00000000000..0d0e5e20ffc --- /dev/null +++ b/app/src/test/kotlin/com/wire/android/ui/home/AppSyncViewModelTest.kt @@ -0,0 +1,115 @@ +/* + * Wire + * Copyright (C) 2024 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.ui.home + +import com.wire.android.config.CoroutineTestExtension +import com.wire.kalium.logic.feature.e2ei.SyncCertificateRevocationListUseCase +import com.wire.kalium.logic.feature.e2ei.usecase.ObserveCertificateRevocationForSelfClientUseCase +import com.wire.kalium.logic.feature.featureConfig.FeatureFlagsSyncWorker +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.InternalCoroutinesApi +import kotlinx.coroutines.delay +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith + +@ExtendWith(CoroutineTestExtension::class) +class AppSyncViewModelTest { + @Test + fun `when startSyncingAppConfig is called then it should call the use case`() = runTest { + val (arrangement, viewModel) = Arrangement().arrange { + withObserveCertificateRevocationForSelfClient() + withFeatureFlagsSyncWorker() + withSyncCertificateRevocationListUseCase() + } + + viewModel.startSyncingAppConfig() + advanceUntilIdle() + + coVerify { arrangement.observeCertificateRevocationForSelfClient.invoke() } + coVerify { arrangement.syncCertificateRevocationListUseCase.invoke() } + coVerify { arrangement.featureFlagsSyncWorker.execute() } + } + + @Test + fun `when startSyncingAppConfig is called multiple times then it should call the use case with delay`() = runTest { + val (arrangement, viewModel) = Arrangement().arrange { + withObserveCertificateRevocationForSelfClient(1000) + withFeatureFlagsSyncWorker(1000) + withSyncCertificateRevocationListUseCase(1000) + } + + viewModel.startSyncingAppConfig() + viewModel.startSyncingAppConfig() + viewModel.startSyncingAppConfig() + advanceUntilIdle() + + coVerify(exactly = 1) { arrangement.observeCertificateRevocationForSelfClient.invoke() } + coVerify(exactly = 1) { arrangement.syncCertificateRevocationListUseCase.invoke() } + coVerify(exactly = 1) { arrangement.featureFlagsSyncWorker.execute() } + } + + private class Arrangement { + + @MockK + lateinit var syncCertificateRevocationListUseCase: SyncCertificateRevocationListUseCase + + @MockK + lateinit var observeCertificateRevocationForSelfClient: ObserveCertificateRevocationForSelfClientUseCase + + @MockK + lateinit var featureFlagsSyncWorker: FeatureFlagsSyncWorker + + init { + MockKAnnotations.init(this) + } + + private val viewModel = AppSyncViewModel( + syncCertificateRevocationListUseCase, + observeCertificateRevocationForSelfClient, + featureFlagsSyncWorker + ) + + @OptIn(InternalCoroutinesApi::class) + fun withObserveCertificateRevocationForSelfClient(delayMs: Long = 0) { + coEvery { observeCertificateRevocationForSelfClient.invoke() } coAnswers { + delay(delayMs) + } + } + + fun withSyncCertificateRevocationListUseCase(delayMs: Long = 0) { + coEvery { syncCertificateRevocationListUseCase.invoke() } coAnswers { + delay(delayMs) + } + } + + fun withFeatureFlagsSyncWorker(delayMs: Long = 0) { + coEvery { featureFlagsSyncWorker.execute() } coAnswers { + delay(delayMs) + } + } + + fun arrange(block: Arrangement.() -> Unit) = apply(block).let { + this to viewModel + } + } +} diff --git a/app/src/test/kotlin/com/wire/android/ui/home/sync/FeatureFlagNotificationViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/sync/FeatureFlagNotificationViewModelTest.kt index 7665124d460..f3dabb22641 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/sync/FeatureFlagNotificationViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/sync/FeatureFlagNotificationViewModelTest.kt @@ -346,7 +346,7 @@ class FeatureFlagNotificationViewModelTest { coEvery { coreLogic.getSessionScope(any()).observeFileSharingStatus.invoke() } returns flowOf() coEvery { coreLogic.getSessionScope(any()).observeGuestRoomLinkFeatureFlag.invoke() } returns flowOf() coEvery { coreLogic.getSessionScope(any()).observeE2EIRequired.invoke() } returns flowOf() - coEvery { coreLogic.getSessionScope(any()).calls.observeEndCallDialog() } returns flowOf() + coEvery { coreLogic.getSessionScope(any()).calls.observeEndCallDueToDegradationDialog() } returns flowOf() coEvery { coreLogic.getSessionScope(any()).observeShouldNotifyForRevokedCertificate() } returns flowOf() every { coreLogic.getSessionScope(any()).markNotifyForRevokedCertificateAsNotified } returns markNotifyForRevokedCertificateAsNotified @@ -386,7 +386,7 @@ class FeatureFlagNotificationViewModelTest { } fun withEndCallDialog() = apply { - coEvery { coreLogic.getSessionScope(any()).calls.observeEndCallDialog() } returns flowOf(Unit) + coEvery { coreLogic.getSessionScope(any()).calls.observeEndCallDueToDegradationDialog() } returns flowOf(Unit) } fun withTeamAppLockEnforce(result: AppLockTeamConfig?) = apply { diff --git a/kalium b/kalium index 147eb18545e..770f49f8e1d 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit 147eb18545ea96053ee2df076338f809d211290d +Subproject commit 770f49f8e1d9d4f5bf96c82efa86c7ba4ec79edb