From ce5094b5752fa6f037b349eac371b6131ee19be8 Mon Sep 17 00:00:00 2001 From: Yamil Medina Date: Thu, 14 Mar 2024 16:19:57 +0100 Subject: [PATCH] fix: periodic checks for ws service to start if necessary (WPB-6343) (#2773) --- .../com/wire/android/WireApplication.kt | 1 + ...rtPersistentWebsocketIfNecessaryUseCase.kt | 76 ++++++++++++++++ .../wire/android/ui/WireActivityViewModel.kt | 8 +- .../android/ui/debug/StartServiceReceiver.kt | 34 +------- .../android/workmanager/WireWorkerFactory.kt | 10 +++ .../worker/PersistentWebsocketCheckWorker.kt | 74 ++++++++++++++++ ...rsistentWebsocketIfNecessaryUseCaseTest.kt | 86 +++++++++++++++++++ .../android/ui/WireActivityViewModelTest.kt | 10 ++- 8 files changed, 266 insertions(+), 33 deletions(-) create mode 100644 app/src/main/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCase.kt create mode 100644 app/src/main/kotlin/com/wire/android/workmanager/worker/PersistentWebsocketCheckWorker.kt create mode 100644 app/src/test/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCaseTest.kt diff --git a/app/src/main/kotlin/com/wire/android/WireApplication.kt b/app/src/main/kotlin/com/wire/android/WireApplication.kt index 5554a2e561..71d41d122d 100644 --- a/app/src/main/kotlin/com/wire/android/WireApplication.kt +++ b/app/src/main/kotlin/com/wire/android/WireApplication.kt @@ -71,6 +71,7 @@ class WireApplication : Application(), Configuration.Provider { override fun getWorkManagerConfiguration(): Configuration { return Configuration.Builder() .setWorkerFactory(wireWorkerFactory.get()) + .setMinimumLoggingLevel(android.util.Log.DEBUG) .build() } diff --git a/app/src/main/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCase.kt b/app/src/main/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCase.kt new file mode 100644 index 0000000000..b2ae0514e2 --- /dev/null +++ b/app/src/main/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCase.kt @@ -0,0 +1,76 @@ +/* + * 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/. + */ +@file:Suppress("StringTemplate") + +package com.wire.android.feature + +import android.content.Context +import android.content.Intent +import android.os.Build +import com.wire.android.appLogger +import com.wire.android.services.PersistentWebSocketService +import dagger.hilt.android.qualifiers.ApplicationContext +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class StartPersistentWebsocketIfNecessaryUseCase @Inject constructor( + @ApplicationContext private val appContext: Context, + private val shouldStartPersistentWebSocketService: ShouldStartPersistentWebSocketServiceUseCase +) { + suspend operator fun invoke() { + val persistentWebSocketServiceIntent = PersistentWebSocketService.newIntent(appContext) + shouldStartPersistentWebSocketService().let { + when (it) { + is ShouldStartPersistentWebSocketServiceUseCase.Result.Failure -> { + appLogger.e("${TAG}: Failure while fetching persistent web socket status flow") + } + + is ShouldStartPersistentWebSocketServiceUseCase.Result.Success -> { + if (it.shouldStartPersistentWebSocketService) { + startForegroundService(persistentWebSocketServiceIntent) + } else { + appLogger.i("${TAG}: Stopping PersistentWebsocketService, no user with persistent web socket enabled found") + appContext.stopService(persistentWebSocketServiceIntent) + } + } + } + } + } + + private fun startForegroundService(persistentWebSocketServiceIntent: Intent) { + when { + PersistentWebSocketService.isServiceStarted -> { + appLogger.i("${TAG}: PersistentWebsocketService already started, not starting again") + } + + else -> { + appLogger.i("${TAG}: Starting PersistentWebsocketService") + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + appContext.startForegroundService(persistentWebSocketServiceIntent) + } else { + appContext.startService(persistentWebSocketServiceIntent) + } + } + } + } + + companion object { + const val TAG = "StartPersistentWebsocketIfNecessaryUseCase" + } +} diff --git a/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt index d3ce07baa9..12ba858696 100644 --- a/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt @@ -24,6 +24,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.setValue import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import androidx.work.WorkManager import com.wire.android.BuildConfig import com.wire.android.appLogger import com.wire.android.datastore.GlobalDataStore @@ -48,6 +49,8 @@ import com.wire.android.util.deeplink.DeepLinkProcessor import com.wire.android.util.deeplink.DeepLinkResult import com.wire.android.util.dispatchers.DispatcherProvider import com.wire.android.util.ui.UIText +import com.wire.android.workmanager.worker.cancelPeriodicPersistentWebsocketCheckWorker +import com.wire.android.workmanager.worker.enqueuePeriodicPersistentWebsocketCheckWorker import com.wire.kalium.logic.CoreLogic import com.wire.kalium.logic.configuration.server.ServerConfig import com.wire.kalium.logic.data.auth.AccountInfo @@ -112,7 +115,8 @@ class WireActivityViewModel @Inject constructor( private val currentScreenManager: CurrentScreenManager, private val observeScreenshotCensoringConfigUseCaseProviderFactory: ObserveScreenshotCensoringConfigUseCaseProvider.Factory, private val globalDataStore: GlobalDataStore, - private val observeIfE2EIRequiredDuringLoginUseCaseProviderFactory: ObserveIfE2EIRequiredDuringLoginUseCaseProvider.Factory + private val observeIfE2EIRequiredDuringLoginUseCaseProviderFactory: ObserveIfE2EIRequiredDuringLoginUseCaseProvider.Factory, + private val workManager: WorkManager, ) : ViewModel() { var globalAppState: GlobalAppState by mutableStateOf(GlobalAppState()) @@ -462,9 +466,11 @@ class WireActivityViewModel @Inject constructor( if (statuses.any { it.isPersistentWebSocketEnabled }) { if (!servicesManager.isPersistentWebSocketServiceRunning()) { servicesManager.startPersistentWebSocketService() + workManager.enqueuePeriodicPersistentWebsocketCheckWorker() } } else { servicesManager.stopPersistentWebSocketService() + workManager.cancelPeriodicPersistentWebsocketCheckWorker() } } } diff --git a/app/src/main/kotlin/com/wire/android/ui/debug/StartServiceReceiver.kt b/app/src/main/kotlin/com/wire/android/ui/debug/StartServiceReceiver.kt index 6b3e61470e..93dd70ce2c 100644 --- a/app/src/main/kotlin/com/wire/android/ui/debug/StartServiceReceiver.kt +++ b/app/src/main/kotlin/com/wire/android/ui/debug/StartServiceReceiver.kt @@ -21,10 +21,8 @@ package com.wire.android.ui.debug import android.content.BroadcastReceiver import android.content.Context import android.content.Intent -import android.os.Build import com.wire.android.appLogger -import com.wire.android.feature.ShouldStartPersistentWebSocketServiceUseCase -import com.wire.android.services.PersistentWebSocketService +import com.wire.android.feature.StartPersistentWebsocketIfNecessaryUseCase import com.wire.android.util.dispatchers.DispatcherProvider import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.CoroutineScope @@ -41,41 +39,15 @@ class StartServiceReceiver : BroadcastReceiver() { lateinit var dispatcherProvider: DispatcherProvider @Inject - lateinit var shouldStartPersistentWebSocketServiceUseCase: ShouldStartPersistentWebSocketServiceUseCase + lateinit var startPersistentWebSocketService: StartPersistentWebsocketIfNecessaryUseCase private val scope by lazy { CoroutineScope(SupervisorJob() + dispatcherProvider.io()) } override fun onReceive(context: Context?, intent: Intent?) { - val persistentWebSocketServiceIntent = PersistentWebSocketService.newIntent(context) appLogger.i("$TAG: onReceive called with action ${intent?.action}") - scope.launch { - shouldStartPersistentWebSocketServiceUseCase().let { - when (it) { - is ShouldStartPersistentWebSocketServiceUseCase.Result.Failure -> { - appLogger.e("$TAG: Failure while fetching persistent web socket status flow") - } - is ShouldStartPersistentWebSocketServiceUseCase.Result.Success -> { - if (it.shouldStartPersistentWebSocketService) { - if (PersistentWebSocketService.isServiceStarted) { - appLogger.i("$TAG: PersistentWebsocketService already started, not starting again") - } else { - appLogger.i("$TAG: Starting PersistentWebsocketService") - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - context?.startForegroundService(persistentWebSocketServiceIntent) - } else { - context?.startService(persistentWebSocketServiceIntent) - } - } - } else { - appLogger.i("$TAG: Stopping PersistentWebsocketService, no user with persistent web socket enabled found") - context?.stopService(persistentWebSocketServiceIntent) - } - } - } - } - } + scope.launch { startPersistentWebSocketService() } } companion object { diff --git a/app/src/main/kotlin/com/wire/android/workmanager/WireWorkerFactory.kt b/app/src/main/kotlin/com/wire/android/workmanager/WireWorkerFactory.kt index e7b64e6f32..e763092fcf 100644 --- a/app/src/main/kotlin/com/wire/android/workmanager/WireWorkerFactory.kt +++ b/app/src/main/kotlin/com/wire/android/workmanager/WireWorkerFactory.kt @@ -23,11 +23,13 @@ import androidx.work.ListenableWorker import androidx.work.WorkerFactory import androidx.work.WorkerParameters import com.wire.android.di.KaliumCoreLogic +import com.wire.android.feature.StartPersistentWebsocketIfNecessaryUseCase import com.wire.android.migration.MigrationManager import com.wire.android.notification.NotificationChannelsManager import com.wire.android.notification.WireNotificationManager import com.wire.android.workmanager.worker.MigrationWorker import com.wire.android.workmanager.worker.NotificationFetchWorker +import com.wire.android.workmanager.worker.PersistentWebsocketCheckWorker import com.wire.android.workmanager.worker.SingleUserMigrationWorker import com.wire.kalium.logic.CoreLogic import com.wire.kalium.logic.sync.WrapperWorker @@ -38,6 +40,7 @@ class WireWorkerFactory @Inject constructor( private val wireNotificationManager: WireNotificationManager, private val notificationChannelsManager: NotificationChannelsManager, private val migrationManager: MigrationManager, + private val startPersistentWebsocketIfNecessary: StartPersistentWebsocketIfNecessaryUseCase, @KaliumCoreLogic private val coreLogic: CoreLogic ) : WorkerFactory() { @@ -47,12 +50,19 @@ class WireWorkerFactory @Inject constructor( WrapperWorker::class.java.canonicalName -> WrapperWorkerFactory(coreLogic, WireForegroundNotificationDetailsProvider) .createWorker(appContext, workerClassName, workerParameters) + NotificationFetchWorker::class.java.canonicalName -> NotificationFetchWorker(appContext, workerParameters, wireNotificationManager, notificationChannelsManager) + MigrationWorker::class.java.canonicalName -> MigrationWorker(appContext, workerParameters, migrationManager, notificationChannelsManager) + SingleUserMigrationWorker::class.java.canonicalName -> SingleUserMigrationWorker(appContext, workerParameters, migrationManager, notificationChannelsManager) + + PersistentWebsocketCheckWorker::class.java.canonicalName -> + PersistentWebsocketCheckWorker(appContext, workerParameters, startPersistentWebsocketIfNecessary) + else -> null } } diff --git a/app/src/main/kotlin/com/wire/android/workmanager/worker/PersistentWebsocketCheckWorker.kt b/app/src/main/kotlin/com/wire/android/workmanager/worker/PersistentWebsocketCheckWorker.kt new file mode 100644 index 0000000000..b3d3be9a2e --- /dev/null +++ b/app/src/main/kotlin/com/wire/android/workmanager/worker/PersistentWebsocketCheckWorker.kt @@ -0,0 +1,74 @@ +/* + * 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/. + */ +@file:Suppress("StringTemplate") + +package com.wire.android.workmanager.worker + +import android.content.Context +import androidx.hilt.work.HiltWorker +import androidx.work.CoroutineWorker +import androidx.work.ExistingPeriodicWorkPolicy +import androidx.work.PeriodicWorkRequestBuilder +import androidx.work.WorkManager +import androidx.work.WorkerParameters +import com.wire.android.appLogger +import com.wire.android.feature.StartPersistentWebsocketIfNecessaryUseCase +import com.wire.android.workmanager.worker.PersistentWebsocketCheckWorker.Companion.NAME +import com.wire.android.workmanager.worker.PersistentWebsocketCheckWorker.Companion.TAG +import com.wire.android.workmanager.worker.PersistentWebsocketCheckWorker.Companion.WORK_INTERVAL +import dagger.assisted.Assisted +import dagger.assisted.AssistedInject +import kotlinx.coroutines.coroutineScope +import kotlin.time.Duration.Companion.hours +import kotlin.time.toJavaDuration + +@HiltWorker +class PersistentWebsocketCheckWorker +@AssistedInject constructor( + @Assisted private val appContext: Context, + @Assisted private val workerParams: WorkerParameters, + private val startPersistentWebsocketIfNecessary: StartPersistentWebsocketIfNecessaryUseCase +) : CoroutineWorker(appContext, workerParams) { + + override suspend fun doWork(): Result = coroutineScope { + appLogger.i("${TAG}: Starting periodic work check for persistent websocket connection") + startPersistentWebsocketIfNecessary() + Result.success() + } + + companion object { + const val NAME = "wss_check_worker" + const val TAG = "PersistentWebsocketCheckWorker" + val WORK_INTERVAL = 24.hours.toJavaDuration() + } +} + +fun WorkManager.enqueuePeriodicPersistentWebsocketCheckWorker() { + appLogger.i("${TAG}: Enqueueing periodic work for $TAG") + enqueueUniquePeriodicWork( + NAME, ExistingPeriodicWorkPolicy.CANCEL_AND_REENQUEUE, + PeriodicWorkRequestBuilder(WORK_INTERVAL) + .addTag(TAG) // adds the tag so we can cancel later all related work. + .build() + ) +} + +fun WorkManager.cancelPeriodicPersistentWebsocketCheckWorker() { + appLogger.i("${TAG}: Cancelling all periodic scheduled work for the tag $TAG") + cancelAllWorkByTag(TAG) +} diff --git a/app/src/test/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCaseTest.kt b/app/src/test/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCaseTest.kt new file mode 100644 index 0000000000..0288e4bae7 --- /dev/null +++ b/app/src/test/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCaseTest.kt @@ -0,0 +1,86 @@ +/* + * 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.feature + +import android.content.ComponentName +import android.content.Context +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test + +class StartPersistentWebsocketIfNecessaryUseCaseTest { + + @Test + fun givenShouldStartPersistentWebsocketTrue_whenInvoking_thenStartService() = + runTest { + // given + val (arrangement, sut) = Arrangement() + .withShouldStartPersistentWebsocketServiceResult(true) + .arrange() + + // when + sut.invoke() + + // then + verify(exactly = 1) { arrangement.applicationContext.startService(any()) } + } + + @Test + fun givenShouldStartPersistentWebsocketFalse_whenInvoking_thenDONTStartService() = + runTest { + // given + val (arrangement, sut) = Arrangement() + .withShouldStartPersistentWebsocketServiceResult(false) + .arrange() + + // when + sut.invoke() + + // then + verify(exactly = 0) { arrangement.applicationContext.startService(any()) } + } + + inner class Arrangement { + + @MockK + lateinit var shouldStartPersistentWebSocketServiceUseCase: ShouldStartPersistentWebSocketServiceUseCase + + @MockK + lateinit var applicationContext: Context + + init { + MockKAnnotations.init(this, relaxUnitFun = true) + every { applicationContext.startService(any()) } returns ComponentName.createRelative("dummy", "class") + every { applicationContext.stopService(any()) } returns true + } + + fun arrange() = this to StartPersistentWebsocketIfNecessaryUseCase( + applicationContext, + shouldStartPersistentWebSocketServiceUseCase + ) + + fun withShouldStartPersistentWebsocketServiceResult(shouldStart: Boolean) = apply { + coEvery { shouldStartPersistentWebSocketServiceUseCase.invoke() } returns + ShouldStartPersistentWebSocketServiceUseCase.Result.Success(shouldStart) + } + } +} diff --git a/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt index d3d858a2a8..4e3905c7ba 100644 --- a/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt @@ -21,6 +21,8 @@ package com.wire.android.ui import android.content.Intent +import androidx.work.WorkManager +import androidx.work.impl.OperationImpl import com.wire.android.config.CoroutineTestExtension import com.wire.android.config.TestDispatcherProvider import com.wire.android.config.mockUri @@ -612,6 +614,8 @@ class WireActivityViewModelTest { coEvery { globalDataStore.selectedThemeOptionFlow() } returns flowOf(ThemeOption.LIGHT) coEvery { observeIfE2EIRequiredDuringLoginUseCaseProviderFactory.create(any()).observeIfE2EIIsRequiredDuringLogin() } returns flowOf(false) + every { workManager.cancelAllWorkByTag(any()) } returns OperationImpl() + every { workManager.enqueueUniquePeriodicWork(any(), any(), any()) } returns OperationImpl() } @MockK @@ -673,6 +677,9 @@ class WireActivityViewModelTest { @MockK lateinit var globalDataStore: GlobalDataStore + @MockK + lateinit var workManager: WorkManager + @MockK(relaxed = true) lateinit var onDeepLinkResult: (DeepLinkResult) -> Unit @@ -699,7 +706,8 @@ class WireActivityViewModelTest { currentScreenManager = currentScreenManager, observeScreenshotCensoringConfigUseCaseProviderFactory = observeScreenshotCensoringConfigUseCaseProviderFactory, globalDataStore = globalDataStore, - observeIfE2EIRequiredDuringLoginUseCaseProviderFactory = observeIfE2EIRequiredDuringLoginUseCaseProviderFactory + observeIfE2EIRequiredDuringLoginUseCaseProviderFactory = observeIfE2EIRequiredDuringLoginUseCaseProviderFactory, + workManager = workManager ) }