Skip to content

Commit

Permalink
fix: app registering many config sync jobs [WPB-10234] (#3275) (#3390)
Browse files Browse the repository at this point in the history
Co-authored-by: Yamil Medina <[email protected]>
  • Loading branch information
MohamadJaara and yamilmedina authored Aug 27, 2024
1 parent 7cdd898 commit b17cc73
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -232,8 +232,8 @@ class UserModule {

@ViewModelScoped
@Provides
fun provideCertificateRevocationListCheckWorker(userScope: UserScope): CertificateRevocationListCheckWorker =
userScope.certificateRevocationListCheckWorker
fun provideCertificateRevocationListCheckWorker(userScope: UserScope): SyncCertificateRevocationListUseCase =
userScope.syncCertificateRevocationListUseCase

@ViewModelScoped
@Provides
Expand Down
65 changes: 51 additions & 14 deletions app/src/main/kotlin/com/wire/android/ui/home/AppSyncViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
115 changes: 115 additions & 0 deletions app/src/test/kotlin/com/wire/android/ui/home/AppSyncViewModelTest.kt
Original file line number Diff line number Diff line change
@@ -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
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion kalium
Submodule kalium updated 36 files
+1 βˆ’1 .github/workflows/benchmarks-check.yml
+9 βˆ’0 logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/UserConfigRepository.kt
+1 βˆ’1 logic/src/commonMain/kotlin/com/wire/kalium/logic/data/e2ei/RevocationListChecker.kt
+3 βˆ’3 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt
+2 βˆ’2 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCase.kt
+42 βˆ’0 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCase.kt
+21 βˆ’2 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/call/CallsScope.kt
+1 βˆ’1 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/call/usecase/EndCallOnConversationChangeUseCase.kt
+16 βˆ’7 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/call/usecase/EndCallResultListener.kt
+4 βˆ’0 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/call/usecase/EndCallUseCase.kt
+42 βˆ’0 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/call/usecase/ObserveAskCallFeedbackUseCase.kt
+4 βˆ’1 ...Main/kotlin/com/wire/kalium/logic/feature/call/usecase/ObserveEndCallDueToConversationDegradationUseCase.kt
+8 βˆ’14 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/e2ei/SyncCertificateRevocationListUseCase.kt
+4 βˆ’6 ...nMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/ObserveCertificateRevocationForSelfClientUseCase.kt
+3 βˆ’36 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/featureConfig/FeatureFlagsSyncWorker.kt
+4 βˆ’4 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/MessageScope.kt
+46 βˆ’0 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/ShouldAskCallFeedbackUseCase.kt
+52 βˆ’0 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/UpdateNextTimeCallFeedbackUseCase.kt
+3 βˆ’5 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/UserScope.kt
+3 βˆ’3 logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandler.kt
+6 βˆ’8 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCaseTest.kt
+87 βˆ’0 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCaseTest.kt
+1 βˆ’1 .../src/commonTest/kotlin/com/wire/kalium/logic/feature/call/usecase/EndCallOnConversationChangeUseCaseTest.kt
+99 βˆ’58 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/call/usecase/EndCallUseCaseTest.kt
+2 βˆ’2 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/e2ei/CertificateRevocationListCheckWorkerTest.kt
+0 βˆ’52 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/featureConfig/FeatureFlagSyncWorkerTest.kt
+1 βˆ’1 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/mlsmigration/MLSMigratorTest.kt
+89 βˆ’0 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/user/ShouldAskCallFeedbackUseCaseTest.kt
+68 βˆ’0 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/user/UpdateNextTimeCallFeedbackUseCaseTest.kt
+2 βˆ’2 logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/ProtocolUpdateEventHandlerTest.kt
+0 βˆ’70 logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/CallRepositoryArrangement.kt
+18 βˆ’14 logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/repository/CallManagerArrangement.kt
+50 βˆ’1 logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/repository/CallRepositoryArrangement.kt
+10 βˆ’0 .../src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/repository/UserConfigRepositoryArrangement.kt
+105 βˆ’2 logic/src/jvmTest/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandlerTest.kt
+8 βˆ’0 persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/unread/UserConfigDAO.kt

0 comments on commit b17cc73

Please sign in to comment.